Specify clip alignment for partial repaint

Author: knopp

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 % horizontal_alignment != 0) {
+    bottom += horizontal_alignment - bottom % horizontal_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 = 1,
+                       int vertical_clip_alignment = 1) 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/surface_frame.h

@@ -32,6 +32,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

shell/common/rasterizer.cc

@@ -570,6 +570,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 be beneficial for tile base renderers.
+  res.horizontal_clip_alignment = 32;
+  res.vertical_clip_alignment = 32;
+
   return res;
 }