Specify clip alignment for partial repaint (#31359)

* Specify clip alignment for partial repaint

ci/licenses_golden/licenses_flutter

@@ -102,6 +102,7 @@ FILE: ../../../flutter/flow/compositor_context.cc
 FILE: ../../../flutter/flow/compositor_context.h
 FILE: ../../../flutter/flow/diff_context.cc
 FILE: ../../../flutter/flow/diff_context.h
+FILE: ../../../flutter/flow/diff_context_unittests.cc
 FILE: ../../../flutter/flow/embedded_view_params_unittests.cc
 FILE: ../../../flutter/flow/embedded_views.cc
 FILE: ../../../flutter/flow/embedded_views.h

flow/BUILD.gn

@@ -128,6 +128,7 @@ if (enable_unittests) {
     testonly = true
 
     sources = [
+      "diff_context_unittests.cc",
       "embedded_view_params_unittests.cc",
       "flow_run_all_unittests.cc",
       "flow_test_utils.cc",

flow/compositor_context.cc

@@ -36,7 +36,9 @@ std::optional<SkRect> FrameDamage::ComputeClipRect(
       layer_tree.root_layer()->Diff(&context, prev_root_layer);
     }
 
-    damage_ = context.ComputeDamage(additional_damage_);
+    damage_ =
+        context.ComputeDamage(additional_damage_, horizontal_clip_alignment_,
+                              vertical_clip_alignment_);
     return SkRect::Make(damage_->buffer_damage);
   } else {
     return std::nullopt;

flow/compositor_context.h

@@ -69,6 +69,12 @@ class FrameDamage {
     additional_damage_.join(damage);
   }
 
+  // Specifies clip rect alignment.
+  void SetClipAlignment(int horizontal, int vertical) {
+    horizontal_clip_alignment_ = horizontal;
+    vertical_clip_alignment_ = vertical;
+  }
+
   // Calculates clip rect for current rasterization. This is diff of layer tree
   // and previous layer tree + any additional provideddamage.
   // If previous layer tree is not specified, clip rect will be nulloptional,
@@ -90,6 +96,8 @@ class FrameDamage {
   SkIRect additional_damage_ = SkIRect::MakeEmpty();
   std::optional<Damage> damage_;
   const LayerTree* prev_layer_tree_ = nullptr;
+  int vertical_clip_alignment_ = 1;
+  int horizontal_clip_alignment_ = 1;
 };
 
 class CompositorContext {

flow/diff_context.cc

@@ -67,8 +67,33 @@ SkRect DiffContext::ApplyFilterBoundsAdjustment(SkRect rect) const {
   return rect;
 }
 
-Damage DiffContext::ComputeDamage(
-    const SkIRect& accumulated_buffer_damage) const {
+void DiffContext::AlignRect(SkIRect& rect,
+                            int horizontal_alignment,
+                            int vertical_alignment) const {
+  auto top = rect.top();
+  auto left = rect.left();
+  auto right = rect.right();
+  auto bottom = rect.bottom();
+  if (top % vertical_alignment != 0) {
+    top -= top % vertical_alignment;
+  }
+  if (left % horizontal_alignment != 0) {
+    left -= left % horizontal_alignment;
+  }
+  if (right % horizontal_alignment != 0) {
+    right += horizontal_alignment - right % horizontal_alignment;
+  }
+  if (bottom % vertical_alignment != 0) {
+    bottom += vertical_alignment - bottom % vertical_alignment;
+  }
+  right = std::min(right, frame_size_.width());
+  bottom = std::min(bottom, frame_size_.height());
+  rect = SkIRect::MakeLTRB(left, top, right, bottom);
+}
+
+Damage DiffContext::ComputeDamage(const SkIRect& accumulated_buffer_damage,
+                                  int horizontal_clip_alignment,
+                                  int vertical_clip_alignment) const {
   SkRect buffer_damage = SkRect::Make(accumulated_buffer_damage);
   buffer_damage.join(damage_);
   SkRect frame_damage(damage_);
@@ -90,6 +115,13 @@ Damage DiffContext::ComputeDamage(
   SkIRect frame_clip = SkIRect::MakeSize(frame_size_);
   res.buffer_damage.intersect(frame_clip);
   res.frame_damage.intersect(frame_clip);
+
+  if (horizontal_clip_alignment > 1 || vertical_clip_alignment > 1) {
+    AlignRect(res.buffer_damage, horizontal_clip_alignment,
+              vertical_clip_alignment);
+    AlignRect(res.frame_damage, horizontal_clip_alignment,
+              vertical_clip_alignment);
+  }
   return res;
 }
 

flow/diff_context.h

@@ -133,7 +133,12 @@ class DiffContext {
   //
   // additional_damage is the previously accumulated frame_damage for
   // current framebuffer
-  Damage ComputeDamage(const SkIRect& additional_damage) const;
+  //
+  // clip_alignment controls the alignment of resulting frame and surface
+  // damage.
+  Damage ComputeDamage(const SkIRect& additional_damage,
+                       int horizontal_clip_alignment = 0,
+                       int vertical_clip_alignment = 0) const;
 
   double frame_device_pixel_ratio() const { return frame_device_pixel_ratio_; };
 
@@ -230,6 +235,10 @@ class DiffContext {
 
   void AddDamage(const SkRect& rect);
 
+  void AlignRect(SkIRect& rect,
+                 int horizontal_alignment,
+                 int vertical_clip_alignment) const;
+
   struct Readback {
     // Index of rects_ entry that this readback belongs to. Used to
     // determine if subtree has any readback

flow/diff_context_unittests.cc

@@ -0,0 +1,36 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/flow/testing/diff_context_test.h"
+
+namespace flutter {
+namespace testing {
+
+TEST_F(DiffContextTest, ClipAlignment) {
+  MockLayerTree t1;
+  t1.root()->Add(
+      CreatePictureLayer(CreatePicture(SkRect::MakeLTRB(30, 30, 50, 50), 1)));
+  auto damage = DiffLayerTree(t1, MockLayerTree(), SkIRect::MakeEmpty(), 0, 0);
+  EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(30, 30, 50, 50));
+  EXPECT_EQ(damage.buffer_damage, SkIRect::MakeLTRB(30, 30, 50, 50));
+
+  damage = DiffLayerTree(t1, MockLayerTree(), SkIRect::MakeEmpty(), 1, 1);
+  EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(30, 30, 50, 50));
+  EXPECT_EQ(damage.buffer_damage, SkIRect::MakeLTRB(30, 30, 50, 50));
+
+  damage = DiffLayerTree(t1, MockLayerTree(), SkIRect::MakeEmpty(), 8, 1);
+  EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(24, 30, 56, 50));
+  EXPECT_EQ(damage.buffer_damage, SkIRect::MakeLTRB(24, 30, 56, 50));
+
+  damage = DiffLayerTree(t1, MockLayerTree(), SkIRect::MakeEmpty(), 1, 8);
+  EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(30, 24, 50, 56));
+  EXPECT_EQ(damage.buffer_damage, SkIRect::MakeLTRB(30, 24, 50, 56));
+
+  damage = DiffLayerTree(t1, MockLayerTree(), SkIRect::MakeEmpty(), 16, 16);
+  EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(16, 16, 64, 64));
+  EXPECT_EQ(damage.buffer_damage, SkIRect::MakeLTRB(16, 16, 64, 64));
+}
+
+}  // namespace testing
+}  // namespace flutter

flow/surface_frame.h

@@ -33,6 +33,12 @@ class SurfaceFrame {
     // this means that the surface will provide valid existing damage.
     bool supports_partial_repaint = false;
 
+    // For some targets it may be beneficial or even required to snap clip
+    // rect to tile grid. I.e. repainting part of a tile may cause performance
+    // degradation if the tile needs to be decompressed first.
+    int vertical_clip_alignment = 1;
+    int horizontal_clip_alignment = 1;
+
     // This is the area of framebuffer that lags behind the front buffer.
     //
     // Correctly providing exiting_damage is necessary for supporting double and

flow/testing/diff_context_test.cc

@@ -15,15 +15,18 @@ DiffContextTest::DiffContextTest()
 
 Damage DiffContextTest::DiffLayerTree(MockLayerTree& layer_tree,
                                       const MockLayerTree& old_layer_tree,
-                                      const SkIRect& additional_damage) {
+                                      const SkIRect& additional_damage,
+                                      int horizontal_clip_alignment,
+                                      int vertical_clip_alignment) {
   FML_CHECK(layer_tree.size() == old_layer_tree.size());
 
   DiffContext dc(layer_tree.size(), 1, layer_tree.paint_region_map(),
                  old_layer_tree.paint_region_map());
   dc.PushCullRect(
       SkRect::MakeIWH(layer_tree.size().width(), layer_tree.size().height()));
   layer_tree.root()->Diff(&dc, old_layer_tree.root());
-  return dc.ComputeDamage(additional_damage);
+  return dc.ComputeDamage(additional_damage, horizontal_clip_alignment,
+                          vertical_clip_alignment);
 }
 
 sk_sp<SkPicture> DiffContextTest::CreatePicture(const SkRect& bounds,

flow/testing/diff_context_test.h

@@ -38,7 +38,9 @@ class DiffContextTest : public ThreadTest {
 
   Damage DiffLayerTree(MockLayerTree& layer_tree,
                        const MockLayerTree& old_layer_tree,
-                       const SkIRect& additional_damage = SkIRect::MakeEmpty());
+                       const SkIRect& additional_damage = SkIRect::MakeEmpty(),
+                       int horizontal_clip_alignment = 0,
+                       int vertical_alignment = 0);
 
   // Create picture consisting of filled rect with given color; Being able
   // to specify different color is useful to test deep comparison of pictures

shell/common/rasterizer.cc

@@ -579,6 +579,9 @@ RasterStatus Rasterizer::DrawToSurfaceUnsafe(
       if (frame->framebuffer_info().existing_damage && !force_full_repaint) {
         damage->SetPreviousLayerTree(last_layer_tree_.get());
         damage->AddAdditonalDamage(*frame->framebuffer_info().existing_damage);
+        damage->SetClipAlignment(
+            frame->framebuffer_info().horizontal_clip_alignment,
+            frame->framebuffer_info().vertical_clip_alignment);
       }
     }
 

shell/platform/android/android_surface_gl.cc

@@ -133,6 +133,13 @@ SurfaceFrame::FramebufferInfo AndroidSurfaceGL::GLContextFramebufferInfo()
   res.supports_readback = true;
   res.supports_partial_repaint = onscreen_surface_->SupportsPartialRepaint();
   res.existing_damage = onscreen_surface_->InitialDamage();
+  // Some devices (Pixel2 XL) needs EGL_KHR_partial_update rect aligned to 4,
+  // otherwise there are glitches
+  // (https://github.com/flutter/flutter/issues/97482#)
+  // Larger alignment might also be beneficial for tile base renderers.
+  res.horizontal_clip_alignment = 32;
+  res.vertical_clip_alignment = 32;
+
   return res;
 }