Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 77f5b69

Browse files
knoppknopp
authored
Fix PhysicalShapeLayer paint bounds with clipping disabled (#31656)
* Fix PhysicalShapeLayer paint bounds with clipping disabled * Add missing SkRect initialization
1 parent 5fc52ce commit 77f5b69

File tree

2 files changed

+79
-13
lines changed

2 files changed

+79
-13
lines changed

‎flow/layers/physical_shape_layer.cc‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,18 +56,24 @@ void PhysicalShapeLayer::Preroll(PrerollContext* context,
5656
Layer::AutoPrerollSaveLayerState save =
5757
Layer::AutoPrerollSaveLayerState::Create(context, UsesSaveLayer());
5858

59-
SkRect child_paint_bounds;
59+
SkRect child_paint_bounds = SkRect::MakeEmpty();
6060
PrerollChildren(context, matrix, &child_paint_bounds);
6161

62+
SkRect paint_bounds;
6263
if (elevation_ == 0) {
63-
set_paint_bounds(path_.getBounds());
64+
paint_bounds = path_.getBounds();
6465
} else {
6566
// We will draw the shadow in Paint(), so add some margin to the paint
66-
// bounds to leave space for the shadow. We fill this whole region and clip
67-
// children to it so we don't need to join the child paint bounds.
68-
set_paint_bounds(DisplayListCanvasDispatcher::ComputeShadowBounds(
69-
path_, elevation_, context->frame_device_pixel_ratio, matrix));
67+
// bounds to leave space for the shadow.
68+
paint_bounds = DisplayListCanvasDispatcher::ComputeShadowBounds(
69+
path_, elevation_, context->frame_device_pixel_ratio, matrix);
7070
}
71+
72+
if (clip_behavior_ == Clip::none) {
73+
paint_bounds.join(child_paint_bounds);
74+
}
75+
76+
set_paint_bounds(paint_bounds);
7177
}
7278

7379
void PhysicalShapeLayer::Paint(PaintContext& context) const {

‎flow/layers/physical_shape_layer_unittests.cc‎

Lines changed: 67 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ TEST_F(PhysicalShapeLayerTest, NonEmptyLayer) {
6666
0, MockCanvas::DrawPathData{layer_path, layer_paint}}}));
6767
}
6868

69-
TEST_F(PhysicalShapeLayerTest, ChildrenLargerThanPath) {
69+
TEST_F(PhysicalShapeLayerTest, ChildrenLargerThanPathClip) {
7070
SkPath layer_path;
7171
layer_path.addRect(5.0f, 6.0f, 20.5f, 21.5f);
7272
SkPath child1_path;
@@ -83,18 +83,74 @@ TEST_F(PhysicalShapeLayerTest, ChildrenLargerThanPath) {
8383
auto layer =
8484
std::make_shared<PhysicalShapeLayer>(SK_ColorGREEN, SK_ColorBLACK,
8585
0.0f, // elevation
86-
layer_path, Clip::none);
86+
layer_path, Clip::hardEdge);
8787
layer->Add(child1);
8888
layer->Add(child2);
8989

90-
SkRect child_paint_bounds;
90+
SkRect child_paint_bounds = SkRect::MakeEmpty();
9191
layer->Preroll(preroll_context(), SkMatrix());
9292
child_paint_bounds.join(child1->paint_bounds());
9393
child_paint_bounds.join(child2->paint_bounds());
9494
EXPECT_EQ(layer->paint_bounds(), layer_path.getBounds());
9595
EXPECT_NE(layer->paint_bounds(), child_paint_bounds);
9696
EXPECT_TRUE(layer->needs_painting(paint_context()));
9797

98+
SkPaint layer_paint;
99+
layer_paint.setColor(SK_ColorGREEN);
100+
layer_paint.setAntiAlias(true);
101+
SkPaint child1_paint;
102+
child1_paint.setColor(SK_ColorRED);
103+
child1_paint.setAntiAlias(true);
104+
SkPaint child2_paint;
105+
child2_paint.setColor(SK_ColorBLUE);
106+
child2_paint.setAntiAlias(true);
107+
layer->Paint(paint_context());
108+
EXPECT_EQ(mock_canvas().draw_calls(),
109+
std::vector({
110+
MockCanvas::DrawCall{
111+
0, MockCanvas::DrawPathData{layer_path, layer_paint}},
112+
MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
113+
MockCanvas::DrawCall{
114+
1, MockCanvas::ClipRectData{layer_path.getBounds(),
115+
SkClipOp::kIntersect}},
116+
MockCanvas::DrawCall{
117+
1, MockCanvas::DrawPathData{child1_path, child1_paint}},
118+
MockCanvas::DrawCall{
119+
1, MockCanvas::DrawPathData{child2_path, child2_paint}},
120+
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}},
121+
}));
122+
}
123+
124+
TEST_F(PhysicalShapeLayerTest, ChildrenLargerThanPathNoClip) {
125+
SkPath layer_path;
126+
layer_path.addRect(5.0f, 6.0f, 20.5f, 21.5f);
127+
SkPath child1_path;
128+
child1_path.addRect(4, 0, 12, 12).close();
129+
SkPath child2_path;
130+
child2_path.addRect(3, 2, 5, 15).close();
131+
auto child1 = std::make_shared<PhysicalShapeLayer>(SK_ColorRED, SK_ColorBLACK,
132+
0.0f, // elevation
133+
child1_path, Clip::none);
134+
auto child2 =
135+
std::make_shared<PhysicalShapeLayer>(SK_ColorBLUE, SK_ColorBLACK,
136+
0.0f, // elevation
137+
child2_path, Clip::none);
138+
auto layer =
139+
std::make_shared<PhysicalShapeLayer>(SK_ColorGREEN, SK_ColorBLACK,
140+
0.0f, // elevation
141+
layer_path, Clip::none);
142+
layer->Add(child1);
143+
layer->Add(child2);
144+
145+
SkRect total_bounds = SkRect::MakeEmpty();
146+
layer->Preroll(preroll_context(), SkMatrix());
147+
total_bounds.join(child1->paint_bounds());
148+
total_bounds.join(child2->paint_bounds());
149+
total_bounds.join(layer_path.getBounds());
150+
EXPECT_NE(layer->paint_bounds(), layer_path.getBounds());
151+
EXPECT_EQ(layer->paint_bounds(), total_bounds);
152+
EXPECT_TRUE(layer->needs_painting(paint_context()));
153+
98154
SkPaint layer_paint;
99155
layer_paint.setColor(SK_ColorGREEN);
100156
layer_paint.setAntiAlias(true);
@@ -174,10 +230,14 @@ TEST_F(PhysicalShapeLayerTest, ElevationComplex) {
174230
// On Fuchsia, the system compositor handles all elevated
175231
// PhysicalShapeLayers and their shadows , so we do not do any painting
176232
// there.
177-
EXPECT_EQ(layers[i]->paint_bounds(),
178-
(DisplayListCanvasDispatcher::ComputeShadowBounds(
179-
layer_path, initial_elevations[i], 1.0f /* pixel_ratio */,
180-
SkMatrix())));
233+
SkRect paint_bounds = DisplayListCanvasDispatcher::ComputeShadowBounds(
234+
layer_path, initial_elevations[i], 1.0f /* pixel_ratio */, SkMatrix());
235+
236+
// Without clipping the children will be painted as well
237+
for (auto layer : layers[i]->layers()) {
238+
paint_bounds.join(layer->paint_bounds());
239+
}
240+
EXPECT_EQ(layers[i]->paint_bounds(), paint_bounds);
181241
EXPECT_TRUE(layers[i]->needs_painting(paint_context()));
182242
}
183243

0 commit comments

Comments
 (0)