Delay matrix call in Preroll of picture layer (#28380)

flow/layers/display_list_layer.cc

@@ -91,14 +91,8 @@ void DisplayListLayer::Preroll(PrerollContext* context,
 
   if (auto* cache = context->raster_cache) {
     TRACE_EVENT0("flutter", "DisplayListLayer::RasterCache (Preroll)");
-
-    SkMatrix ctm = matrix;
-    ctm.preTranslate(offset_.x(), offset_.y());
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-    ctm = RasterCache::GetIntegralTransCTM(ctm);
-#endif
-    cache->Prepare(context->gr_context, disp_list, ctm,
-                   context->dst_color_space, is_complex_, will_change_);
+    cache->Prepare(context, disp_list, is_complex_, will_change_, matrix,
+                   offset_);
   }
 
   SkRect bounds = disp_list->bounds().makeOffset(offset_.x(), offset_.y());

flow/layers/picture_layer.cc

@@ -113,14 +113,8 @@ void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
 
   if (auto* cache = context->raster_cache) {
     TRACE_EVENT0("flutter", "PictureLayer::RasterCache (Preroll)");
-
-    SkMatrix ctm = matrix;
-    ctm.preTranslate(offset_.x(), offset_.y());
-#ifndef SUPPORT_FRACTIONAL_TRANSLATION
-    ctm = RasterCache::GetIntegralTransCTM(ctm);
-#endif
-    cache->Prepare(context->gr_context, sk_picture, ctm,
-                   context->dst_color_space, is_complex_, will_change_);
+    cache->Prepare(context, sk_picture, is_complex_, will_change_, matrix,
+                   offset_);
   }
 
   SkRect bounds = sk_picture->cullRect().makeOffset(offset_.x(), offset_.y());

flow/raster_cache.cc

@@ -37,39 +37,13 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const {
 }
 
 RasterCache::RasterCache(size_t access_threshold,
-                         size_t picture_cache_limit_per_frame)
+                         size_t picture_and_display_list_cache_limit_per_frame)
     : access_threshold_(access_threshold),
-      picture_cache_limit_per_frame_(picture_cache_limit_per_frame),
+      picture_and_display_list_cache_limit_per_frame_(
+          picture_and_display_list_cache_limit_per_frame),
       checkerboard_images_(false) {}
 
-static bool CanRasterizePicture(SkPicture* picture) {
-  if (picture == nullptr) {
-    return false;
-  }
-
-  const SkRect cull_rect = picture->cullRect();
-
-  if (cull_rect.isEmpty()) {
-    // No point in ever rasterizing an empty picture.
-    return false;
-  }
-
-  if (!cull_rect.isFinite()) {
-    // Cannot attempt to rasterize into an infinitely large surface.
-    FML_LOG(INFO) << "Attempted to raster cache non-finite picture";
-    return false;
-  }
-
-  return true;
-}
-
-static bool CanRasterizeDisplayList(DisplayList* display_list) {
-  if (display_list == nullptr) {
-    return false;
-  }
-
-  const SkRect cull_rect = display_list->bounds();
-
+static bool CanRasterizeRect(const SkRect& cull_rect) {
   if (cull_rect.isEmpty()) {
     // No point in ever rasterizing an empty display list.
     return false;
@@ -93,7 +67,7 @@ static bool IsPictureWorthRasterizing(SkPicture* picture,
     return false;
   }
 
-  if (!CanRasterizePicture(picture)) {
+  if (picture == nullptr || !CanRasterizeRect(picture->cullRect())) {
     // No point in deciding whether the picture is worth rasterizing if it
     // cannot be rasterized at all.
     return false;
@@ -119,7 +93,7 @@ static bool IsDisplayListWorthRasterizing(DisplayList* display_list,
     return false;
   }
 
-  if (!CanRasterizeDisplayList(display_list)) {
+  if (display_list == nullptr || !CanRasterizeRect(display_list->bounds())) {
     // No point in deciding whether the display list is worth rasterizing if it
     // cannot be rasterized at all.
     return false;
@@ -238,24 +212,24 @@ std::unique_ptr<RasterCacheResult> RasterCache::RasterizeLayer(
       });
 }
 
-bool RasterCache::Prepare(GrDirectContext* context,
+bool RasterCache::Prepare(PrerollContext* context,
                           SkPicture* picture,
-                          const SkMatrix& transformation_matrix,
-                          SkColorSpace* dst_color_space,
                           bool is_complex,
-                          bool will_change) {
-  // Disabling caching when access_threshold is zero is historic behavior.
-  if (access_threshold_ == 0) {
-    return false;
-  }
-  if (picture_cached_this_frame_ >= picture_cache_limit_per_frame_) {
+                          bool will_change,
+                          const SkMatrix& untranslated_matrix,
+                          const SkPoint& offset) {
+  if (!GenerateNewCacheInThisFrame()) {
     return false;
   }
+
   if (!IsPictureWorthRasterizing(picture, will_change, is_complex)) {
     // We only deal with pictures that are worthy of rasterization.
     return false;
   }
 
+  SkMatrix transformation_matrix = untranslated_matrix;
+  transformation_matrix.preTranslate(offset.x(), offset.y());
+
   // Decompose the matrix (once) for all subsequent operations. We want to make
   // sure to avoid volumetric distortions while accounting for scaling.
   const MatrixDecomposition matrix(transformation_matrix);
@@ -275,31 +249,38 @@ bool RasterCache::Prepare(GrDirectContext* context,
   }
 
   if (!entry.image) {
-    entry.image = RasterizePicture(picture, context, transformation_matrix,
-                                   dst_color_space, checkerboard_images_);
+    // GetIntegralTransCTM effect for matrix which only contains scale,
+    // translate, so it won't affect result of matrix decomposition and cache
+    // key.
+#ifndef SUPPORT_FRACTIONAL_TRANSLATION
+    transformation_matrix = GetIntegralTransCTM(transformation_matrix);
+#endif
+    entry.image =
+        RasterizePicture(picture, context->gr_context, transformation_matrix,
+                         context->dst_color_space, checkerboard_images_);
     picture_cached_this_frame_++;
   }
   return true;
 }
 
-bool RasterCache::Prepare(GrDirectContext* context,
+bool RasterCache::Prepare(PrerollContext* context,
                           DisplayList* display_list,
-                          const SkMatrix& transformation_matrix,
-                          SkColorSpace* dst_color_space,
                           bool is_complex,
-                          bool will_change) {
-  // Disabling caching when access_threshold is zero is historic behavior.
-  if (access_threshold_ == 0) {
-    return false;
-  }
-  if (picture_cached_this_frame_ >= picture_cache_limit_per_frame_) {
+                          bool will_change,
+                          const SkMatrix& untranslated_matrix,
+                          const SkPoint& offset) {
+  if (!GenerateNewCacheInThisFrame()) {
     return false;
   }
+
   if (!IsDisplayListWorthRasterizing(display_list, will_change, is_complex)) {
     // We only deal with display lists that are worthy of rasterization.
     return false;
   }
 
+  SkMatrix transformation_matrix = untranslated_matrix;
+  transformation_matrix.preTranslate(offset.x(), offset.y());
+
   // Decompose the matrix (once) for all subsequent operations. We want to make
   // sure to avoid volumetric distortions while accounting for scaling.
   const MatrixDecomposition matrix(transformation_matrix);
@@ -320,10 +301,16 @@ bool RasterCache::Prepare(GrDirectContext* context,
   }
 
   if (!entry.image) {
-    entry.image =
-        RasterizeDisplayList(display_list, context, transformation_matrix,
-                             dst_color_space, checkerboard_images_);
-    picture_cached_this_frame_++;
+    // GetIntegralTransCTM effect for matrix which only contains scale,
+    // translate, so it won't affect result of matrix decomposition and cache
+    // key.
+#ifndef SUPPORT_FRACTIONAL_TRANSLATION
+    transformation_matrix = GetIntegralTransCTM(transformation_matrix);
+#endif
+    entry.image = RasterizeDisplayList(
+        display_list, context->gr_context, transformation_matrix,
+        context->dst_color_space, checkerboard_images_);
+    display_list_cached_this_frame_++;
   }
   return true;
 }
@@ -395,6 +382,7 @@ void RasterCache::SweepAfterFrame() {
   SweepOneCacheAfterFrame(display_list_cache_);
   SweepOneCacheAfterFrame(layer_cache_);
   picture_cached_this_frame_ = 0;
+  display_list_cached_this_frame_ = 0;
   sweep_count_++;
 }
 

flow/raster_cache.h

@@ -42,15 +42,15 @@ struct PrerollContext;
 
 class RasterCache {
  public:
-  // The default max number of picture raster caches to be generated per frame.
-  // Generating too many caches in one frame may cause jank on that frame. This
-  // limit allows us to throttle the cache and distribute the work across
-  // multiple frames.
-  static constexpr int kDefaultPictureCacheLimitPerFrame = 3;
+  // The default max number of picture and display list raster caches to be
+  // generated per frame. Generating too many caches in one frame may cause jank
+  // on that frame. This limit allows us to throttle the cache and distribute
+  // the work across multiple frames.
+  static constexpr int kDefaultPictureAndDispLayListCacheLimitPerFrame = 3;
 
-  explicit RasterCache(
-      size_t access_threshold = 3,
-      size_t picture_cache_limit_per_frame = kDefaultPictureCacheLimitPerFrame);
+  explicit RasterCache(size_t access_threshold = 3,
+                       size_t picture_and_display_list_cache_limit_per_frame =
+                           kDefaultPictureAndDispLayListCacheLimitPerFrame);
 
   virtual ~RasterCache() = default;
 
@@ -134,23 +134,23 @@ class RasterCache {
   // Return true if the cache is generated.
   //
   // We may return false and not generate the cache if
-  // 1. The picture is not worth rasterizing
-  // 2. The matrix is singular
-  // 3. The picture is accessed too few times
-  // 4. There are too many pictures to be cached in the current frame.
-  //    (See also kDefaultPictureCacheLimitPerFrame.)
-  bool Prepare(GrDirectContext* context,
+  // 1. There are too many pictures to be cached in the current frame.
+  //    (See also kDefaultPictureAndDispLayListCacheLimitPerFrame.)
+  // 2. The picture is not worth rasterizing
+  // 3. The matrix is singular
+  // 4. The picture is accessed too few times
+  bool Prepare(PrerollContext* context,
                SkPicture* picture,
-               const SkMatrix& transformation_matrix,
-               SkColorSpace* dst_color_space,
                bool is_complex,
-               bool will_change);
-  bool Prepare(GrDirectContext* context,
+               bool will_change,
+               const SkMatrix& untranslated_matrix,
+               const SkPoint& offset = SkPoint());
+  bool Prepare(PrerollContext* context,
                DisplayList* display_list,
-               const SkMatrix& transformation_matrix,
-               SkColorSpace* dst_color_space,
                bool is_complex,
-               bool will_change);
+               bool will_change,
+               const SkMatrix& untranslated_matrix,
+               const SkPoint& offset = SkPoint());
 
   void Prepare(PrerollContext* context, Layer* layer, const SkMatrix& ctm);
 
@@ -250,9 +250,17 @@ class RasterCache {
     }
   }
 
+  bool GenerateNewCacheInThisFrame() const {
+    // Disabling caching when access_threshold is zero is historic behavior.
+    return access_threshold_ != 0 &&
+           picture_cached_this_frame_ + display_list_cached_this_frame_ <
+               picture_and_display_list_cache_limit_per_frame_;
+  }
+
   const size_t access_threshold_;
-  const size_t picture_cache_limit_per_frame_;
+  const size_t picture_and_display_list_cache_limit_per_frame_;
   size_t picture_cached_this_frame_ = 0;
+  size_t display_list_cached_this_frame_ = 0;
   int sweep_count_ = 0;
   mutable PictureRasterCacheKey::Map<Entry> picture_cache_;
   mutable DisplayListRasterCacheKey::Map<Entry> display_list_cache_;

flow/raster_cache_unittests.cc

@@ -4,6 +4,7 @@
 
 #include "flutter/flow/raster_cache.h"
 
+#include "flutter/flow/testing/mock_raster_cache.h"
 #include "gtest/gtest.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkPaint.h"
@@ -39,20 +40,19 @@ TEST(RasterCache, ThresholdIsRespected) {
 
   auto picture = GetSamplePicture();
 
-  sk_sp<SkImage> image;
-
   SkCanvas dummy_canvas;
 
-  sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
-  ASSERT_FALSE(
-      cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
+  PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder();
+
+  ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context,
+                             picture.get(), true, false, matrix));
   // 1st access.
   ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
 
   cache.SweepAfterFrame();
 
-  ASSERT_FALSE(
-      cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
+  ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context,
+                             picture.get(), true, false, matrix));
 
   // 2nd access.
   ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
@@ -60,8 +60,8 @@ TEST(RasterCache, ThresholdIsRespected) {
   cache.SweepAfterFrame();
 
   // Now Prepare should cache it.
-  ASSERT_TRUE(
-      cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
+  ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context,
+                            picture.get(), true, false, matrix));
   ASSERT_TRUE(cache.Draw(*picture, dummy_canvas));
 }
 
@@ -73,13 +73,12 @@ TEST(RasterCache, AccessThresholdOfZeroDisablesCaching) {
 
   auto picture = GetSamplePicture();
 
-  sk_sp<SkImage> image;
-
   SkCanvas dummy_canvas;
 
-  sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
-  ASSERT_FALSE(
-      cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
+  PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder();
+
+  ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context,
+                             picture.get(), true, false, matrix));
 
   ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
 }
@@ -92,13 +91,12 @@ TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZero) {
 
   auto picture = GetSamplePicture();
 
-  sk_sp<SkImage> image;
-
   SkCanvas dummy_canvas;
 
-  sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
-  ASSERT_FALSE(
-      cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
+  PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder();
+
+  ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context,
+                             picture.get(), true, false, matrix));
 
   ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
 }
@@ -111,19 +109,18 @@ TEST(RasterCache, SweepsRemoveUnusedFrames) {
 
   auto picture = GetSamplePicture();
 
-  sk_sp<SkImage> image;
-
   SkCanvas dummy_canvas;
 
-  sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
-  ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
-                             false));  // 1
+  PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder();
+
+  ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context,
+                             picture.get(), true, false, matrix));  // 1
   ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
 
   cache.SweepAfterFrame();
 
-  ASSERT_TRUE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
-                            false));  // 2
+  ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context,
+                            picture.get(), true, false, matrix));  // 2
   ASSERT_TRUE(cache.Draw(*picture, dummy_canvas));
 
   cache.SweepAfterFrame();
