Fix composition when multiple platform views and layers are combined (flutter#25900)

Author: Emmanuel Garcia

shell/platform/android/external_view_embedder/external_view_embedder.cc

@@ -84,7 +84,7 @@ void AndroidExternalViewEmbedder::SubmitFrame(
     return;
   }
 
-  std::unordered_map<int64_t, std::list<SkRect>> overlay_layers;
+  std::unordered_map<int64_t, SkRect> overlay_layers;
   std::unordered_map<int64_t, sk_sp<SkPicture>> pictures;
   SkCanvas* background_canvas = frame->SkiaCanvas();
   auto current_frame_view_count = composition_order_.size();
@@ -101,9 +101,9 @@ void AndroidExternalViewEmbedder::SubmitFrame(
     FML_CHECK(picture);
     pictures.insert({view_id, picture});
 
-    overlay_layers.insert({view_id, {}});
-
     sk_sp<RTree> rtree = view_rtrees_.at(view_id);
+    SkRect joined_rect = SkRect::MakeEmpty();
+
     // Determinate if Flutter UI intersects with any of the previous
     // platform views stacked by z position.
     //
@@ -116,33 +116,31 @@ void AndroidExternalViewEmbedder::SubmitFrame(
       // Each rect corresponds to a native view that renders Flutter UI.
       std::list<SkRect> intersection_rects =
           rtree->searchNonOverlappingDrawnRects(current_view_rect);
-      auto allocation_size = intersection_rects.size();
 
       // Limit the number of native views, so it doesn't grow forever.
       //
       // In this case, the rects are merged into a single one that is the union
       // of all the rects.
-      if (allocation_size > kMaxLayerAllocations) {
-        SkRect joined_rect;
-        for (const SkRect& rect : intersection_rects) {
-          joined_rect.join(rect);
-        }
-        intersection_rects.clear();
-        intersection_rects.push_back(joined_rect);
-      }
-      for (SkRect& intersection_rect : intersection_rects) {
-        // Subpixels in the platform may not align with the canvas subpixels.
-        //
-        // To workaround it, round the floating point bounds and make the rect
-        // slightly larger. For example, {0.3, 0.5, 3.1, 4.7} becomes {0, 0, 4,
-        // 5}.
-        intersection_rect.set(intersection_rect.roundOut());
-        overlay_layers.at(view_id).push_back(intersection_rect);
-        // Clip the background canvas, so it doesn't contain any of the pixels
-        // drawn on the overlay layer.
-        background_canvas->clipRect(intersection_rect, SkClipOp::kDifference);
+      for (const SkRect& rect : intersection_rects) {
+        joined_rect.join(rect);
       }
     }
+
+    if (joined_rect.isEmpty()) {
+      continue;
+    }
+
+    // Subpixels in the platform may not align with the canvas subpixels.
+    //
+    // To workaround it, round the floating point bounds and make the rect
+    // slightly larger.
+    //
+    // For example, {0.3, 0.5, 3.1, 4.7} becomes {0, 0, 4, 5}.
+    joined_rect.set(joined_rect.roundOut());
+    overlay_layers.insert({view_id, joined_rect});
+    // Clip the background canvas, so it doesn't contain any of the pixels
+    // drawn on the overlay layer.
+    background_canvas->clipRect(joined_rect, SkClipOp::kDifference);
     background_canvas->drawPicture(pictures.at(view_id));
   }
   // Submit the background canvas frame before switching the GL context to
@@ -170,16 +168,15 @@ void AndroidExternalViewEmbedder::SubmitFrame(
         params.sizePoints().height() * device_pixel_ratio_,
         params.mutatorsStack()  //
     );
-    for (const SkRect& overlay_rect : overlay_layers.at(view_id)) {
-      std::unique_ptr<SurfaceFrame> frame =
-          CreateSurfaceIfNeeded(context,               //
-                                view_id,               //
-                                pictures.at(view_id),  //
-                                overlay_rect           //
-          );
-      if (should_submit_current_frame) {
-        frame->Submit();
-      }
+    const SkRect& overlay_rect = overlay_layers.at(view_id);
+    std::unique_ptr<SurfaceFrame> frame =
+        CreateSurfaceIfNeeded(context,               //
+                              view_id,               //
+                              pictures.at(view_id),  //
+                              overlay_rect           //
+        );
+    if (should_submit_current_frame) {
+      frame->Submit();
     }
   }
 }

shell/platform/android/external_view_embedder/external_view_embedder.h

@@ -80,8 +80,6 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder {
   SkRect GetViewRect(int view_id) const;
 
  private:
-  static const int kMaxLayerAllocations = 2;
-
   // The number of frames the rasterizer task runner will continue
   // to run on the platform thread after no platform view is rendered.
   //

shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc

@@ -477,6 +477,108 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame) {
   }
 }
 
