[CP] Remove LayerStateStack premature culling (#38163)

* Fix premature LayerStateStack layer culling (#38159) * remove premature opacity culling * remove premature clip culling * empty
flutter · Dec 16, 2022 · fdead7a · fdead7a
1 parent b379dc9
commit fdead7a
Show file tree

Hide file tree

Showing 9 changed files with 130 additions and 12 deletions.
diff --git a/flow/layers/clip_path_layer_unittests.cc b/flow/layers/clip_path_layer_unittests.cc
@@ -6,8 +6,10 @@
 
 #include "flutter/flow/layers/layer_tree.h"
 #include "flutter/flow/layers/opacity_layer.h"
+#include "flutter/flow/layers/platform_view_layer.h"
 #include "flutter/flow/raster_cache_item.h"
 #include "flutter/flow/testing/layer_test.h"
+#include "flutter/flow/testing/mock_embedder.h"
 #include "flutter/flow/testing/mock_layer.h"
 #include "flutter/fml/macros.h"
 #include "flutter/testing/mock_canvas.h"
@@ -533,5 +535,30 @@ TEST_F(ClipPathLayerTest, LayerCached) {
                                    cache_canvas, &paint));
 }
 
+TEST_F(ClipPathLayerTest, EmptyClipDoesNotCullPlatformView) {
+  const SkPoint view_offset = SkPoint::Make(0.0f, 0.0f);
+  const SkSize view_size = SkSize::Make(8.0f, 8.0f);
+  const int64_t view_id = 42;
+  auto platform_view =
+      std::make_shared<PlatformViewLayer>(view_offset, view_size, view_id);
+
+  auto layer_clip = SkPath().addRect(kEmptyRect);
+  auto clip =
+      std::make_shared<ClipPathLayer>(layer_clip, Clip::antiAliasWithSaveLayer);
+  clip->Add(platform_view);
+
+  auto embedder = MockViewEmbedder();
+  SkCanvas fake_overlay_canvas;
+  embedder.AddCanvas(&fake_overlay_canvas);
+  preroll_context()->view_embedder = &embedder;
+  paint_context().view_embedder = &embedder;
+
+  clip->Preroll(preroll_context());
+  EXPECT_EQ(embedder.prerolled_views(), std::vector<int64_t>({view_id}));
+
+  clip->Paint(paint_context());
+  EXPECT_EQ(embedder.painted_views(), std::vector<int64_t>({view_id}));
+}
+
 }  // namespace testing
 }  // namespace flutter
diff --git a/flow/layers/clip_rect_layer_unittests.cc b/flow/layers/clip_rect_layer_unittests.cc
@@ -6,7 +6,9 @@
 
 #include "flutter/flow/layers/layer_tree.h"
 #include "flutter/flow/layers/opacity_layer.h"
+#include "flutter/flow/layers/platform_view_layer.h"
 #include "flutter/flow/testing/layer_test.h"
+#include "flutter/flow/testing/mock_embedder.h"
 #include "flutter/flow/testing/mock_layer.h"
 #include "flutter/fml/macros.h"
 #include "flutter/testing/mock_canvas.h"
@@ -514,5 +516,28 @@ TEST_F(ClipRectLayerTest, LayerCached) {
                                    cache_canvas, &paint));
 }
 
+TEST_F(ClipRectLayerTest, EmptyClipDoesNotCullPlatformView) {
+  const SkPoint view_offset = SkPoint::Make(0.0f, 0.0f);
+  const SkSize view_size = SkSize::Make(8.0f, 8.0f);
+  const int64_t view_id = 42;
+  auto platform_view =
+      std::make_shared<PlatformViewLayer>(view_offset, view_size, view_id);
+
+  auto clip = std::make_shared<ClipRectLayer>(kEmptyRect, Clip::hardEdge);
+  clip->Add(platform_view);
+
+  auto embedder = MockViewEmbedder();
+  SkCanvas fake_overlay_canvas;
+  embedder.AddCanvas(&fake_overlay_canvas);
+  preroll_context()->view_embedder = &embedder;
+  paint_context().view_embedder = &embedder;
+
+  clip->Preroll(preroll_context());
+  EXPECT_EQ(embedder.prerolled_views(), std::vector<int64_t>({view_id}));
+
+  clip->Paint(paint_context());
+  EXPECT_EQ(embedder.painted_views(), std::vector<int64_t>({view_id}));
+}
+
 }  // namespace testing
 }  // namespace flutter
diff --git a/flow/layers/clip_rrect_layer_unittests.cc b/flow/layers/clip_rrect_layer_unittests.cc
@@ -6,7 +6,9 @@
 
 #include "flutter/flow/layers/layer_tree.h"
 #include "flutter/flow/layers/opacity_layer.h"
+#include "flutter/flow/layers/platform_view_layer.h"
 #include "flutter/flow/testing/layer_test.h"
+#include "flutter/flow/testing/mock_embedder.h"
 #include "flutter/flow/testing/mock_layer.h"
 #include "flutter/fml/macros.h"
 #include "flutter/testing/mock_canvas.h"
@@ -562,5 +564,29 @@ TEST_F(ClipRRectLayerTest, NoSaveLayerShouldNotCache) {
   EXPECT_EQ(clip_cache_item->cache_state(), RasterCacheItem::CacheState::kNone);
 }
 
+TEST_F(ClipRRectLayerTest, EmptyClipDoesNotCullPlatformView) {
+  const SkPoint view_offset = SkPoint::Make(0.0f, 0.0f);
+  const SkSize view_size = SkSize::Make(8.0f, 8.0f);
+  const int64_t view_id = 42;
+  auto platform_view =
+      std::make_shared<PlatformViewLayer>(view_offset, view_size, view_id);
+
+  SkRRect clip_rrect = SkRRect::MakeRectXY(kEmptyRect, 20, 20);
+  auto clip = std::make_shared<ClipRRectLayer>(clip_rrect, Clip::antiAlias);
+  clip->Add(platform_view);
+
+  auto embedder = MockViewEmbedder();
+  SkCanvas fake_overlay_canvas;
+  embedder.AddCanvas(&fake_overlay_canvas);
+  preroll_context()->view_embedder = &embedder;
+  paint_context().view_embedder = &embedder;
+
+  clip->Preroll(preroll_context());
+  EXPECT_EQ(embedder.prerolled_views(), std::vector<int64_t>({view_id}));
+
+  clip->Paint(paint_context());
+  EXPECT_EQ(embedder.painted_views(), std::vector<int64_t>({view_id}));
+}
+
 }  // namespace testing
 }  // namespace flutter
diff --git a/flow/layers/clip_shape_layer.h b/flow/layers/clip_shape_layer.h
@@ -78,9 +78,6 @@ class ClipShapeLayer : public CacheableContainerLayer {
 
     auto mutator = context.state_stack.save();
     ApplyClip(mutator);
-    if (context.state_stack.content_culled(child_paint_bounds())) {
-      return;
-    }
 
     if (!UsesSaveLayer()) {
       PaintChildren(context);

diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc
@@ -87,9 +87,7 @@ void OpacityLayer::Paint(PaintContext& context) const {
     }
   }
 
-  if (!context.state_stack.painting_is_nop()) {
-    PaintChildren(context);
-  }
+  PaintChildren(context);
 }
 
 }  // namespace flutter
diff --git a/flow/layers/opacity_layer_unittests.cc b/flow/layers/opacity_layer_unittests.cc
@@ -7,10 +7,12 @@
 #include "flutter/flow/layers/clip_rect_layer.h"
 #include "flutter/flow/layers/image_filter_layer.h"
 #include "flutter/flow/layers/layer_tree.h"
+#include "flutter/flow/layers/platform_view_layer.h"
 #include "flutter/flow/layers/transform_layer.h"
 #include "flutter/flow/raster_cache_util.h"
 #include "flutter/flow/testing/diff_context_test.h"
 #include "flutter/flow/testing/layer_test.h"
+#include "flutter/flow/testing/mock_embedder.h"
 #include "flutter/flow/testing/mock_layer.h"
 #include "flutter/fml/macros.h"
 #include "flutter/testing/display_list_testing.h"
@@ -274,11 +276,16 @@ TEST_F(OpacityLayerTest, FullyTransparent) {
       mock_layer->parent_mutators(),
       std::vector({Mutator(layer_transform), Mutator(SK_AlphaTRANSPARENT)}));
 
-  auto expected_draw_calls =
-      std::vector({MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
-                   MockCanvas::DrawCall{
-                       1, MockCanvas::ConcatMatrixData{SkM44(layer_transform)}},
-                   MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}});
+  auto expected_draw_calls = std::vector(
+      {MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
+       MockCanvas::DrawCall{
+           1, MockCanvas::ConcatMatrixData{SkM44(layer_transform)}},
+       MockCanvas::DrawCall{1, MockCanvas::SaveData{2}},
+       MockCanvas::DrawCall{
+           2, MockCanvas::ClipRectData{kEmptyRect, SkClipOp::kIntersect,
+                                       MockCanvas::kHard_ClipEdgeStyle}},
+       MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
+       MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}});
   layer->Paint(paint_context());
   EXPECT_EQ(mock_canvas().draw_calls(), expected_draw_calls);
 }
