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

[iOS] Avoid crash when backdrop filter is null for PlatformViews #43150

Merged
merged 3 commits into from
Jul 5, 2023
Merged

[iOS] Avoid crash when backdrop filter is null for PlatformViews #43150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
081821b
draft
Jun 22, 2023
601b35a
forma
Jun 22, 2023
c8c719d
do not add backdrop filter to mutator stack if the filter is null
Jun 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion flow/layers/backdrop_filter_layer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ void BackdropFilterLayer::Diff(DiffContext* context, const Layer* old_layer) {
void BackdropFilterLayer::Preroll(PrerollContext* context) {
Layer::AutoPrerollSaveLayerState save =
Layer::AutoPrerollSaveLayerState::Create(context, true, bool(filter_));
if (context->view_embedder != nullptr) {
if (filter_ && context->view_embedder != nullptr) {
context->view_embedder->PushFilterToVisitedPlatformViews(
filter_, context->state_stack.device_cull_rect());
}
Expand Down
4 changes: 4 additions & 0 deletions testing/scenario_app/ios/Scenarios/Scenarios.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
6860CE272A01B2FF00B68EC5 /* golden_two_platform_view_clip_path_iPhone SE (3rd generation)_16.2_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 6860CE242A01B2FF00B68EC5 /* golden_two_platform_view_clip_path_iPhone SE (3rd generation)_16.2_simulator.png */; };
68A5B63423EB71D300BDBCDB /* PlatformViewGestureRecognizerTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 68A5B63323EB71D300BDBCDB /* PlatformViewGestureRecognizerTests.m */; };
68D4017D2564859300ECD91A /* ContinuousTexture.m in Sources */ = {isa = PBXBuildFile; fileRef = 68D4017C2564859300ECD91A /* ContinuousTexture.m */; };
68D93AEE2A46097E0054AB6D /* golden_platform_view_with_negative_backdrop_filter_iPhone SE (3rd generation)_16.2_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 68D93AED2A46097E0054AB6D /* golden_platform_view_with_negative_backdrop_filter_iPhone SE (3rd generation)_16.2_simulator.png */; };
F26F15B8268B6B5600EC54D3 /* iPadGestureTests.m in Sources */ = {isa = PBXBuildFile; fileRef = F26F15B7268B6B5500EC54D3 /* iPadGestureTests.m */; };
/* End PBXBuildFile section */

