From 99f8d007aaefcf2cb0a0a40c0b3c6a6fd7556f5c Mon Sep 17 00:00:00 2001 From: liyuqian Date: Thu, 16 Apr 2020 15:41:07 -0700 Subject: [PATCH] Remove layer integral offset snapping (#17712) This fixes https://github.com/flutter/flutter/issues/53288 and https://github.com/flutter/flutter/issues/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. --- flow/layers/image_filter_layer.cc | 9 --- flow/layers/image_filter_layer_unittests.cc | 68 ++++++++------------- flow/layers/opacity_layer.cc | 11 +--- flow/layers/opacity_layer_unittests.cc | 39 ------------ flow/layers/picture_layer.cc | 7 --- flow/layers/picture_layer_unittests.cc | 19 ++---- flow/raster_cache.h | 7 --- flow/raster_cache_key.h | 7 +-- 8 files changed, 33 insertions(+), 134 deletions(-) diff --git a/flow/layers/image_filter_layer.cc b/flow/layers/image_filter_layer.cc index 212c396b6efbf..4df19eb66416b 100644 --- a/flow/layers/image_filter_layer.cc +++ b/flow/layers/image_filter_layer.cc @@ -31,9 +31,6 @@ void ImageFilterLayer::Preroll(PrerollContext* context, if (!context->has_platform_view && context->raster_cache && SkRect::Intersects(context->cull_rect, paint_bounds())) { SkMatrix ctm = matrix; -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - ctm = RasterCache::GetIntegralTransCTM(ctm); -#endif context->raster_cache->Prepare(context, this, ctm); } } @@ -42,12 +39,6 @@ void ImageFilterLayer::Paint(PaintContext& context) const { TRACE_EVENT0("flutter", "ImageFilterLayer::Paint"); FML_DCHECK(needs_painting()); -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - SkAutoCanvasRestore save(context.leaf_nodes_canvas, true); - context.leaf_nodes_canvas->setMatrix(RasterCache::GetIntegralTransCTM( - context.leaf_nodes_canvas->getTotalMatrix())); -#endif - if (context.raster_cache) { const SkMatrix& ctm = context.leaf_nodes_canvas->getTotalMatrix(); RasterCacheResult layer_cache = diff --git a/flow/layers/image_filter_layer_unittests.cc b/flow/layers/image_filter_layer_unittests.cc index f4f621a529a08..305ec01611068 100644 --- a/flow/layers/image_filter_layer_unittests.cc +++ b/flow/layers/image_filter_layer_unittests.cc @@ -60,14 +60,11 @@ TEST_F(ImageFilterLayerTest, EmptyFilter) { layer->Paint(paint_context()); EXPECT_EQ(mock_canvas().draw_calls(), std::vector({ - MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, - MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}}, MockCanvas::DrawCall{ - 1, MockCanvas::SaveLayerData{child_bounds, filter_paint, - nullptr, 2}}, + 0, MockCanvas::SaveLayerData{child_bounds, filter_paint, + nullptr, 1}}, MockCanvas::DrawCall{ - 2, MockCanvas::DrawPathData{child_path, child_paint}}, - MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}}, + 1, MockCanvas::DrawPathData{child_path, child_paint}}, MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}, })); } @@ -96,14 +93,11 @@ TEST_F(ImageFilterLayerTest, SimpleFilter) { layer->Paint(paint_context()); EXPECT_EQ(mock_canvas().draw_calls(), std::vector({ - MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, - MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}}, MockCanvas::DrawCall{ - 1, MockCanvas::SaveLayerData{child_bounds, filter_paint, - nullptr, 2}}, + 0, MockCanvas::SaveLayerData{child_bounds, filter_paint, + nullptr, 1}}, MockCanvas::DrawCall{ - 2, MockCanvas::DrawPathData{child_path, child_paint}}, - MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}}, + 1, MockCanvas::DrawPathData{child_path, child_paint}}, MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}, })); } @@ -132,14 +126,11 @@ TEST_F(ImageFilterLayerTest, SimpleFilterBounds) { layer->Paint(paint_context()); EXPECT_EQ(mock_canvas().draw_calls(), std::vector({ - MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, - MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}}, MockCanvas::DrawCall{ - 1, MockCanvas::SaveLayerData{child_bounds, filter_paint, - nullptr, 2}}, + 0, MockCanvas::SaveLayerData{child_bounds, filter_paint, + nullptr, 1}}, MockCanvas::DrawCall{ - 2, MockCanvas::DrawPathData{child_path, child_paint}}, - MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}}, + 1, MockCanvas::DrawPathData{child_path, child_paint}}, MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}, })); } @@ -177,19 +168,16 @@ TEST_F(ImageFilterLayerTest, MultipleChildren) { SkPaint filter_paint; filter_paint.setImageFilter(layer_filter); layer->Paint(paint_context()); - EXPECT_EQ(mock_canvas().draw_calls(), - std::vector( - {MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, - MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}}, - MockCanvas::DrawCall{ - 1, MockCanvas::SaveLayerData{children_bounds, filter_paint, - nullptr, 2}}, - MockCanvas::DrawCall{ - 2, MockCanvas::DrawPathData{child_path1, child_paint1}}, - MockCanvas::DrawCall{ - 2, MockCanvas::DrawPathData{child_path2, child_paint2}}, - MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}}, - MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}})); + EXPECT_EQ( + mock_canvas().draw_calls(), + std::vector({MockCanvas::DrawCall{ + 0, MockCanvas::SaveLayerData{children_bounds, + filter_paint, nullptr, 1}}, + MockCanvas::DrawCall{ + 1, MockCanvas::DrawPathData{child_path1, child_paint1}}, + MockCanvas::DrawCall{ + 1, MockCanvas::DrawPathData{child_path2, child_paint2}}, + MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}})); } TEST_F(ImageFilterLayerTest, Nested) { @@ -237,22 +225,16 @@ TEST_F(ImageFilterLayerTest, Nested) { layer1->Paint(paint_context()); EXPECT_EQ(mock_canvas().draw_calls(), std::vector({ - MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, - MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}}, MockCanvas::DrawCall{ - 1, MockCanvas::SaveLayerData{children_bounds, filter_paint1, - nullptr, 2}}, + 0, MockCanvas::SaveLayerData{children_bounds, filter_paint1, + nullptr, 1}}, MockCanvas::DrawCall{ - 2, MockCanvas::DrawPathData{child_path1, child_paint1}}, - MockCanvas::DrawCall{2, MockCanvas::SaveData{3}}, - MockCanvas::DrawCall{3, MockCanvas::SetMatrixData{SkMatrix()}}, + 1, MockCanvas::DrawPathData{child_path1, child_paint1}}, MockCanvas::DrawCall{ - 3, MockCanvas::SaveLayerData{child_path2.getBounds(), - filter_paint2, nullptr, 4}}, + 1, MockCanvas::SaveLayerData{child_path2.getBounds(), + filter_paint2, nullptr, 2}}, MockCanvas::DrawCall{ - 4, MockCanvas::DrawPathData{child_path2, child_paint2}}, - MockCanvas::DrawCall{4, MockCanvas::RestoreData{3}}, - MockCanvas::DrawCall{3, MockCanvas::RestoreData{2}}, + 2, MockCanvas::DrawPathData{child_path2, child_paint2}}, MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}}, MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}, })); diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc index 7899c31784b89..51fcc6c507fcb 100644 --- a/flow/layers/opacity_layer.cc +++ b/flow/layers/opacity_layer.cc @@ -51,9 +51,6 @@ void OpacityLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { if (!context->has_platform_view && context->raster_cache && SkRect::Intersects(context->cull_rect, paint_bounds())) { SkMatrix ctm = child_matrix; -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - ctm = RasterCache::GetIntegralTransCTM(ctm); -#endif context->raster_cache->Prepare(context, container, ctm); } } @@ -69,11 +66,6 @@ void OpacityLayer::Paint(PaintContext& context) const { SkAutoCanvasRestore save(context.internal_nodes_canvas, true); context.internal_nodes_canvas->translate(offset_.fX, offset_.fY); -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - context.internal_nodes_canvas->setMatrix(RasterCache::GetIntegralTransCTM( - context.leaf_nodes_canvas->getTotalMatrix())); -#endif - if (context.raster_cache) { ContainerLayer* container = GetChildContainer(); const SkMatrix& ctm = context.leaf_nodes_canvas->getTotalMatrix(); @@ -87,8 +79,7 @@ void OpacityLayer::Paint(PaintContext& context) const { // Skia may clip the content with saveLayerBounds (although it's not a // guaranteed clip). So we have to provide a big enough saveLayerBounds. To do // so, we first remove the offset from paint bounds since it's already in the - // matrix. Then we round out the bounds because of our - // RasterCache::GetIntegralTransCTM optimization. + // matrix. Then we round out the bounds. // // Note that the following lines are only accessible when the raster cache is // not available (e.g., when we're using the software backend in golden diff --git a/flow/layers/opacity_layer_unittests.cc b/flow/layers/opacity_layer_unittests.cc index f7cc77ffe2ddd..ae08109fa25cf 100644 --- a/flow/layers/opacity_layer_unittests.cc +++ b/flow/layers/opacity_layer_unittests.cc @@ -58,10 +58,6 @@ TEST_F(OpacityLayerTest, FullyOpaque) { const SkMatrix initial_transform = SkMatrix::MakeTrans(0.5f, 0.5f); const SkMatrix layer_transform = SkMatrix::MakeTrans(layer_offset.fX, layer_offset.fY); -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - const SkMatrix integral_layer_transform = RasterCache::GetIntegralTransCTM( - SkMatrix::Concat(initial_transform, layer_transform)); -#endif const SkPaint child_paint = SkPaint(SkColors::kGreen); const SkRect expected_layer_bounds = layer_transform.mapRect(child_path.getBounds()); @@ -86,10 +82,6 @@ TEST_F(OpacityLayerTest, FullyOpaque) { auto expected_draw_calls = std::vector( {MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer_transform}}, -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - MockCanvas::DrawCall{ - 1, MockCanvas::SetMatrixData{integral_layer_transform}}, -#endif MockCanvas::DrawCall{ 1, MockCanvas::SaveLayerData{opacity_bounds, opacity_paint, nullptr, 2}}, @@ -107,10 +99,6 @@ TEST_F(OpacityLayerTest, FullyTransparent) { const SkMatrix initial_transform = SkMatrix::MakeTrans(0.5f, 0.5f); const SkMatrix layer_transform = SkMatrix::MakeTrans(layer_offset.fX, layer_offset.fY); -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - const SkMatrix integral_layer_transform = RasterCache::GetIntegralTransCTM( - SkMatrix::Concat(initial_transform, layer_transform)); -#endif const SkPaint child_paint = SkPaint(SkColors::kGreen); const SkRect expected_layer_bounds = layer_transform.mapRect(child_path.getBounds()); @@ -133,10 +121,6 @@ TEST_F(OpacityLayerTest, FullyTransparent) { auto expected_draw_calls = std::vector( {MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer_transform}}, -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - MockCanvas::DrawCall{ - 1, MockCanvas::SetMatrixData{integral_layer_transform}}, -#endif MockCanvas::DrawCall{1, MockCanvas::SaveData{2}}, MockCanvas::DrawCall{ 2, MockCanvas::ClipRectData{kEmptyRect, SkClipOp::kIntersect, @@ -155,10 +139,6 @@ TEST_F(OpacityLayerTest, HalfTransparent) { const SkMatrix initial_transform = SkMatrix::MakeTrans(0.5f, 0.5f); const SkMatrix layer_transform = SkMatrix::MakeTrans(layer_offset.fX, layer_offset.fY); -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - const SkMatrix integral_layer_transform = RasterCache::GetIntegralTransCTM( - SkMatrix::Concat(initial_transform, layer_transform)); -#endif const SkPaint child_paint = SkPaint(SkColors::kGreen); const SkRect expected_layer_bounds = layer_transform.mapRect(child_path.getBounds()); @@ -185,10 +165,6 @@ TEST_F(OpacityLayerTest, HalfTransparent) { auto expected_draw_calls = std::vector( {MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer_transform}}, -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - MockCanvas::DrawCall{ - 1, MockCanvas::SetMatrixData{integral_layer_transform}}, -#endif MockCanvas::DrawCall{ 1, MockCanvas::SaveLayerData{opacity_bounds, opacity_paint, nullptr, 2}}, @@ -211,13 +187,6 @@ TEST_F(OpacityLayerTest, Nested) { SkMatrix::MakeTrans(layer1_offset.fX, layer1_offset.fY); const SkMatrix layer2_transform = SkMatrix::MakeTrans(layer2_offset.fX, layer2_offset.fY); -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - const SkMatrix integral_layer1_transform = RasterCache::GetIntegralTransCTM( - SkMatrix::Concat(initial_transform, layer1_transform)); - const SkMatrix integral_layer2_transform = RasterCache::GetIntegralTransCTM( - SkMatrix::Concat(SkMatrix::Concat(initial_transform, layer1_transform), - layer2_transform)); -#endif const SkPaint child1_paint = SkPaint(SkColors::kRed); const SkPaint child2_paint = SkPaint(SkColors::kBlue); const SkPaint child3_paint = SkPaint(SkColors::kGreen); @@ -278,10 +247,6 @@ TEST_F(OpacityLayerTest, Nested) { auto expected_draw_calls = std::vector( {MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer1_transform}}, -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - MockCanvas::DrawCall{ - 1, MockCanvas::SetMatrixData{integral_layer1_transform}}, -#endif MockCanvas::DrawCall{ 1, MockCanvas::SaveLayerData{opacity1_bounds, opacity1_paint, nullptr, 2}}, @@ -289,10 +254,6 @@ TEST_F(OpacityLayerTest, Nested) { 2, MockCanvas::DrawPathData{child1_path, child1_paint}}, MockCanvas::DrawCall{2, MockCanvas::SaveData{3}}, MockCanvas::DrawCall{3, MockCanvas::ConcatMatrixData{layer2_transform}}, -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - MockCanvas::DrawCall{ - 3, MockCanvas::SetMatrixData{integral_layer2_transform}}, -#endif MockCanvas::DrawCall{ 3, MockCanvas::SaveLayerData{opacity2_bounds, opacity2_paint, nullptr, 4}}, diff --git a/flow/layers/picture_layer.cc b/flow/layers/picture_layer.cc index 08c09cc9e833b..5a615168b1de5 100644 --- a/flow/layers/picture_layer.cc +++ b/flow/layers/picture_layer.cc @@ -26,9 +26,6 @@ void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { SkMatrix ctm = matrix; ctm.postTranslate(offset_.x(), offset_.y()); -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - ctm = RasterCache::GetIntegralTransCTM(ctm); -#endif cache->Prepare(context->gr_context, sk_picture, ctm, context->dst_color_space, is_complex_, will_change_); } @@ -44,10 +41,6 @@ void PictureLayer::Paint(PaintContext& context) const { SkAutoCanvasRestore save(context.leaf_nodes_canvas, true); context.leaf_nodes_canvas->translate(offset_.x(), offset_.y()); -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - context.leaf_nodes_canvas->setMatrix(RasterCache::GetIntegralTransCTM( - context.leaf_nodes_canvas->getTotalMatrix())); -#endif if (context.raster_cache) { const SkMatrix& ctm = context.leaf_nodes_canvas->getTotalMatrix(); diff --git a/flow/layers/picture_layer_unittests.cc b/flow/layers/picture_layer_unittests.cc index 4f565cf500ecc..0cdb2f94f4542 100644 --- a/flow/layers/picture_layer_unittests.cc +++ b/flow/layers/picture_layer_unittests.cc @@ -11,10 +11,6 @@ #include "flutter/testing/mock_canvas.h" #include "third_party/skia/include/core/SkPicture.h" -#ifndef SUPPORT_FRACTIONAL_TRANSLATION -#include "flutter/flow/raster_cache.h" -#endif - namespace flutter { namespace testing { @@ -85,16 +81,11 @@ TEST_F(PictureLayerTest, SimplePicture) { EXPECT_FALSE(layer->needs_system_composite()); layer->Paint(paint_context()); - auto expected_draw_calls = std::vector( - {MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, - MockCanvas::DrawCall{1, - MockCanvas::ConcatMatrixData{layer_offset_matrix}}, -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - MockCanvas::DrawCall{ - 1, MockCanvas::SetMatrixData{RasterCache::GetIntegralTransCTM( - layer_offset_matrix)}}, -#endif - MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}}); + auto expected_draw_calls = + std::vector({MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, + MockCanvas::DrawCall{ + 1, MockCanvas::ConcatMatrixData{layer_offset_matrix}}, + MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}}); EXPECT_EQ(mock_canvas().draw_calls(), expected_draw_calls); } diff --git a/flow/raster_cache.h b/flow/raster_cache.h index b51382a5c8d2d..2cbe07db5c6ba 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -62,13 +62,6 @@ class RasterCache { return bounds; } - static SkMatrix GetIntegralTransCTM(const SkMatrix& ctm) { - SkMatrix result = ctm; - result[SkMatrix::kMTransX] = SkScalarRoundToScalar(ctm.getTranslateX()); - result[SkMatrix::kMTransY] = SkScalarRoundToScalar(ctm.getTranslateY()); - return result; - } - // Return true if the cache is generated. // // We may return false and not generate the cache if diff --git a/flow/raster_cache_key.h b/flow/raster_cache_key.h index 5eae4d3dccacb..f5fd1e0ad1edb 100644 --- a/flow/raster_cache_key.h +++ b/flow/raster_cache_key.h @@ -15,11 +15,8 @@ template class RasterCacheKey { public: RasterCacheKey(ID id, const SkMatrix& ctm) : id_(id), matrix_(ctm) { - matrix_[SkMatrix::kMTransX] = SkScalarFraction(ctm.getTranslateX()); - matrix_[SkMatrix::kMTransY] = SkScalarFraction(ctm.getTranslateY()); -#ifndef SUPPORT_FRACTIONAL_TRANSLATION - FML_DCHECK(matrix_.getTranslateX() == 0 && matrix_.getTranslateY() == 0); -#endif + matrix_[SkMatrix::kMTransX] = 0; + matrix_[SkMatrix::kMTransY] = 0; } ID id() const { return id_; }