Reland: Partial repaint platform views (#54231)

Relands #54219 reverted in
#54230.

The tracked area of `PlatformViewLayer` now covers entire frame ensuring
full repaint when platform view is removed.
Added `FullRepaintAfterRemovingLayer` test.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or the PR is [test-exempt]. See [testing the engine] for
instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat

Author: knopp

flow/diff_context.cc

@@ -27,7 +27,7 @@ void DiffContext::BeginSubtree() {
   bool had_integral_transform = state_.integral_transform;
   state_.rect_index = rects_->size();
   state_.has_filter_bounds_adjustment = false;
-  state_.has_texture = false;
+  state_.has_volatile_layer = false;
   state_.integral_transform = false;
 
   if (had_integral_transform) {
@@ -203,14 +203,20 @@ void DiffContext::AddLayerBounds(const SkRect& rect) {
   }
 }
 
-void DiffContext::MarkSubtreeHasTextureLayer() {
+void DiffContext::RepaintEntireFrame() {
+  auto frame_rect = SkRect::MakeIWH(frame_size_.width(), frame_size_.height());
+  rects_->push_back(frame_rect);
+  AddDamage(frame_rect);
+}
+
+void DiffContext::MarkSubtreeHasVolatileLayer() {
   // Set the has_texture flag on current state and all parent states. That
   // way we'll know that we can't skip diff for retained layers because
   // they contain a TextureLayer.
   for (auto& state : state_stack_) {
-    state.has_texture = true;
+    state.has_volatile_layer = true;
   }
-  state_.has_texture = true;
+  state_.has_volatile_layer = true;
 }
 
 void DiffContext::AddExistingPaintRegion(const PaintRegion& region) {
@@ -238,7 +244,7 @@ PaintRegion DiffContext::CurrentSubtreeRegion() const {
       readbacks_.begin(), readbacks_.end(),
       [&](const Readback& r) { return r.position >= state_.rect_index; });
   return PaintRegion(rects_, state_.rect_index, rects_->size(), has_readback,
-                     state_.has_texture);
+                     state_.has_volatile_layer);
 }
 
 void DiffContext::AddDamage(const PaintRegion& damage) {

flow/diff_context.h

@@ -107,14 +107,18 @@ class DiffContext {
 
   bool IsSubtreeDirty() const { return state_.dirty; }
 
-  // Marks that current subtree contains a TextureLayer. This is needed to
-  // ensure that we'll Diff the TextureLayer even if inside retained layer.
-  void MarkSubtreeHasTextureLayer();
+  // Marks that current subtree contains a volatile layer. A volatile layer will
+  // do diffing even if it is a retained subtree. Necessary for TextureLayer
+  // and PlatformViewLayer.
+  void MarkSubtreeHasVolatileLayer();
 
   // Add layer bounds to current paint region; rect is in "local" (layer)
   // coordinates.
   void AddLayerBounds(const SkRect& rect);
 
+  // Marks entire frame as dirty.
+  void RepaintEntireFrame();
+
   // Add entire paint region of retained layer for current subtree. This can
   // only be used in subtrees that are not dirty, otherwise ancestor transforms
   // or clips may result in different paint region.
@@ -231,7 +235,7 @@ class DiffContext {
     bool has_filter_bounds_adjustment = false;
 
     // Whether there is a texture layer in this subtree.
-    bool has_texture = false;
+    bool has_volatile_layer = false;
   };
 
   void MakeTransformIntegral(DisplayListMatrixClipState& matrix_clip);

flow/layers/container_layer.cc

@@ -78,7 +78,7 @@ void ContainerLayer::DiffChildren(DiffContext* context,
       auto prev_layer = prev_layers[i_prev];
       auto paint_region = context->GetOldLayerPaintRegion(prev_layer.get());
       if (layer == prev_layer && !paint_region.has_readback() &&
-          !paint_region.has_texture()) {
+          !paint_region.has_volatile_layer()) {
         // for retained layers, stop processing the subtree and add existing
         // region; We know current subtree is not dirty (every ancestor up to
         // here matches) so the retained subtree will render identically to

flow/layers/platform_view_layer.cc

@@ -13,6 +13,17 @@ PlatformViewLayer::PlatformViewLayer(const SkPoint& offset,
                                      int64_t view_id)
     : offset_(offset), size_(size), view_id_(view_id) {}
 
+void PlatformViewLayer::Diff(DiffContext* context, const Layer* old_layer) {
+  DiffContext::AutoSubtreeRestore subtree(context);
+  // Ensure that Diff is called again even if this layer is in retained subtree.
+  context->MarkSubtreeHasVolatileLayer();
+  // Partial repaint is disabled when platform view is present. This will also
+  // set whole frame as the layer paint region, which will ensure full repaint
+  // when the layer is removed.
+  context->RepaintEntireFrame();
+  context->SetLayerPaintRegion(this, context->CurrentSubtreeRegion());
+}
+
 void PlatformViewLayer::Preroll(PrerollContext* context) {
   set_paint_bounds(SkRect::MakeXYWH(offset_.x(), offset_.y(), size_.width(),
                                     size_.height()));

flow/layers/platform_view_layer.h

@@ -16,6 +16,7 @@ class PlatformViewLayer : public Layer {
  public:
   PlatformViewLayer(const SkPoint& offset, const SkSize& size, int64_t view_id);
 
+  void Diff(DiffContext* context, const Layer* old_layer) override;
   void Preroll(PrerollContext* context) override;
   void Paint(PaintContext& context) const override;
 

flow/layers/platform_view_layer_unittests.cc

@@ -6,6 +6,7 @@
 #include "flutter/flow/layers/platform_view_layer.h"
 #include "flutter/flow/layers/transform_layer.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"
@@ -139,5 +140,45 @@ TEST_F(PlatformViewLayerTest, StateTransfer) {
   transform_layer1->Paint(paint_ctx);
 }
 
+using PlatformViewLayerDiffTest = DiffContextTest;
+
+TEST_F(PlatformViewLayerDiffTest, PlatformViewRetainedLayer) {
+  MockLayerTree tree1(SkISize::Make(800, 600));
+  auto container = std::make_shared<ContainerLayer>();
+  tree1.root()->Add(container);
+  auto layer = std::make_shared<PlatformViewLayer>(SkPoint::Make(100, 100),
+                                                   SkSize::Make(100, 100), 0);
+  container->Add(layer);
+
+  MockLayerTree tree2(SkISize::Make(800, 600));
+  tree2.root()->Add(container);  // retained layer
+
+  auto damage = DiffLayerTree(tree1, MockLayerTree(SkISize::Make(800, 600)));
+  EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 800, 600));
+
+  damage = DiffLayerTree(tree2, tree1);
+  EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 800, 600));
+}
+
+TEST_F(PlatformViewLayerDiffTest, FullRepaintAfterRemovingLayer) {
+  MockLayerTree tree1(SkISize::Make(800, 600));
+  auto container = std::make_shared<ContainerLayer>();
+  tree1.root()->Add(container);
+  auto layer = std::make_shared<PlatformViewLayer>(SkPoint::Make(100, 100),
+                                                   SkSize::Make(100, 100), 0);
+  container->Add(layer);
+
+  auto damage = DiffLayerTree(tree1, MockLayerTree(SkISize::Make(800, 600)));
+  EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 800, 600));
+
+  // Second layer tree with the PlatformViewLayer removed.
+  MockLayerTree tree2(SkISize::Make(800, 600));
+  auto container2 = std::make_shared<ContainerLayer>();
+  tree2.root()->Add(container2);
+
+  damage = DiffLayerTree(tree2, tree1);
+  EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 800, 600));
+}
+
 }  // namespace testing
 }  // namespace flutter

