Reland again "Remove layer integral offset snapping #17112" (#18160)

This reverts commit a7a25d3b and relands our reland https://github.com/flutter/engine/pull/17915.

Additionally, we fixed the cull rect logic in `OpacityLayer::Preroll` which is  the root cause of https://github.com/flutter/flutter/issues/56298. We've always had that root problem before but it did not trigger performance issues because we were using the OpacityLayer's `paint_bounds`, instead of its child's `paint_bounds` for preparing the layer raster cache. A correct handling of the cull rect should allow us to cull at any level.

It also turns out that our ios32 (iPhone4s) performacne can regress a lot
without snapping. My theory is that although the picture has a
fractional top left corner, many drawing operations inside the picture
have integral coordinations. In older hardwares, keeping those
coordinates integral seems to be performance critical.

To avoid flutter/flutter#41654, the snapping
will still be disabled if the matrix has non-scale-translation
transformations.

flow/layers/container_layer.cc

@@ -78,6 +78,15 @@ void ContainerLayer::PaintChildren(PaintContext& context) const {
   }
 }
 
+void ContainerLayer::TryToPrepareRasterCache(PrerollContext* context,
+                                             Layer* layer,
+                                             const SkMatrix& matrix) {
+  if (!context->has_platform_view && context->raster_cache &&
+      SkRect::Intersects(context->cull_rect, layer->paint_bounds())) {
+    context->raster_cache->Prepare(context, layer, matrix);
+  }
+}
+
 #if defined(OS_FUCHSIA)
 
 void ContainerLayer::CheckForChildLayerBelow(PrerollContext* context) {

flow/layers/container_layer.h

@@ -38,6 +38,20 @@ class ContainerLayer : public Layer {
   // For OpacityLayer to restructure to have a single child.
   void ClearChildren() { layers_.clear(); }
 
+  // Try to prepare the raster cache for a given layer.
+  //
+  // The raster cache would fail if either of the followings is true:
+  // 1. The context has a platform view.
+  // 2. The context does not have a valid raster cache.
+  // 3. The layer's paint bounds does not intersect with the cull rect.
+  //
+  // We make this a static function instead of a member function that directy
+  // uses the "this" pointer as the layer because we sometimes need to raster
+  // cache a child layer and one can't access its child's protected method.
+  static void TryToPrepareRasterCache(PrerollContext* context,
+                                      Layer* layer,
+                                      const SkMatrix& matrix);
+
  private:
   std::vector<std::shared_ptr<Layer>> layers_;
 

flow/layers/image_filter_layer.cc

@@ -28,26 +28,13 @@ void ImageFilterLayer::Preroll(PrerollContext* context,
     set_paint_bounds(child_paint_bounds_);
   }
 
-  if (!context->has_platform_view && context->raster_cache &&
-      SkRect::Intersects(context->cull_rect, paint_bounds())) {
-    SkMatrix ctm = matrix;
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-    ctm = RasterCache::GetIntegralTransCTM(ctm);
-#endif
-    context->raster_cache->Prepare(context, this, ctm);
-  }
+  TryToPrepareRasterCache(context, this, matrix);
 }
 
 void ImageFilterLayer::Paint(PaintContext& context) const {
   TRACE_EVENT0("flutter", "ImageFilterLayer::Paint");
   FML_DCHECK(needs_painting());
 
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-  SkAutoCanvasRestore save(context.leaf_nodes_canvas, true);
-  context.leaf_nodes_canvas->setMatrix(RasterCache::GetIntegralTransCTM(
-      context.leaf_nodes_canvas->getTotalMatrix()));
-#endif
-
   if (context.raster_cache &&
       context.raster_cache->Draw(this, *context.leaf_nodes_canvas)) {
     return;

flow/layers/image_filter_layer_unittests.cc

@@ -60,14 +60,11 @@ TEST_F(ImageFilterLayerTest, EmptyFilter) {
   layer->Paint(paint_context());
   EXPECT_EQ(mock_canvas().draw_calls(),
             std::vector({
-                MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
-                MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}},
                 MockCanvas::DrawCall{
-                    1, MockCanvas::SaveLayerData{child_bounds, filter_paint,
-                                                 nullptr, 2}},
+                    0, MockCanvas::SaveLayerData{child_bounds, filter_paint,
+                                                 nullptr, 1}},
                 MockCanvas::DrawCall{
-                    2, MockCanvas::DrawPathData{child_path, child_paint}},
-                MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
+                    1, MockCanvas::DrawPathData{child_path, child_paint}},
                 MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}},
             }));
 }
