Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 99f8d00

Browse files
authored
Remove layer integral offset snapping (#17712)
This fixes flutter/flutter#53288 and flutter/flutter#41654. It removes the problematic `GetIntegralTransCTM`, but preserves the rect round-out in `RasterCacheResult::draw` for performance considerations: the average frame raster time doesn't change much but the worst frame raster time significantly regressed if rect round-out is removed. That's probably because a new shader needs to be compiled to draw raster cache with fractional offsets.
1 parent f4d6ce1 commit 99f8d00

8 files changed

+33
-134
lines changed

flow/layers/image_filter_layer.cc

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,6 @@ void ImageFilterLayer::Preroll(PrerollContext* context,
3131
if (!context->has_platform_view && context->raster_cache &&
3232
SkRect::Intersects(context->cull_rect, paint_bounds())) {
3333
SkMatrix ctm = matrix;
34-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
35-
ctm = RasterCache::GetIntegralTransCTM(ctm);
36-
#endif
3734
context->raster_cache->Prepare(context, this, ctm);
3835
}
3936
}
@@ -42,12 +39,6 @@ void ImageFilterLayer::Paint(PaintContext& context) const {
4239
TRACE_EVENT0("flutter", "ImageFilterLayer::Paint");
4340
FML_DCHECK(needs_painting());
4441

45-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
46-
SkAutoCanvasRestore save(context.leaf_nodes_canvas, true);
47-
context.leaf_nodes_canvas->setMatrix(RasterCache::GetIntegralTransCTM(
48-
context.leaf_nodes_canvas->getTotalMatrix()));
49-
#endif
50-
5142
if (context.raster_cache) {
5243
const SkMatrix& ctm = context.leaf_nodes_canvas->getTotalMatrix();
5344
RasterCacheResult layer_cache =

flow/layers/image_filter_layer_unittests.cc

Lines changed: 25 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,11 @@ TEST_F(ImageFilterLayerTest, EmptyFilter) {
6060
layer->Paint(paint_context());
6161
EXPECT_EQ(mock_canvas().draw_calls(),
6262
std::vector({
63-
MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
64-
MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}},
6563
MockCanvas::DrawCall{
66-
1, MockCanvas::SaveLayerData{child_bounds, filter_paint,
67-
nullptr, 2}},
64+
0, MockCanvas::SaveLayerData{child_bounds, filter_paint,
65+
nullptr, 1}},
6866
MockCanvas::DrawCall{
69-
2, MockCanvas::DrawPathData{child_path, child_paint}},
70-
MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
67+
1, MockCanvas::DrawPathData{child_path, child_paint}},
7168
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}},
7269
}));
7370
}
@@ -96,14 +93,11 @@ TEST_F(ImageFilterLayerTest, SimpleFilter) {
9693
layer->Paint(paint_context());
9794
EXPECT_EQ(mock_canvas().draw_calls(),
9895
std::vector({
99-
MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
100-
MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}},
10196
MockCanvas::DrawCall{
102-
1, MockCanvas::SaveLayerData{child_bounds, filter_paint,
103-
nullptr, 2}},
97+
0, MockCanvas::SaveLayerData{child_bounds, filter_paint,
98+
nullptr, 1}},
10499
MockCanvas::DrawCall{
105-
2, MockCanvas::DrawPathData{child_path, child_paint}},
106-
MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
100+
1, MockCanvas::DrawPathData{child_path, child_paint}},
107101
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}},
108102
}));
109103
}
@@ -132,14 +126,11 @@ TEST_F(ImageFilterLayerTest, SimpleFilterBounds) {
132126
layer->Paint(paint_context());
133127
EXPECT_EQ(mock_canvas().draw_calls(),
134128
std::vector({
135-
MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
136-
MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}},
137129
MockCanvas::DrawCall{
138-
1, MockCanvas::SaveLayerData{child_bounds, filter_paint,
139-
nullptr, 2}},
130+
0, MockCanvas::SaveLayerData{child_bounds, filter_paint,
131+
nullptr, 1}},
140132
MockCanvas::DrawCall{
141-
2, MockCanvas::DrawPathData{child_path, child_paint}},
142-
MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
133+
1, MockCanvas::DrawPathData{child_path, child_paint}},
143134
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}},
144135
}));
145136
}
@@ -177,19 +168,16 @@ TEST_F(ImageFilterLayerTest, MultipleChildren) {
177168
SkPaint filter_paint;
178169
filter_paint.setImageFilter(layer_filter);
179170
layer->Paint(paint_context());
180-
EXPECT_EQ(mock_canvas().draw_calls(),
181-
std::vector(
182-
{MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
183-
MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}},
184-
MockCanvas::DrawCall{
185-
1, MockCanvas::SaveLayerData{children_bounds, filter_paint,
186-
nullptr, 2}},
187-
MockCanvas::DrawCall{
188-
2, MockCanvas::DrawPathData{child_path1, child_paint1}},
189-
MockCanvas::DrawCall{
190-
2, MockCanvas::DrawPathData{child_path2, child_paint2}},
191-
MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
192-
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}}));
171+
EXPECT_EQ(
172+
mock_canvas().draw_calls(),
173+
std::vector({MockCanvas::DrawCall{
174+
0, MockCanvas::SaveLayerData{children_bounds,
175+
filter_paint, nullptr, 1}},
176+
MockCanvas::DrawCall{
177+
1, MockCanvas::DrawPathData{child_path1, child_paint1}},
178+
MockCanvas::DrawCall{
179+
1, MockCanvas::DrawPathData{child_path2, child_paint2}},
180+
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}}));
193181
}
194182