@@ -152,12 +149,14 @@ TEST(RasterCache, DeviceRectRoundOut) {
   SkCanvas canvas(100, 100, nullptr);
   canvas.setMatrix(ctm);
 
-  sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
-  ASSERT_FALSE(
-      cache.Prepare(NULL, picture.get(), ctm, srgb.get(), true, false));
+  PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder();
+
+  ASSERT_FALSE(cache.Prepare(&preroll_context_holder.preroll_context,
+                             picture.get(), true, false, ctm));
   ASSERT_FALSE(cache.Draw(*picture, canvas));
   cache.SweepAfterFrame();
-  ASSERT_TRUE(cache.Prepare(NULL, picture.get(), ctm, srgb.get(), true, false));
+  ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context,
+                            picture.get(), true, false, ctm));
   ASSERT_TRUE(cache.Draw(*picture, canvas));
 
   canvas.translate(248, 0);

flow/testing/mock_raster_cache.cc

@@ -4,7 +4,6 @@
 
 #include "flutter/flow/testing/mock_raster_cache.h"
 
-#include "flutter/flow/layers/layer.h"
 #include "third_party/skia/include/core/SkPictureRecorder.h"
 
 namespace flutter {
@@ -57,11 +56,37 @@ void MockRasterCache::AddMockPicture(int width, int height) {
       SkRect::MakeLTRB(0, 0, 200 + width, 200 + height), &rtree_factory);
   recorder_canvas->drawPath(path, SkPaint());
   sk_sp<SkPicture> picture = skp_recorder.finishRecordingAsPicture();
+  PrerollContextHolder holder = GetSamplePrerollContextHolder();
+  holder.preroll_context.dst_color_space = color_space_;
   for (int i = 0; i < access_threshold(); i++) {
-    Prepare(nullptr, picture.get(), ctm, color_space_, true, false);
+    Prepare(&holder.preroll_context, picture.get(), true, false, ctm);
     Draw(*picture, mock_canvas_);
   }
-  Prepare(nullptr, picture.get(), ctm, color_space_, true, false);
+  Prepare(&holder.preroll_context, picture.get(), true, false, ctm);
+}
+
+PrerollContextHolder GetSamplePrerollContextHolder() {
+  Stopwatch raster_time;
+  Stopwatch ui_time;
+  MutatorsStack mutators_stack;
+  TextureRegistry texture_registry;
+  sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
+  PrerollContextHolder holder = {
+      {
+          nullptr,                    /* raster_cache */
+          nullptr,                    /* gr_context */
+          nullptr,                    /* external_view_embedder */
+          mutators_stack, srgb.get(), /* color_space */
+          kGiantRect,                 /* cull_rect */
+          false,                      /* layer reads from surface */
+          raster_time, ui_time, texture_registry,
+          false, /* checkerboard_offscreen_layers */
+          1.0f,  /* frame_device_pixel_ratio */
+          false, /* has_platform_view */
+      },
+      srgb};
+
+  return holder;
 }
 
 }  // namespace testing

