Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Impeller] RRect blur improvements #38417

Merged
merged 1 commit into from
Dec 20, 2022
Merged

[Impeller] RRect blur improvements #38417

merged 1 commit into from
Dec 20, 2022

Conversation

bdero
Copy link
Member

@bdero bdero commented Dec 20, 2022 •
edited
Loading

Address the RRect blur discontinuities brought up in flutter/flutter#116584 by increasing the radius per sigma ratio from sqrt(3) to 2 (~15%). Upon close examination, I noticed that Skia's blurs appear to extend out about this much further.

To offset the additional GPU load, I applied a couple of speed-ups I've been meaning to get around to:

  • Decrease the number of samples from 5 to 4.
  • Use a faster approximation for the Gaussian integral. For this I just grabbed the logistic function and fit it as close to the Gaussian integral as possible. It's very close! I recall seeing one of Skia's blur paths use a sigmoid like this to populate a LUT a while back... this might be why some of Skia's blurs look a bit softer/lighter than ours in some places.

Blue is the old integral, purple is the new one. s is sigma:

Screen.Recording.2022-12-19.at.9.40.13.PM.mov

Before:
Screen Shot 2022-12-19 at 9 26 09 PM

After:
Screen Shot 2022-12-19 at 9 25 19 PM

@bdero bdero self-assigned this Dec 20, 2022
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@chinmaygarde
Copy link
Member

Top tier stuff! I think cases like this would really benefit from the shader profiling stuff discussed earlier (that Zach is following up on). flutter/flutter#116584 has been disambiguated to only deal with the shadow/blur quality. Can we resolve that now?

@bdero
Copy link
Member Author

bdero commented Dec 20, 2022

Closing out the issue. Hoping this will also be a net reduction on Impeller's Android/iOS benchmarks. 🤞

