Try rasterizing images and layers only once, even when their rasterization fails. Further enforce the same access threshold on layers as on Pictures. Previously layers would always be cached. The latter is a semantic change. (#16545)

If Rasterization fails, i.e. image.is_valid() is false, the cache might try rasterizing the image again on the next frame. Not only is this wasteful put might also prevent other pictures to be cached within the current frame budget.

Author: ignatz

flow/raster_cache.cc

@@ -119,24 +119,15 @@ static RasterCacheResult Rasterize(
   return {surface->makeImageSnapshot(), logical_rect};
 }
 
-RasterCacheResult RasterizePicture(SkPicture* picture,
-                                   GrContext* context,
-                                   const SkMatrix& ctm,
-                                   SkColorSpace* dst_color_space,
-                                   bool checkerboard) {
-  return Rasterize(context, ctm, dst_color_space, checkerboard,
-                   picture->cullRect(),
-                   [=](SkCanvas* canvas) { canvas->drawPicture(picture); });
-}
-
-void RasterCache::Prepare(PrerollContext* context,
+bool RasterCache::Prepare(PrerollContext* context,
                           Layer* layer,
                           const SkMatrix& ctm) {
   LayerRasterCacheKey cache_key(layer->unique_id(), ctm);
+
   Entry& entry = layer_cache_[cache_key];
-  entry.access_count++;
-  entry.used_this_frame = true;
-  if (!entry.image.is_valid()) {
+
+  if (!entry.did_rasterize && !entry.image.is_valid() &&
+      entry.access_count >= access_threshold_) {
     entry.image = Rasterize(
         context->gr_context, ctm, context->dst_color_space,
         checkerboard_images_, layer->paint_bounds(),
@@ -161,7 +152,12 @@ void RasterCache::Prepare(PrerollContext* context,
             layer->Paint(paintContext);
           }
         });
+
+    entry.did_rasterize = true;
+
+    return true;
   }
+  return false;
 }
 
 bool RasterCache::Prepare(GrContext* context,
@@ -200,12 +196,19 @@ bool RasterCache::Prepare(GrContext* context,
     return false;
   }
 
-  if (!entry.image.is_valid()) {
-    entry.image = RasterizePicture(picture, context, transformation_matrix,
-                                   dst_color_space, checkerboard_images_);
+  // Don't try to rasterize pictures that were already attempted to be
+  // rasterized even if the image is invalid.
+  if (!entry.did_rasterize && !entry.image.is_valid()) {
+    entry.image =
+        Rasterize(context, transformation_matrix, dst_color_space,
+                  checkerboard_images_, picture->cullRect(),
+                  [=](SkCanvas* canvas) { canvas->drawPicture(picture); });
+
+    entry.did_rasterize = true;
     picture_cached_this_frame_++;
+    return true;
   }
-  return true;
+  return false;
 }
 
 RasterCacheResult RasterCache::Get(const SkPicture& picture,

flow/raster_cache.h

@@ -84,7 +84,7 @@ class RasterCache {
                bool is_complex,
                bool will_change);
 
-  void Prepare(PrerollContext* context, Layer* layer, const SkMatrix& ctm);
+  bool Prepare(PrerollContext* context, Layer* layer, const SkMatrix& ctm);
 
   RasterCacheResult Get(const SkPicture& picture, const SkMatrix& ctm) const;
 
@@ -101,6 +101,7 @@ class RasterCache {
  private:
   struct Entry {
     bool used_this_frame = false;
+    bool did_rasterize = false;
     size_t access_count = 0;
     RasterCacheResult image;
   };

flow/raster_cache_unittests.cc

@@ -4,6 +4,7 @@
 
 #include "flutter/flow/raster_cache.h"
 
+#include "flutter/flow/layers/container_layer.h"
 #include "gtest/gtest.h"
 #include "third_party/skia/include/core/SkPicture.h"
 #include "third_party/skia/include/core/SkPictureRecorder.h"
@@ -29,16 +30,14 @@ TEST(RasterCache, SimpleInitialization) {
   ASSERT_TRUE(true);
 }
 
-TEST(RasterCache, ThresholdIsRespected) {
+TEST(RasterCache, ThresholdIsRespectedForPictures) {
   size_t threshold = 2;
   flutter::RasterCache cache(threshold);
 
   SkMatrix matrix = SkMatrix::I();
 
   auto picture = GetSamplePicture();
 
-  sk_sp<SkImage> image;
-
   sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
   ASSERT_FALSE(
       cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
@@ -61,21 +60,28 @@ TEST(RasterCache, ThresholdIsRespected) {
   ASSERT_TRUE(cache.Get(*picture, matrix).is_valid());
 }
 
-TEST(RasterCache, AccessThresholdOfZeroDisablesCaching) {
-  size_t threshold = 0;
+TEST(RasterCache, ThresholdIsRespectedForLayers) {
+  size_t threshold = 2;
   flutter::RasterCache cache(threshold);
 
   SkMatrix matrix = SkMatrix::I();
 
-  auto picture = GetSamplePicture();
-
-  sk_sp<SkImage> image;
+  ContainerLayer layer;
 
   sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
-  ASSERT_FALSE(
-      cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
+  ASSERT_FALSE(cache.Prepare(nullptr, &layer, matrix));
+  ASSERT_FALSE(cache.Prepare(nullptr, &layer, matrix));
+  ASSERT_FALSE(cache.Prepare(nullptr, &layer, matrix));
 
-  ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
+  // 1st access.
+  ASSERT_FALSE(cache.Get(&layer, matrix).is_valid());
+
+  ASSERT_FALSE(cache.Prepare(nullptr, &layer, matrix));
+
+  // 2st access.
+  ASSERT_FALSE(cache.Get(&layer, matrix).is_valid());
+
+  // Calling Prepare now would crash due to the nullptr.
 }
 
 TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZero) {
@@ -86,8 +92,6 @@ TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZero) {
 
   auto picture = GetSamplePicture();
 
-  sk_sp<SkImage> image;
-
   sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
   ASSERT_FALSE(
       cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
@@ -103,8 +107,6 @@ TEST(RasterCache, SweepsRemoveUnusedFrames) {
 
   auto picture = GetSamplePicture();
 
-  sk_sp<SkImage> image;
-
   sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
   ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
                              false));  // 1
@@ -122,5 +124,29 @@ TEST(RasterCache, SweepsRemoveUnusedFrames) {
   ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
 }
 
+TEST(RasterCache, TryRasterizngOnlyOnce) {
+  size_t threshold = 1;
+  flutter::RasterCache cache(threshold);
+
+  SkMatrix matrix = SkMatrix::I();
+  // Test picture too large to successfully rasterize.
+  auto picture = SkPicture::MakePlaceholder(SkRect::MakeWH(2e12, 2e12));
+
+  sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
+  ASSERT_FALSE(cache.Prepare(nullptr, picture.get(), matrix, srgb.get(), true,
+                             false));  // 1
+  ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
+
+  // Rasterization ran, though Get() below returns an invalid image.
+  ASSERT_TRUE(cache.Prepare(nullptr, picture.get(), matrix, srgb.get(), true,
+                            false));  // 2
+  ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
+
+  // This time we should not try again to rasterize.
+  ASSERT_FALSE(cache.Prepare(nullptr, picture.get(), matrix, srgb.get(), true,
+                             false));  // 2
+  ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
+}
+
 }  // namespace testing
 }  // namespace flutter