@@ -96,14 +93,11 @@ TEST_F(ImageFilterLayerTest, SimpleFilter) {
   layer->Paint(paint_context());
   EXPECT_EQ(mock_canvas().draw_calls(),
             std::vector({
-                MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
-                MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}},
                 MockCanvas::DrawCall{
-                    1, MockCanvas::SaveLayerData{child_bounds, filter_paint,
-                                                 nullptr, 2}},
+                    0, MockCanvas::SaveLayerData{child_bounds, filter_paint,
+                                                 nullptr, 1}},
                 MockCanvas::DrawCall{
-                    2, MockCanvas::DrawPathData{child_path, child_paint}},
-                MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
+                    1, MockCanvas::DrawPathData{child_path, child_paint}},
                 MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}},
             }));
 }
@@ -132,14 +126,11 @@ TEST_F(ImageFilterLayerTest, SimpleFilterBounds) {
   layer->Paint(paint_context());
   EXPECT_EQ(mock_canvas().draw_calls(),
             std::vector({
-                MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
-                MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}},
                 MockCanvas::DrawCall{
-                    1, MockCanvas::SaveLayerData{child_bounds, filter_paint,
-                                                 nullptr, 2}},
+                    0, MockCanvas::SaveLayerData{child_bounds, filter_paint,
+                                                 nullptr, 1}},
                 MockCanvas::DrawCall{
-                    2, MockCanvas::DrawPathData{child_path, child_paint}},
-                MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
+                    1, MockCanvas::DrawPathData{child_path, child_paint}},
                 MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}},
             }));
 }
@@ -177,19 +168,16 @@ TEST_F(ImageFilterLayerTest, MultipleChildren) {
   SkPaint filter_paint;
   filter_paint.setImageFilter(layer_filter);
   layer->Paint(paint_context());
-  EXPECT_EQ(mock_canvas().draw_calls(),
-            std::vector(
-                {MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
-                 MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}},
-                 MockCanvas::DrawCall{
-                     1, MockCanvas::SaveLayerData{children_bounds, filter_paint,
-                                                  nullptr, 2}},
-                 MockCanvas::DrawCall{
-                     2, MockCanvas::DrawPathData{child_path1, child_paint1}},
-                 MockCanvas::DrawCall{
-                     2, MockCanvas::DrawPathData{child_path2, child_paint2}},
-                 MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
-                 MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}}));
+  EXPECT_EQ(
+      mock_canvas().draw_calls(),
+      std::vector({MockCanvas::DrawCall{
+                       0, MockCanvas::SaveLayerData{children_bounds,
+                                                    filter_paint, nullptr, 1}},
+                   MockCanvas::DrawCall{
+                       1, MockCanvas::DrawPathData{child_path1, child_paint1}},
+                   MockCanvas::DrawCall{
+                       1, MockCanvas::DrawPathData{child_path2, child_paint2}},
+                   MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}}));
 }
 
 TEST_F(ImageFilterLayerTest, Nested) {
@@ -237,22 +225,16 @@ TEST_F(ImageFilterLayerTest, Nested) {
   layer1->Paint(paint_context());
   EXPECT_EQ(mock_canvas().draw_calls(),
             std::vector({
-                MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
-                MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}},
                 MockCanvas::DrawCall{
-                    1, MockCanvas::SaveLayerData{children_bounds, filter_paint1,
-                                                 nullptr, 2}},
+                    0, MockCanvas::SaveLayerData{children_bounds, filter_paint1,
+                                                 nullptr, 1}},
                 MockCanvas::DrawCall{
-                    2, MockCanvas::DrawPathData{child_path1, child_paint1}},
-                MockCanvas::DrawCall{2, MockCanvas::SaveData{3}},
-                MockCanvas::DrawCall{3, MockCanvas::SetMatrixData{SkMatrix()}},
+                    1, MockCanvas::DrawPathData{child_path1, child_paint1}},
                 MockCanvas::DrawCall{
-                    3, MockCanvas::SaveLayerData{child_path2.getBounds(),
-                                                 filter_paint2, nullptr, 4}},
+                    1, MockCanvas::SaveLayerData{child_path2.getBounds(),
+                                                 filter_paint2, nullptr, 2}},
                 MockCanvas::DrawCall{
-                    4, MockCanvas::DrawPathData{child_path2, child_paint2}},
-                MockCanvas::DrawCall{4, MockCanvas::RestoreData{3}},
-                MockCanvas::DrawCall{3, MockCanvas::RestoreData{2}},
+                    2, MockCanvas::DrawPathData{child_path2, child_paint2}},
                 MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
                 MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}},
             }));

