Skip to content

Commit

Permalink
Move layer clip culling to Paint() method to fix child caching (flutt…
Browse files Browse the repository at this point in the history 
…er#22336)
  • Loading branch information
@flar
flar authored Nov 13, 2020
1 parent 8bb47c5 commit ecfe5ae
Show file tree
Hide file tree
Showing 39 changed files with 323 additions and 296 deletions.
2 changes: 1 addition & 1 deletion flow/layers/backdrop_filter_layer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ void BackdropFilterLayer::Preroll(PrerollContext* context,

void BackdropFilterLayer::Paint(PaintContext& context) const {
TRACE_EVENT0("flutter", "BackdropFilterLayer::Paint");
FML_DCHECK(needs_painting());
FML_DCHECK(needs_painting(context));

Layer::AutoSaveLayer save = Layer::AutoSaveLayer::Create(
context,
Expand Down
24 changes: 12 additions & 12 deletions flow/layers/backdrop_filter_layer_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ TEST_F(BackdropFilterLayerTest, PaintingEmptyLayerDies) {

layer->Preroll(preroll_context(), SkMatrix());
EXPECT_EQ(layer->paint_bounds(), kEmptyRect);
EXPECT_FALSE(layer->needs_painting());
EXPECT_FALSE(layer->needs_painting(paint_context()));
EXPECT_FALSE(layer->needs_system_composite());

EXPECT_DEATH_IF_SUPPORTED(layer->Paint(paint_context()),
"needs_painting\\(\\)");
"needs_painting\\(context\\)");
}

TEST_F(BackdropFilterLayerTest, PaintBeforePrerollDies) {
Expand All @@ -38,7 +38,7 @@ TEST_F(BackdropFilterLayerTest, PaintBeforePrerollDies) {

EXPECT_EQ(layer->paint_bounds(), kEmptyRect);
EXPECT_DEATH_IF_SUPPORTED(layer->Paint(paint_context()),
"needs_painting\\(\\)");
"needs_painting\\(context\\)");
}
#endif

Expand All @@ -53,7 +53,7 @@ TEST_F(BackdropFilterLayerTest, EmptyFilter) {

layer->Preroll(preroll_context(), initial_transform);
EXPECT_EQ(layer->paint_bounds(), child_bounds);
EXPECT_TRUE(layer->needs_painting());
EXPECT_TRUE(layer->needs_painting(paint_context()));
EXPECT_EQ(mock_layer->parent_matrix(), initial_transform);

layer->Paint(paint_context());
Expand All @@ -79,7 +79,7 @@ TEST_F(BackdropFilterLayerTest, SimpleFilter) {

layer->Preroll(preroll_context(), initial_transform);
EXPECT_EQ(layer->paint_bounds(), child_bounds);
EXPECT_TRUE(layer->needs_painting());
EXPECT_TRUE(layer->needs_painting(paint_context()));
EXPECT_EQ(mock_layer->parent_matrix(), initial_transform);

layer->Paint(paint_context());
Expand Down Expand Up @@ -114,9 +114,9 @@ TEST_F(BackdropFilterLayerTest, MultipleChildren) {
EXPECT_EQ(mock_layer1->paint_bounds(), child_path1.getBounds());
EXPECT_EQ(mock_layer2->paint_bounds(), child_path2.getBounds());
EXPECT_EQ(layer->paint_bounds(), children_bounds);
EXPECT_TRUE(mock_layer1->needs_painting());
EXPECT_TRUE(mock_layer2->needs_painting());
EXPECT_TRUE(layer->needs_painting());
EXPECT_TRUE(mock_layer1->needs_painting(paint_context()));
EXPECT_TRUE(mock_layer2->needs_painting(paint_context()));
EXPECT_TRUE(layer->needs_painting(paint_context()));
EXPECT_EQ(mock_layer1->parent_matrix(), initial_transform);
EXPECT_EQ(mock_layer2->parent_matrix(), initial_transform);

Expand Down Expand Up @@ -158,10 +158,10 @@ TEST_F(BackdropFilterLayerTest, Nested) {
EXPECT_EQ(mock_layer2->paint_bounds(), child_path2.getBounds());
EXPECT_EQ(layer1->paint_bounds(), children_bounds);
EXPECT_EQ(layer2->paint_bounds(), mock_layer2->paint_bounds());
EXPECT_TRUE(mock_layer1->needs_painting());
EXPECT_TRUE(mock_layer2->needs_painting());
EXPECT_TRUE(layer1->needs_painting());
EXPECT_TRUE(layer2->needs_painting());
EXPECT_TRUE(mock_layer1->needs_painting(paint_context()));
EXPECT_TRUE(mock_layer2->needs_painting(paint_context()));
EXPECT_TRUE(layer1->needs_painting(paint_context()));
EXPECT_TRUE(layer2->needs_painting(paint_context()));
EXPECT_EQ(mock_layer1->parent_matrix(), initial_transform);
EXPECT_EQ(mock_layer2->parent_matrix(), initial_transform);

Expand Down
28 changes: 14 additions & 14 deletions flow/layers/checkerboard_layertree_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ TEST_F(CheckerBoardLayerTest, ClipRectSaveLayerNotCheckBoard) {
EXPECT_TRUE(preroll_context()->mutators_stack.is_empty()); // Untouched
EXPECT_EQ(mock_layer->paint_bounds(), child_bounds);
EXPECT_EQ(layer->paint_bounds(), child_intersect_bounds);
EXPECT_TRUE(mock_layer->needs_painting());
EXPECT_TRUE(layer->needs_painting());
EXPECT_TRUE(mock_layer->needs_painting(paint_context()));
EXPECT_TRUE(layer->needs_painting(paint_context()));
EXPECT_EQ(mock_layer->parent_cull_rect(), intersect_bounds);
EXPECT_EQ(mock_layer->parent_matrix(), initial_matrix);
EXPECT_EQ(mock_layer->parent_mutators(),
Expand Down Expand Up @@ -93,8 +93,8 @@ TEST_F(CheckerBoardLayerTest, ClipRectSaveLayerCheckBoard) {
EXPECT_TRUE(preroll_context()->mutators_stack.is_empty()); // Untouched
EXPECT_EQ(mock_layer->paint_bounds(), child_bounds);
EXPECT_EQ(layer->paint_bounds(), child_intersect_bounds);
EXPECT_TRUE(mock_layer->needs_painting());
EXPECT_TRUE(layer->needs_painting());
EXPECT_TRUE(mock_layer->needs_painting(paint_context()));
EXPECT_TRUE(layer->needs_painting(paint_context()));
EXPECT_EQ(mock_layer->parent_cull_rect(), intersect_bounds);
EXPECT_EQ(mock_layer->parent_matrix(), initial_matrix);
EXPECT_EQ(mock_layer->parent_mutators(),
Expand Down Expand Up @@ -136,8 +136,8 @@ TEST_F(CheckerBoardLayerTest, ClipPathSaveLayerNotCheckBoard) {
EXPECT_TRUE(preroll_context()->mutators_stack.is_empty()); // Untouched
EXPECT_EQ(mock_layer->paint_bounds(), child_bounds);
EXPECT_EQ(layer->paint_bounds(), mock_layer->paint_bounds());
EXPECT_TRUE(mock_layer->needs_painting());
EXPECT_TRUE(layer->needs_painting());
EXPECT_TRUE(mock_layer->needs_painting(paint_context()));
EXPECT_TRUE(layer->needs_painting(paint_context()));
EXPECT_EQ(mock_layer->parent_cull_rect(), layer_bounds);
EXPECT_EQ(mock_layer->parent_matrix(), initial_matrix);
EXPECT_EQ(mock_layer->parent_mutators(), std::vector({Mutator(layer_path)}));
Expand Down Expand Up @@ -177,8 +177,8 @@ TEST_F(CheckerBoardLayerTest, ClipPathSaveLayerCheckBoard) {
EXPECT_TRUE(preroll_context()->mutators_stack.is_empty()); // Untouched
EXPECT_EQ(mock_layer->paint_bounds(), child_bounds);
EXPECT_EQ(layer->paint_bounds(), mock_layer->paint_bounds());
EXPECT_TRUE(mock_layer->needs_painting());
EXPECT_TRUE(layer->needs_painting());
EXPECT_TRUE(mock_layer->needs_painting(paint_context()));
EXPECT_TRUE(layer->needs_painting(paint_context()));
EXPECT_EQ(mock_layer->parent_cull_rect(), layer_bounds);
EXPECT_EQ(mock_layer->parent_matrix(), initial_matrix);
EXPECT_EQ(mock_layer->parent_mutators(), std::vector({Mutator(layer_path)}));
Expand Down Expand Up @@ -218,8 +218,8 @@ TEST_F(CheckerBoardLayerTest, ClipRRectSaveLayerNotCheckBoard) {
EXPECT_TRUE(preroll_context()->mutators_stack.is_empty()); // Untouched
EXPECT_EQ(mock_layer->paint_bounds(), child_bounds);
EXPECT_EQ(layer->paint_bounds(), mock_layer->paint_bounds());
EXPECT_TRUE(mock_layer->needs_painting());
EXPECT_TRUE(layer->needs_painting());
EXPECT_TRUE(mock_layer->needs_painting(paint_context()));
EXPECT_TRUE(layer->needs_painting(paint_context()));
EXPECT_EQ(mock_layer->parent_cull_rect(), layer_bounds);
EXPECT_EQ(mock_layer->parent_matrix(), initial_matrix);
EXPECT_EQ(mock_layer->parent_mutators(), std::vector({Mutator(layer_rrect)}));
Expand Down Expand Up @@ -259,8 +259,8 @@ TEST_F(CheckerBoardLayerTest, ClipRRectSaveLayerCheckBoard) {
EXPECT_TRUE(preroll_context()->mutators_stack.is_empty()); // Untouched
EXPECT_EQ(mock_layer->paint_bounds(), child_bounds);
EXPECT_EQ(layer->paint_bounds(), mock_layer->paint_bounds());
EXPECT_TRUE(mock_layer->needs_painting());
EXPECT_TRUE(layer->needs_painting());
EXPECT_TRUE(mock_layer->needs_painting(paint_context()));
EXPECT_TRUE(layer->needs_painting(paint_context()));
EXPECT_EQ(mock_layer->parent_cull_rect(), layer_bounds);
EXPECT_EQ(mock_layer->parent_matrix(), initial_matrix);
EXPECT_EQ(mock_layer->parent_mutators(), std::vector({Mutator(layer_rrect)}));
Expand Down Expand Up @@ -296,7 +296,7 @@ TEST_F(CheckerBoardLayerTest, PhysicalSaveLayerNotCheckBoard) {
EXPECT_EQ(layer->paint_bounds(),
PhysicalShapeLayer::ComputeShadowBounds(layer_path.getBounds(),
initial_elevation, 1.0f));
EXPECT_TRUE(layer->needs_painting());
EXPECT_TRUE(layer->needs_painting(paint_context()));
EXPECT_FALSE(layer->needs_system_composite());
EXPECT_EQ(layer->elevation(), initial_elevation);

Expand Down Expand Up @@ -340,7 +340,7 @@ TEST_F(CheckerBoardLayerTest, PhysicalSaveLayerCheckBoard) {
EXPECT_EQ(layer->paint_bounds(),
PhysicalShapeLayer::ComputeShadowBounds(layer_path.getBounds(),
initial_elevation, 1.0f));
EXPECT_TRUE(layer->needs_painting());
EXPECT_TRUE(layer->needs_painting(paint_context()));
EXPECT_FALSE(layer->needs_system_composite());
EXPECT_EQ(layer->elevation(), initial_elevation);

Expand Down
32 changes: 12 additions & 20 deletions flow/layers/clip_path_layer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,18 @@ void ClipPathLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {

SkRect previous_cull_rect = context->cull_rect;
SkRect clip_path_bounds = clip_path_.getBounds();
children_inside_clip_ = context->cull_rect.intersect(clip_path_bounds);
if (children_inside_clip_) {
TRACE_EVENT_INSTANT0("flutter", "children inside clip rect");

Layer::AutoPrerollSaveLayerState save =
Layer::AutoPrerollSaveLayerState::Create(context, UsesSaveLayer());
context->mutators_stack.PushClipPath(clip_path_);
SkRect child_paint_bounds = SkRect::MakeEmpty();
PrerollChildren(context, matrix, &child_paint_bounds);

if (child_paint_bounds.intersect(clip_path_bounds)) {
set_paint_bounds(child_paint_bounds);
}
context->mutators_stack.Pop();
context->cull_rect.intersect(clip_path_bounds);
Layer::AutoPrerollSaveLayerState save =
Layer::AutoPrerollSaveLayerState::Create(context, UsesSaveLayer());
context->mutators_stack.PushClipPath(clip_path_);

SkRect child_paint_bounds = SkRect::MakeEmpty();
PrerollChildren(context, matrix, &child_paint_bounds);
if (child_paint_bounds.intersect(clip_path_bounds)) {
set_paint_bounds(child_paint_bounds);
}

context->mutators_stack.Pop();
context->cull_rect = previous_cull_rect;
}

Expand All @@ -54,12 +51,7 @@ void ClipPathLayer::UpdateScene(std::shared_ptr<SceneUpdateContext> context) {

void ClipPathLayer::Paint(PaintContext& context) const {
TRACE_EVENT0("flutter", "ClipPathLayer::Paint");
FML_DCHECK(needs_painting());

if (!children_inside_clip_) {
TRACE_EVENT_INSTANT0("flutter", "children not inside clip rect, skipping");
return;
}
FML_DCHECK(needs_painting(context));

SkAutoCanvasRestore save(context.internal_nodes_canvas, true);
context.internal_nodes_canvas->clipPath(clip_path_,
Expand Down
1 change: 0 additions & 1 deletion flow/layers/clip_path_layer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ class ClipPathLayer : public ContainerLayer {
private:
SkPath clip_path_;
Clip clip_behavior_;
bool children_inside_clip_ = false;

FML_DISALLOW_COPY_AND_ASSIGN(ClipPathLayer);
};
Expand Down
44 changes: 24 additions & 20 deletions flow/layers/clip_path_layer_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,48 +28,52 @@ TEST_F(ClipPathLayerTest, PaintingEmptyLayerDies) {
EXPECT_EQ(preroll_context()->cull_rect, kGiantRect); // Untouched
EXPECT_TRUE(preroll_context()->mutators_stack.is_empty()); // Untouched
EXPECT_EQ(layer->paint_bounds(), kEmptyRect);
EXPECT_FALSE(layer->needs_painting());
EXPECT_FALSE(layer->needs_painting(paint_context()));

EXPECT_DEATH_IF_SUPPORTED(layer->Paint(paint_context()),
"needs_painting\\(\\)");
"needs_painting\\(context\\)");
}

TEST_F(ClipPathLayerTest, PaintBeforePrerollDies) {
const SkRect layer_bounds = SkRect::MakeXYWH(0.5, 1.0, 5.0, 6.0);
const SkPath layer_path = SkPath().addRect(layer_bounds);
auto layer = std::make_shared<ClipPathLayer>(layer_path, Clip::hardEdge);
EXPECT_EQ(layer->paint_bounds(), kEmptyRect);
EXPECT_FALSE(layer->needs_painting());
EXPECT_FALSE(layer->needs_painting(paint_context()));

EXPECT_DEATH_IF_SUPPORTED(layer->Paint(paint_context()),
"needs_painting\\(\\)");
"needs_painting\\(context\\)");
}

TEST_F(ClipPathLayerTest, PaintingCulledLayerDies) {
const SkMatrix initial_matrix = SkMatrix::Translate(0.5f, 1.0f);
const SkRect child_bounds = SkRect::MakeXYWH(1.0, 2.0, 2.0, 2.0);
const SkRect layer_bounds = SkRect::MakeXYWH(0.5, 1.0, 5.0, 6.0);
const SkRect distant_bounds = SkRect::MakeXYWH(100.0, 100.0, 10.0, 10.0);
const SkPath child_path = SkPath().addRect(child_bounds);
const SkPath layer_path = SkPath().addRect(layer_bounds);
auto mock_layer = std::make_shared<MockLayer>(child_path);
auto layer = std::make_shared<ClipPathLayer>(layer_path, Clip::hardEdge);
layer->Add(mock_layer);

preroll_context()->cull_rect = kEmptyRect; // Cull everything
preroll_context()->cull_rect = distant_bounds; // Cull these children

layer->Preroll(preroll_context(), initial_matrix);
EXPECT_EQ(preroll_context()->cull_rect, kEmptyRect); // Untouched
EXPECT_EQ(preroll_context()->cull_rect, distant_bounds); // Untouched
EXPECT_TRUE(preroll_context()->mutators_stack.is_empty()); // Untouched
EXPECT_EQ(mock_layer->paint_bounds(), kEmptyRect);
EXPECT_EQ(layer->paint_bounds(), kEmptyRect);
EXPECT_FALSE(mock_layer->needs_painting());
EXPECT_FALSE(layer->needs_painting());
EXPECT_EQ(mock_layer->parent_cull_rect(), kEmptyRect);
EXPECT_EQ(mock_layer->parent_matrix(), SkMatrix());
EXPECT_EQ(mock_layer->parent_mutators(), std::vector<Mutator>());
EXPECT_EQ(mock_layer->paint_bounds(), child_bounds);
EXPECT_EQ(layer->paint_bounds(), child_bounds);
EXPECT_TRUE(mock_layer->needs_painting(paint_context()));
EXPECT_TRUE(layer->needs_painting(paint_context()));
EXPECT_EQ(mock_layer->parent_cull_rect(), distant_bounds);
EXPECT_EQ(mock_layer->parent_matrix(), initial_matrix);
EXPECT_EQ(mock_layer->parent_mutators(), std::vector({Mutator(layer_path)}));

paint_context().internal_nodes_canvas->clipRect(distant_bounds, false);
EXPECT_FALSE(mock_layer->needs_painting(paint_context()));
EXPECT_FALSE(layer->needs_painting(paint_context()));
EXPECT_DEATH_IF_SUPPORTED(layer->Paint(paint_context()),
"needs_painting\\(\\)");
"needs_painting\\(context\\)");
}
#endif

Expand All @@ -96,8 +100,8 @@ TEST_F(ClipPathLayerTest, ChildOutsideBounds) {
EXPECT_TRUE(preroll_context()->mutators_stack.is_empty()); // Untouched
EXPECT_EQ(mock_layer->paint_bounds(), child_bounds);
EXPECT_EQ(layer->paint_bounds(), child_intersect_bounds);
EXPECT_TRUE(mock_layer->needs_painting());
EXPECT_TRUE(layer->needs_painting());
EXPECT_TRUE(mock_layer->needs_painting(paint_context()));
EXPECT_TRUE(layer->needs_painting(paint_context()));
EXPECT_EQ(mock_layer->parent_cull_rect(), intersect_bounds);
EXPECT_EQ(mock_layer->parent_matrix(), initial_matrix);
EXPECT_EQ(mock_layer->parent_mutators(), std::vector({Mutator(layer_path)}));
Expand Down Expand Up @@ -131,8 +135,8 @@ TEST_F(ClipPathLayerTest, FullyContainedChild) {
EXPECT_TRUE(preroll_context()->mutators_stack.is_empty()); // Untouched
EXPECT_EQ(mock_layer->paint_bounds(), child_bounds);
EXPECT_EQ(layer->paint_bounds(), mock_layer->paint_bounds());
EXPECT_TRUE(mock_layer->needs_painting());
EXPECT_TRUE(layer->needs_painting());
EXPECT_TRUE(mock_layer->needs_painting(paint_context()));
EXPECT_TRUE(layer->needs_painting(paint_context()));
EXPECT_EQ(mock_layer->parent_cull_rect(), layer_bounds);
EXPECT_EQ(mock_layer->parent_matrix(), initial_matrix);
EXPECT_EQ(mock_layer->parent_mutators(), std::vector({Mutator(layer_path)}));
Expand Down Expand Up @@ -173,8 +177,8 @@ TEST_F(ClipPathLayerTest, PartiallyContainedChild) {
EXPECT_TRUE(preroll_context()->mutators_stack.is_empty()); // Untouched
EXPECT_EQ(mock_layer->paint_bounds(), child_bounds);
EXPECT_EQ(layer->paint_bounds(), child_intersect_bounds);
EXPECT_TRUE(mock_layer->needs_painting());
EXPECT_TRUE(layer->needs_painting());
EXPECT_TRUE(mock_layer->needs_painting(paint_context()));
EXPECT_TRUE(layer->needs_painting(paint_context()));
EXPECT_EQ(mock_layer->parent_cull_rect(), intersect_bounds);
EXPECT_EQ(mock_layer->parent_matrix(), initial_matrix);
EXPECT_EQ(mock_layer->parent_mutators(), std::vector({Mutator(layer_path)}));
Expand Down
32 changes: 12 additions & 20 deletions flow/layers/clip_rect_layer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,18 @@ void ClipRectLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
TRACE_EVENT0("flutter", "ClipRectLayer::Preroll");

SkRect previous_cull_rect = context->cull_rect;
children_inside_clip_ = context->cull_rect.intersect(clip_rect_);
if (children_inside_clip_) {
TRACE_EVENT_INSTANT0("flutter", "children inside clip rect");

Layer::AutoPrerollSaveLayerState save =
Layer::AutoPrerollSaveLayerState::Create(context, UsesSaveLayer());
context->mutators_stack.PushClipRect(clip_rect_);
SkRect child_paint_bounds = SkRect::MakeEmpty();
PrerollChildren(context, matrix, &child_paint_bounds);

if (child_paint_bounds.intersect(clip_rect_)) {
set_paint_bounds(child_paint_bounds);
}
context->mutators_stack.Pop();
context->cull_rect.intersect(clip_rect_);
Layer::AutoPrerollSaveLayerState save =
Layer::AutoPrerollSaveLayerState::Create(context, UsesSaveLayer());
context->mutators_stack.PushClipRect(clip_rect_);

SkRect child_paint_bounds = SkRect::MakeEmpty();
PrerollChildren(context, matrix, &child_paint_bounds);
if (child_paint_bounds.intersect(clip_rect_)) {
set_paint_bounds(child_paint_bounds);
}

context->mutators_stack.Pop();
context->cull_rect = previous_cull_rect;
}

Expand All @@ -49,12 +46,7 @@ void ClipRectLayer::UpdateScene(std::shared_ptr<SceneUpdateContext> context) {

void ClipRectLayer::Paint(PaintContext& context) const {
TRACE_EVENT0("flutter", "ClipRectLayer::Paint");
FML_DCHECK(needs_painting());

if (!children_inside_clip_) {
TRACE_EVENT_INSTANT0("flutter", "children not inside clip rect, skipping");
return;
}
FML_DCHECK(needs_painting(context));

SkAutoCanvasRestore save(context.internal_nodes_canvas, true);
context.internal_nodes_canvas->clipRect(clip_rect_,
Expand Down
1 change: 0 additions & 1 deletion flow/layers/clip_rect_layer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class ClipRectLayer : public ContainerLayer {
private:
SkRect clip_rect_;
Clip clip_behavior_;
bool children_inside_clip_ = false;

FML_DISALLOW_COPY_AND_ASSIGN(ClipRectLayer);
};
Expand Down
Loading

0 comments on commit ecfe5ae

Please sign in to comment.