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

[platform_view] fix overlapping platform view not touchable #39527

Merged
merged 4 commits into from
Feb 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 3 additions & 16 deletions shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm
Original file line number Diff line number Diff line change
Expand Up @@ -771,29 +771,16 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
void FlutterPlatformViewsController::BringLayersIntoView(LayersMap layer_map) {
FML_DCHECK(flutter_view_);
UIView* flutter_view = flutter_view_.get();
auto zIndex = 0;
// Clear the `active_composition_order_`, which will be populated down below.
active_composition_order_.clear();
for (size_t i = 0; i < composition_order_.size(); i++) {
int64_t platform_view_id = composition_order_[i];
std::vector<std::shared_ptr<FlutterPlatformViewLayer>> layers = layer_map[platform_view_id];
UIView* platform_view_root = root_views_[platform_view_id].get();

if (platform_view_root.superview != flutter_view) {
[flutter_view addSubview:platform_view_root];
}
// Make sure the platform_view_root is higher than the last platform_view_root in
// composition_order_.
platform_view_root.layer.zPosition = zIndex++;

// `addSubview` will automatically reorder subview if it is already added.
[flutter_view addSubview:platform_view_root];
for (const std::shared_ptr<FlutterPlatformViewLayer>& layer : layers) {
if ([layer->overlay_view_wrapper.get() superview] != flutter_view) {
[flutter_view addSubview:layer->overlay_view_wrapper];
}
// Make sure all the overlays are higher than the platform view.
layer->overlay_view_wrapper.get().layer.zPosition = zIndex++;
FML_DCHECK(layer->overlay_view_wrapper.get().layer.zPosition >
platform_view_root.layer.zPosition);
[flutter_view addSubview:layer->overlay_view_wrapper];
Copy link
Member

Choose a reason for hiding this comment

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

Can you manually test that this change didn't regress flutter/flutter#86787 (see #29930)

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 can confirm that it didn't regress flutter/flutter#86787.

Though I found a different regression (see pic below). The navigation bar becomes transparent, so the platform view is displayed underneath. This seems to be similar to flutter/flutter#119485.

It is NOT caused by this PR because I get the same behaviors with or without this change; but it is a regression, because the released build is working fine. I will comment in that issue about this finding.

Simulator Screen Shot - iPhone 14 Pro - 2023-02-21 at 13 32 53

}
active_composition_order_.push_back(platform_view_id);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2345,6 +2345,101 @@ - (void)testFlutterPlatformViewControllerBeginFrameShouldResetCompisitionOrder {
XCTAssertEqual(flutterPlatformViewsController->GetCurrentBuilders().size(), 1UL);
}

- (void)testFlutterPlatformViewControllerSubmitFrameShouldOrderSubviewsCorrectly {
flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate;
auto thread_task_runner = CreateNewThread("FlutterPlatformViewsTest");
flutter::TaskRunners runners(/*label=*/self.name.UTF8String,
/*platform=*/thread_task_runner,
/*raster=*/thread_task_runner,
/*ui=*/thread_task_runner,
/*io=*/thread_task_runner);
auto flutterPlatformViewsController = std::make_shared<flutter::FlutterPlatformViewsController>();
auto platform_view = std::make_unique<flutter::PlatformViewIOS>(
/*delegate=*/mock_delegate,
/*rendering_api=*/flutter::IOSRenderingAPI::kSoftware,
/*platform_views_controller=*/flutterPlatformViewsController,
/*task_runners=*/runners);

UIView* mockFlutterView = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 500, 500)] autorelease];
flutterPlatformViewsController->SetFlutterView(mockFlutterView);

FlutterPlatformViewsTestMockFlutterPlatformFactory* factory =
[[FlutterPlatformViewsTestMockFlutterPlatformFactory new] autorelease];
flutterPlatformViewsController->RegisterViewFactory(
factory, @"MockFlutterPlatformView",
FlutterPlatformViewGestureRecognizersBlockingPolicyEager);
FlutterResult result = ^(id result) {
};
flutterPlatformViewsController->OnMethodCall(
[FlutterMethodCall
methodCallWithMethodName:@"create"
arguments:@{@"id" : @0, @"viewType" : @"MockFlutterPlatformView"}],
result);
UIView* view1 = gMockPlatformView;