@bdero bdero merged commit bd4a604 into flutter:main Dec 20, 2022
@bdero bdero mentioned this pull request Dec 20, 2022
Closed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Dec 21, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 21, 2022
@engine-flutter-autoroll
bd4a60454 [Impeller] RRect blur improvements (flutter/engine#38417)
ffdbeec
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Dec 21, 2022
@engine-flutter-autoroll
Roll Flutter Engine from 7ee3bf518036 to 75d75575d0ea (12 revisions) (#…
014b8f7 
…117421)

* 2dd2afb49 Roll Skia from e8c3fa6d7d2f to c42beb57e108 (2 revisions) (flutter/engine#38416)

* 333741df5 Roll Fuchsia Mac SDK from NS4fVXM2KhKcZ1uyD... to ev2n-_c3kgBw1h4RG... (flutter/engine#38418)

* 73801d376 Roll Skia from c42beb57e108 to 557183808708 (2 revisions) (flutter/engine#38419)

* 6b7ed7802 Use DisplayListMatrixClipTracker in DisplayListBuilder (flutter/engine#38349)

* e3e288be8 Roll Skia from 557183808708 to 68dbdbdc2e49 (1 revision) (flutter/engine#38420)

* 8e8d7b5d2 Roll Fuchsia Linux SDK from uKNwsaf92uZcX_QiY... to iQT5jpUhipvetxSiH... (flutter/engine#38421)

* c08907c38 Roll Skia from 68dbdbdc2e49 to a8378cd12673 (1 revision) (flutter/engine#38422)

* cf69289fb Roll Skia from a8378cd12673 to eca2fed907ac (3 revisions) (flutter/engine#38423)

* bd4a60454 [Impeller] RRect blur improvements (flutter/engine#38417)

* d4929a7a7 Roll Fuchsia Mac SDK from ev2n-_c3kgBw1h4RG... to nJJfWIwH5zElheIX8... (flutter/engine#38424)

* 91dc9645f Roll Skia from eca2fed907ac to 34fb45763ef7 (3 revisions) (flutter/engine#38425)

* 75d75575d Roll Skia from 34fb45763ef7 to 09d796c0a728 (8 revisions) (flutter/engine#38428)
mit-mit pushed a commit to mit-mit/flutter that referenced this pull request Dec 21, 2022
@engine-flutter-autoroll @mit-mit
Roll Flutter Engine from 7ee3bf518036 to 75d75575d0ea (12 revisions) (f…
f77ae6f 
…lutter#117421)

* 2dd2afb49 Roll Skia from e8c3fa6d7d2f to c42beb57e108 (2 revisions) (flutter/engine#38416)

* 333741df5 Roll Fuchsia Mac SDK from NS4fVXM2KhKcZ1uyD... to ev2n-_c3kgBw1h4RG... (flutter/engine#38418)

* 73801d376 Roll Skia from c42beb57e108 to 557183808708 (2 revisions) (flutter/engine#38419)

* 6b7ed7802 Use DisplayListMatrixClipTracker in DisplayListBuilder (flutter/engine#38349)

* e3e288be8 Roll Skia from 557183808708 to 68dbdbdc2e49 (1 revision) (flutter/engine#38420)

* 8e8d7b5d2 Roll Fuchsia Linux SDK from uKNwsaf92uZcX_QiY... to iQT5jpUhipvetxSiH... (flutter/engine#38421)

* c08907c38 Roll Skia from 68dbdbdc2e49 to a8378cd12673 (1 revision) (flutter/engine#38422)

* cf69289fb Roll Skia from a8378cd12673 to eca2fed907ac (3 revisions) (flutter/engine#38423)

* bd4a60454 [Impeller] RRect blur improvements (flutter/engine#38417)

* d4929a7a7 Roll Fuchsia Mac SDK from ev2n-_c3kgBw1h4RG... to nJJfWIwH5zElheIX8... (flutter/engine#38424)

* 91dc9645f Roll Skia from eca2fed907ac to 34fb45763ef7 (3 revisions) (flutter/engine#38425)

* 75d75575d Roll Skia from 34fb45763ef7 to 09d796c0a728 (8 revisions) (flutter/engine#38428)
loic-sharma pushed a commit to loic-sharma/flutter-engine that referenced this pull request Jan 3, 2023
@bdero @loic-sharma
[Impeller] RRect blur improvements (flutter#38417)
3d4d5fd
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jan 19, 2023
@engine-flutter-autoroll @gspencergoog
Roll Flutter Engine from 7ee3bf518036 to 75d75575d0ea (12 revisions) (f…
58f7a81 
…lutter#117421)

* 2dd2afb49 Roll Skia from e8c3fa6d7d2f to c42beb57e108 (2 revisions) (flutter/engine#38416)

* 333741df5 Roll Fuchsia Mac SDK from NS4fVXM2KhKcZ1uyD... to ev2n-_c3kgBw1h4RG... (flutter/engine#38418)

* 73801d376 Roll Skia from c42beb57e108 to 557183808708 (2 revisions) (flutter/engine#38419)

* 6b7ed7802 Use DisplayListMatrixClipTracker in DisplayListBuilder (flutter/engine#38349)

* e3e288be8 Roll Skia from 557183808708 to 68dbdbdc2e49 (1 revision) (flutter/engine#38420)

* 8e8d7b5d2 Roll Fuchsia Linux SDK from uKNwsaf92uZcX_QiY... to iQT5jpUhipvetxSiH... (flutter/engine#38421)

* c08907c38 Roll Skia from 68dbdbdc2e49 to a8378cd12673 (1 revision) (flutter/engine#38422)

* cf69289fb Roll Skia from a8378cd12673 to eca2fed907ac (3 revisions) (flutter/engine#38423)

* bd4a60454 [Impeller] RRect blur improvements (flutter/engine#38417)

* d4929a7a7 Roll Fuchsia Mac SDK from ev2n-_c3kgBw1h4RG... to nJJfWIwH5zElheIX8... (flutter/engine#38424)

* 91dc9645f Roll Skia from eca2fed907ac to 34fb45763ef7 (3 revisions) (flutter/engine#38425)

* 75d75575d Roll Skia from 34fb45763ef7 to 09d796c0a728 (8 revisions) (flutter/engine#38428)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chinmaygarde chinmaygarde chinmaygarde approved these changes

@zanderso zanderso Awaiting requested review from zanderso

Assignees

@bdero bdero

Labels
e: impeller needs tests
Projects
No open projects
Impeller
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bdero @chinmaygarde