flow/testing/mock_raster_cache.h

@@ -5,6 +5,7 @@
 #ifndef FLOW_TESTING_MOCK_RASTER_CACHE_H_
 #define FLOW_TESTING_MOCK_RASTER_CACHE_H_
 
+#include "flutter/flow/layers/layer.h"
 #include "flutter/flow/raster_cache.h"
 #include "flutter/flow/testing/mock_layer.h"
 #include "flutter/testing/mock_canvas.h"
@@ -48,8 +49,10 @@ class MockRasterCache : public RasterCache {
  public:
   explicit MockRasterCache(
       size_t access_threshold = 3,
-      size_t picture_cache_limit_per_frame = kDefaultPictureCacheLimitPerFrame)
-      : RasterCache(access_threshold, picture_cache_limit_per_frame) {}
+      size_t picture_and_display_list_cache_limit_per_frame =
+          kDefaultPictureAndDispLayListCacheLimitPerFrame)
+      : RasterCache(access_threshold,
+                    picture_and_display_list_cache_limit_per_frame) {}
 
   std::unique_ptr<RasterCacheResult> RasterizePicture(
       SkPicture* picture,
@@ -92,6 +95,13 @@ class MockRasterCache : public RasterCache {
   };
 };
 
+struct PrerollContextHolder {
+  PrerollContext preroll_context;
+  sk_sp<SkColorSpace> srgb;
+};
+
+PrerollContextHolder GetSamplePrerollContextHolder();
+
 }  // namespace testing
 }  // namespace flutter
 