// This overwrites `gMockPlatformView` to another view.
flutterPlatformViewsController->OnMethodCall(
[FlutterMethodCall
methodCallWithMethodName:@"create"
arguments:@{@"id" : @1, @"viewType" : @"MockFlutterPlatformView"}],
result);
UIView* view2 = gMockPlatformView;

flutterPlatformViewsController->BeginFrame(SkISize::Make(300, 300));
flutter::MutatorsStack stack;
SkMatrix finalMatrix;
auto embeddedViewParams1 =
std::make_unique<flutter::EmbeddedViewParams>(finalMatrix, SkSize::Make(300, 300), stack);
flutterPlatformViewsController->PrerollCompositeEmbeddedView(0, std::move(embeddedViewParams1));
flutterPlatformViewsController->CompositeEmbeddedView(0);
auto embeddedViewParams2 =
std::make_unique<flutter::EmbeddedViewParams>(finalMatrix, SkSize::Make(500, 500), stack);
flutterPlatformViewsController->PrerollCompositeEmbeddedView(1, std::move(embeddedViewParams2));
flutterPlatformViewsController->CompositeEmbeddedView(1);

// SKSurface is required if the root FlutterView is present.
const SkImageInfo image_info = SkImageInfo::MakeN32Premul(1000, 1000);
sk_sp<SkSurface> mock_sk_surface = SkSurface::MakeRaster(image_info);
flutter::SurfaceFrame::FramebufferInfo framebuffer_info;
auto mock_surface = std::make_unique<flutter::SurfaceFrame>(
std::move(mock_sk_surface), framebuffer_info,
[](const flutter::SurfaceFrame& surface_frame, SkCanvas* canvas) { return true; },
/*frame_size=*/SkISize::Make(800, 600));

XCTAssertTrue(
flutterPlatformViewsController->SubmitFrame(nullptr, nullptr, std::move(mock_surface)));
// platform view is wrapped by touch interceptor, which itself is wrapped by clipping view.
UIView* clippingView1 = view1.superview.superview;
UIView* clippingView2 = view2.superview.superview;
UIView* flutterView = clippingView1.superview;
XCTAssertTrue([flutterView.subviews indexOfObject:clippingView1] <
[flutterView.subviews indexOfObject:clippingView2],
@"The first clipping view should be added before the second clipping view.");

// Need to recreate these params since they are `std::move`ed.
flutterPlatformViewsController->BeginFrame(SkISize::Make(300, 300));
// Process the second frame in the opposite order.
embeddedViewParams2 =
std::make_unique<flutter::EmbeddedViewParams>(finalMatrix, SkSize::Make(500, 500), stack);
flutterPlatformViewsController->PrerollCompositeEmbeddedView(1, std::move(embeddedViewParams2));
flutterPlatformViewsController->CompositeEmbeddedView(1);
embeddedViewParams1 =
std::make_unique<flutter::EmbeddedViewParams>(finalMatrix, SkSize::Make(300, 300), stack);
flutterPlatformViewsController->PrerollCompositeEmbeddedView(0, std::move(embeddedViewParams1));
flutterPlatformViewsController->CompositeEmbeddedView(0);

mock_sk_surface = SkSurface::MakeRaster(image_info);
mock_surface = std::make_unique<flutter::SurfaceFrame>(
std::move(mock_sk_surface), framebuffer_info,
[](const flutter::SurfaceFrame& surface_frame, SkCanvas* canvas) { return true; },
/*frame_size=*/SkISize::Make(800, 600));
XCTAssertTrue(
flutterPlatformViewsController->SubmitFrame(nullptr, nullptr, std::move(mock_surface)));
XCTAssertTrue([flutterView.subviews indexOfObject:clippingView1] >
[flutterView.subviews indexOfObject:clippingView2],
@"The first clipping view should be added after the second clipping view.");
}

