This repository was archived by the owner on Feb 25, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6k
Reland (x2) Skwasm overlay optimizations #56067
Merged
auto-submit
merged 6 commits into
flutter:main
from
eyebrowsoffire:reland_x2_skwasm_placement
Oct 25, 2024
Merged
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
13ae52d
Reapply "Reland [skwasm] Scene builder optimizations for platform vie…
eyebrowsoffire a988504
Explicitly size the picture recorder.
eyebrowsoffire 8216982
Optimize simple rendering case.
eyebrowsoffire cf7531c
Fix unit test.
eyebrowsoffire 20208b0
Addressed Harry's comments.
eyebrowsoffire f4f75b4
Merge branch 'main' into reland_x2_skwasm_placement
eyebrowsoffire File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -172,6 +172,10 @@ class EngineSceneBuilder implements ui.SceneBuilder { | |
|
|
||
| final List<SceneSlice> sceneSlices = <SceneSlice>[SceneSlice()]; | ||
|
|
||
| // This represents the simplest case with no platform views, which is a fast path | ||
| // that allows us to avoid work tracking the pictures themselves. | ||
| bool _isSimple = true; | ||
|
|
||
| @override | ||
| void addPerformanceOverlay(int enabledOptions, ui.Rect bounds) { | ||
| // We don't plan to implement this on the web. | ||
|
|
@@ -185,7 +189,7 @@ class EngineSceneBuilder implements ui.SceneBuilder { | |
| bool isComplexHint = false, | ||
| bool willChangeHint = false | ||
| }) { | ||
| final int sliceIndex = _placePicture(offset, picture as ScenePicture); | ||
| final int sliceIndex = _placePicture(offset, picture as ScenePicture, currentBuilder.globalPlatformViewStyling); | ||
| currentBuilder.addPicture( | ||
| offset, | ||
| picture, | ||
|
|
@@ -200,9 +204,14 @@ class EngineSceneBuilder implements ui.SceneBuilder { | |
| // in the slice or it intersects with a platform view in the preceding slice. If the | ||
| // picture intersects with a platform view in the last slice, a new slice is added at | ||
| // the end and the picture goes in there. | ||
| int _placePicture(ui.Offset offset, ScenePicture picture) { | ||
| int _placePicture(ui.Offset offset, ScenePicture picture, PlatformViewStyling styling) { | ||
| if (_isSimple) { | ||
| // This is the fast path where there are no platform views. The picture should | ||
| // just be placed on the bottom (and only) slice. | ||
| return 0; | ||
| } | ||
| final ui.Rect cullRect = picture.cullRect.shift(offset); | ||
| final ui.Rect mappedCullRect = currentBuilder.globalPlatformViewStyling.mapLocalToGlobal(cullRect); | ||
| final ui.Rect mappedCullRect = styling.mapLocalToGlobal(cullRect); | ||
| int sliceIndex = sceneSlices.length; | ||
| while (sliceIndex > 0) { | ||
| final SceneSlice sliceBelow = sceneSlices[sliceIndex - 1]; | ||
|
|
@@ -214,6 +223,11 @@ class EngineSceneBuilder implements ui.SceneBuilder { | |
| break; | ||
| } | ||
| } | ||
| if (sliceIndex == 0) { | ||
| // Don't bother to populate the lowest occlusion map with pictures, since | ||
| // we never hit test against pictures in the bottom slice. | ||
| return sliceIndex; | ||
| } | ||
| if (sliceIndex == sceneSlices.length) { | ||
| // Insert a new slice. | ||
| sceneSlices.add(SceneSlice()); | ||
|
|
@@ -231,7 +245,7 @@ class EngineSceneBuilder implements ui.SceneBuilder { | |
| double height = 0.0 | ||
| }) { | ||
| final ui.Rect platformViewRect = ui.Rect.fromLTWH(offset.dx, offset.dy, width, height); | ||
| final int sliceIndex = _placePlatformView(viewId, platformViewRect); | ||
| final int sliceIndex = _placePlatformView(viewId, platformViewRect, currentBuilder.globalPlatformViewStyling); | ||
| currentBuilder.addPlatformView( | ||
| viewId, | ||
| bounds: platformViewRect, | ||
|
|
@@ -246,11 +260,13 @@ class EngineSceneBuilder implements ui.SceneBuilder { | |
| // or a platform view. | ||
| int _placePlatformView( | ||
| int viewId, | ||
| ui.Rect rect, { | ||
| PlatformViewStyling styling = const PlatformViewStyling(), | ||
| }) { | ||
| final PlatformViewStyling combinedStyling = PlatformViewStyling.combine(currentBuilder.globalPlatformViewStyling, styling); | ||
| final ui.Rect globalPlatformViewRect = combinedStyling.mapLocalToGlobal(rect); | ||
| ui.Rect rect, | ||
| PlatformViewStyling styling, | ||
| ) { | ||
| // Once we add a platform view, we actually have to do proper occlusion tracking. | ||
| _isSimple = false; | ||
|
|
||
| final ui.Rect globalPlatformViewRect = styling.mapLocalToGlobal(rect); | ||
| int sliceIndex = sceneSlices.length - 1; | ||
| while (sliceIndex > 0) { | ||
| final SceneSlice slice = sceneSlices[sliceIndex]; | ||
|
|
@@ -260,36 +276,44 @@ class EngineSceneBuilder implements ui.SceneBuilder { | |
| } | ||
| sliceIndex--; | ||
| } | ||
| sliceIndex = 0; | ||
| final SceneSlice slice = sceneSlices[sliceIndex]; | ||
| slice.platformViewOcclusionMap.addRect(globalPlatformViewRect); | ||
| print('placed platform view. localRect: $rect globalRect: $globalPlatformViewRect sliceIndex: $sliceIndex'); | ||
|
||
| return sliceIndex; | ||
| } | ||
|
|
||
| @override | ||
| void addRetained(ui.EngineLayer retainedLayer) { | ||
| final PictureEngineLayer placedEngineLayer = _placeRetainedLayer(retainedLayer as PictureEngineLayer); | ||
| final PictureEngineLayer placedEngineLayer = _placeRetainedLayer(retainedLayer as PictureEngineLayer, currentBuilder.globalPlatformViewStyling); | ||
| currentBuilder.mergeLayer(placedEngineLayer); | ||
| } | ||
|
|
||
| PictureEngineLayer _placeRetainedLayer(PictureEngineLayer retainedLayer) { | ||
| PictureEngineLayer _placeRetainedLayer(PictureEngineLayer retainedLayer, PlatformViewStyling styling) { | ||
| if (_isSimple && retainedLayer.isSimple) { | ||
| // There are no platform views, so we don't need to do any occlusion tracking | ||
| // and can simply merge the layer. | ||
| return retainedLayer; | ||
| } | ||
| bool needsRebuild = false; | ||
| final List<LayerDrawCommand> revisedDrawCommands = []; | ||
| final PlatformViewStyling combinedStyling = PlatformViewStyling.combine(styling, retainedLayer.platformViewStyling); | ||
| for (final LayerDrawCommand command in retainedLayer.drawCommands) { | ||
| switch (command) { | ||
| case PictureDrawCommand(offset: final ui.Offset offset, picture: final ScenePicture picture): | ||
| final int sliceIndex = _placePicture(offset, picture); | ||
| final int sliceIndex = _placePicture(offset, picture, combinedStyling); | ||
| if (command.sliceIndex != sliceIndex) { | ||
| needsRebuild = true; | ||
| } | ||
| revisedDrawCommands.add(PictureDrawCommand(offset, picture, sliceIndex)); | ||
| case PlatformViewDrawCommand(viewId: final int viewId, bounds: final ui.Rect bounds): | ||
| final int sliceIndex = _placePlatformView(viewId, bounds); | ||
| final int sliceIndex = _placePlatformView(viewId, bounds, combinedStyling); | ||
| if (command.sliceIndex != sliceIndex) { | ||
| needsRebuild = true; | ||
| } | ||
| revisedDrawCommands.add(PlatformViewDrawCommand(viewId, bounds, sliceIndex)); | ||
| case RetainedLayerDrawCommand(layer: final PictureEngineLayer sublayer): | ||
| final PictureEngineLayer revisedSublayer = _placeRetainedLayer(sublayer); | ||
| final PictureEngineLayer revisedSublayer = _placeRetainedLayer(sublayer, combinedStyling); | ||
| if (sublayer != revisedSublayer) { | ||
| needsRebuild = true; | ||
| } | ||
|
|
@@ -336,7 +360,7 @@ class EngineSceneBuilder implements ui.SceneBuilder { | |
| ui.BackdropFilterEngineLayer pushBackdropFilter( | ||
| ui.ImageFilter filter, { | ||
| ui.BlendMode blendMode = ui.BlendMode.srcOver, | ||
| ui.BackdropFilterEngineLayer? oldLayer | ||
| ui.BackdropFilterEngineLayer? oldLayer, | ||
| int? backdropId, | ||
| }) => pushLayer<BackdropFilterLayer>(BackdropFilterLayer(BackdropFilterOperation(filter, blendMode))); | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ void main() { | |
|
|
||
| void testMain() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add a regression test for the bug which caused you to have to revert the original PR? |
||
| setUpAll(() { | ||
| LayerSliceBuilder.debugRecorderFactory = () { | ||
| LayerSliceBuilder.debugRecorderFactory = (ui.Rect rect) { | ||
| final StubSceneCanvas canvas = StubSceneCanvas(); | ||
| final StubPictureRecorder recorder = StubPictureRecorder(canvas); | ||
| return (recorder, canvas); | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe worth pulling the logic to create a recorder (using either the debug factory or the default factory) into its own method