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

[html] fix: combine backdrop filter to child container to fix rendering issue on Chromium #43392

Closed
wants to merge 3 commits into from

Conversation

Kingtous
Copy link
Contributor

@Kingtous Kingtous commented Jul 4, 2023

Hi from Dora AI team, which powers by Flutter Web to boost devs to build their 3D websites in just few seconds. We've encountered the backdrop-filter problem when running in HTML mode.

This PR combines the backdrop-filter with the interior one, which fixes flutter/flutter#129896.

image

The reason may be the different behavior when treating backdrop-filter properties by Chromium, and Safari. Only Chrome seems to follow the W3C draft, mentioned by Kawalo here (https://stackoverflow.com/a/76207141/11069997), which causes the inconsistent result with canvaskit or other browsers running on HTML mode.

I've tested the main problem on Chrome is that the backdrop filter should not be a leaf on the DOM node, and should be directly applied to its children. If we apply it on its children, the backdrop filter works great and has no issues with the nested backdrop filter.

This bug is reported a year ago, but it seems they do not have a schedule to fix this issue. So this workaround is
necessary now to be applied to the Flutter engine to fix the rendering problem that happens on Chromium-based browsers when running in HTML mode.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

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

@google-cla
Copy link

google-cla bot commented Jul 4, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot added the platform-web Code specifically for the web engine label Jul 4, 2023
@Kingtous Kingtous changed the title fix: combine backdrop filter to child container to fix rendering issue on Chromium [Web/HTML] fix: combine backdrop filter to child container to fix rendering issue on Chromium Jul 4, 2023
@Kingtous Kingtous changed the title [Web/HTML] fix: combine backdrop filter to child container to fix rendering issue on Chromium [html] fix: combine backdrop filter to child container to fix rendering issue on Chromium Jul 4, 2023
@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.

Changes reported for pull request #43392 at sha 2d6e2e0

@Kingtous Kingtous marked this pull request as draft July 4, 2023 07:58
@Kingtous Kingtous marked this pull request as ready for review July 5, 2023 06:11
@flutter-dashboard
Copy link

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #43392 at sha 69319a1

@Kingtous Kingtous marked this pull request as draft July 5, 2023 06:25
@flutter-dashboard
Copy link

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

@Hixie
Copy link
Contributor

Hixie commented Aug 22, 2023

@Kingtous thanks for your contribution! is this still something you are interested in working on? How can we help you?

@Hixie
Copy link
Contributor

Hixie commented Nov 7, 2023

@KDogeOne Thanks for your contribution; I'm going to close this PR for now but please don't hesitate to open a new one if you wish to get back to it! In the meantime I will link to it from the issue in case anyone else wants to take it over. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs tests platform-web Code specifically for the web engine will affect goldens
Projects
None yet
2 participants