No shrinking for BackdropFilter's cull rect #28174
Conversation
liyuqian
force-pushed
the
backdropfilter_no_shrink
branch
from
7d828c1 to
4272285
Compare
liyuqian
force-pushed
the
backdropfilter_no_shrink
branch
from
4272285 to
98a8c08
Compare
|
Can you document on BackdropFilter that it may potentially need to be wrapped in a ClipRect? |
|
CC @Hixie for the breaking change. Since there's no user complaining yet, we may wait a while and send an ask/announcement to flutter-dev before landing this. |
There was a problem hiding this comment.
Can't this be statically defined? Instead of instantiating a new closure on each paint call.
There was a problem hiding this comment.
If I make a static, I can no longer call super.paint which is non-static?
Don't worry, I'll remove this hacky callback soon. See my 3-step plan in #29070 (comment)
There was a problem hiding this comment.
I meant make this function statically defined, not make this function static.
e.g.
void paintingCallback(PaintingContext context, Offset offset) {
blah
}
void paint(PaintingContext context, Offset offset) {
context.pushLayer(BackdropFilter(filter: _filter), paintingCallback, offset);
}There was a problem hiding this comment.
Oh, great, that's a good idea. Thanks!
|
LGTM \o/ |
We should definitely do the breaking change policy if we land this, regardless of who is complaining. |
|
This change should have a corresponding documentation change. Either fixing what we say now, or adding documentation to explain what's going on. We need examples of how to get the effect we want, ideally with actual diagrams (see the assets-for-api-docs repo for how to make diagrams) and sample code. |
zoechi
added
the
framework
|
This took us a while to figure out initially and now we are adding a container with |
liyuqian
force-pushed
the
backdropfilter_no_shrink
branch
from
98a8c08 to
c245265
Compare
|
Doc updated. Added issue #29070 with the breaking change announcement. Please review the updated PR. |
There was a problem hiding this comment.
nit: remove extra blank line?
liyuqian
force-pushed
the
backdropfilter_no_shrink
branch
from
c0f2f7a to
53d1cab
Compare
|
@dnfield after your PhysicalShapeLayer revert, my golden version is having a conflict since my golden patch is after your golden patch. Should I wait for your reland before merging this? (It seems that no new goldens can be updated unless you either reland the PR, or revert the golden commit that includes the PhysicalShapeLayer fix.) |
|
If you can wait for the engine roll to land that would be idea. |
|
Either that, or take the same goldens.version as the engine roll has here: #29363 |
|
I'll wait for the manual roll :) |
liyuqian
force-pushed
the
backdropfilter_no_shrink
branch
from
53d1cab to
be54bc3
Compare
This will be a breaking change. Our old behavior may generate confusions for a sample app like our added golden test: Skia will shrink the cull rect (and thus the filtered area) to the text. The new behavior will fill the BackdropFilter to its parent/ancestor clip. This is more in align with our clip behaviors (no clip by default). If this breaks your app, wrap the BackdropFilter with a ClipRect. [wip] The golden images are not uploaded yet. I'll wait for the initial round of review to approve the golden test before uploading them.
liyuqian
force-pushed
the
backdropfilter_no_shrink
branch
from
7f836de to
5c5ff7a
Compare
10 participants
Fixes #29070
This will be a breaking change. Our old behavior may generate confusions
for a sample app like our added golden test: Skia will shrink the cull
rect (and thus the filtered area) to the text. The new behavior will
fill the BackdropFilter to its parent/ancestor clip. This is more
in line with our clip behaviors (no clip by default).
If this breaks your app, wrap the BackdropFilter with a ClipRect (or some other clip of your choice).
[wip] The golden images are not uploaded yet. I'll wait for the initial
round of review to approve the golden test before uploading them.
The correct golden image:
The incorrect golden image before this PR:
