Partial revert of fractional translation changes to raster cache

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<SkData>& snapshot,
-                                     const SkRect& bounds)
+                                     const SkIRect& bounds)
     : layer_unique_id_(layer_unique_id),
       duration_(duration),
       snapshot_(snapshot),

flow/layer_snapshot_store.h

@@ -26,7 +26,7 @@ class LayerSnapshotData {
   LayerSnapshotData(int64_t layer_unique_id,
                     const fml::TimeDelta& duration,
                     const sk_sp<SkData>& snapshot,
-                    const SkRect& bounds);
+                    const SkIRect& bounds);
 
   ~LayerSnapshotData() = default;
 
@@ -36,13 +36,13 @@ class LayerSnapshotData {
 
   sk_sp<SkData> 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<SkData> snapshot_;
-  const SkRect bounds_;
+  const SkIRect bounds_;
 };
 
 /// Collects snapshots of layers during frame rasterization.

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<SkData> raster_data = offscreen_surface->GetRasterData(true);
     LayerSnapshotData snapshot_data(unique_id(), offscreen_render_time,

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<RasterCacheResult> 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<SkSurface> surface =
       context.gr_context ? SkSurface::MakeRenderTarget(
@@ -100,7 +86,7 @@ std::unique_ptr<RasterCacheResult> 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);
 

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<DisplayList> 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

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;

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<MockRasterCacheResult>(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<MockRasterCacheResult>(cache_rect);
                    });

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<RasterCacheItem*> raster_cache_items_;

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);