From 0abdf766d395ed3b7059511425f431589eca05f6 Mon Sep 17 00:00:00 2001 From: senorblanco Date: Thu, 20 Aug 2015 11:10:41 -0700 Subject: [PATCH] Reland of Implement canComputeFastBounds() for image filters. (patchset #1 id:1 of https://codereview.chromium.org/1300403003/ ) Reason for revert: The Mac compile issue was fixed here: https://chromium.googlesource.com/chromium/src/+/fdd331a42ae0b9a6909a121020735161ab61c6e5 Original issue's description: > Revert of Implement canComputeFastBounds() for image filters. (patchset #8 id:130001 of https://codereview.chromium.org/1296943002/ ) > > Reason for revert: > This causes a syntax error. > > http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg_ng/builds/87819/steps/compile%20%28with%20patch%29/logs/stdio > > Original issue's description: > > Implement canComputeFastBounds() for image filters. > > > > Image filters have never implemented this check, which means that > > filters which affect transparent black falsely claim they can compute > > their bounds. > > > > Implemented an affectsTransparentBlack() virtual for image > > filters, and a similar helper function for color filters. > > > > This will affect the following GMs: imagefiltersscaled > > (lighting, perlin noise now filter to clip), > > colorfilterimagefilter (new test case), imagefiltersclipped > > (perlin noise now filters to clip). > > > > Note: I de-inlined SkPaint::canComputeFastBounds() to avoid adding > > a dependency from SkPaint.h to SkImageFilter.h.h. Skia benches show > > no impact from this change, but will watch the perf bots carefully. > > > > BUG=4212 > > > > Committed: https://skia.googlesource.com/skia/+/915881fe743f9a789037695f543bc6ea189cd0cb > > TBR=reed@google.com,senorblanco@chromium.org > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG=4212 > > Committed: https://skia.googlesource.com/skia/+/12d8472d31ea5edb636d7d5214db253570115c40 TBR=reed@google.com,herb@google.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=4212 Review URL: https://codereview.chromium.org/1301823005 --- gm/colorfilterimagefilter.cpp | 8 +++- gm/imagefiltersscaled.cpp | 1 + include/core/SkColorFilter.h | 4 ++ include/core/SkImageFilter.h | 12 +++++ include/core/SkPaint.h | 9 +--- include/effects/SkColorFilterImageFilter.h | 1 + include/effects/SkLightingImageFilter.h | 1 + include/effects/SkRectShaderImageFilter.h | 1 + src/core/SkImageFilter.cpp | 17 +++++++ src/core/SkPaint.cpp | 10 ++++ src/effects/SkColorFilterImageFilter.cpp | 4 ++ src/effects/SkRectShaderImageFilter.cpp | 4 ++ tests/ImageFilterTest.cpp | 53 ++++++++++++++++++++++ 13 files changed, 117 insertions(+), 8 deletions(-) diff --git a/gm/colorfilterimagefilter.cpp b/gm/colorfilterimagefilter.cpp index b34058f9d7..500450f61b 100644 --- a/gm/colorfilterimagefilter.cpp +++ b/gm/colorfilterimagefilter.cpp @@ -45,7 +45,7 @@ static SkImageFilter* make_grayscale(SkImageFilter* input = NULL) { static SkImageFilter* make_mode_blue(SkImageFilter* input = NULL) { SkAutoTUnref filter( - SkColorFilter::CreateModeFilter(SK_ColorBLUE, SkXfermode::kSrcIn_Mode)); + SkColorFilter::CreateModeFilter(SK_ColorBLUE, SkXfermode::kSrc_Mode)); return SkColorFilterImageFilter::Create(filter, input); } @@ -120,6 +120,12 @@ protected: drawClippedRect(canvas, r, paint, 3); canvas->translate(FILTER_WIDTH + MARGIN, 0); } + { + SkAutoTUnref blue(make_mode_blue()); + paint.setImageFilter(blue.get()); + drawClippedRect(canvas, r, paint, 5); + canvas->translate(FILTER_WIDTH + MARGIN, 0); + } } private: diff --git a/gm/imagefiltersscaled.cpp b/gm/imagefiltersscaled.cpp index e7a68d747d..2e4756a9b3 100644 --- a/gm/imagefiltersscaled.cpp +++ b/gm/imagefiltersscaled.cpp @@ -131,6 +131,7 @@ protected: paint.setAntiAlias(true); canvas->save(); canvas->scale(scales[j].fX, scales[j].fY); + canvas->clipRect(r); if (5 == i) { canvas->translate(SkIntToScalar(-32), 0); } else if (6 == i) { diff --git a/include/core/SkColorFilter.h b/include/core/SkColorFilter.h index df006ed140..51c07f1cdb 100644 --- a/include/core/SkColorFilter.h +++ b/include/core/SkColorFilter.h @@ -140,6 +140,10 @@ public: return false; } + bool affectsTransparentBlack() const { + return this->filterColor(0) != 0; + } + SK_TO_STRING_PUREVIRT() SK_DECLARE_FLATTENABLE_REGISTRAR_GROUP() diff --git a/include/core/SkImageFilter.h b/include/core/SkImageFilter.h index 9dc7507081..1ca1c01c08 100644 --- a/include/core/SkImageFilter.h +++ b/include/core/SkImageFilter.h @@ -178,6 +178,7 @@ public: bool asAColorFilter(SkColorFilter** filterPtr) const { return this->countInputs() > 0 && NULL == this->getInput(0) && + !this->affectsTransparentBlack() && this->isColorFilterNode(filterPtr); } @@ -214,6 +215,9 @@ public: // Default impl returns union of all input bounds. virtual void computeFastBounds(const SkRect&, SkRect*) const; + // Can this filter DAG compute the resulting bounds of an object-space rectangle? + bool canComputeFastBounds() const; + /** * Create an SkMatrixImageFilter, which transforms its input by the given matrix. */ @@ -362,6 +366,14 @@ protected: virtual bool asFragmentProcessor(GrFragmentProcessor**, GrProcessorDataManager*, GrTexture*, const SkMatrix&, const SkIRect& bounds) const; + /** + * Returns true if this filter can cause transparent black pixels to become + * visible (ie., alpha > 0). The default implementation returns false. This + * function is non-recursive, i.e., only queries this filter and not its + * inputs. + */ + virtual bool affectsTransparentBlack() const; + private: friend class SkGraphics; static void PurgeCache(); diff --git a/include/core/SkPaint.h b/include/core/SkPaint.h index b163792047..1b993fc5ea 100644 --- a/include/core/SkPaint.h +++ b/include/core/SkPaint.h @@ -9,7 +9,6 @@ #define SkPaint_DEFINED #include "SkColor.h" -#include "SkDrawLooper.h" #include "SkFilterQuality.h" #include "SkMatrix.h" #include "SkXfermode.h" @@ -20,6 +19,7 @@ class SkAutoGlyphCache; class SkColorFilter; class SkData; class SkDescriptor; +class SkDrawLooper; class SkReadBuffer; class SkWriteBuffer; class SkGlyph; @@ -909,12 +909,7 @@ public: bounds (i.e. there is nothing complex like a patheffect that would make the bounds computation expensive. */ - bool canComputeFastBounds() const { - if (this->getLooper()) { - return this->getLooper()->canComputeFastBounds(*this); - } - return !this->getRasterizer(); - } + bool canComputeFastBounds() const; /** Only call this if canComputeFastBounds() returned true. This takes a raw rectangle (the raw bounds of a shape), and adjusts it for stylistic diff --git a/include/effects/SkColorFilterImageFilter.h b/include/effects/SkColorFilterImageFilter.h index 0076a87c20..f23f16dfd1 100644 --- a/include/effects/SkColorFilterImageFilter.h +++ b/include/effects/SkColorFilterImageFilter.h @@ -29,6 +29,7 @@ protected: SkBitmap* result, SkIPoint* loc) const override; bool onIsColorFilterNode(SkColorFilter**) const override; + bool affectsTransparentBlack() const override; private: SkColorFilterImageFilter(SkColorFilter* cf, diff --git a/include/effects/SkLightingImageFilter.h b/include/effects/SkLightingImageFilter.h index d0b489d6a8..fb356c52e4 100644 --- a/include/effects/SkLightingImageFilter.h +++ b/include/effects/SkLightingImageFilter.h @@ -49,6 +49,7 @@ protected: void flatten(SkWriteBuffer&) const override; const SkImageFilterLight* light() const { return fLight.get(); } SkScalar surfaceScale() const { return fSurfaceScale; } + bool affectsTransparentBlack() const override { return true; } private: typedef SkImageFilter INHERITED; diff --git a/include/effects/SkRectShaderImageFilter.h b/include/effects/SkRectShaderImageFilter.h index 64b42c701e..d78162876f 100644 --- a/include/effects/SkRectShaderImageFilter.h +++ b/include/effects/SkRectShaderImageFilter.h @@ -30,6 +30,7 @@ public: static SkRectShaderImageFilter* Create(SkShader* s, const CropRect* rect = NULL); virtual ~SkRectShaderImageFilter(); + bool affectsTransparentBlack() const override; SK_TO_STRING_OVERRIDE() SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkRectShaderImageFilter) diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp index 65012155ed..5157db2dc9 100644 --- a/src/core/SkImageFilter.cpp +++ b/src/core/SkImageFilter.cpp @@ -284,6 +284,23 @@ void SkImageFilter::computeFastBounds(const SkRect& src, SkRect* dst) const { } } +bool SkImageFilter::canComputeFastBounds() const { + if (this->affectsTransparentBlack()) { + return false; + } + for (int i = 0; i < fInputCount; i++) { + SkImageFilter* input = this->getInput(i); + if (input && !input->canComputeFastBounds()) { + return false; + } + } + return true; +} + +bool SkImageFilter::affectsTransparentBlack() const { + return false; +} + bool SkImageFilter::onFilterImage(Proxy*, const SkBitmap&, const Context&, SkBitmap*, SkIPoint*) const { return false; diff --git a/src/core/SkPaint.cpp b/src/core/SkPaint.cpp index 6c1323c592..4400994000 100644 --- a/src/core/SkPaint.cpp +++ b/src/core/SkPaint.cpp @@ -2063,6 +2063,16 @@ bool SkPaint::getFillPath(const SkPath& src, SkPath* dst, const SkRect* cullRect return !rec.isHairlineStyle(); } +bool SkPaint::canComputeFastBounds() const { + if (this->getLooper()) { + return this->getLooper()->canComputeFastBounds(*this); + } + if (this->getImageFilter() && !this->getImageFilter()->canComputeFastBounds()) { + return false; + } + return !this->getRasterizer(); +} + const SkRect& SkPaint::doComputeFastBounds(const SkRect& origSrc, SkRect* storage, Style style) const { diff --git a/src/effects/SkColorFilterImageFilter.cpp b/src/effects/SkColorFilterImageFilter.cpp index 2eb720e5c4..94b530da8d 100755 --- a/src/effects/SkColorFilterImageFilter.cpp +++ b/src/effects/SkColorFilterImageFilter.cpp @@ -98,6 +98,10 @@ bool SkColorFilterImageFilter::onIsColorFilterNode(SkColorFilter** filter) const return false; } +bool SkColorFilterImageFilter::affectsTransparentBlack() const { + return fColorFilter->affectsTransparentBlack(); +} + #ifndef SK_IGNORE_TO_STRING void SkColorFilterImageFilter::toString(SkString* str) const { str->appendf("SkColorFilterImageFilter: ("); diff --git a/src/effects/SkRectShaderImageFilter.cpp b/src/effects/SkRectShaderImageFilter.cpp index cdf03131c1..45962537c8 100644 --- a/src/effects/SkRectShaderImageFilter.cpp +++ b/src/effects/SkRectShaderImageFilter.cpp @@ -79,6 +79,10 @@ bool SkRectShaderImageFilter::onFilterImage(Proxy* proxy, return true; } +bool SkRectShaderImageFilter::affectsTransparentBlack() const { + return true; +} + #ifndef SK_IGNORE_TO_STRING void SkRectShaderImageFilter::toString(SkString* str) const { str->appendf("SkRectShaderImageFilter: ("); diff --git a/tests/ImageFilterTest.cpp b/tests/ImageFilterTest.cpp index d822212ac6..c15d719a64 100644 --- a/tests/ImageFilterTest.cpp +++ b/tests/ImageFilterTest.cpp @@ -30,6 +30,7 @@ #include "SkReadBuffer.h" #include "SkRect.h" #include "SkRectShaderImageFilter.h" +#include "SkTableColorFilter.h" #include "SkTileImageFilter.h" #include "SkXfermodeImageFilter.h" #include "Test.h" @@ -1159,6 +1160,58 @@ DEF_TEST(PartialCropRect, reporter) { REPORTER_ASSERT(reporter, result.height() == 30); } +DEF_TEST(ImageFilterCanComputeFastBounds, reporter) { + + SkPoint3 location = SkPoint3::Make(0, 0, SK_Scalar1); + SkAutoTUnref lighting(SkLightingImageFilter::CreatePointLitDiffuse( + location, SK_ColorGREEN, 0, 0)); + REPORTER_ASSERT(reporter, !lighting->canComputeFastBounds()); + + SkAutoTUnref gray(make_grayscale(nullptr, nullptr)); + REPORTER_ASSERT(reporter, gray->canComputeFastBounds()); + { + SkColorFilter* grayCF; + REPORTER_ASSERT(reporter, gray->asAColorFilter(&grayCF)); + REPORTER_ASSERT(reporter, !grayCF->affectsTransparentBlack()); + grayCF->unref(); + } + REPORTER_ASSERT(reporter, gray->canComputeFastBounds()); + + SkAutoTUnref grayBlur(SkBlurImageFilter::Create(SK_Scalar1, SK_Scalar1, gray.get())); + REPORTER_ASSERT(reporter, grayBlur->canComputeFastBounds()); + + SkScalar greenMatrix[20] = { 0, 0, 0, 0, 0, + 0, 0, 0, 0, 1, + 0, 0, 0, 0, 0, + 0, 0, 0, 0, 1 }; + SkAutoTUnref greenCF(SkColorMatrixFilter::Create(greenMatrix)); + SkAutoTUnref green(SkColorFilterImageFilter::Create(greenCF)); + + REPORTER_ASSERT(reporter, greenCF->affectsTransparentBlack()); + REPORTER_ASSERT(reporter, !green->canComputeFastBounds()); + + SkAutoTUnref greenBlur(SkBlurImageFilter::Create(SK_Scalar1, SK_Scalar1, green.get())); + REPORTER_ASSERT(reporter, !greenBlur->canComputeFastBounds()); + + uint8_t allOne[256], identity[256]; + for (int i = 0; i < 256; ++i) { + identity[i] = i; + allOne[i] = 255; + } + + SkAutoTUnref identityCF( + SkTableColorFilter::CreateARGB(identity, identity, identity, allOne)); + SkAutoTUnref identityFilter(SkColorFilterImageFilter::Create(identityCF.get())); + REPORTER_ASSERT(reporter, !identityCF->affectsTransparentBlack()); + REPORTER_ASSERT(reporter, identityFilter->canComputeFastBounds()); + + SkAutoTUnref forceOpaqueCF( + SkTableColorFilter::CreateARGB(allOne, identity, identity, identity)); + SkAutoTUnref forceOpaque(SkColorFilterImageFilter::Create(forceOpaqueCF.get())); + REPORTER_ASSERT(reporter, forceOpaqueCF->affectsTransparentBlack()); + REPORTER_ASSERT(reporter, !forceOpaque->canComputeFastBounds()); +} + #if SK_SUPPORT_GPU DEF_GPUTEST(ImageFilterCropRectGPU, reporter, factory) {