195183
TEST_F(ImageFilterLayerTest, Nested) {
@@ -237,22 +225,16 @@ TEST_F(ImageFilterLayerTest, Nested) {
237225
layer1->Paint(paint_context());
238226
EXPECT_EQ(mock_canvas().draw_calls(),
239227
std::vector({
240-
MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
241-
MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}},
242228
MockCanvas::DrawCall{
243-
1, MockCanvas::SaveLayerData{children_bounds, filter_paint1,
244-
nullptr, 2}},
229+
0, MockCanvas::SaveLayerData{children_bounds, filter_paint1,
230+
nullptr, 1}},
245231
MockCanvas::DrawCall{
246-
2, MockCanvas::DrawPathData{child_path1, child_paint1}},
247-
MockCanvas::DrawCall{2, MockCanvas::SaveData{3}},
248-
MockCanvas::DrawCall{3, MockCanvas::SetMatrixData{SkMatrix()}},
232+
1, MockCanvas::DrawPathData{child_path1, child_paint1}},
249233
MockCanvas::DrawCall{
250-
3, MockCanvas::SaveLayerData{child_path2.getBounds(),
251-
filter_paint2, nullptr, 4}},
234+
1, MockCanvas::SaveLayerData{child_path2.getBounds(),
235+
filter_paint2, nullptr, 2}},
252236
MockCanvas::DrawCall{
253-
4, MockCanvas::DrawPathData{child_path2, child_paint2}},
254-
MockCanvas::DrawCall{4, MockCanvas::RestoreData{3}},
255-
MockCanvas::DrawCall{3, MockCanvas::RestoreData{2}},
237+
2, MockCanvas::DrawPathData{child_path2, child_paint2}},
256238
MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
257239
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}},
258240
}));

