diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 5aad6605aaceb..745c91d9dd2f5 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -339,31 +339,10 @@ 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/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index de5109d988940..6d605414e5b4f 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -695,6 +695,8 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, auto did_submit = true; auto num_platform_views = composition_order_.size(); + // TODO(hellohuanlin) this double for-loop is expensive with wasted computations. + // See: https://github.com/flutter/flutter/issues/145802 for (size_t i = 0; i < num_platform_views; i++) { int64_t platform_view_id = composition_order_[i]; EmbedderViewSlice* slice = slices_[platform_view_id].get(); @@ -705,8 +707,28 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, 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->roundedInRegion(platform_view_rect).getRects(); + std::vector intersection_rects = slice->region(platform_view_rect).getRects(); + const SkIRect rounded_in_platform_view_rect = platform_view_rect.roundIn(); + // Ignore intersections of single width/height on the edge of the platform view. + // This is to address the following performance issue when interleaving adjacent + // platform views and layers: + // Since we `roundOut` both platform view rects and the layer rects, as long as + // the coordinate is fractional, there will be an intersection of a single pixel width + // (or height) after rounding out, even if they do not intersect before rounding out. + // We have to round out both platform view rect and the layer rect. + // Rounding in platform view rect will result in missing pixel on the intersection edge. + // Rounding in layer rect will result in missing pixel on the edge of the layer on top + // of the platform view. + for (auto it = intersection_rects.begin(); it != intersection_rects.end(); /*no-op*/) { + // If intersection_rect does not intersect with the *rounded in* platform + // view rect, then the intersection must be a single pixel width (or height) on edge. + if (!SkIRect::Intersects(*it, rounded_in_platform_view_rect)) { + it = intersection_rects.erase(it); + } else { + ++it; + } + } + auto allocation_size = intersection_rects.size(); // For testing purposes, the overlay id is used to find the overlay view. diff --git a/testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m b/testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m index 4393b245135f6..47188144fcca7 100644 --- a/testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m +++ b/testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m @@ -52,7 +52,10 @@ - (BOOL)application:(UIApplication*)application @"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-surrounding-layers-fractional-coordinate" : + @"platform_view_surrounding_layers_fractional_coordinate", + @"--platform-view-partial-intersection-fractional-coordinate" : + @"platform_view_partial_intersection_fractional_coordinate", @"--platform-view-multiple" : @"platform_view_multiple", @"--platform-view-multiple-background-foreground" : @"platform_view_multiple_background_foreground", diff --git a/testing/scenario_app/ios/Scenarios/ScenariosUITests/UnobstructedPlatformViewTests.m b/testing/scenario_app/ios/Scenarios/ScenariosUITests/UnobstructedPlatformViewTests.m index d1014e2186b00..5fc45893706f8 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosUITests/UnobstructedPlatformViewTests.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosUITests/UnobstructedPlatformViewTests.m @@ -313,9 +313,9 @@ - (void)testPlatformViewsMaxOverlays { // +---+----+---+ // | D | // +----+ -- (void)testPlatformViewsWithAdjacentSurroundingLayers { +- (void)testPlatformViewsWithAdjacentSurroundingLayersAndFractionalCoordinate { XCUIApplication* app = [[XCUIApplication alloc] init]; - app.launchArguments = @[ @"--platform-view-surrounding-layers" ]; + app.launchArguments = @[ @"--platform-view-surrounding-layers-fractional-coordinate" ]; [app launch]; XCUIElement* platform_view = app.otherElements[@"platform_view[0]"]; @@ -331,4 +331,33 @@ - (void)testPlatformViewsWithAdjacentSurroundingLayers { XCTAssertFalse(overlay.exists); } +// Platform view partially intersect with a layer in fractional coordinate. +// +-------+ +// | | +// | PV +--+--+ +// | | | +// +----+ A | +// | | +// +-----+ +- (void)testPlatformViewsWithPartialIntersectionAndFractionalCoordinate { + XCUIApplication* app = [[XCUIApplication alloc] init]; + app.launchArguments = @[ @"--platform-view-partial-intersection-fractional-coordinate" ]; + [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, 0.5); + XCTAssertEqual(platform_view.frame.origin.y * scale, 0.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]"]; + XCTAssert(overlay.exists); + + // We want to make sure the overlay covers the edge (which is at 100.5). + XCTAssertEqual(CGRectGetMaxX(overlay.frame) * scale, 101); + XCTAssertEqual(CGRectGetMaxY(overlay.frame) * scale, 101); +} @end diff --git a/testing/scenario_app/lib/src/platform_view.dart b/testing/scenario_app/lib/src/platform_view.dart index 799f53e36b894..5d1099a522350 100644 --- a/testing/scenario_app/lib/src/platform_view.dart +++ b/testing/scenario_app/lib/src/platform_view.dart @@ -367,10 +367,10 @@ class PlatformViewMaxOverlaysScenario extends Scenario } /// A platform view with adjacent surrounding layers should not create overlays. -class PlatformViewSurroundingLayersScenario extends Scenario +class PlatformViewSurroundingLayersFractionalCoordinateScenario extends Scenario with _BasePlatformViewScenarioMixin { /// Creates the PlatformView scenario. - PlatformViewSurroundingLayersScenario( + PlatformViewSurroundingLayersFractionalCoordinateScenario( super.view, { required this.id, }); @@ -438,6 +438,55 @@ class PlatformViewSurroundingLayersScenario extends Scenario } } +/// A platform view partially intersect with a layer, both with fractional coordinates. +class PlatformViewPartialIntersectionFractionalCoordinateScenario extends Scenario + with _BasePlatformViewScenarioMixin { + /// Creates the PlatformView scenario. + PlatformViewPartialIntersectionFractionalCoordinateScenario( + 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 (0, 0) to (100, 100) + addPlatformView( + id, + width: 100, + height: 100, + dispatcher: view.platformDispatcher, + sceneBuilder: builder, + ); + + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + + canvas.drawRect( + const Rect.fromLTWH(50, 50, 100, 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/testing/scenario_app/lib/src/scenarios.dart b/testing/scenario_app/lib/src/scenarios.dart index d49693af719c4..bcd2c0cd908b6 100644 --- a/testing/scenario_app/lib/src/scenarios.dart +++ b/testing/scenario_app/lib/src/scenarios.dart @@ -34,7 +34,8 @@ 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_surrounding_layers_fractional_coordinate': (FlutterView view) => PlatformViewSurroundingLayersFractionalCoordinateScenario(view, id: _viewId++), + 'platform_view_partial_intersection_fractional_coordinate': (FlutterView view) => PlatformViewPartialIntersectionFractionalCoordinateScenario(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++),