[Impeller] Turned on new blur. #48472

gaaclarke · 2023-11-28T21:26:20Z

This new blur should perform faster since it scales down the image before blurring it. Jonah did early testing of it and found it to be faster. Scrolling around with the blur perf bug it seems faster. It also has a wider test bed and is hopefully easier to maintain since it contains all of its logic for both directions.

testing: There are existing blur tests and we've backfilled more as we've added features to this blur.

fixes flutter/flutter#131580
fixes flutter/flutter#138259

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 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.

gaaclarke · 2023-11-28T23:40:43Z

Here are the tests that currently need addressing. Fixing the clipped blur failures may also help with flutter/flutter#139165. Some of it seems related to clamping the blur sigma too, where the old blur does it, and the new one doesn't.

gaaclarke · 2023-12-07T23:48:17Z

This is looking good now except for the case where we aren't clamping the sigma.

flutter-dashboard · 2023-12-14T17:53:45Z

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #48472 at sha e6fc470

gaaclarke · 2023-12-14T18:04:02Z

We still have a few outstanding optimizations we could do. I think this is feature complete enough to land. I did manual testing with the blur perf bug's sample code and wonderous.

jonahwilliams

LGTM!

…140195) flutter/engine@0e7248d...4cb3ba7 2023-12-15 [email protected] Roll Skia from 79f23e8d8b5d to cd2f06086c96 (1 revision) (flutter/engine#49069) 2023-12-15 [email protected] Roll Dart SDK from 0471164827b9 to f1e37ed8917e (1 revision) (flutter/engine#49065) 2023-12-15 [email protected] [Impeller] impellerc: delete unused code (flutter/engine#49061) 2023-12-15 [email protected] [Impeller] split out gradient tests from aiks_unittests (flutter/engine#49050) 2023-12-15 [email protected] Revert "[Impeller] Turned on new blur." (flutter/engine#49062) 2023-12-14 [email protected] Fix header-guard naming convention in `shell/`. (flutter/engine#49006) 2023-12-14 [email protected] Manual roll of Dart SDK from a677378ae254 to 0471164827b9 (flutter/engine#49054) 2023-12-14 [email protected] Rename `font-subset` to `font_subset`. (flutter/engine#49051) 2023-12-14 [email protected] Roll Skia from 92935b91193a to 79f23e8d8b5d (1 revision) (flutter/engine#49056) 2023-12-14 [email protected] Suppress warning for the new Activity library. (flutter/engine#49046) 2023-12-14 [email protected] [Impeller] distinguish between no clear color and transparent black clear color. (flutter/engine#49038) 2023-12-14 [email protected] Unpin mac_toolchain version (flutter/engine#48994) 2023-12-14 [email protected] [Impeller] Turned on new blur. (flutter/engine#48472) 2023-12-14 [email protected] Remove unused metadata in DEPS from vuln scanning (flutter/engine#48995) 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

gaaclarke · 2023-12-18T18:55:00Z

Here are the results from backdrop_filter_perf_ios__timeline_summary. Despite earlier positive early results the new blur seems to be slower too. We should implement some of our planned optimizations before switching over.

before

    "average_vsync_transitions_missed": 3.861904761904762,
    "90th_percentile_vsync_transitions_missed": 4.0,
    "99th_percentile_vsync_transitions_missed": 4.0,
    "average_vsync_frame_lag": 0.0,
    "90th_percentile_vsync_frame_lag": 0.0,
    "99th_percentile_vsync_frame_lag": 0.0,
    "average_layer_cache_count": 0.0,
    "90th_percentile_layer_cache_count": 0.0,
    "99th_percentile_layer_cache_count": 0.0,
    "average_frame_request_pending_latency": 16648.66,
    "90th_percentile_frame_request_pending_latency": 16720.0,
    "99th_percentile_frame_request_pending_latency": 16783.0,
    "worst_layer_cache_count": 0.0,
    "average_layer_cache_memory": 0.0,
    "90th_percentile_layer_cache_memory": 0.0,
    "99th_percentile_layer_cache_memory": 0.0,
    "worst_layer_cache_memory": 0.0,
    "average_picture_cache_count": 0.0,
    "90th_percentile_picture_cache_count": 0.0,
    "99th_percentile_picture_cache_count": 0.0,
    "worst_picture_cache_count": 0.0,
    "average_picture_cache_memory": 0.0,
    "90th_percentile_picture_cache_memory": 0.0,
    "99th_percentile_picture_cache_memory": 0.0,
    "worst_picture_cache_memory": 0.0,
    "total_ui_gc_time": 0.119,
    "30hz_frame_percentage": 0.0,
    "60hz_frame_percentage": 100.0,
    "80hz_frame_percentage": 0.0,
    "90hz_frame_percentage": 0.0,
    "120hz_frame_percentage": 0.0,
    "illegal_refresh_rate_frame_count": 0,
    "average_gpu_frame_time": 51.48809523809524,
    "90th_percentile_gpu_frame_time": 62.5,
    "99th_percentile_gpu_frame_time": 62.5,
    "worst_gpu_frame_time": 62.5,
    "average_cpu_usage": 97.33600004000002,
    "90th_percentile_cpu_usage": 99.100001,
    "99th_percentile_cpu_usage": 99.400001,
    "average_gpu_usage": 100.0,
    "90th_percentile_gpu_usage": 100.0,
    "99th_percentile_gpu_usage": 100.0,
    "average_memory_usage": 118.8809375,
    "90th_percentile_memory_usage": 139.796875,
    "99th_percentile_memory_usage": 145.390625
  },

