Revert "Reland: Partial repaint platform views" (#54537)

Reverts #54231

Speculatively reverting for
flutter/flutter#153335

Author: zanderso

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_volatile_layer = false;
+  state_.has_texture = false;
   state_.integral_transform = false;
 
   if (had_integral_transform) {
@@ -203,20 +203,14 @@ void DiffContext::AddLayerBounds(const SkRect& rect) {
   }
 }
 
-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() {
+void DiffContext::MarkSubtreeHasTextureLayer() {
   // 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_volatile_layer = true;
+    state.has_texture = true;
   }
-  state_.has_volatile_layer = true;
+  state_.has_texture = true;
 }
 
 void DiffContext::AddExistingPaintRegion(const PaintRegion& region) {
@@ -244,7 +238,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_volatile_layer);
+                     state_.has_texture);
 }
 
 void DiffContext::AddDamage(const PaintRegion& damage) {

flow/diff_context.h

@@ -107,18 +107,14 @@ class DiffContext {
 
   bool IsSubtreeDirty() const { return state_.dirty; }
 
-  // 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();
+  // 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();
 
   // 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.
@@ -235,7 +231,7 @@ class DiffContext {
     bool has_filter_bounds_adjustment = false;
 
     // Whether there is a texture layer in this subtree.
-    bool has_volatile_layer = false;
+    bool has_texture = 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_volatile_layer()) {
+          !paint_region.has_texture()) {
         // 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,17 +13,6 @@ 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,7 +16,6 @@ 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,7 +6,6 @@
 #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"
@@ -140,45 +139,5 @@ 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->MarkSubtreeHasVolatileLayer();
+  context->MarkSubtreeHasTextureLayer();
   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_volatile_layer)
+              bool has_texture)
       : rects_(std::move(rects)),
         from_(from),
         to_(to),
         has_readback_(has_readback),
-        has_volatile_layer_(has_volatile_layer) {}
+        has_texture_(has_texture) {}
 
   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 or PlatformViewLayer in subtree
-  // represented by this region.
-  bool has_volatile_layer() const { return has_volatile_layer_; }
+  // Returns whether there is a TextureLayer in subtree represented by this
+  // region.
+  bool has_texture() const { return has_texture_; }
 
  private:
   std::shared_ptr<std::vector<SkRect>> rects_;
   size_t from_ = 0;
   size_t to_ = 0;
   bool has_readback_ = false;
-  bool has_volatile_layer_ = false;
+  bool has_texture_ = false;
 };
 
 }  // namespace flutter

shell/common/rasterizer.cc

@@ -743,9 +743,17 @@ 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()) {
+      if (existing_damage.has_value() && !force_full_repaint) {
         damage->SetPreviousLayerTree(GetLastLayerTree(view_id));
         damage->AddAdditionalDamage(existing_damage.value());
         damage->SetClipAlignment(