Avoid unnecessary transform resets for pixel snapping

flow/diff_context.cc

@@ -61,11 +61,17 @@ 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()));
+    SkM44 integral;
+    if (RasterCacheUtil::ComputeIntegralTransCTM(clip_tracker_.matrix_4x4(),
+                                                 &integral)) {
+      clip_tracker_.setTransform(integral);
+    }
   } else {
-    clip_tracker_.setTransform(
-        RasterCacheUtil::GetIntegralTransCTM(clip_tracker_.matrix_3x3()));
+    SkMatrix integral;
+    if (RasterCacheUtil::ComputeIntegralTransCTM(clip_tracker_.matrix_3x3(),
+                                                 &integral)) {
+      clip_tracker_.setTransform(integral);
+    }
   }
 }
 

flow/layers/display_list_layer_unittests.cc

@@ -249,7 +249,7 @@ TEST_F(DisplayListLayerTest, CachedIncompatibleDisplayListOpacityInheritance) {
                               &paint_context(), false);
 
   int opacity_alpha = 0x7F;
-  SkPoint opacity_offset = SkPoint::Make(10, 10);
+  SkPoint opacity_offset = SkPoint::Make(10.2, 10.2);
   auto opacity_layer =
       std::make_shared<OpacityLayer>(opacity_alpha, opacity_offset);
   opacity_layer->Add(display_list_layer);

flow/layers/image_filter_layer_unittests.cc

