Skip to content
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

Add BackdropFilter blend mode #80129

Merged
merged 6 commits into from Apr 27, 2021

Conversation

flar
Copy link
Contributor

@flar flar commented Apr 9, 2021

This PR is the framework half of flutter/engine#19631 which provides a way to control the blending of the BackdropFilter filter output onto the scene important for cases where the widget is inside of another parent widget that uses a saveLayer - most typically an Opacity widget.

I'd be particularly interested in hearing from @ferhatb as to whether the caveats about using this feature with the DOM renderer are appropriately worded (and in particular, how should we canonically refer to that platform?).

@flar flar requested review from Hixie and ferhatb April 9, 2021 17:13
@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Apr 9, 2021
@google-cla google-cla bot added the cla: yes label Apr 9, 2021
@skia-gold
Copy link

Gold has detected about 1 untriaged digest(s) on patchset 2.
View them at https://flutter-gold.skia.org/cl/github/80129

@flar flar force-pushed the add-backdropfilter-blend-mode branch from 725ba72 to 27a208c Compare April 9, 2021 17:19
@flar
Copy link
Contributor Author

flar commented Apr 12, 2021

I fixed a minor discrepancy I noticed in the BackdropFilterLayer class - markNeedsAddToScene should only be called if the value changes.

@skia-gold
Copy link

Gold has detected about 1 untriaged digest(s) on patchset 3.
View them at https://flutter-gold.skia.org/cl/github/80129

@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #80129 at sha ccd62850adfc9234f1673a0edd7b8852a417a545

@flutter-dashboard flutter-dashboard bot added the will affect goldens Changes to golden files label Apr 13, 2021
@flar
Copy link
Contributor Author

flar commented Apr 13, 2021

@ferhatb one of the golden images showed the blend mode not applying like the others. It's listed as Chrome on Linux, but there is no indication of whether it is using the DOM or the CanvasKit renderer. I'm assuming that it is the DOM renderer, but since the results don't really convey this, it is hard to click on "Approve" here. Is there a way we can factor the renderer into the results so that we can be more certain of the conditions under which we are approving the golden images?

@flar
Copy link
Contributor Author

flar commented Apr 14, 2021

With input from @yjbanov on Discord, I've approved the last golden change. It is the expected output from the HTML renderer (until HTML standards support this property).

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM if the web team is happy with the wording.

Looks like this will need to be rebased to the latest master to make CI happy.

packages/flutter/lib/src/rendering/layer.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/rendering/layer.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/rendering/proxy_box.dart Outdated Show resolved Hide resolved
@flar
Copy link
Contributor Author

flar commented Apr 17, 2021

I think I've pushed changes for all of the feedback, let me know if I missed something.

@flar flar force-pushed the add-backdropfilter-blend-mode branch from 8e6e922 to a9e9d47 Compare April 19, 2021 21:41
@flar flar force-pushed the add-backdropfilter-blend-mode branch from 0c4d50f to 08e27e9 Compare April 22, 2021 23:11
@flar
Copy link
Contributor Author

flar commented Apr 23, 2021

I believe I have resolved the outstanding issues. @goderbauer ?

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
framework flutter/packages/flutter repository. See also f: labels. will affect goldens Changes to golden files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants