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

Commit f31b39c

Browse files
committed
[ios]ignore single edge pixel instead of rounding
1 parent 5fc95a3 commit f31b39c

File tree

6 files changed

+125
-30
lines changed

6 files changed

+125
-30
lines changed

flow/embedded_views.h

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -339,31 +339,10 @@ class EmbedderViewSlice {
339339
virtual DlCanvas* canvas() = 0;
340340
virtual void end_recording() = 0;
341341
virtual const DlRegion& getRegion() const = 0;
342-
// TODO(hellohuanlin): We should deprecate this function if we migrate
343-
// all platforms to use `roundedInRegion`. Then we should rename
344-
// `roundedInRegion` to just `region`.
345342
DlRegion region(const SkRect& query) const {
346343
return DlRegion::MakeIntersection(getRegion(), DlRegion(query.roundOut()));
347344
}
348345

349-
// TODO(hellohuanlin): iOS only for now, but we should try it on other
350-
// platforms.
351-
DlRegion roundedInRegion(const SkRect& query) const {
352-
// Use `roundIn` to address a performance issue when we interleave embedded
353-
// view (the queried rect) and flutter widgets (the slice regions).
354-
// Rounding out both the queried rect and slice regions will
355-
// result in an intersection region of 1 px height, which is then used to
356-
// create an overlay layer. For each overlay, we acquire a surface frame,
357-
// paint the pixels and submit the frame. This resulted in performance
358-
// issues since the surface frame acquisition is expensive. Since slice
359-
// regions are already rounded out (see:
360-
// https://github.com/flutter/engine/blob/5f40c9f49f88729bc3e71390356209dbe29ec788/display_list/geometry/dl_rtree.cc#L209),
361-
// we can simply round in the queried rect to avoid the situation.
362-
// After rounding in, it will ignore a single (or partial) pixel overlap,
363-
// and give the ownership to the platform view.
364-
return DlRegion::MakeIntersection(getRegion(), DlRegion(query.roundIn()));
365-
}
366-
367346
virtual void render_into(DlCanvas* canvas) = 0;
368347
};
369348

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,19 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
8181
return transformed_rrect.contains(platformview_boundingrect);
8282
}
8383

