add tests for state tracking after a platform layer and fix a bug

Author: flar

flow/layers/layer_state_stack.cc

@@ -11,11 +11,19 @@ namespace flutter {
 using AutoRestore = LayerStateStack::AutoRestore;
 using MutatorContext = LayerStateStack::MutatorContext;
 
-void LayerStateStack::set_canvas_delegate(SkCanvas* canvas) {
+void LayerStateStack::clear_delegates() {
   if (canvas_) {
     canvas_->restoreToCount(canvas_restore_count_);
     canvas_ = nullptr;
   }
+  if (builder_) {
+    builder_->restoreToCount(builder_restore_count_);
+    builder_ = nullptr;
+  }
+}
+
+void LayerStateStack::set_canvas_delegate(SkCanvas* canvas) {
+  clear_delegates();
   if (canvas) {
     canvas_restore_count_ = canvas->getSaveCount();
     canvas_ = canvas;
@@ -24,10 +32,7 @@ void LayerStateStack::set_canvas_delegate(SkCanvas* canvas) {
 }
 
 void LayerStateStack::set_builder_delegate(DisplayListBuilder* builder) {
-  if (builder_) {
-    builder_->restoreToCount(builder_restore_count_);
-    builder_ = nullptr;
-  }
+  clear_delegates();
   if (builder) {
     builder_restore_count_ = builder->getSaveCount();
     builder_ = builder;

flow/layers/layer_state_stack.h

@@ -19,14 +19,23 @@ class LayerStateStack {
     do_checkerboard_ = checkerboard;
   }
 
+  // Clears out any old delegate to make room for a new one.
+  void clear_delegates();
+
   SkCanvas* canvas_delegate() { return canvas_; }
+  // Clears the old delegate and sets the canvas delegate to the indicated
+  // canvas (if not nullptr). This ensures that only one delegate - either
+  // a canvas or a builder - is present at any one time.
   void set_canvas_delegate(SkCanvas* canvas);
-  void set_builder_delegate(sk_sp<DisplayListBuilder> builder) {
-    set_builder_delegate(builder.get());
-  }
 
   DisplayListBuilder* builder_delegate() { return builder_; }
+  // Clears the old delegate and sets the builder delegate to the indicated
+  // buider (if not nullptr). This ensures that only one delegate - either
+  // a canvas or a builder - is present at any one time.
   void set_builder_delegate(DisplayListBuilder* builder);
+  void set_builder_delegate(sk_sp<DisplayListBuilder> builder) {
+    set_builder_delegate(builder.get());
+  }
   void set_builder_delegate(DisplayListCanvasRecorder& recorder) {
     set_builder_delegate(recorder.builder().get());
   }

flow/layers/layer_state_stack_unittests.cc

@@ -8,6 +8,7 @@
 #include "flutter/display_list/display_list_image_filter.h"
 #include "flutter/flow/layers/layer_state_stack.h"
 #include "flutter/testing/display_list_testing.h"
+#include "flutter/testing/mock_canvas.h"
 
 namespace flutter {
 namespace testing {
@@ -32,6 +33,31 @@ TEST(LayerStateStack, Defaults) {
   ASSERT_EQ(dl_paint, DlPaint());
 }
 
+TEST(LayerStateStack, OneDelegateAtATime) {
+  LayerStateStack state_stack;
+  ASSERT_EQ(state_stack.canvas_delegate(), nullptr);
+  ASSERT_EQ(state_stack.builder_delegate(), nullptr);
+
+  DisplayListBuilder builder;
+  state_stack.set_builder_delegate(&builder);
+  ASSERT_EQ(state_stack.canvas_delegate(), nullptr);
+  ASSERT_EQ(state_stack.builder_delegate(), &builder);
+
+  MockCanvas canvas;
+  state_stack.set_canvas_delegate(&canvas);
+  ASSERT_EQ(state_stack.canvas_delegate(), &canvas);
+  ASSERT_EQ(state_stack.builder_delegate(), nullptr);
+
+  state_stack.clear_delegates();
+  ASSERT_EQ(state_stack.canvas_delegate(), nullptr);
+  ASSERT_EQ(state_stack.builder_delegate(), nullptr);
+
+  state_stack.set_builder_delegate(&builder);
+  state_stack.clear_delegates();
+  ASSERT_EQ(state_stack.canvas_delegate(), nullptr);
+  ASSERT_EQ(state_stack.builder_delegate(), nullptr);
+}
+
 TEST(LayerStateStack, Opacity) {
   SkRect rect = {10, 10, 20, 20};
 

flow/layers/platform_view_layer.cc

@@ -41,9 +41,12 @@ void PlatformViewLayer::Paint(PaintContext& context) const {
   EmbedderPaintContext embedder_context =
       context.view_embedder->CompositeEmbeddedView(view_id_);
   context.canvas = embedder_context.canvas;
-  context.state_stack.set_canvas_delegate(context.canvas);
   context.builder = embedder_context.builder;
-  context.state_stack.set_builder_delegate(context.builder);
+  if (context.builder) {
+    context.state_stack.set_builder_delegate(context.builder);
+  } else {
+    context.state_stack.set_canvas_delegate(context.canvas);
+  }
 }
 
 }  // namespace flutter

flow/layers/platform_view_layer_unittests.cc

@@ -4,6 +4,7 @@
 
 #include "flutter/flow/layers/clip_rect_layer.h"
 #include "flutter/flow/layers/platform_view_layer.h"
+#include "flutter/flow/layers/transform_layer.h"
 
 #include "flutter/flow/testing/layer_test.h"
 #include "flutter/flow/testing/mock_embedder.h"
@@ -95,5 +96,46 @@ TEST_F(PlatformViewLayerTest, OpacityInheritance) {
   EXPECT_EQ(context->renderable_state_flags, 0);
 }
 
+TEST_F(PlatformViewLayerTest, StateTransfer) {
+  const SkMatrix transform1 = SkMatrix::Translate(5, 5);
+  const SkMatrix transform2 = SkMatrix::Translate(15, 15);
+  const SkMatrix combined_transform = SkMatrix::Translate(20, 20);
+  const SkPoint layer_offset = SkPoint::Make(0.0f, 0.0f);
+  const SkSize layer_size = SkSize::Make(8.0f, 8.0f);
+  const int64_t view_id = 0;
+  const SkPath path1 = SkPath().addOval({10, 10, 20, 20});
+  const SkPath path2 = SkPath().addOval({15, 15, 30, 30});
+
+  // transform_layer1
+  //   |- child1
+  //   |- platform_layer
+  //   |- transform_layer2
+  //        |- child2
+  auto transform_layer1 = std::make_shared<TransformLayer>(transform1);
+  auto transform_layer2 = std::make_shared<TransformLayer>(transform2);
+  auto platform_layer =
+      std::make_shared<PlatformViewLayer>(layer_offset, layer_size, view_id);
+  auto child1 = std::make_shared<MockLayer>(path1);
+  child1->set_expected_paint_matrix(transform1);
+  auto child2 = std::make_shared<MockLayer>(path2);
+  child2->set_expected_paint_matrix(combined_transform);
+  transform_layer1->Add(child1);
+  transform_layer1->Add(platform_layer);
+  transform_layer1->Add(transform_layer2);
+  transform_layer2->Add(child2);
+
+  auto embedder = MockViewEmbedder();
+  DisplayListCanvasRecorder recorder({0, 0, 500, 500});
+  embedder.AddRecorder(&recorder);
+
+  PrerollContext* preroll_ctx = preroll_context();
+  preroll_ctx->view_embedder = &embedder;
+  transform_layer1->Preroll(preroll_ctx, SkMatrix());
+
+  PaintContext& paint_ctx = paint_context();
+  paint_ctx.view_embedder = &embedder;
+  transform_layer1->Paint(paint_ctx);
+}
+
 }  // namespace testing
 }  // namespace flutter

flow/testing/mock_embedder.cc

@@ -11,6 +11,15 @@ MockViewEmbedder::MockViewEmbedder() = default;
 
 MockViewEmbedder::~MockViewEmbedder() = default;
 
+void MockViewEmbedder::AddCanvas(SkCanvas* canvas) {
+  contexts_.emplace_back(EmbedderPaintContext{canvas, nullptr});
+}
+
+void MockViewEmbedder::AddRecorder(DisplayListCanvasRecorder* recorder) {
+  contexts_.emplace_back(
+      EmbedderPaintContext{recorder, recorder->builder().get()});
+}
+
 // |ExternalViewEmbedder|
 SkCanvas* MockViewEmbedder::GetRootCanvas() {
   return nullptr;
@@ -43,7 +52,9 @@ std::vector<DisplayListBuilder*> MockViewEmbedder::GetCurrentBuilders() {
 
 // |ExternalViewEmbedder|
 EmbedderPaintContext MockViewEmbedder::CompositeEmbeddedView(int view_id) {
-  return {nullptr, nullptr};
+  EmbedderPaintContext context = contexts_.front();
+  contexts_.pop_front();
+  return context;
 }
 
 }  // namespace testing

flow/testing/mock_embedder.h

@@ -16,6 +16,9 @@ class MockViewEmbedder : public ExternalViewEmbedder {
 
   ~MockViewEmbedder();
 
+  void AddCanvas(SkCanvas* canvas);
+  void AddRecorder(DisplayListCanvasRecorder* recorder);
+
   // |ExternalViewEmbedder|
   SkCanvas* GetRootCanvas() override;
 
@@ -42,6 +45,9 @@ class MockViewEmbedder : public ExternalViewEmbedder {
 
   // |ExternalViewEmbedder|
   EmbedderPaintContext CompositeEmbeddedView(int view_id) override;
+
+ private:
+  std::deque<EmbedderPaintContext> contexts_;
 };
 
 }  // namespace testing

flow/testing/mock_layer.cc

@@ -52,6 +52,13 @@ void MockLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
 void MockLayer::Paint(PaintContext& context) const {
   FML_DCHECK(needs_painting(context));
 
+  if (expected_paint_matrix_.has_value()) {
+    SkMatrix matrix = context.builder ? context.builder->getTransform()
+                                      : context.canvas->getTotalMatrix();
+
+    ASSERT_EQ(matrix, expected_paint_matrix_.value());
+  }
+
   SkPaint sk_paint = fake_paint_;
   context.state_stack.fill(sk_paint);
   context.canvas->drawPath(fake_paint_path_, sk_paint);

flow/testing/mock_layer.h

@@ -102,12 +102,17 @@ class MockLayer : public Layer {
     return *this;
   }
 
+  void set_expected_paint_matrix(const SkMatrix& matrix) {
+    expected_paint_matrix_ = matrix;
+  }
+
  private:
   MutatorsStack parent_mutators_;
   SkMatrix parent_matrix_;
   SkRect parent_cull_rect_ = SkRect::MakeEmpty();
   SkPath fake_paint_path_;
   SkPaint fake_paint_;
+  std::optional<SkMatrix> expected_paint_matrix_;
 
   static constexpr int kParentHasPlatformView = 1 << 0;
   static constexpr int kParentHasTextureLayer = 1 << 1;