@@ -454,9 +454,8 @@ TEST_F(ImageFilterLayerTest, CacheChildren) {
     expected_builder.Save();
     {
       expected_builder.Translate(offset.fX, offset.fY);
-      // snap translation components to pixels due to using raster cache
-      expected_builder.TransformReset();
-      expected_builder.Transform(snapped_matrix);
+      // translation components already snapped to pixels, intent to
+      // use raster cache won't change them
       DlPaint dl_paint;
       dl_paint.setImageFilter(transformed_filter.get());
       raster_cache()->Draw(cacheable_image_filter_item->GetId().value(),

flow/layers/layer_state_stack.cc

@@ -114,8 +114,10 @@ class DlCanvasDelegate : public LayerStateStack::Delegate {
     canvas_->Transform(matrix);
   }
   void integralTransform() override {
-    SkM44 matrix = RasterCacheUtil::GetIntegralTransCTM(matrix_4x4());
-    canvas_->SetTransform(matrix);
+    SkM44 integral;
+    if (RasterCacheUtil::ComputeIntegralTransCTM(matrix_4x4(), &integral)) {
+      canvas_->SetTransform(integral);
+    }
   }
 
   void clipRect(const SkRect& rect, ClipOp op, bool is_aa) override {
@@ -168,11 +170,17 @@ class PrerollDelegate : public LayerStateStack::Delegate {
   }
   void integralTransform() override {
     if (tracker_.using_4x4_matrix()) {
-      tracker_.setTransform(
-          RasterCacheUtil::GetIntegralTransCTM(tracker_.matrix_4x4()));
+      SkM44 integral;
+      if (RasterCacheUtil::ComputeIntegralTransCTM(tracker_.matrix_4x4(),
+                                                   &integral)) {
+        tracker_.setTransform(integral);
+      }
     } else {
-      tracker_.setTransform(
-          RasterCacheUtil::GetIntegralTransCTM(tracker_.matrix_3x3()));
+      SkMatrix integral;
+      if (RasterCacheUtil::ComputeIntegralTransCTM(tracker_.matrix_3x3(),
+                                                   &integral)) {
+        tracker_.setTransform(integral);
+      }
     }
   }
 

flow/layers/shader_mask_layer_unittests.cc

@@ -443,10 +443,10 @@ TEST_F(ShaderMaskLayerTest, SimpleFilterWithRasterCacheLayerNotCached) {
   /* (ShaderMask)layer::Paint */ {
     expected_builder.Save();
     {
-      expected_builder.TransformReset();
-      // The layer will perform this Identity transform operation by default,
-      // but it should be ignored both here and in the layer paint
-      expected_builder.Transform(SkMatrix());
+      // The layer will notice that the CTM is already an integer matrix
+      // and will not perform an Integral CTM operation.
+      // expected_builder.TransformReset();
+      // expected_builder.Transform(SkMatrix());
       expected_builder.SaveLayer(&child_bounds);
       {
         /* mock_layer::Paint */ {

flow/raster_cache_unittests.cc

@@ -809,6 +809,281 @@ TEST_F(RasterCacheTest, RasterCacheKeyIDLayerChildrenIds) {
   ASSERT_EQ(ids, expected_ids);
 }
 
+TEST(RasterCacheUtilsTest, SkMatrixIntegralTransCTM) {
+#define EXPECT_EQ_WITH_TRANSLATE(test, expected, expected_tx, expected_ty) \
+  do {                                                                     \
+    EXPECT_EQ(test[SkMatrix::kMScaleX], expected[SkMatrix::kMScaleX]);     \
+    EXPECT_EQ(test[SkMatrix::kMSkewX], expected[SkMatrix::kMSkewX]);       \
+    EXPECT_EQ(test[SkMatrix::kMScaleY], expected[SkMatrix::kMScaleY]);     \
+    EXPECT_EQ(test[SkMatrix::kMSkewY], expected[SkMatrix::kMSkewY]);       \
+    EXPECT_EQ(test[SkMatrix::kMSkewX], expected[SkMatrix::kMSkewX]);       \
+    EXPECT_EQ(test[SkMatrix::kMPersp0], expected[SkMatrix::kMPersp0]);     \
+    EXPECT_EQ(test[SkMatrix::kMPersp1], expected[SkMatrix::kMPersp1]);     \
+    EXPECT_EQ(test[SkMatrix::kMPersp2], expected[SkMatrix::kMPersp2]);     \
+    EXPECT_EQ(test[SkMatrix::kMTransX], expected_tx);                      \
+    EXPECT_EQ(test[SkMatrix::kMTransY], expected_ty);                      \
+  } while (0)
+
+#define EXPECT_NON_INTEGER_TRANSLATION(matrix)                        \
+  EXPECT_TRUE(SkScalarFraction(matrix[SkMatrix::kMTransX]) != 0.0f || \
+              SkScalarFraction(matrix[SkMatrix::kMTransY]) != 0.0f)
+
+  {
+    // Identity
+    SkMatrix matrix = SkMatrix::I();
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ(get, matrix);
+  }
+  {
+    // Integer translate
+    SkMatrix matrix = SkMatrix::Translate(10.0f, 12.0f);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ(get, matrix);
+  }
+  {
+    // Fractional x translate
+    SkMatrix matrix = SkMatrix::Translate(10.2f, 12.0f);
+    EXPECT_NON_INTEGER_TRANSLATION(matrix);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_TRUE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ_WITH_TRANSLATE(get, matrix, 10.0f, 12.0f);
+    EXPECT_EQ(get, compute);
+  }
+  {
+    // Fractional y translate
+    SkMatrix matrix = SkMatrix::Translate(10.0f, 12.3f);
+    EXPECT_NON_INTEGER_TRANSLATION(matrix);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_TRUE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ_WITH_TRANSLATE(get, matrix, 10.0f, 12.0f);
+    EXPECT_EQ(get, compute);
+  }
+  {
+    // Fractional x & y translate
+    SkMatrix matrix = SkMatrix::Translate(10.7f, 12.3f);
+    EXPECT_NON_INTEGER_TRANSLATION(matrix);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_TRUE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ_WITH_TRANSLATE(get, matrix, 11.0f, 12.0f);
+    EXPECT_EQ(get, compute);
+  }
+  {
+    // Scale
+    SkMatrix matrix = SkMatrix::Scale(2.0f, 3.0f);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ(get, matrix);
+  }
+  {
+    // Scale, Integer translate
+    SkMatrix matrix = SkMatrix::Scale(2.0f, 3.0f);
+    matrix.preTranslate(10.0f, 12.0f);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ(get, matrix);
+  }
+  {
+    // Scale, Fractional translate
+    SkMatrix matrix = SkMatrix::Scale(2.0f, 3.0f);
+    matrix.preTranslate(10.7f, 12.1f);
+    EXPECT_NON_INTEGER_TRANSLATION(matrix);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_TRUE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ_WITH_TRANSLATE(get, matrix, 21.0f, 36.0f);
+    EXPECT_EQ(get, compute);
+  }
+  {
+    // Skew
+    SkMatrix matrix = SkMatrix::Skew(0.5f, 0.1f);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ(get, matrix);
+  }
+  {
+    // Skew, Fractional translate - should be NOP
+    SkMatrix matrix = SkMatrix::Skew(0.5f, 0.1f);
+    matrix.preTranslate(10.7f, 12.1f);
+    EXPECT_NON_INTEGER_TRANSLATION(matrix);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ(get, matrix);
+  }
+  {
+    // Rotate
+    SkMatrix matrix = SkMatrix::RotateDeg(45);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ(get, matrix);
+  }
+  {
+    // Rotate, Fractional Translate - should be NOP
+    SkMatrix matrix = SkMatrix::RotateDeg(45);
+    matrix.preTranslate(10.7f, 12.1f);
+    EXPECT_NON_INTEGER_TRANSLATION(matrix);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ(get, matrix);
+  }
+  {
+    // Perspective x
+    SkMatrix matrix = SkMatrix::I();
+    matrix.setPerspX(0.1);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ(get, matrix);
+  }
+  {
+    // Perspective x, Fractional Translate - should be NOP
+    SkMatrix matrix = SkMatrix::I();
+    matrix.setPerspX(0.1);
+    matrix.preTranslate(10.7f, 12.1f);
+    EXPECT_NON_INTEGER_TRANSLATION(matrix);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ(get, matrix);
+  }
+  {
+    // Perspective y
+    SkMatrix matrix = SkMatrix::I();
+    matrix.setPerspY(0.1);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ(get, matrix);
+  }
+  {
+    // Perspective y, Fractional Translate - should be NOP
+    SkMatrix matrix = SkMatrix::I();
+    matrix.setPerspY(0.1);
+    matrix.preTranslate(10.7f, 12.1f);
+    EXPECT_NON_INTEGER_TRANSLATION(matrix);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ(get, matrix);
+  }
+  {
+    // Perspective weight
+    // clang-format off
+    SkMatrix matrix = SkMatrix::MakeAll(
+        1.0f, 0.0f, 0.0f,
+        0.0f, 1.0f, 0.0f,
+        0.0f, 0.0f, 0.9f);
+    // clang-format on
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ(get, matrix);
+  }
+  {
+    // Perspective weight, Fractional Translate - should be NOP
+    // clang-format off
+    SkMatrix matrix = SkMatrix::MakeAll(
+        1.0f, 0.0f, 0.0f,
+        0.0f, 1.0f, 0.0f,
+        0.0f, 0.0f, 0.9f);
+    // clang-format on
+    matrix.preTranslate(10.7f, 12.1f);
+    EXPECT_NON_INTEGER_TRANSLATION(matrix);
+    SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+    SkMatrix compute;
+    EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
+    EXPECT_EQ(get, matrix);
+  }
+#undef EXPECT_NON_INTEGER_TRANSLATION
+#undef EXPECT_EQ_WITH_TRANSLATE
+}
+
+TEST(RasterCacheUtilsTest, SkM44IntegralTransCTM) {
+#define EXPECT_EQ_WITH_TRANSLATE(test, expected, tx, ty, label) \
+  do {                                                          \
+    EXPECT_EQ(test.rc(0, 0), expected.rc(0, 0)) << label;       \
+    EXPECT_EQ(test.rc(0, 1), expected.rc(0, 1)) << label;       \
+    EXPECT_EQ(test.rc(0, 2), expected.rc(0, 2)) << label;       \
+    EXPECT_EQ(test.rc(0, 3), tx) << label;                      \
+    EXPECT_EQ(test.rc(1, 0), expected.rc(1, 0)) << label;       \
+    EXPECT_EQ(test.rc(1, 1), expected.rc(1, 1)) << label;       \
+    EXPECT_EQ(test.rc(1, 2), expected.rc(1, 2)) << label;       \
+    EXPECT_EQ(test.rc(1, 3), ty) << label;                      \
+    EXPECT_EQ(test.rc(2, 0), expected.rc(2, 0)) << label;       \
+    EXPECT_EQ(test.rc(2, 1), expected.rc(2, 1)) << label;       \
+    EXPECT_EQ(test.rc(2, 2), expected.rc(2, 2)) << label;       \
+    EXPECT_EQ(test.rc(2, 3), expected.rc(2, 3)) << label;       \
+    EXPECT_EQ(test.rc(3, 0), expected.rc(3, 0)) << label;       \
+    EXPECT_EQ(test.rc(3, 1), expected.rc(3, 1)) << label;       \
+    EXPECT_EQ(test.rc(3, 2), expected.rc(3, 2)) << label;       \
+    EXPECT_EQ(test.rc(3, 3), expected.rc(3, 3)) << label;       \
+  } while (0)
+
+#define EXPECT_NON_INTEGER_TRANSLATION(matrix)             \
+  EXPECT_TRUE(SkScalarFraction(matrix.rc(0, 3)) != 0.0f || \
+              SkScalarFraction(matrix.rc(1, 3)) != 0.0f)
+
+  for (int r = 0; r < 4; r++) {
+    for (int c = 0; c < 4; c++) {
+      bool snaps;
+      switch (r) {
+        case 0:  // X equation
+          if (c == 3) {
+            continue;  // TranslateX, the value we are testing, skip
+          }
+          snaps = (c == 0);  // X Scale value yes, Skew by Y or Z no
+          break;
+        case 1:  // Y equation
+          if (c == 3) {
+            continue;  // TranslateY, the value we are testing, skip
+          }
+          snaps = (c == 1);  // Y Scale value yes, Skew by X or Z no
+          break;
+        case 2:  // Z equation, ignored, will snap
+          snaps = true;
+          break;
+        case 3:  // W equation, modifications prevent snapping
+          snaps = false;
+          break;
+        default:
+          FML_UNREACHABLE();
+      }
+      auto label = std::to_string(r) + ", " + std::to_string(c);
+      SkM44 matrix = SkM44::Translate(10.7f, 12.1f);
+      EXPECT_NON_INTEGER_TRANSLATION(matrix) << label;
+      matrix.setRC(r, c, 0.5f);
+      if (snaps) {
+        SkM44 compute;
+        SkM44 get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+        EXPECT_TRUE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute))
+            << label;
+        EXPECT_EQ_WITH_TRANSLATE(get, matrix, 11.0f, 12.0f, label);
+        EXPECT_EQ(get, compute) << label;
+      } else {
+        SkM44 compute;
+        SkM44 get = RasterCacheUtil::GetIntegralTransCTM(matrix);
+        EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute))
+            << label;
+        EXPECT_EQ(get, matrix) << label;
+      }
+    }
+  }
+#undef EXPECT_NON_INTEGER_TRANSLATION
+#undef EXPECT_EQ_WITH_TRANSLATE
+}
+
 }  // namespace testing
 }  // namespace flutter