From 120784394c160d009bc3aa88dd217c13c105a6ca Mon Sep 17 00:00:00 2001 From: Robert Phillips Date: Thu, 17 May 2018 11:17:39 -0400 Subject: Fix srcBounds computation in SkMatrixConvolutionImageFilter Note that this does change the behavior of the cropRect for the repeated case. The cropRect now only acts as a hard clip on the output. BUG= skia:7766 Change-Id: I1d66678bc797cd4835701cd20c36e68b22ac880a Reviewed-on: https://skia-review.googlesource.com/127338 Reviewed-by: Herb Derby Commit-Queue: Robert Phillips --- tests/ImageFilterTest.cpp | 144 +++++++++++++++++++++++++++------------------- 1 file changed, 86 insertions(+), 58 deletions(-) (limited to 'tests/ImageFilterTest.cpp') diff --git a/tests/ImageFilterTest.cpp b/tests/ImageFilterTest.cpp index 556c3f040d..8e46715fbb 100644 --- a/tests/ImageFilterTest.cpp +++ b/tests/ImageFilterTest.cpp @@ -150,7 +150,6 @@ public: FilterList(sk_sp input, const SkImageFilter::CropRect* cropRect = nullptr) { SkPoint3 location = SkPoint3::Make(0, 0, SK_Scalar1); const SkScalar five = SkIntToScalar(5); - { sk_sp cf(SkColorFilter::MakeModeFilter(SK_ColorRED, SkBlendMode::kSrcIn)); @@ -158,7 +157,6 @@ public: this->addFilter("color filter", SkColorFilterImageFilter::Make(std::move(cf), input, cropRect)); } - { sk_sp gradientImage(SkImage::MakeFromBitmap(make_gradient_circle(64, 64))); sk_sp gradientSource(SkImageSource::Make(std::move(gradientImage))); @@ -169,7 +167,6 @@ public: 20.0f, std::move(gradientSource), input, cropRect)); } - this->addFilter("blur", SkBlurImageFilter::Make(SK_Scalar1, SK_Scalar1, input, @@ -193,13 +190,14 @@ public: const SkISize kernelSize = SkISize::Make(3, 3); const SkScalar gain = SK_Scalar1, bias = 0; + // This filter needs a saveLayer bc it is in repeat mode this->addFilter("matrix convolution", - SkMatrixConvolutionImageFilter::Make( - kernelSize, kernel, gain, bias, SkIPoint::Make(1, 1), - SkMatrixConvolutionImageFilter::kRepeat_TileMode, false, - input, cropRect)); + SkMatrixConvolutionImageFilter::Make( + kernelSize, kernel, gain, bias, SkIPoint::Make(1, 1), + SkMatrixConvolutionImageFilter::kRepeat_TileMode, false, + input, cropRect), + true); } - this->addFilter("merge", SkMergeImageFilter::Make(input, input, cropRect)); { @@ -273,18 +271,21 @@ public: int count() const { return fFilters.count(); } SkImageFilter* getFilter(int index) const { return fFilters[index].fFilter.get(); } const char* getName(int index) const { return fFilters[index].fName; } + bool needsSaveLayer(int index) const { return fFilters[index].fNeedsSaveLayer; } private: struct Filter { - Filter() : fName(nullptr) {} - Filter(const char* name, sk_sp filter) + Filter() : fName(nullptr), fNeedsSaveLayer(false) {} + Filter(const char* name, sk_sp filter, bool needsSaveLayer) : fName(name) - , fFilter(std::move(filter)) { + , fFilter(std::move(filter)) + , fNeedsSaveLayer(needsSaveLayer) { } const char* fName; sk_sp fFilter; + bool fNeedsSaveLayer; }; - void addFilter(const char* name, sk_sp filter) { - fFilters.push_back(Filter(name, std::move(filter))); + void addFilter(const char* name, sk_sp filter, bool needsSaveLayer = false) { + fFilters.push_back(Filter(name, std::move(filter), needsSaveLayer)); } SkTArray fFilters; @@ -305,7 +306,8 @@ private: } sk_sp onMakeColorSpace(SkColorSpaceXformer*) const override { return nullptr; } - SkIRect onFilterBounds(const SkIRect&, const SkMatrix&, MapDirection) const override { + SkIRect onFilterBounds(const SkIRect&, const SkMatrix&, + MapDirection, const SkIRect*) const override { return fBounds; } @@ -743,34 +745,51 @@ DEF_TEST(ImageFilterDrawTiled, reporter) { tiledResult.allocN32Pixels(width, height); SkCanvas tiledCanvas(tiledResult); SkCanvas untiledCanvas(untiledResult); - int tileSize = 8; + const int tileSize = 8; + + SkPaint textPaint; + textPaint.setTextSize(SkIntToScalar(height)); + textPaint.setColor(SK_ColorWHITE); + + const char* text = "ABC"; + const SkScalar yPos = SkIntToScalar(height); for (int scale = 1; scale <= 2; ++scale) { for (int i = 0; i < filters.count(); ++i) { - tiledCanvas.clear(0); - untiledCanvas.clear(0); - SkPaint paint; - paint.setImageFilter(sk_ref_sp(filters.getFilter(i))); - paint.setTextSize(SkIntToScalar(height)); - paint.setColor(SK_ColorWHITE); - SkString str; - const char* text = "ABC"; - SkScalar ypos = SkIntToScalar(height); + SkPaint combinedPaint; + combinedPaint.setTextSize(SkIntToScalar(height)); + combinedPaint.setColor(SK_ColorWHITE); + combinedPaint.setImageFilter(sk_ref_sp(filters.getFilter(i))); + + untiledCanvas.clear(SK_ColorTRANSPARENT); untiledCanvas.save(); untiledCanvas.scale(SkIntToScalar(scale), SkIntToScalar(scale)); - untiledCanvas.drawString(text, 0, ypos, paint); + untiledCanvas.drawString(text, 0, yPos, combinedPaint); untiledCanvas.restore(); + untiledCanvas.flush(); + + tiledCanvas.clear(SK_ColorTRANSPARENT); for (int y = 0; y < height; y += tileSize) { for (int x = 0; x < width; x += tileSize) { tiledCanvas.save(); - tiledCanvas.clipRect(SkRect::Make(SkIRect::MakeXYWH(x, y, tileSize, tileSize))); - tiledCanvas.scale(SkIntToScalar(scale), SkIntToScalar(scale)); - tiledCanvas.drawString(text, 0, ypos, paint); + const SkRect clipRect = SkRect::MakeXYWH(x, y, tileSize, tileSize); + tiledCanvas.clipRect(clipRect); + if (filters.needsSaveLayer(i)) { + const SkRect layerBounds = SkRect::MakeWH(width, height); + tiledCanvas.saveLayer(&layerBounds, &combinedPaint); + tiledCanvas.scale(SkIntToScalar(scale), SkIntToScalar(scale)); + tiledCanvas.drawString(text, 0, yPos, textPaint); + tiledCanvas.restore(); + } else { + tiledCanvas.scale(SkIntToScalar(scale), SkIntToScalar(scale)); + tiledCanvas.drawString(text, 0, yPos, combinedPaint); + } + tiledCanvas.restore(); } } - untiledCanvas.flush(); tiledCanvas.flush(); + if (!sk_tool_utils::equal_pixels(untiledResult, tiledResult, 1)) { REPORTER_ASSERT(reporter, false, filters.getName(i)); break; @@ -850,7 +869,8 @@ DEF_TEST(ImageFilterBlurThenShadowBounds, reporter) { SkIRect bounds = SkIRect::MakeXYWH(0, 0, 100, 100); SkIRect expectedBounds = SkIRect::MakeXYWH(-133, -133, 236, 236); - bounds = filter2->filterBounds(bounds, SkMatrix::I(), SkImageFilter::kReverse_MapDirection); + bounds = filter2->filterBounds(bounds, SkMatrix::I(), + SkImageFilter::kReverse_MapDirection, &bounds); REPORTER_ASSERT(reporter, bounds == expectedBounds); } @@ -861,7 +881,8 @@ DEF_TEST(ImageFilterShadowThenBlurBounds, reporter) { SkIRect bounds = SkIRect::MakeXYWH(0, 0, 100, 100); SkIRect expectedBounds = SkIRect::MakeXYWH(-133, -133, 236, 236); - bounds = filter2->filterBounds(bounds, SkMatrix::I(), SkImageFilter::kReverse_MapDirection); + bounds = filter2->filterBounds(bounds, SkMatrix::I(), + SkImageFilter::kReverse_MapDirection, &bounds); REPORTER_ASSERT(reporter, bounds == expectedBounds); } @@ -872,7 +893,8 @@ DEF_TEST(ImageFilterDilateThenBlurBounds, reporter) { SkIRect bounds = SkIRect::MakeXYWH(0, 0, 100, 100); SkIRect expectedBounds = SkIRect::MakeXYWH(-132, -132, 234, 234); - bounds = filter2->filterBounds(bounds, SkMatrix::I(), SkImageFilter::kReverse_MapDirection); + bounds = filter2->filterBounds(bounds, SkMatrix::I(), + SkImageFilter::kReverse_MapDirection, &bounds); REPORTER_ASSERT(reporter, bounds == expectedBounds); } @@ -891,20 +913,20 @@ DEF_TEST(ImageFilterScaledBlurRadius, reporter) { SkIRect expectedBlurBounds = SkIRect::MakeLTRB(-6, -6, 206, 206); SkIRect blurBounds = blur->filterBounds( - bounds, scaleMatrix, SkImageFilter::kForward_MapDirection); + bounds, scaleMatrix, SkImageFilter::kForward_MapDirection, nullptr); REPORTER_ASSERT(reporter, blurBounds == expectedBlurBounds); SkIRect reverseBlurBounds = blur->filterBounds( - bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection); + bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection, &bounds); REPORTER_ASSERT(reporter, reverseBlurBounds == expectedBlurBounds); SkIRect expectedShadowBounds = SkIRect::MakeLTRB(0, 0, 460, 460); SkIRect shadowBounds = dropShadow->filterBounds( - bounds, scaleMatrix, SkImageFilter::kForward_MapDirection); + bounds, scaleMatrix, SkImageFilter::kForward_MapDirection, nullptr); REPORTER_ASSERT(reporter, shadowBounds == expectedShadowBounds); SkIRect expectedReverseShadowBounds = SkIRect::MakeLTRB(-260, -260, 200, 200); SkIRect reverseShadowBounds = dropShadow->filterBounds( - bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection); + bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection, &bounds); REPORTER_ASSERT(reporter, reverseShadowBounds == expectedReverseShadowBounds); } @@ -916,20 +938,20 @@ DEF_TEST(ImageFilterScaledBlurRadius, reporter) { SkIRect expectedBlurBounds = SkIRect::MakeLTRB(-3, -103, 103, 3); SkIRect blurBounds = blur->filterBounds( - bounds, scaleMatrix, SkImageFilter::kForward_MapDirection); + bounds, scaleMatrix, SkImageFilter::kForward_MapDirection, nullptr); REPORTER_ASSERT(reporter, blurBounds == expectedBlurBounds); SkIRect reverseBlurBounds = blur->filterBounds( - bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection); + bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection, &bounds); REPORTER_ASSERT(reporter, reverseBlurBounds == expectedBlurBounds); SkIRect expectedShadowBounds = SkIRect::MakeLTRB(0, -230, 230, 0); SkIRect shadowBounds = dropShadow->filterBounds( - bounds, scaleMatrix, SkImageFilter::kForward_MapDirection); + bounds, scaleMatrix, SkImageFilter::kForward_MapDirection, nullptr); REPORTER_ASSERT(reporter, shadowBounds == expectedShadowBounds); SkIRect expectedReverseShadowBounds = SkIRect::MakeLTRB(-130, -100, 100, 130); SkIRect reverseShadowBounds = dropShadow->filterBounds( - bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection); + bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection, &bounds); REPORTER_ASSERT(reporter, reverseShadowBounds == expectedReverseShadowBounds); } @@ -1919,8 +1941,8 @@ DEF_TEST(XfermodeImageFilterBounds, reporter) { for (int i = 0; i < kModeCount; ++i) { sk_sp xfermode(SkXfermodeImageFilter::Make(static_cast(i), background, foreground, nullptr)); - auto bounds = - xfermode->filterBounds(src, SkMatrix::I(), SkImageFilter::kForward_MapDirection); + auto bounds = xfermode->filterBounds(src, SkMatrix::I(), + SkImageFilter::kForward_MapDirection, nullptr); REPORTER_ASSERT(reporter, bounds == expectedBounds[i]); } @@ -1929,7 +1951,8 @@ DEF_TEST(XfermodeImageFilterBounds, reporter) { sk_sp foreground2(new FixedBoundsImageFilter(SkIRect::MakeXYWH(40, 40, 50, 50))); sk_sp xfermode(SkXfermodeImageFilter::Make( SkBlendMode::kSrcIn, std::move(background2), std::move(foreground2), nullptr)); - auto bounds = xfermode->filterBounds(src, SkMatrix::I(), SkImageFilter::kForward_MapDirection); + auto bounds = xfermode->filterBounds(src, SkMatrix::I(), + SkImageFilter::kForward_MapDirection, nullptr); REPORTER_ASSERT(reporter, bounds.isEmpty()); } @@ -1939,12 +1962,12 @@ DEF_TEST(OffsetImageFilterBounds, reporter) { SkIRect expectedForward = SkIRect::MakeXYWH(-50, -50, 100, 100); SkIRect boundsForward = offset->filterBounds(src, SkMatrix::I(), - SkImageFilter::kForward_MapDirection); + SkImageFilter::kForward_MapDirection, nullptr); REPORTER_ASSERT(reporter, boundsForward == expectedForward); SkIRect expectedReverse = SkIRect::MakeXYWH(50, 50, 100, 100); SkIRect boundsReverse = offset->filterBounds(src, SkMatrix::I(), - SkImageFilter::kReverse_MapDirection); + SkImageFilter::kReverse_MapDirection, &src); REPORTER_ASSERT(reporter, boundsReverse == expectedReverse); } @@ -1956,7 +1979,7 @@ static void test_arithmetic_bounds(skiatest::Reporter* reporter, float k1, float SkArithmeticImageFilter::Make(k1, k2, k3, k4, false, background, foreground, crop)); // The value of the input rect doesn't matter because we use inputs with fixed bounds. SkIRect bounds = arithmetic->filterBounds(SkIRect::MakeXYWH(11, 22, 33, 44), SkMatrix::I(), - SkImageFilter::kForward_MapDirection); + SkImageFilter::kForward_MapDirection, nullptr); REPORTER_ASSERT(reporter, expected == bounds); } @@ -2027,18 +2050,20 @@ DEF_TEST(ImageSourceBounds, reporter) { SkIRect input(SkIRect::MakeXYWH(10, 20, 30, 40)); REPORTER_ASSERT(reporter, imageBounds == source1->filterBounds(input, SkMatrix::I(), - SkImageFilter::kForward_MapDirection)); + SkImageFilter::kForward_MapDirection, + nullptr)); REPORTER_ASSERT(reporter, input == source1->filterBounds(input, SkMatrix::I(), - SkImageFilter::kReverse_MapDirection)); + SkImageFilter::kReverse_MapDirection, &input)); SkMatrix scale(SkMatrix::MakeScale(2)); SkIRect scaledBounds = SkIRect::MakeWH(128, 128); REPORTER_ASSERT(reporter, scaledBounds == source1->filterBounds(input, scale, - SkImageFilter::kForward_MapDirection)); - REPORTER_ASSERT( - reporter, - input == source1->filterBounds(input, scale, SkImageFilter::kReverse_MapDirection)); + SkImageFilter::kForward_MapDirection, + nullptr)); + REPORTER_ASSERT(reporter, input == source1->filterBounds(input, scale, + SkImageFilter::kReverse_MapDirection, + &input)); // Specified src and dst rects. SkRect src(SkRect::MakeXYWH(0.5, 0.5, 100.5, 100.5)); @@ -2046,16 +2071,19 @@ DEF_TEST(ImageSourceBounds, reporter) { sk_sp source2(SkImageSource::Make(image, src, dst, kMedium_SkFilterQuality)); REPORTER_ASSERT(reporter, dst.roundOut() == source2->filterBounds(input, SkMatrix::I(), - SkImageFilter::kForward_MapDirection)); + SkImageFilter::kForward_MapDirection, + nullptr)); REPORTER_ASSERT(reporter, input == source2->filterBounds(input, SkMatrix::I(), - SkImageFilter::kReverse_MapDirection)); + SkImageFilter::kReverse_MapDirection, &input)); scale.mapRect(&dst); scale.mapRect(&src); REPORTER_ASSERT(reporter, dst.roundOut() == source2->filterBounds(input, scale, - SkImageFilter::kForward_MapDirection)); - REPORTER_ASSERT( - reporter, - input == source2->filterBounds(input, scale, SkImageFilter::kReverse_MapDirection)); + SkImageFilter::kForward_MapDirection, + nullptr)); + REPORTER_ASSERT(reporter, input == source2->filterBounds(input, scale, + SkImageFilter::kReverse_MapDirection, + &input)); } + -- cgit v1.2.3