Raster cache should preserve RTree for overlay layers #42552

knopp · 2023-06-04T16:02:52Z

When raster cache paints to a layer over platform view, it will draw the individual rectangles from the content RTree, to ensure that the destination layer RTree is not clobbered. This is necessary for raster cache to not break unobstructed platform views and/or hit testing.

When raster cache paints in the root (bottom-most) layer the behavior it just copies the entire image as it did before. This is to minimise risk of performance regressions.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

knopp · 2023-06-04T16:22:06Z

cc @cyanglaz as this potentially affects unobstructed platform views on iOS.

zanderso · 2023-06-08T20:12:39Z

From PR review triage: It looks like this is ready for a review from @flar.

flar · 2023-06-12T21:23:34Z

Layer trees should never be rendered from cache with a PlatformView present (see LayerRasterCacheItem::PrerollFinalize). DisplayList layers will already have an rtree. Does that simplify the implementation here?

flow/raster_cache.cc

flar

Nothing really wrong in this review, just a couple of options for efficiency and a naming issue.

I have no idea if we should care about checkerboards in this stuff, so I'll leave the answer to that one to you. I will point out that checkerboards are a debugging tool, so performance isn't an issue, but would a developer want to have them on, but still have more precise interaction with their performance overlays?

flow/layers/display_list_layer_unittests.cc

flow/raster_cache.cc

flow/raster_cache.h

flow/raster_cache.cc

knopp · 2023-06-14T19:13:06Z

I rebased my other branch on top of this and it seems to have regressed. Setting to WIP until I figure out why.

knopp · 2023-06-14T20:01:25Z

Fixed by properly transforming local r-tree coordinates to screen space. I also updated the test to use a non-identity transform to catch this.

flar

Looks good to go except for some typos in one of the comments.

flow/raster_cache.h

…rects

…ter/engine#42552)

