diff --git a/engine/src/flutter/flow/embedded_views.h b/engine/src/flutter/flow/embedded_views.h index 745c91d9dd2..5aad6605aac 100644 --- a/engine/src/flutter/flow/embedded_views.h +++ b/engine/src/flutter/flow/embedded_views.h @@ -339,10 +339,31 @@ class EmbedderViewSlice { virtual DlCanvas* canvas() = 0; virtual void end_recording() = 0; virtual const DlRegion& getRegion() const = 0; + // TODO(hellohuanlin): We should deprecate this function if we migrate + // all platforms to use `roundedInRegion`. Then we should rename + // `roundedInRegion` to just `region`. DlRegion region(const SkRect& query) const { return DlRegion::MakeIntersection(getRegion(), DlRegion(query.roundOut())); } + // TODO(hellohuanlin): iOS only for now, but we should try it on other + // platforms. + DlRegion roundedInRegion(const SkRect& query) const { + // Use `roundIn` to address a performance issue when we interleave embedded + // view (the queried rect) and flutter widgets (the slice regions). + // Rounding out both the queried rect and slice regions will + // result in an intersection region of 1 px height, which is then used to + // create an overlay layer. For each overlay, we acquire a surface frame, + // paint the pixels and submit the frame. This resulted in performance + // issues since the surface frame acquisition is expensive. Since slice + // regions are already rounded out (see: + // https://github.com/flutter/engine/blob/5f40c9f49f88729bc3e71390356209dbe29ec788/display_list/geometry/dl_rtree.cc#L209), + // we can simply round in the queried rect to avoid the situation. + // After rounding in, it will ignore a single (or partial) pixel overlap, + // and give the ownership to the platform view. + return DlRegion::MakeIntersection(getRegion(), DlRegion(query.roundIn())); + } + virtual void render_into(DlCanvas* canvas) = 0; }; diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 7e824653823..de5109d9889 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -705,7 +705,8 @@ bool FlutterPlatformViewsController::SubmitFrame(GrDirectContext* gr_context, for (size_t j = i + 1; j > 0; j--) { int64_t current_platform_view_id = composition_order_[j - 1]; SkRect platform_view_rect = GetPlatformViewRect(current_platform_view_id); - std::vector intersection_rects = slice->region(platform_view_rect).getRects(); + std::vector intersection_rects = + slice->roundedInRegion(platform_view_rect).getRects(); auto allocation_size = intersection_rects.size(); // For testing purposes, the overlay id is used to find the overlay view. diff --git a/engine/src/flutter/testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m b/engine/src/flutter/testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m index 324ee3aa8ff..4393b245135 100644 --- a/engine/src/flutter/testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m +++ b/engine/src/flutter/testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m @@ -52,6 +52,7 @@ @"platform_view_one_overlay_two_intersecting_overlays", @"--platform-view-multiple-without-overlays" : @"platform_view_multiple_without_overlays", @"--platform-view-max-overlays" : @"platform_view_max_overlays", + @"--platform-view-surrounding-layers" : @"platform_view_surrounding_layers", @"--platform-view-multiple" : @"platform_view_multiple", @"--platform-view-multiple-background-foreground" : @"platform_view_multiple_background_foreground", diff --git a/engine/src/flutter/testing/scenario_app/ios/Scenarios/ScenariosUITests/UnobstructedPlatformViewTests.m b/engine/src/flutter/testing/scenario_app/ios/Scenarios/ScenariosUITests/UnobstructedPlatformViewTests.m index 2e9ed55dc61..d1014e2186b 100644 --- a/engine/src/flutter/testing/scenario_app/ios/Scenarios/ScenariosUITests/UnobstructedPlatformViewTests.m +++ b/engine/src/flutter/testing/scenario_app/ios/Scenarios/ScenariosUITests/UnobstructedPlatformViewTests.m @@ -305,4 +305,30 @@ static const CGFloat kCompareAccuracy = 0.001; XCTAssertFalse(overlayView1.exists); } +// Platform view surrounded by adjacent layers on each side should not create any overlays. +// +----+ +// | B | +// +---+----+---+ +// | A | PV | C | +// +---+----+---+ +// | D | +// +----+ +- (void)testPlatformViewsWithAdjacentSurroundingLayers { + XCUIApplication* app = [[XCUIApplication alloc] init]; + app.launchArguments = @[ @"--platform-view-surrounding-layers" ]; + [app launch]; + + XCUIElement* platform_view = app.otherElements[@"platform_view[0]"]; + XCTAssertTrue([platform_view waitForExistenceWithTimeout:1.0]); + + CGFloat scale = [UIScreen mainScreen].scale; + XCTAssertEqual(platform_view.frame.origin.x * scale, 100.5); + XCTAssertEqual(platform_view.frame.origin.y * scale, 100.5); + XCTAssertEqual(platform_view.frame.size.width * scale, 100); + XCTAssertEqual(platform_view.frame.size.height * scale, 100); + + XCUIElement* overlay = app.otherElements[@"platform_view[0].overlay[0]"]; + XCTAssertFalse(overlay.exists); +} + @end diff --git a/engine/src/flutter/testing/scenario_app/lib/src/platform_view.dart b/engine/src/flutter/testing/scenario_app/lib/src/platform_view.dart index e2e8d8a3fb0..799f53e36b8 100644 --- a/engine/src/flutter/testing/scenario_app/lib/src/platform_view.dart +++ b/engine/src/flutter/testing/scenario_app/lib/src/platform_view.dart @@ -366,6 +366,78 @@ class PlatformViewMaxOverlaysScenario extends Scenario } } +/// A platform view with adjacent surrounding layers should not create overlays. +class PlatformViewSurroundingLayersScenario extends Scenario + with _BasePlatformViewScenarioMixin { + /// Creates the PlatformView scenario. + PlatformViewSurroundingLayersScenario( + super.view, { + required this.id, + }); + + /// The platform view identifier. + final int id; + + @override + void onBeginFrame(Duration duration) { + final SceneBuilder builder = SceneBuilder(); + + // Simulate partial pixel offsets as we would see while scrolling. + // All objects in the scene below are then on sub-pixel boundaries. + builder.pushOffset(0.5, 0.5); + + // a platform view from (100, 100) to (200, 200) + builder.pushOffset(100, 100); + addPlatformView( + id, + width: 100, + height: 100, + dispatcher: view.platformDispatcher, + sceneBuilder: builder, + ); + builder.pop(); + + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + + const Rect rect = Rect.fromLTWH(100, 100, 100, 100); + + // Rect at the left of platform view + canvas.drawRect( + rect.shift(const Offset(-100, 0)), + Paint()..color = const Color(0x22FF0000), + ); + + // Rect at the right of platform view + canvas.drawRect( + rect.shift(const Offset(100, 0)), + Paint()..color = const Color(0x22FF0000), + ); + + // Rect at the top of platform view + canvas.drawRect( + rect.shift(const Offset(0, -100)), + Paint()..color = const Color(0x22FF0000), + ); + + // Rect at the bottom of platform view + canvas.drawRect( + rect.shift(const Offset(0, 100)), + Paint()..color = const Color(0x22FF0000), + ); + + final Picture picture = recorder.endRecording(); + builder.addPicture(Offset.zero, picture); + + // Pop the (0.5, 0.5) offset. + builder.pop(); + + final Scene scene = builder.build(); + view.render(scene); + scene.dispose(); + } +} + /// Builds a scene with 2 platform views. class MultiPlatformViewScenario extends Scenario with _BasePlatformViewScenarioMixin { diff --git a/engine/src/flutter/testing/scenario_app/lib/src/scenarios.dart b/engine/src/flutter/testing/scenario_app/lib/src/scenarios.dart index bd09ed21efb..d49693af719 100644 --- a/engine/src/flutter/testing/scenario_app/lib/src/scenarios.dart +++ b/engine/src/flutter/testing/scenario_app/lib/src/scenarios.dart @@ -34,6 +34,7 @@ Map _scenarios = { 'platform_view_one_overlay_two_intersecting_overlays': (FlutterView view) => PlatformViewOneOverlayTwoIntersectingOverlaysScenario(view, id: _viewId++), 'platform_view_multiple_without_overlays': (FlutterView view) => MultiPlatformViewWithoutOverlaysScenario(view, firstId: _viewId++, secondId: _viewId++), 'platform_view_max_overlays': (FlutterView view) => PlatformViewMaxOverlaysScenario(view, id: _viewId++), + 'platform_view_surrounding_layers': (FlutterView view) => PlatformViewSurroundingLayersScenario(view, id: _viewId++), 'platform_view_cliprect': (FlutterView view) => PlatformViewClipRectScenario(view, id: _viewId++), 'platform_view_cliprect_with_transform': (FlutterView view) => PlatformViewClipRectWithTransformScenario(view, id: _viewId++), 'platform_view_cliprect_after_moved': (FlutterView view) => PlatformViewClipRectAfterMovedScenario(view, id: _viewId++),