SkBlurImageFilter returns input when sigma = 0

src/effects/SkDropShadowImageFilter.cpp

 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkDropShadowImageFilter.h"
 
 #include "SkBitmap.h"
@@ skipping 59 matching lines @@
     SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(bounds.width(), bounds.height()));
     if (nullptr == device.get()) {
         return false;
     }
     SkCanvas canvas(device.get());
 
     SkVector sigma = SkVector::Make(fSigmaX, fSigmaY);
     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);
     ctx.ctm().mapVectors(&offsetVec, 1);
     canvas.translate(SkIntToScalar(srcOffset.fX - bounds.fLeft),
                      SkIntToScalar(srcOffset.fY - bounds.fTop));
     canvas.drawBitmap(src, offsetVec.fX, offsetVec.fY, &paint);
     if (fShadowMode == kDrawShadowAndForeground_ShadowMode) {
         canvas.drawBitmap(src, 0, 0);
     }
@@ skipping 56 matching lines @@
         "kDrawShadowAndForeground", "kDrawShadowOnly"
     };
 
     static_assert(kShadowModeCount == SK_ARRAY_COUNT(gModeStrings), "enum_mismatch");
 
     str->appendf(" mode: %s", gModeStrings[fShadowMode]);
 
     str->append(")");
 }
 #endif