-
Notifications
You must be signed in to change notification settings - Fork 6k
[Impeller] new blur: round downsample to power of two #50245
Conversation
jonahwilliams
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Does this improve the animated blur case? Follow up question, if we're scaling down by a power of two, doesn't this imply that we're sampling at an exact mip level? Could we bypass the downsample step entirely and configure the first pass of the blur to sample from a specific miplevel?
Do you mean the animated sigma? That's whats in the video above. Or are you referring to the animated content under the blur? I didn't look into that one but thinking about it now it may be worse since we are potentially downscaling earlier (I used
Yep, brandon has brought up that optimization in the past. I'm trying to avoid creating a test harness for this case. But if we wanted to we could generate multiple images and use root mean square error to quantify jitters in the animating sigma case. It gets a bit harder in the "animate content under blur" case. |
|
I'll try to modify the |
|
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. |
|
@jonahwilliams at first blush, it's looking good bluranimated.mp4 |
…142745) flutter/engine@39415c3...9beb7e8 2024-02-01 [email protected] [Impeller] new blur: round downsample to power of two (flutter/engine#50245) 2024-02-01 [email protected] Provide a more helpful error message in the case of UnsatisfiedLinkError (flutter/engine#50247) 2024-02-01 [email protected] Update expected golden number. (flutter/engine#50249) 2024-02-01 [email protected] [Impeller] remove drawPicture from Aiks Canvas. (flutter/engine#50242) 2024-02-01 [email protected] Reland "[Windows] Introduce egl::Surface and egl::WindowSurface" (flutter/engine#50148) 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
This makes the downsampling step of the new blur round to the nearest power of 2. This makes the changes in downsampling less frequent and the output of downsampling hypothetically higher quality since downsampling by a power of 2 is easier.
issue: flutter/flutter#141510
before
pow2blur-before.mp4
after
pow2blur-after.mp4
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.