Expand Down Expand Up @@ -183,6 +184,7 @@
68A5B63323EB71D300BDBCDB /* PlatformViewGestureRecognizerTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = PlatformViewGestureRecognizerTests.m; sourceTree = "<group>"; };
68D4017B2564859300ECD91A /* ContinuousTexture.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ContinuousTexture.h; sourceTree = "<group>"; };
68D4017C2564859300ECD91A /* ContinuousTexture.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ContinuousTexture.m; sourceTree = "<group>"; };
68D93AED2A46097E0054AB6D /* golden_platform_view_with_negative_backdrop_filter_iPhone SE (3rd generation)_16.2_simulator.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "golden_platform_view_with_negative_backdrop_filter_iPhone SE (3rd generation)_16.2_simulator.png"; sourceTree = "<group>"; };
F26F15B7268B6B5500EC54D3 /* iPadGestureTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = iPadGestureTests.m; sourceTree = "<group>"; };
F72114B628EF99F500184A2D /* Info_Impeller.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = Info_Impeller.plist; sourceTree = "<group>"; };
/* End PBXFileReference section */
Expand Down Expand Up @@ -308,6 +310,7 @@
F7B464DC2759D02B00079189 /* Goldens */ = {
isa = PBXGroup;
children = (
68D93AED2A46097E0054AB6D /* golden_platform_view_with_negative_backdrop_filter_iPhone SE (3rd generation)_16.2_simulator.png */,
6860CE242A01B2FF00B68EC5 /* golden_two_platform_view_clip_path_iPhone SE (3rd generation)_16.2_simulator.png */,
6860CE232A01B2FF00B68EC5 /* golden_two_platform_view_clip_rect_iPhone SE (3rd generation)_16.2_simulator.png */,
6860CE222A01B2FF00B68EC5 /* golden_two_platform_view_clip_rrect_iPhone SE (3rd generation)_16.2_simulator.png */,
Expand Down Expand Up @@ -477,6 +480,7 @@
684FFF8629F9C10700281002 /* golden_bogus_font_text_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
684FFF7E29F9C10700281002 /* golden_platform_view_opacity_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
684FFF8729F9C10700281002 /* golden_platform_view_multiple_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
68D93AEE2A46097E0054AB6D /* golden_platform_view_with_negative_backdrop_filter_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
684FFF8D29F9C10700281002 /* golden_platform_view_large_cliprrect_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
684FFF8329F9C10700281002 /* golden_platform_view_transform_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
684FFF8B29F9C10700281002 /* golden_platform_view_clippath_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
Expand Down
2 changes: 2 additions & 0 deletions testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ - (BOOL)application:(UIApplication*)application
@"--platform-view-with-other-backdrop-filter" : @"platform_view_with_other_backdrop_filter",
@"--two-platform-views-with-other-backdrop-filter" :
@"two_platform_views_with_other_backdrop_filter",
@"--platform-view-with-negative-backdrop-filter" :
@"platform_view_with_negative_backdrop_filter",
@"--platform-view-rotate" : @"platform_view_rotate",
@"--non-full-screen-flutter-view-platform-view" : @"non_full_screen_flutter_view_platform_view",
@"--gesture-reject-after-touches-ended" : @"platform_view_gesture_reject_after_touches_ended",
Expand Down
2 changes: 2 additions & 0 deletions testing/scenario_app/ios/Scenarios/ScenariosUITests/GoldenTestManager.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ - (instancetype)initWithLaunchArg:(NSString*)launchArg {
@"--platform-view-with-other-backdrop-filter" : @"platform_view_with_other_backdrop_filter",
@"--two-platform-views-with-other-backdrop-filter" :
@"two_platform_views_with_other_backdrop_filter",
@"--platform-view-with-negative-backdrop-filter" :
@"platform_view_with_negative_backdrop_filter",
@"--platform-view-rotate" : @"platform_view_rotate",
@"--non-full-screen-flutter-view-platform-view" :
@"non_full_screen_flutter_view_platform_view",
Expand Down
18 changes: 18 additions & 0 deletions testing/scenario_app/ios/Scenarios/ScenariosUITests/PlatformViewUITests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,24 @@ - (void)testPlatformView {

@end

@interface PlatformViewWithNegativeOtherBackDropFilterTests : GoldenPlatformViewTests

@end

@implementation PlatformViewWithNegativeOtherBackDropFilterTests

- (instancetype)initWithInvocation:(NSInvocation*)invocation {
GoldenTestManager* manager = [[GoldenTestManager alloc]
initWithLaunchArg:@"--platform-view-with-negative-backdrop-filter"];
return [super initWithManager:manager invocation:invocation];
}

- (void)testPlatformView {
[self checkPlatformViewGolden];
}

@end

@interface PlatformViewRotation : GoldenPlatformViewTests
@end

Expand Down
Binary file added ...iew_with_negative_backdrop_filter_iPhone SE (3rd generation)_16.2_simulator.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
64 changes: 62 additions & 2 deletions testing/scenario_app/lib/src/platform_view.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1443,8 +1443,6 @@ class PlatformViewWithOtherBackDropFilter extends PlatformViewScenario {
/// 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.

/// the result will not including a filtered pv1.
class TwoPlatformViewsWithOtherBackDropFilter extends Scenario
with _BasePlatformViewScenarioMixin {
/// Constructs the scenario.
Expand Down Expand Up @@ -1533,6 +1531,68 @@ class TwoPlatformViewsWithOtherBackDropFilter extends Scenario
}
}

/// 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.

/// https://github.com/flutter/flutter/issues/127095
class PlatformViewWithNegativeBackDropFilter extends Scenario
with _BasePlatformViewScenarioMixin {
/// Constructs the scenario.
PlatformViewWithNegativeBackDropFilter(
super.view, {
required int id,
}) : _id = id;

final int _id;

@override
void onBeginFrame(Duration duration) {
final SceneBuilder builder = SceneBuilder();

final PictureRecorder recorder = PictureRecorder();
final Canvas canvas = Canvas(recorder);
// This is just a background picture to make the result more viewable.
canvas.drawRect(
const Rect.fromLTRB(0, 0, 600, 1000),
Paint()..color = const Color(0xFFFF0000),
);
canvas.drawRect(
const Rect.fromLTRB(0, 0, 300, 300),
Paint()..color = const Color(0xFF00FF00),
);
final Picture picture1 = recorder.endRecording();
builder.addPicture(Offset.zero, picture1);

builder.pushOffset(0, 200);

addPlatformView(
_id,
dispatcher: view.platformDispatcher,
sceneBuilder: builder,
width: 100,
height: 100,
text: 'platform view 1'
);

final PictureRecorder recorder2 = PictureRecorder();
final Canvas canvas2 = Canvas(recorder2);
canvas2.drawCircle(
const Offset(200, 100),
50,
Paint()..color = const Color(0xFF0000EF),
);
final Picture picture2 = recorder2.endRecording();
builder.addPicture(const Offset(100, 100), picture2);

final ImageFilter filter = ImageFilter.blur(sigmaX: -8, sigmaY: 8);
builder.pushBackdropFilter(filter);

final Scene scene = builder.build();
view.render(scene);
scene.dispose();
}
}

/// Builds a scenario where many platform views are scrolling and pass under a picture.
class PlatformViewScrollingUnderWidget extends Scenario
with _BasePlatformViewScenarioMixin {
Expand Down
1 change: 1 addition & 0 deletions testing/scenario_app/lib/src/scenarios.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Map<String, _ScenarioFactory> _scenarios = <String, _ScenarioFactory>{
'platform_view_opacity': (FlutterView view) => PlatformViewOpacityScenario(view, id: _viewId++),
'platform_view_with_other_backdrop_filter': (FlutterView view) => PlatformViewWithOtherBackDropFilter(view, id: _viewId++),
'two_platform_views_with_other_backdrop_filter': (FlutterView view) => TwoPlatformViewsWithOtherBackDropFilter(view, firstId: _viewId++, secondId: _viewId++),
'platform_view_with_negative_backdrop_filter': (FlutterView view) => PlatformViewWithNegativeBackDropFilter(view, id: _viewId++),
'platform_view_multiple': (FlutterView view) => MultiPlatformViewScenario(view, firstId: _viewId++, secondId: _viewId++),
'platform_view_multiple_background_foreground': (FlutterView view) => MultiPlatformViewBackgroundForegroundScenario(view, firstId: _viewId++, secondId: _viewId++),
'non_full_screen_flutter_view_platform_view': (FlutterView view) => NonFullScreenFlutterViewPlatformViewScenario(view, id: _viewId++),
Expand Down