Reland of Implement canComputeFastBounds() for image filters. (patchset #1 id:1 of...

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

diff --git a/gm/colorfilterimagefilter.cpp b/gm/colorfilterimagefilter.cpp
index b34058f9d74d3c2aba6e6caf9ca8a68d2194e07d..500450f61bd9687d33abc24955c216f18edd38f3 100644 (file)
--- 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<SkColorFilter> 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<SkImageFilter> 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 e7a68d747d69b140872552e398e5d3daee57450d..2e4756a9b3ccb14073a55b447ca668b77f9614e1 100644 (file)
--- 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 df006ed1400b39d125663f7e12e6d537b6ccb3fb..51c07f1cdbea1f1f4674fb072b0389bb8b368de0 100644 (file)
--- 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 9dc75070815bf023d2f30d3095f425819e131308..1ca1c01c08b2b4c49827358b0d52c2fa8a2dd35d 100644 (file)
--- 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 b16379204791a0eedb681cb6ae7507f95dc47ad3..1b993fc5ea8a92b947d3d48355b329917265b22b 100644 (file)
--- 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 0076a87c20802e7ff51e6bd73442c59ea229cf4d..f23f16dfd1f42cb7649123dc4b1a1de2ce82db95 100644 (file)
--- 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 d0b489d6a8fa6ad1e133636b4b05576ab8d7c558..fb356c52e43413a019d57ee7514b86713ae4a28d 100644 (file)
--- 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 64b42c701e43ed0b1c6fdd5f1da6992965b547b9..d78162876fa213b4eaee21be33fdead82d8b2335 100644 (file)
--- 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 65012155ed2b2a9b3ec03e2df1ae281287f9e751..5157db2dc935ad0f2a5b1bb243981cdeeb711e4c 100644 (file)
--- 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 6c1323c592699baa77da05a8c4d92efd724cbf48..4400994000e5a5d2c3c6ccebed3ecf6558ae6446 100644 (file)
--- 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 2eb720e5c4cfcf093f815522af66e2839ebfb04e..94b530da8d689bd2eb4f79cd644553aefa5bbdb4 100755 (executable)
--- 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 cdf03131c1cd89f1e0cf2ec2f97aaef9f4cf5983..45962537c8244f70b5c6c59277a22be560afbb5d 100644 (file)
--- 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 d822212ac6ff41acd4a7301ea16cb6057de336e7..c15d719a64212dc8a552dbd485e0d8a76dcd837a 100644 (file)
--- 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<SkImageFilter> lighting(SkLightingImageFilter::CreatePointLitDiffuse(
+          location, SK_ColorGREEN, 0, 0));
+    REPORTER_ASSERT(reporter, !lighting->canComputeFastBounds());
+
+    SkAutoTUnref<SkImageFilter> 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<SkImageFilter> 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<SkColorFilter> greenCF(SkColorMatrixFilter::Create(greenMatrix));
+    SkAutoTUnref<SkImageFilter> green(SkColorFilterImageFilter::Create(greenCF));
+
+    REPORTER_ASSERT(reporter, greenCF->affectsTransparentBlack());
+    REPORTER_ASSERT(reporter, !green->canComputeFastBounds());
+
+    SkAutoTUnref<SkImageFilter> 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<SkColorFilter> identityCF(
+        SkTableColorFilter::CreateARGB(identity, identity, identity, allOne));
+    SkAutoTUnref<SkImageFilter> identityFilter(SkColorFilterImageFilter::Create(identityCF.get()));
+    REPORTER_ASSERT(reporter, !identityCF->affectsTransparentBlack());
+    REPORTER_ASSERT(reporter, identityFilter->canComputeFastBounds());
+
+    SkAutoTUnref<SkColorFilter> forceOpaqueCF(
+        SkTableColorFilter::CreateARGB(allOne, identity, identity, identity));
+    SkAutoTUnref<SkImageFilter> forceOpaque(SkColorFilterImageFilter::Create(forceOpaqueCF.get()));
+    REPORTER_ASSERT(reporter, forceOpaqueCF->affectsTransparentBlack());
+    REPORTER_ASSERT(reporter, !forceOpaque->canComputeFastBounds());
+}
+
 #if SK_SUPPORT_GPU
 
 DEF_GPUTEST(ImageFilterCropRectGPU, reporter, factory) {