SkBlurImageFilter returns input when sigma = 0

src/effects/SkDropShadowImageFilter.cpp

Index: src/effects/SkDropShadowImageFilter.cpp
diff --git a/src/effects/SkDropShadowImageFilter.cpp b/src/effects/SkDropShadowImageFilter.cpp
index eb05cf09eca99dfdbf0e381050fb6ae1b0074a42..55a75a8325e886ce590c1ceb652b8973c06905d9 100644
--- a/src/effects/SkDropShadowImageFilter.cpp
+++ b/src/effects/SkDropShadowImageFilter.cpp
@@ -77,11 +77,16 @@ bool SkDropShadowImageFilter::onFilterImage(Proxy* proxy, const SkBitmap& source
     ctx.ctm().mapVectors(&sigma, 1);
     sigma.fX = SkMaxScalar(0, sigma.fX);
     sigma.fY = SkMaxScalar(0, sigma.fY);
-    SkAutoTUnref<SkImageFilter> blurFilter(SkBlurImageFilter::Create(sigma.fX, sigma.fY));
+    SkAutoTUnref<SkImageFilter> blurFilter;
+    if (sigma.fX != 0 || sigma.fY != 0)

[reed1, 2015/12/10 13:36:49]

It seems that for the most part, if the sigmas are zero, they will always have
been zero (i.e. at Create factory time).

Perhaps the factory itself should check for zero sigmas, and return an
OffsetImageFilter instead of a DropShaderImageFilter...

[Stephen White, 2015/12/10 14:57:18]

If it's drawing shadow & foreground, an offset may not be enough. It'd need a
composite as well. And you'd have to check for crop rect.

For this particular case, I'd prefer that this was done in
SkBlurImageFilter::Create(); something like:

if (sigma.fX == 0 && sigma.fY == 0 && nullptr == cropRect) {
    return input;
}

Would that work?

Note that we may have to check the call sites in Chrome to ensure they aren't
expecting a non-null pointer (running the blink layout tests should be enough).

[xidachen, 2015/12/10 15:16:41]

Done.

+        blurFilter = SkAutoTUnref<SkImageFilter>(SkBlurImageFilter::Create(sigma.fX, sigma.fY));
     SkAutoTUnref<SkColorFilter> colorFilter(
         SkColorFilter::CreateModeFilter(fColor, SkXfermode::kSrcIn_Mode));
     SkPaint paint;
-    paint.setImageFilter(blurFilter.get());
+    if (blurFilter)

[reed1, 2015/12/10 13:36:49]

This check seems unnecessary, since line 87 will execute correctly either way.

[xidachen, 2015/12/10 15:16:41]

Done.

+        paint.setImageFilter(blurFilter.get());
+    else
+        paint.setImageFilter(nullptr);
     paint.setColorFilter(colorFilter.get());
     paint.setXfermodeMode(SkXfermode::kSrcOver_Mode);
     SkVector offsetVec = SkVector::Make(fDx, fDy);