[Impeller] add herustic for ignoring coverage limit w/ image filters. #55030
Conversation
jonahwilliams
added
the
autosubmit
| return coverage.TransformBounds(effect_transform) | ||
| .Intersection(source_coverage_limit.value()); | ||
| // Trimming the content coverage by the coverage limit can reduce memory | ||
| // coverage. limit can reduce memory bandwith. But in cases where there are |
| // to non-optimal allocation patterns as differently sized textures cannot | ||
| // be reused. Hence the following herustic: If the coverage is within a | ||
| // semi-arbitrary percentage of the intersected coverage, then just use the | ||
| // transformed coverage. In other cases, use the intersection. |
There was a problem hiding this comment.
What about going further and using surfaces that are rounded up to a tile size or some other quantization size? Would we be able to reuse the surfaces?
Some thought would need to be taken with kClamp/repeat/mirror tile modes, though.
There was a problem hiding this comment.
OOh, that is a good idea.
There was a problem hiding this comment.
Can the clamp/repeat modes restrict themselves to a coordinate sample not on the edge?
There was a problem hiding this comment.
We control the input UVs for all of our image filters, so should be able to easily (tm) restrict the input range.
…154844) flutter/engine@1b2656a...813ad2d 2024-09-09 [email protected] [Impeller] add herustic for ignoring coverage limit w/ image filters. (flutter/engine#55030) 2024-09-09 [email protected] Roll Skia from f7e4ddabb754 to 8103f53635fd (1 revision) (flutter/engine#55043) 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://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
…flutter#55030) Switching to save_layer_utils regressed perf/memory usage on several devicelab benchmarks, see https://flutter-flutter-perf.skia.org/e/?queries=device_type%3DPixel_7_Pro%26sub_result%3D90th_percentile_gpu_memory_mb%26sub_result%3Daverage_gpu_memory_mb%26test%3Dnew_gallery_impeller_old_zoom__transition_perf&selected=commit%3D42418%26name%3D%252Carch%253Dintel%252Cbranch%253Dmaster%252Cconfig%253Ddefault%252Cdevice_type%253DPixel_7_Pro%252Cdevice_version%253Dnone%252Chost_type%253Dlinux%252Csub_result%253D90th_percentile_gpu_memory_mb%252Ctest%253Dnew_gallery_impeller_old_zoom__transition_perf%252C The reason is that the source_coverage_limit [intersection](https://github.com/flutter/engine/blob/main/impeller/entity/save_layer_utils.cc#L67-L68) is highly unstable from frame to frame if there is an animated blur and/or image filter. This change mostly matches the old entity pass behavior. . In cases where the difference between the intersected coverage result is small (< 20% in both dimensions), we just use the transformed content coverage. The old behavior would _always_ use the transformed content coverage even if it was substantially large than the coverage limit. This posed problems if folks drew content larger than the max texture size. This herustic attempts to prevent that by allowing the source coverage limit to kick in once the difference is larger than 20% in either direction
|
@jonahwilliams heads up this caused a slight change in 90th percentile frame time. I didn't file an issue because it seemed small but I wanted to give you a heads up if you want to investigate it further: https://flutter-flutter-perf.skia.org/e/?keys=X5391c5b0fd131dcde9b6d63c4ef189c9&selected=commit%3D42494%26name%3D%252Carch%253Dintel%252Cbranch%253Dmaster%252Cconfig%253Ddefault%252Cdevice_type%253Dmokey%252Cdevice_version%253Dnone%252Chost_type%253Dlinux%252Csub_result%253D90th_percentile_gpu_frame_time%252Ctest%253Dnew_gallery_opengles_impeller__transition_perf%252C&xbaroffset=42494 |
Switching to save_layer_utils regressed perf/memory usage on several devicelab benchmarks, see https://flutter-flutter-perf.skia.org/e/?queries=device_type%3DPixel_7_Pro%26sub_result%3D90th_percentile_gpu_memory_mb%26sub_result%3Daverage_gpu_memory_mb%26test%3Dnew_gallery_impeller_old_zoom__transition_perf&selected=commit%3D42418%26name%3D%252Carch%253Dintel%252Cbranch%253Dmaster%252Cconfig%253Ddefault%252Cdevice_type%253DPixel_7_Pro%252Cdevice_version%253Dnone%252Chost_type%253Dlinux%252Csub_result%253D90th_percentile_gpu_memory_mb%252Ctest%253Dnew_gallery_impeller_old_zoom__transition_perf%252C
The reason is that the source_coverage_limit intersection is highly unstable from frame to frame if there is an animated blur and/or image filter.
This change mostly matches the old entity pass behavior. . In cases where the difference between the intersected coverage result is small (< 20% in both dimensions), we just use the transformed content coverage.
The old behavior would always use the transformed content coverage even if it was substantially large than the coverage limit. This posed problems if folks drew content larger than the max texture size. This herustic attempts to prevent that by allowing the source coverage limit to kick in once the difference is larger than 20% in either direction