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

Mark BackdropFilter as inheriting opacity, enabling fading of BDFs #34435

Merged
auto-submit merged 1 commit into from
Jul 7, 2022
Merged

Mark BackdropFilter as inheriting opacity, enabling fading of BDFs #34435

auto-submit merged 1 commit into from
Jul 7, 2022

Conversation

@flar
Copy link
Contributor

@flar flar commented Jul 1, 2022

Customers have complained about BackdropFilter not working well with an enclosing OpacityLayer. This led to a bug in iOS dialogs from a few years back (see flutter/flutter#31706).

While this PR does not solve all cases where a BackdropFilter was nullified by being inside an enclosing saveLayer, it does solve a common case where the BackdropFilter is a direct (and only) child of an OpacityLayer.

Basically the fix is to mark the BackdropFilterLayer as able to inherit opacity which allows it to combine the opacity of a parent OpacityLayer into its own saveLayer.

Some refactoring of the AutoCachePaint facility was involved to more easily enable sharing of the object for both DisplayList and SkCanvas paths in the layers (as the 2 rendering variants of BackdropFilterLayer now need to do).

@flar
Copy link
Contributor Author

flar commented Jul 1, 2022

CC @antholeole who was running into this issue recently

@antholeole
Copy link
Contributor

antholeole commented Jul 2, 2022

I can confirm that this unblocks me!

Before:
https://user-images.githubusercontent.com/48811365/176983029-9666e3be-4ec5-4c55-a024-b40217c7f18a.mov

After:
https://user-images.githubusercontent.com/48811365/176983047-95718301-66cf-4e9f-b888-4708dcff80f5.mov

Thanks for the fix! This is awesome :)

@zanderso zanderso requested a review from bdero July 2, 2022 02:29
@antholeole
Copy link
Contributor

antholeole commented Jul 6, 2022

There's a bit of logspam, not sure if this is because I pulled your branch first and merged this over:

[VERBOSE-2:backdrop_filter_layer.cc(58)] Inherited opacity: 0x70000a6b8f30
[VERBOSE-2:backdrop_filter_layer.cc(58)] Inherited opacity: 0x70000a6b8f30
[VERBOSE-2:backdrop_filter_layer.cc(58)] Inherited opacity: 0x70000a6b8f30
... (~20x)

@bdero
Copy link
Member

bdero commented Jul 6, 2022

Haven't looked at the log spam mentioned above, but setting opacity on BDF layers is fine in Impeller.

@flar
Copy link
Contributor Author

flar commented Jul 6, 2022 •
edited
Loading

There's a bit of logspam, not sure if this is because I pulled your branch first and merged this over:

[VERBOSE-2:backdrop_filter_layer.cc(58)] Inherited opacity: 0x70000a6b8f30
[VERBOSE-2:backdrop_filter_layer.cc(58)] Inherited opacity: 0x70000a6b8f30
[VERBOSE-2:backdrop_filter_layer.cc(58)] Inherited opacity: 0x70000a6b8f30
... (~20x)

Thanks for checking! There should be a pre-check that maybe doesn't reject pushes, but indicates if there are new calls to FML_LOG in them.

Actually, I don't see that any more. Were you running off the patch I sent for testing rather than the PR patch?

@flar flar added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 6, 2022
@antholeole
Copy link
Contributor

antholeole commented Jul 6, 2022

There's a bit of logspam, not sure if this is because I pulled your branch first and merged this over:

[VERBOSE-2:backdrop_filter_layer.cc(58)] Inherited opacity: 0x70000a6b8f30
[VERBOSE-2:backdrop_filter_layer.cc(58)] Inherited opacity: 0x70000a6b8f30
[VERBOSE-2:backdrop_filter_layer.cc(58)] Inherited opacity: 0x70000a6b8f30
... (~20x)

Thanks for checking! There should be a pre-check that maybe doesn't reject pushes, but indicates if there are new calls to FML_LOG in them.

Actually, I don't see that any more. Were you running off the patch I sent for testing rather than the PR patch?

Yep, you're right. I pulled but didn't rebuild, sorry about that. No logs in the most recent commit :)

@auto-submit auto-submit bot merged commit 5c86c96 into flutter:main Jul 7, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 7, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 7, 2022
@engine-flutter-autoroll
5c86c96 Mark BackdropFilter as inheriting opacity, enabling fading of…
7b6e563 
… BDFs (flutter/engine#34435)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 7, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 7, 2022
@engine-flutter-autoroll
5c86c96 Mark BackdropFilter as inheriting opacity, enabling fading of…
5e8aee2 
… BDFs (flutter/engine#34435)
@flar flar mentioned this pull request Jul 7, 2022
Open
@antholeole antholeole mentioned this pull request Jul 12, 2022
Merged
17 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@bdero bdero bdero approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@flar @antholeole @bdero