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

Commit 0507f4a

Browse files
authored
Revert "Fix composition when multiple platform views and layers are combined (#25900)" (#26142)
This appears to be reliably crashing the devicelab linux_android_view_scroll_perf__timeline_summary test. Example crashes: * https://ci.chromium.org/ui/p/flutter/builders/prod/Linux%20android_view_scroll_perf__timeline_summary/1369/overview * https://ci.chromium.org/ui/p/flutter/builders/prod/Linux%20android_view_scroll_perf__timeline_summary/1370/overview This reverts commit cf18983.
1 parent 8773295 commit 0507f4a

File tree

3 files changed

+35
-132
lines changed

3 files changed

+35
-132
lines changed

shell/platform/android/external_view_embedder/external_view_embedder.cc

Lines changed: 33 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ void AndroidExternalViewEmbedder::SubmitFrame(
8484
return;
8585
}
8686

87-
std::unordered_map<int64_t, SkRect> overlay_layers;
87+
std::unordered_map<int64_t, std::list<SkRect>> overlay_layers;
8888
std::unordered_map<int64_t, sk_sp<SkPicture>> pictures;
8989
SkCanvas* background_canvas = frame->SkiaCanvas();
9090
auto current_frame_view_count = composition_order_.size();
@@ -101,9 +101,9 @@ void AndroidExternalViewEmbedder::SubmitFrame(
101101
FML_CHECK(picture);
102102
pictures.insert({view_id, picture});
103103

104-
sk_sp<RTree> rtree = view_rtrees_.at(view_id);
105-
SkRect joined_rect = SkRect::MakeEmpty();
104+
overlay_layers.insert({view_id, {}});
106105

106+
sk_sp<RTree> rtree = view_rtrees_.at(view_id);
107107
// Determinate if Flutter UI intersects with any of the previous
108108
// platform views stacked by z position.
109109
//
@@ -116,31 +116,33 @@ void AndroidExternalViewEmbedder::SubmitFrame(
116116
// Each rect corresponds to a native view that renders Flutter UI.
117117
std::list<SkRect> intersection_rects =
118118
rtree->searchNonOverlappingDrawnRects(current_view_rect);
119+
auto allocation_size = intersection_rects.size();
119120

120121
// Limit the number of native views, so it doesn't grow forever.
121122
//
122123
// In this case, the rects are merged into a single one that is the union
123124
// of all the rects.
124-
for (const SkRect& rect : intersection_rects) {
125-
joined_rect.join(rect);
125+
if (allocation_size > kMaxLayerAllocations) {
126+
SkRect joined_rect;
127+
for (const SkRect& rect : intersection_rects) {
128+
joined_rect.join(rect);
129+
}
130+
intersection_rects.clear();
131+
intersection_rects.push_back(joined_rect);
132+
}
133+
for (SkRect& intersection_rect : intersection_rects) {
134+
// Subpixels in the platform may not align with the canvas subpixels.
135+
//
136+
// To workaround it, round the floating point bounds and make the rect
137+
// slightly larger. For example, {0.3, 0.5, 3.1, 4.7} becomes {0, 0, 4,
138+
// 5}.
139+
intersection_rect.set(intersection_rect.roundOut());
140+
overlay_layers.at(view_id).push_back(intersection_rect);
141+
// Clip the background canvas, so it doesn't contain any of the pixels
142+
// drawn on the overlay layer.
143+
background_canvas->clipRect(intersection_rect, SkClipOp::kDifference);
126144
}
127145
}
128-
129-
if (joined_rect.isEmpty()) {
130-
continue;
131-
}
132-
133-
// Subpixels in the platform may not align with the canvas subpixels.
134-
//
135-
// To workaround it, round the floating point bounds and make the rect
136-
// slightly larger.
137-
//
138-
// For example, {0.3, 0.5, 3.1, 4.7} becomes {0, 0, 4, 5}.
139-
joined_rect.set(joined_rect.roundOut());
140-
overlay_layers.insert({view_id, joined_rect});
141-
// Clip the background canvas, so it doesn't contain any of the pixels
142-
// drawn on the overlay layer.
143-
background_canvas->clipRect(joined_rect, SkClipOp::kDifference);
144146
background_canvas->drawPicture(pictures.at(view_id));
145147
}
146148
// Submit the background canvas frame before switching the GL context to
@@ -168,15 +170,16 @@ void AndroidExternalViewEmbedder::SubmitFrame(
168170
params.sizePoints().height() * device_pixel_ratio_,
169171
params.mutatorsStack() //
170172
);
171-
const SkRect& overlay_rect = overlay_layers.at(view_id);
172-
std::unique_ptr<SurfaceFrame> frame =
173-
CreateSurfaceIfNeeded(context, //
174-
view_id, //
175-
pictures.at(view_id), //
176-
overlay_rect //
177-
);
178-
if (should_submit_current_frame) {
179-
frame->Submit();
173+
for (const SkRect& overlay_rect : overlay_layers.at(view_id)) {
174+
std::unique_ptr<SurfaceFrame> frame =
175+
CreateSurfaceIfNeeded(context, //
176+
view_id, //
177+
pictures.at(view_id), //
178+
overlay_rect //
179+
);
180+
if (should_submit_current_frame) {
181+
frame->Submit();
182+
}
180183
}
181184
}
182185
}

shell/platform/android/external_view_embedder/external_view_embedder.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder {
8080
SkRect GetViewRect(int view_id) const;
8181

8282
private:
83+
static const int kMaxLayerAllocations = 2;
84+
8385
// The number of frames the rasterizer task runner will continue
8486
// to run on the platform thread after no platform view is rendered.
8587
//

shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc

Lines changed: 0 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -477,108 +477,6 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame) {
477477
}
478478
}
479479

