diff --git a/flow/layer_snapshot_store.cc b/flow/layer_snapshot_store.cc index 00f0ed946374f..7e3cc8e6f849c 100644 --- a/flow/layer_snapshot_store.cc +++ b/flow/layer_snapshot_store.cc @@ -12,7 +12,7 @@ namespace flutter { LayerSnapshotData::LayerSnapshotData(int64_t layer_unique_id, const fml::TimeDelta& duration, const sk_sp& snapshot, - const SkRect& bounds) + const SkIRect& bounds) : layer_unique_id_(layer_unique_id), duration_(duration), snapshot_(snapshot), diff --git a/flow/layer_snapshot_store.h b/flow/layer_snapshot_store.h index eb444d2dcc390..fd255b5186d75 100644 --- a/flow/layer_snapshot_store.h +++ b/flow/layer_snapshot_store.h @@ -26,7 +26,7 @@ class LayerSnapshotData { LayerSnapshotData(int64_t layer_unique_id, const fml::TimeDelta& duration, const sk_sp& snapshot, - const SkRect& bounds); + const SkIRect& bounds); ~LayerSnapshotData() = default; @@ -36,13 +36,13 @@ class LayerSnapshotData { sk_sp GetSnapshot() const { return snapshot_; } - SkRect GetBounds() const { return bounds_; } + SkIRect GetBounds() const { return bounds_; } private: const int64_t layer_unique_id_; const fml::TimeDelta duration_; const sk_sp snapshot_; - const SkRect bounds_; + const SkIRect bounds_; }; /// Collects snapshots of layers during frame rasterization. diff --git a/flow/layers/display_list_layer.cc b/flow/layers/display_list_layer.cc index 5039feae31c3b..963de5fa9c96f 100644 --- a/flow/layers/display_list_layer.cc +++ b/flow/layers/display_list_layer.cc @@ -141,7 +141,7 @@ void DisplayListLayer::Paint(PaintContext& context) const { const fml::TimeDelta offscreen_render_time = fml::TimePoint::Now() - start_time; - const SkRect device_bounds = + const SkIRect device_bounds = RasterCacheUtil::GetDeviceBounds(paint_bounds(), ctm); sk_sp raster_data = offscreen_surface->GetRasterData(true); LayerSnapshotData snapshot_data(unique_id(), offscreen_render_time, diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 9b6e72d43c2c8..ef61845ee8046 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -32,9 +32,11 @@ 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()); + SkRect raw_bounds = RasterCacheUtil::GetRawDeviceBounds( + logical_rect_, canvas.getTotalMatrix()); #ifndef NDEBUG + SkIRect bounds = + RasterCacheUtil::GetDeviceBounds(logical_rect_, canvas.getTotalMatrix()); // 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 @@ -47,23 +49,8 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const { #endif canvas.resetMatrix(); flow_.Step(); - - bool exceeds_bounds = bounds.fLeft + image_->dimensions().width() > - SkScalarCeilToScalar(bounds.fRight) || - bounds.fTop + image_->dimensions().height() > - SkScalarCeilToScalar(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.drawImage(image_, bounds.fLeft, bounds.fTop, SkSamplingOptions(), - paint); - if (exceeds_bounds) { - canvas.restore(); - } + canvas.drawImage(image_, raw_bounds.fLeft, raw_bounds.fRight, + SkSamplingOptions(), paint); } RasterCache::RasterCache(size_t access_threshold, @@ -80,14 +67,13 @@ std::unique_ptr RasterCache::Rasterize( const { TRACE_EVENT0("flutter", "RasterCachePopulate"); - SkRect dest_rect = + SkIRect cache_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)); + SkRect raw_cache_rect = + RasterCacheUtil::GetRawDeviceBounds(context.logical_rect, context.matrix); + const SkImageInfo image_info = + SkImageInfo::MakeN32Premul(cache_rect.width(), cache_rect.height(), + sk_ref_sp(context.dst_color_space)); sk_sp surface = context.gr_context ? SkSurface::MakeRenderTarget( @@ -100,7 +86,7 @@ std::unique_ptr RasterCache::Rasterize( SkCanvas* canvas = surface->getCanvas(); canvas->clear(SK_ColorTRANSPARENT); - canvas->translate(-dest_rect.left(), -dest_rect.top()); + canvas->translate(-raw_cache_rect.left(), -raw_cache_rect.top()); canvas->concat(context.matrix); draw_function(canvas); diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index 9a72da8e698ff..e8d815b7e1737 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -402,56 +402,7 @@ TEST(RasterCache, ComputeDeviceRectBasedOnFractionalTranslation) { SkRect logical_rect = SkRect::MakeLTRB(0, 0, 300.2, 300.3); SkMatrix ctm = SkMatrix::MakeAll(2.0, 0, 0, 0, 2.0, 0, 0, 0, 1); auto result = RasterCacheUtil::GetDeviceBounds(logical_rect, ctm); - ASSERT_EQ(result, SkRect::MakeLTRB(0.0, 0.0, 600.4, 600.6)); -} - -// Construct a cache result whose device target rectangle rounds out to be one -// pixel wider than the cached image. Verify that it can be drawn without -// triggering any assertions. -TEST(RasterCache, DeviceRectRoundOutForDisplayList) { - size_t threshold = 1; - flutter::RasterCache cache(threshold); - - SkRect logical_rect = SkRect::MakeLTRB(28, 0, 354.56731, 310.288); - DisplayListBuilder builder(logical_rect); - builder.setColor(SK_ColorRED); - builder.drawRect(logical_rect); - sk_sp display_list = builder.Build(); - - SkMatrix ctm = SkMatrix::MakeAll(1.3312, 0, 233, 0, 1.3312, 206, 0, 0, 1); - SkPaint paint; - - SkCanvas canvas(100, 100, nullptr); - canvas.setMatrix(ctm); - - FixedRefreshRateStopwatch raster_time; - FixedRefreshRateStopwatch ui_time; - MutatorsStack mutators_stack; - PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder( - &cache, &raster_time, &ui_time, &mutators_stack); - PaintContextHolder paint_context_holder = - GetSamplePaintContextHolder(&cache, &raster_time, &ui_time); - auto& preroll_context = preroll_context_holder.preroll_context; - auto& paint_context = paint_context_holder.paint_context; - - cache.BeginFrame(); - DisplayListRasterCacheItem display_list_item(display_list.get(), SkPoint(), - true, false); - - ASSERT_FALSE(RasterCacheItemPrerollAndTryToRasterCache( - display_list_item, preroll_context, paint_context, ctm)); - ASSERT_FALSE(display_list_item.Draw(paint_context, &canvas, &paint)); - - cache.EndFrame(); - cache.BeginFrame(); - - ASSERT_TRUE(RasterCacheItemPrerollAndTryToRasterCache( - display_list_item, preroll_context, paint_context, ctm)); - ASSERT_TRUE(display_list_item.Draw(paint_context, &canvas, &paint)); - - canvas.translate(248, 0); - ASSERT_TRUE(cache.Draw(display_list_item.GetId().value(), canvas, &paint)); - ASSERT_TRUE(display_list_item.Draw(paint_context, &canvas, &paint)); + ASSERT_EQ(result, SkIRect::MakeLTRB(0, 0, 601, 601)); } TEST(RasterCache, NestedOpCountMetricUsedForDisplayList) { @@ -787,67 +738,5 @@ TEST_F(RasterCacheTest, RasterCacheKeyID_LayerChildrenIds) { ASSERT_EQ(ids, expected_ids); } -TEST_F(RasterCacheTest, RasterCacheBleedingNoClipNeeded) { - SkImageInfo info = - SkImageInfo::MakeN32(40, 40, SkAlphaType::kOpaque_SkAlphaType); - - auto image = SkImage::MakeRasterData( - info, SkData::MakeUninitialized(40 * 40 * 4), 40 * 4); - auto canvas = MockCanvas(); - 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 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::DrawImageData{image, 200.6, 200.6, - SkSamplingOptions(), paint}}, - MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}, - })); -} - -TEST_F(RasterCacheTest, RasterCacheBleedingClipNeeded) { - SkImageInfo info = - SkImageInfo::MakeN32(40, 40, SkAlphaType::kOpaque_SkAlphaType); - - auto image = SkImage::MakeRasterData( - info, SkData::MakeUninitialized(40 * 40 * 4), 40 * 4); - 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 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}}, - })); -} - } // namespace testing } // namespace flutter diff --git a/flow/raster_cache_util.h b/flow/raster_cache_util.h index 35abaf2d5f56c..2e76dacec8fab 100644 --- a/flow/raster_cache_util.h +++ b/flow/raster_cache_util.h @@ -49,7 +49,25 @@ struct RasterCacheUtil { return true; } - static SkRect GetDeviceBounds(const SkRect& rect, const SkMatrix& ctm) { + static SkIRect GetDeviceBounds(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; + } + + static SkRect GetRawDeviceBounds(const SkRect& rect, const SkMatrix& ctm) { SkRect device_rect; ctm.mapRect(&device_rect, rect); return device_rect; diff --git a/flow/testing/mock_raster_cache.cc b/flow/testing/mock_raster_cache.cc index 33b81c15030b3..885e56b862961 100644 --- a/flow/testing/mock_raster_cache.cc +++ b/flow/testing/mock_raster_cache.cc @@ -16,7 +16,7 @@ namespace flutter { namespace testing { -MockRasterCacheResult::MockRasterCacheResult(SkRect device_rect) +MockRasterCacheResult::MockRasterCacheResult(SkIRect device_rect) : RasterCacheResult(nullptr, SkRect::MakeEmpty(), "RasterCacheFlow::test"), device_rect_(device_rect) {} @@ -40,7 +40,7 @@ void MockRasterCache::AddMockLayer(int width, int height) { UpdateCacheEntry( RasterCacheKeyID(layer.unique_id(), RasterCacheKeyType::kLayer), r_context, [&](SkCanvas* canvas) { - SkRect cache_rect = RasterCacheUtil::GetDeviceBounds( + SkIRect cache_rect = RasterCacheUtil::GetDeviceBounds( r_context.logical_rect, r_context.matrix); return std::make_unique(cache_rect); }); @@ -78,7 +78,7 @@ void MockRasterCache::AddMockPicture(int width, int height) { UpdateCacheEntry(RasterCacheKeyID(display_list->unique_id(), RasterCacheKeyType::kDisplayList), r_context, [&](SkCanvas* canvas) { - SkRect cache_rect = RasterCacheUtil::GetDeviceBounds( + SkIRect cache_rect = RasterCacheUtil::GetDeviceBounds( r_context.logical_rect, r_context.matrix); return std::make_unique(cache_rect); }); diff --git a/flow/testing/mock_raster_cache.h b/flow/testing/mock_raster_cache.h index 37742486409bb..4a543e32d72e9 100644 --- a/flow/testing/mock_raster_cache.h +++ b/flow/testing/mock_raster_cache.h @@ -29,7 +29,7 @@ namespace testing { */ class MockRasterCacheResult : public RasterCacheResult { public: - explicit MockRasterCacheResult(SkRect device_rect); + explicit MockRasterCacheResult(SkIRect device_rect); void draw(SkCanvas& canvas, const SkPaint* paint = nullptr) const override{}; @@ -43,7 +43,7 @@ class MockRasterCacheResult : public RasterCacheResult { } private: - SkRect device_rect_; + SkIRect device_rect_; }; static std::vector raster_cache_items_; diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 50f81d8d57b92..cf6127a894d8a 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -1840,7 +1840,7 @@ static rapidjson::Value SerializeLayerSnapshot( result.AddMember("duration_micros", snapshot.GetDuration().ToMicroseconds(), allocator); - const SkRect bounds = snapshot.GetBounds(); + const SkIRect bounds = snapshot.GetBounds(); result.AddMember("top", bounds.top(), allocator); result.AddMember("left", bounds.left(), allocator); result.AddMember("width", bounds.width(), allocator);