Skip to content

Commit af19ea7

Browse files
liyuqianliyuqian
authored
Replace RasterCache::Get with RasterCache:Draw (flutter#17791)
This avoids the possible matrix mismatch between RasterCache::Get and RasterCacheResult::draw. See flutter/engine#17790 for an example that tries to fix an earlier mismatch.
1 parent a544b45 commit af19ea7

File tree

6 files changed

+70
-51
lines changed

6 files changed

+70
-51
lines changed

‎flow/layers/image_filter_layer.cc‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,9 @@ void ImageFilterLayer::Paint(PaintContext& context) const {
4848
context.leaf_nodes_canvas->getTotalMatrix()));
4949
#endif
5050

51-
if (context.raster_cache) {
52-
const SkMatrix& ctm = context.leaf_nodes_canvas->getTotalMatrix();
53-
RasterCacheResult layer_cache =
54-
context.raster_cache->Get((Layer*)this, ctm);
55-
if (layer_cache.is_valid()) {
56-
layer_cache.draw(*context.leaf_nodes_canvas);
57-
return;
58-
}
51+
if (context.raster_cache &&
52+
context.raster_cache->Draw(this, *context.leaf_nodes_canvas)) {
53+
return;
5954
}
6055

6156
SkPaint paint;

‎flow/layers/opacity_layer.cc‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,10 @@ void OpacityLayer::Paint(PaintContext& context) const {
7474
context.leaf_nodes_canvas->getTotalMatrix()));
7575
#endif
7676

77-
if (context.raster_cache) {
78-
ContainerLayer* container = GetChildContainer();
79-
const SkMatrix& ctm = context.leaf_nodes_canvas->getTotalMatrix();
80-
RasterCacheResult child_cache = context.raster_cache->Get(container, ctm);
81-
if (child_cache.is_valid()) {
82-
child_cache.draw(*context.leaf_nodes_canvas, &paint);
83-
return;
84-
}
77+
if (context.raster_cache &&
78+
context.raster_cache->Draw(GetChildContainer(),
79+
*context.leaf_nodes_canvas, &paint)) {
80+
return;
8581
}
8682

8783
// Skia may clip the content with saveLayerBounds (although it's not a

‎flow/layers/picture_layer.cc‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,10 @@ void PictureLayer::Paint(PaintContext& context) const {
4949
context.leaf_nodes_canvas->getTotalMatrix()));
5050
#endif
5151

52-
if (context.raster_cache) {
53-
const SkMatrix& ctm = context.leaf_nodes_canvas->getTotalMatrix();
54-
RasterCacheResult result = context.raster_cache->Get(*picture(), ctm);
55-
if (result.is_valid()) {
56-
TRACE_EVENT_INSTANT0("flutter", "raster cache hit");
57-
58-
result.draw(*context.leaf_nodes_canvas);
59-
return;
60-
}
52+
if (context.raster_cache &&
53+
context.raster_cache->Draw(*picture(), *context.leaf_nodes_canvas)) {
54+
TRACE_EVENT_INSTANT0("flutter", "raster cache hit");
55+
return;
6156
}
6257
picture()->playback(context.leaf_nodes_canvas);
6358
}

‎flow/raster_cache.cc‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -210,33 +210,44 @@ bool RasterCache::Prepare(GrContext* context,
210210
return true;
211211
}
212212

