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

Conversation

@cyanglaz
Copy link
Contributor

During certain partial repaint logic, the backdrop filter layer is present but the filter itself is null. We need to account for it when deciding whether to add such filter to the mutator stack.

Fixes flutter/flutter#127095

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.

@cyanglaz cyanglaz requested a review from hellohuanlin June 23, 2023 17:19
/// A simple platform view for testing backDropFilter with a platform view in the scene.
///
/// The stack would look like: picture 1 -> pv1 -> picture 2 -> filter -> pv2 - > picture 3.
/// Because backdrop filter on platform views has not been implemented(see: https://github.com/flutter/flutter/issues/43902),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing this out dated comment, the backdrop filter on platform views have been implemented.

@cyanglaz
Copy link
Contributor Author

cyanglaz commented Jun 26, 2023

@hellohuanlin Friendly ping :) Do you mind to review this?


/// A simple platform view for testing backDropFilter with a platform view in the scene.
///
/// The backdrop filter sigma value is negative, which tries to reproduce a crash, see:
Copy link
Contributor

Choose a reason for hiding this comment

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

curious what does a negative sigma mean? is it not supposed to happen? maybe make sense to print a warning if so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not entirely sure why the negative sigma is used in one of the example code to reproduce the bug, however, the fix is more about accounting the possibility where the filter object is null.

As a follow up, I think we also need to disable partial repaint when there is a platform view.

@cyanglaz cyanglaz added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 5, 2023
@auto-submit auto-submit bot merged commit 01bb6af into flutter:main Jul 5, 2023
@sang-makebetter
Copy link

@hellohuanlin Hello, can I ask on which Flutter release that will apply these changes. Thank you!

@cyanglaz cyanglaz deleted the backdrop_filter_platform_view_crash branch July 6, 2023 17:26
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 6, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 6, 2023
…sions) (#130097)

Manual roll Flutter Engine from bd2e42b203e1 to 06c936205d96 (27 revisions)

Manual roll requested by [email protected]

flutter/engine@bd2e42b...06c9362

2023-07-06 [email protected] Handle nested display list clips in Impeller dispatcher (flutter/engine#43442)
2023-07-06 [email protected] Add Look Up to iOS selection controls (flutter/engine#43308)
2023-07-06 [email protected] Roll Skia from 6aaef9cbc3af to 10a43e57e0a6 (1 revision) (flutter/engine#43444)
2023-07-06 [email protected] Roll Skia from 620de5ac9f6b to 6aaef9cbc3af (1 revision) (flutter/engine#43441)
2023-07-06 [email protected] Revert "[iOS][Keyboard] Wait vsync on UI thread and update viewport inset to avoid jitter." (flutter/engine#43422)
2023-07-06 [email protected] Roll ICU from a2961dc659b4 to e8c3bc9ea97d (7 revisions) (flutter/engine#43440)
2023-07-06 [email protected] Further shard clang-tidy runs (flutter/engine#43428)
2023-07-06 [email protected] Release log capture at end of test (flutter/engine#43429)
2023-07-06 [email protected] Roll Fuchsia Mac SDK from uGhka7LBG80dM_I3a... to 3D0ft09qP4-Hp_3mQ... (flutter/engine#43439)
2023-07-06 [email protected] Roll Fuchsia Linux SDK from amH2MWTworhTLdMWy... to MEXi9Xyj7u24PSx_J... (flutter/engine#43438)
2023-07-06 [email protected] Use Windows Display Language (flutter/engine#43341)
2023-07-06 [email protected] Roll Skia from b2ba6e1d8c0e to 620de5ac9f6b (2 revisions) (flutter/engine#43437)
2023-07-06 [email protected] Roll Skia from 50e866b51a64 to b2ba6e1d8c0e (2 revisions) (flutter/engine#43436)
2023-07-06 [email protected] Roll Skia from 8ed969b60e98 to 50e866b51a64 (1 revision) (flutter/engine#43435)
2023-07-06 [email protected] Fix typo in painting.dart (flutter/engine#43378)
2023-07-06 [email protected] Roll Fuchsia Mac SDK from C3Q7MJBYkiin8zw-f... to uGhka7LBG80dM_I3a... (flutter/engine#43433)
2023-07-06 [email protected] Roll Fuchsia Linux SDK from iwgWLB4KaXslnaGwK... to amH2MWTworhTLdMWy... (flutter/engine#43432)
2023-07-05 [email protected] Roll Skia from a39421eb8d7b to 8ed969b60e98 (6 revisions) (flutter/engine#43431)
2023-07-05 [email protected] [Impeller] Allocate fewer textures in dedicated memory and adjust buffer flags. (flutter/engine#43313)
2023-07-05 [email protected] Roll Skia from d6e941012b87 to a39421eb8d7b (4 revisions) (flutter/engine#43427)
2023-07-05 [email protected] Roll Skia from e0b9e047332d to d6e941012b87 (2 revisions) (flutter/engine#43424)
2023-07-05 [email protected] [iOS] Avoid crash when backdrop filter is null for PlatformViews (flutter/engine#43150)
2023-07-05 [email protected] Roll Skia from 401c397f7235 to e0b9e047332d (1 revision) (flutter/engine#43420)
2023-07-05 [email protected] [Windows] Fix `FlutterWindow::GetNativeViewAccessible` crash (flutter/engine#43368)
2023-07-05 [email protected] [iOS] Fix FlutterViewController retain cycle (flutter/engine#43379)
2023-07-05 [email protected] Roll Skia from 23052eb28808 to 401c397f7235 (1 revision) (flutter/engine#43419)
2023-07-05 [email protected] Roll Skia from d58324bf653d to 23052eb28808 (3 revisions) (flutter/engine#43418)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from iwgWLB4KaXsl to MEXi9Xyj7u24
  fuchsia/sdk/core/mac-amd64 from C3Q7MJBYkiin to 3D0ft09qP4-H

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
...
cyanglaz pushed a commit to cyanglaz/engine that referenced this pull request Jul 10, 2023
…tter#43150)

During certain partial repaint logic, the backdrop filter layer is present but the filter itself is null. We need to account for it when deciding whether to add such filter to the mutator stack.

Fixes flutter/flutter#127095

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
kjlubick pushed a commit to kjlubick/engine that referenced this pull request Jul 14, 2023
…tter#43150)

During certain partial repaint logic, the backdrop filter layer is present but the filter itself is null. We need to account for it when deciding whether to add such filter to the mutator stack.

Fixes flutter/flutter#127095

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
iassal pushed a commit to iassal/engine that referenced this pull request Jul 26, 2023
…tter#43150)

During certain partial repaint logic, the backdrop filter layer is present but the filter itself is null. We need to account for it when deciding whether to add such filter to the mutator stack.

Fixes flutter/flutter#127095

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iOS crash when using PlatformViews and BackdropFilter

3 participants