[Impeller] shrunk the buffer for the rrect_blur #53068

gaaclarke · 2024-05-28T17:36:20Z

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 · 2024-05-28T17:57:13Z

impeller/entity/contents/solid_rrect_blur_contents.cc

 Scalar PadForSigma(Scalar sigma) {
-  return sigma * 4.0;
+  Scalar scalar = std::min((1.0f / 47.6f) * sigma + 2.5f, 3.5f);


We could probably get this down to 2.0x for small sigmas if we instead made the curve fit something like ln(x). I tried 2.0 as the bottom for the linear function but would hit some edges somewhere around sigma = 10.

jonahwilliams

LGTM

flutter-dashboard · 2024-05-28T18:09:43Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #53068 at sha d993821

jonahwilliams · 2024-05-28T18:12:11Z

Some of the circle goldens probably need a closer look.

gaaclarke · 2024-05-28T18:21:25Z

Some of the circle goldens probably need a closer look.

Here it is against a black backdrop and scaled up 2x. Mathematically there is a hard edge there but it isn't perceptible. I'm fine with this.

gaaclarke · 2024-05-28T18:23:28Z

It's also nice that the goldens verified that 3.5 is a better ceiling than 4.0 since it didn't register any difference there.

jonahwilliams · 2024-05-28T18:25:56Z

Ship it!

…gine#53068)

…sions) (#149220) Manual roll requested by jacksongardner@google.com flutter/engine@d032390...b26e1b0 2024-05-28 34871572+gmackall@users.noreply.github.com Manual revert of #53001 (flutter/engine#53075) 2024-05-28 chinmaygarde@google.com Remove --ios-cpu flag. Only the arm64 variant is supported. (flutter/engine#53044) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 229d94a8807e to ac454b80130c (1 revision) (flutter/engine#53074) 2024-05-28 jonahwilliams@google.com [Impeller] make strokes slightly lighter. (flutter/engine#53067) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 23ddbb590e44 to 229d94a8807e (2 revisions) (flutter/engine#53071) 2024-05-28 jmccandless@google.com FlutterFragment predictive back (flutter/engine#52302) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 02c359cf8233 to 23ddbb590e44 (2 revisions) (flutter/engine#53070) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 4f91b3865441 to 02c359cf8233 (1 revision) (flutter/engine#53069) 2024-05-28 30870216+gaaclarke@users.noreply.github.com [Impeller] shrunk the buffer for the rrect_blur (flutter/engine#53068) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 91cd2b48377a to 4f91b3865441 (4 revisions) (flutter/engine#53066) 2024-05-28 34871572+gmackall@users.noreply.github.com Upgrade all[most] androidx dependencies to latest (flutter/engine#53001) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 0c2c490021b7 to 91cd2b48377a (3 revisions) (flutter/engine#53065) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 545203f95d4e to 0c2c490021b7 (2 revisions) (flutter/engine#53063) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 74b4d97be6ab to 545203f95d4e (1 revision) (flutter/engine#53062) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 848d9498fd68 to 74b4d97be6ab (1 revision) (flutter/engine#53061) 2024-05-28 johnniwinther@google.com Remove use of --nnbd-agnostic (flutter/engine#53055) 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 jacksongardner@google.com,rmistry@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

gaaclarke · 2024-05-29T22:11:18Z

I manual tested this against the new rrect_blur benchmark (flutter/flutter#149261), this was about a 20% speed increase.

…sions) (flutter#149220) Manual roll requested by jacksongardner@google.com flutter/engine@d032390...b26e1b0 2024-05-28 34871572+gmackall@users.noreply.github.com Manual revert of flutter#53001 (flutter/engine#53075) 2024-05-28 chinmaygarde@google.com Remove --ios-cpu flag. Only the arm64 variant is supported. (flutter/engine#53044) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 229d94a8807e to ac454b80130c (1 revision) (flutter/engine#53074) 2024-05-28 jonahwilliams@google.com [Impeller] make strokes slightly lighter. (flutter/engine#53067) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 23ddbb590e44 to 229d94a8807e (2 revisions) (flutter/engine#53071) 2024-05-28 jmccandless@google.com FlutterFragment predictive back (flutter/engine#52302) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 02c359cf8233 to 23ddbb590e44 (2 revisions) (flutter/engine#53070) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 4f91b3865441 to 02c359cf8233 (1 revision) (flutter/engine#53069) 2024-05-28 30870216+gaaclarke@users.noreply.github.com [Impeller] shrunk the buffer for the rrect_blur (flutter/engine#53068) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 91cd2b48377a to 4f91b3865441 (4 revisions) (flutter/engine#53066) 2024-05-28 34871572+gmackall@users.noreply.github.com Upgrade all[most] androidx dependencies to latest (flutter/engine#53001) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 0c2c490021b7 to 91cd2b48377a (3 revisions) (flutter/engine#53065) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 545203f95d4e to 0c2c490021b7 (2 revisions) (flutter/engine#53063) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 74b4d97be6ab to 545203f95d4e (1 revision) (flutter/engine#53062) 2024-05-28 skia-flutter-autoroll@skia.org Roll Skia from 848d9498fd68 to 74b4d97be6ab (1 revision) (flutter/engine#53061) 2024-05-28 johnniwinther@google.com Remove use of --nnbd-agnostic (flutter/engine#53055) 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 jacksongardner@google.com,rmistry@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

[Impeller] shrunk the buffer for the rrect_blur

d993821

gaaclarke requested a review from jonahwilliams May 28, 2024 17:36

github-actions bot added the e: impeller label May 28, 2024

gaaclarke commented May 28, 2024

View reviewed changes

jonahwilliams approved these changes May 28, 2024

View reviewed changes

flutter-dashboard bot added the will affect goldens label May 28, 2024

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label May 28, 2024

auto-submit bot merged commit 414229b into flutter:main May 28, 2024
32 checks passed

engine-flutter-autoroll mentioned this pull request May 28, 2024

Roll Flutter Engine from d0323905fc2f to 414229bc8a1a (8 revisions) flutter/flutter#149205

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 28, 2024

414229bc8 [Impeller] shrunk the buffer for the rrect_blur (flutter/en…

2c7a9a5

…gine#53068)

engine-flutter-autoroll mentioned this pull request May 28, 2024

Roll Flutter Engine from d0323905fc2f to 2a5056139366 (9 revisions) flutter/flutter#149206

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 28, 2024

414229bc8 [Impeller] shrunk the buffer for the rrect_blur (flutter/en…

568602d

…gine#53068)

engine-flutter-autoroll mentioned this pull request May 28, 2024

Roll Flutter Engine from d0323905fc2f to 4681b0915a4c (10 revisions) flutter/flutter#149211

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 28, 2024

414229bc8 [Impeller] shrunk the buffer for the rrect_blur (flutter/en…

0bb894e

…gine#53068)

engine-flutter-autoroll mentioned this pull request May 28, 2024

Roll Flutter Engine from d0323905fc2f to c364b294c1f3 (11 revisions) flutter/flutter#149213

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 28, 2024

414229bc8 [Impeller] shrunk the buffer for the rrect_blur (flutter/en…

7ce9440

…gine#53068)

engine-flutter-autoroll mentioned this pull request May 28, 2024

Roll Flutter Engine from d0323905fc2f to dd019f6d111d (12 revisions) flutter/flutter#149215

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 28, 2024

414229bc8 [Impeller] shrunk the buffer for the rrect_blur (flutter/en…

d21ef71

…gine#53068)

engine-flutter-autoroll mentioned this pull request May 28, 2024

Roll Flutter Engine from d0323905fc2f to 06944ac16662 (13 revisions) flutter/flutter#149217

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 28, 2024

414229bc8 [Impeller] shrunk the buffer for the rrect_blur (flutter/en…

7ee39b6

…gine#53068)

engine-flutter-autoroll mentioned this pull request May 29, 2024

Manual roll Flutter Engine from d0323905fc2f to b26e1b023cdb (16 revisions) flutter/flutter#149220

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 29, 2024

414229bc8 [Impeller] shrunk the buffer for the rrect_blur (flutter/en…

43582f4

…gine#53068)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Impeller] shrunk the buffer for the rrect_blur #53068

[Impeller] shrunk the buffer for the rrect_blur #53068

gaaclarke commented May 28, 2024

gaaclarke May 28, 2024

jonahwilliams left a comment

flutter-dashboard bot commented May 28, 2024

jonahwilliams commented May 28, 2024

gaaclarke commented May 28, 2024

gaaclarke commented May 28, 2024

jonahwilliams commented May 28, 2024

gaaclarke commented May 29, 2024

[Impeller] shrunk the buffer for the rrect_blur #53068

[Impeller] shrunk the buffer for the rrect_blur #53068

Conversation

gaaclarke commented May 28, 2024

Pre-launch Checklist

gaaclarke May 28, 2024

Choose a reason for hiding this comment

jonahwilliams left a comment

Choose a reason for hiding this comment

flutter-dashboard bot commented May 28, 2024

jonahwilliams commented May 28, 2024

gaaclarke commented May 28, 2024

gaaclarke commented May 28, 2024

jonahwilliams commented May 28, 2024

gaaclarke commented May 29, 2024