shell/common/shell_unittests.cc

@@ -2146,23 +2146,7 @@ TEST_F(ShellTest, OnServiceProtocolEstimateRasterCacheMemoryWorks) {
       [&shell, &rasterized, &picture, &picture_layer] {
         auto* compositor_context = shell->GetRasterizer()->compositor_context();
         auto& raster_cache = compositor_context->raster_cache();
-        // 2.1. Rasterize the picture. Call Draw multiple times to pass the
-        // access threshold (default to 3) so a cache can be generated.
-        SkCanvas dummy_canvas;
-        bool picture_cache_generated;
-        for (int i = 0; i < 4; i += 1) {
-          picture_cache_generated =
-              raster_cache.Prepare(nullptr,  // GrDirectContext
-                                   picture.get(), SkMatrix::I(),
-                                   nullptr,  // SkColorSpace
-                                   true,     // isComplex
-                                   false     // willChange
-              );
-          raster_cache.Draw(*picture, dummy_canvas);
-        }
-        ASSERT_TRUE(picture_cache_generated);
 
-        // 2.2. Rasterize the picture layer.
         Stopwatch raster_time;
         Stopwatch ui_time;
         MutatorsStack mutators_stack;
@@ -2179,6 +2163,21 @@ TEST_F(ShellTest, OnServiceProtocolEstimateRasterCacheMemoryWorks) {
             1.0f,  /* frame_device_pixel_ratio */
             false, /* has_platform_view */
         };
+
+        // 2.1. Rasterize the picture. Call Draw multiple times to pass the
+        // access threshold (default to 3) so a cache can be generated.
+        SkCanvas dummy_canvas;
+        bool picture_cache_generated;
+        for (int i = 0; i < 4; i += 1) {
+          SkMatrix matrix = SkMatrix::I();
+
+          picture_cache_generated = raster_cache.Prepare(
+              &preroll_context, picture.get(), true, false, matrix);
+          raster_cache.Draw(*picture, dummy_canvas);
+        }
+        ASSERT_TRUE(picture_cache_generated);
+
+        // 2.2. Rasterize the picture layer.
         raster_cache.Prepare(&preroll_context, picture_layer.get(),
                              SkMatrix::I());
         rasterized.set_value(true);