84+
// Determines if the intersection rect of a layer and a platform view is a single pixel width
85+
// or height, and on the edge of the platform view.
86+
static bool IsIntersectionSinglePixelWidthOrHeightOnEdgeOfPlatformView(
87+
const SkIRect& rounded_out_platform_view_rect,
88+
const SkIRect& intersection_rect) {
89+
bool onLeftOrRight = intersection_rect.left() == rounded_out_platform_view_rect.left() ||
90+
intersection_rect.right() == rounded_out_platform_view_rect.right();
91+
bool onTopOrBottom = intersection_rect.top() == rounded_out_platform_view_rect.top() ||
92+
intersection_rect.bottom() == rounded_out_platform_view_rect.bottom();
93+
return (intersection_rect.width() == 1 && onLeftOrRight) ||
94+
(intersection_rect.height() == 1 && onTopOrBottom);
95+
}
96+
8497
namespace flutter {
8598
// Becomes NO if Apple's API changes and blurred backdrop filters cannot be applied.
8699
BOOL canApplyBlurBackdrop = YES;
@@ -705,8 +718,27 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
705718
for (size_t j = i + 1; j > 0; j--) {
706719
int64_t current_platform_view_id = composition_order_[j - 1];
707720
SkRect platform_view_rect = GetPlatformViewRect(current_platform_view_id);
708-
std::vector<SkIRect> intersection_rects =
709-
slice->roundedInRegion(platform_view_rect).getRects();
721+
std::vector<SkIRect> intersection_rects = slice->region(platform_view_rect).getRects();
722+
const SkIRect rounded_out_platform_view_rect = platform_view_rect.roundOut();
723+
// Ignore intersections of single width/height on the edge of the platform view.
724+
// This is to address the following performance issue when interleaving adjacent
725+
// platform views and layers:
726+
// Since we `roundOut` both platform view rects and the layer rects, as long as
727+
// the coordinate is fractional, there will be an intersection of a single pixel width
728+
// (or height) after rounding out, even if they do not intersect before rounding out.
729+
// We have to round out both platform view rect and the layer rect.
730+
// Rounding in platform view rect will result in missing pixel on the intersection edge.
731+
// Rounding in layer rect will result in missing pixel on the edge of the layer on top
732+
// of the platform view.
733+
for (auto it = intersection_rects.begin(); it != intersection_rects.end(); /*no-op*/) {
734+
if (IsIntersectionSinglePixelWidthOrHeightOnEdgeOfPlatformView(
735+
rounded_out_platform_view_rect, *it)) {
736+
it = intersection_rects.erase(it);
737+
} else {
738+
++it;
739+
}
740+
}
741+
710742
auto allocation_size = intersection_rects.size();
711743

712744
// For testing purposes, the overlay id is used to find the overlay view.
@@ -731,7 +763,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
731763
for (SkIRect& joined_rect : intersection_rects) {
732764
// Get the intersection rect between the current rect
733765
// and the platform view rect.
734-
joined_rect.intersect(platform_view_rect.roundOut());
766+
joined_rect.intersect(rounded_out_platform_view_rect);
735767
// Clip the background canvas, so it doesn't contain any of the pixels drawn
736768
// on the overlay layer.
737769
background_canvas->ClipRect(SkRect::Make(joined_rect), DlCanvas::ClipOp::kDifference);

testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,10 @@ - (BOOL)application:(UIApplication*)application
5252
@"platform_view_one_overlay_two_intersecting_overlays",
5353
@"--platform-view-multiple-without-overlays" : @"platform_view_multiple_without_overlays",
5454
@"--platform-view-max-overlays" : @"platform_view_max_overlays",
55-
@"--platform-view-surrounding-layers" : @"platform_view_surrounding_layers",
55+
@"--platform-view-surrounding-layers-fractional-coordinate" :
56+
@"platform_view_surrounding_layers_fractional_coordinate",
57+
@"--platform-view-partial-intersection-fractional-coordinate" :
58+
@"platform_view_partial_intersection_fractional_coordinate",
5659
@"--platform-view-multiple" : @"platform_view_multiple",
5760
@"--platform-view-multiple-background-foreground" :
5861
@"platform_view_multiple_background_foreground",

testing/scenario_app/ios/Scenarios/ScenariosUITests/UnobstructedPlatformViewTests.m

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,9 +313,9 @@ - (void)testPlatformViewsMaxOverlays {
313313
// +---+----+---+
314314
// | D |
315315
// +----+
316-
- (void)testPlatformViewsWithAdjacentSurroundingLayers {
316+
- (void)testPlatformViewsWithAdjacentSurroundingLayersAndFractionalCoordinate {
317317
XCUIApplication* app = [[XCUIApplication alloc] init];
318-
app.launchArguments = @[ @"--platform-view-surrounding-layers" ];
318+
app.launchArguments = @[ @"--platform-view-surrounding-layers-fractional-coordinate" ];
319319
[app launch];
320320

321321
XCUIElement* platform_view = app.otherElements[@"platform_view[0]"];
@@ -331,4 +331,35 @@ - (void)testPlatformViewsWithAdjacentSurroundingLayers {
331331
XCTAssertFalse(overlay.exists);
332332
}
333333

334+
// Platform view partially intersect with a layer in fractional coordinate.
335+
// +-------+
336+
// | |
337+
// | PV +--+--+
338+
// | | |
339+
// +----+ A |
340+
// | |
341+
// +-----+
342+
- (void)testPlatformViewsWithPartialIntersectionAndFractionalCoordinate {
343+
XCUIApplication* app = [[XCUIApplication alloc] init];
344+
app.launchArguments = @[ @"--platform-view-partial-intersection-fractional-coordinate" ];
345+
[app launch];
346+
347+
XCUIElement* platform_view = app.otherElements[@"platform_view[0]"];
348+
XCTAssertTrue([platform_view waitForExistenceWithTimeout:1.0]);
349+
350+
CGFloat scale = [UIScreen mainScreen].scale;
351+
XCTAssertEqual(platform_view.frame.origin.x * scale, 0.5);
352+
XCTAssertEqual(platform_view.frame.origin.y * scale, 0.5);
353+
XCTAssertEqual(platform_view.frame.size.width * scale, 100);
354+
XCTAssertEqual(platform_view.frame.size.height * scale, 100);
355+
356+
XCUIElement* overlay = app.otherElements[@"platform_view[0].overlay[0]"];
357+
XCTAssert(overlay.exists);
358+
359+
// We want to make sure the overlay covers the edge (which is at 100.5).
360+
// Without the pixel 101, there will be a "gap" on the edge of the intersecton,
361+
// which results in a thin line along this edge.
362+
XCTAssertEqual(CGRectGetMaxX(overlay.frame) * scale, 101);
363+
XCTAssertEqual(CGRectGetMaxY(overlay.frame) * scale, 101);
364+
}
334365
@end

testing/scenario_app/lib/src/platform_view.dart

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -367,10 +367,10 @@ class PlatformViewMaxOverlaysScenario extends Scenario
367367
}
368368

369369
/// A platform view with adjacent surrounding layers should not create overlays.
370-
class PlatformViewSurroundingLayersScenario extends Scenario
370+
class PlatformViewSurroundingLayersFractionalCoordinateScenario extends Scenario
371371
with _BasePlatformViewScenarioMixin {
372372
/// Creates the PlatformView scenario.
373-
PlatformViewSurroundingLayersScenario(
373+
PlatformViewSurroundingLayersFractionalCoordinateScenario(
374374
super.view, {
375375
required this.id,
376376
});
@@ -438,6 +438,55 @@ class PlatformViewSurroundingLayersScenario extends Scenario
438438
}
439439
}
440440

441+
/// A platform view partially intersect with a layer, both with fractional coordinates.
442+
class PlatformViewPartialIntersectionFractionalCoordinateScenario extends Scenario
443+
with _BasePlatformViewScenarioMixin {
444+
/// Creates the PlatformView scenario.
445+
PlatformViewPartialIntersectionFractionalCoordinateScenario(
446+
super.view, {
447+
required this.id,
448+
});
449+
450+
/// The platform view identifier.
451+
final int id;
452+
453+
@override
454+
void onBeginFrame(Duration duration) {
455+
final SceneBuilder builder = SceneBuilder();
456+
457+
// Simulate partial pixel offsets as we would see while scrolling.
458+
// All objects in the scene below are then on sub-pixel boundaries.
459+
builder.pushOffset(0.5, 0.5);
460+
461+
// a platform view from (0, 0) to (100, 100)
462+
addPlatformView(
463+
id,
464+
width: 100,
465+
height: 100,
466+
dispatcher: view.platformDispatcher,
467+
sceneBuilder: builder,
468+
);
469+
470+
final PictureRecorder recorder = PictureRecorder();
471+
final Canvas canvas = Canvas(recorder);
472+
473+
canvas.drawRect(
474+
const Rect.fromLTWH(50, 50, 100, 100),
475+
Paint()..color = const Color(0x22FF0000),
476+
);
477+
478+
final Picture picture = recorder.endRecording();
479+
builder.addPicture(Offset.zero, picture);
480+
481+
// Pop the (0.5, 0.5) offset.
482+
builder.pop();
483+
484+
final Scene scene = builder.build();
485+
view.render(scene);
486+
scene.dispose();
487+
}
488+
}
489+
441490
/// Builds a scene with 2 platform views.
442491
class MultiPlatformViewScenario extends Scenario
443492
with _BasePlatformViewScenarioMixin {

testing/scenario_app/lib/src/scenarios.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ Map<String, _ScenarioFactory> _scenarios = <String, _ScenarioFactory>{
3434
'platform_view_one_overlay_two_intersecting_overlays': (FlutterView view) => PlatformViewOneOverlayTwoIntersectingOverlaysScenario(view, id: _viewId++),
3535
'platform_view_multiple_without_overlays': (FlutterView view) => MultiPlatformViewWithoutOverlaysScenario(view, firstId: _viewId++, secondId: _viewId++),
3636
'platform_view_max_overlays': (FlutterView view) => PlatformViewMaxOverlaysScenario(view, id: _viewId++),
37-
'platform_view_surrounding_layers': (FlutterView view) => PlatformViewSurroundingLayersScenario(view, id: _viewId++),
37+
'platform_view_surrounding_layers_fractional_coordinate': (FlutterView view) => PlatformViewSurroundingLayersFractionalCoordinateScenario(view, id: _viewId++),
38+
'platform_view_partial_intersection_fractional_coordinate': (FlutterView view) => PlatformViewPartialIntersectionFractionalCoordinateScenario(view, id: _viewId++),
3839
'platform_view_cliprect': (FlutterView view) => PlatformViewClipRectScenario(view, id: _viewId++),
3940
'platform_view_cliprect_with_transform': (FlutterView view) => PlatformViewClipRectWithTransformScenario(view, id: _viewId++),
4041
'platform_view_cliprect_after_moved': (FlutterView view) => PlatformViewClipRectAfterMovedScenario(view, id: _viewId++),

0 commit comments

Comments
 (0)