From 01a52b9947054f57398f2aaa2b59e7d501506580 Mon Sep 17 00:00:00 2001 From: Sebastian Jeltsch Date: Fri, 28 Feb 2020 21:13:22 +0100 Subject: [PATCH] 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. --- flow/raster_cache.cc | 39 ++++++++++++----------- flow/raster_cache.h | 3 +- flow/raster_cache_unittests.cc | 56 +++++++++++++++++++++++++--------- 3 files changed, 64 insertions(+), 34 deletions(-) diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index f179905ed..46253c97c 100644 --- a/flow/raster_cache.cc +++ b/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, diff --git a/flow/raster_cache.h b/flow/raster_cache.h index b51382a5c..93e6200cb 100644 --- a/flow/raster_cache.h +++ b/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; }; diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index 03e3b3879..16abea65e 100644 --- a/flow/raster_cache_unittests.cc +++ b/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,7 +30,7 @@ TEST(RasterCache, SimpleInitialization) { ASSERT_TRUE(true); } -TEST(RasterCache, ThresholdIsRespected) { +TEST(RasterCache, ThresholdIsRespectedForPictures) { size_t threshold = 2; flutter::RasterCache cache(threshold); @@ -37,8 +38,6 @@ TEST(RasterCache, ThresholdIsRespected) { auto picture = GetSamplePicture(); - sk_sp image; - sk_sp 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 image; + ContainerLayer layer; sk_sp 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 image; - sk_sp 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 image; - sk_sp 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 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 -- GitLab