Fixed Shadow blur for transparent objects

Source/platform/graphics/GraphicsContext.cpp

 /*
  * Copyright (C) 2003, 2004, 2005, 2006, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 33 matching lines @@
 #include "third_party/skia/include/core/SkColorFilter.h"
 #include "third_party/skia/include/core/SkData.h"
 #include "third_party/skia/include/core/SkDevice.h"
 #include "third_party/skia/include/core/SkPicture.h"
 #include "third_party/skia/include/core/SkPictureRecorder.h"
 #include "third_party/skia/include/core/SkRRect.h"
 #include "third_party/skia/include/core/SkRefCnt.h"
 #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"
 #include "third_party/skia/include/gpu/GrRenderTarget.h"
 #include "third_party/skia/include/gpu/GrTexture.h"
 #include "wtf/Assertions.h"
 #include "wtf/MathExtras.h"
 
 namespace {
 
@@ skipping 250 matching lines @@
     if (!color.alpha() || (!offset.width() && !offset.height() && !blur)) {
         if (shadowMode == DrawShadowOnly) {
             // shadow only, but there is no shadow: use an empty draw looper to disable rendering of the source primitive
             setDrawLooper(drawLooperBuilder.release());
             return;
         }
         clearShadow();
         return;
     }
 
     drawLooperBuilder->addShadow(offset, blur, color, shadowTransformMode, shadowAlphaMode);

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

We shouldn't be setting up a draw looper in cases where we are going to end up
using an image filter. It should be one or the other.

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

And it is. See preparePaintForDrawRectToRec().

     if (shadowMode == DrawShadowAndForeground) {
         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)
 {
     if (contextDisabled())
         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()
 {
     if (contextDisabled())
         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
 {
     if (contextDisabled())
         return false;
     ASSERT(m_canvas);
     SkIRect skIBounds;
     if (!m_canvas->getClipDeviceBounds(&skIBounds))
         return false;
@@ skipping 1600 matching lines @@
         m_textRegion.join(textRect);
     }
 }
 
 void GraphicsContext::preparePaintForDrawRectToRect(
     SkPaint* paint,
     const SkRect& srcRect,
     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;
     if (this->isAccelerated()) {
         resampling = InterpolationLow;
     } else if (this->printing()) {
         resampling = InterpolationNone;
     } else if (isLazyDecoded) {
         resampling = InterpolationHigh;
     } else {
@@ skipping 13 matching lines @@
         // FIXME: This is to not break tests (it results in the filter bitmap flag
         // being set to true). We need to decide if we respect InterpolationNone
         // being returned from computeInterpolationQuality.
         resampling = InterpolationLow;
     }
     resampling = limitInterpolationQuality(this, resampling);
     paint->setFilterLevel(static_cast<SkPaint::FilterLevel>(resampling));
 }
 
 } // namespace blink