Changeset 85299 in webkit


Ignore:
Timestamp:
Apr 28, 2011 9:07:33 PM (13 years ago)
Author:
Simon Fraser
Message:

2011-04-28 Simon Fraser <Simon Fraser>

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.
Location:
trunk
Files:
3 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r85298 r85299  
     12011-04-28  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Reviewed by Dirk Schulze.
     4
     5        1px box-shadow looks ugly
     6        https://bugs.webkit.org/show_bug.cgi?id=58100
     7        and
     8        ShadowBlur incorrectly handles zero-sized blur radius in one axis
     9        https://bugs.webkit.org/show_bug.cgi?id=59710
     10       
     11        Pixel test for a single-pixel shadow.
     12
     13        * fast/box-shadow/single-pixel-shadow.html: Added.
     14        * platform/mac/fast/box-shadow/single-pixel-shadow-expected.png: Added.
     15        * platform/mac/fast/box-shadow/single-pixel-shadow-expected.txt: Added.
     16        * platform/mac/svg/dynamic-updates/SVGFEDropShadowElement-dom-shadow-color-attr-expected.png:
     17        * platform/mac/svg/dynamic-updates/SVGFEDropShadowElement-svgdom-shadow-color-prop-expected.png:
     18        * platform/mac/svg/filters/feDropShadow-expected.png:
     19
    1202011-04-28  Adam Roben  <aroben@apple.com>
    221

  • trunk/Source/WebCore/ChangeLog

    r85296 r85299  
     12011-04-28  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Reviewed by Dirk Schulze.
     4
     5        1px box-shadow looks ugly
     6        https://bugs.webkit.org/show_bug.cgi?id=58100
     7        and
     8        ShadowBlur incorrectly handles zero-sized blur radius in one axis
     9        https://bugs.webkit.org/show_bug.cgi?id=59710
     10       
     11        blurLayerImage() has issues at the edges if the blur radius
     12        is one, so in that case bump the buffer size out by a pixel.
     13        This results in a correct, symmetrical blur.
     14       
     15        Also fix an issue noticed during testing where a zero
     16        height or width radius would still blur on that axis,
     17        because we clamp the kernel size to a minimum of two.
     18
     19        Test: fast/box-shadow/single-pixel-shadow.html
     20
     21        * platform/graphics/ShadowBlur.h:
     22        * platform/graphics/ShadowBlur.cpp:
     23        (WebCore::ShadowBlur::blurLayerImage):
     24        Skip horizontal or vertial passes if the radius on that axis is zero.
     25        Move the "if (pass && m_blurRadius.width() != m_blurRadius.height())"
     26        clause to the end of the loop, since it only needs to execute once
     27        after the first pass.       
     28        (WebCore::ShadowBlur::blurredEdgeSize):
     29        New method to compute the width of the blurred edge (radius + extra
     30        pixel when necessary).
     31        (WebCore::ShadowBlur::calculateLayerBoundingRect):
     32        (WebCore::ShadowBlur::templateSize):
     33        (WebCore::ShadowBlur::drawRectShadow):
     34        (WebCore::ShadowBlur::drawInsetShadow):
     35        (WebCore::ShadowBlur::drawInsetShadowWithTiling):
     36        (WebCore::ShadowBlur::drawRectShadowWithTiling):
     37        (WebCore::ShadowBlur::drawLayerPieces):
     38        Use the result of blurredEdgeSize() rather than recomputing.
     39
    1402011-04-28  Yael Aharon  <yael.aharon@nokia.com>
    241

  • trunk/Source/WebCore/platform/graphics/ShadowBlur.cpp

    r85094 r85299  
    253253    // Two stages: horizontal and vertical
    254254    for (int pass = 0; pass < 2; ++pass) {
    255         if (pass && m_blurRadius.width() != m_blurRadius.height())
    256             calculateLobes(lobes, m_blurRadius.height(), m_shadowsIgnoreTransforms);
    257255        unsigned char* pixels = imageData;
     256       
     257        if (!pass && !m_blurRadius.width())
     258            final = 0; // Do no work if horizonal blur is zero.
    258259
    259260        for (int j = 0; j < final; ++j, pixels += delta) {
    … …  
    310311        final = size.width();
    311312        dim = size.height();
     313
     314        if (!m_blurRadius.height())
     315            break;
     316
     317        if (m_blurRadius.width() != m_blurRadius.height())
     318            calculateLobes(lobes, m_blurRadius.height(), m_shadowsIgnoreTransforms);
    312319    }
    313320}
    … …  
    344351}
    345352
     353IntSize ShadowBlur::blurredEdgeSize() const
     354{
     355    IntSize edgeSize = expandedIntSize(m_blurRadius);
     356
     357    // To avoid slowing down blurLayerImage() for radius == 1, we give it two empty pixels on each side.
     358    if (edgeSize.width() == 1)
     359        edgeSize.setWidth(2);
     360
     361    if (edgeSize.height() == 1)
     362        edgeSize.setHeight(2);
     363
     364    return edgeSize;
     365}
     366
    346367IntRect ShadowBlur::calculateLayerBoundingRect(GraphicsContext* context, const FloatRect& shadowedRect, const IntRect& clipRect)
    347368{
    348     const IntSize roundedRadius = expandedIntSize(m_blurRadius);
     369    IntSize edgeSize = blurredEdgeSize();
    349370
    350371    // Calculate the destination of the blurred and/or transformed layer.
    … …  
    364385    // We expand the area by the blur radius to give extra space for the blur transition.
    365386    if (m_type == BlurShadow) {
    366         layerRect.inflateX(roundedRadius.width());
    367         layerRect.inflateY(roundedRadius.height());
    368         inflation = roundedRadius;
     387        layerRect.inflateX(edgeSize.width());
     388        layerRect.inflateY(edgeSize.height());
     389        inflation = edgeSize;
    369390    }
    370391
    … …  
    380401        // so intersect with the clip inflated by the blur.
    381402        if (m_type == BlurShadow) {
    382             inflatedClip.inflateX(roundedRadius.width());
    383             inflatedClip.inflateY(roundedRadius.height());
     403            inflatedClip.inflateX(edgeSize.width());
     404            inflatedClip.inflateY(edgeSize.height());
    384405        }
    385406       
    … …  
    434455}
    435456
    436 IntSize ShadowBlur::templateSize(const RoundedIntRect::Radii& radii) const
     457IntSize ShadowBlur::templateSize(const IntSize& radiusPadding, const RoundedIntRect::Radii& radii) const
    437458{
    438459    const int templateSideLength = 1;
    … …  
    442463    int topSlice;
    443464    int bottomSlice;
    444     IntSize twiceRadius = expandedIntSize(m_blurRadius);
    445     twiceRadius.scale(2);
    446 
    447     computeSliceSizesFromRadii(twiceRadius, radii, leftSlice, rightSlice, topSlice, bottomSlice);
     465   
     466    IntSize blurExpansion = radiusPadding;
     467    blurExpansion.scale(2);
     468
     469    computeSliceSizesFromRadii(blurExpansion, radii, leftSlice, rightSlice, topSlice, bottomSlice);
    448470   
    449471    return IntSize(templateSideLength + leftSlice + rightSlice,
    … …  
    466488    }
    467489
    468     IntSize templateSize = this->templateSize(radii);
     490    IntSize edgeSize = blurredEdgeSize();
     491    IntSize templateSize = this->templateSize(edgeSize, radii);
    469492
    470493    if (templateSize.width() > shadowedRect.width() || templateSize.height() > shadowedRect.height()
    … …  
    474497    }
    475498
    476     drawRectShadowWithTiling(graphicsContext, shadowedRect, radii, templateSize);
     499    drawRectShadowWithTiling(graphicsContext, shadowedRect, radii, templateSize, edgeSize);
    477500}
    478501
    … …  
    492515    }
    493516
    494     IntSize templateSize = this->templateSize(holeRadii);
     517    IntSize edgeSize = blurredEdgeSize();
     518    IntSize templateSize = this->templateSize(edgeSize, holeRadii);
    495519
    496520    if (templateSize.width() > holeRect.width() || templateSize.height() > holeRect.height()
    … …  
    500524    }
    501525
    502     drawInsetShadowWithTiling(graphicsContext, rect, holeRect, holeRadii, templateSize);
     526    drawInsetShadowWithTiling(graphicsContext, rect, holeRect, holeRadii, templateSize, edgeSize);
    503527}
    504528
    … …  
    610634 */
    611635
    612 void ShadowBlur::drawInsetShadowWithTiling(GraphicsContext* graphicsContext, const FloatRect& rect, const FloatRect& holeRect, const RoundedIntRect::Radii& radii, const IntSize& templateSize)
     636void ShadowBlur::drawInsetShadowWithTiling(GraphicsContext* graphicsContext, const FloatRect& rect, const FloatRect& holeRect, const RoundedIntRect::Radii& radii, const IntSize& templateSize, const IntSize& edgeSize)
    613637{
    614638    GraphicsContextStateSaver stateSaver(*graphicsContext);
    615639    graphicsContext->clearShadow();
    616640
    617     const IntSize roundedRadius = expandedIntSize(m_blurRadius);
    618     const IntSize twiceRadius = IntSize(roundedRadius.width() * 2, roundedRadius.height() * 2);
    619 
    620641    m_layerImage = ScratchBuffer::shared().getScratchBuffer(templateSize);
    621642    if (!m_layerImage)
    … …  
    624645    // Draw the rectangle with hole.
    625646    FloatRect templateBounds(0, 0, templateSize.width(), templateSize.height());
    626     FloatRect templateHole = FloatRect(roundedRadius.width(), roundedRadius.height(), templateSize.width() - twiceRadius.width(), templateSize.height() - twiceRadius.height());
     647    FloatRect templateHole = FloatRect(edgeSize.width(), edgeSize.height(), templateSize.width() - 2 * edgeSize.width(), templateSize.height() - 2 * edgeSize.height());
    627648
    628649    if (!ScratchBuffer::shared().matchesLastInsetShadow(m_blurRadius, m_color, m_colorSpace, templateBounds, templateHole, radii)) {
    … …  
    654675    destHoleRect.move(m_offset);
    655676    FloatRect destHoleBounds = destHoleRect;
    656     destHoleBounds.inflateX(roundedRadius.width());
    657     destHoleBounds.inflateY(roundedRadius.height());
     677    destHoleBounds.inflateX(edgeSize.width());
     678    destHoleBounds.inflateY(edgeSize.height());
    658679
    659680    // Fill the external part of the shadow (which may be visible because of offset).
    … …  
    669690    }
    670691   
    671     drawLayerPieces(graphicsContext, destHoleBounds, radii, roundedRadius, templateSize, InnerShadow);
     692    drawLayerPieces(graphicsContext, destHoleBounds, radii, edgeSize, templateSize, InnerShadow);
    672693
    673694    m_layerImage = 0;
    … …  
    675696}
    676697
    677 void ShadowBlur::drawRectShadowWithTiling(GraphicsContext* graphicsContext, const FloatRect& shadowedRect, const RoundedIntRect::Radii& radii, const IntSize& templateSize)
     698void ShadowBlur::drawRectShadowWithTiling(GraphicsContext* graphicsContext, const FloatRect& shadowedRect, const RoundedIntRect::Radii& radii, const IntSize& templateSize, const IntSize& edgeSize)
    678699{
    679700    GraphicsContextStateSaver stateSaver(*graphicsContext);
    680701    graphicsContext->clearShadow();
    681702
    682     const IntSize roundedRadius = expandedIntSize(m_blurRadius);
    683     const IntSize twiceRadius = IntSize(roundedRadius.width() * 2, roundedRadius.height() * 2);
    684 
    685703    m_layerImage = ScratchBuffer::shared().getScratchBuffer(templateSize);
    686704    if (!m_layerImage)
    687705        return;
    688706
    689     FloatRect templateShadow = FloatRect(roundedRadius.width(), roundedRadius.height(), templateSize.width() - twiceRadius.width(), templateSize.height() - twiceRadius.height());
     707    FloatRect templateShadow = FloatRect(edgeSize.width(), edgeSize.height(), templateSize.width() - 2 * edgeSize.width(), templateSize.height() - 2 * edgeSize.height());
    690708
    691709    if (!ScratchBuffer::shared().matchesLastShadow(m_blurRadius, m_color, m_colorSpace, templateShadow, radii)) {
    … …  
    712730    FloatRect shadowBounds = shadowedRect;
    713731    shadowBounds.move(m_offset.width(), m_offset.height());
    714     shadowBounds.inflateX(roundedRadius.width());
    715     shadowBounds.inflateY(roundedRadius.height());
    716 
    717     drawLayerPieces(graphicsContext, shadowBounds, radii, roundedRadius, templateSize, OuterShadow);
     732    shadowBounds.inflateX(edgeSize.width());
     733    shadowBounds.inflateY(edgeSize.height());
     734
     735    drawLayerPieces(graphicsContext, shadowBounds, radii, edgeSize, templateSize, OuterShadow);
    718736
    719737    m_layerImage = 0;
    … …  
    721739}
    722740
    723 void ShadowBlur::drawLayerPieces(GraphicsContext* graphicsContext, const FloatRect& shadowBounds, const RoundedIntRect::Radii& radii, const IntSize& roundedRadius, const IntSize& templateSize, ShadowDirection direction)
    724 {
    725     const IntSize twiceRadius = IntSize(roundedRadius.width() * 2, roundedRadius.height() * 2);
     741void ShadowBlur::drawLayerPieces(GraphicsContext* graphicsContext, const FloatRect& shadowBounds, const RoundedIntRect::Radii& radii, const IntSize& bufferPadding, const IntSize& templateSize, ShadowDirection direction)
     742{
     743    const IntSize twiceRadius = IntSize(bufferPadding.width() * 2, bufferPadding.height() * 2);
    726744
    727745    int leftSlice;

  • trunk/Source/WebCore/platform/graphics/ShadowBlur.h

    r84522 r85299  
    6666   
    6767    IntRect calculateLayerBoundingRect(GraphicsContext*, const FloatRect& layerArea, const IntRect& clipRect);
    68     IntSize templateSize(const RoundedIntRect::Radii&) const;
     68    IntSize templateSize(const IntSize& blurredEdgeSize, const RoundedIntRect::Radii&) const;
    6969
    7070    void drawRectShadowWithoutTiling(GraphicsContext*, const FloatRect&, const RoundedIntRect::Radii&, const IntRect& layerRect);
    71     void drawRectShadowWithTiling(GraphicsContext*, const FloatRect&, const RoundedIntRect::Radii&, const IntSize& shadowTemplateSize);
     71    void drawRectShadowWithTiling(GraphicsContext*, const FloatRect&, const RoundedIntRect::Radii&, const IntSize& shadowTemplateSize, const IntSize& blurredEdgeSize);
    7272
    7373    void drawInsetShadowWithoutTiling(GraphicsContext*, const FloatRect&, const FloatRect& holeRect, const RoundedIntRect::Radii&, const IntRect& layerRect);
    74     void drawInsetShadowWithTiling(GraphicsContext*, const FloatRect&, const FloatRect& holeRect, const RoundedIntRect::Radii&, const IntSize& shadowTemplateSize);
     74    void drawInsetShadowWithTiling(GraphicsContext*, const FloatRect&, const FloatRect& holeRect, const RoundedIntRect::Radii&, const IntSize& shadowTemplateSize, const IntSize& blurredEdgeSize);
    7575   
    7676    void drawLayerPieces(GraphicsContext*, const FloatRect& shadowBounds, const RoundedIntRect::Radii&, const IntSize& roundedRadius, const IntSize& templateSize, ShadowDirection);
    … …  
    7878    void blurShadowBuffer(const IntSize& templateSize);
    7979    void blurAndColorShadowBuffer(const IntSize& templateSize);
     80   
     81    IntSize blurredEdgeSize() const;
    8082   
    8183    enum ShadowType {
Note: See TracChangeset for help on using the changeset viewer.