Fixed Shadow blur for transparent objects

Source/platform/graphics/GraphicsContext.cpp

Index: Source/platform/graphics/GraphicsContext.cpp
diff --git a/Source/platform/graphics/GraphicsContext.cpp b/Source/platform/graphics/GraphicsContext.cpp
index 38d4105bd7b6f85283bc7d9e443f692781b8ef1a..20387f524490e8823b3be9f9433afefce515e883 100644
--- a/Source/platform/graphics/GraphicsContext.cpp
+++ b/Source/platform/graphics/GraphicsContext.cpp
@@ -51,6 +51,7 @@
 #include "third_party/skia/include/core/SkSurface.h"
 #include "third_party/skia/include/effects/SkBlurMaskFilter.h"
 #include "third_party/skia/include/effects/SkCornerPathEffect.h"
+#include "third_party/skia/include/effects/SkDropShadowImageFilter.h"
 #include "third_party/skia/include/effects/SkLumaColorFilter.h"
 #include "third_party/skia/include/effects/SkMatrixImageFilter.h"
 #include "third_party/skia/include/effects/SkPictureImageFilter.h"
@@ -326,6 +327,14 @@ void GraphicsContext::setShadow(const FloatSize& offset, float blur, const Color
         drawLooperBuilder->addUnmodifiedContent();
     }
     setDrawLooper(drawLooperBuilder.release());
+
+    // The image filter MUST be set after the draw looper, because setting the draw looper will reset the image filter

[Stephen White, 2014/10/23 01:45:47]

I think we should just have setDrawLooper leave the image filter alone, and have
clearShadow() clear both the looper and the image filter. Then ordering would be
irrelevant, and caveat emptor for the callers (don't leave a setShadow() or a
setDrawLooper() active). Would that make sense?

[sugoi1, 2014/10/23 17:43:17]

Done.

+    if (shadowTransformMode == DrawLooperBuilder::ShadowIgnoresTransforms
+        && shadowAlphaMode == DrawLooperBuilder::ShadowRespectsAlpha) {

[Justin Novosad, 2014/10/23 02:29:21]

With this condition, you will regress performance in cases that are rendering
correctly today.  Look all the layout tests that are affected. Most of those
tests are drawing opaque primitives and should continue to use the old faster
code path (and therefore should not need to be rebaselined)

[sugoi1, 2014/10/23 17:43:17]

This image filter will only be used to replace the drawLooper created above if
the image is not opaque. Added the comment for clarity.

+        SkColor skColor = color.rgb();
+        const SkScalar sigma = 0.288675f * blur + 0.5f;
+        setImageFilter(adoptRef(SkDropShadowImageFilter::Create(offset.width(), offset.height(), sigma, sigma, skColor)));

[Stephen White, 2014/10/23 01:45:47]

I think we should put this in a RefPtr<SkImageFilter> for clarity, then pass it
on the next line.

[Justin Novosad, 2014/10/23 02:29:21]

You are not supporting the shadowMode==ShadowOnly case here. You will also need
a new layout test to cover that specific code path. One way of doing that would
be to create a test that draws an image with an alpha mask, with shadows turned
on, with the global composite operation set to destination-in.  Come to think of
it, Dongseong submitted a great layout test for shadows earlier (see here:
https://codereview.chromium.org/672593002/). You should create a new variant of
that test that uses drawImage with an image that has an alpha mask.

[sugoi1, 2014/10/23 17:43:17]

Done.

[sugoi1, 2014/10/23 17:43:17]

I'm not going to fix this now, so I'll skip this part of the fix for now and fix
it in a follow-up.

+    }
 }
 
 void GraphicsContext::setDrawLooper(PassOwnPtr<DrawLooperBuilder> drawLooperBuilder)
@@ -334,6 +343,8 @@ void GraphicsContext::setDrawLooper(PassOwnPtr<DrawLooperBuilder> drawLooperBuil
         return;
 
     mutableState()->setDrawLooper(drawLooperBuilder->detachDrawLooper());
+    // If the draw looper is overwritten externally, remove the image filter, as it is no longer equivalent to the draw looper
+    clearImageFilter();

[Stephen White, 2014/10/23 01:45:47]

So if you agree with the above, remove this.

[sugoi1, 2014/10/23 17:43:17]

Done.

 }
 
 void GraphicsContext::clearDrawLooper()
@@ -342,11 +353,29 @@ void GraphicsContext::clearDrawLooper()
         return;
 
     mutableState()->clearDrawLooper();
+    // If the draw looper is overwritten externally, remove the image filter, as it is no longer equivalent to the draw looper
+    clearImageFilter();

[Stephen White, 2014/10/23 01:45:47]

And this.

[sugoi1, 2014/10/23 17:43:17]

Done.

+}
+
+void GraphicsContext::setImageFilter(PassRefPtr<SkImageFilter> imageFilter)
+{
+    if (contextDisabled())
+        return;
+
+    mutableState()->setImageFilter(imageFilter);
+}
+
+void GraphicsContext::clearImageFilter()
+{
+    if (contextDisabled())
+        return;
+
+    mutableState()->clearImageFilter();
 }
 
 bool GraphicsContext::hasShadow() const
 {
-    return !!immutableState()->drawLooper();
+    return !!immutableState()->drawLooper() || !!immutableState()->imageFilter();
 }
 
 bool GraphicsContext::getTransformedClipBounds(FloatRect* bounds) const
@@ -1967,13 +1996,18 @@ void GraphicsContext::preparePaintForDrawRectToRect(
     const SkRect& destRect,
     CompositeOperator compositeOp,
     WebBlendMode blendMode,
+    bool isOpaque,
     bool isLazyDecoded,
     bool isDataComplete) const
 {
     paint->setXfermodeMode(WebCoreCompositeToSkiaComposite(compositeOp, blendMode));
     paint->setColorFilter(this->colorFilter());
     paint->setAlpha(this->getNormalizedAlpha());
-    paint->setLooper(this->drawLooper());
+    if (this->imageFilter() && !isOpaque) {
+        paint->setImageFilter(this->imageFilter());
+    } else {
+        paint->setLooper(this->drawLooper());
+    }
     paint->setAntiAlias(shouldDrawAntiAliased(this, destRect));
 
     InterpolationQuality resampling;