engine icon indicating copy to clipboard operation
engine copied to clipboard

Published 20 hours ago • verified publisher icon flutter

PlatformView Blur for Backdrop Filter

Open emilyabest opened this issue 2 years ago • 5 comments

Description: This PR applies a Gaussian Blur filter to PlatformViews. Currently, the blur is applied when transform mutations are called, however this PR will be combined with PR #34355 (Create BackdropFilter Mutator) to blur PlatformViews for the backdrop filter mutator.

Fixed issues:

  • PlatformView was not programmed to blur

Pre-launch Checklist

  • [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
  • [x] I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • [x] I listed at least one issue that this PR fixes in the description above.
  • [x] 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.
  • [x] I updated/added relevant documentation (doc comments with ///).
  • [x] 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.

emilyabest avatar Jul 11 '22 23:07 emilyabest

Also need to rebase or merge upstream/main and resolve conflicts.

cyanglaz avatar Jul 12 '22 17:07 cyanglaz

The new tests seem to be failing: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8808283911804283841/+/u/test:Host_Tests_for_ios_debug_sim__2/stdout?format=raw

cyanglaz avatar Jul 18 '22 17:07 cyanglaz

@cyanglaz Looks like all the comments have been addressed. But @emilyabest, the test failures do look related.

chinmaygarde avatar Jul 21 '22 20:07 chinmaygarde

@cyanglaz Looks like all the comments have been addressed. But @emilyabest, the test failures do look related.

@chinmaygarde We realized the implementation is missing functionality for multiple backdrop filters and removing backdrop filters. I'm testing that code now and will look into the failing tests after pushing it to the PR. Thanks for looking over the comments!

emilyabest avatar Jul 21 '22 20:07 emilyabest

Looks like there are some tests failures on Mac Unopt for -[FlutterPlatformViewsTest testEditBackdropFilters] The Linux test failure is unrelated, you can resolve it by rebasing main.

cyanglaz avatar Jul 26 '22 01:07 cyanglaz

The scenario test is failing due to blur is implemented in this PR. However, the scenario app uses a none-zero origin for the PlatformView, making the blur not working probably. We need to fix the step 1 in https://github.com/flutter/flutter/issues/109700 before landing this PR.

cyanglaz avatar Aug 17 '22 16:08 cyanglaz

Need to merge https://github.com/flutter/engine/pull/34355 before landing this.

cyanglaz avatar Aug 19 '22 17:08 cyanglaz

@cyanglaz Trying to understand the status here. Per your last comment, this patch depends on https://github.com/flutter/engine/pull/34355 which landed but had to be reverted due to https://github.com/flutter/flutter/issues/109831 and now the perf fixes are going to be incorporated here. Is that right?

chinmaygarde avatar Aug 25 '22 20:08 chinmaygarde

@cyanglaz Trying to understand the status here. Per your last comment, this patch depends on #34355 which landed but had to be reverted due to flutter/flutter#109831 and now the perf fixes are going to be incorporated here. Is that right?

Yes, https://github.com/flutter/engine/pull/34355 is now included in this PR with the fix. This PR is now waiting for https://github.com/flutter/engine/pull/35501 to land so the scenario app's backdrop filter blur screenshot can be updated more appropriately in this PR.

cyanglaz avatar Aug 25 '22 23:08 cyanglaz

Wait until https://github.com/flutter/flutter/pull/110688 is landed before landing this.

cyanglaz avatar Sep 01 '22 18:09 cyanglaz