Use DisplayListMatrixClipTracker in DiffContext

display_list/display_list_matrix_clip_tracker.h

@@ -23,7 +23,7 @@ class DisplayListMatrixClipTracker {
   DisplayListMatrixClipTracker(const SkRect& cull_rect, const SkMatrix& matrix);
   DisplayListMatrixClipTracker(const SkRect& cull_rect, const SkM44& matrix);
 
-  bool using_4x4_matrix() { return current_->is_4x4(); }
+  bool using_4x4_matrix() const { return current_->is_4x4(); }
 
   SkM44 matrix_4x4() const { return current_->matrix_4x4(); }
   SkMatrix matrix_3x3() const { return current_->matrix_3x3(); }

flow/diff_context.cc

@@ -12,7 +12,8 @@ DiffContext::DiffContext(SkISize frame_size,
                          PaintRegionMap& this_frame_paint_region_map,
                          const PaintRegionMap& last_frame_paint_region_map,
                          bool has_raster_cache)
-    : rects_(std::make_shared<std::vector<SkRect>>()),
+    : clip_tracker_(DisplayListMatrixClipTracker(kGiantRect, SkMatrix::I())),
+      rects_(std::make_shared<std::vector<SkRect>>()),
       frame_size_(frame_size),
       frame_device_pixel_ratio_(frame_device_pixel_ratio),
       this_frame_paint_region_map_(this_frame_paint_region_map),
@@ -21,12 +22,18 @@ DiffContext::DiffContext(SkISize frame_size,
 
 void DiffContext::BeginSubtree() {
   state_stack_.push_back(state_);
-  state_.rect_index_ = rects_->size();
+
+  bool had_integral_transform = state_.integral_transform;
+  state_.rect_index = rects_->size();
   state_.has_filter_bounds_adjustment = false;
   state_.has_texture = false;
-  if (state_.transform_override) {
-    state_.transform = *state_.transform_override;
-    state_.transform_override = std::nullopt;
+  state_.integral_transform = false;
+
+  state_.clip_tracker_save_count = clip_tracker_.getSaveCount();
+  clip_tracker_.save();
+
+  if (had_integral_transform) {
+    MakeCurrentTransformIntegral();
   }
 }
 
@@ -35,23 +42,33 @@ void DiffContext::EndSubtree() {
   if (state_.has_filter_bounds_adjustment) {
     filter_bounds_adjustment_stack_.pop_back();
   }
+  clip_tracker_.restoreToCount(state_.clip_tracker_save_count);
   state_ = state_stack_.back();
   state_stack_.pop_back();
 }
 
 DiffContext::State::State()
     : dirty(false),
-      cull_rect(kGiantRect),
-      rect_index_(0),
+      rect_index(0),
+      integral_transform(false),
+      clip_tracker_save_count(0),
       has_filter_bounds_adjustment(false),
       has_texture(false) {}
 
 void DiffContext::PushTransform(const SkMatrix& transform) {
-  state_.transform.preConcat(transform);
+  clip_tracker_.transform(transform);
 }
 
-void DiffContext::SetTransform(const SkMatrix& transform) {
-  state_.transform_override = transform;
+void DiffContext::MakeCurrentTransformIntegral() {
+  // TODO(knopp): This is duplicated from LayerStack. Maybe should be part of
+  // clip tracker?
+  if (clip_tracker_.using_4x4_matrix()) {
+    clip_tracker_.setTransform(
+        RasterCacheUtil::GetIntegralTransCTM(clip_tracker_.matrix_4x4()));
+  } else {
+    clip_tracker_.setTransform(
+        RasterCacheUtil::GetIntegralTransCTM(clip_tracker_.matrix_3x3()));
+  }
 }
 
 void DiffContext::PushFilterBoundsAdjustment(
@@ -128,21 +145,23 @@ Damage DiffContext::ComputeDamage(const SkIRect& accumulated_buffer_damage,
   return res;
 }
 
+SkRect DiffContext::MapRect(const SkRect& rect) {
+  SkRect mapped_rect(rect);
+  clip_tracker_.mapRect(&mapped_rect);
+  return mapped_rect;
+}
+
 bool DiffContext::PushCullRect(const SkRect& clip) {
-  SkRect cull_rect = state_.transform.mapRect(clip);
-  return state_.cull_rect.intersect(cull_rect);
+  clip_tracker_.clipRect(clip, SkClipOp::kIntersect, false);
+  return !clip_tracker_.device_cull_rect().isEmpty();
+}
+
+SkMatrix DiffContext::GetTransform3x3() const {
+  return clip_tracker_.matrix_3x3();
 }
 
 SkRect DiffContext::GetCullRect() const {
-  SkMatrix inverse_transform;
-  // Perspective projections don't produce rectangles that are useful for
-  // culling for some reason.
-  if (!state_.transform.hasPerspective() &&
-      state_.transform.invert(&inverse_transform)) {
-    return inverse_transform.mapRect(state_.cull_rect);
-  } else {
-    return kGiantRect;
-  }
+  return clip_tracker_.local_cull_rect();
 }
 
 void DiffContext::MarkSubtreeDirty(const PaintRegion& previous_paint_region) {
@@ -163,16 +182,17 @@ void DiffContext::AddLayerBounds(const SkRect& rect) {
   // During painting we cull based on non-overriden transform and then
   // override the transform right before paint. Do the same thing here to get
   // identical paint rect.
-  auto transformed_rect =
-      ApplyFilterBoundsAdjustment(state_.transform.mapRect(rect));
-  if (transformed_rect.intersects(state_.cull_rect)) {
-    auto paint_rect = state_.transform_override
-                          ? ApplyFilterBoundsAdjustment(
-                                state_.transform_override->mapRect(rect))
-                          : transformed_rect;
-    rects_->push_back(paint_rect);
+  auto transformed_rect = ApplyFilterBoundsAdjustment(MapRect(rect));
+  if (transformed_rect.intersects(clip_tracker_.device_cull_rect())) {
+    if (state_.integral_transform) {
+      clip_tracker_.save();
+      MakeCurrentTransformIntegral();
+      transformed_rect = ApplyFilterBoundsAdjustment(MapRect(rect));
+      clip_tracker_.restore();
+    }
+    rects_->push_back(transformed_rect);
     if (IsSubtreeDirty()) {
-      AddDamage(paint_rect);
+      AddDamage(transformed_rect);
     }
   }
 }
@@ -208,8 +228,8 @@ void DiffContext::AddReadbackRegion(const SkIRect& rect) {
 PaintRegion DiffContext::CurrentSubtreeRegion() const {
   bool has_readback = std::any_of(
       readbacks_.begin(), readbacks_.end(),
-      [&](const Readback& r) { return r.position >= state_.rect_index_; });
-  return PaintRegion(rects_, state_.rect_index_, rects_->size(), has_readback,
+      [&](const Readback& r) { return r.position >= state_.rect_index; });
+  return PaintRegion(rects_, state_.rect_index, rects_->size(), has_readback,
                      state_.has_texture);
 }
 

flow/diff_context.h

@@ -9,8 +9,8 @@
 #include <map>
 #include <optional>
 #include <vector>
+#include "display_list/display_list_matrix_clip_tracker.h"
 #include "flutter/flow/paint_region.h"
-#include "flutter/fml/logging.h"
 #include "flutter/fml/macros.h"
 #include "third_party/skia/include/core/SkMatrix.h"
 #include "third_party/skia/include/core/SkRect.h"
@@ -83,16 +83,16 @@ class DiffContext {
   // subtree will have bounds adjusted by this function.
   void PushFilterBoundsAdjustment(const FilterBoundsAdjustment& filter);
 
-  // Returns transform matrix for current subtree
-  const SkMatrix& GetTransform() const { return state_.transform; }
+  // Instruct DiffContext that current layer will paint with integral transform.
+  void WillPaintWithIntegralTransform() { state_.integral_transform = true; }
 
-  // Overrides transform matrix for current subtree
-  void SetTransform(const SkMatrix& transform);
+  // Returns current transform as SkMatrix.
+  SkMatrix GetTransform3x3() const;
 
-  // Return cull rect for current subtree (in local coordinates)
+  // Return cull rect for current subtree (in local coordinates).
   SkRect GetCullRect() const;
 
-  // Sets the dirty flag on current subtree;
+  // Sets the dirty flag on current subtree.
   //
   // previous_paint_region, which should represent region of previous subtree
   // at this level will be added to damage area.
@@ -196,24 +196,29 @@ class DiffContext {
 
   Statistics& statistics() { return statistics_; }
 
+  SkRect MapRect(const SkRect& rect);
+
  private:
   struct State {
     State();
 
     bool dirty;
-    SkRect cull_rect;  // in screen coordinates
-
-    // In order to replicate paint process closely, we need both the original
-    // transform, and the overriden transform (set for layers that need to paint
-    // on integer coordinates). The reason for this is that during paint the
-    // transform matrix is overriden only after layer passes the cull check
-    // first (with original transform). So to cull layer we use transform, but
-    // to get paint coordinates we use transform_override. Child layers are
-    // painted after transform override, so if set we use transform_override as
-    // base when diffing child layers.
-    SkMatrix transform;
-    std::optional<SkMatrix> transform_override;
-    size_t rect_index_;
+
+    size_t rect_index;
+
+    // In order to replicate paint process closely, DiffContext needs to take
+    // into account that some layers are painted with transform translation
+    // snapped to integral coordinates.
+    //
+    // It's not possible to simply snap the transform itself, because culling
+    // needs to happen with original (unsnapped) transform, just like it does
+    // during paint. This means the integral coordinates must be applied after
+    // culling before painting the layer content (either the layer itself, or
+    // when starting subtree to paint layer children).
+    bool integral_transform;
+
+    // Used to restoring clip tracker when popping state.
+    int clip_tracker_save_count;
 
     // Whether this subtree has filter bounds adjustment function. If so,
     // it will need to be removed from stack when subtree is closed.
@@ -223,6 +228,9 @@ class DiffContext {
     bool has_texture;
   };
 
+  void MakeCurrentTransformIntegral();
+
+  DisplayListMatrixClipTracker clip_tracker_;
   std::shared_ptr<std::vector<SkRect>> rects_;
   State state_;
   SkISize frame_size_;

flow/layers/backdrop_filter_layer.cc

@@ -26,11 +26,11 @@ void BackdropFilterLayer::Diff(DiffContext* context, const Layer* old_layer) {
   context->AddLayerBounds(paint_bounds);
 
   if (filter_) {
-    context->GetTransform().mapRect(&paint_bounds);
+    paint_bounds = context->MapRect(paint_bounds);
     auto filter_target_bounds = paint_bounds.roundOut();
     SkIRect filter_input_bounds;  // in screen coordinates
     filter_->get_input_device_bounds(
-        filter_target_bounds, context->GetTransform(), filter_input_bounds);
+        filter_target_bounds, context->GetTransform3x3(), filter_input_bounds);
     context->AddReadbackRegion(filter_input_bounds);
   }
 

flow/layers/backdrop_filter_layer_unittests.cc

@@ -383,6 +383,7 @@ TEST_F(BackdropLayerDiffTest, BackdropLayer) {
   auto path1 = SkPath().addRect(SkRect::MakeLTRB(180, 180, 190, 190));
   l4.root()->Add(std::make_shared<MockLayer>(path1));
   damage = DiffLayerTree(l4, l3);
+
   EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(180, 180, 190, 190));
 
   MockLayerTree l5;

flow/layers/clip_shape_layer.h

@@ -33,8 +33,7 @@ class ClipShapeLayer : public CacheableContainerLayer {
       }
     }
     if (UsesSaveLayer() && context->has_raster_cache()) {
-      context->SetTransform(
-          RasterCacheUtil::GetIntegralTransCTM(context->GetTransform()));
+      context->WillPaintWithIntegralTransform();
     }
     if (context->PushCullRect(clip_shape_bounds())) {
       DiffChildren(context, prev);

flow/layers/color_filter_layer.cc

@@ -28,8 +28,7 @@ void ColorFilterLayer::Diff(DiffContext* context, const Layer* old_layer) {
   }
 
   if (context->has_raster_cache()) {
-    context->SetTransform(
-        RasterCacheUtil::GetIntegralTransCTM(context->GetTransform()));
+    context->WillPaintWithIntegralTransform();
   }
 
   DiffChildren(context, prev);

flow/layers/display_list_layer.cc

@@ -53,8 +53,7 @@ void DisplayListLayer::Diff(DiffContext* context, const Layer* old_layer) {
   }
   context->PushTransform(SkMatrix::Translate(offset_.x(), offset_.y()));
   if (context->has_raster_cache()) {
-    context->SetTransform(
-        RasterCacheUtil::GetIntegralTransCTM(context->GetTransform()));
+    context->WillPaintWithIntegralTransform();
   }
   context->AddLayerBounds(display_list()->bounds());
   context->SetLayerPaintRegion(this, context->CurrentSubtreeRegion());

flow/layers/image_filter_layer.cc

@@ -29,12 +29,11 @@ void ImageFilterLayer::Diff(DiffContext* context, const Layer* old_layer) {
 
   context->PushTransform(SkMatrix::Translate(offset_.fX, offset_.fY));
   if (context->has_raster_cache()) {
-    context->SetTransform(
-        RasterCacheUtil::GetIntegralTransCTM(context->GetTransform()));
+    context->WillPaintWithIntegralTransform();
   }
 
   if (filter_) {
-    auto filter = filter_->makeWithLocalMatrix(context->GetTransform());
+    auto filter = filter_->makeWithLocalMatrix(context->GetTransform3x3());
     if (filter) {
       // This transform will be applied to every child rect in the subtree
       context->PushFilterBoundsAdjustment([filter](SkRect rect) {

flow/layers/opacity_layer.cc

@@ -29,8 +29,7 @@ void OpacityLayer::Diff(DiffContext* context, const Layer* old_layer) {
   }
   context->PushTransform(SkMatrix::Translate(offset_.fX, offset_.fY));
   if (context->has_raster_cache()) {
-    context->SetTransform(
-        RasterCacheUtil::GetIntegralTransCTM(context->GetTransform()));
+    context->WillPaintWithIntegralTransform();
   }
   DiffChildren(context, prev);
   context->SetLayerPaintRegion(this, context->CurrentSubtreeRegion());

flow/layers/physical_shape_layer.cc

@@ -38,7 +38,7 @@ void PhysicalShapeLayer::Diff(DiffContext* context, const Layer* old_layer) {
   } else {
     bounds = DisplayListCanvasDispatcher::ComputeShadowBounds(
         path_, elevation_, context->frame_device_pixel_ratio(),
-        context->GetTransform());
+        context->GetTransform3x3());
   }
 
   context->AddLayerBounds(bounds);

flow/layers/shader_mask_layer.cc

@@ -27,8 +27,7 @@ void ShaderMaskLayer::Diff(DiffContext* context, const Layer* old_layer) {
     }
   }
   if (context->has_raster_cache()) {
-    context->SetTransform(
-        RasterCacheUtil::GetIntegralTransCTM(context->GetTransform()));
+    context->WillPaintWithIntegralTransform();
   }
   DiffChildren(context, prev);