flow/layers/texture_layer.cc

@@ -34,7 +34,7 @@ void TextureLayer::Diff(DiffContext* context, const Layer* old_layer) {
   // TextureLayer is inside retained layer.
   // See ContainerLayer::DiffChildren
   // https://github.com/flutter/flutter/issues/92925
-  context->MarkSubtreeHasTextureLayer();
+  context->MarkSubtreeHasVolatileLayer();
   context->AddLayerBounds(SkRect::MakeXYWH(offset_.x(), offset_.y(),
                                            size_.width(), size_.height()));
   context->SetLayerPaintRegion(this, context->CurrentSubtreeRegion());

flow/paint_region.h

@@ -30,12 +30,12 @@ class PaintRegion {
               size_t from,
               size_t to,
               bool has_readback,
-              bool has_texture)
+              bool has_volatile_layer)
       : rects_(std::move(rects)),
         from_(from),
         to_(to),
         has_readback_(has_readback),
-        has_texture_(has_texture) {}
+        has_volatile_layer_(has_volatile_layer) {}
 
   std::vector<SkRect>::const_iterator begin() const {
     FML_DCHECK(is_valid());
@@ -56,16 +56,16 @@ class PaintRegion {
   // that performs readback
   bool has_readback() const { return has_readback_; }
 
-  // Returns whether there is a TextureLayer in subtree represented by this
-  // region.
-  bool has_texture() const { return has_texture_; }
+  // Returns whether there is a TextureLayer or PlatformViewLayer in subtree
+  // represented by this region.
+  bool has_volatile_layer() const { return has_volatile_layer_; }
 
  private:
   std::shared_ptr<std::vector<SkRect>> rects_;
   size_t from_ = 0;
   size_t to_ = 0;
   bool has_readback_ = false;
-  bool has_texture_ = false;
+  bool has_volatile_layer_ = false;
 };
 
 }  // namespace flutter

shell/common/rasterizer.cc

@@ -743,17 +743,9 @@ DrawSurfaceStatus Rasterizer::DrawToSurfaceUnsafe(
     // when leaf layer tracing is enabled we wish to repaint the whole frame
     // for accurate performance metrics.
     if (frame->framebuffer_info().supports_partial_repaint) {
-      // Disable partial repaint if external_view_embedder_ SubmitFlutterView is
-      // involved - ExternalViewEmbedder unconditionally clears the entire
-      // surface and also partial repaint with platform view present is
-      // something that still need to be figured out.
-      bool force_full_repaint =
-          external_view_embedder_ &&
-          (!raster_thread_merger_ || raster_thread_merger_->IsMerged());
-
       damage = std::make_unique<FrameDamage>();
       auto existing_damage = frame->framebuffer_info().existing_damage;
-      if (existing_damage.has_value() && !force_full_repaint) {
+      if (existing_damage.has_value()) {
         damage->SetPreviousLayerTree(GetLastLayerTree(view_id));
         damage->AddAdditionalDamage(existing_damage.value());
         damage->SetClipAlignment(