flow/layers/opacity_layer.cc

@@ -35,6 +35,10 @@ void OpacityLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
   SkMatrix child_matrix = matrix;
   child_matrix.postTranslate(offset_.fX, offset_.fY);
 
+  // Similar to what's done in TransformLayer::Preroll, we have to apply the
+  // reverse transformation to the cull rect to properly cull child layers.
+  context->cull_rect = context->cull_rect.makeOffset(-offset_.fX, -offset_.fY);
+
   context->is_opaque = parent_is_opaque && (alpha_ == SK_AlphaOPAQUE);
   context->mutators_stack.PushTransform(
       SkMatrix::MakeTrans(offset_.fX, offset_.fY));
@@ -48,15 +52,11 @@ void OpacityLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
 
   {
     set_paint_bounds(paint_bounds().makeOffset(offset_.fX, offset_.fY));
-    if (!context->has_platform_view && context->raster_cache &&
-        SkRect::Intersects(context->cull_rect, paint_bounds())) {
-      SkMatrix ctm = child_matrix;
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-      ctm = RasterCache::GetIntegralTransCTM(ctm);
-#endif
-      context->raster_cache->Prepare(context, container, ctm);
-    }
+    TryToPrepareRasterCache(context, container, child_matrix);
   }
+
+  // Restore cull_rect
+  context->cull_rect = context->cull_rect.makeOffset(offset_.fX, offset_.fY);
 }
 
 void OpacityLayer::Paint(PaintContext& context) const {
@@ -69,11 +69,6 @@ void OpacityLayer::Paint(PaintContext& context) const {
   SkAutoCanvasRestore save(context.internal_nodes_canvas, true);
   context.internal_nodes_canvas->translate(offset_.fX, offset_.fY);
 
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-  context.internal_nodes_canvas->setMatrix(RasterCache::GetIntegralTransCTM(
-      context.leaf_nodes_canvas->getTotalMatrix()));
-#endif
-
   if (context.raster_cache &&
       context.raster_cache->Draw(GetChildContainer(),
                                  *context.leaf_nodes_canvas, &paint)) {
@@ -83,8 +78,7 @@ void OpacityLayer::Paint(PaintContext& context) const {
   // Skia may clip the content with saveLayerBounds (although it's not a
   // guaranteed clip). So we have to provide a big enough saveLayerBounds. To do
   // so, we first remove the offset from paint bounds since it's already in the
-  // matrix. Then we round out the bounds because of our
-  // RasterCache::GetIntegralTransCTM optimization.
+  // matrix. Then we round out the bounds.
   //
   // Note that the following lines are only accessible when the raster cache is
   // not available (e.g., when we're using the software backend in golden

flow/layers/opacity_layer_unittests.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "flutter/flow/layers/clip_rect_layer.h"
 #include "flutter/flow/layers/opacity_layer.h"
 
 #include "flutter/flow/testing/layer_test.h"
@@ -58,10 +59,6 @@ TEST_F(OpacityLayerTest, FullyOpaque) {
   const SkMatrix initial_transform = SkMatrix::MakeTrans(0.5f, 0.5f);
   const SkMatrix layer_transform =
       SkMatrix::MakeTrans(layer_offset.fX, layer_offset.fY);
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-  const SkMatrix integral_layer_transform = RasterCache::GetIntegralTransCTM(
-      SkMatrix::Concat(initial_transform, layer_transform));
-#endif
   const SkPaint child_paint = SkPaint(SkColors::kGreen);
   const SkRect expected_layer_bounds =
       layer_transform.mapRect(child_path.getBounds());
@@ -86,10 +83,6 @@ TEST_F(OpacityLayerTest, FullyOpaque) {
   auto expected_draw_calls = std::vector(
       {MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
        MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer_transform}},
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-       MockCanvas::DrawCall{
-           1, MockCanvas::SetMatrixData{integral_layer_transform}},
-#endif
        MockCanvas::DrawCall{
            1, MockCanvas::SaveLayerData{opacity_bounds, opacity_paint, nullptr,
                                         2}},
@@ -107,10 +100,6 @@ TEST_F(OpacityLayerTest, FullyTransparent) {
   const SkMatrix initial_transform = SkMatrix::MakeTrans(0.5f, 0.5f);
   const SkMatrix layer_transform =
       SkMatrix::MakeTrans(layer_offset.fX, layer_offset.fY);
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-  const SkMatrix integral_layer_transform = RasterCache::GetIntegralTransCTM(
-      SkMatrix::Concat(initial_transform, layer_transform));
-#endif
   const SkPaint child_paint = SkPaint(SkColors::kGreen);
   const SkRect expected_layer_bounds =
       layer_transform.mapRect(child_path.getBounds());
@@ -133,10 +122,6 @@ TEST_F(OpacityLayerTest, FullyTransparent) {
   auto expected_draw_calls = std::vector(
       {MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
        MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer_transform}},
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-       MockCanvas::DrawCall{
-           1, MockCanvas::SetMatrixData{integral_layer_transform}},
-#endif
        MockCanvas::DrawCall{1, MockCanvas::SaveData{2}},
        MockCanvas::DrawCall{
            2, MockCanvas::ClipRectData{kEmptyRect, SkClipOp::kIntersect,
@@ -155,10 +140,6 @@ TEST_F(OpacityLayerTest, HalfTransparent) {
   const SkMatrix initial_transform = SkMatrix::MakeTrans(0.5f, 0.5f);
   const SkMatrix layer_transform =
       SkMatrix::MakeTrans(layer_offset.fX, layer_offset.fY);
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-  const SkMatrix integral_layer_transform = RasterCache::GetIntegralTransCTM(
-      SkMatrix::Concat(initial_transform, layer_transform));
-#endif
   const SkPaint child_paint = SkPaint(SkColors::kGreen);
   const SkRect expected_layer_bounds =
       layer_transform.mapRect(child_path.getBounds());
@@ -185,10 +166,6 @@ TEST_F(OpacityLayerTest, HalfTransparent) {
   auto expected_draw_calls = std::vector(
       {MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
        MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer_transform}},
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-       MockCanvas::DrawCall{
-           1, MockCanvas::SetMatrixData{integral_layer_transform}},
-#endif
        MockCanvas::DrawCall{
            1, MockCanvas::SaveLayerData{opacity_bounds, opacity_paint, nullptr,
                                         2}},
@@ -211,13 +188,6 @@ TEST_F(OpacityLayerTest, Nested) {
       SkMatrix::MakeTrans(layer1_offset.fX, layer1_offset.fY);
   const SkMatrix layer2_transform =
       SkMatrix::MakeTrans(layer2_offset.fX, layer2_offset.fY);
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-  const SkMatrix integral_layer1_transform = RasterCache::GetIntegralTransCTM(
-      SkMatrix::Concat(initial_transform, layer1_transform));
-  const SkMatrix integral_layer2_transform = RasterCache::GetIntegralTransCTM(
-      SkMatrix::Concat(SkMatrix::Concat(initial_transform, layer1_transform),
-                       layer2_transform));
-#endif
   const SkPaint child1_paint = SkPaint(SkColors::kRed);
   const SkPaint child2_paint = SkPaint(SkColors::kBlue);
   const SkPaint child3_paint = SkPaint(SkColors::kGreen);
@@ -278,10 +248,6 @@ TEST_F(OpacityLayerTest, Nested) {
   auto expected_draw_calls = std::vector(
       {MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
        MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer1_transform}},
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-       MockCanvas::DrawCall{
-           1, MockCanvas::SetMatrixData{integral_layer1_transform}},
-#endif
        MockCanvas::DrawCall{
            1, MockCanvas::SaveLayerData{opacity1_bounds, opacity1_paint,
                                         nullptr, 2}},
@@ -289,10 +255,6 @@ TEST_F(OpacityLayerTest, Nested) {
            2, MockCanvas::DrawPathData{child1_path, child1_paint}},
        MockCanvas::DrawCall{2, MockCanvas::SaveData{3}},
        MockCanvas::DrawCall{3, MockCanvas::ConcatMatrixData{layer2_transform}},
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-       MockCanvas::DrawCall{
-           3, MockCanvas::SetMatrixData{integral_layer2_transform}},
-#endif
        MockCanvas::DrawCall{
            3, MockCanvas::SaveLayerData{opacity2_bounds, opacity2_paint,
                                         nullptr, 4}},
@@ -327,5 +289,18 @@ TEST_F(OpacityLayerTest, Readback) {
   EXPECT_FALSE(preroll_context()->surface_needs_readback);
 }
 
+TEST_F(OpacityLayerTest, CullRectIsTransformed) {
+  auto clipRectLayer = std::make_shared<ClipRectLayer>(
+      SkRect::MakeLTRB(0, 0, 10, 10), flutter::hardEdge);
+  auto opacityLayer =
+      std::make_shared<OpacityLayer>(128, SkPoint::Make(20, 20));
+  auto mockLayer = std::make_shared<MockLayer>(SkPath());
+  clipRectLayer->Add(opacityLayer);
+  opacityLayer->Add(mockLayer);
+  clipRectLayer->Preroll(preroll_context(), SkMatrix::I());
+  EXPECT_EQ(mockLayer->parent_cull_rect().fLeft, -20);
+  EXPECT_EQ(mockLayer->parent_cull_rect().fTop, -20);
+}
+
 }  // namespace testing
 }  // namespace flutter

flow/raster_cache.h

@@ -62,7 +62,21 @@ class RasterCache {
     return bounds;
   }
 
+  /**
+   * @brief Snap the translation components of the matrix to integers.
+   *
+   * The snapping will only happen if the matrix only has scale and translation
+   * transformations.
+   *
+   * @param ctm the current transformation matrix.
+   * @return SkMatrix the snapped transformation matrix.
+   */
   static SkMatrix GetIntegralTransCTM(const SkMatrix& ctm) {
+    // Avoid integral snapping if the matrix has complex transformation to avoid
+    // the artifact observed in https://github.com/flutter/flutter/issues/41654.
+    if (!ctm.isScaleTranslate()) {
+      return ctm;
+    }
     SkMatrix result = ctm;
     result[SkMatrix::kMTransX] = SkScalarRoundToScalar(ctm.getTranslateX());
     result[SkMatrix::kMTransY] = SkScalarRoundToScalar(ctm.getTranslateY());

flow/raster_cache_key.h

@@ -15,11 +15,8 @@ template <typename ID>
 class RasterCacheKey {
  public:
   RasterCacheKey(ID id, const SkMatrix& ctm) : id_(id), matrix_(ctm) {
-    matrix_[SkMatrix::kMTransX] = SkScalarFraction(ctm.getTranslateX());
-    matrix_[SkMatrix::kMTransY] = SkScalarFraction(ctm.getTranslateY());
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-    FML_DCHECK(matrix_.getTranslateX() == 0 && matrix_.getTranslateY() == 0);
-#endif
+    matrix_[SkMatrix::kMTransX] = 0;
+    matrix_[SkMatrix::kMTransY] = 0;
   }
 
   ID id() const { return id_; }

flow/raster_cache_unittests.cc

@@ -159,9 +159,6 @@ TEST(RasterCache, DeviceRectRoundOut) {
   ASSERT_TRUE(cache.Draw(*picture, canvas));
 
   canvas.translate(248, 0);
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-  canvas.setMatrix(RasterCache::GetIntegralTransCTM(canvas.getTotalMatrix()));
-#endif
   ASSERT_TRUE(cache.Draw(*picture, canvas));
 }