213-
RasterCacheResult RasterCache::Get(const SkPicture& picture,
214-
const SkMatrix& ctm) const {
215-
PictureRasterCacheKey cache_key(picture.uniqueID(), ctm);
213+
bool RasterCache::Draw(const SkPicture& picture, SkCanvas& canvas) const {
214+
PictureRasterCacheKey cache_key(picture.uniqueID(), canvas.getTotalMatrix());
216215
auto it = picture_cache_.find(cache_key);
217216
if (it == picture_cache_.end()) {
218-
return RasterCacheResult();
217+
return false;
219218
}
220219

221220
Entry& entry = it->second;
222221
entry.access_count++;
223222
entry.used_this_frame = true;
224223

225-
return entry.image;
224+
if (entry.image.is_valid()) {
225+
entry.image.draw(canvas);
226+
return true;
227+
}
228+
229+
return false;
226230
}
227231

228-
RasterCacheResult RasterCache::Get(Layer* layer, const SkMatrix& ctm) const {
229-
LayerRasterCacheKey cache_key(layer->unique_id(), ctm);
232+
bool RasterCache::Draw(const Layer* layer,
233+
SkCanvas& canvas,
234+
SkPaint* paint) const {
235+
LayerRasterCacheKey cache_key(layer->unique_id(), canvas.getTotalMatrix());
230236
auto it = layer_cache_.find(cache_key);
231237
if (it == layer_cache_.end()) {
232-
return RasterCacheResult();
238+
return false;
233239
}
234240

235241
Entry& entry = it->second;
236242
entry.access_count++;
237243
entry.used_this_frame = true;
238244

239-
return entry.image;
245+
if (entry.image.is_valid()) {
246+
entry.image.draw(canvas, paint);
247+
return true;
248+
}
249+
250+
return false;
240251
}
241252

242253
void RasterCache::SweepAfterFrame() {

‎flow/raster_cache.h‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,20 @@ class RasterCache {
8686

8787
void Prepare(PrerollContext* context, Layer* layer, const SkMatrix& ctm);
8888

89-
RasterCacheResult Get(const SkPicture& picture, const SkMatrix& ctm) const;
89+
// Find the raster cache for the picture and draw it to the canvas.
90+
//
91+
// Return true if it's found and drawn.
92+
bool Draw(const SkPicture& picture, SkCanvas& canvas) const;
9093

91-
RasterCacheResult Get(Layer* layer, const SkMatrix& ctm) const;
94+
// Find the raster cache for the layer and draw it to the canvas.
95+
//
96+
// Addional paint can be given to change how the raster cache is drawn (e.g.,
97+
// draw the raster cache with some opacity).
98+
//
99+
// Return true if the layer raster cache is found and drawn.
100+
bool Draw(const Layer* layer,
101+
SkCanvas& canvas,
102+
SkPaint* paint = nullptr) const;
92103

93104
void SweepAfterFrame();
94105

‎flow/raster_cache_unittests.cc‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,26 +39,28 @@ TEST(RasterCache, ThresholdIsRespected) {
3939

4040
sk_sp<SkImage> image;
4141

42+
SkCanvas dummy_canvas;
43+
4244
sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
4345
ASSERT_FALSE(
4446
cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
4547
// 1st access.
46-
ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
48+
ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
4749

4850
cache.SweepAfterFrame();
4951

5052
ASSERT_FALSE(
5153
cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
5254

53-
// 2st access.
54-
ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
55+
// 2nd access.
56+
ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
5557

5658
cache.SweepAfterFrame();
5759

5860
// Now Prepare should cache it.
5961
ASSERT_TRUE(
6062
cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
61-
ASSERT_TRUE(cache.Get(*picture, matrix).is_valid());
63+
ASSERT_TRUE(cache.Draw(*picture, dummy_canvas));
6264
}
6365

6466
TEST(RasterCache, AccessThresholdOfZeroDisablesCaching) {
@@ -71,11 +73,13 @@ TEST(RasterCache, AccessThresholdOfZeroDisablesCaching) {
7173

7274
sk_sp<SkImage> image;
7375

76+
SkCanvas dummy_canvas;
77+
7478
sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
7579
ASSERT_FALSE(
7680
cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
7781

78-
ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
82+
ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
7983
}
8084

8185
TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZero) {
@@ -88,11 +92,13 @@ TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZero) {
8892

8993
sk_sp<SkImage> image;
9094

95+
SkCanvas dummy_canvas;
96+
9197
sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
9298
ASSERT_FALSE(
9399
cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
94100

95-
ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
101+
ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
96102
}
97103

98104
TEST(RasterCache, SweepsRemoveUnusedFrames) {
@@ -105,21 +111,23 @@ TEST(RasterCache, SweepsRemoveUnusedFrames) {
105111

106112
sk_sp<SkImage> image;
107113

114+
SkCanvas dummy_canvas;
115+
108116
sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
109117
ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
110118
false)); // 1
111-
ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
119+
ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
112120

113121
cache.SweepAfterFrame();
114122

115123
ASSERT_TRUE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
116124
false)); // 2
117-
ASSERT_TRUE(cache.Get(*picture, matrix).is_valid());
125+
ASSERT_TRUE(cache.Draw(*picture, dummy_canvas));
118126

119127
cache.SweepAfterFrame();
120128
cache.SweepAfterFrame(); // Extra frame without a Get image access.
121129

122-
ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
130+
ASSERT_FALSE(cache.Draw(*picture, dummy_canvas));
123131
}
124132

125133
// Construct a cache result whose device target rectangle rounds out to be one
@@ -139,19 +147,22 @@ TEST(RasterCache, DeviceRectRoundOut) {
139147

140148
SkMatrix ctm = SkMatrix::MakeAll(1.3312, 0, 233, 0, 1.3312, 206, 0, 0, 1);
141149

150+
SkCanvas canvas(100, 100, nullptr);
151+
canvas.setMatrix(ctm);
152+
142153
sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
143154
ASSERT_FALSE(
144155
cache.Prepare(NULL, picture.get(), ctm, srgb.get(), true, false));
145-
ASSERT_FALSE(cache.Get(*picture, ctm).is_valid());
156+
ASSERT_FALSE(cache.Draw(*picture, canvas));
146157
cache.SweepAfterFrame();
147158
ASSERT_TRUE(cache.Prepare(NULL, picture.get(), ctm, srgb.get(), true, false));
148-
ASSERT_TRUE(cache.Get(*picture, ctm).is_valid());
159+
ASSERT_TRUE(cache.Draw(*picture, canvas));
149160

150-
SkCanvas canvas(100, 100, nullptr);
151-
canvas.setMatrix(ctm);
152161
canvas.translate(248, 0);
153-
154-
cache.Get(*picture, ctm).draw(canvas);
162+
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
163+
canvas.setMatrix(RasterCache::GetIntegralTransCTM(canvas.getTotalMatrix()));
164+
#endif
165+
ASSERT_TRUE(cache.Draw(*picture, canvas));
155166
}
156167

157168
} // namespace testing

0 commit comments

Comments
 (0)