diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index a3bbb20b9b05a..f179905ed8e53 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -17,12 +17,6 @@ namespace flutter { -RasterCacheResult::RasterCacheResult() {} - -RasterCacheResult::RasterCacheResult(const RasterCacheResult& other) = default; - -RasterCacheResult::~RasterCacheResult() = default; - RasterCacheResult::RasterCacheResult(sk_sp image, const SkRect& logical_rect) : image_(std::move(image)), logical_rect_(logical_rect) {} @@ -41,10 +35,7 @@ RasterCache::RasterCache(size_t access_threshold, size_t picture_cache_limit_per_frame) : access_threshold_(access_threshold), picture_cache_limit_per_frame_(picture_cache_limit_per_frame), - checkerboard_images_(false), - weak_factory_(this) {} - -RasterCache::~RasterCache() = default; + checkerboard_images_(false) {} static bool CanRasterizePicture(SkPicture* picture) { if (picture == nullptr) { @@ -138,24 +129,12 @@ RasterCacheResult RasterizePicture(SkPicture* picture, [=](SkCanvas* canvas) { canvas->drawPicture(picture); }); } -static inline size_t ClampSize(size_t value, size_t min, size_t max) { - if (value > max) { - return max; - } - - if (value < min) { - return min; - } - - return value; -} - void 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 = ClampSize(entry.access_count + 1, 0, access_threshold_); + entry.access_count++; entry.used_this_frame = true; if (!entry.image.is_valid()) { entry.image = Rasterize( @@ -191,6 +170,10 @@ bool RasterCache::Prepare(GrContext* context, 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_) { return false; } @@ -210,11 +193,9 @@ bool RasterCache::Prepare(GrContext* context, PictureRasterCacheKey cache_key(picture->uniqueID(), transformation_matrix); + // Creates an entry, if not present prior. Entry& entry = picture_cache_[cache_key]; - entry.access_count = ClampSize(entry.access_count + 1, 0, access_threshold_); - entry.used_this_frame = true; - - if (entry.access_count < access_threshold_ || access_threshold_ == 0) { + if (entry.access_count < access_threshold_) { // Frame threshold has not yet been reached. return false; } @@ -231,20 +212,34 @@ RasterCacheResult RasterCache::Get(const SkPicture& picture, const SkMatrix& ctm) const { PictureRasterCacheKey cache_key(picture.uniqueID(), ctm); auto it = picture_cache_.find(cache_key); - return it == picture_cache_.end() ? RasterCacheResult() : it->second.image; + if (it == picture_cache_.end()) { + return RasterCacheResult(); + } + + Entry& entry = it->second; + entry.access_count++; + entry.used_this_frame = true; + + return entry.image; } RasterCacheResult RasterCache::Get(Layer* layer, const SkMatrix& ctm) const { LayerRasterCacheKey cache_key(layer->unique_id(), ctm); auto it = layer_cache_.find(cache_key); - return it == layer_cache_.end() ? RasterCacheResult() : it->second.image; + if (it == layer_cache_.end()) { + return RasterCacheResult(); + } + + Entry& entry = it->second; + entry.access_count++; + entry.used_this_frame = true; + + return entry.image; } void RasterCache::SweepAfterFrame() { - using PictureCache = PictureRasterCacheKey::Map; - using LayerCache = LayerRasterCacheKey::Map; - SweepOneCacheAfterFrame(picture_cache_); - SweepOneCacheAfterFrame(layer_cache_); + SweepOneCacheAfterFrame(picture_cache_); + SweepOneCacheAfterFrame(layer_cache_); picture_cached_this_frame_ = 0; TraceStatsToTimeline(); } diff --git a/flow/raster_cache.h b/flow/raster_cache.h index 3ea92588e1fd7..b51382a5c8d2d 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -19,11 +19,9 @@ namespace flutter { class RasterCacheResult { public: - RasterCacheResult(); + RasterCacheResult() = default; - RasterCacheResult(const RasterCacheResult& other); - - ~RasterCacheResult(); + RasterCacheResult(const RasterCacheResult& other) = default; RasterCacheResult(sk_sp image, const SkRect& logical_rect); @@ -56,8 +54,6 @@ class RasterCache { size_t access_threshold = 3, size_t picture_cache_limit_per_frame = kDefaultPictureCacheLimitPerFrame); - ~RasterCache(); - static SkIRect GetDeviceBounds(const SkRect& rect, const SkMatrix& ctm) { SkRect device_rect; ctm.mapRect(&device_rect, rect); @@ -109,9 +105,9 @@ class RasterCache { RasterCacheResult image; }; - template + template static void SweepOneCacheAfterFrame(Cache& cache) { - std::vector dead; + std::vector dead; for (auto it = cache.begin(); it != cache.end(); ++it) { Entry& entry = it->second; @@ -129,10 +125,9 @@ class RasterCache { const size_t access_threshold_; const size_t picture_cache_limit_per_frame_; size_t picture_cached_this_frame_ = 0; - PictureRasterCacheKey::Map picture_cache_; - LayerRasterCacheKey::Map layer_cache_; + mutable PictureRasterCacheKey::Map picture_cache_; + mutable LayerRasterCacheKey::Map layer_cache_; bool checkerboard_images_; - fml::WeakPtrFactory weak_factory_; void TraceStatsToTimeline() const; diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index bd83d807875f2..03e3b3879c469 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -30,7 +30,7 @@ TEST(RasterCache, SimpleInitialization) { } TEST(RasterCache, ThresholdIsRespected) { - size_t threshold = 3; + size_t threshold = 2; flutter::RasterCache cache(threshold); SkMatrix matrix = SkMatrix::I(); @@ -40,18 +40,28 @@ TEST(RasterCache, ThresholdIsRespected) { sk_sp image; sk_sp srgb = SkColorSpace::MakeSRGB(); - ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, - false)); // 1 - cache.SweepAfterFrame(); - ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, - false)); // 2 + ASSERT_FALSE( + cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false)); + // 1st access. + ASSERT_FALSE(cache.Get(*picture, matrix).is_valid()); + cache.SweepAfterFrame(); - ASSERT_TRUE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, - false)); // 3 + + ASSERT_FALSE( + cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false)); + + // 2st access. + ASSERT_FALSE(cache.Get(*picture, matrix).is_valid()); + 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()); } -TEST(RasterCache, ThresholdIsRespectedWhenZero) { +TEST(RasterCache, AccessThresholdOfZeroDisablesCaching) { size_t threshold = 0; flutter::RasterCache cache(threshold); @@ -62,19 +72,31 @@ TEST(RasterCache, ThresholdIsRespectedWhenZero) { sk_sp image; sk_sp srgb = SkColorSpace::MakeSRGB(); - ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, - false)); // 1 - cache.SweepAfterFrame(); - ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, - false)); // 2 - cache.SweepAfterFrame(); - ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, - false)); // 3 - cache.SweepAfterFrame(); + ASSERT_FALSE( + cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false)); + + ASSERT_FALSE(cache.Get(*picture, matrix).is_valid()); +} + +TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZero) { + size_t picture_cache_limit_per_frame = 0; + flutter::RasterCache cache(3, picture_cache_limit_per_frame); + + SkMatrix matrix = SkMatrix::I(); + + auto picture = GetSamplePicture(); + + sk_sp image; + + sk_sp srgb = SkColorSpace::MakeSRGB(); + ASSERT_FALSE( + cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false)); + + ASSERT_FALSE(cache.Get(*picture, matrix).is_valid()); } TEST(RasterCache, SweepsRemoveUnusedFrames) { - size_t threshold = 3; + size_t threshold = 1; flutter::RasterCache cache(threshold); SkMatrix matrix = SkMatrix::I(); @@ -86,19 +108,18 @@ TEST(RasterCache, SweepsRemoveUnusedFrames) { sk_sp srgb = SkColorSpace::MakeSRGB(); ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false)); // 1 + ASSERT_FALSE(cache.Get(*picture, matrix).is_valid()); + cache.SweepAfterFrame(); - ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, - false)); // 2 - cache.SweepAfterFrame(); - ASSERT_TRUE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, - false)); // 3 - cache.SweepAfterFrame(); + ASSERT_TRUE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, - false)); // 4 + false)); // 2 + ASSERT_TRUE(cache.Get(*picture, matrix).is_valid()); + cache.SweepAfterFrame(); - cache.SweepAfterFrame(); // Extra frame without a preroll image access. - ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, - false)); // 5 + cache.SweepAfterFrame(); // Extra frame without a Get image access. + + ASSERT_FALSE(cache.Get(*picture, matrix).is_valid()); } } // namespace testing