use nested op counts to determine picture complexity for raster cache #29265
Conversation
flar
added
affects: engine
perf: speed
|
Do we already have a benchmark that will be improved by this change, or do we need to add one that will catch regressions in the performance that this change buys? |
|
Just wondering, does this conflict with #29155? For instance, say you had an Outer Picture that's animating a property on Canvas.drawPicture for a complex static Inner Picture. Once the inner picture is cached the outer picture becomes relatively simple to draw, so it shouldn't be raster cached. |
No conceptual conflicts. If the outer picture is cacheable then we should cache that and this PR should help us identify more of those. If the outer picture is not cacheable then the other PR will help us cache identifiable and stable sub-pictures to cache instead. We need to make sure we don't fumble and try to cache both in that situation, though. |
To be more clear, it is up to #29155 to avoid any double-caching conflict. Whether or not we change the metrics here, that other PR is introducing a parent/child caching situation that requires care. All this PR is doing is making sure we don't fool ourselves about the complexity of a given picture. |
flar
changed the title
|
Taking into account nested pictures is the right thing to do. Obviously, the mechanism to determine the complexity of a single picture is rather naive. But with the picture complexity scoring work in the pipeline, that should get better too. I think we should land this now. |
flar
added
the
waiting for tree to go green
5 participants
Fixes: flutter/flutter#91621
This PR corrects a long-standing issue whereby we were counting only the ops in the top level picture object for determining its complexity. While this metric may eventually be replaced by more compute/GPU-aware metrics as we develop our support and optimizations for the DisplayList mechanism, for now we can at least fix the current complexity test so that it counts the ops for all nested pictures in the picture object being considered for caching.
This PR also fixes our nested counts to match the behavior of SkPicture nested op counts (mainly, don't count a
drawPictureordrawDisplayListrecord as an op if you are going to include its nested ops in the count). Tests were also back-filled for testing the DisplayList variants of the RasterCache based on the existing tests for the SkPicture methods and new tests were added both for making sure our nested counts match the existing SkPicture results and that the nested metrics are used for the cache.