- (void)testThreadMergeAtEndFrame {
flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate;
auto thread_task_runner_platform = CreateNewThread("FlutterPlatformViewsTest1");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
FLUTTER_ENGINE[arch=x86_64]=ios_debug_sim_unopt
FLUTTER_ENGINE[arch=arm64]=ios_debug_sim_unopt_arm64
FLUTTER_ENGINE=ios_debug_sim_unopt
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jmagman Not sure if you want to keep this or fix it in a separate PR?

Copy link
Member

Choose a reason for hiding this comment

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

Bleh may as well keep it, my attempt to do the right thing based on architecture was totally thwarted by Xcode passing in ARCH=unknown at certain build steps.

2 changes: 2 additions & 0 deletions testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ - (BOOL)application:(UIApplication*)application
@"--gesture-reject-after-touches-ended" : @"platform_view_gesture_reject_after_touches_ended",
@"--gesture-reject-eager" : @"platform_view_gesture_reject_eager",
@"--gesture-accept" : @"platform_view_gesture_accept",
@"--gesture-accept-with-overlapping-platform-views" :
@"platform_view_gesture_accept_with_overlapping_platform_views",
@"--tap-status-bar" : @"tap_status_bar",
@"--animated-color-square" : @"animated_color_square",
@"--platform-view-with-continuous-texture" : @"platform_view_with_continuous_texture",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,4 +165,32 @@ - (XCUICoordinate*)getNormalizedCoordinate:(XCUIApplication*)app point:(CGVector
return coordinate;
}

- (void)testGestureWithOverlappingPlatformViews {
XCUIApplication* app = [[XCUIApplication alloc] init];
app.launchArguments = @[ @"--gesture-accept-with-overlapping-platform-views" ];
[app launch];

XCUIElement* foreground = app.otherElements[@"platform_view[0]"];
XCTAssertEqual(foreground.frame.origin.x, 50);
XCTAssertEqual(foreground.frame.origin.y, 50);
XCTAssertEqual(foreground.frame.size.width, 50);
XCTAssertEqual(foreground.frame.size.height, 50);
XCTAssertTrue([foreground waitForExistenceWithTimeout:kSecondsToWaitForPlatformView]);

XCUIElement* background = app.otherElements[@"platform_view[1]"];
XCTAssertEqual(background.frame.origin.x, 0);
XCTAssertEqual(background.frame.origin.y, 0);
XCTAssertEqual(background.frame.size.width, 150);
XCTAssertEqual(background.frame.size.height, 150);
XCTAssertTrue([background waitForExistenceWithTimeout:kSecondsToWaitForPlatformView]);

XCUIElement* textView = foreground.textViews.firstMatch;
XCTAssertTrue(textView.exists);

XCTAssertTrue(foreground.isHittable);
[foreground tap];

XCTAssertEqualObjects(textView.label,
@"-gestureTouchesBegan-gestureTouchesEnded-platformViewTapped");
}
@end
127 changes: 127 additions & 0 deletions testing/scenario_app/lib/src/platform_view.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1056,6 +1056,133 @@ class PlatformViewForTouchIOSScenario extends Scenario
}
}

