Skip to content

Commit df94213

Browse files
cbrackencbracken
authored
Revert "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)" (#16889)
This caused regression in several benchmarks, including: animated_placeholder_perf. Regression tracked in #51776. This reverts commit 01a52b9.
1 parent 5073cc7 commit df94213

File tree

3 files changed

+34
-64
lines changed

3 files changed

+34
-64
lines changed

‎flow/raster_cache.cc

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -119,15 +119,24 @@ static RasterCacheResult Rasterize(
119119
return {surface->makeImageSnapshot(), logical_rect};
120120
}
121121

122-
bool RasterCache::Prepare(PrerollContext* context,
122+
RasterCacheResult RasterizePicture(SkPicture* picture,
123+
GrContext* context,
124+
const SkMatrix& ctm,
125+
SkColorSpace* dst_color_space,
126+
bool checkerboard) {
127+
return Rasterize(context, ctm, dst_color_space, checkerboard,
128+
picture->cullRect(),
129+
[=](SkCanvas* canvas) { canvas->drawPicture(picture); });
130+
}
131+
132+
void RasterCache::Prepare(PrerollContext* context,
123133
Layer* layer,
124134
const SkMatrix& ctm) {
125135
LayerRasterCacheKey cache_key(layer->unique_id(), ctm);
126-
127136
Entry& entry = layer_cache_[cache_key];
128-
129-
if (!entry.did_rasterize && !entry.image.is_valid() &&
130-
entry.access_count >= access_threshold_) {
137+
entry.access_count++;
138+
entry.used_this_frame = true;
139+
if (!entry.image.is_valid()) {
131140
entry.image = Rasterize(
132141
context->gr_context, ctm, context->dst_color_space,
133142
checkerboard_images_, layer->paint_bounds(),
@@ -152,12 +161,7 @@ bool RasterCache::Prepare(PrerollContext* context,
152161
layer->Paint(paintContext);
153162
}
154163
});
155-
156-
entry.did_rasterize = true;
157-
158-
return true;
159164
}
160-
return false;
161165
}
162166

163167
bool RasterCache::Prepare(GrContext* context,
@@ -196,19 +200,12 @@ bool RasterCache::Prepare(GrContext* context,
196200
return false;
197201
}
198202

199-
// Don't try to rasterize pictures that were already attempted to be
200-
// rasterized even if the image is invalid.
201-
if (!entry.did_rasterize && !entry.image.is_valid()) {
202-
entry.image =
203-
Rasterize(context, transformation_matrix, dst_color_space,
204-
checkerboard_images_, picture->cullRect(),
205-
[=](SkCanvas* canvas) { canvas->drawPicture(picture); });
206-
207-
entry.did_rasterize = true;
203+
if (!entry.image.is_valid()) {
204+
entry.image = RasterizePicture(picture, context, transformation_matrix,
205+
dst_color_space, checkerboard_images_);
208206
picture_cached_this_frame_++;
209-
return true;
210207
}
211-
return false;
208+
return true;
212209
}
213210

214211
RasterCacheResult RasterCache::Get(const SkPicture& picture,

‎flow/raster_cache.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ class RasterCache {
8484
bool is_complex,
8585
bool will_change);
8686

87-
bool Prepare(PrerollContext* context, Layer* layer, const SkMatrix& ctm);
87+
void Prepare(PrerollContext* context, Layer* layer, const SkMatrix& ctm);
8888

8989
RasterCacheResult Get(const SkPicture& picture, const SkMatrix& ctm) const;
9090

@@ -101,7 +101,6 @@ class RasterCache {
101101
private:
102102
struct Entry {
103103
bool used_this_frame = false;
104-
bool did_rasterize = false;
105104
size_t access_count = 0;
106105
RasterCacheResult image;
107106
};

‎flow/raster_cache_unittests.cc

Lines changed: 15 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
#include "flutter/flow/raster_cache.h"
66

7-
#include "flutter/flow/layers/container_layer.h"
87
#include "gtest/gtest.h"
98
#include "third_party/skia/include/core/SkPicture.h"
109
#include "third_party/skia/include/core/SkPictureRecorder.h"
@@ -30,14 +29,16 @@ TEST(RasterCache, SimpleInitialization) {
3029
ASSERT_TRUE(true);
3130
}
3231

33-
TEST(RasterCache, ThresholdIsRespectedForPictures) {
32+
TEST(RasterCache, ThresholdIsRespected) {
3433
size_t threshold = 2;
3534
flutter::RasterCache cache(threshold);
3635

3736
SkMatrix matrix = SkMatrix::I();
3837

3938
auto picture = GetSamplePicture();
4039

40+
sk_sp<SkImage> image;
41+
4142
sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
4243
ASSERT_FALSE(
4344
cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
@@ -60,28 +61,21 @@ TEST(RasterCache, ThresholdIsRespectedForPictures) {
6061
ASSERT_TRUE(cache.Get(*picture, matrix).is_valid());
6162
}
6263

63-
TEST(RasterCache, ThresholdIsRespectedForLayers) {
64-
size_t threshold = 2;
64+
TEST(RasterCache, AccessThresholdOfZeroDisablesCaching) {
65+
size_t threshold = 0;
6566
flutter::RasterCache cache(threshold);
6667

6768
SkMatrix matrix = SkMatrix::I();
6869

69-
ContainerLayer layer;
70-
71-
sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
72-
ASSERT_FALSE(cache.Prepare(nullptr, &layer, matrix));
73-
ASSERT_FALSE(cache.Prepare(nullptr, &layer, matrix));
74-
ASSERT_FALSE(cache.Prepare(nullptr, &layer, matrix));
75-
76-
// 1st access.
77-
ASSERT_FALSE(cache.Get(&layer, matrix).is_valid());
70+
auto picture = GetSamplePicture();
7871

79-
ASSERT_FALSE(cache.Prepare(nullptr, &layer, matrix));
72+
sk_sp<SkImage> image;
8073

81-
// 2st access.
82-
ASSERT_FALSE(cache.Get(&layer, matrix).is_valid());
74+
sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
75+
ASSERT_FALSE(
76+
cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
8377

84-
// Calling Prepare now would crash due to the nullptr.
78+
ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
8579
}
8680

8781
TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZero) {
@@ -92,6 +86,8 @@ TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZero) {
9286

9387
auto picture = GetSamplePicture();
9488

89+
sk_sp<SkImage> image;
90+
9591
sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
9692
ASSERT_FALSE(
9793
cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
@@ -107,6 +103,8 @@ TEST(RasterCache, SweepsRemoveUnusedFrames) {
107103

108104
auto picture = GetSamplePicture();
109105

106+
sk_sp<SkImage> image;
107+
110108
sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
111109
ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
112110
false)); // 1
@@ -124,29 +122,5 @@ TEST(RasterCache, SweepsRemoveUnusedFrames) {
124122
ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
125123
}
126124

127-
TEST(RasterCache, TryRasterizngOnlyOnce) {
128-
size_t threshold = 1;
129-
flutter::RasterCache cache(threshold);
130-
131-
SkMatrix matrix = SkMatrix::I();
132-
// Test picture too large to successfully rasterize.
133-
auto picture = SkPicture::MakePlaceholder(SkRect::MakeWH(2e12, 2e12));
134-
135-
sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
136-
ASSERT_FALSE(cache.Prepare(nullptr, picture.get(), matrix, srgb.get(), true,
137-
false)); // 1
138-
ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
139-
140-
// Rasterization ran, though Get() below returns an invalid image.
141-
ASSERT_TRUE(cache.Prepare(nullptr, picture.get(), matrix, srgb.get(), true,
142-
false)); // 2
143-
ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
144-
145-
// This time we should not try again to rasterize.
146-
ASSERT_FALSE(cache.Prepare(nullptr, picture.get(), matrix, srgb.get(), true,
147-
false)); // 2
148-
ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
149-
}
150-
151125
} // namespace testing
152126
} // namespace flutter

0 commit comments

Comments
 (0)