-
Notifications
You must be signed in to change notification settings - Fork 6k
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.
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
[Impeller] Make transparent fills have no coverage #34055
[Impeller] Make transparent fills have no coverage #34055
Conversation
I'm curious about how this interacts with various use cases. If I have a drawing and I want to blur it and I use a blur with kClamp, but I surround it with a transparent fill to give it a border of transparency, would this optimize out the transparent fill and cause the blur to use clamp mode on the border of the drawing? Should it? (Does it currently with Flutter on Skia?) To clarify - it would be OK to not apply the rendering in that case, but should it contribute to the size of a layer in a saveLayer? |
I may be wrong about using the coverage size of a rendering to compute the input for a filter. https://fiddle.skia.org/c/51a32450b524fed60bff5894f22ed55b It looks like Skia assumes a boundary of transparency if you don't specify a bounds. And if you do specify a bounds, then you are in control of where the edge pixels to "clamp" will come from. |
Good point. We don't support clamp sampling in the blur filter yet (It only behaves like decal), but once we do, I think the correct behavior would be to always force decal sampling when used on a SaveLayer. Coverage is kind of an implementation detail in Impeller used to save on resources and can't be relied on for clipping (occasionally coverage is too large, like with solid strokes). In general, if every override of |
My spidey sense says flutter/flutter#72526 (comment) is relevant? |
Oh, indeed it is. Note that the render method ducks out and doesn't draw anything if the color is transparent as-is, which is a bug because if the blend mode is kSrc, for example, it would result in contents changing on the screen. However, when taking into account a layer in isolation, expanding the coverage in this case would have no affect, as the additional pixels would initialize as 0, 0, 0, 0. And any blend with fully transparent black as both the source and destination should wind up remaining at 0, 0, 0, 0. BUT there's an another potential problem that needs to be investigated -- consider this render list:
We know for sure that the bottom right quadrant of the green rectangle should end up being black on-screen, but should the layer itself expand with the invisible rectangle? This would cause an additional 50 pixel black border to appear around the right and bottom sides of the green rectangle. |
Without checking what Skia does, I'd expect it to expand the layer size - but it'd be good to just run this through Skia fiddle and see what it does. If it doesn't expand, then we're probably safe to drop the coverage since it won't cause regressions. If it does then we either have to detect the blend modes to see if it's safe, or keep it always if that's too hard. |
Right, I'm trying it in Skia fiddle at the moment but I think I missed the forest for the trees here. For
So the TL;DR is: Skipping over coverage should actually be safe here, but we need to add logic to always expand subpass coverage to cover the entire screen if the subpass blend mode may modify the destination color when given a source color of fully transparent black. Going to make a bug to follow up on this. |
SGTM. Please link the bug to this discussio nfor context :) |
Oh and one comment on your fiddle - using nullptr for the bounds of the saveLayer call in Skia tells it to use the current clip. If you specify bounds matching the rect you're going to draw it doesn't fill the whole clip and you get a green square inside the red one. |
c27fb8f
to
3e0d9cc
Compare
I was confused about the saveLayer trying to compute bounds. I thought I saw that in a discussion from long ago, but I wouldn't know where to look for it now. As Dan indicates and Skia just confirmed, the bounds of a saveLayer in the absence of a parameter are the entire clip or surface. So, I'm not exactly sure where computing the coverage of a transparent operation would matter (modulo fixing the case of a destructive blend mode)? Where does this coverage get used? For determining which pixels to target with quads/tris on a given rendering op? |
No description provided.