@@ -686,5 +693,30 @@ TEST_F(OpacityLayerTest, FullyOpaqueWithFractionalValues) {
   EXPECT_EQ(mock_canvas().draw_calls(), expected_draw_calls);
 }
 
+TEST_F(OpacityLayerTest, FullyTransparentDoesNotCullPlatformView) {
+  const SkPoint opacity_offset = SkPoint::Make(0.5f, 1.5f);
+  const SkPoint view_offset = SkPoint::Make(0.0f, 0.0f);
+  const SkSize view_size = SkSize::Make(8.0f, 8.0f);
+  const int64_t view_id = 42;
+  auto platform_view =
+      std::make_shared<PlatformViewLayer>(view_offset, view_size, view_id);
+
+  auto opacity =
+      std::make_shared<OpacityLayer>(SK_AlphaTRANSPARENT, opacity_offset);
+  opacity->Add(platform_view);
+
+  auto embedder = MockViewEmbedder();
+  SkCanvas fake_overlay_canvas;
+  embedder.AddCanvas(&fake_overlay_canvas);
+  preroll_context()->view_embedder = &embedder;
+  paint_context().view_embedder = &embedder;
+
+  opacity->Preroll(preroll_context());
+  EXPECT_EQ(embedder.prerolled_views(), std::vector<int64_t>({view_id}));
+
+  opacity->Paint(paint_context());
+  EXPECT_EQ(embedder.painted_views(), std::vector<int64_t>({view_id}));
+}
+
 }  // namespace testing
 }  // namespace flutter
diff --git a/flow/layers/platform_view_layer_unittests.cc b/flow/layers/platform_view_layer_unittests.cc
@@ -76,6 +76,11 @@ TEST_F(PlatformViewLayerTest, ClippedPlatformViewPrerollsAndPaintsNothing) {
            MockCanvas::DrawCall{
                1, MockCanvas::ClipRectData{parent_clip, SkClipOp::kIntersect,
                                            MockCanvas::kHard_ClipEdgeStyle}},
+           MockCanvas::DrawCall{1, MockCanvas::SaveData{2}},
+           MockCanvas::DrawCall{
+               2, MockCanvas::ClipRectData{child_clip, SkClipOp::kIntersect,
+                                           MockCanvas::kHard_ClipEdgeStyle}},
+           MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
            MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}}));
 }
 

diff --git a/flow/testing/mock_embedder.cc b/flow/testing/mock_embedder.cc
@@ -38,7 +38,9 @@ void MockViewEmbedder::BeginFrame(
 // |ExternalViewEmbedder|
 void MockViewEmbedder::PrerollCompositeEmbeddedView(
     int view_id,
-    std::unique_ptr<EmbeddedViewParams> params) {}
+    std::unique_ptr<EmbeddedViewParams> params) {
+  prerolled_views_.emplace_back(view_id);
+}
 
 // |ExternalViewEmbedder|
 std::vector<SkCanvas*> MockViewEmbedder::GetCurrentCanvases() {
@@ -52,6 +54,7 @@ std::vector<DisplayListBuilder*> MockViewEmbedder::GetCurrentBuilders() {
 
 // |ExternalViewEmbedder|
 EmbedderPaintContext MockViewEmbedder::CompositeEmbeddedView(int view_id) {
+  painted_views_.emplace_back(view_id);
   EmbedderPaintContext context = contexts_.front();
   contexts_.pop_front();
   return context;

diff --git a/flow/testing/mock_embedder.h b/flow/testing/mock_embedder.h
@@ -46,8 +46,13 @@ class MockViewEmbedder : public ExternalViewEmbedder {
   // |ExternalViewEmbedder|
   EmbedderPaintContext CompositeEmbeddedView(int view_id) override;
 
+  std::vector<int64_t> prerolled_views() const { return prerolled_views_; }
+  std::vector<int64_t> painted_views() const { return painted_views_; }
+
  private:
   std::deque<EmbedderPaintContext> contexts_;
+  std::vector<int64_t> prerolled_views_;
+  std::vector<int64_t> painted_views_;
 };
 
 }  // namespace testing