Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@flar
Copy link
Contributor

@flar flar commented Feb 18, 2021

Currently the application of the BackdropFilter depends on its child in surprising ways.

It will not be added to the Scene being built if its child is null. That behavior is governed by the RenderObject associated with BackdropFilter and should be considered separately to this PR.

Another behavior is that if the child is non-null, but happens to not draw anything, then it will not apply the filter to the background either. This behavior is an interaction between the RenderObject which does add a backdrop layer to the scene for it, but there will be no child layers for that backdrop layer because nothing was generated by the widget's child and so the native implementation of the layer fails to apply itself for obscure cut-and-paste errors in the C++ code.

Most engine layers will only have work to do if their children draw something, so they determine their bounds from their children and pass that along. If the children don't draw anything then their bounds will be empty and so the wrapping container layer also has empty bounds. The BackdropLayer copies this same implementation and has its bounds determined by the children.

The issue is that regardless of the size of the children, the BackdropLayer's output will only be limited by the cull_rect it inherits (typically limited by either the size of the framebuffer, or a parent clip), and will have nothing to do with the dimensions of the children. So, by passing along its child dimensions as its own paint_bounds, it is lying about the size of its output.

That implementation glitch combines with code in the Paint() recursion which omits the painting of any layer with an empty bounds to not even ask the backdrop layer to paint in the first place.

Note that this issue will also cause problems later when we try to determine which parts of a scene are changing as the BackdropLayer will not report accurate bounds for what it changed in the scene.

This PR addresses that engine layer implementation issue by having the BackdropFilter layer report the cull rect as its bounds.

@flar
Copy link
Contributor Author

flar commented Feb 18, 2021

@knopp originally discovered this issue during his work on the DiffContext as it was interfering with his attempts to record damage for individual layer trees.

In fact, this PR may clash with his soon-to-be merged PR, and may become obsolete at that time, or may need to be heavily modified.

@knopp
Copy link
Member

knopp commented Feb 18, 2021

This definitely needs to be addressed one way or another for partial updates to work correctly.

@flar flar requested review from arbreng and chinmaygarde February 18, 2021 23:23
@flar
Copy link
Contributor Author

flar commented Feb 18, 2021

I'll need to update the web rendering as well, and probably include a golden test...

--web-renderer html already does filtering with a non-rendering child
--web-renderer canvaskit does not, so needs a similar fix

@flar
Copy link
Contributor Author

flar commented Feb 19, 2021

With the last commit, the backdrop layer now performs the blur on canvaskit as well.

I am still looking at adding a golden test or two to verify the new behavior.

@flar flar requested a review from yjbanov February 19, 2021 19:14
Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goldens_lock.yaml needs to be updated with the new SHA. Code LGTM.

lgtm

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants