Replace RasterCache::Get with RasterCache:Draw (#17791)

This avoids the possible matrix mismatch between RasterCache::Get and
RasterCacheResult::draw. See
https://github.com/flutter/engine/pull/17790 for an example that tries
to fix an earlier mismatch.

flow/layers/image_filter_layer.cc

@@ -48,14 +48,9 @@ void ImageFilterLayer::Paint(PaintContext& context) const {
       context.leaf_nodes_canvas->getTotalMatrix()));
 #endif
 
-  if (context.raster_cache) {
-    const SkMatrix& ctm = context.leaf_nodes_canvas->getTotalMatrix();
-    RasterCacheResult layer_cache =
-        context.raster_cache->Get((Layer*)this, ctm);
-    if (layer_cache.is_valid()) {
-      layer_cache.draw(*context.leaf_nodes_canvas);
-      return;
-    }
+  if (context.raster_cache &&
+      context.raster_cache->Draw(this, *context.leaf_nodes_canvas)) {
+    return;
   }
 
   SkPaint paint;

flow/layers/opacity_layer.cc

@@ -74,14 +74,10 @@ void OpacityLayer::Paint(PaintContext& context) const {
       context.leaf_nodes_canvas->getTotalMatrix()));
 #endif
 
-  if (context.raster_cache) {
-    ContainerLayer* container = GetChildContainer();
-    const SkMatrix& ctm = context.leaf_nodes_canvas->getTotalMatrix();
-    RasterCacheResult child_cache = context.raster_cache->Get(container, ctm);
-    if (child_cache.is_valid()) {
-      child_cache.draw(*context.leaf_nodes_canvas, &paint);
-      return;
-    }
+  if (context.raster_cache &&
+      context.raster_cache->Draw(GetChildContainer(),
+                                 *context.leaf_nodes_canvas, &paint)) {
+    return;
   }
 
   // Skia may clip the content with saveLayerBounds (although it's not a

flow/layers/picture_layer.cc

@@ -49,15 +49,10 @@ void PictureLayer::Paint(PaintContext& context) const {
       context.leaf_nodes_canvas->getTotalMatrix()));
 #endif
 
-  if (context.raster_cache) {
-    const SkMatrix& ctm = context.leaf_nodes_canvas->getTotalMatrix();
-    RasterCacheResult result = context.raster_cache->Get(*picture(), ctm);
-    if (result.is_valid()) {
-      TRACE_EVENT_INSTANT0("flutter", "raster cache hit");
-
-      result.draw(*context.leaf_nodes_canvas);
-      return;
-    }
+  if (context.raster_cache &&
+      context.raster_cache->Draw(*picture(), *context.leaf_nodes_canvas)) {
+    TRACE_EVENT_INSTANT0("flutter", "raster cache hit");
+    return;
   }
   picture()->playback(context.leaf_nodes_canvas);
 }

flow/raster_cache.cc

@@ -210,33 +210,44 @@ bool RasterCache::Prepare(GrContext* context,
   return true;
 }
 
-RasterCacheResult RasterCache::Get(const SkPicture& picture,
-                                   const SkMatrix& ctm) const {
-  PictureRasterCacheKey cache_key(picture.uniqueID(), ctm);
+bool RasterCache::Draw(const SkPicture& picture, SkCanvas& canvas) const {
+  PictureRasterCacheKey cache_key(picture.uniqueID(), canvas.getTotalMatrix());
   auto it = picture_cache_.find(cache_key);
   if (it == picture_cache_.end()) {
-    return RasterCacheResult();
+    return false;
   }
 
   Entry& entry = it->second;
   entry.access_count++;
   entry.used_this_frame = true;
 
-  return entry.image;
+  if (entry.image.is_valid()) {
+    entry.image.draw(canvas);
+    return true;
+  }
+
+  return false;
 }
 
-RasterCacheResult RasterCache::Get(Layer* layer, const SkMatrix& ctm) const {
-  LayerRasterCacheKey cache_key(layer->unique_id(), ctm);
+bool RasterCache::Draw(const Layer* layer,
+                       SkCanvas& canvas,
+                       SkPaint* paint) const {
+  LayerRasterCacheKey cache_key(layer->unique_id(), canvas.getTotalMatrix());
   auto it = layer_cache_.find(cache_key);
   if (it == layer_cache_.end()) {
-    return RasterCacheResult();
+    return false;
   }
 
   Entry& entry = it->second;
   entry.access_count++;
   entry.used_this_frame = true;
 
-  return entry.image;
+  if (entry.image.is_valid()) {
+    entry.image.draw(canvas, paint);
+    return true;
+  }
+
+  return false;
 }
 
 void RasterCache::SweepAfterFrame() {

flow/raster_cache.h

@@ -86,9 +86,20 @@ class RasterCache {
 
   void Prepare(PrerollContext* context, Layer* layer, const SkMatrix& ctm);
 
-  RasterCacheResult Get(const SkPicture& picture, const SkMatrix& ctm) const;
+  // Find the raster cache for the picture and draw it to the canvas.
+  //
+  // Return true if it's found and drawn.
+  bool Draw(const SkPicture& picture, SkCanvas& canvas) const;
 
-  RasterCacheResult Get(Layer* layer, const SkMatrix& ctm) const;
+  // Find the raster cache for the layer and draw it to the canvas.
+  //
+  // Addional paint can be given to change how the raster cache is drawn (e.g.,
+  // draw the raster cache with some opacity).
+  //
+  // Return true if the layer raster cache is found and drawn.
+  bool Draw(const Layer* layer,
+            SkCanvas& canvas,
+            SkPaint* paint = nullptr) const;
 
   void SweepAfterFrame();
 

flow/raster_cache_unittests.cc

@@ -39,26 +39,28 @@ TEST(RasterCache, ThresholdIsRespected) {
 
   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));
   // 1st access.
-  ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
+  ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
 
   cache.SweepAfterFrame();
 
   ASSERT_FALSE(
       cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
 
-  // 2st access.
-  ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
+  // 2nd access.
+  ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
 
   cache.SweepAfterFrame();
 
   // Now Prepare should cache it.
   ASSERT_TRUE(
       cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
-  ASSERT_TRUE(cache.Get(*picture, matrix).is_valid());
+  ASSERT_TRUE(cache.Draw(*picture, dummy_canvas));
 }
 
 TEST(RasterCache, AccessThresholdOfZeroDisablesCaching) {
@@ -71,11 +73,13 @@ TEST(RasterCache, AccessThresholdOfZeroDisablesCaching) {
 
   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));
 
-  ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
+  ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
 }
 
 TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZero) {
@@ -88,11 +92,13 @@ TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZero) {
 
   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));
 
-  ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
+  ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
 }
 
 TEST(RasterCache, SweepsRemoveUnusedFrames) {
@@ -105,21 +111,23 @@ TEST(RasterCache, SweepsRemoveUnusedFrames) {
 
   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
-  ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
+  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.Get(*picture, matrix).is_valid());
+  ASSERT_TRUE(cache.Draw(*picture, dummy_canvas));
 
   cache.SweepAfterFrame();
   cache.SweepAfterFrame();  // Extra frame without a Get image access.
 
-  ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
+  ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
 }
 
 // Construct a cache result whose device target rectangle rounds out to be one
@@ -139,19 +147,22 @@ TEST(RasterCache, DeviceRectRoundOut) {
 
   SkMatrix ctm = SkMatrix::MakeAll(1.3312, 0, 233, 0, 1.3312, 206, 0, 0, 1);
 
+  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));
-  ASSERT_FALSE(cache.Get(*picture, ctm).is_valid());
+  ASSERT_FALSE(cache.Draw(*picture, canvas));
   cache.SweepAfterFrame();
   ASSERT_TRUE(cache.Prepare(NULL, picture.get(), ctm, srgb.get(), true, false));
-  ASSERT_TRUE(cache.Get(*picture, ctm).is_valid());
+  ASSERT_TRUE(cache.Draw(*picture, canvas));
 
-  SkCanvas canvas(100, 100, nullptr);
-  canvas.setMatrix(ctm);
   canvas.translate(248, 0);
-
-  cache.Get(*picture, ctm).draw(canvas);
+#ifndef SUPPORT_FRACTIONAL_TRANSLATION
+  canvas.setMatrix(RasterCache::GetIntegralTransCTM(canvas.getTotalMatrix()));
+#endif
+  ASSERT_TRUE(cache.Draw(*picture, canvas));
 }
 
 }  // namespace testing