+TEST(AndroidExternalViewEmbedder, SubmitFrame__overlayComposition) {
+  auto jni_mock = std::make_shared<JNIMock>();
+  auto android_context =
+      std::make_shared<AndroidContext>(AndroidRenderingAPI::kSoftware);
+
+  auto window = fml::MakeRefCounted<AndroidNativeWindow>(nullptr);
+  auto gr_context = GrDirectContext::MakeMock(nullptr);
+  auto frame_size = SkISize::Make(1000, 1000);
+  auto surface_factory = std::make_shared<TestAndroidSurfaceFactory>(
+      [&android_context, gr_context, window, frame_size]() {
+        auto surface_frame_1 = std::make_unique<SurfaceFrame>(
+            SkSurface::MakeNull(1000, 1000), false,
+            [](const SurfaceFrame& surface_frame, SkCanvas* canvas) {
+              return true;
+            });
+
+        auto surface_mock = std::make_unique<SurfaceMock>();
+        EXPECT_CALL(*surface_mock, AcquireFrame(frame_size))
+            .Times(1 /* frames */)
+            .WillOnce(Return(ByMove(std::move(surface_frame_1))));
+
+        auto android_surface_mock =
+            std::make_unique<AndroidSurfaceMock>(android_context);
+        EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true));
+
+        EXPECT_CALL(*android_surface_mock, CreateGPUSurface(gr_context.get()))
+            .WillOnce(Return(ByMove(std::move(surface_mock))));
+
+        EXPECT_CALL(*android_surface_mock, SetNativeWindow(window));
+        return android_surface_mock;
+      });
+  auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
+      *android_context, jni_mock, surface_factory);
+
+  auto raster_thread_merger =
+      GetThreadMergerFromPlatformThread(/*merged=*/true);
+
+  EXPECT_CALL(*jni_mock, FlutterViewBeginFrame());
+  embedder->BeginFrame(frame_size, nullptr, 1.5, raster_thread_merger);
+
+  {
+    // Add first Android view.
+    SkMatrix matrix;
+    MutatorsStack stack;
+    stack.PushTransform(SkMatrix::Translate(0, 0));
+
+    embedder->PrerollCompositeEmbeddedView(
+        0, std::make_unique<EmbeddedViewParams>(matrix, SkSize::Make(200, 200),
+                                                stack));
+    EXPECT_CALL(*jni_mock, FlutterViewOnDisplayPlatformView(0, 0, 0, 200, 200,
+                                                            300, 300, stack));
+  }
+
+  auto rect_paint = SkPaint();
+  rect_paint.setColor(SkColors::kCyan);
+  rect_paint.setStyle(SkPaint::Style::kFill_Style);
+
+  // This simulates Flutter UI that intersects with the first Android view.
+  embedder->CompositeEmbeddedView(0)->drawRect(
+      SkRect::MakeXYWH(25, 25, 80, 150), rect_paint);
+
+  {
+    // Add second Android view.
+    SkMatrix matrix;
+    MutatorsStack stack;
+    stack.PushTransform(SkMatrix::Translate(0, 100));
+
+    embedder->PrerollCompositeEmbeddedView(
+        1, std::make_unique<EmbeddedViewParams>(matrix, SkSize::Make(100, 100),
+                                                stack));
+    EXPECT_CALL(*jni_mock, FlutterViewOnDisplayPlatformView(1, 0, 0, 100, 100,
+                                                            150, 150, stack));
+  }
+  // This simulates Flutter UI that intersects with the first and second Android
+  // views.
+  embedder->CompositeEmbeddedView(1)->drawRect(SkRect::MakeXYWH(25, 25, 80, 50),
+                                               rect_paint);
+
+  embedder->CompositeEmbeddedView(1)->drawRect(
+      SkRect::MakeXYWH(75, 75, 30, 100), rect_paint);
+
+  EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface())
+      .WillRepeatedly([&]() {
+        return std::make_unique<PlatformViewAndroidJNI::OverlayMetadata>(
+            1, window);
+      });
+
+  EXPECT_CALL(*jni_mock, FlutterViewDisplayOverlaySurface(1, 25, 25, 80, 150))
+      .Times(2);
+
+  auto surface_frame = std::make_unique<SurfaceFrame>(
+      SkSurface::MakeNull(1000, 1000), false,
+      [](const SurfaceFrame& surface_frame, SkCanvas* canvas) mutable {
+        return true;
+      });
+
+  embedder->SubmitFrame(gr_context.get(), std::move(surface_frame), nullptr);
+
+  EXPECT_CALL(*jni_mock, FlutterViewEndFrame());
+  embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger);
+}
+
 TEST(AndroidExternalViewEmbedder, DoesNotCallJNIPlatformThreadOnlyMethods) {
   auto jni_mock = std::make_shared<JNIMock>();