flow/layers/opacity_layer.cc

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,6 @@ void OpacityLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
5151
if (!context->has_platform_view && context->raster_cache &&
5252
SkRect::Intersects(context->cull_rect, paint_bounds())) {
5353
SkMatrix ctm = child_matrix;
54-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
55-
ctm = RasterCache::GetIntegralTransCTM(ctm);
56-
#endif
5754
context->raster_cache->Prepare(context, container, ctm);
5855
}
5956
}
@@ -69,11 +66,6 @@ void OpacityLayer::Paint(PaintContext& context) const {
6966
SkAutoCanvasRestore save(context.internal_nodes_canvas, true);
7067
context.internal_nodes_canvas->translate(offset_.fX, offset_.fY);
7168

72-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
73-
context.internal_nodes_canvas->setMatrix(RasterCache::GetIntegralTransCTM(
74-
context.leaf_nodes_canvas->getTotalMatrix()));
75-
#endif
76-
7769
if (context.raster_cache) {
7870
ContainerLayer* container = GetChildContainer();
7971
const SkMatrix& ctm = context.leaf_nodes_canvas->getTotalMatrix();
@@ -87,8 +79,7 @@ void OpacityLayer::Paint(PaintContext& context) const {
8779
// Skia may clip the content with saveLayerBounds (although it's not a
8880
// guaranteed clip). So we have to provide a big enough saveLayerBounds. To do
8981
// so, we first remove the offset from paint bounds since it's already in the
90-
// matrix. Then we round out the bounds because of our
91-
// RasterCache::GetIntegralTransCTM optimization.
82+
// matrix. Then we round out the bounds.
9283
//
9384
// Note that the following lines are only accessible when the raster cache is
9485
// not available (e.g., when we're using the software backend in golden

flow/layers/opacity_layer_unittests.cc

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,6 @@ TEST_F(OpacityLayerTest, FullyOpaque) {
5858
const SkMatrix initial_transform = SkMatrix::MakeTrans(0.5f, 0.5f);
5959
const SkMatrix layer_transform =
6060
SkMatrix::MakeTrans(layer_offset.fX, layer_offset.fY);
61-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
62-
const SkMatrix integral_layer_transform = RasterCache::GetIntegralTransCTM(
63-
SkMatrix::Concat(initial_transform, layer_transform));
64-
#endif
6561
const SkPaint child_paint = SkPaint(SkColors::kGreen);
6662
const SkRect expected_layer_bounds =
6763
layer_transform.mapRect(child_path.getBounds());
@@ -86,10 +82,6 @@ TEST_F(OpacityLayerTest, FullyOpaque) {
8682
auto expected_draw_calls = std::vector(
8783
{MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
8884
MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer_transform}},
89-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
90-
MockCanvas::DrawCall{
91-
1, MockCanvas::SetMatrixData{integral_layer_transform}},
92-
#endif
9385
MockCanvas::DrawCall{
9486
1, MockCanvas::SaveLayerData{opacity_bounds, opacity_paint, nullptr,
9587
2}},
@@ -107,10 +99,6 @@ TEST_F(OpacityLayerTest, FullyTransparent) {
10799
const SkMatrix initial_transform = SkMatrix::MakeTrans(0.5f, 0.5f);
108100
const SkMatrix layer_transform =
109101
SkMatrix::MakeTrans(layer_offset.fX, layer_offset.fY);
110-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
111-
const SkMatrix integral_layer_transform = RasterCache::GetIntegralTransCTM(
112-
SkMatrix::Concat(initial_transform, layer_transform));
113-
#endif
114102
const SkPaint child_paint = SkPaint(SkColors::kGreen);
115103
const SkRect expected_layer_bounds =
116104
layer_transform.mapRect(child_path.getBounds());
@@ -133,10 +121,6 @@ TEST_F(OpacityLayerTest, FullyTransparent) {
133121
auto expected_draw_calls = std::vector(
134122
{MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
135123
MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer_transform}},
136-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
137-
MockCanvas::DrawCall{
138-
1, MockCanvas::SetMatrixData{integral_layer_transform}},
139-
#endif
140124
MockCanvas::DrawCall{1, MockCanvas::SaveData{2}},
141125
MockCanvas::DrawCall{
142126
2, MockCanvas::ClipRectData{kEmptyRect, SkClipOp::kIntersect,
@@ -155,10 +139,6 @@ TEST_F(OpacityLayerTest, HalfTransparent) {
155139
const SkMatrix initial_transform = SkMatrix::MakeTrans(0.5f, 0.5f);
156140
const SkMatrix layer_transform =
157141
SkMatrix::MakeTrans(layer_offset.fX, layer_offset.fY);
158-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
159-
const SkMatrix integral_layer_transform = RasterCache::GetIntegralTransCTM(
160-
SkMatrix::Concat(initial_transform, layer_transform));
161-
#endif
162142
const SkPaint child_paint = SkPaint(SkColors::kGreen);
163143
const SkRect expected_layer_bounds =
164144
layer_transform.mapRect(child_path.getBounds());
@@ -185,10 +165,6 @@ TEST_F(OpacityLayerTest, HalfTransparent) {
185165
auto expected_draw_calls = std::vector(
186166
{MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
187167
MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer_transform}},
188-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
189-
MockCanvas::DrawCall{
190-
1, MockCanvas::SetMatrixData{integral_layer_transform}},
191-
#endif
192168
MockCanvas::DrawCall{
193169
1, MockCanvas::SaveLayerData{opacity_bounds, opacity_paint, nullptr,
194170
2}},
@@ -211,13 +187,6 @@ TEST_F(OpacityLayerTest, Nested) {
211187
SkMatrix::MakeTrans(layer1_offset.fX, layer1_offset.fY);
212188
const SkMatrix layer2_transform =
213189
SkMatrix::MakeTrans(layer2_offset.fX, layer2_offset.fY);
214-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
215-
const SkMatrix integral_layer1_transform = RasterCache::GetIntegralTransCTM(
216-
SkMatrix::Concat(initial_transform, layer1_transform));
217-
const SkMatrix integral_layer2_transform = RasterCache::GetIntegralTransCTM(
218-
SkMatrix::Concat(SkMatrix::Concat(initial_transform, layer1_transform),
219-
layer2_transform));
220-
#endif
221190
const SkPaint child1_paint = SkPaint(SkColors::kRed);
222191
const SkPaint child2_paint = SkPaint(SkColors::kBlue);
223192
const SkPaint child3_paint = SkPaint(SkColors::kGreen);
@@ -278,21 +247,13 @@ TEST_F(OpacityLayerTest, Nested) {
278247
auto expected_draw_calls = std::vector(
279248
{MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
280249
MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer1_transform}},
281-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
282-
MockCanvas::DrawCall{
283-
1, MockCanvas::SetMatrixData{integral_layer1_transform}},
284-
#endif
285250
MockCanvas::DrawCall{
286251
1, MockCanvas::SaveLayerData{opacity1_bounds, opacity1_paint,
287252
nullptr, 2}},
288253
MockCanvas::DrawCall{
289254
2, MockCanvas::DrawPathData{child1_path, child1_paint}},
290255
MockCanvas::DrawCall{2, MockCanvas::SaveData{3}},
291256
MockCanvas::DrawCall{3, MockCanvas::ConcatMatrixData{layer2_transform}},
292-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
293-
MockCanvas::DrawCall{
294-
3, MockCanvas::SetMatrixData{integral_layer2_transform}},
295-
#endif
296257
MockCanvas::DrawCall{
297258
3, MockCanvas::SaveLayerData{opacity2_bounds, opacity2_paint,
298259
nullptr, 4}},

flow/layers/picture_layer.cc

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,6 @@ void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
2626

2727
SkMatrix ctm = matrix;
2828
ctm.postTranslate(offset_.x(), offset_.y());
29-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
30-
ctm = RasterCache::GetIntegralTransCTM(ctm);
31-
#endif
3229
cache->Prepare(context->gr_context, sk_picture, ctm,
3330
context->dst_color_space, is_complex_, will_change_);
3431
}
@@ -44,10 +41,6 @@ void PictureLayer::Paint(PaintContext& context) const {
4441

4542
SkAutoCanvasRestore save(context.leaf_nodes_canvas, true);
4643
context.leaf_nodes_canvas->translate(offset_.x(), offset_.y());
47-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
48-
context.leaf_nodes_canvas->setMatrix(RasterCache::GetIntegralTransCTM(
49-
context.leaf_nodes_canvas->getTotalMatrix()));
50-
#endif
5144

5245
if (context.raster_cache) {
5346
const SkMatrix& ctm = context.leaf_nodes_canvas->getTotalMatrix();

flow/layers/picture_layer_unittests.cc

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,6 @@
1111
#include "flutter/testing/mock_canvas.h"
1212
#include "third_party/skia/include/core/SkPicture.h"
1313

14-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
15-
#include "flutter/flow/raster_cache.h"
16-
#endif
17-
1814
namespace flutter {
1915
namespace testing {
2016

@@ -85,16 +81,11 @@ TEST_F(PictureLayerTest, SimplePicture) {
8581
EXPECT_FALSE(layer->needs_system_composite());
8682

8783
layer->Paint(paint_context());
88-
auto expected_draw_calls = std::vector(
89-
{MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
90-
MockCanvas::DrawCall{1,
91-
MockCanvas::ConcatMatrixData{layer_offset_matrix}},
92-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
93-
MockCanvas::DrawCall{
94-
1, MockCanvas::SetMatrixData{RasterCache::GetIntegralTransCTM(
95-
layer_offset_matrix)}},
96-
#endif
97-
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}});
84+
auto expected_draw_calls =
85+
std::vector({MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
86+
MockCanvas::DrawCall{
87+
1, MockCanvas::ConcatMatrixData{layer_offset_matrix}},
88+
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}});
9889
EXPECT_EQ(mock_canvas().draw_calls(), expected_draw_calls);
9990
}
10091

flow/raster_cache.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,6 @@ class RasterCache {
6262
return bounds;
6363
}
6464

65-
static SkMatrix GetIntegralTransCTM(const SkMatrix& ctm) {
66-
SkMatrix result = ctm;
67-
result[SkMatrix::kMTransX] = SkScalarRoundToScalar(ctm.getTranslateX());
68-
result[SkMatrix::kMTransY] = SkScalarRoundToScalar(ctm.getTranslateY());
69-
return result;
70-
}
71-
7265
// Return true if the cache is generated.
7366
//
7467
// We may return false and not generate the cache if

flow/raster_cache_key.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,8 @@ template <typename ID>
1515
class RasterCacheKey {
1616
public:
1717
RasterCacheKey(ID id, const SkMatrix& ctm) : id_(id), matrix_(ctm) {
18-
matrix_[SkMatrix::kMTransX] = SkScalarFraction(ctm.getTranslateX());
19-
matrix_[SkMatrix::kMTransY] = SkScalarFraction(ctm.getTranslateY());
20-
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
21-
FML_DCHECK(matrix_.getTranslateX() == 0 && matrix_.getTranslateY() == 0);
22-
#endif
18+
matrix_[SkMatrix::kMTransX] = 0;
19+
matrix_[SkMatrix::kMTransY] = 0;
2320
}
2421

2522
ID id() const { return id_; }

0 commit comments

Comments
 (0)