Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion flow/layers/display_list_raster_cache_item.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
2 changes: 1 addition & 1 deletion flow/layers/layer_raster_cache_item.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
78 changes: 36 additions & 42 deletions flow/raster_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,45 +25,32 @@ namespace flutter {

RasterCacheResult::RasterCacheResult(sk_sp<SkImage> 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");
SkAutoCanvasRestore auto_restore(&canvas, true);

SkRect bounds =
SkIRect rounded_bounds = RasterCacheUtil::GetRoundedOutDeviceBounds(
logical_rect_, canvas.getTotalMatrix());
SkRect raw_bounds =
RasterCacheUtil::GetDeviceBounds(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() - 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(std::abs(rounded_bounds.size().width() -
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this still might be incorrect. We may have been unconditionally aligning the origin to a physical pixel before, thus the round out would at most add 1 pixel in each direction. Now the the origin can be fractional, rounding out may add up to two pixels in each direction

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A potential fix would be to only pixel snap in display_list_layers. That would not have the instability issues that pixel snapping in all composited layers does, but may have other tradeoffs that I'm not aware of

image_->dimensions().width()) <= 1 &&
std::abs(rounded_bounds.size().height() -
image_->dimensions().height()) <= 1);
#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_, rounded_bounds.fLeft, rounded_bounds.fTop,
SkSamplingOptions(), paint);
}

RasterCache::RasterCache(size_t access_threshold,
Expand All @@ -75,20 +62,17 @@ RasterCache::RasterCache(size_t access_threshold,
/// @note Procedure doesn't copy all closures.
std::unique_ptr<RasterCacheResult> RasterCache::Rasterize(
const RasterCache::Context& context,
bool align_image,
const std::function<void(SkCanvas*)>& draw_function,
const std::function<void(SkCanvas*, const SkRect& rect)>& draw_checkerboard)
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<SkSurface> surface =
context.gr_context ? SkSurface::MakeRenderTarget(
context.gr_context, SkBudgeted::kYes, image_info)
Expand All @@ -100,27 +84,37 @@ std::unique_ptr<RasterCacheResult> RasterCache::Rasterize(

SkCanvas* canvas = surface->getCanvas();
canvas->clear(SK_ColorTRANSPARENT);
canvas->translate(-dest_rect.left(), -dest_rect.top());
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_) {
draw_checkerboard(canvas, context.logical_rect);
}

return std::make_unique<RasterCacheResult>(
surface->makeImageSnapshot(), context.logical_rect, context.flow_type);
return std::make_unique<RasterCacheResult>(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<void(SkCanvas*)>& 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: {
Expand Down
4 changes: 4 additions & 0 deletions flow/raster_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class RasterCacheResult {
public:
RasterCacheResult(sk_sp<SkImage> image,
const SkRect& logical_rect,
bool align_image,
const char* type);

virtual ~RasterCacheResult() = default;
Expand All @@ -48,6 +49,7 @@ class RasterCacheResult {
private:
sk_sp<SkImage> image_;
SkRect logical_rect_;
bool align_image_;
fml::tracing::TraceFlow flow_;
};

Expand Down Expand Up @@ -124,6 +126,7 @@ class RasterCache {

std::unique_ptr<RasterCacheResult> Rasterize(
const RasterCache::Context& context,
bool align_image,
const std::function<void(SkCanvas*)>& draw_function,
const std::function<void(SkCanvas*, const SkRect& rect)>&
draw_checkerboard) const;
Expand Down Expand Up @@ -233,6 +236,7 @@ class RasterCache {

bool UpdateCacheEntry(
const RasterCacheKeyID& id,
bool align_image,
const Context& raster_cache_context,
const std::function<void(SkCanvas*)>& render_function) const;

Expand Down
45 changes: 19 additions & 26 deletions flow/raster_cache_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);

Expand All @@ -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);

Expand All @@ -824,29 +824,22 @@ TEST_F(RasterCacheTest, RasterCacheBleedingClipNeeded) {
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);

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
Expand Down
30 changes: 30 additions & 0 deletions flow/raster_cache_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,36 @@ 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;
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
Expand Down
9 changes: 6 additions & 3 deletions flow/testing/mock_raster_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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<MockRasterCacheResult>(cache_rect);
Expand Down Expand Up @@ -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<MockRasterCacheResult>(cache_rect);
Expand Down