[Impeller] Add backdrop filter support; refactor EntityPass #33887
Conversation
bdero
changed the title
| UNIMPLEMENTED; | ||
| canvas_.SaveLayer(paint, ToRect(bounds), ToImageFilterProc(backdrop)); | ||
| } else { | ||
| canvas_.SaveLayer(paint, ToRect(bounds)); |
There was a problem hiding this comment.
canvas_.SaveLayer takes an optional backdrop. Does having a special call omitting the argument save much?
impeller/entity/entity_pass.cc
Outdated
| } | ||
|
|
||
| if (subpass_coverage->size.IsEmpty()) { | ||
| // It is not an error to have an empty subpass. But subpasses that can't | ||
| // create their intermediates must trip errors. | ||
| continue; | ||
| return EntityPass::EntityResult::Skip(); |
There was a problem hiding this comment.
Is it possible for the subpass to be non-"empty", but produce an empty coverage? If so, then we don't want to skip here if there is a backdrop filter.
There was a problem hiding this comment.
Another way to look at this is that the subpass has some coverage, and if there is an optional backdrop filter, then it also has some coverage, and then you combine the two (and clip the combined coverage). Now, if that combined clipped coverage is null, then it's OK to skip.
impeller/entity/entity_pass.cc
Outdated
| const auto subpass_coverage = GetSubpassCoverage(*subpass); | ||
| if (subpass->elements_.empty() && backdrop_filter_proc_.has_value()) { | ||
| // An empty subpass with a backdrop filter should render the backdrop over | ||
| // the entire coverage of the current pass. |
There was a problem hiding this comment.
Actually, even if the subpass is not empty, the presence of a backdrop filter indicates that the entire background will be filtered (output restricted by any existing clips).
There was a problem hiding this comment.
Hmm, I see... what about when SaveLayer bounds are given? That should still clip the filtered background, right?
There was a problem hiding this comment.
I think this is fixed now. Added a video of the test demonstrating the clipping behavior in the PR description.
There was a problem hiding this comment.
Actually no, the saveLayer bounds do not clip the filtered background. I don't even know if they clip the children (subpass) either in this case, Skia fiddle would show the expected behavior.
There was a problem hiding this comment.
I find this behavior a bit odd, but confirmed in Skia fiddle: https://fiddle.skia.org/c/51c183699c942925d0b13d967f932010
Fixed and updated the demo video in the description.
…Render pass management
bdero
force-pushed
the
bdero/backdrop-filter
branch
from
fc77a26
to
30f3f21
Compare
| ToRect(bounds)); | ||
| auto paint = options.renders_with_attributes() ? paint_ : Paint{}; | ||
| canvas_.SaveLayer(paint, ToRect(bounds), | ||
| backdrop ? ToImageFilterProc(backdrop) : std::nullopt); |
There was a problem hiding this comment.
I was thinking more along the lines of just:
canvas_.SaveLayer(paint, ToRect(bounds), ToImageFilterProc(backdrop));
since ToIFP handles null already...
There was a problem hiding this comment.
Whoopsies, went too fast here. Fixed.
impeller/entity/entity_pass.cc
Outdated
| } | ||
| if (subpass->backdrop_filter_proc_.has_value()) { | ||
| subpass_coverage = Rect( | ||
| position, Size(pass_context.GetRenderTarget().GetRenderTargetSize())); |
There was a problem hiding this comment.
It won't really happen with a blur, but it is possible for the filtered output to be smaller than the saveLayer rect. Consider the erode filter, for instance. With a large erode radius it could drop the total size of the filtered backdrop down to a very small size.
Although you don't seem to be computing the filtered size here, just the size of the backdrop...?
There was a problem hiding this comment.
I did some more comprehensive testing in Skia fiddle and confirmed that coverage of the filter is important. Right now there are 2 problems with my coverage approach here:
- As you mentioned, the post-filter coverage is not getting incorporated. This means that if you use a blur filter on a small layer, the expanded sides of the blur will get cut off (which is wrong).
- Only the backdrop texture is getting incorporated in coverage when a filter is present, which means entities added to the layer with the filter may get cut off, which is also wrong.
There was a problem hiding this comment.
In # 1, by "small layer", do you mean the saveLayer bounds? Presumably if the parent layer is small, the blur would be clipped to it anyway.
For # 2, I'm not sure what you mean by "entities added to the layer"? Do you mean things you draw in the sub-pass, or the content of the saveLayer? I think those would normally (in Skia?) be cut off if the saveLayer bounds were provided without a backdrop filter, but not if the backdrop filter requires a larger layer.
My impression is that the bounds are:
Union(backdrop(parent layer).output_bounds, saveLayer bounds).IntersectWith(clip and parent layer bounds).
There was a problem hiding this comment.
Presumably if the parent layer is small, the blur would be clipped to it anyway
This is what I thought too, but the blur seems to be getting properly incorporated into the layer's coverage (which in turn impacts the parent layer's size): https://fiddle.skia.org/c/7cb23795f3cfc78a10e50c3503379d38
In the above Skia fiddle, I expected to see the blur to get cut off on the bottom and right side of the middle circle (#2), But to my surprise, the parent layer's coverage seems to expand appropriately and nothing is getting cut off.
There's a dependency loop. Assuming SaveLayer B is a child of SaveLayer A, and SaveLayer B has a backdrop filter:
- The SaveLayer A's coverage depends on the coverage of SaveLayer B.
- The SaveLayer B's coverage depends on the backdrop filter coverage.
- The backdrop filter coverage depends on the parent pass texture size.
- The parent pass texture size is the SaveLayer A's coverage.
This can be solved by extending the filter coverage API to allow the caller to supply a list of prospective coverage rectangles for the inputs (as opposed to resolving the FilterInput coverage) via the Contents::GetCoverage implementation. Then, EntityPass coverage routine can incorporate subpass filter coverage without having to allocate textures. Another alternative would be to add mock/coverage-only filter inputs.
There was a problem hiding this comment.
If you specify bounds for A's saveLayer then those get used as a clip, but yeah. B's saveLayer is a rendering operation within A so its bounds affect the default bounds for A. Also, if you applied a blur filter to B (not a backdrop, but a blur of the layer), those blur bounds would also be accumulated into A's default bounds.
There was a problem hiding this comment.
Alright, I changed the coverage computation to union with the backdrop filter's coverage before creating the child pass texture.
However, this union currently happens outside the actual coverage routine, which means the backdrop filter coverage still doesn't properly propagate up to parent passes. Fixing this is a bit more involved, so I wrote an issue with a plan for how to address it in a future patch: flutter/flutter#105810
Adding backdrop filter support required adding a little bit of additional logic to the
EntityPass, which was in dire need for some simplification, so I ended up doing some refactoring here.The first 3 commits of this PR are mostly a refactor to wrangle the EntityPass logic in a more reasonable way:
InlinePassContextwith clear contracts. Passes can be ended at any time duringEntityPass::OnRender. A newRenderPasswith the right configuration will get created if necessary whenever the pass is fetched. This removes most of the state in the render method and makes reasoning aboutEntityPass::OnRendermethod way easier.EntityPass::GetEntityForElementConveniently, most of the behavior we actually needed to support the backdrop filter was already done thanks to the prior work on advanced blends:
EntityPasswhenSaveLayeris called.TextureFilterInput, and then renders the resulting contents before any pass elements are rendered.contains_advanced_blends_flag is nowreads_from_pass_texture_, and is set totruewhen theEntityPasscontains any advanced blended elements or subpasses with backdrop filters. Behavior of this flag remains otherwise unchanged (it makes the root pass create an intermediate texture, and it makes subpasses create non-transient stencils).Screen.Recording.2022-06-07.at.5.12.36.PM.mov
The demo started out more visually interesting, but it turns out that the presence of the backdrop filter is supposed to override the layer's coverage, whether or not entities or the SaveLayer bounds hint is present.