[Impeller] Don't cull readbacks outside the damage rect. (flutter/engine#46705)

Resolves https://github.com/flutter/flutter/issues/136058.

The damage rect properly expands to include the backdrop filter readback area to a certain extent, but it suddenly gets excluded once it no longer partially overlaps with the damage rect. This caused the illusion of EntityPass improperly culling the backdrop entity, leading me into a bit of a wild goose chase.

Before:

https://github.com/flutter/engine/assets/919017/94b8c077-0945-4a2c-96e0-27230d980c38

After:

https://github.com/flutter/engine/assets/919017/f1c78365-6e9b-46cb-9e69-33472d488831
This commit is contained in:
Brandon DeRosier
2023-10-10 13:55:05 -07:00
committed by GitHub
parent d9469ed169
commit 30fe68f402
4 changed files with 51 additions and 14 deletions

View File

@@ -123,12 +123,16 @@ Damage DiffContext::ComputeDamage(const SkIRect& accumulated_buffer_damage,
SkRect frame_damage(damage_);
for (const auto& r : readbacks_) {
SkRect rect = SkRect::Make(r.rect);
if (rect.intersects(frame_damage)) {
frame_damage.join(rect);
}
if (rect.intersects(buffer_damage)) {
buffer_damage.join(rect);
SkRect paint_rect = SkRect::Make(r.paint_rect);
SkRect readback_rect = SkRect::Make(r.readback_rect);
// Changes either in readback or paint rect require repainting both readback
// and paint rect.
if (paint_rect.intersects(frame_damage) ||
readback_rect.intersects(frame_damage)) {
frame_damage.join(readback_rect);
frame_damage.join(paint_rect);
buffer_damage.join(readback_rect);
buffer_damage.join(paint_rect);
}
}
@@ -220,9 +224,11 @@ void DiffContext::AddExistingPaintRegion(const PaintRegion& region) {
}
}
void DiffContext::AddReadbackRegion(const SkIRect& rect) {
void DiffContext::AddReadbackRegion(const SkIRect& paint_rect,
const SkIRect& readback_rect) {
Readback readback;
readback.rect = rect;
readback.paint_rect = paint_rect;
readback.readback_rect = readback_rect;
readback.position = rects_->size();
// Push empty rect as a placeholder for position in current subtree
rects_->push_back(SkRect::MakeEmpty());

View File

@@ -123,8 +123,12 @@ class DiffContext {
// The idea of readback region is that if any part of the readback region
// needs to be repainted, then the whole readback region must be repainted;
//
// Readback rect is in screen coordinates.
void AddReadbackRegion(const SkIRect& rect);
// paint_rect - rectangle where the filter paints contents (in screen
// coordinates)
// readback_rect - rectangle where the filter samples from (in screen
// coordinates)
void AddReadbackRegion(const SkIRect& paint_rect,
const SkIRect& readback_rect);
// Returns the paint region for current subtree; Each rect in paint region is
// in screen coordinates; Once a layer accumulates the paint regions of its
@@ -261,8 +265,11 @@ class DiffContext {
// determine if subtree has any readback
size_t position;
// readback area, in screen coordinates
SkIRect rect;
// Paint region of the filter performing readback, in screen coordinates.
SkIRect paint_rect;
// Readback area of the filter, in screen coordinates.
SkIRect readback_rect;
};
std::vector<Readback> readbacks_;

View File

@@ -31,7 +31,7 @@ void BackdropFilterLayer::Diff(DiffContext* context, const Layer* old_layer) {
SkIRect filter_input_bounds; // in screen coordinates
filter_->get_input_device_bounds(
filter_target_bounds, context->GetTransform3x3(), filter_input_bounds);
context->AddReadbackRegion(filter_input_bounds);
context->AddReadbackRegion(filter_target_bounds, filter_input_bounds);
}
DiffChildren(context, prev);

View File

@@ -469,7 +469,6 @@ TEST_F(BackdropLayerDiffTest, BackdropLayer) {
auto path1 = SkPath().addRect(SkRect::MakeLTRB(180, 180, 190, 190));
l4.root()->Add(std::make_shared<MockLayer>(path1));
damage = DiffLayerTree(l4, l3);
EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(180, 180, 190, 190));
MockLayerTree l5;
@@ -482,6 +481,31 @@ TEST_F(BackdropLayerDiffTest, BackdropLayer) {
EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 190, 190));
}
TEST_F(BackdropLayerDiffTest, ReadbackOutsideOfPaintArea) {
auto filter = DlMatrixImageFilter(SkMatrix::Translate(50, 50),
DlImageSampling::kLinear);
MockLayerTree l1(SkISize::Make(100, 100));
auto clip = std::make_shared<ClipRectLayer>(SkRect::MakeLTRB(60, 60, 80, 80),
Clip::hardEdge);
clip->Add(std::make_shared<BackdropFilterLayer>(filter.shared(),
DlBlendMode::kSrcOver));
l1.root()->Add(clip);
auto damage = DiffLayerTree(l1, MockLayerTree(SkISize::Make(100, 100)));
EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(60 - 50, 60 - 50, 80, 80));
MockLayerTree l2(SkISize::Make(100, 100));
// path inside readback area must trigger whole readback repaint + filter
// repaint.
auto path2 = SkPath().addRect(SkRect::MakeXYWH(60 - 50, 60 - 50, 10, 10));
l2.root()->Add(clip);
l2.root()->Add(std::make_shared<MockLayer>(path2));
damage = DiffLayerTree(l2, l1);
EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(60 - 50, 60 - 50, 80, 80));
}
TEST_F(BackdropLayerDiffTest, BackdropLayerInvalidTransform) {
auto filter = DlBlurImageFilter(10, 10, DlTileMode::kClamp);