/// Scenario for verifying overlapping platform views can accept touch gesture.
/// See: https://github.com/flutter/flutter/issues/118366.
///
/// Renders the first frame with a foreground platform view.
/// Then renders the second frame with the foreground platform view covering
/// a new background platform view.
///
class PlatformViewForOverlappingPlatformViewsScenario extends Scenario
with _BasePlatformViewScenarioMixin {

/// Creates the PlatformViewForOverlappingPlatformViewsScenario.
///
/// The [dispatcher] parameter must not be null.
PlatformViewForOverlappingPlatformViewsScenario(
PlatformDispatcher dispatcher, {
required this.foregroundId,
required this.backgroundId,
}) : super(dispatcher) {
_nextFrame = _firstFrame;
}

/// The id for a foreground platform view that covers another background platform view.
/// A good example is a dialog prompt in a real app.
final int foregroundId;

/// The id for a background platform view that is covered by a foreground platform view.
final int backgroundId;

late void Function() _nextFrame;

@override
void onBeginFrame(Duration duration) {
_nextFrame();
}

void _firstFrame() {
final SceneBuilder builder = SceneBuilder();

builder.pushOffset(100, 100);
addPlatformView(
foregroundId,
width: 100,
height: 100,
dispatcher: dispatcher,
sceneBuilder: builder,
text: 'Foreground',
);
builder.pop();

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

void _secondFrame() {
final SceneBuilder builder = SceneBuilder();

builder.pushOffset(0, 0);
addPlatformView(
backgroundId,
width: 300,
height: 300,
dispatcher: dispatcher,
sceneBuilder: builder,
text: 'Background',
);
builder.pop();

builder.pushOffset(100, 100);
addPlatformView(
foregroundId,
width: 100,
height: 100,
dispatcher: dispatcher,
sceneBuilder: builder,
text: 'Foreground',
);
builder.pop();

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

int _frameCount = 0;

@override
void onDrawFrame() {
_frameCount += 1;
// TODO(hellohuanlin): Need further investigation - the first 2 frames are dropped for some reason.
// Wait for 60 frames to ensure the first frame has actually been rendered
// (Minimum required is 3 frames, but just to be safe)
if (_nextFrame == _firstFrame && _frameCount == 60) {
_nextFrame = _secondFrame;
}
window.scheduleFrame();
super.onDrawFrame();
}

@override
void onPointerDataPacket(PointerDataPacket packet) {
final PointerData data = packet.data.first;
final double x = data.physicalX;
final double y = data.physicalY;
if (data.change == PointerChange.up && 100 <= x && x < 200 && 100 <= y && y < 200) {
const int valueString = 7;
const int valueInt32 = 3;
const int valueMap = 13;
final Uint8List message = Uint8List.fromList(<int>[
valueString,
..._encodeString('acceptGesture'),
valueMap,
1,
valueString,
..._encodeString('id'),
valueInt32,
..._to32(foregroundId),
]);
window.sendPlatformMessage(
'flutter/platform_views',
message.buffer.asByteData(),
(ByteData? response) {},
);
}
}
}

/// A simple platform view for testing platform view with a continuous texture layer.
/// For example, it simulates a video being played.
class PlatformViewWithContinuousTexture extends PlatformViewScenario {
Expand Down
1 change: 1 addition & 0 deletions testing/scenario_app/lib/src/scenarios.dart
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ Map<String, ScenarioFactory> _scenarios = <String, ScenarioFactory>{
'platform_view_gesture_reject_eager': () => PlatformViewForTouchIOSScenario(PlatformDispatcher.instance, id: _viewId++, accept: false),
'platform_view_gesture_accept': () => PlatformViewForTouchIOSScenario(PlatformDispatcher.instance, id: _viewId++, accept: true),
'platform_view_gesture_reject_after_touches_ended': () => PlatformViewForTouchIOSScenario(PlatformDispatcher.instance, id: _viewId++, accept: false, rejectUntilTouchesEnded: true),
'platform_view_gesture_accept_with_overlapping_platform_views': () => PlatformViewForOverlappingPlatformViewsScenario(PlatformDispatcher.instance, foregroundId: _viewId++, backgroundId: _viewId++),
'platform_view_scrolling_under_widget':()=>PlatformViewScrollingUnderWidget(PlatformDispatcher.instance, firstPlatformViewId: _viewId++, lastPlatformViewId: _viewId+=16),
'tap_status_bar': () => TouchesScenario(PlatformDispatcher.instance),
'initial_route_reply': () => InitialRouteReply(PlatformDispatcher.instance),
Expand Down