480-
TEST(AndroidExternalViewEmbedder, SubmitFrame__overlayComposition) {
481-
auto jni_mock = std::make_shared<JNIMock>();
482-
auto android_context =
483-
std::make_shared<AndroidContext>(AndroidRenderingAPI::kSoftware);
484-
485-
auto window = fml::MakeRefCounted<AndroidNativeWindow>(nullptr);
486-
auto gr_context = GrDirectContext::MakeMock(nullptr);
487-
auto frame_size = SkISize::Make(1000, 1000);
488-
auto surface_factory = std::make_shared<TestAndroidSurfaceFactory>(
489-
[&android_context, gr_context, window, frame_size]() {
490-
auto surface_frame_1 = std::make_unique<SurfaceFrame>(
491-
SkSurface::MakeNull(1000, 1000), false,
492-
[](const SurfaceFrame& surface_frame, SkCanvas* canvas) {
493-
return true;
494-
});
495-
496-
auto surface_mock = std::make_unique<SurfaceMock>();
497-
EXPECT_CALL(*surface_mock, AcquireFrame(frame_size))
498-
.Times(1 /* frames */)
499-
.WillOnce(Return(ByMove(std::move(surface_frame_1))));
500-
501-
auto android_surface_mock =
502-
std::make_unique<AndroidSurfaceMock>(android_context);
503-
EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true));
504-
505-
EXPECT_CALL(*android_surface_mock, CreateGPUSurface(gr_context.get()))
506-
.WillOnce(Return(ByMove(std::move(surface_mock))));
507-
508-
EXPECT_CALL(*android_surface_mock, SetNativeWindow(window));
509-
return android_surface_mock;
510-
});
511-
auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
512-
*android_context, jni_mock, surface_factory);
513-
514-
auto raster_thread_merger =
515-
GetThreadMergerFromPlatformThread(/*merged=*/true);
516-
517-
EXPECT_CALL(*jni_mock, FlutterViewBeginFrame());
518-
embedder->BeginFrame(frame_size, nullptr, 1.5, raster_thread_merger);
519-
520-
{
521-
// Add first Android view.
522-
SkMatrix matrix;
523-
MutatorsStack stack;
524-
stack.PushTransform(SkMatrix::Translate(0, 0));
525-
526-
embedder->PrerollCompositeEmbeddedView(
527-
0, std::make_unique<EmbeddedViewParams>(matrix, SkSize::Make(200, 200),
528-
stack));
529-
EXPECT_CALL(*jni_mock, FlutterViewOnDisplayPlatformView(0, 0, 0, 200, 200,
530-
300, 300, stack));
531-
}
532-
533-
auto rect_paint = SkPaint();
534-
rect_paint.setColor(SkColors::kCyan);
535-
rect_paint.setStyle(SkPaint::Style::kFill_Style);
536-
537-
// This simulates Flutter UI that intersects with the first Android view.
538-
embedder->CompositeEmbeddedView(0)->drawRect(
539-
SkRect::MakeXYWH(25, 25, 80, 150), rect_paint);
540-
541-
{
542-
// Add second Android view.
543-
SkMatrix matrix;
544-
MutatorsStack stack;
545-
stack.PushTransform(SkMatrix::Translate(0, 100));
546-
547-
embedder->PrerollCompositeEmbeddedView(
548-
1, std::make_unique<EmbeddedViewParams>(matrix, SkSize::Make(100, 100),
549-
stack));
550-
EXPECT_CALL(*jni_mock, FlutterViewOnDisplayPlatformView(1, 0, 0, 100, 100,
551-
150, 150, stack));
552-
}
553-
// This simulates Flutter UI that intersects with the first and second Android
554-
// views.
555-
embedder->CompositeEmbeddedView(1)->drawRect(SkRect::MakeXYWH(25, 25, 80, 50),
556-
rect_paint);
557-
558-
embedder->CompositeEmbeddedView(1)->drawRect(
559-
SkRect::MakeXYWH(75, 75, 30, 100), rect_paint);
560-
561-
EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface())
562-
.WillRepeatedly([&]() {
563-
return std::make_unique<PlatformViewAndroidJNI::OverlayMetadata>(
564-
1, window);
565-
});
566-
567-
EXPECT_CALL(*jni_mock, FlutterViewDisplayOverlaySurface(1, 25, 25, 80, 150))
568-
.Times(2);
569-
570-
auto surface_frame = std::make_unique<SurfaceFrame>(
571-
SkSurface::MakeNull(1000, 1000), false,
572-
[](const SurfaceFrame& surface_frame, SkCanvas* canvas) mutable {
573-
return true;
574-
});
575-
576-
embedder->SubmitFrame(gr_context.get(), std::move(surface_frame), nullptr);
577-
578-
EXPECT_CALL(*jni_mock, FlutterViewEndFrame());
579-
embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger);
580-
}
581-
582480
TEST(AndroidExternalViewEmbedder, DoesNotCallJNIPlatformThreadOnlyMethods) {
583481
auto jni_mock = std::make_shared<JNIMock>();
584482

0 commit comments

Comments
 (0)