2011-04-28 Simon Fraser <simon.fraser@apple.com>
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Apr 2011 04:07:33 +0000 (04:07 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Apr 2011 04:07:33 +0000 (04:07 +0000)
        Reviewed by Dirk Schulze.

        1px box-shadow looks ugly
        https://bugs.webkit.org/show_bug.cgi?id=58100
        and
        ShadowBlur incorrectly handles zero-sized blur radius in one axis
        https://bugs.webkit.org/show_bug.cgi?id=59710

        blurLayerImage() has issues at the edges if the blur radius
        is one, so in that case bump the buffer size out by a pixel.
        This results in a correct, symmetrical blur.

        Also fix an issue noticed during testing where a zero
        height or width radius would still blur on that axis,
        because we clamp the kernel size to a minimum of two.

        Test: fast/box-shadow/single-pixel-shadow.html

        * platform/graphics/ShadowBlur.h:
        * platform/graphics/ShadowBlur.cpp:
        (WebCore::ShadowBlur::blurLayerImage):
        Skip horizontal or vertial passes if the radius on that axis is zero.
        Move the "if (pass && m_blurRadius.width() != m_blurRadius.height())"
        clause to the end of the loop, since it only needs to execute once
        after the first pass.
        (WebCore::ShadowBlur::blurredEdgeSize):
        New method to compute the width of the blurred edge (radius + extra
        pixel when necessary).
        (WebCore::ShadowBlur::calculateLayerBoundingRect):
        (WebCore::ShadowBlur::templateSize):
        (WebCore::ShadowBlur::drawRectShadow):
        (WebCore::ShadowBlur::drawInsetShadow):
        (WebCore::ShadowBlur::drawInsetShadowWithTiling):
        (WebCore::ShadowBlur::drawRectShadowWithTiling):
        (WebCore::ShadowBlur::drawLayerPieces):
        Use the result of blurredEdgeSize() rather than recomputing.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@85299 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/box-shadow/single-pixel-shadow.html [new file with mode: 0644]
LayoutTests/platform/mac/fast/box-shadow/single-pixel-shadow-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/fast/box-shadow/single-pixel-shadow-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/svg/dynamic-updates/SVGFEDropShadowElement-dom-shadow-color-attr-expected.png
LayoutTests/platform/mac/svg/dynamic-updates/SVGFEDropShadowElement-svgdom-shadow-color-prop-expected.png
LayoutTests/platform/mac/svg/filters/feDropShadow-expected.png
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/ShadowBlur.cpp
Source/WebCore/platform/graphics/ShadowBlur.h

index 1d0c7bd..54815a0 100644 (file)
@@ -1,3 +1,22 @@
+2011-04-28  Simon Fraser  <simon.fraser@apple.com>
+
+        Reviewed by Dirk Schulze.
+
+        1px box-shadow looks ugly
+        https://bugs.webkit.org/show_bug.cgi?id=58100
+        and
+        ShadowBlur incorrectly handles zero-sized blur radius in one axis
+        https://bugs.webkit.org/show_bug.cgi?id=59710
+        
+        Pixel test for a single-pixel shadow.
+
+        * fast/box-shadow/single-pixel-shadow.html: Added.
+        * platform/mac/fast/box-shadow/single-pixel-shadow-expected.png: Added.
+        * platform/mac/fast/box-shadow/single-pixel-shadow-expected.txt: Added.
+        * platform/mac/svg/dynamic-updates/SVGFEDropShadowElement-dom-shadow-color-attr-expected.png:
+        * platform/mac/svg/dynamic-updates/SVGFEDropShadowElement-svgdom-shadow-color-prop-expected.png:
+        * platform/mac/svg/filters/feDropShadow-expected.png:
+
 2011-04-28  Adam Roben  <aroben@apple.com>
 
         Skip all fast/dom/shadow tests on Mac
diff --git a/LayoutTests/fast/box-shadow/single-pixel-shadow.html b/LayoutTests/fast/box-shadow/single-pixel-shadow.html
new file mode 100644 (file)
index 0000000..792fc55
--- /dev/null
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+  <style>
+    .box {
+      height: 4px;
+      width: 4px;
+      margin: 20px;
+      box-shadow: 0 0 1px black;
+    }
+    
+    .scaled {
+      -webkit-transform-origin: -1px -1px;
+      -webkit-transform: scale(20);
+    }
+  </style>
+</head>
+<body>
+  <div class="box"></div>
+  <div class="scaled box"></div>
+</body>
+</html>
diff --git a/LayoutTests/platform/mac/fast/box-shadow/single-pixel-shadow-expected.png b/LayoutTests/platform/mac/fast/box-shadow/single-pixel-shadow-expected.png
new file mode 100644 (file)
index 0000000..957d467
Binary files /dev/null and b/LayoutTests/platform/mac/fast/box-shadow/single-pixel-shadow-expected.png differ
diff --git a/LayoutTests/platform/mac/fast/box-shadow/single-pixel-shadow-expected.txt b/LayoutTests/platform/mac/fast/box-shadow/single-pixel-shadow-expected.txt
new file mode 100644 (file)
index 0000000..90231e9
--- /dev/null
@@ -0,0 +1,8 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x68
+  RenderBlock {HTML} at (0,0) size 800x68
+    RenderBody {BODY} at (8,20) size 784x28
+      RenderBlock {DIV} at (20,0) size 4x4
+layer at (28,44) size 4x4
+  RenderBlock {DIV} at (20,24) size 4x4
index c34b257..cb2b6f9 100644 (file)
Binary files a/LayoutTests/platform/mac/svg/dynamic-updates/SVGFEDropShadowElement-dom-shadow-color-attr-expected.png and b/LayoutTests/platform/mac/svg/dynamic-updates/SVGFEDropShadowElement-dom-shadow-color-attr-expected.png differ
index d0cf79e..27b6262 100644 (file)
Binary files a/LayoutTests/platform/mac/svg/dynamic-updates/SVGFEDropShadowElement-svgdom-shadow-color-prop-expected.png and b/LayoutTests/platform/mac/svg/dynamic-updates/SVGFEDropShadowElement-svgdom-shadow-color-prop-expected.png differ
index ca63109..1d8056f 100644 (file)
Binary files a/LayoutTests/platform/mac/svg/filters/feDropShadow-expected.png and b/LayoutTests/platform/mac/svg/filters/feDropShadow-expected.png differ
index 9e3359f..b01d344 100644 (file)
@@ -1,3 +1,42 @@
+2011-04-28  Simon Fraser  <simon.fraser@apple.com>
+
+        Reviewed by Dirk Schulze.
+
+        1px box-shadow looks ugly
+        https://bugs.webkit.org/show_bug.cgi?id=58100
+        and
+        ShadowBlur incorrectly handles zero-sized blur radius in one axis
+        https://bugs.webkit.org/show_bug.cgi?id=59710
+        
+        blurLayerImage() has issues at the edges if the blur radius
+        is one, so in that case bump the buffer size out by a pixel.
+        This results in a correct, symmetrical blur.
+        
+        Also fix an issue noticed during testing where a zero
+        height or width radius would still blur on that axis,
+        because we clamp the kernel size to a minimum of two.
+
+        Test: fast/box-shadow/single-pixel-shadow.html
+
+        * platform/graphics/ShadowBlur.h:
+        * platform/graphics/ShadowBlur.cpp:
+        (WebCore::ShadowBlur::blurLayerImage):
+        Skip horizontal or vertial passes if the radius on that axis is zero.
+        Move the "if (pass && m_blurRadius.width() != m_blurRadius.height())"
+        clause to the end of the loop, since it only needs to execute once
+        after the first pass.        
+        (WebCore::ShadowBlur::blurredEdgeSize):
+        New method to compute the width of the blurred edge (radius + extra
+        pixel when necessary).
+        (WebCore::ShadowBlur::calculateLayerBoundingRect):
+        (WebCore::ShadowBlur::templateSize):
+        (WebCore::ShadowBlur::drawRectShadow):
+        (WebCore::ShadowBlur::drawInsetShadow):
+        (WebCore::ShadowBlur::drawInsetShadowWithTiling):
+        (WebCore::ShadowBlur::drawRectShadowWithTiling):
+        (WebCore::ShadowBlur::drawLayerPieces):
+        Use the result of blurredEdgeSize() rather than recomputing.
+
 2011-04-28  Yael Aharon  <yael.aharon@nokia.com>
 
         Reviewed by Beth Dakin.
index b3d1f89..ae5ec48 100644 (file)
@@ -252,9 +252,10 @@ void ShadowBlur::blurLayerImage(unsigned char* imageData, const IntSize& size, i
 
     // Two stages: horizontal and vertical
     for (int pass = 0; pass < 2; ++pass) {
-        if (pass && m_blurRadius.width() != m_blurRadius.height())
-            calculateLobes(lobes, m_blurRadius.height(), m_shadowsIgnoreTransforms);
         unsigned char* pixels = imageData;
+        
+        if (!pass && !m_blurRadius.width())
+            final = 0; // Do no work if horizonal blur is zero.
 
         for (int j = 0; j < final; ++j, pixels += delta) {
             // For each step, we blur the alpha in a channel and store the result
@@ -309,6 +310,12 @@ void ShadowBlur::blurLayerImage(unsigned char* imageData, const IntSize& size, i
         delta = 4;
         final = size.width();
         dim = size.height();
+
+        if (!m_blurRadius.height())
+            break;
+
+        if (m_blurRadius.width() != m_blurRadius.height())
+            calculateLobes(lobes, m_blurRadius.height(), m_shadowsIgnoreTransforms);
     }
 }
 
@@ -343,9 +350,23 @@ void ShadowBlur::adjustBlurRadius(GraphicsContext* context)
     m_blurRadius.scale(1 / xAxisScale, 1 / yAxisScale);
 }
 
+IntSize ShadowBlur::blurredEdgeSize() const
+{
+    IntSize edgeSize = expandedIntSize(m_blurRadius);
+
+    // To avoid slowing down blurLayerImage() for radius == 1, we give it two empty pixels on each side.
+    if (edgeSize.width() == 1)
+        edgeSize.setWidth(2);
+
+    if (edgeSize.height() == 1)
+        edgeSize.setHeight(2);
+
+    return edgeSize;
+}
+
 IntRect ShadowBlur::calculateLayerBoundingRect(GraphicsContext* context, const FloatRect& shadowedRect, const IntRect& clipRect)
 {
-    const IntSize roundedRadius = expandedIntSize(m_blurRadius);
+    IntSize edgeSize = blurredEdgeSize();
 
     // Calculate the destination of the blurred and/or transformed layer.
     FloatRect layerRect;
@@ -363,9 +384,9 @@ IntRect ShadowBlur::calculateLayerBoundingRect(GraphicsContext* context, const F
 
     // We expand the area by the blur radius to give extra space for the blur transition.
     if (m_type == BlurShadow) {
-        layerRect.inflateX(roundedRadius.width());
-        layerRect.inflateY(roundedRadius.height());
-        inflation = roundedRadius;
+        layerRect.inflateX(edgeSize.width());
+        layerRect.inflateY(edgeSize.height());
+        inflation = edgeSize;
     }
 
     FloatRect unclippedLayerRect = layerRect;
@@ -379,8 +400,8 @@ IntRect ShadowBlur::calculateLayerBoundingRect(GraphicsContext* context, const F
         // Pixels at the edges can be affected by pixels outside the buffer,
         // so intersect with the clip inflated by the blur.
         if (m_type == BlurShadow) {
-            inflatedClip.inflateX(roundedRadius.width());
-            inflatedClip.inflateY(roundedRadius.height());
+            inflatedClip.inflateX(edgeSize.width());
+            inflatedClip.inflateY(edgeSize.height());
         }
         
         layerRect.intersect(inflatedClip);
@@ -433,7 +454,7 @@ static void computeSliceSizesFromRadii(const IntSize& twiceRadius, const Rounded
     bottomSlice = twiceRadius.height() + max(radii.bottomLeft().height(), radii.bottomRight().height());
 }
 
-IntSize ShadowBlur::templateSize(const RoundedIntRect::Radii& radii) const
+IntSize ShadowBlur::templateSize(const IntSize& radiusPadding, const RoundedIntRect::Radii& radii) const
 {
     const int templateSideLength = 1;
 
@@ -441,10 +462,11 @@ IntSize ShadowBlur::templateSize(const RoundedIntRect::Radii& radii) const
     int rightSlice;
     int topSlice;
     int bottomSlice;
-    IntSize twiceRadius = expandedIntSize(m_blurRadius);
-    twiceRadius.scale(2);
+    
+    IntSize blurExpansion = radiusPadding;
+    blurExpansion.scale(2);
 
-    computeSliceSizesFromRadii(twiceRadius, radii, leftSlice, rightSlice, topSlice, bottomSlice);
+    computeSliceSizesFromRadii(blurExpansion, radii, leftSlice, rightSlice, topSlice, bottomSlice);
     
     return IntSize(templateSideLength + leftSlice + rightSlice,
                    templateSideLength + topSlice + bottomSlice);
@@ -465,7 +487,8 @@ void ShadowBlur::drawRectShadow(GraphicsContext* graphicsContext, const FloatRec
         return;
     }
 
-    IntSize templateSize = this->templateSize(radii);
+    IntSize edgeSize = blurredEdgeSize();
+    IntSize templateSize = this->templateSize(edgeSize, radii);
 
     if (templateSize.width() > shadowedRect.width() || templateSize.height() > shadowedRect.height()
         || (templateSize.width() * templateSize.height() > m_sourceRect.width() * m_sourceRect.height())) {
@@ -473,7 +496,7 @@ void ShadowBlur::drawRectShadow(GraphicsContext* graphicsContext, const FloatRec
         return;
     }
 
-    drawRectShadowWithTiling(graphicsContext, shadowedRect, radii, templateSize);
+    drawRectShadowWithTiling(graphicsContext, shadowedRect, radii, templateSize, edgeSize);
 }
 
 void ShadowBlur::drawInsetShadow(GraphicsContext* graphicsContext, const FloatRect& rect, const FloatRect& holeRect, const RoundedIntRect::Radii& holeRadii)
@@ -491,7 +514,8 @@ void ShadowBlur::drawInsetShadow(GraphicsContext* graphicsContext, const FloatRe
         return;
     }
 
-    IntSize templateSize = this->templateSize(holeRadii);
+    IntSize edgeSize = blurredEdgeSize();
+    IntSize templateSize = this->templateSize(edgeSize, holeRadii);
 
     if (templateSize.width() > holeRect.width() || templateSize.height() > holeRect.height()
         || (templateSize.width() * templateSize.height() > holeRect.width() * holeRect.height())) {
@@ -499,7 +523,7 @@ void ShadowBlur::drawInsetShadow(GraphicsContext* graphicsContext, const FloatRe
         return;
     }
 
-    drawInsetShadowWithTiling(graphicsContext, rect, holeRect, holeRadii, templateSize);
+    drawInsetShadowWithTiling(graphicsContext, rect, holeRect, holeRadii, templateSize, edgeSize);
 }
 
 void ShadowBlur::drawRectShadowWithoutTiling(GraphicsContext* graphicsContext, const FloatRect& shadowedRect, const RoundedIntRect::Radii& radii, const IntRect& layerRect)
@@ -609,21 +633,18 @@ void ShadowBlur::drawInsetShadowWithoutTiling(GraphicsContext* graphicsContext,
      the shadow.
  */
 
-void ShadowBlur::drawInsetShadowWithTiling(GraphicsContext* graphicsContext, const FloatRect& rect, const FloatRect& holeRect, const RoundedIntRect::Radii& radii, const IntSize& templateSize)
+void ShadowBlur::drawInsetShadowWithTiling(GraphicsContext* graphicsContext, const FloatRect& rect, const FloatRect& holeRect, const RoundedIntRect::Radii& radii, const IntSize& templateSize, const IntSize& edgeSize)
 {
     GraphicsContextStateSaver stateSaver(*graphicsContext);
     graphicsContext->clearShadow();
 
-    const IntSize roundedRadius = expandedIntSize(m_blurRadius);
-    const IntSize twiceRadius = IntSize(roundedRadius.width() * 2, roundedRadius.height() * 2);
-
     m_layerImage = ScratchBuffer::shared().getScratchBuffer(templateSize);
     if (!m_layerImage)
         return;
 
     // Draw the rectangle with hole.
     FloatRect templateBounds(0, 0, templateSize.width(), templateSize.height());
-    FloatRect templateHole = FloatRect(roundedRadius.width(), roundedRadius.height(), templateSize.width() - twiceRadius.width(), templateSize.height() - twiceRadius.height());
+    FloatRect templateHole = FloatRect(edgeSize.width(), edgeSize.height(), templateSize.width() - 2 * edgeSize.width(), templateSize.height() - 2 * edgeSize.height());
 
     if (!ScratchBuffer::shared().matchesLastInsetShadow(m_blurRadius, m_color, m_colorSpace, templateBounds, templateHole, radii)) {
         // Draw shadow into a new ImageBuffer.
@@ -653,8 +674,8 @@ void ShadowBlur::drawInsetShadowWithTiling(GraphicsContext* graphicsContext, con
     FloatRect destHoleRect = holeRect;
     destHoleRect.move(m_offset);
     FloatRect destHoleBounds = destHoleRect;
-    destHoleBounds.inflateX(roundedRadius.width());
-    destHoleBounds.inflateY(roundedRadius.height());
+    destHoleBounds.inflateX(edgeSize.width());
+    destHoleBounds.inflateY(edgeSize.height());
 
     // Fill the external part of the shadow (which may be visible because of offset).
     Path exteriorPath;
@@ -668,25 +689,22 @@ void ShadowBlur::drawInsetShadowWithTiling(GraphicsContext* graphicsContext, con
         graphicsContext->fillPath(exteriorPath);
     }
     
-    drawLayerPieces(graphicsContext, destHoleBounds, radii, roundedRadius, templateSize, InnerShadow);
+    drawLayerPieces(graphicsContext, destHoleBounds, radii, edgeSize, templateSize, InnerShadow);
 
     m_layerImage = 0;
     ScratchBuffer::shared().scheduleScratchBufferPurge();
 }
 
-void ShadowBlur::drawRectShadowWithTiling(GraphicsContext* graphicsContext, const FloatRect& shadowedRect, const RoundedIntRect::Radii& radii, const IntSize& templateSize)
+void ShadowBlur::drawRectShadowWithTiling(GraphicsContext* graphicsContext, const FloatRect& shadowedRect, const RoundedIntRect::Radii& radii, const IntSize& templateSize, const IntSize& edgeSize)
 {
     GraphicsContextStateSaver stateSaver(*graphicsContext);
     graphicsContext->clearShadow();
 
-    const IntSize roundedRadius = expandedIntSize(m_blurRadius);
-    const IntSize twiceRadius = IntSize(roundedRadius.width() * 2, roundedRadius.height() * 2);
-
     m_layerImage = ScratchBuffer::shared().getScratchBuffer(templateSize);
     if (!m_layerImage)
         return;
 
-    FloatRect templateShadow = FloatRect(roundedRadius.width(), roundedRadius.height(), templateSize.width() - twiceRadius.width(), templateSize.height() - twiceRadius.height());
+    FloatRect templateShadow = FloatRect(edgeSize.width(), edgeSize.height(), templateSize.width() - 2 * edgeSize.width(), templateSize.height() - 2 * edgeSize.height());
 
     if (!ScratchBuffer::shared().matchesLastShadow(m_blurRadius, m_color, m_colorSpace, templateShadow, radii)) {
         // Draw shadow into the ImageBuffer.
@@ -711,18 +729,18 @@ void ShadowBlur::drawRectShadowWithTiling(GraphicsContext* graphicsContext, cons
 
     FloatRect shadowBounds = shadowedRect;
     shadowBounds.move(m_offset.width(), m_offset.height());
-    shadowBounds.inflateX(roundedRadius.width());
-    shadowBounds.inflateY(roundedRadius.height());
+    shadowBounds.inflateX(edgeSize.width());
+    shadowBounds.inflateY(edgeSize.height());
 
-    drawLayerPieces(graphicsContext, shadowBounds, radii, roundedRadius, templateSize, OuterShadow);
+    drawLayerPieces(graphicsContext, shadowBounds, radii, edgeSize, templateSize, OuterShadow);
 
     m_layerImage = 0;
     ScratchBuffer::shared().scheduleScratchBufferPurge();
 }
 
-void ShadowBlur::drawLayerPieces(GraphicsContext* graphicsContext, const FloatRect& shadowBounds, const RoundedIntRect::Radii& radii, const IntSize& roundedRadius, const IntSize& templateSize, ShadowDirection direction)
+void ShadowBlur::drawLayerPieces(GraphicsContext* graphicsContext, const FloatRect& shadowBounds, const RoundedIntRect::Radii& radii, const IntSize& bufferPadding, const IntSize& templateSize, ShadowDirection direction)
 {
-    const IntSize twiceRadius = IntSize(roundedRadius.width() * 2, roundedRadius.height() * 2);
+    const IntSize twiceRadius = IntSize(bufferPadding.width() * 2, bufferPadding.height() * 2);
 
     int leftSlice;
     int rightSlice;
index 2ba87ff..202ff3d 100644 (file)
@@ -65,19 +65,21 @@ private:
     };
     
     IntRect calculateLayerBoundingRect(GraphicsContext*, const FloatRect& layerArea, const IntRect& clipRect);
-    IntSize templateSize(const RoundedIntRect::Radii&) const;
+    IntSize templateSize(const IntSize& blurredEdgeSize, const RoundedIntRect::Radii&) const;
 
     void drawRectShadowWithoutTiling(GraphicsContext*, const FloatRect&, const RoundedIntRect::Radii&, const IntRect& layerRect);
-    void drawRectShadowWithTiling(GraphicsContext*, const FloatRect&, const RoundedIntRect::Radii&, const IntSize& shadowTemplateSize);
+    void drawRectShadowWithTiling(GraphicsContext*, const FloatRect&, const RoundedIntRect::Radii&, const IntSize& shadowTemplateSize, const IntSize& blurredEdgeSize);
 
     void drawInsetShadowWithoutTiling(GraphicsContext*, const FloatRect&, const FloatRect& holeRect, const RoundedIntRect::Radii&, const IntRect& layerRect);
-    void drawInsetShadowWithTiling(GraphicsContext*, const FloatRect&, const FloatRect& holeRect, const RoundedIntRect::Radii&, const IntSize& shadowTemplateSize);
+    void drawInsetShadowWithTiling(GraphicsContext*, const FloatRect&, const FloatRect& holeRect, const RoundedIntRect::Radii&, const IntSize& shadowTemplateSize, const IntSize& blurredEdgeSize);
     
     void drawLayerPieces(GraphicsContext*, const FloatRect& shadowBounds, const RoundedIntRect::Radii&, const IntSize& roundedRadius, const IntSize& templateSize, ShadowDirection);
     
     void blurShadowBuffer(const IntSize& templateSize);
     void blurAndColorShadowBuffer(const IntSize& templateSize);
     
+    IntSize blurredEdgeSize() const;
+    
     enum ShadowType {
         NoShadow,
         SolidShadow,