…128959) flutter/engine@9934c0d...48e0b4e 2023-06-15 [email protected] Roll Skia from 0b66c6928dcf to 2d531d020c26 (3 revisions) (flutter/engine#42883) 2023-06-15 [email protected] Raster cache should preserve RTree for overlay layers (flutter/engine#42552) 2023-06-15 [email protected] Roll Skia from c0c74b433117 to 0b66c6928dcf (1 revision) (flutter/engine#42879) 2023-06-15 [email protected] [Linux] Allow BasicMessageChannel sending and responding to null message (flutter/engine#42808) 2023-06-15 [email protected] Roll Skia from 12375fb6f3c8 to c0c74b433117 (1 revision) (flutter/engine#42876) 2023-06-15 [email protected] Roll Skia from d62221bd33a6 to 12375fb6f3c8 (1 revision) (flutter/engine#42873) 2023-06-15 [email protected] Roll Skia from e2e0256d4c6a to d62221bd33a6 (1 revision) (flutter/engine#42871) 2023-06-15 [email protected] Roll Skia from c3abd540c7f9 to e2e0256d4c6a (1 revision) (flutter/engine#42869) 2023-06-15 [email protected] Roll Fuchsia Linux SDK from uvmDF7KM34dWGdsuK... to 53EjCyuRu91oFTBf2... (flutter/engine#42868) 2023-06-15 [email protected] Roll Fuchsia Mac SDK from h3-8RUVrC889UXou7... to P7QA6bfO_Ij5dre7B... (flutter/engine#42867) 2023-06-15 [email protected] Roll Skia from 2718866006d2 to c3abd540c7f9 (1 revision) (flutter/engine#42866) 2023-06-15 [email protected] Roll Skia from 19051bc5fc90 to 2718866006d2 (33 revisions) (flutter/engine#42864) 2023-06-15 [email protected] Roll Dart SDK from 922c315b2c34 to 8eaed3382237 (1 revision) (flutter/engine#42862) 2023-06-15 [email protected] Add missing artifact to the android_arm64_profile config. (flutter/engine#42858) 2023-06-14 [email protected] Build skia with expat (flutter/engine#42859) 2023-06-14 [email protected] [ios] use interfaceOrientation orientation on iOS 13 and above (flutter/engine#42846) 2023-06-14 [email protected] Manual roll Dart SDK from f1387834bfd9 to 922c315b2c34 (4 revisions) (flutter/engine#42855) 2023-06-14 [email protected] [Impeller] Make interleaved layout (more) explicit in generated headers. (flutter/engine#42628) 2023-06-14 [email protected] Renamed validation layers build (flutter/engine#42826) 2023-06-14 [email protected] [ios] view controller based status bar (flutter/engine#42643) 2023-06-14 [email protected] Roll Skia from 6d5dc31d88e2 to 19051bc5fc90 (25 revisions) (flutter/engine#42828) 2023-06-14 [email protected] Roll Fuchsia Linux SDK from Xi3c5nti2LKnEOqYt... to uvmDF7KM34dWGdsuK... (flutter/engine#42842) 2023-06-14 [email protected] Fix generateLockfiles running directory for documentation (flutter/engine#42734) 2023-06-14 [email protected] Roll Fuchsia Mac SDK from Cld7-rm6ZmCOO8j-K... to h3-8RUVrC889UXou7... (flutter/engine#42839) 2023-06-14 [email protected] Roll ANGLE from 7e075469ff02 to 3a3a3c655a96 (8 revisions) (flutter/engine#42834) 2023-06-14 [email protected] Roll Dart SDK from c4e9794df8af to f1387834bfd9 (1 revision) (flutter/engine#42833) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from Xi3c5nti2LKn to 53EjCyuRu91o fuchsia/sdk/core/mac-amd64 from Cld7-rm6ZmCO to P7QA6bfO_Ij5 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 [email protected],[email protected],[email protected] 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

knopp requested review from flar and jonahwilliams June 4, 2023 16:21

knopp mentioned this pull request Jun 4, 2023

Support PlatformViews on Windows using DirectComposition flutter/flutter#108486

Open

flar reviewed Jun 12, 2023

View reviewed changes

flow/raster_cache.cc Outdated Show resolved Hide resolved

flow/raster_cache.cc Show resolved Hide resolved

knopp force-pushed the raster_cache_preserve_rtree branch from 66a2dcb to e95b027 Compare June 13, 2023 15:57

knopp mentioned this pull request Jun 13, 2023

[DisplayList] consider caching the DlRegion inside DlRTree flutter/flutter#128822

Closed

flar reviewed Jun 13, 2023

View reviewed changes

flar reviewed Jun 14, 2023

View reviewed changes

flow/raster_cache.cc Show resolved Hide resolved

knopp changed the title ~~Raster cache should preserve RTree for overlay layers~~ WIP: Raster cache should preserve RTree for overlay layers Jun 14, 2023

knopp changed the title ~~WIP: Raster cache should preserve RTree for overlay layers~~ Raster cache should preserve RTree for overlay layers Jun 14, 2023

flar approved these changes Jun 15, 2023

View reviewed changes

flow/raster_cache.h Outdated Show resolved Hide resolved

flow/raster_cache.h Outdated Show resolved Hide resolved

knopp added 8 commits June 15, 2023 15:45

Raster cache should preserve RTree for overlay layers

67d9077

Reuse existing RTree from DisplayListLayer

78218b4

Add DlRTree::bounds()

3814a21

Use DlRTree::bounds() and translate the canvas instead of individual …

def7e39

…rects

Missing asserts in unit test

1b9952a

Rename argument name and paint context flag

1b7813c

Properly transform local R-Tree coordiates to screen space

f3af18a

Fix comment

dc7e36d

knopp force-pushed the raster_cache_preserve_rtree branch from 9bf2b8f to dc7e36d Compare June 15, 2023 13:46

knopp merged commit d6e96c0 into flutter:main Jun 15, 2023

knopp deleted the raster_cache_preserve_rtree branch June 15, 2023 15:25

engine-flutter-autoroll mentioned this pull request Jun 15, 2023

Roll Flutter Engine from 9934c0de738c to 48e0b4e66422 (26 revisions) flutter/flutter#128959

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 15, 2023

d6e96c055 Raster cache should preserve RTree for overlay layers (flut…

6323e96

…ter/engine#42552)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Raster cache should preserve RTree for overlay layers #42552

Raster cache should preserve RTree for overlay layers #42552

Uh oh!

knopp commented Jun 4, 2023 •

edited

Loading

Uh oh!

knopp commented Jun 4, 2023

Uh oh!

zanderso commented Jun 8, 2023

Uh oh!

flar commented Jun 12, 2023

Uh oh!

Uh oh!

Uh oh!

flar left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

knopp commented Jun 14, 2023

Uh oh!

knopp commented Jun 14, 2023

Uh oh!

flar left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Raster cache should preserve RTree for overlay layers #42552

Raster cache should preserve RTree for overlay layers #42552

Uh oh!

Conversation

knopp commented Jun 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

knopp commented Jun 4, 2023

Uh oh!

zanderso commented Jun 8, 2023

Uh oh!

flar commented Jun 12, 2023

Uh oh!

Uh oh!

Uh oh!

flar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

knopp commented Jun 14, 2023

Uh oh!

knopp commented Jun 14, 2023

Uh oh!

flar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

knopp commented Jun 4, 2023 •

edited

Loading