Skip to content

Commit b82e902

Browse files
author
Jonah Williams
authored
[Impeller] Skip clip entity replay that cannot impact current clip. (#162113)
Fixes flutter/flutter#161262 Sometimes we end up with clip replay entities that have clip depth values substantially below the current depth. I suspect this is due to either mismatched save/restore or a bug in our code. Update: this isn't a bug/bug but its definitely a bug. We can have multiple clips per save, but the restore will remove at most one from the record/replay. If a clip has a depth value that is less than the current clip depth, it cannot by definition impact anything that draws after it.
1 parent 74ade43 commit b82e902

File tree

4 files changed

+38
-0
lines changed

4 files changed

+38
-0
lines changed

engine/src/flutter/impeller/display_list/aiks_dl_basic_unittests.cc

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1592,5 +1592,33 @@ TEST_P(AiksTest, NoDimplesInRRectPath) {
15921592
ASSERT_TRUE(OpenPlaygroundHere(callback));
15931593
}
15941594

1595+
TEST_P(AiksTest, BackdropFilterOverUnclosedClip) {
1596+
DisplayListBuilder builder;
1597+
1598+
builder.DrawPaint(DlPaint().setColor(DlColor::kWhite()));
1599+
builder.Save();
1600+
{
1601+
builder.ClipRect(DlRect::MakeLTRB(100, 100, 800, 800));
1602+
1603+
builder.Save();
1604+
{
1605+
builder.ClipRect(DlRect::MakeLTRB(600, 600, 800, 800));
1606+
builder.DrawPaint(DlPaint().setColor(DlColor::kRed()));
1607+
builder.DrawPaint(DlPaint().setColor(DlColor::kBlue().withAlphaF(0.5)));
1608+
builder.ClipRect(DlRect::MakeLTRB(700, 700, 750, 800));
1609+
builder.DrawPaint(DlPaint().setColor(DlColor::kRed().withAlphaF(0.5)));
1610+
}
1611+
builder.Restore();
1612+
1613+
auto image_filter = DlImageFilter::MakeBlur(10, 10, DlTileMode::kDecal);
1614+
builder.SaveLayer(std::nullopt, nullptr, image_filter.get());
1615+
}
1616+
builder.Restore();
1617+
builder.DrawCircle(SkPoint{100, 100}, 100,
1618+
DlPaint().setColor(DlColor::kAqua()));
1619+
1620+
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
1621+
}
1622+
15951623
} // namespace testing
15961624
} // namespace impeller

engine/src/flutter/impeller/display_list/canvas.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1662,6 +1662,10 @@ std::shared_ptr<Texture> Canvas::FlipBackdrop(Point global_pass_position,
16621662
// applied.
16631663
auto& replay_entities = clip_coverage_stack_.GetReplayEntities();
16641664
for (const auto& replay : replay_entities) {
1665+
if (replay.clip_depth <= current_depth_) {
1666+
continue;
1667+
}
1668+
16651669
SetClipScissor(replay.clip_coverage, current_render_pass,
16661670
global_pass_position);
16671671
if (!replay.clip_contents.Render(renderer_, current_render_pass,

engine/src/flutter/impeller/entity/entity_pass_clip_stack.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ EntityPassClipStack::ClipStateResult EntityPassClipStack::RecordRestore(
8383
if (subpass_state.clip_coverage.back().coverage.has_value()) {
8484
FML_DCHECK(next_replay_index_ <=
8585
subpass_state.rendered_clip_entities.size());
86+
// https://github.com/flutter/flutter/issues/162172
87+
// This code is slightly wrong and should be popping more than one clip
88+
// entry.
8689
if (!subpass_state.rendered_clip_entities.empty()) {
8790
subpass_state.rendered_clip_entities.pop_back();
8891

engine/src/flutter/testing/impeller_golden_tests_output.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ impeller_GoldenTests_ConicalGradient.png
33
impeller_Play_AiksTest_AdvancedBlendColorFilterWithDestinationOpacity_Metal.png
44
impeller_Play_AiksTest_AdvancedBlendColorFilterWithDestinationOpacity_OpenGLES.png
55
impeller_Play_AiksTest_AdvancedBlendColorFilterWithDestinationOpacity_Vulkan.png
6+
impeller_Play_AiksTest_BackdropFilterOverUnclosedClip_Metal.png
7+
impeller_Play_AiksTest_BackdropFilterOverUnclosedClip_OpenGLES.png
8+
impeller_Play_AiksTest_BackdropFilterOverUnclosedClip_Vulkan.png
69
impeller_Play_AiksTest_BackdropRestoreUsesCorrectCoverageForFirstRestoredClip_Metal.png
710
impeller_Play_AiksTest_BackdropRestoreUsesCorrectCoverageForFirstRestoredClip_OpenGLES.png
811
impeller_Play_AiksTest_BackdropRestoreUsesCorrectCoverageForFirstRestoredClip_Vulkan.png

0 commit comments

Comments
 (0)