diff --git a/engine/src/flutter/display_list/display_list_image_filter.h b/engine/src/flutter/display_list/display_list_image_filter.h index ea98b494508..2cea0007066 100644 --- a/engine/src/flutter/display_list/display_list_image_filter.h +++ b/engine/src/flutter/display_list/display_list_image_filter.h @@ -102,10 +102,9 @@ class DlImageFilter // based on the supplied input bounds where both are measured in the local // (untransformed) coordinate space. // - // The method will return a pointer to the output_bounds parameter if it - // can successfully compute the output bounds of the filter, otherwise the - // method will return a nullptr and the output_bounds will be filled with - // a best guess for the answer, even if just a copy of the input_bounds. + // The output bounds parameter must be supplied and the method will either + // return a pointer to it with the result filled in, or it will return a + // nullptr if it cannot determine the results. virtual SkRect* map_local_bounds(const SkRect& input_bounds, SkRect& output_bounds) const = 0; @@ -116,108 +115,12 @@ class DlImageFilter // is used in a rendering operation (for example, the blur radius of a // Blur filter will expand based on the ctm). // - // The method will return a pointer to the output_bounds parameter if it - // can successfully compute the output bounds of the filter, otherwise the - // method will return a nullptr and the output_bounds will be filled with - // a best guess for the answer, even if just a copy of the input_bounds. + // The output bounds parameter must be supplied and the method will either + // return a pointer to it with the result filled in, or it will return a + // nullptr if it cannot determine the results. virtual SkIRect* map_device_bounds(const SkIRect& input_bounds, const SkMatrix& ctm, SkIRect& output_bounds) const = 0; - - // Return the input bounds that will be needed in order for the filter to - // properly fill the indicated output_bounds under the specified - // transformation matrix. Both output_bounds and input_bounds are taken to - // be relative to the transformed coordinate space of the provided |ctm|. - // - // The method will return a pointer to the input_bounds parameter if it - // can successfully compute the required input bounds, otherwise the - // method will return a nullptr and the input_bounds will be filled with - // a best guess for the answer, even if just a copy of the output_bounds. - virtual SkIRect* get_input_device_bounds(const SkIRect& output_bounds, - const SkMatrix& ctm, - SkIRect& input_bounds) const = 0; - - protected: - static SkVector map_vectors_affine(const SkMatrix& ctm, - SkScalar x, - SkScalar y) { - FML_DCHECK(SkScalarIsFinite(x) && x >= 0); - FML_DCHECK(SkScalarIsFinite(y) && y >= 0); - FML_DCHECK(ctm.isFinite() && !ctm.hasPerspective()); - - // The x and y scalars would have been used to expand a local space - // rectangle which is then transformed by ctm. In order to do the - // expansion correctly, we should look at the relevant math. The - // 4 corners will be moved outward by the following vectors: - // (UL,UR,LR,LL) = ((-x, -y), (+x, -y), (+x, +y), (-x, +y)) - // After applying the transform, each of these vectors could be - // pointing in any direction so we need to examine each transformed - // delta vector and how it affected the bounds. - // Looking at just the affine 2x3 entries of the CTM we can delta - // transform these corner offsets and get the following: - // UL = dCTM(-x, -y) = (- x*m00 - y*m01, - x*m10 - y*m11) - // UR = dCTM(+x, -y) = ( x*m00 - y*m01, x*m10 - y*m11) - // LR = dCTM(+x, +y) = ( x*m00 + y*m01, x*m10 + y*m11) - // LL = dCTM(-x, +y) = (- x*m00 + y*m01, - x*m10 + y*m11) - // The X vectors are all some variation of adding or subtracting - // the sum of x*m00 and y*m01 or their difference. Similarly the Y - // vectors are +/- the associated sum/difference of x*m10 and y*m11. - // The largest displacements, both left/right or up/down, will - // happen when the signs of the m00/m01/m10/m11 matrix entries - // coincide with the signs of the scalars, i.e. are all positive. - return {x * abs(ctm[0]) + y * abs(ctm[1]), - x * abs(ctm[3]) + y * abs(ctm[4])}; - } - - static SkIRect* inset_device_bounds(const SkIRect& input_bounds, - SkScalar radius_x, - SkScalar radius_y, - const SkMatrix& ctm, - SkIRect& output_bounds) { - if (ctm.isFinite()) { - if (ctm.hasPerspective()) { - SkMatrix inverse; - if (ctm.invert(&inverse)) { - SkRect local_bounds = inverse.mapRect(SkRect::Make(input_bounds)); - local_bounds.inset(radius_x, radius_y); - output_bounds = ctm.mapRect(local_bounds).roundOut(); - return &output_bounds; - } - } else { - SkVector device_radius = map_vectors_affine(ctm, radius_x, radius_y); - output_bounds = input_bounds.makeInset(floor(device_radius.fX), // - floor(device_radius.fY)); - return &output_bounds; - } - } - output_bounds = input_bounds; - return nullptr; - } - - static SkIRect* outset_device_bounds(const SkIRect& input_bounds, - SkScalar radius_x, - SkScalar radius_y, - const SkMatrix& ctm, - SkIRect& output_bounds) { - if (ctm.isFinite()) { - if (ctm.hasPerspective()) { - SkMatrix inverse; - if (ctm.invert(&inverse)) { - SkRect local_bounds = inverse.mapRect(SkRect::Make(input_bounds)); - local_bounds.outset(radius_x, radius_y); - output_bounds = ctm.mapRect(local_bounds).roundOut(); - return &output_bounds; - } - } else { - SkVector device_radius = map_vectors_affine(ctm, radius_x, radius_y); - output_bounds = input_bounds.makeOutset(ceil(device_radius.fX), // - ceil(device_radius.fY)); - return &output_bounds; - } - } - output_bounds = input_bounds; - return nullptr; - } }; class DlBlurImageFilter final : public DlImageFilter { @@ -251,15 +154,16 @@ class DlBlurImageFilter final : public DlImageFilter { SkIRect* map_device_bounds(const SkIRect& input_bounds, const SkMatrix& ctm, SkIRect& output_bounds) const override { - return outset_device_bounds(input_bounds, sigma_x_ * 3.0, sigma_y_ * 3.0, - ctm, output_bounds); - } - - SkIRect* get_input_device_bounds(const SkIRect& output_bounds, - const SkMatrix& ctm, - SkIRect& input_bounds) const override { - // Blurs are symmetric in terms of output-for-input and input-for-output - return map_device_bounds(output_bounds, ctm, input_bounds); + SkVector device_sigma = ctm.mapVector(sigma_x_ * 3, sigma_y_ * 3); + if (!SkScalarIsFinite(device_sigma.fX)) { + device_sigma.fX = 0; + } + if (!SkScalarIsFinite(device_sigma.fY)) { + device_sigma.fY = 0; + } + output_bounds = input_bounds.makeOutset(ceil(abs(device_sigma.fX)), + ceil(abs(device_sigma.fY))); + return &output_bounds; } SkScalar sigma_x() const { return sigma_x_; } @@ -313,15 +217,16 @@ class DlDilateImageFilter final : public DlImageFilter { SkIRect* map_device_bounds(const SkIRect& input_bounds, const SkMatrix& ctm, SkIRect& output_bounds) const override { - return outset_device_bounds(input_bounds, radius_x_, radius_y_, ctm, - output_bounds); - } - - SkIRect* get_input_device_bounds(const SkIRect& output_bounds, - const SkMatrix& ctm, - SkIRect& input_bounds) const override { - return inset_device_bounds(output_bounds, radius_x_, radius_y_, ctm, - input_bounds); + SkVector device_radius = ctm.mapVector(radius_x_, radius_y_); + if (!SkScalarIsFinite(device_radius.fX)) { + device_radius.fX = 0; + } + if (!SkScalarIsFinite(device_radius.fY)) { + device_radius.fY = 0; + } + output_bounds = input_bounds.makeOutset(ceil(abs(device_radius.fX)), + ceil(abs(device_radius.fY))); + return &output_bounds; } SkScalar radius_x() const { return radius_x_; } @@ -365,22 +270,23 @@ class DlErodeImageFilter final : public DlImageFilter { SkRect* map_local_bounds(const SkRect& input_bounds, SkRect& output_bounds) const override { - output_bounds = input_bounds.makeInset(radius_x_, radius_y_); + output_bounds = input_bounds.makeOutset(radius_x_, radius_y_); return &output_bounds; } SkIRect* map_device_bounds(const SkIRect& input_bounds, const SkMatrix& ctm, SkIRect& output_bounds) const override { - return inset_device_bounds(input_bounds, radius_x_, radius_y_, ctm, - output_bounds); - } - - SkIRect* get_input_device_bounds(const SkIRect& output_bounds, - const SkMatrix& ctm, - SkIRect& input_bounds) const override { - return outset_device_bounds(output_bounds, radius_x_, radius_y_, ctm, - input_bounds); + SkVector device_radius = ctm.mapVector(radius_x_, radius_y_); + if (!SkScalarIsFinite(device_radius.fX)) { + device_radius.fX = 0; + } + if (!SkScalarIsFinite(device_radius.fY)) { + device_radius.fY = 0; + } + output_bounds = input_bounds.makeOutset(ceil(abs(device_radius.fX)), + ceil(abs(device_radius.fY))); + return &output_bounds; } SkScalar radius_x() const { return radius_x_; } @@ -447,23 +353,6 @@ class DlMatrixImageFilter final : public DlImageFilter { return &output_bounds; } - SkIRect* get_input_device_bounds(const SkIRect& output_bounds, - const SkMatrix& ctm, - SkIRect& input_bounds) const override { - SkMatrix matrix = SkMatrix::Concat(ctm, matrix_); - SkMatrix inverse; - if (!matrix.invert(&inverse)) { - input_bounds = output_bounds; - return nullptr; - } - inverse.postConcat(ctm); - SkRect bounds; - bounds.set(output_bounds); - inverse.mapRect(&bounds); - input_bounds = bounds.roundOut(); - return &input_bounds; - } - sk_sp skia_object() const override { return SkImageFilters::MatrixTransform(matrix_, ToSk(sampling_), nullptr); } @@ -520,61 +409,41 @@ class DlComposeImageFilter final : public DlImageFilter { SkRect* map_local_bounds(const SkRect& input_bounds, SkRect& output_bounds) const override { - SkRect cur_bounds = input_bounds; - // We set this result in case neither filter is present. - output_bounds = input_bounds; + SkRect* ret = &output_bounds; if (inner_) { - if (!inner_->map_local_bounds(cur_bounds, output_bounds)) { - return nullptr; - } - cur_bounds = output_bounds; - } - if (outer_) { - if (!outer_->map_local_bounds(cur_bounds, output_bounds)) { - return nullptr; + if (!inner_->map_local_bounds(input_bounds, output_bounds)) { + ret = nullptr; } } - return &output_bounds; + if (ret && outer_) { + if (!outer_->map_local_bounds(input_bounds, output_bounds)) { + ret = nullptr; + } + } + if (!ret) { + output_bounds = input_bounds; + } + return ret; } SkIRect* map_device_bounds(const SkIRect& input_bounds, const SkMatrix& ctm, SkIRect& output_bounds) const override { - SkIRect cur_bounds = input_bounds; - // We set this result in case neither filter is present. - output_bounds = input_bounds; + SkIRect* ret = &output_bounds; if (inner_) { - if (!inner_->map_device_bounds(cur_bounds, ctm, output_bounds)) { - return nullptr; - } - cur_bounds = output_bounds; - } - if (outer_) { - if (!outer_->map_device_bounds(cur_bounds, ctm, output_bounds)) { - return nullptr; + if (!inner_->map_device_bounds(input_bounds, ctm, output_bounds)) { + ret = nullptr; } } - return &output_bounds; - } - - SkIRect* get_input_device_bounds(const SkIRect& output_bounds, - const SkMatrix& ctm, - SkIRect& input_bounds) const override { - SkIRect cur_bounds = output_bounds; - // We set this result in case neither filter is present. - input_bounds = output_bounds; - if (outer_) { - if (!outer_->get_input_device_bounds(cur_bounds, ctm, input_bounds)) { - return nullptr; - } - cur_bounds = output_bounds; - } - if (inner_) { - if (!inner_->get_input_device_bounds(cur_bounds, ctm, input_bounds)) { - return nullptr; + if (ret && outer_) { + if (!outer_->map_device_bounds(input_bounds, ctm, output_bounds)) { + ret = nullptr; } } - return &input_bounds; + if (!ret) { + output_bounds = input_bounds; + } + return ret; } sk_sp skia_object() const override { @@ -644,12 +513,6 @@ class DlColorFilterImageFilter final : public DlImageFilter { return modifies_transparent_black() ? nullptr : &output_bounds; } - SkIRect* get_input_device_bounds(const SkIRect& output_bounds, - const SkMatrix& ctm, - SkIRect& input_bounds) const override { - return map_device_bounds(output_bounds, ctm, input_bounds); - } - sk_sp skia_object() const override { return SkImageFilters::ColorFilter(color_filter_->skia_object(), nullptr); } @@ -701,8 +564,8 @@ class DlUnknownImageFilter final : public DlImageFilter { SkRect* map_local_bounds(const SkRect& input_bounds, SkRect& output_bounds) const override { + output_bounds = input_bounds; if (modifies_transparent_black()) { - output_bounds = input_bounds; return nullptr; } output_bounds = sk_filter_->computeFastBounds(input_bounds); @@ -712,8 +575,8 @@ class DlUnknownImageFilter final : public DlImageFilter { SkIRect* map_device_bounds(const SkIRect& input_bounds, const SkMatrix& ctm, SkIRect& output_bounds) const override { + output_bounds = input_bounds; if (modifies_transparent_black()) { - output_bounds = input_bounds; return nullptr; } output_bounds = sk_filter_->filterBounds( @@ -721,18 +584,6 @@ class DlUnknownImageFilter final : public DlImageFilter { return &output_bounds; } - SkIRect* get_input_device_bounds(const SkIRect& output_bounds, - const SkMatrix& ctm, - SkIRect& input_bounds) const override { - if (modifies_transparent_black()) { - input_bounds = output_bounds; - return nullptr; - } - input_bounds = sk_filter_->filterBounds( - output_bounds, ctm, SkImageFilter::kReverse_MapDirection); - return &input_bounds; - } - sk_sp skia_object() const override { return sk_filter_; } virtual ~DlUnknownImageFilter() = default; diff --git a/engine/src/flutter/display_list/display_list_image_filter_unittests.cc b/engine/src/flutter/display_list/display_list_image_filter_unittests.cc index 6a3f1935584..729de1f242b 100644 --- a/engine/src/flutter/display_list/display_list_image_filter_unittests.cc +++ b/engine/src/flutter/display_list/display_list_image_filter_unittests.cc @@ -142,135 +142,6 @@ TEST(DisplayListImageFilter, FromSkiaColorFilterImageFilter) { ASSERT_NE(filter->asColorFilter(), nullptr); } -// SkRect::contains treats the rect as a half-open interval which is -// appropriate for so many operations. Unfortunately, we are using -// it here to test containment of the corners of a transformed quad -// so the corners of the quad that are measured against the right -// and bottom edges are contained even if they are on the right or -// bottom edge. This method does the "all sides inclusive" version -// of SkRect::contains. -static bool containsInclusive(const SkRect rect, const SkPoint p) { - // Test with a slight offset of 1E-9 to "forgive" IEEE bit-rounding - // Ending up with bounds that are off by 1E-9 (these numbers are all - // being tested in device space with this method) will be off by a - // negligible amount of a pixel that wouldn't contribute to changing - // the color of a pixel. - return (p.fX >= rect.fLeft - 1E-9 && // - p.fX <= rect.fRight + 1E-9 && // - p.fY >= rect.fTop - 1E-9 && // - p.fY <= rect.fBottom + 1E-9); -} - -static bool containsInclusive(const SkRect rect, const SkPoint quad[4]) { - return (containsInclusive(rect, quad[0]) && // - containsInclusive(rect, quad[1]) && // - containsInclusive(rect, quad[2]) && // - containsInclusive(rect, quad[3])); -} - -static bool containsInclusive(const SkIRect rect, const SkPoint quad[4]) { - return containsInclusive(SkRect::Make(rect), quad); -} - -static bool containsInclusive(const SkIRect rect, const SkRect bounds) { - return (bounds.fLeft >= rect.fLeft - 1E-9 && - bounds.fTop >= rect.fTop - 1E-9 && - bounds.fRight <= rect.fRight + 1E-9 && - bounds.fBottom <= rect.fBottom + 1E-9); -} - -// Used to verify that the expected output bounds and revers engineered -// "input bounds for output bounds" rectangles are included in the rectangle -// returned from the various bounds computation methods under the specified -// matrix. -static void TestBoundsWithMatrix(const DlImageFilter& filter, - const SkMatrix& matrix, - const SkRect& sourceBounds, - const SkPoint expectedLocalOutputQuad[4]) { - SkRect deviceInputBounds = matrix.mapRect(sourceBounds); - SkPoint expectedDeviceOutputQuad[4]; - matrix.mapPoints(expectedDeviceOutputQuad, expectedLocalOutputQuad, 4); - - SkIRect deviceFilterIBounds; - ASSERT_EQ(filter.map_device_bounds(deviceInputBounds.roundOut(), matrix, - deviceFilterIBounds), - &deviceFilterIBounds); - ASSERT_TRUE(containsInclusive(deviceFilterIBounds, expectedDeviceOutputQuad)); - - SkIRect reverseInputIBounds; - ASSERT_EQ(filter.get_input_device_bounds(deviceFilterIBounds, matrix, - reverseInputIBounds), - &reverseInputIBounds); - ASSERT_TRUE(containsInclusive(reverseInputIBounds, deviceInputBounds)); -} - -static void TestInvalidBounds(const DlImageFilter& filter, - const SkMatrix& matrix, - const SkRect& localInputBounds) { - SkIRect deviceInputBounds = matrix.mapRect(localInputBounds).roundOut(); - - SkRect localFilterBounds; - ASSERT_EQ(filter.map_local_bounds(localInputBounds, localFilterBounds), - nullptr); - ASSERT_EQ(localFilterBounds, localInputBounds); - - SkIRect deviceFilterIBounds; - ASSERT_EQ( - filter.map_device_bounds(deviceInputBounds, matrix, deviceFilterIBounds), - nullptr); - ASSERT_EQ(deviceFilterIBounds, deviceInputBounds); - - SkIRect reverseInputIBounds; - ASSERT_EQ(filter.get_input_device_bounds(deviceInputBounds, matrix, - reverseInputIBounds), - nullptr); - ASSERT_EQ(reverseInputIBounds, deviceInputBounds); -} - -// localInputBounds is a sample bounds for testing as input to the filter. -// localExpectOutputBounds is the theoretical output bounds for applying -// the filter to the localInputBounds. -// localExpectInputBounds is the theoretical input bounds required for the -// filter to cover the localExpectOutputBounds -// If either of the expected bounds are nullptr then the bounds methods will -// be assumed to be unable to perform their computations for the given -// image filter and will be returning null. -static void TestBounds(const DlImageFilter& filter, - const SkRect& sourceBounds, - const SkPoint expectedLocalOutputQuad[4]) { - SkRect localFilterBounds; - ASSERT_EQ(filter.map_local_bounds(sourceBounds, localFilterBounds), - &localFilterBounds); - ASSERT_TRUE(containsInclusive(localFilterBounds, expectedLocalOutputQuad)); - - for (int scale = 1; scale <= 4; scale++) { - for (int skew = 0; skew < 8; skew++) { - for (int degrees = 0; degrees <= 360; degrees += 15) { - SkMatrix matrix; - matrix.setScale(scale, scale); - matrix.postSkew(skew / 8.0, skew / 8.0); - matrix.postRotate(degrees); - ASSERT_TRUE(matrix.invert(nullptr)); - TestBoundsWithMatrix(filter, matrix, sourceBounds, - expectedLocalOutputQuad); - matrix.setPerspX(0.01); - matrix.setPerspY(0.01); - ASSERT_TRUE(matrix.invert(nullptr)); - TestBoundsWithMatrix(filter, matrix, sourceBounds, - expectedLocalOutputQuad); - } - } - } -} - -static void TestBounds(const DlImageFilter& filter, - const SkRect& sourceBounds, - const SkRect& expectedLocalOutputBounds) { - SkPoint expectedLocalOutputQuad[4]; - expectedLocalOutputBounds.toQuad(expectedLocalOutputQuad); - TestBounds(filter, sourceBounds, expectedLocalOutputQuad); -} - TEST(DisplayListImageFilter, BlurConstructor) { DlBlurImageFilter filter(5.0, 6.0, DlTileMode::kMirror); } @@ -315,14 +186,6 @@ TEST(DisplayListImageFilter, BlurNotEquals) { TestNotEquals(filter1, filter4, "Tile Mode differs"); } -TEST(DisplayListImageFilter, BlurBounds) { - DlBlurImageFilter filter = DlBlurImageFilter(5, 10, DlTileMode::kDecal); - SkRect inputBounds = SkRect::MakeLTRB(20, 20, 80, 80); - SkRect expectOutputBounds = inputBounds.makeOutset(15, 30); - // SkRect expectInputBounds = expectOutputBounds.makeOutset(15, 30); - TestBounds(filter, inputBounds, expectOutputBounds); -} - TEST(DisplayListImageFilter, DilateConstructor) { DlDilateImageFilter filter(5.0, 6.0); } @@ -364,13 +227,6 @@ TEST(DisplayListImageFilter, DilateNotEquals) { TestNotEquals(filter1, filter3, "Radius Y differs"); } -TEST(DisplayListImageFilter, DilateBounds) { - DlDilateImageFilter filter = DlDilateImageFilter(5, 10); - SkRect inputBounds = SkRect::MakeLTRB(20, 20, 80, 80); - SkRect expectOutputBounds = inputBounds.makeOutset(5, 10); - TestBounds(filter, inputBounds, expectOutputBounds); -} - TEST(DisplayListImageFilter, ErodeConstructor) { DlErodeImageFilter filter(5.0, 6.0); } @@ -412,13 +268,6 @@ TEST(DisplayListImageFilter, ErodeNotEquals) { TestNotEquals(filter1, filter3, "Radius Y differs"); } -TEST(DisplayListImageFilter, ErodeBounds) { - DlErodeImageFilter filter = DlErodeImageFilter(5, 10); - SkRect inputBounds = SkRect::MakeLTRB(20, 20, 80, 80); - SkRect expectOutputBounds = inputBounds.makeInset(5, 10); - TestBounds(filter, inputBounds, expectOutputBounds); -} - TEST(DisplayListImageFilter, MatrixConstructor) { DlMatrixImageFilter filter(SkMatrix::MakeAll(2.0, 0.0, 10, // 0.5, 3.0, 15, // @@ -481,23 +330,6 @@ TEST(DisplayListImageFilter, MatrixNotEquals) { TestNotEquals(filter1, filter3, "Sampling differs"); } -TEST(DisplayListImageFilter, MatrixBounds) { - SkMatrix matrix = SkMatrix::MakeAll(2.0, 0.0, 10, // - 0.5, 3.0, 7, // - 0.0, 0.0, 1); - SkMatrix inverse; - ASSERT_TRUE(matrix.invert(&inverse)); - DlMatrixImageFilter filter(matrix, DisplayList::LinearSampling); - SkRect inputBounds = SkRect::MakeLTRB(20, 20, 80, 80); - SkPoint expectedOutputQuad[4] = { - {50, 77}, // (20,20) => (20*2 + 10, 20/2 + 20*3 + 7) == (50, 77) - {50, 257}, // (20,80) => (20*2 + 10, 20/2 + 80*3 + 7) == (50, 257) - {170, 287}, // (80,80) => (80*2 + 10, 80/2 + 80*3 + 7) == (170, 287) - {170, 107}, // (80,20) => (80*2 + 10, 80/2 + 20*3 + 7) == (170, 107) - }; - TestBounds(filter, inputBounds, expectedOutputQuad); -} - TEST(DisplayListImageFilter, ComposeConstructor) { DlMatrixImageFilter outer(SkMatrix::MakeAll(2.0, 0.0, 10, // 0.5, 3.0, 15, // @@ -582,15 +414,6 @@ TEST(DisplayListImageFilter, ComposeNotEquals) { TestNotEquals(filter1, filter3, "Inner differs"); } -TEST(DisplayListImageFilter, ComposeBounds) { - DlDilateImageFilter outer = DlDilateImageFilter(5, 10); - DlBlurImageFilter inner = DlBlurImageFilter(12, 5, DlTileMode::kDecal); - DlComposeImageFilter filter = DlComposeImageFilter(outer, inner); - SkRect inputBounds = SkRect::MakeLTRB(20, 20, 80, 80); - SkRect expectOutputBounds = inputBounds.makeOutset(36, 15).makeOutset(5, 10); - TestBounds(filter, inputBounds, expectOutputBounds); -} - TEST(DisplayListImageFilter, ColorFilterConstructor) { DlBlendColorFilter dl_color_filter(DlColor::kRed(), DlBlendMode::kLighten); DlColorFilterImageFilter filter(dl_color_filter); @@ -642,20 +465,6 @@ TEST(DisplayListImageFilter, ColorFilterNotEquals) { TestNotEquals(filter1, filter3, "Blend Mode differs"); } -TEST(DisplayListImageFilter, ColorFilterBounds) { - DlBlendColorFilter dl_color_filter(DlColor::kRed(), DlBlendMode::kSrcIn); - DlColorFilterImageFilter filter(dl_color_filter); - SkRect inputBounds = SkRect::MakeLTRB(20, 20, 80, 80); - TestBounds(filter, inputBounds, inputBounds); -} - -TEST(DisplayListImageFilter, ColorFilterModifiesTransparencyBounds) { - DlBlendColorFilter dl_color_filter(DlColor::kRed(), DlBlendMode::kSrcOver); - DlColorFilterImageFilter filter(dl_color_filter); - SkRect inputBounds = SkRect::MakeLTRB(20, 20, 80, 80); - TestInvalidBounds(filter, SkMatrix::I(), inputBounds); -} - TEST(DisplayListImageFilter, UnknownConstructor) { DlUnknownImageFilter filter( SkImageFilters::Blur(5.0, 6.0, SkTileMode::kRepeat, nullptr)); diff --git a/engine/src/flutter/flow/layers/backdrop_filter_layer.cc b/engine/src/flutter/flow/layers/backdrop_filter_layer.cc index cc3a00753c1..8e8ef2628a1 100644 --- a/engine/src/flutter/flow/layers/backdrop_filter_layer.cc +++ b/engine/src/flutter/flow/layers/backdrop_filter_layer.cc @@ -6,9 +6,8 @@ namespace flutter { -BackdropFilterLayer::BackdropFilterLayer( - std::shared_ptr filter, - DlBlendMode blend_mode) +BackdropFilterLayer::BackdropFilterLayer(sk_sp filter, + SkBlendMode blend_mode) : filter_(std::move(filter)), blend_mode_(blend_mode) {} void BackdropFilterLayer::Diff(DiffContext* context, const Layer* old_layer) { @@ -16,7 +15,7 @@ void BackdropFilterLayer::Diff(DiffContext* context, const Layer* old_layer) { auto* prev = static_cast(old_layer); if (!context->IsSubtreeDirty()) { FML_DCHECK(prev); - if (NotEquals(filter_, prev->filter_)) { + if (filter_ != prev->filter_) { context->MarkSubtreeDirty(context->GetOldLayerPaintRegion(old_layer)); } } @@ -27,11 +26,11 @@ void BackdropFilterLayer::Diff(DiffContext* context, const Layer* old_layer) { if (filter_) { context->GetTransform().mapRect(&paint_bounds); - auto filter_target_bounds = paint_bounds.roundOut(); - SkIRect filter_input_bounds; // in screen coordinates - filter_->get_input_device_bounds( - filter_target_bounds, context->GetTransform(), filter_input_bounds); - context->AddReadbackRegion(filter_input_bounds); + auto input_filter_bounds = paint_bounds.roundOut(); + auto filter_bounds = // in screen coordinates + filter_->filterBounds(input_filter_bounds, context->GetTransform(), + SkImageFilter::kReverse_MapDirection); + context->AddReadbackRegion(filter_bounds); } DiffChildren(context, prev); @@ -53,28 +52,15 @@ void BackdropFilterLayer::Paint(PaintContext& context) const { TRACE_EVENT0("flutter", "BackdropFilterLayer::Paint"); FML_DCHECK(needs_painting(context)); - if (context.leaf_nodes_builder) { - DlPaint paint; - paint.setBlendMode(blend_mode_); - context.leaf_nodes_builder->saveLayer(&paint_bounds(), &paint, - filter_.get()); - - PaintChildren(context); - - context.leaf_nodes_builder->restore(); - } else { - SkPaint paint; - paint.setBlendMode(ToSk(blend_mode_)); - auto sk_filter = filter_ ? filter_->skia_object() : nullptr; - Layer::AutoSaveLayer save = Layer::AutoSaveLayer::Create( - context, - SkCanvas::SaveLayerRec{&paint_bounds(), &paint, sk_filter.get(), 0}, - // BackdropFilter should only happen on the leaf nodes canvas. - // See https:://flutter.dev/go/backdrop-filter-with-overlay-canvas - AutoSaveLayer::SaveMode::kLeafNodesCanvas); - - PaintChildren(context); - } + SkPaint paint; + paint.setBlendMode(blend_mode_); + Layer::AutoSaveLayer save = Layer::AutoSaveLayer::Create( + context, + SkCanvas::SaveLayerRec{&paint_bounds(), &paint, filter_.get(), 0}, + // BackdropFilter should only happen on the leaf nodes canvas. + // See https:://flutter.dev/go/backdrop-filter-with-overlay-canvas + AutoSaveLayer::SaveMode::kLeafNodesCanvas); + PaintChildren(context); } } // namespace flutter diff --git a/engine/src/flutter/flow/layers/backdrop_filter_layer.h b/engine/src/flutter/flow/layers/backdrop_filter_layer.h index 8584413698a..c8cbeba9215 100644 --- a/engine/src/flutter/flow/layers/backdrop_filter_layer.h +++ b/engine/src/flutter/flow/layers/backdrop_filter_layer.h @@ -12,8 +12,7 @@ namespace flutter { class BackdropFilterLayer : public ContainerLayer { public: - BackdropFilterLayer(std::shared_ptr filter, - DlBlendMode blend_mode); + BackdropFilterLayer(sk_sp filter, SkBlendMode blend_mode); void Diff(DiffContext* context, const Layer* old_layer) override; @@ -22,8 +21,8 @@ class BackdropFilterLayer : public ContainerLayer { void Paint(PaintContext& context) const override; private: - std::shared_ptr filter_; - DlBlendMode blend_mode_; + sk_sp filter_; + SkBlendMode blend_mode_; FML_DISALLOW_COPY_AND_ASSIGN(BackdropFilterLayer); }; diff --git a/engine/src/flutter/flow/layers/backdrop_filter_layer_unittests.cc b/engine/src/flutter/flow/layers/backdrop_filter_layer_unittests.cc index 0cb82f39c59..33792a7b3ce 100644 --- a/engine/src/flutter/flow/layers/backdrop_filter_layer_unittests.cc +++ b/engine/src/flutter/flow/layers/backdrop_filter_layer_unittests.cc @@ -22,9 +22,8 @@ using BackdropFilterLayerTest = LayerTest; #ifndef NDEBUG TEST_F(BackdropFilterLayerTest, PaintingEmptyLayerDies) { - auto filter = DlBlurImageFilter(5, 5, DlTileMode::kClamp); - auto layer = std::make_shared(filter.shared(), - DlBlendMode::kSrcOver); + auto layer = std::make_shared(sk_sp(), + SkBlendMode::kSrcOver); auto parent = std::make_shared(kEmptyRect, Clip::hardEdge); parent->Add(layer); @@ -41,9 +40,8 @@ TEST_F(BackdropFilterLayerTest, PaintBeforePrerollDies) { const SkRect child_bounds = SkRect::MakeLTRB(5.0f, 6.0f, 20.5f, 21.5f); const SkPath child_path = SkPath().addRect(child_bounds); auto mock_layer = std::make_shared(child_path); - auto filter = DlBlurImageFilter(5, 5, DlTileMode::kClamp); - auto layer = std::make_shared(filter.shared(), - DlBlendMode::kSrcOver); + auto layer = std::make_shared(sk_sp(), + SkBlendMode::kSrcOver); layer->Add(mock_layer); EXPECT_EQ(layer->paint_bounds(), kEmptyRect); @@ -60,7 +58,7 @@ TEST_F(BackdropFilterLayerTest, EmptyFilter) { const SkPaint child_paint = SkPaint(SkColors::kYellow); auto mock_layer = std::make_shared(child_path, child_paint); auto layer = - std::make_shared(nullptr, DlBlendMode::kSrcOver); + std::make_shared(nullptr, SkBlendMode::kSrcOver); layer->Add(mock_layer); auto parent = std::make_shared(child_bounds, Clip::hardEdge); parent->Add(layer); @@ -89,8 +87,8 @@ TEST_F(BackdropFilterLayerTest, SimpleFilter) { const SkPaint child_paint = SkPaint(SkColors::kYellow); auto layer_filter = SkImageFilters::Paint(SkPaint(SkColors::kMagenta)); auto mock_layer = std::make_shared(child_path, child_paint); - auto layer = std::make_shared( - DlImageFilter::From(layer_filter), DlBlendMode::kSrcOver); + auto layer = std::make_shared(layer_filter, + SkBlendMode::kSrcOver); layer->Add(mock_layer); auto parent = std::make_shared(child_bounds, Clip::hardEdge); parent->Add(layer); @@ -119,8 +117,8 @@ TEST_F(BackdropFilterLayerTest, NonSrcOverBlend) { const SkPaint child_paint = SkPaint(SkColors::kYellow); auto layer_filter = SkImageFilters::Paint(SkPaint(SkColors::kMagenta)); auto mock_layer = std::make_shared(child_path, child_paint); - auto layer = std::make_shared( - DlImageFilter::From(layer_filter), DlBlendMode::kSrc); + auto layer = + std::make_shared(layer_filter, SkBlendMode::kSrc); layer->Add(mock_layer); auto parent = std::make_shared(child_bounds, Clip::hardEdge); parent->Add(layer); @@ -158,8 +156,8 @@ TEST_F(BackdropFilterLayerTest, MultipleChildren) { auto layer_filter = SkImageFilters::Paint(SkPaint(SkColors::kMagenta)); auto mock_layer1 = std::make_shared(child_path1, child_paint1); auto mock_layer2 = std::make_shared(child_path2, child_paint2); - auto layer = std::make_shared( - DlImageFilter::From(layer_filter), DlBlendMode::kSrcOver); + auto layer = std::make_shared(layer_filter, + SkBlendMode::kSrcOver); layer->Add(mock_layer1); layer->Add(mock_layer2); auto parent = @@ -204,10 +202,10 @@ TEST_F(BackdropFilterLayerTest, Nested) { auto layer_filter2 = SkImageFilters::Paint(SkPaint(SkColors::kDkGray)); auto mock_layer1 = std::make_shared(child_path1, child_paint1); auto mock_layer2 = std::make_shared(child_path2, child_paint2); - auto layer1 = std::make_shared( - DlImageFilter::From(layer_filter1), DlBlendMode::kSrcOver); - auto layer2 = std::make_shared( - DlImageFilter::From(layer_filter2), DlBlendMode::kSrcOver); + auto layer1 = std::make_shared(layer_filter1, + SkBlendMode::kSrcOver); + auto layer2 = std::make_shared(layer_filter2, + SkBlendMode::kSrcOver); layer2->Add(mock_layer2); layer1->Add(mock_layer1); layer1->Add(layer2); @@ -245,20 +243,20 @@ TEST_F(BackdropFilterLayerTest, Nested) { } TEST_F(BackdropFilterLayerTest, Readback) { - std::shared_ptr no_filter; - auto layer_filter = DlBlurImageFilter(5, 5, DlTileMode::kClamp); + sk_sp no_filter; + auto layer_filter = SkImageFilters::Paint(SkPaint(SkColors::kMagenta)); auto initial_transform = SkMatrix(); // BDF with filter always reads from surface - auto layer1 = std::make_shared(layer_filter.shared(), - DlBlendMode::kSrcOver); + auto layer1 = std::make_shared(layer_filter, + SkBlendMode::kSrcOver); preroll_context()->surface_needs_readback = false; layer1->Preroll(preroll_context(), initial_transform); EXPECT_TRUE(preroll_context()->surface_needs_readback); // BDF with no filter does not read from surface itself auto layer2 = - std::make_shared(no_filter, DlBlendMode::kSrcOver); + std::make_shared(no_filter, SkBlendMode::kSrcOver); preroll_context()->surface_needs_readback = false; layer2->Preroll(preroll_context(), initial_transform); EXPECT_FALSE(preroll_context()->surface_needs_readback); @@ -280,20 +278,18 @@ TEST_F(BackdropFilterLayerTest, Readback) { using BackdropLayerDiffTest = DiffContextTest; TEST_F(BackdropLayerDiffTest, BackdropLayer) { - auto filter = DlBlurImageFilter(10, 10, DlTileMode::kClamp); + auto filter = SkImageFilters::Blur(10, 10, SkTileMode::kClamp, nullptr); { // tests later assume 30px readback area, fail early if that's not the case - SkIRect readback; - EXPECT_EQ(filter.get_input_device_bounds(SkIRect::MakeWH(10, 10), - SkMatrix::I(), readback), - &readback); + auto readback = filter->filterBounds(SkIRect::MakeWH(10, 10), SkMatrix::I(), + SkImageFilter::kReverse_MapDirection); EXPECT_EQ(readback, SkIRect::MakeLTRB(-30, -30, 40, 40)); } MockLayerTree l1(SkISize::Make(100, 100)); - l1.root()->Add(std::make_shared(filter.shared(), - DlBlendMode::kSrcOver)); + l1.root()->Add( + std::make_shared(filter, SkBlendMode::kSrcOver)); // no clip, effect over entire surface auto damage = DiffLayerTree(l1, MockLayerTree(SkISize::Make(100, 100))); @@ -303,8 +299,8 @@ TEST_F(BackdropLayerDiffTest, BackdropLayer) { auto clip = std::make_shared(SkRect::MakeLTRB(20, 20, 60, 60), Clip::hardEdge); - clip->Add(std::make_shared(filter.shared(), - DlBlendMode::kSrcOver)); + clip->Add( + std::make_shared(filter, SkBlendMode::kSrcOver)); l2.root()->Add(clip); damage = DiffLayerTree(l2, MockLayerTree(SkISize::Make(100, 100))); @@ -338,14 +334,12 @@ TEST_F(BackdropLayerDiffTest, BackdropLayer) { } TEST_F(BackdropLayerDiffTest, BackdropLayerInvalidTransform) { - auto filter = DlBlurImageFilter(10, 10, DlTileMode::kClamp); + auto filter = SkImageFilters::Blur(10, 10, SkTileMode::kClamp, nullptr); { // tests later assume 30px readback area, fail early if that's not the case - SkIRect readback; - EXPECT_EQ(filter.get_input_device_bounds(SkIRect::MakeWH(10, 10), - SkMatrix::I(), readback), - &readback); + auto readback = filter->filterBounds(SkIRect::MakeWH(10, 10), SkMatrix::I(), + SkImageFilter::kReverse_MapDirection); EXPECT_EQ(readback, SkIRect::MakeLTRB(-30, -30, 40, 40)); } @@ -356,11 +350,11 @@ TEST_F(BackdropLayerDiffTest, BackdropLayerInvalidTransform) { auto transform_layer = std::make_shared(transform); l1.root()->Add(transform_layer); - transform_layer->Add(std::make_shared( - filter.shared(), DlBlendMode::kSrcOver)); + transform_layer->Add( + std::make_shared(filter, SkBlendMode::kSrcOver)); auto damage = DiffLayerTree(l1, MockLayerTree(SkISize::Make(100, 100))); - EXPECT_EQ(damage.frame_damage, SkIRect::MakeWH(100, 100)); + EXPECT_EQ(damage.frame_damage, SkIRect::MakeWH(15, 15)); } } // namespace testing diff --git a/engine/src/flutter/lib/ui/compositing/scene_builder.cc b/engine/src/flutter/lib/ui/compositing/scene_builder.cc index 297e3b7c109..2a2810ba99f 100644 --- a/engine/src/flutter/lib/ui/compositing/scene_builder.cc +++ b/engine/src/flutter/lib/ui/compositing/scene_builder.cc @@ -203,7 +203,7 @@ void SceneBuilder::pushBackdropFilter(Dart_Handle layer_handle, int blendMode, fml::RefPtr oldLayer) { auto layer = std::make_shared( - filter->filter(), static_cast(blendMode)); + filter->filter()->skia_object(), static_cast(blendMode)); PushLayer(layer); EngineLayer::MakeRetained(layer_handle, layer);