Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Reland (x2) Skwasm overlay optimizations #56067

Merged
merged 6 commits into from
Oct 25, 2024

Conversation

eyebrowsoffire
Copy link
Contributor

This is an attempt to reland #55468

We now defer actually drawing the pictures until the build of the layer slices, so that we can calculate an accurate cullRect for the picture recorder

This also contains a further optimization for the "simple" rendering case (no platform views) where we don't actually do any unnecessary occlusion calculations.

@github-actions github-actions bot added the platform-web Code specifically for the web engine label Oct 23, 2024
Copy link
Contributor

@harryterkelsen harryterkelsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nits. Also I suggest adding a regression test for the case that caused the original PR to be reverted


LayerSlice buildWithOperation(LayerOperation operation) {
final ui.Rect recorderRect = operation.mapRect(cullRect ?? ui.Rect.zero);
final (recorder, canvas) = debugRecorderFactory != null ? debugRecorderFactory!(recorderRect) : defaultRecorderFactory(recorderRect);
Copy link
Contributor

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

@@ -16,7 +16,7 @@ void main() {

void testMain() {
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

final SceneSlice slice = sceneSlices[sliceIndex];
slice.platformViewOcclusionMap.addRect(globalPlatformViewRect);
print('placed platform view. localRect: $rect globalRect: $globalPlatformViewRect sliceIndex: $sliceIndex');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete

Copy link
Contributor

@harryterkelsen harryterkelsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@eyebrowsoffire eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 25, 2024
@auto-submit auto-submit bot merged commit 33f9064 into flutter:main Oct 25, 2024
30 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 25, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 26, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 26, 2024
…157662)

flutter/engine@7c5c5fe...c9b8ac9

2024-10-26 skia-flutter-autoroll@skia.org Roll Skia from da6c17329e0b to cadf2538dcde (3 revisions) (flutter/engine#56147)
2024-10-25 30870216+gaaclarke@users.noreply.github.com Removed clamping from dithering (flutter/engine#56140)
2024-10-25 jacksongardner@google.com Reland (x2) Skwasm overlay optimizations (flutter/engine#56067)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC codefu@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
nick9822 pushed a commit to nick9822/flutter that referenced this pull request Dec 18, 2024
This is an attempt to reland flutter/engine#55468

We now defer actually drawing the pictures until the build of the layer slices, so that we can calculate an accurate `cullRect` for the picture recorder

This also contains a further optimization for the "simple" rendering case (no platform views) where we don't actually do any unnecessary occlusion calculations.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants