[Impeller] Downsample gaussian blur passes bidirectionally #35561
Conversation
|
|
||
| // Compute the blur radius in the Y direction by projecting it onto the basis | ||
| // vectors. | ||
| // Note that this assumes both passes of the blur have the same sigma and are |
There was a problem hiding this comment.
I might be misunderstanding, but the API does allow the x and y direction to have different sigmas like here: https://api.flutter.dev/flutter/dart-ui/ImageFilter/ImageFilter.blur.html.
There was a problem hiding this comment.
Yes, the framework supports distinct sigmaX/Y values. That said, I would guess the vast majority of blur usages have either both values the same, or one value as zero.
There was a problem hiding this comment.
Whoops, I've been staring at this code for too long. It might be time to refactor this blur to combine the passes and make these optimizations easier.
There was a problem hiding this comment.
Realized combining the blurs won't really simplify the optimizations because we need to support blurring in arbitrary directions relative to the texture. The input texture can be an arbitrarily complex Contents that was transformed in interesting ways before it was rendered to a texture.
Also, it would be nice to continue supporting arbitrary single direction blur since we could one day expose this to users who want to do high fidelity motion blurs. :)
bdero
force-pushed
the
bdero/symmetrical-blur-scale
branch
from
6737b76 to
532d4b0
Compare
|
Fixed this up and it should be good to go. It's now rebased on top of #35615. |
Stop information loss on first pass Add secondary sigma param to directional gaussian blur
bdero
force-pushed
the
bdero/symmetrical-blur-scale
branch
from
532d4b0 to
56db529
Compare
| blur->SetBlurStyle(blur_style); | ||
| blur->SetTileMode(tile_mode); | ||
| blur->SetSourceOverride(source_override); | ||
| blur->SetSecondarySigma(secondary_sigma); |
There was a problem hiding this comment.
nit: considerjust having a method that takes both sigmas so this doesn't get forgotten in a refactor.
There was a problem hiding this comment.
Whoops, I merged too quickly and missed this comment. I forgot to mark it private, but this param should be hidden as an implementation detail and never be used when making a 1D blur directly (only the factory for the 2D blur should ever use it), whereas the regular sigma param should remain as part of the API.
There was a problem hiding this comment.
You could have the second parameter be optional defaulted to zero right?
This is a nitpick though. It's ok to ignore if you think it should be.
dnfield
left a comment
dnfield
left a comment
There was a problem hiding this comment.
I haven't verified this against the app but LGTM assuming it's visually good.
4 participants
Speeds up the blur in apps we're testing by another 3x or so.
The directional gaussian blur pass has several new assumptions: