From 9f6e6bbd892c945dd321c97a852a568fea0d9e5b Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 5 Mar 2020 13:25:46 -0800 Subject: [PATCH 1/5] Fix bounds of image_filter_layer --- flow/layers/image_filter_layer.cc | 14 +++++- flow/layers/image_filter_layer_unittests.cc | 52 ++++++++++++++++----- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/flow/layers/image_filter_layer.cc b/flow/layers/image_filter_layer.cc index 119fddc181fb8..c44303a43f7d5 100644 --- a/flow/layers/image_filter_layer.cc +++ b/flow/layers/image_filter_layer.cc @@ -11,9 +11,21 @@ ImageFilterLayer::ImageFilterLayer(sk_sp filter) void ImageFilterLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { + TRACE_EVENT0("flutter", "ImageFilterLayer::Preroll"); + Layer::AutoPrerollSaveLayerState save = Layer::AutoPrerollSaveLayerState::Create(context); - ContainerLayer::Preroll(context, matrix); + + SkRect child_paint_bounds = SkRect::MakeEmpty(); + PrerollChildren(context, matrix, &child_paint_bounds); + if (filter_) { + const SkIRect filter_input_bounds = child_paint_bounds.roundOut(); + SkIRect filter_output_bounds = + filter_->filterBounds(filter_input_bounds, SkMatrix::I(), + SkImageFilter::kForward_MapDirection); + child_paint_bounds = SkRect::Make(filter_output_bounds); + } + set_paint_bounds(child_paint_bounds); if (!context->has_platform_view && context->raster_cache && SkRect::Intersects(context->cull_rect, paint_bounds())) { diff --git a/flow/layers/image_filter_layer_unittests.cc b/flow/layers/image_filter_layer_unittests.cc index 63357fbd89ce8..94f9f902a3107 100644 --- a/flow/layers/image_filter_layer_unittests.cc +++ b/flow/layers/image_filter_layer_unittests.cc @@ -83,8 +83,11 @@ TEST_F(ImageFilterLayerTest, SimpleFilter) { auto layer = std::make_shared(layer_filter); layer->Add(mock_layer); + const SkRect child_rounded_bounds = + SkRect::MakeLTRB(5.0f, 6.0f, 21.0f, 22.0f); + layer->Preroll(preroll_context(), initial_transform); - EXPECT_EQ(layer->paint_bounds(), child_bounds); + EXPECT_EQ(layer->paint_bounds(), child_rounded_bounds); EXPECT_TRUE(layer->needs_painting()); EXPECT_EQ(mock_layer->parent_matrix(), initial_transform); @@ -96,8 +99,8 @@ TEST_F(ImageFilterLayerTest, SimpleFilter) { MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}}, MockCanvas::DrawCall{ - 1, MockCanvas::SaveLayerData{child_bounds, filter_paint, - nullptr, 2}}, + 1, MockCanvas::SaveLayerData{child_rounded_bounds, + filter_paint, nullptr, 2}}, MockCanvas::DrawCall{ 2, MockCanvas::DrawPathData{child_path, child_paint}}, MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}}, @@ -105,6 +108,26 @@ TEST_F(ImageFilterLayerTest, SimpleFilter) { })); } +TEST_F(ImageFilterLayerTest, SimpleFilterBounds) { + const SkMatrix initial_transform = SkMatrix::MakeTrans(0.5f, 1.0f); + const SkRect child_bounds = SkRect::MakeLTRB(5.0f, 6.0f, 20.5f, 21.5f); + const SkPath child_path = SkPath().addRect(child_bounds); + const SkPaint child_paint = SkPaint(SkColors::kYellow); + const SkMatrix filter_transform = SkMatrix::MakeScale(2.0, 2.0); + auto layer_filter = SkImageFilter::MakeMatrixFilter( + filter_transform, SkFilterQuality::kMedium_SkFilterQuality, nullptr); + auto mock_layer = std::make_shared(child_path, child_paint); + auto layer = std::make_shared(layer_filter); + layer->Add(mock_layer); + + const SkRect filter_bounds = SkRect::MakeLTRB(10.0f, 12.0f, 42.0f, 44.0f); + + layer->Preroll(preroll_context(), initial_transform); + EXPECT_EQ(layer->paint_bounds(), filter_bounds); + EXPECT_TRUE(layer->needs_painting()); + EXPECT_EQ(mock_layer->parent_matrix(), initial_transform); +} + TEST_F(ImageFilterLayerTest, MultipleChildren) { const SkMatrix initial_transform = SkMatrix::MakeTrans(0.5f, 1.0f); const SkRect child_bounds = SkRect::MakeLTRB(5.0f, 6.0f, 2.5f, 3.5f); @@ -123,10 +146,12 @@ TEST_F(ImageFilterLayerTest, MultipleChildren) { SkRect children_bounds = child_path1.getBounds(); children_bounds.join(child_path2.getBounds()); + SkRect children_rounded_bounds = SkRect::Make(children_bounds.roundOut()); + layer->Preroll(preroll_context(), initial_transform); 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_EQ(layer->paint_bounds(), children_rounded_bounds); EXPECT_TRUE(mock_layer1->needs_painting()); EXPECT_TRUE(mock_layer2->needs_painting()); EXPECT_TRUE(layer->needs_painting()); @@ -141,8 +166,8 @@ TEST_F(ImageFilterLayerTest, MultipleChildren) { {MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}}, MockCanvas::DrawCall{ - 1, MockCanvas::SaveLayerData{children_bounds, filter_paint, - nullptr, 2}}, + 1, MockCanvas::SaveLayerData{children_rounded_bounds, + filter_paint, nullptr, 2}}, MockCanvas::DrawCall{ 2, MockCanvas::DrawPathData{child_path1, child_paint1}}, MockCanvas::DrawCall{ @@ -173,11 +198,16 @@ TEST_F(ImageFilterLayerTest, Nested) { SkRect children_bounds = child_path1.getBounds(); children_bounds.join(child_path2.getBounds()); + const SkRect children_rounded_bounds = + SkRect::Make(children_bounds.roundOut()); + const SkRect mock_layer2_rounded_bounds = + SkRect::Make(child_path2.getBounds().roundOut()); + layer1->Preroll(preroll_context(), initial_transform); EXPECT_EQ(mock_layer1->paint_bounds(), child_path1.getBounds()); 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_EQ(layer1->paint_bounds(), children_rounded_bounds); + EXPECT_EQ(layer2->paint_bounds(), mock_layer2_rounded_bounds); EXPECT_TRUE(mock_layer1->needs_painting()); EXPECT_TRUE(mock_layer2->needs_painting()); EXPECT_TRUE(layer1->needs_painting()); @@ -194,14 +224,14 @@ TEST_F(ImageFilterLayerTest, Nested) { MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}}, MockCanvas::DrawCall{ - 1, MockCanvas::SaveLayerData{children_bounds, filter_paint1, - nullptr, 2}}, + 1, MockCanvas::SaveLayerData{children_rounded_bounds, + filter_paint1, nullptr, 2}}, MockCanvas::DrawCall{ 2, MockCanvas::DrawPathData{child_path1, child_paint1}}, MockCanvas::DrawCall{2, MockCanvas::SaveData{3}}, MockCanvas::DrawCall{3, MockCanvas::SetMatrixData{SkMatrix()}}, MockCanvas::DrawCall{ - 3, MockCanvas::SaveLayerData{child_path2.getBounds(), + 3, MockCanvas::SaveLayerData{mock_layer2_rounded_bounds, filter_paint2, nullptr, 4}}, MockCanvas::DrawCall{ 4, MockCanvas::DrawPathData{child_path2, child_paint2}}, From 2f971c827803fb89b3c10a4f293904d2124de0b0 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 10 Mar 2020 14:12:27 -0700 Subject: [PATCH 2/5] use unfiltered bounds for child savelayer --- flow/layers/image_filter_layer.cc | 13 +++++++------ flow/layers/image_filter_layer.h | 1 + 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/flow/layers/image_filter_layer.cc b/flow/layers/image_filter_layer.cc index c44303a43f7d5..731fafc5c56e6 100644 --- a/flow/layers/image_filter_layer.cc +++ b/flow/layers/image_filter_layer.cc @@ -16,16 +16,17 @@ void ImageFilterLayer::Preroll(PrerollContext* context, Layer::AutoPrerollSaveLayerState save = Layer::AutoPrerollSaveLayerState::Create(context); - SkRect child_paint_bounds = SkRect::MakeEmpty(); - PrerollChildren(context, matrix, &child_paint_bounds); + child_paint_bounds_ = SkRect::MakeEmpty(); + PrerollChildren(context, matrix, &child_paint_bounds_); if (filter_) { - const SkIRect filter_input_bounds = child_paint_bounds.roundOut(); + const SkIRect filter_input_bounds = child_paint_bounds_.roundOut(); SkIRect filter_output_bounds = filter_->filterBounds(filter_input_bounds, SkMatrix::I(), SkImageFilter::kForward_MapDirection); - child_paint_bounds = SkRect::Make(filter_output_bounds); + set_paint_bounds(SkRect::Make(filter_output_bounds)); + } else { + set_paint_bounds(child_paint_bounds_); } - set_paint_bounds(child_paint_bounds); if (!context->has_platform_view && context->raster_cache && SkRect::Intersects(context->cull_rect, paint_bounds())) { @@ -61,7 +62,7 @@ void ImageFilterLayer::Paint(PaintContext& context) const { paint.setImageFilter(filter_); Layer::AutoSaveLayer save_layer = - Layer::AutoSaveLayer::Create(context, paint_bounds(), &paint); + Layer::AutoSaveLayer::Create(context, child_paint_bounds_, &paint); PaintChildren(context); } diff --git a/flow/layers/image_filter_layer.h b/flow/layers/image_filter_layer.h index 30ec99935ff0a..8e40a9ab339ba 100644 --- a/flow/layers/image_filter_layer.h +++ b/flow/layers/image_filter_layer.h @@ -21,6 +21,7 @@ class ImageFilterLayer : public ContainerLayer { private: sk_sp filter_; + SkRect child_paint_bounds_; FML_DISALLOW_COPY_AND_ASSIGN(ImageFilterLayer); }; From 0e34df60d79ad88e952b68f1bf80c61473a00762 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 10 Mar 2020 15:17:29 -0700 Subject: [PATCH 3/5] fix image_filter_layer unit tests to match new child bounds --- flow/layers/image_filter_layer_unittests.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/flow/layers/image_filter_layer_unittests.cc b/flow/layers/image_filter_layer_unittests.cc index 94f9f902a3107..1f95606df9930 100644 --- a/flow/layers/image_filter_layer_unittests.cc +++ b/flow/layers/image_filter_layer_unittests.cc @@ -99,8 +99,8 @@ TEST_F(ImageFilterLayerTest, SimpleFilter) { MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}}, MockCanvas::DrawCall{ - 1, MockCanvas::SaveLayerData{child_rounded_bounds, - filter_paint, nullptr, 2}}, + 1, MockCanvas::SaveLayerData{child_bounds, filter_paint, + nullptr, 2}}, MockCanvas::DrawCall{ 2, MockCanvas::DrawPathData{child_path, child_paint}}, MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}}, @@ -166,8 +166,8 @@ TEST_F(ImageFilterLayerTest, MultipleChildren) { {MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}}, MockCanvas::DrawCall{ - 1, MockCanvas::SaveLayerData{children_rounded_bounds, - filter_paint, nullptr, 2}}, + 1, MockCanvas::SaveLayerData{children_bounds, filter_paint, + nullptr, 2}}, MockCanvas::DrawCall{ 2, MockCanvas::DrawPathData{child_path1, child_paint1}}, MockCanvas::DrawCall{ @@ -197,7 +197,7 @@ TEST_F(ImageFilterLayerTest, Nested) { layer1->Add(layer2); SkRect children_bounds = child_path1.getBounds(); - children_bounds.join(child_path2.getBounds()); + children_bounds.join(SkRect::Make(child_path2.getBounds().roundOut())); const SkRect children_rounded_bounds = SkRect::Make(children_bounds.roundOut()); const SkRect mock_layer2_rounded_bounds = @@ -224,14 +224,14 @@ TEST_F(ImageFilterLayerTest, Nested) { MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}}, MockCanvas::DrawCall{ - 1, MockCanvas::SaveLayerData{children_rounded_bounds, - filter_paint1, nullptr, 2}}, + 1, MockCanvas::SaveLayerData{children_bounds, filter_paint1, + nullptr, 2}}, MockCanvas::DrawCall{ 2, MockCanvas::DrawPathData{child_path1, child_paint1}}, MockCanvas::DrawCall{2, MockCanvas::SaveData{3}}, MockCanvas::DrawCall{3, MockCanvas::SetMatrixData{SkMatrix()}}, MockCanvas::DrawCall{ - 3, MockCanvas::SaveLayerData{mock_layer2_rounded_bounds, + 3, MockCanvas::SaveLayerData{child_path2.getBounds(), filter_paint2, nullptr, 4}}, MockCanvas::DrawCall{ 4, MockCanvas::DrawPathData{child_path2, child_paint2}}, From 18fe3562d1d61f663e6d9137c3f3190c4289a85f Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 10 Mar 2020 16:04:04 -0700 Subject: [PATCH 4/5] verify mock drawing calls for new bounds test --- flow/layers/image_filter_layer_unittests.cc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/flow/layers/image_filter_layer_unittests.cc b/flow/layers/image_filter_layer_unittests.cc index 1f95606df9930..f4f621a529a08 100644 --- a/flow/layers/image_filter_layer_unittests.cc +++ b/flow/layers/image_filter_layer_unittests.cc @@ -126,6 +126,22 @@ TEST_F(ImageFilterLayerTest, SimpleFilterBounds) { EXPECT_EQ(layer->paint_bounds(), filter_bounds); EXPECT_TRUE(layer->needs_painting()); EXPECT_EQ(mock_layer->parent_matrix(), initial_transform); + + SkPaint filter_paint; + filter_paint.setImageFilter(layer_filter); + layer->Paint(paint_context()); + EXPECT_EQ(mock_canvas().draw_calls(), + std::vector({ + MockCanvas::DrawCall{0, MockCanvas::SaveData{1}}, + MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}}, + MockCanvas::DrawCall{ + 1, MockCanvas::SaveLayerData{child_bounds, filter_paint, + nullptr, 2}}, + MockCanvas::DrawCall{ + 2, MockCanvas::DrawPathData{child_path, child_paint}}, + MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}}, + MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}, + })); } TEST_F(ImageFilterLayerTest, MultipleChildren) { From da42c89b5a98f31cbf83ba55a302a6fa19fd5ddb Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 12 Mar 2020 15:45:10 -0700 Subject: [PATCH 5/5] add comment about change in save_layer bounds --- flow/layers/image_filter_layer.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/flow/layers/image_filter_layer.cc b/flow/layers/image_filter_layer.cc index 731fafc5c56e6..212c396b6efbf 100644 --- a/flow/layers/image_filter_layer.cc +++ b/flow/layers/image_filter_layer.cc @@ -61,6 +61,10 @@ void ImageFilterLayer::Paint(PaintContext& context) const { SkPaint paint; paint.setImageFilter(filter_); + // Normally a save_layer is sized to the current layer bounds, but in this + // case the bounds of the child may not be the same as the filtered version + // so we use the child_paint_bounds_ which were snapshotted from the + // Preroll on the children before we adjusted them based on the filter. Layer::AutoSaveLayer save_layer = Layer::AutoSaveLayer::Create(context, child_paint_bounds_, &paint); PaintChildren(context);