From d592d619455c226efb0377788f91a8f251a83fe2 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 5 Sep 2022 17:33:56 -0700 Subject: [PATCH 01/12] round_out certain cache bounds --- flow/raster_cache.cc | 15 +++++++-------- flow/raster_cache_util.h | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 9b6e72d43c2c8..65033a964a8f2 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -80,14 +80,13 @@ std::unique_ptr RasterCache::Rasterize( const { TRACE_EVENT0("flutter", "RasterCachePopulate"); - SkRect dest_rect = - RasterCacheUtil::GetDeviceBounds(context.logical_rect, context.matrix); - // we always round out here so that the texture is integer sized. - int width = SkScalarCeilToInt(dest_rect.width()); - int height = SkScalarCeilToInt(dest_rect.height()); - - const SkImageInfo image_info = SkImageInfo::MakeN32Premul( - width, height, sk_ref_sp(context.dst_color_space)); + SkIRect dest_rect = RasterCacheUtil::GetRoundedOutDeviceBounds( + context.logical_rect, context.matrix); + ; + + const SkImageInfo image_info = + SkImageInfo::MakeN32Premul(dest_rect.width(), dest_rect.height(), + sk_ref_sp(context.dst_color_space)); sk_sp surface = context.gr_context ? SkSurface::MakeRenderTarget( diff --git a/flow/raster_cache_util.h b/flow/raster_cache_util.h index 35abaf2d5f56c..f309b5dbb54aa 100644 --- a/flow/raster_cache_util.h +++ b/flow/raster_cache_util.h @@ -54,6 +54,24 @@ struct RasterCacheUtil { ctm.mapRect(&device_rect, rect); return device_rect; } + + static SkIRect GetRoundedOutDeviceBounds(const SkRect& rect, const SkMatrix& ctm) { + SkRect device_rect; + ctm.mapRect(&device_rect, rect); + SkIRect bounds; + // Rather than roundOut, we first subtract the fractional portion of fLeft + // and fTop from fRight and fBottom. This ensures that we are more likely to + // round up to the nearest physical pixel in each direction instead o + // potentially two physical pixels in each direction. + auto fractionalLeft = bounds.fLeft - SkScalarFloorToInt(bounds.fLeft); + auto fractionalTop = bounds.fTop - SkScalarFloorToInt(bounds.fTop); + auto adjustedBottom = SkScalarCeilToInt(bounds.fBottom - fractionalTop); + auto adjustedRight = SkScalarCeilToInt(bounds.fRight - fractionalLeft); + bounds.setLTRB(SkScalarFloorToInt(bounds.fLeft), + SkScalarFloorToInt(bounds.fTop), adjustedRight, + adjustedBottom); + return bounds; + } }; } // namespace flutter From 0211f11b8f4f2599c35e965415eda6a91c580887 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 5 Sep 2022 18:45:58 -0700 Subject: [PATCH 02/12] round out --- flow/raster_cache.cc | 8 ++++---- flow/raster_cache_util.h | 15 +++------------ 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 65033a964a8f2..dc6044ccf9545 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -34,6 +34,8 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { SkRect bounds = RasterCacheUtil::GetDeviceBounds(logical_rect_, canvas.getTotalMatrix()); + SkIRect rounded_bounds = RasterCacheUtil::GetRoundedOutDeviceBounds( + logical_rect_, canvas.getTotalMatrix()); #ifndef NDEBUG // The image dimensions should always be larger than the device bounds and // smaller than the device bounds plus one pixel, at the same time, we must @@ -59,8 +61,8 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { canvas.save(); canvas.clipRect(SkRect::Make(bounds.roundOut())); } - canvas.drawImage(image_, bounds.fLeft, bounds.fTop, SkSamplingOptions(), - paint); + canvas.drawImage(image_, rounded_bounds.fLeft, rounded_bounds.fTop, + SkSamplingOptions(), paint); if (exceeds_bounds) { canvas.restore(); } @@ -82,8 +84,6 @@ std::unique_ptr RasterCache::Rasterize( SkIRect dest_rect = RasterCacheUtil::GetRoundedOutDeviceBounds( context.logical_rect, context.matrix); - ; - const SkImageInfo image_info = SkImageInfo::MakeN32Premul(dest_rect.width(), dest_rect.height(), sk_ref_sp(context.dst_color_space)); diff --git a/flow/raster_cache_util.h b/flow/raster_cache_util.h index f309b5dbb54aa..3732797200ec1 100644 --- a/flow/raster_cache_util.h +++ b/flow/raster_cache_util.h @@ -55,21 +55,12 @@ struct RasterCacheUtil { return device_rect; } - static SkIRect GetRoundedOutDeviceBounds(const SkRect& rect, const SkMatrix& ctm) { + static SkIRect GetRoundedOutDeviceBounds(const SkRect& rect, + const SkMatrix& ctm) { SkRect device_rect; ctm.mapRect(&device_rect, rect); SkIRect bounds; - // Rather than roundOut, we first subtract the fractional portion of fLeft - // and fTop from fRight and fBottom. This ensures that we are more likely to - // round up to the nearest physical pixel in each direction instead o - // potentially two physical pixels in each direction. - auto fractionalLeft = bounds.fLeft - SkScalarFloorToInt(bounds.fLeft); - auto fractionalTop = bounds.fTop - SkScalarFloorToInt(bounds.fTop); - auto adjustedBottom = SkScalarCeilToInt(bounds.fBottom - fractionalTop); - auto adjustedRight = SkScalarCeilToInt(bounds.fRight - fractionalLeft); - bounds.setLTRB(SkScalarFloorToInt(bounds.fLeft), - SkScalarFloorToInt(bounds.fTop), adjustedRight, - adjustedBottom); + device_rect.roundOut(&bounds); return bounds; } }; From 80124064f52d4e36c82e531abcda4457ba799e49 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 5 Sep 2022 19:53:27 -0700 Subject: [PATCH 03/12] track leading edge --- flow/raster_cache.cc | 20 +++++++++++++++++--- flow/raster_cache.h | 3 +++ flow/raster_cache_unittests.cc | 7 ++++--- flow/testing/mock_raster_cache.cc | 5 ++++- 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index dc6044ccf9545..afbb223bc8133 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -25,8 +25,12 @@ namespace flutter { RasterCacheResult::RasterCacheResult(sk_sp image, const SkRect& logical_rect, + const SkPoint& texture_edge, const char* type) - : image_(std::move(image)), logical_rect_(logical_rect), flow_(type) {} + : image_(std::move(image)), + logical_rect_(logical_rect), + texture_edge_(texture_edge), + flow_(type) {} void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { TRACE_EVENT0("flutter", "RasterCacheResult::draw"); @@ -84,6 +88,15 @@ std::unique_ptr RasterCache::Rasterize( SkIRect dest_rect = RasterCacheUtil::GetRoundedOutDeviceBounds( context.logical_rect, context.matrix); + SkRect raw_dest_rect = + RasterCacheUtil::GetDeviceBounds(context.logical_rect, context.matrix); + + // Rounding out the destination rect to size for a texture produces a small + // offset from the top corner of the logical rect to the top corner of the + // texture. Track this value so that cached layer children can be offset + // appropriately. + SkPoint texture_edge = SkPoint::Make(raw_dest_rect.fLeft - dest_rect.fLeft, + raw_dest_rect.fTop - dest_rect.fTop); const SkImageInfo image_info = SkImageInfo::MakeN32Premul(dest_rect.width(), dest_rect.height(), sk_ref_sp(context.dst_color_space)); @@ -107,8 +120,9 @@ std::unique_ptr RasterCache::Rasterize( draw_checkerboard(canvas, context.logical_rect); } - return std::make_unique( - surface->makeImageSnapshot(), context.logical_rect, context.flow_type); + return std::make_unique(surface->makeImageSnapshot(), + context.logical_rect, texture_edge, + context.flow_type); } bool RasterCache::UpdateCacheEntry( diff --git a/flow/raster_cache.h b/flow/raster_cache.h index 8eb33d532999e..e27c038c681d8 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -31,6 +31,7 @@ class RasterCacheResult { public: RasterCacheResult(sk_sp image, const SkRect& logical_rect, + const SkPoint& texture_edge, const char* type); virtual ~RasterCacheResult() = default; @@ -48,6 +49,8 @@ class RasterCacheResult { private: sk_sp image_; SkRect logical_rect_; + // TBD: what to do with this. Find example where texture still jumps + SkPoint texture_edge_; fml::tracing::TraceFlow flow_; }; diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index 9a72da8e698ff..6cef5aed376e9 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -797,8 +797,8 @@ TEST_F(RasterCacheTest, RasterCacheBleedingNoClipNeeded) { canvas.setMatrix(SkMatrix::Scale(2, 2)); // Drawing cached image does not exceeds physical pixels of the original // bounds and does not need to be clipped. - auto cache_result = - RasterCacheResult(image, SkRect::MakeXYWH(100.3, 100.3, 20, 20), ""); + auto cache_result = RasterCacheResult( + image, SkRect::MakeXYWH(100.3, 100.3, 20, 20), SkPoint::Make(0, 0), ""); auto paint = SkPaint(); cache_result.draw(canvas, &paint); @@ -825,7 +825,8 @@ TEST_F(RasterCacheTest, RasterCacheBleedingClipNeeded) { canvas.setMatrix(SkMatrix::Scale(2, 2)); auto cache_result = - RasterCacheResult(image, SkRect::MakeXYWH(100.3, 100.3, 19.6, 19.6), ""); + RasterCacheResult(image, SkRect::MakeXYWH(100.3, 100.3, 19.6, 19.6), + SkPoint::Make(0, 0), ""); auto paint = SkPaint(); cache_result.draw(canvas, &paint); diff --git a/flow/testing/mock_raster_cache.cc b/flow/testing/mock_raster_cache.cc index 33b81c15030b3..fbfd9557e4128 100644 --- a/flow/testing/mock_raster_cache.cc +++ b/flow/testing/mock_raster_cache.cc @@ -17,7 +17,10 @@ namespace flutter { namespace testing { MockRasterCacheResult::MockRasterCacheResult(SkRect device_rect) - : RasterCacheResult(nullptr, SkRect::MakeEmpty(), "RasterCacheFlow::test"), + : RasterCacheResult(nullptr, + SkRect::MakeEmpty(), + SkPoint::Make(0, 0), + "RasterCacheFlow::test"), device_rect_(device_rect) {} void MockRasterCache::AddMockLayer(int width, int height) { From 3db77772a10e1bdad2184be57b2a391acbeffee0 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 6 Sep 2022 14:06:06 -0700 Subject: [PATCH 04/12] cleanup --- flow/layers/layer_raster_cache_item.cc | 1 + flow/raster_cache.cc | 42 +++++++++----------------- flow/raster_cache.h | 3 -- flow/raster_cache_unittests.cc | 7 ++--- flow/testing/mock_raster_cache.cc | 5 +-- 5 files changed, 19 insertions(+), 39 deletions(-) diff --git a/flow/layers/layer_raster_cache_item.cc b/flow/layers/layer_raster_cache_item.cc index d5ddc17224b5a..2bcfd1a13d548 100644 --- a/flow/layers/layer_raster_cache_item.cc +++ b/flow/layers/layer_raster_cache_item.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "flutter/flow/layers/layer_raster_cache_item.h" +#include #include "flutter/flow/layers/container_layer.h" #include "flutter/flow/raster_cache_item.h" diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index afbb223bc8133..7b41da379baea 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -25,19 +25,13 @@ namespace flutter { RasterCacheResult::RasterCacheResult(sk_sp image, const SkRect& logical_rect, - const SkPoint& texture_edge, const char* type) - : image_(std::move(image)), - logical_rect_(logical_rect), - texture_edge_(texture_edge), - flow_(type) {} + : image_(std::move(image)), logical_rect_(logical_rect), flow_(type) {} void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { TRACE_EVENT0("flutter", "RasterCacheResult::draw"); SkAutoCanvasRestore auto_restore(&canvas, true); - SkRect bounds = - RasterCacheUtil::GetDeviceBounds(logical_rect_, canvas.getTotalMatrix()); SkIRect rounded_bounds = RasterCacheUtil::GetRoundedOutDeviceBounds( logical_rect_, canvas.getTotalMatrix()); #ifndef NDEBUG @@ -46,25 +40,27 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { // introduce epsilon to solve the round-off error. The value of epsilon is // 1/512, which represents half of an AA sample. float epsilon = 1 / 512.0; - FML_DCHECK(image_->dimensions().width() - bounds.width() > -epsilon && - image_->dimensions().height() - bounds.height() > -epsilon && - image_->dimensions().width() - bounds.width() < 1 + epsilon && - image_->dimensions().height() - bounds.height() < 1 + epsilon); + FML_DCHECK( + image_->dimensions().width() - rounded_bounds.width() > -epsilon && + image_->dimensions().height() - rounded_bounds.height() > -epsilon && + image_->dimensions().width() - rounded_bounds.width() < 1 + epsilon && + image_->dimensions().height() - rounded_bounds.height() < 1 + epsilon); #endif canvas.resetMatrix(); flow_.Step(); - bool exceeds_bounds = bounds.fLeft + image_->dimensions().width() > - SkScalarCeilToScalar(bounds.fRight) || - bounds.fTop + image_->dimensions().height() > - SkScalarCeilToScalar(bounds.fBottom); + bool exceeds_bounds = rounded_bounds.fLeft + image_->dimensions().width() > + SkScalarCeilToScalar(rounded_bounds.fRight) || + rounded_bounds.fTop + image_->dimensions().height() > + SkScalarCeilToScalar(rounded_bounds.fBottom); // Make sure raster cache doesn't bleed to physical pixels outside of // original bounds. https://github.com/flutter/flutter/issues/110002 if (exceeds_bounds) { canvas.save(); - canvas.clipRect(SkRect::Make(bounds.roundOut())); + canvas.clipRect(SkRect::Make(rounded_bounds)); } + canvas.drawImage(image_, rounded_bounds.fLeft, rounded_bounds.fTop, SkSamplingOptions(), paint); if (exceeds_bounds) { @@ -88,15 +84,6 @@ std::unique_ptr RasterCache::Rasterize( SkIRect dest_rect = RasterCacheUtil::GetRoundedOutDeviceBounds( context.logical_rect, context.matrix); - SkRect raw_dest_rect = - RasterCacheUtil::GetDeviceBounds(context.logical_rect, context.matrix); - - // Rounding out the destination rect to size for a texture produces a small - // offset from the top corner of the logical rect to the top corner of the - // texture. Track this value so that cached layer children can be offset - // appropriately. - SkPoint texture_edge = SkPoint::Make(raw_dest_rect.fLeft - dest_rect.fLeft, - raw_dest_rect.fTop - dest_rect.fTop); const SkImageInfo image_info = SkImageInfo::MakeN32Premul(dest_rect.width(), dest_rect.height(), sk_ref_sp(context.dst_color_space)); @@ -120,9 +107,8 @@ std::unique_ptr RasterCache::Rasterize( draw_checkerboard(canvas, context.logical_rect); } - return std::make_unique(surface->makeImageSnapshot(), - context.logical_rect, texture_edge, - context.flow_type); + return std::make_unique( + surface->makeImageSnapshot(), context.logical_rect, context.flow_type); } bool RasterCache::UpdateCacheEntry( diff --git a/flow/raster_cache.h b/flow/raster_cache.h index e27c038c681d8..8eb33d532999e 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -31,7 +31,6 @@ class RasterCacheResult { public: RasterCacheResult(sk_sp image, const SkRect& logical_rect, - const SkPoint& texture_edge, const char* type); virtual ~RasterCacheResult() = default; @@ -49,8 +48,6 @@ class RasterCacheResult { private: sk_sp image_; SkRect logical_rect_; - // TBD: what to do with this. Find example where texture still jumps - SkPoint texture_edge_; fml::tracing::TraceFlow flow_; }; diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index 6cef5aed376e9..9a72da8e698ff 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -797,8 +797,8 @@ TEST_F(RasterCacheTest, RasterCacheBleedingNoClipNeeded) { canvas.setMatrix(SkMatrix::Scale(2, 2)); // Drawing cached image does not exceeds physical pixels of the original // bounds and does not need to be clipped. - auto cache_result = RasterCacheResult( - image, SkRect::MakeXYWH(100.3, 100.3, 20, 20), SkPoint::Make(0, 0), ""); + auto cache_result = + RasterCacheResult(image, SkRect::MakeXYWH(100.3, 100.3, 20, 20), ""); auto paint = SkPaint(); cache_result.draw(canvas, &paint); @@ -825,8 +825,7 @@ TEST_F(RasterCacheTest, RasterCacheBleedingClipNeeded) { canvas.setMatrix(SkMatrix::Scale(2, 2)); auto cache_result = - RasterCacheResult(image, SkRect::MakeXYWH(100.3, 100.3, 19.6, 19.6), - SkPoint::Make(0, 0), ""); + RasterCacheResult(image, SkRect::MakeXYWH(100.3, 100.3, 19.6, 19.6), ""); auto paint = SkPaint(); cache_result.draw(canvas, &paint); diff --git a/flow/testing/mock_raster_cache.cc b/flow/testing/mock_raster_cache.cc index fbfd9557e4128..33b81c15030b3 100644 --- a/flow/testing/mock_raster_cache.cc +++ b/flow/testing/mock_raster_cache.cc @@ -17,10 +17,7 @@ namespace flutter { namespace testing { MockRasterCacheResult::MockRasterCacheResult(SkRect device_rect) - : RasterCacheResult(nullptr, - SkRect::MakeEmpty(), - SkPoint::Make(0, 0), - "RasterCacheFlow::test"), + : RasterCacheResult(nullptr, SkRect::MakeEmpty(), "RasterCacheFlow::test"), device_rect_(device_rect) {} void MockRasterCache::AddMockLayer(int width, int height) { From 67826ee6a1e556041b6b0d630627e0b7af4ad214 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 6 Sep 2022 14:15:35 -0700 Subject: [PATCH 05/12] ++ --- flow/layers/layer_raster_cache_item.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/flow/layers/layer_raster_cache_item.cc b/flow/layers/layer_raster_cache_item.cc index 2bcfd1a13d548..d5ddc17224b5a 100644 --- a/flow/layers/layer_raster_cache_item.cc +++ b/flow/layers/layer_raster_cache_item.cc @@ -3,7 +3,6 @@ // found in the LICENSE file. #include "flutter/flow/layers/layer_raster_cache_item.h" -#include #include "flutter/flow/layers/container_layer.h" #include "flutter/flow/raster_cache_item.h" From 0c462a35afb8c56db8bd5ba5f8f7990df24db031 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 6 Sep 2022 14:45:18 -0700 Subject: [PATCH 06/12] ++ --- flow/raster_cache.cc | 15 --------------- flow/raster_cache_unittests.cc | 33 +++++++++++++-------------------- 2 files changed, 13 insertions(+), 35 deletions(-) diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 7b41da379baea..5e09a21040f31 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -49,23 +49,8 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { canvas.resetMatrix(); flow_.Step(); - bool exceeds_bounds = rounded_bounds.fLeft + image_->dimensions().width() > - SkScalarCeilToScalar(rounded_bounds.fRight) || - rounded_bounds.fTop + image_->dimensions().height() > - SkScalarCeilToScalar(rounded_bounds.fBottom); - - // Make sure raster cache doesn't bleed to physical pixels outside of - // original bounds. https://github.com/flutter/flutter/issues/110002 - if (exceeds_bounds) { - canvas.save(); - canvas.clipRect(SkRect::Make(rounded_bounds)); - } - canvas.drawImage(image_, rounded_bounds.fLeft, rounded_bounds.fTop, SkSamplingOptions(), paint); - if (exceeds_bounds) { - canvas.restore(); - } } RasterCache::RasterCache(size_t access_threshold, diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index 9a72da8e698ff..c145661dae9b5 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -809,13 +809,13 @@ TEST_F(RasterCacheTest, RasterCacheBleedingNoClipNeeded) { MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkM44()}}, MockCanvas::DrawCall{ - 1, MockCanvas::DrawImageData{image, 200.6, 200.6, + 1, MockCanvas::DrawImageData{image, 200, 200, SkSamplingOptions(), paint}}, MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}, })); } -TEST_F(RasterCacheTest, RasterCacheBleedingClipNeeded) { +TEST_F(RasterCacheTest, RasterCacheBleedingClipStillNotNeeded) { SkImageInfo info = SkImageInfo::MakeN32(40, 40, SkAlphaType::kOpaque_SkAlphaType); @@ -829,24 +829,17 @@ TEST_F(RasterCacheTest, RasterCacheBleedingClipNeeded) { auto paint = SkPaint(); cache_result.draw(canvas, &paint); - EXPECT_EQ( - canvas.draw_calls(), - std::vector({ - MockCanvas::DrawCall{0, - MockCanvas::SetMatrixData{SkM44::Scale(2, 2)}}, - MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, - MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkM44()}}, - MockCanvas::DrawCall{1, MockCanvas::SaveData{2}}, - MockCanvas::DrawCall{ - 2, MockCanvas::ClipRectData{SkRect::MakeLTRB(200, 200, 240, 240), - SkClipOp::kIntersect, - MockCanvas::kHard_ClipEdgeStyle}}, - MockCanvas::DrawCall{ - 2, MockCanvas::DrawImageData{image, 200.6, 200.6, - SkSamplingOptions(), paint}}, - MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}}, - MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}, - })); + EXPECT_EQ(canvas.draw_calls(), + std::vector({ + MockCanvas::DrawCall{ + 0, MockCanvas::SetMatrixData{SkM44::Scale(2, 2)}}, + MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, + MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkM44()}}, + MockCanvas::DrawCall{ + 1, MockCanvas::DrawImageData{image, 200, 200, + SkSamplingOptions(), paint}}, + MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}, + })); } } // namespace testing From 69989a2ecd70bee37626c16af9081b84270408da Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 6 Sep 2022 21:31:59 -0700 Subject: [PATCH 07/12] ++ --- flow/raster_cache.cc | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 5e09a21040f31..275c9afff8116 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -35,16 +35,8 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { SkIRect rounded_bounds = RasterCacheUtil::GetRoundedOutDeviceBounds( logical_rect_, canvas.getTotalMatrix()); #ifndef NDEBUG - // The image dimensions should always be larger than the device bounds and - // smaller than the device bounds plus one pixel, at the same time, we must - // introduce epsilon to solve the round-off error. The value of epsilon is - // 1/512, which represents half of an AA sample. - float epsilon = 1 / 512.0; - FML_DCHECK( - image_->dimensions().width() - rounded_bounds.width() > -epsilon && - image_->dimensions().height() - rounded_bounds.height() > -epsilon && - image_->dimensions().width() - rounded_bounds.width() < 1 + epsilon && - image_->dimensions().height() - rounded_bounds.height() < 1 + epsilon); + FML_DCHECK(image_->dimensions().width() == rounded_bounds.width() && + image_->dimensions().height() == rounded_bounds.height()); #endif canvas.resetMatrix(); flow_.Step(); From 6a4b275fcb56db94977ceaf4cfb4352fa60ff493 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 6 Sep 2022 22:32:56 -0700 Subject: [PATCH 08/12] restore old condition --- flow/raster_cache.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 275c9afff8116..70ff5d64da3a8 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -35,8 +35,9 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { SkIRect rounded_bounds = RasterCacheUtil::GetRoundedOutDeviceBounds( logical_rect_, canvas.getTotalMatrix()); #ifndef NDEBUG - FML_DCHECK(image_->dimensions().width() == rounded_bounds.width() && - image_->dimensions().height() == rounded_bounds.height()); + FML_DCHECK( + std::abs(bounds.size().width() - image_->dimensions().width()) <= 1 && + std::abs(bounds.size().height() - image_->dimensions().height()) <= 1); #endif canvas.resetMatrix(); flow_.Step(); From c130819a12c5dc68f6e5c146864aad4bd9e1458f Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 6 Sep 2022 22:54:44 -0700 Subject: [PATCH 09/12] ++ --- flow/raster_cache.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 70ff5d64da3a8..3577e5ecf49c5 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -35,9 +35,10 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { SkIRect rounded_bounds = RasterCacheUtil::GetRoundedOutDeviceBounds( logical_rect_, canvas.getTotalMatrix()); #ifndef NDEBUG - FML_DCHECK( - std::abs(bounds.size().width() - image_->dimensions().width()) <= 1 && - std::abs(bounds.size().height() - image_->dimensions().height()) <= 1); + FML_DCHECK(std::abs(rounded_bounds.size().width() - + image_->dimensions().width()) <= 1 && + std::abs(rounded_bounds.size().height() - + image_->dimensions().height()) <= 1); #endif canvas.resetMatrix(); flow_.Step(); From 4021683d41556b147a4b6a85c51fd710b17dfc06 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 7 Sep 2022 10:37:55 -0700 Subject: [PATCH 10/12] cache display list differently from layer --- flow/layers/display_list_raster_cache_item.cc | 2 +- flow/layers/layer_raster_cache_item.cc | 2 +- flow/raster_cache.cc | 71 +++++++++++++++---- flow/raster_cache.h | 4 ++ flow/raster_cache_unittests.cc | 12 ++-- flow/testing/mock_raster_cache.cc | 9 ++- 6 files changed, 76 insertions(+), 24 deletions(-) diff --git a/flow/layers/display_list_raster_cache_item.cc b/flow/layers/display_list_raster_cache_item.cc index 10a64cff3a45e..9b5085d640306 100644 --- a/flow/layers/display_list_raster_cache_item.cc +++ b/flow/layers/display_list_raster_cache_item.cc @@ -162,7 +162,7 @@ bool DisplayListRasterCacheItem::TryToPrepareRasterCache( // clang-format on }; return context.raster_cache->UpdateCacheEntry( - GetId().value(), r_context, + GetId().value(), true, r_context, [display_list = display_list_](SkCanvas* canvas) { display_list->RenderTo(canvas); }); diff --git a/flow/layers/layer_raster_cache_item.cc b/flow/layers/layer_raster_cache_item.cc index d5ddc17224b5a..c6f6812ee79e6 100644 --- a/flow/layers/layer_raster_cache_item.cc +++ b/flow/layers/layer_raster_cache_item.cc @@ -156,7 +156,7 @@ bool LayerRasterCacheItem::TryToPrepareRasterCache(const PaintContext& context, // clang-format on }; return context.raster_cache->UpdateCacheEntry( - GetId().value(), r_context, + GetId().value(), false, r_context, [ctx = context, cache_state = cache_state_, layer = layer_](SkCanvas* canvas) { Rasterize(cache_state, layer, ctx, canvas); diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 3577e5ecf49c5..fa59c5c2c087d 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -25,8 +25,12 @@ namespace flutter { RasterCacheResult::RasterCacheResult(sk_sp image, const SkRect& logical_rect, + bool align_image, const char* type) - : image_(std::move(image)), logical_rect_(logical_rect), flow_(type) {} + : image_(std::move(image)), + logical_rect_(logical_rect), + align_image_(align_image), + flow_(type) {} void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { TRACE_EVENT0("flutter", "RasterCacheResult::draw"); @@ -34,6 +38,8 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { SkIRect rounded_bounds = RasterCacheUtil::GetRoundedOutDeviceBounds( logical_rect_, canvas.getTotalMatrix()); + SkRect raw_bounds = + RasterCacheUtil::GetDeviceBounds(logical_rect_, canvas.getTotalMatrix()); #ifndef NDEBUG FML_DCHECK(std::abs(rounded_bounds.size().width() - image_->dimensions().width()) <= 1 && @@ -43,8 +49,28 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { canvas.resetMatrix(); flow_.Step(); - canvas.drawImage(image_, rounded_bounds.fLeft, rounded_bounds.fTop, - SkSamplingOptions(), paint); + if (align_image_) { + canvas.drawImage(image_, rounded_bounds.fLeft, rounded_bounds.fTop, + SkSamplingOptions(), paint); + } else { + bool exceeds_bounds = raw_bounds.fLeft + image_->dimensions().width() > + SkScalarCeilToScalar(raw_bounds.fRight) || + raw_bounds.fTop + image_->dimensions().height() > + SkScalarCeilToScalar(raw_bounds.fBottom); + + // Make sure raster cache doesn't bleed to physical pixels outside of + // original bounds. https://github.com/flutter/flutter/issues/110002 + if (exceeds_bounds) { + canvas.save(); + canvas.clipRect(SkRect::Make(raw_bounds.roundOut())); + } + canvas.drawImage(image_, raw_bounds.fLeft, raw_bounds.fTop, + SkSamplingOptions(SkFilterMode::kNearest), paint); + + if (exceeds_bounds) { + canvas.restore(); + } + } } RasterCache::RasterCache(size_t access_threshold, @@ -56,16 +82,33 @@ RasterCache::RasterCache(size_t access_threshold, /// @note Procedure doesn't copy all closures. std::unique_ptr RasterCache::Rasterize( const RasterCache::Context& context, + bool align_image, const std::function& draw_function, const std::function& draw_checkerboard) const { TRACE_EVENT0("flutter", "RasterCachePopulate"); - SkIRect dest_rect = RasterCacheUtil::GetRoundedOutDeviceBounds( - context.logical_rect, context.matrix); - const SkImageInfo image_info = - SkImageInfo::MakeN32Premul(dest_rect.width(), dest_rect.height(), - sk_ref_sp(context.dst_color_space)); + SkScalar left_offset; + SkScalar top_offset; + SkImageInfo image_info; + if (align_image) { + SkIRect dest_rect = RasterCacheUtil::GetRoundedOutDeviceBounds( + context.logical_rect, context.matrix); + image_info = + SkImageInfo::MakeN32Premul(dest_rect.width(), dest_rect.height(), + sk_ref_sp(context.dst_color_space)); + left_offset = dest_rect.fLeft; + top_offset = dest_rect.fTop; + } else { + SkRect dest_rect = + RasterCacheUtil::GetDeviceBounds(context.logical_rect, context.matrix); + image_info = + SkImageInfo::MakeN32Premul(SkScalarCeilToInt(dest_rect.width()), + SkScalarCeilToInt(dest_rect.height()), + sk_ref_sp(context.dst_color_space)); + left_offset = dest_rect.fLeft; + top_offset = dest_rect.fTop; + } sk_sp surface = context.gr_context ? SkSurface::MakeRenderTarget( @@ -78,7 +121,7 @@ std::unique_ptr RasterCache::Rasterize( SkCanvas* canvas = surface->getCanvas(); canvas->clear(SK_ColorTRANSPARENT); - canvas->translate(-dest_rect.left(), -dest_rect.top()); + canvas->translate(-left_offset, -top_offset); canvas->concat(context.matrix); draw_function(canvas); @@ -86,19 +129,21 @@ std::unique_ptr RasterCache::Rasterize( draw_checkerboard(canvas, context.logical_rect); } - return std::make_unique( - surface->makeImageSnapshot(), context.logical_rect, context.flow_type); + return std::make_unique(surface->makeImageSnapshot(), + context.logical_rect, align_image, + context.flow_type); } bool RasterCache::UpdateCacheEntry( const RasterCacheKeyID& id, + bool align_image, const Context& raster_cache_context, const std::function& render_function) const { RasterCacheKey key = RasterCacheKey(id, raster_cache_context.matrix); Entry& entry = cache_[key]; if (!entry.image) { - entry.image = - Rasterize(raster_cache_context, render_function, DrawCheckerboard); + entry.image = Rasterize(raster_cache_context, align_image, render_function, + DrawCheckerboard); if (entry.image != nullptr) { switch (id.type()) { case RasterCacheKeyType::kDisplayList: { diff --git a/flow/raster_cache.h b/flow/raster_cache.h index 8eb33d532999e..1e442be540caf 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -31,6 +31,7 @@ class RasterCacheResult { public: RasterCacheResult(sk_sp image, const SkRect& logical_rect, + bool align_image, const char* type); virtual ~RasterCacheResult() = default; @@ -48,6 +49,7 @@ class RasterCacheResult { private: sk_sp image_; SkRect logical_rect_; + bool align_image_; fml::tracing::TraceFlow flow_; }; @@ -124,6 +126,7 @@ class RasterCache { std::unique_ptr Rasterize( const RasterCache::Context& context, + bool align_image, const std::function& draw_function, const std::function& draw_checkerboard) const; @@ -233,6 +236,7 @@ class RasterCache { bool UpdateCacheEntry( const RasterCacheKeyID& id, + bool align_image, const Context& raster_cache_context, const std::function& render_function) const; diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index c145661dae9b5..ddccccb9896fa 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -159,11 +159,11 @@ TEST(RasterCache, SetCheckboardCacheImages) { }; cache.SetCheckboardCacheImages(false); - cache.Rasterize(r_context, dummy_draw_function, draw_checkerboard); + cache.Rasterize(r_context, true, dummy_draw_function, draw_checkerboard); ASSERT_FALSE(did_draw_checkerboard); cache.SetCheckboardCacheImages(true); - cache.Rasterize(r_context, dummy_draw_function, draw_checkerboard); + cache.Rasterize(r_context, true, dummy_draw_function, draw_checkerboard); ASSERT_TRUE(did_draw_checkerboard); } @@ -797,8 +797,8 @@ TEST_F(RasterCacheTest, RasterCacheBleedingNoClipNeeded) { canvas.setMatrix(SkMatrix::Scale(2, 2)); // Drawing cached image does not exceeds physical pixels of the original // bounds and does not need to be clipped. - auto cache_result = - RasterCacheResult(image, SkRect::MakeXYWH(100.3, 100.3, 20, 20), ""); + auto cache_result = RasterCacheResult( + image, SkRect::MakeXYWH(100.3, 100.3, 20, 20), true, ""); auto paint = SkPaint(); cache_result.draw(canvas, &paint); @@ -824,8 +824,8 @@ TEST_F(RasterCacheTest, RasterCacheBleedingClipStillNotNeeded) { auto canvas = MockCanvas(); canvas.setMatrix(SkMatrix::Scale(2, 2)); - auto cache_result = - RasterCacheResult(image, SkRect::MakeXYWH(100.3, 100.3, 19.6, 19.6), ""); + auto cache_result = RasterCacheResult( + image, SkRect::MakeXYWH(100.3, 100.3, 19.6, 19.6), true, ""); auto paint = SkPaint(); cache_result.draw(canvas, &paint); diff --git a/flow/testing/mock_raster_cache.cc b/flow/testing/mock_raster_cache.cc index 33b81c15030b3..6f5eee0214bb9 100644 --- a/flow/testing/mock_raster_cache.cc +++ b/flow/testing/mock_raster_cache.cc @@ -17,7 +17,10 @@ namespace flutter { namespace testing { MockRasterCacheResult::MockRasterCacheResult(SkRect device_rect) - : RasterCacheResult(nullptr, SkRect::MakeEmpty(), "RasterCacheFlow::test"), + : RasterCacheResult(nullptr, + SkRect::MakeEmpty(), + false, + "RasterCacheFlow::test"), device_rect_(device_rect) {} void MockRasterCache::AddMockLayer(int width, int height) { @@ -39,7 +42,7 @@ void MockRasterCache::AddMockLayer(int width, int height) { }; UpdateCacheEntry( RasterCacheKeyID(layer.unique_id(), RasterCacheKeyType::kLayer), - r_context, [&](SkCanvas* canvas) { + false, r_context, [&](SkCanvas* canvas) { SkRect cache_rect = RasterCacheUtil::GetDeviceBounds( r_context.logical_rect, r_context.matrix); return std::make_unique(cache_rect); @@ -77,7 +80,7 @@ void MockRasterCache::AddMockPicture(int width, int height) { }; UpdateCacheEntry(RasterCacheKeyID(display_list->unique_id(), RasterCacheKeyType::kDisplayList), - r_context, [&](SkCanvas* canvas) { + false, r_context, [&](SkCanvas* canvas) { SkRect cache_rect = RasterCacheUtil::GetDeviceBounds( r_context.logical_rect, r_context.matrix); return std::make_unique(cache_rect); From c8e44bac25b69b485501e106d5bc8d7a802404b3 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 7 Sep 2022 10:38:14 -0700 Subject: [PATCH 11/12] ++ --- flow/raster_cache.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index fa59c5c2c087d..76acde549da66 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -65,7 +65,7 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { canvas.clipRect(SkRect::Make(raw_bounds.roundOut())); } canvas.drawImage(image_, raw_bounds.fLeft, raw_bounds.fTop, - SkSamplingOptions(SkFilterMode::kNearest), paint); + SkSamplingOptions(), paint); if (exceeds_bounds) { canvas.restore(); From dbd6ab549c22f58132c4e07e34fd7351d960b6ce Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 7 Sep 2022 12:11:22 -0700 Subject: [PATCH 12/12] use integral CTM --- flow/raster_cache.cc | 63 +++++++++++----------------------------- flow/raster_cache_util.h | 21 ++++++++++++++ 2 files changed, 38 insertions(+), 46 deletions(-) diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 76acde549da66..cd6c500dc27a4 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -49,28 +49,8 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { canvas.resetMatrix(); flow_.Step(); - if (align_image_) { - canvas.drawImage(image_, rounded_bounds.fLeft, rounded_bounds.fTop, - SkSamplingOptions(), paint); - } else { - bool exceeds_bounds = raw_bounds.fLeft + image_->dimensions().width() > - SkScalarCeilToScalar(raw_bounds.fRight) || - raw_bounds.fTop + image_->dimensions().height() > - SkScalarCeilToScalar(raw_bounds.fBottom); - - // Make sure raster cache doesn't bleed to physical pixels outside of - // original bounds. https://github.com/flutter/flutter/issues/110002 - if (exceeds_bounds) { - canvas.save(); - canvas.clipRect(SkRect::Make(raw_bounds.roundOut())); - } - canvas.drawImage(image_, raw_bounds.fLeft, raw_bounds.fTop, - SkSamplingOptions(), paint); - - if (exceeds_bounds) { - canvas.restore(); - } - } + canvas.drawImage(image_, rounded_bounds.fLeft, rounded_bounds.fTop, + SkSamplingOptions(), paint); } RasterCache::RasterCache(size_t access_threshold, @@ -88,28 +68,11 @@ std::unique_ptr RasterCache::Rasterize( const { TRACE_EVENT0("flutter", "RasterCachePopulate"); - SkScalar left_offset; - SkScalar top_offset; - SkImageInfo image_info; - if (align_image) { - SkIRect dest_rect = RasterCacheUtil::GetRoundedOutDeviceBounds( - context.logical_rect, context.matrix); - image_info = - SkImageInfo::MakeN32Premul(dest_rect.width(), dest_rect.height(), - sk_ref_sp(context.dst_color_space)); - left_offset = dest_rect.fLeft; - top_offset = dest_rect.fTop; - } else { - SkRect dest_rect = - RasterCacheUtil::GetDeviceBounds(context.logical_rect, context.matrix); - image_info = - SkImageInfo::MakeN32Premul(SkScalarCeilToInt(dest_rect.width()), - SkScalarCeilToInt(dest_rect.height()), - sk_ref_sp(context.dst_color_space)); - left_offset = dest_rect.fLeft; - top_offset = dest_rect.fTop; - } - + SkIRect dest_rect = RasterCacheUtil::GetRoundedOutDeviceBounds( + context.logical_rect, context.matrix); + const SkImageInfo image_info = + SkImageInfo::MakeN32Premul(dest_rect.width(), dest_rect.height(), + sk_ref_sp(context.dst_color_space)); sk_sp surface = context.gr_context ? SkSurface::MakeRenderTarget( context.gr_context, SkBudgeted::kYes, image_info) @@ -121,8 +84,16 @@ std::unique_ptr RasterCache::Rasterize( SkCanvas* canvas = surface->getCanvas(); canvas->clear(SK_ColorTRANSPARENT); - canvas->translate(-left_offset, -top_offset); - canvas->concat(context.matrix); + canvas->translate(-dest_rect.fLeft, -dest_rect.fTop); + + if (!align_image) { + SkMatrix integral_ctm = + RasterCacheUtil::GetIntegralTransCTM(context.matrix); + canvas->concat(integral_ctm); + } else { + canvas->concat(context.matrix); + } + draw_function(canvas); if (checkerboard_images_) { diff --git a/flow/raster_cache_util.h b/flow/raster_cache_util.h index 3732797200ec1..74950966b44ed 100644 --- a/flow/raster_cache_util.h +++ b/flow/raster_cache_util.h @@ -63,6 +63,27 @@ struct RasterCacheUtil { device_rect.roundOut(&bounds); return bounds; } + + /** + * @brief Snap the translation components of the matrix to integers. + * + * The snapping will only happen if the matrix only has scale and translation + * transformations. + * + * @param ctm the current transformation matrix. + * @return SkMatrix the snapped transformation matrix. + */ + static SkMatrix GetIntegralTransCTM(const SkMatrix& ctm) { + // Avoid integral snapping if the matrix has complex transformation to avoid + // the artifact observed in https://github.com/flutter/flutter/issues/41654. + if (!ctm.isScaleTranslate()) { + return ctm; + } + SkMatrix result = ctm; + result[SkMatrix::kMTransX] = SkScalarRoundToScalar(ctm.getTranslateX()); + result[SkMatrix::kMTransY] = SkScalarRoundToScalar(ctm.getTranslateY()); + return result; + } }; } // namespace flutter