PlatformView partial blur #36015

cyanglaz · 2022-09-08T21:42:43Z

This PR makes the PlatformView able to be partially blurred based on the cullrect.
This PR modifies the UIVisualEffectView's filters so that it uses the correct inputRadius from the backdropfilter widget.
The UIVisualEffectView is then set to the correct frame based on cullrect, added to the PlatformView as a subview.

Fixes: flutter/flutter#50183, flutter/flutter#113508

Things to do in this PR:
~~1. Unit Tests~~
~~2. Scenario Tests~~
~~3. Manual submission tests since some private API is accessed.~~
~~4. Design doc update.~~

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.

LongCatIsLooong

So behavior-wise, this only changes the Rect of the blur?

LongCatIsLooong · 2022-10-21T03:50:08Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

-        // We only support DlBlurImageFilter for BackdropFilter.
-        if ((*iter)->GetFilter().asBlur() && canApplyBlurBackdrop) {
+        // Only support DlBlurImageFilter for BackdropFilter.
+        if ((*iter)->GetFilterMutation().GetFilter().asBlur() && canApplyBlurBackdrop) {


Is it possible to stop pushing the mutation if it's not supported?

Could be an optimization in the future. Pushing is done in the shared code across all platforms, to stop pushing we would have to create a way for the iOS PlatformView communicate back to the layer tree.

LongCatIsLooong · 2022-10-21T03:58:26Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

+          CGFloat blurRadius = (*iter)->GetFilterMutation().GetFilter().asBlur()->sigma_x();
+          CGRect filterRect =
+              flutter::GetCGRectFromSkRect((*iter)->GetFilterMutation().GetFilterRect());
+          // `filterRect` reprents the rect that shouuld be filtered inside the `flutter_view_`.


nit: typo, should

LongCatIsLooong · 2022-10-21T04:05:43Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

+          // The `PlatformViewFilter` needs the frame inside the `clipView` that needs to be
+          // filtered.
+          CGRect intersection = CGRectIntersection(filterRect, clipView.frame);
+          if (CGRectContainsRect(clipView.frame, intersection)) {


nit: CGRectIsNull(_:) instead?

I feel like CGRectContainsRect is more readable. Also, we would have to check both [CGRectIsNull(_:)](https://developer.apple.com/documentation/coregraphics/1455471-cgrectisnull) and CGRectIsEmpty if going the other route.

https://developer.apple.com/documentation/coregraphics/1455346-cgrectintersection says use CGRectIsNull to check if the two rects don't overlap.

LongCatIsLooong · 2022-10-21T04:38:59Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm

+
+- (instancetype)initWithFrame:(CGRect)frame
+                   blurRadius:(CGFloat)blurRadius
+             visualEffectView:(UIVisualEffectView*)visualEffectView {


you could copy an existing filter instead of instantiating a new UIVisualEffectView and finding the filter?

So we need a new UIVisualEffectView instance to blur anyway. The problem comes down to these 2 approaches:
1, We store a gaussianFilter Object after first time, and copy it later when needed. (I think this is what you meant)
2. We get the gaussianFilter for the new UIVisualEffectView instance.

Option 1 takes an addition space to store the gaussianFilter
Option 2 takes an addition loop ( for (NSObject* filter in view.layer.filters)) to find the filter, which is currently looping through 2 filters.

I feel like both are similar, maybe Option 2 is less future proof if more filters are added to the layer in some future SDK?

Also you'd have to allocate space for a UIVisualEffectView and run its initializer every time. But probably not a big deal.

I have updated the code to cache a gaussianFilter object and eliminated code to find the views, filters etc, PTAL.

LongCatIsLooong · 2022-10-21T04:57:43Z

flow/embedded_views.h

+  const SkRect& GetFilterRect() const { return filter_rect_; }
+
+  bool operator==(const ImageFilterMutation& other) const {
+    return *filter_ == *other.filter_ && filter_rect_ == other.filter_rect_;


Is this comparing pointer equality?

This is comparing the data. I added a test for it in my latest commit

LongCatIsLooong · 2022-10-24T16:33:44Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

+          // The `PlatformViewFilter` needs the frame inside the `clipView` that needs to be
+          // filtered.
+          CGRect intersection = CGRectIntersection(filterRect, clipView.frame);
+          if (CGRectContainsRect(clipView.frame, intersection)) {


https://developer.apple.com/documentation/coregraphics/1455346-cgrectintersection says use CGRectIsNull to check if the two rects don't overlap.

LongCatIsLooong · 2022-10-24T16:49:09Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm

+
+- (instancetype)initWithFrame:(CGRect)frame
+                   blurRadius:(CGFloat)blurRadius
+             visualEffectView:(UIVisualEffectView*)visualEffectView {


Also you'd have to allocate space for a UIVisualEffectView and run its initializer every time. But probably not a big deal.

LongCatIsLooong · 2022-10-25T21:20:28Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm

+        if ([[filter valueForKey:@"name"] isEqual:@"gaussianBlur"] &&
+            [[filter valueForKey:@"inputRadius"] isKindOfClass:[NSNumber class]]) {
+          GaussianBlurFilter = filter;
+          IsUIVisualEffectViewImplementationValid = YES;


Do you still need this? You can check if the index < 0 I think?

I think you were somehow looking at an old version of the code.

LongCatIsLooong · 2022-10-25T21:21:15Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm

+    return;
+  }
+  NSUInteger index = 0;
+  for (UIView* view in visualEffectView.subviews) {


nit: if you need the index, use for(int i; i < length; i++) instead?

I thought fast enumeration (for x in y) is faster? Probably doesn't really matter in this case because the number of subviews won't be too big. I will take the suggestion to avoid future confusion since the performance difference will be very insignificant.

The enumeration is considerably more efficient than, for example, using NSEnumerator directly.

It's faster than NSEnumerator. I think with NSArray you have random access so it probably won't be slower than for(x in y).

LongCatIsLooong · 2022-10-25T21:29:29Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm

 }

+ (void)prepareIfNecessary:(UIVisualEffectView*)visualEffectView {
+  if (GaussianBlurFilter) {


What is GaussianBlurFilter for? It's seems it's not referenced anywhere else?

I think you were somehow looking at an old version of the code.

This reverts commit abc3aab.

…114178) * c33eae133 Macos re-enable TestSetMenu (flutter/engine#37058) * abc3aabad PlatformView partial blur (flutter/engine#36015) * 8fcf413f8 Revert "PlatformView partial blur" (flutter/engine#37085) * 8bb0441e5 Revert "Fixes Android text field to use hint text for accessibility (#36846)" (flutter/engine#37083)

flutter-dashboard bot added the platform-ios label Sep 8, 2022

cyanglaz mentioned this pull request Sep 8, 2022

Alert dialog is transparent when they are drawn over platform view on iOS flutter/flutter#50183

Closed

cyanglaz force-pushed the platform_view_filter_view branch 2 times, most recently from c1e3f4e to 4445f86 Compare October 18, 2022 20:01

cyanglaz mentioned this pull request Oct 18, 2022

[iOS] Entire PlatformView (WebView) become blurred when using ImageFilter.blur flutter/flutter#113508

Closed

cyanglaz requested a review from LongCatIsLooong October 18, 2022 20:07

Chris Yang added 2 commits October 20, 2022 09:33

Use UIVisualEffectView to blur

211e9cc

update golden

a0fc0b9

cyanglaz force-pushed the platform_view_filter_view branch from b060125 to a0fc0b9 Compare October 20, 2022 16:37

Chris Yang added 2 commits October 20, 2022 10:25

clean

3ffceb5

clean

598a11b

cyanglaz marked this pull request as ready for review October 20, 2022 17:37

LongCatIsLooong reviewed Oct 21, 2022

View reviewed changes

review

d3fb40a

LongCatIsLooong approved these changes Oct 24, 2022

View reviewed changes

Chris Yang added 3 commits October 24, 2022 11:44

review

5c1e65a

prepareOnce

d4d74c1

rename static vars

5c0a33e

LongCatIsLooong reviewed Oct 25, 2022

View reviewed changes

do no use fast enumeration

89f648e

cyanglaz added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 27, 2022

auto-submit bot merged commit abc3aab into flutter:main Oct 27, 2022

cyanglaz pushed a commit that referenced this pull request Oct 27, 2022

Revert "PlatformView partial blur (#36015)"

67ebf99

This reverts commit abc3aab.

cyanglaz mentioned this pull request Oct 27, 2022

Revert "PlatformView partial blur" #37085

Merged

auto-submit bot mentioned this pull request Oct 27, 2022

Review request for Revert PR flutter/engine#37085 flutter/flutter#114170

Closed

cyanglaz mentioned this pull request Oct 27, 2022

Reland "PlatformView partial blur #36015" #37086

Merged

8 tasks

engine-flutter-autoroll mentioned this pull request Oct 27, 2022

Roll Flutter Engine from ecb513689262 to 8fcf413f8f41 (3 revisions) flutter/flutter#114172

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 27, 2022

abc3aabad PlatformView partial blur (flutter/engine#36015)

4ac78fe

engine-flutter-autoroll mentioned this pull request Oct 27, 2022

Roll Flutter Engine from ecb513689262 to 8bb0441e592f (4 revisions) flutter/flutter#114178

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 27, 2022

abc3aabad PlatformView partial blur (flutter/engine#36015)

9ce4ce6

auto-submit bot pushed a commit that referenced this pull request Oct 28, 2022

Reland "PlatformView partial blur #36015" (#37086)

8b242e0

schwa423 pushed a commit to schwa423/engine that referenced this pull request Nov 16, 2022

Reland "PlatformView partial blur flutter#36015" (flutter#37086)

80b85ef

jmagman added the blog Items that should be included in a blog label Jan 4, 2023

PlatformView partial blur #36015

PlatformView partial blur #36015

Uh oh!

Conversation

cyanglaz commented Sep 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

LongCatIsLooong left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cyanglaz Oct 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cyanglaz commented Sep 8, 2022 •

edited

Loading

cyanglaz Oct 21, 2022 •

edited

Loading