From bd3011e6fbd7aa7a8c170ed587797f9d8939360c Mon Sep 17 00:00:00 2001 From: Huan Lin Date: Mon, 6 Mar 2023 10:27:11 -0800 Subject: [PATCH] [platform_view]fix regression for addSubview when re-ordering --- .../framework/Source/FlutterPlatformViews.mm | 23 ++++- .../Source/FlutterPlatformViewsTest.mm | 99 ++++++++++++++++++- 2 files changed, 118 insertions(+), 4 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 1665024ccf7db..65dacb252c833 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -750,17 +750,34 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, UIView* flutter_view = flutter_view_.get(); // Clear the `active_composition_order_`, which will be populated down below. active_composition_order_.clear(); + NSMutableArray* desired_platform_subviews = [NSMutableArray array]; for (size_t i = 0; i < composition_order_.size(); i++) { int64_t platform_view_id = composition_order_[i]; std::vector> layers = layer_map[platform_view_id]; UIView* platform_view_root = root_views_[platform_view_id].get(); - // `addSubview` will automatically reorder subview if it is already added. - [flutter_view addSubview:platform_view_root]; + [desired_platform_subviews addObject:platform_view_root]; for (const std::shared_ptr& layer : layers) { - [flutter_view addSubview:layer->overlay_view_wrapper]; + [desired_platform_subviews addObject:layer->overlay_view_wrapper]; } active_composition_order_.push_back(platform_view_id); } + + NSSet* desired_platform_subviews_set = [NSSet setWithArray:desired_platform_subviews]; + NSArray* existing_platform_subviews = [flutter_view.subviews + filteredArrayUsingPredicate:[NSPredicate predicateWithBlock:^BOOL(id object, + NSDictionary* bindings) { + return [desired_platform_subviews_set containsObject:object]; + }]]; + // Manipulate view hierarchy only if needed, to address a performance issue where + // `BringLayersIntoView` is called even when view hierarchy stays the same. + // See: https://github.com/flutter/flutter/issues/121833 + // TODO(hellohuanlin): investigate if it is possible to skip unnecessary BringLayersIntoView. + if (![desired_platform_subviews isEqualToArray:existing_platform_subviews]) { + for (UIView* subview in desired_platform_subviews) { + // `addSubview` will automatically reorder subview if it is already added. + [flutter_view addSubview:subview]; + } + } } std::shared_ptr FlutterPlatformViewsController::GetLayer( diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm index 5c0c20c2792cf..ebae0c59d2e9c 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm @@ -2342,7 +2342,8 @@ - (void)testFlutterPlatformViewControllerBeginFrameShouldResetCompisitionOrder { XCTAssertEqual(flutterPlatformViewsController->EmbeddedViewCount(), 1UL); } -- (void)testFlutterPlatformViewControllerSubmitFrameShouldOrderSubviewsCorrectly { +- (void) + testFlutterPlatformViewControllerSubmitFrameShouldOrderSubviewsCorrectlyWithDifferentViewHierarchy { flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate; auto thread_task_runner = CreateNewThread("FlutterPlatformViewsTest"); flutter::TaskRunners runners(/*label=*/self.name.UTF8String, @@ -2437,6 +2438,102 @@ - (void)testFlutterPlatformViewControllerSubmitFrameShouldOrderSubviewsCorrectly @"The first clipping view should be added after the second clipping view."); } +- (void) + testFlutterPlatformViewControllerSubmitFrameShouldOrderSubviewsCorrectlyWithSameViewHierarchy { + 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(); + auto platform_view = std::make_unique( + /*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(finalMatrix, SkSize::Make(300, 300), stack); + flutterPlatformViewsController->PrerollCompositeEmbeddedView(0, std::move(embeddedViewParams1)); + flutterPlatformViewsController->CompositeEmbeddedView(0); + auto embeddedViewParams2 = + std::make_unique(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 mock_sk_surface = SkSurface::MakeRaster(image_info); + flutter::SurfaceFrame::FramebufferInfo framebuffer_info; + auto mock_surface = std::make_unique( + std::move(mock_sk_surface), framebuffer_info, + [](const flutter::SurfaceFrame& surface_frame, flutter::DlCanvas* 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 same order. + embeddedViewParams1 = + std::make_unique(finalMatrix, SkSize::Make(300, 300), stack); + flutterPlatformViewsController->PrerollCompositeEmbeddedView(0, std::move(embeddedViewParams1)); + flutterPlatformViewsController->CompositeEmbeddedView(0); + embeddedViewParams2 = + std::make_unique(finalMatrix, SkSize::Make(500, 500), stack); + flutterPlatformViewsController->PrerollCompositeEmbeddedView(1, std::move(embeddedViewParams2)); + flutterPlatformViewsController->CompositeEmbeddedView(1); + + mock_sk_surface = SkSurface::MakeRaster(image_info); + mock_surface = std::make_unique( + std::move(mock_sk_surface), framebuffer_info, + [](const flutter::SurfaceFrame& surface_frame, flutter::DlCanvas* 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 before the second clipping view."); +} + - (void)testThreadMergeAtEndFrame { flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate; auto thread_task_runner_platform = CreateNewThread("FlutterPlatformViewsTest1");