after

    "average_vsync_transitions_missed": 5.830645161290323,
    "90th_percentile_vsync_transitions_missed": 6.0,
    "99th_percentile_vsync_transitions_missed": 6.0,
    "average_vsync_frame_lag": 0.0,
    "90th_percentile_vsync_frame_lag": 0.0,
    "99th_percentile_vsync_frame_lag": 0.0,
    "average_layer_cache_count": 0.0,
    "90th_percentile_layer_cache_count": 0.0,
    "99th_percentile_layer_cache_count": 0.0,
    "average_frame_request_pending_latency": 16638.835,
    "90th_percentile_frame_request_pending_latency": 16727.0,
    "99th_percentile_frame_request_pending_latency": 16772.0,
    "worst_layer_cache_count": 0.0,
    "average_layer_cache_memory": 0.0,
    "90th_percentile_layer_cache_memory": 0.0,
    "99th_percentile_layer_cache_memory": 0.0,
    "worst_layer_cache_memory": 0.0,
    "average_picture_cache_count": 0.0,
    "90th_percentile_picture_cache_count": 0.0,
    "99th_percentile_picture_cache_count": 0.0,
    "worst_picture_cache_count": 0.0,
    "average_picture_cache_memory": 0.0,
    "90th_percentile_picture_cache_memory": 0.0,
    "99th_percentile_picture_cache_memory": 0.0,
    "worst_picture_cache_memory": 0.0,
    "total_ui_gc_time": 2.286,
    "30hz_frame_percentage": 0.0,
    "60hz_frame_percentage": 100.0,
    "80hz_frame_percentage": 0.0,
    "90hz_frame_percentage": 0.0,
    "120hz_frame_percentage": 0.0,
    "illegal_refresh_rate_frame_count": 0,
    "average_gpu_frame_time": 87.70161290322581,
    "90th_percentile_gpu_frame_time": 125.0,
    "99th_percentile_gpu_frame_time": 125.0,
    "worst_gpu_frame_time": 125.0,
    "average_cpu_usage": 118.07500012500005,
    "90th_percentile_cpu_usage": 126.800002,
    "99th_percentile_cpu_usage": 129.600001,
    "average_gpu_usage": 100.0,
    "90th_percentile_gpu_usage": 100.0,
    "99th_percentile_gpu_usage": 100.0,
    "average_memory_usage": 104.82259114583333,
    "90th_percentile_memory_usage": 119.875,
    "99th_percentile_memory_usage": 123.8125
  },

bdero · 2023-12-20T12:34:48Z

I opened the EntityTest.GaussianBlurFilter playground in xcode and it's leaking like a firehose. Not sure if it's at all relevant to the reason for revert or if it's just a problem with this playground, but it doesn't seem to be happening for other interactive playgrounds

gaaclarke · 2023-12-20T17:19:23Z

I opened the EntityTest.GaussianBlurFilter playground in xcode and it's leaking like a firehose. Not sure if it's at all relevant to the reason for revert or if it's just a problem with this playground, but it doesn't seem to be happening for other interactive playgrounds

I called this out the other week when I left it running when I went to lunch and it almost took down my computer. I did measurements for the benchmark in flutter/flutter#140189 (comment). I didn't post the results but I looked at cpu results too and they looked stable. So I think this leak is unique to the interactive test.

…ndering of an entity in the playgrounds This fixes the memory leak in the GaussianBlurFilter playground described in flutter#48472

…ndering of an entity in the playgrounds (#49576) This fixes the memory leak in the GaussianBlurFilter playground described in #48472

jason-simmons · 2024-01-09T00:46:02Z

Landed a fix for the leak described in #48472 (comment)

(The issue only affects the playground framework - see #49576)

This new blur should perform faster since it scales down the image before blurring it. Jonah did early testing of it and found it to be faster. Scrolling around with the blur perf bug it seems faster. It also has a wider test bed and is hopefully easier to maintain since it contains all of its logic for both directions. testing: There are existing blur tests and we've backfilled more as we've added features to this blur. fixes flutter/flutter#131580 fixes flutter/flutter#138259 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat

This new blur should perform faster since it scales down the image before blurring it. Jonah did early testing of it and found it to be faster. Scrolling around with the blur perf bug it seems faster. It also has a wider test bed and is hopefully easier to maintain since it contains all of its logic for both directions. testing: There are existing blur tests and we've backfilled more as we've added features to this blur. fixes flutter/flutter#131580 fixes flutter/flutter#138259 - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat

This is a reland of the new gaussian blur. Changes since revert: 1) Textures are now recycled with ping ponging to reduce memory usage 1) Mipmaps are generated to diminish the shimmering that happens with animated blurs (in metal) --- This new blur should perform faster since it scales down the image before blurring it. Jonah did early testing of it and found it to be faster. Scrolling around with the blur perf bug it seems faster. It also has a wider test bed and is hopefully easier to maintain since it contains all of its logic for both directions. testing: There are existing blur tests and we've backfilled more as we've added features to this blur. fixes flutter/flutter#131580 fixes flutter/flutter#138259 https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat

github-actions bot added the e: impeller label Nov 28, 2023

gaaclarke marked this pull request as ready for review November 28, 2023 21:44

gaaclarke marked this pull request as draft November 28, 2023 21:44

gaaclarke marked this pull request as ready for review November 28, 2023 22:48

gaaclarke force-pushed the turn-on-blur branch from ae401d4 to 9a2e982 Compare November 28, 2023 22:48