Changeset 261334 in webkit


Ignore:
Timestamp:
May 7, 2020 2:02:37 PM (4 years ago)
Author:
Simon Fraser
Message:

REGRESSION (r252161): Animation of box-shadow with border-radius set is flashy
https://bugs.webkit.org/show_bug.cgi?id=211530
<rdar://problem/62570229>

Reviewed by Zalan Bujtas.

Source/WebCore:

Drawing inset box shadows with spread was broken, and in some cases generated invalid
rounded rects, causing inset shadows with border-radius to lose the radius.

First, with split inlines, the spread was not getting removed from the excluded edges.
Fix in the includeLogicalLeftEdge/includeLogicalRightEdge clauses, adding shiftXEdgeBy()/shiftYEdgeBy() helpers
to LayoutRect to simplify logic.

Second, when computing the rounded hole rect, we'd use the result of style.getRoundedInnerBorderFor()
but that doesn't take shadow spread into account, so we'd build a rounded rect with a rect that
accounted for spread, but radii computed without. That could result in unrenderable rounded rects.
Fix by calling style.getRoundedInnerBorderFor() a second time if we have spread; this will fix
up the radii for spread.

Tests: fast/box-shadow/inset-box-shadow-fractional-radius.html

fast/box-shadow/inset-shadow-split-inline.html
fast/box-shadow/inset-spread-box-shadow-split-inline.html

  • platform/graphics/LayoutRect.h:

(WebCore::LayoutRect::shiftXEdgeBy):
(WebCore::LayoutRect::shiftYEdgeBy):

  • rendering/RenderBoxModelObject.cpp:

(WebCore::RenderBoxModelObject::paintBoxShadow):

LayoutTests:

Some ref tests for inset shadows with border radius, with spread, and when on
split inlines.

  • fast/box-shadow/inset-box-shadow-expected.html:
  • fast/box-shadow/inset-box-shadow-fractional-radius-expected.html: Added.
  • fast/box-shadow/inset-box-shadow-fractional-radius.html: Added.
  • fast/box-shadow/inset-box-shadow.html:
  • fast/box-shadow/inset-shadow-split-inline-expected.html: Added.
  • fast/box-shadow/inset-shadow-split-inline.html: Added.
  • fast/box-shadow/inset-spread-box-shadow-split-inline-expected.html: Added.
  • fast/box-shadow/inset-spread-box-shadow-split-inline.html: Added.
Location:
trunk
Files:
6 added
8 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r261330 r261334  
     12020-05-07  Simon Fraser  <simon.fraser@apple.com>
     2
     3        REGRESSION (r252161): Animation of box-shadow with border-radius set is flashy
     4        https://bugs.webkit.org/show_bug.cgi?id=211530
     5        <rdar://problem/62570229>
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        Some ref tests for inset shadows with border radius, with spread, and when on
     10        split inlines.
     11
     12        * fast/box-shadow/inset-box-shadow-expected.html:
     13        * fast/box-shadow/inset-box-shadow-fractional-radius-expected.html: Added.
     14        * fast/box-shadow/inset-box-shadow-fractional-radius.html: Added.
     15        * fast/box-shadow/inset-box-shadow.html:
     16        * fast/box-shadow/inset-shadow-split-inline-expected.html: Added.
     17        * fast/box-shadow/inset-shadow-split-inline.html: Added.
     18        * fast/box-shadow/inset-spread-box-shadow-split-inline-expected.html: Added.
     19        * fast/box-shadow/inset-spread-box-shadow-split-inline.html: Added.
     20
    1212020-05-07  Ryan Haddad  <ryanhaddad@apple.com>
    222
  • trunk/LayoutTests/fast/box-shadow/inset-box-shadow-expected.html

    r252689 r261334  
    11<style>
    2 
    32    div {
    43        position: absolute;
     
    1110    }
    1211</style>
    13 <p>You should see a black outlined square here</p>
     12<p>You should see a black circle here</p>
    1413<div></div>
  • trunk/LayoutTests/fast/box-shadow/inset-box-shadow.html

    r252689 r261334  
    1010    }
    1111</style>
    12 <p>You should see a black outlined square here</p>
     12<p>You should see a black circle here</p>
    1313<div></div>
  • trunk/Source/WebCore/ChangeLog

    r261333 r261334  
     12020-05-07  Simon Fraser  <simon.fraser@apple.com>
     2
     3        REGRESSION (r252161): Animation of box-shadow with border-radius set is flashy
     4        https://bugs.webkit.org/show_bug.cgi?id=211530
     5        <rdar://problem/62570229>
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        Drawing inset box shadows with spread was broken, and in some cases generated invalid
     10        rounded rects, causing inset shadows with border-radius to lose the radius.
     11
     12        First, with split inlines, the spread was not getting removed from the excluded edges.
     13        Fix in the includeLogicalLeftEdge/includeLogicalRightEdge clauses, adding shiftXEdgeBy()/shiftYEdgeBy() helpers
     14        to LayoutRect to simplify logic.
     15
     16        Second, when computing the rounded hole rect, we'd use the result of style.getRoundedInnerBorderFor()
     17        but that doesn't take shadow spread into account, so we'd build a rounded rect with a rect that
     18        accounted for spread, but radii computed without. That could result in unrenderable rounded rects.
     19        Fix by calling style.getRoundedInnerBorderFor() a second time if we have spread; this will fix
     20        up the radii for spread.
     21
     22
     23        Tests: fast/box-shadow/inset-box-shadow-fractional-radius.html
     24               fast/box-shadow/inset-shadow-split-inline.html
     25               fast/box-shadow/inset-spread-box-shadow-split-inline.html
     26
     27        * platform/graphics/LayoutRect.h:
     28        (WebCore::LayoutRect::shiftXEdgeBy):
     29        (WebCore::LayoutRect::shiftYEdgeBy):
     30        * rendering/RenderBoxModelObject.cpp:
     31        (WebCore::RenderBoxModelObject::paintBoxShadow):
     32
    1332020-05-07  Don Olmstead  <don.olmstead@sony.com>
    234
  • trunk/Source/WebCore/platform/graphics/FloatRect.h

    r259843 r261334  
    125125        setWidth(std::max(0.0f, width() - delta));
    126126    }
     127
    127128    void shiftMaxXEdgeTo(float edge)
    128129    {
     
    130131        setWidth(std::max(0.0f, width() + delta));
    131132    }
     133
    132134    void shiftYEdgeTo(float edge)
    133135    {
     
    136138        setHeight(std::max(0.0f, height() - delta));
    137139    }
     140
    138141    void shiftMaxYEdgeTo(float edge)
    139142    {
    140143        float delta = edge - maxY();
    141144        setHeight(std::max(0.0f, height() + delta));
     145    }
     146
     147    void shiftXEdgeBy(float delta)
     148    {
     149        move(delta, 0);
     150        setWidth(std::max(0.0f, width() - delta));
     151    }
     152
     153    void shiftYEdgeBy(float delta)
     154    {
     155        move(0, delta);
     156        setHeight(std::max(0.0f, height() - delta));
    142157    }
    143158
  • trunk/Source/WebCore/platform/graphics/IntRect.h

    r261023 r261334  
    124124        setWidth(std::max(0, width() - delta));
    125125    }
     126
    126127    void shiftMaxXEdgeTo(int edge)
    127128    {
     
    129130        setWidth(std::max(0, width() + delta));
    130131    }
     132
    131133    void shiftYEdgeTo(int edge)
    132134    {
     
    135137        setHeight(std::max(0, height() - delta));
    136138    }
     139
    137140    void shiftMaxYEdgeTo(int edge)
    138141    {
    139142        int delta = edge - maxY();
    140143        setHeight(std::max(0, height() + delta));
     144    }
     145
     146    void shiftXEdgeBy(int delta)
     147    {
     148        move(delta, 0);
     149        setWidth(std::max(0, width() - delta));
     150    }
     151
     152    void shiftYEdgeBy(int delta)
     153    {
     154        move(0, delta);
     155        setHeight(std::max(0, height() - delta));
    141156    }
    142157
  • trunk/Source/WebCore/platform/graphics/LayoutRect.h

    r254555 r261334  
    130130        setWidth(std::max<LayoutUnit>(0, width() - delta));
    131131    }
     132
    132133    void shiftMaxXEdgeTo(LayoutUnit edge)
    133134    {
     
    135136        setWidth(std::max<LayoutUnit>(0, width() + delta));
    136137    }
     138
    137139    void shiftYEdgeTo(LayoutUnit edge)
    138140    {
     
    141143        setHeight(std::max<LayoutUnit>(0, height() - delta));
    142144    }
     145
    143146    void shiftMaxYEdgeTo(LayoutUnit edge)
    144147    {
    145148        LayoutUnit delta = edge - maxY();
    146149        setHeight(std::max<LayoutUnit>(0, height() + delta));
     150    }
     151
     152    void shiftXEdgeBy(LayoutUnit delta)
     153    {
     154        move(delta, 0);
     155        setWidth(std::max<LayoutUnit>(0, width() - delta));
     156    }
     157
     158    void shiftYEdgeBy(LayoutUnit delta)
     159    {
     160        move(0, delta);
     161        setHeight(std::max<LayoutUnit>(0, height() - delta));
    147162    }
    148163
  • trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp

    r260851 r261334  
    23592359        return;
    23602360
    2361     RoundedRect border = (shadowStyle == ShadowStyle::Inset) ? style.getRoundedInnerBorderFor(paintRect, includeLogicalLeftEdge, includeLogicalRightEdge)
     2361    RoundedRect borderRect = (shadowStyle == ShadowStyle::Inset) ? style.getRoundedInnerBorderFor(paintRect, includeLogicalLeftEdge, includeLogicalRightEdge)
    23622362        : style.getRoundedBorderFor(paintRect, includeLogicalLeftEdge, includeLogicalRightEdge);
    23632363
    23642364    bool hasBorderRadius = style.hasBorderRadius();
    2365     bool isHorizontal = style.isHorizontalWritingMode();
    23662365    float deviceScaleFactor = document().deviceScaleFactor();
    23672366
     
    23822381
    23832382        if (shadow->style() == ShadowStyle::Normal) {
    2384             RoundedRect fillRect = border;
     2383            auto fillRect = borderRect;
    23852384            fillRect.inflate(shadowSpread);
    23862385            if (fillRect.isEmpty())
    23872386                continue;
    23882387
    2389             LayoutRect shadowRect = border.rect();
     2388            auto shadowRect = borderRect.rect();
    23902389            shadowRect.inflate(shadowPaintingExtent + shadowSpread);
    23912390            shadowRect.move(shadowOffset);
    2392             FloatRect pixelSnappedShadowRect = snapRectToDevicePixels(shadowRect, deviceScaleFactor);
     2391            auto pixelSnappedShadowRect = snapRectToDevicePixels(shadowRect, deviceScaleFactor);
    23932392
    23942393            GraphicsContextStateSaver stateSaver(context);
     
    24022401            fillRect.move(extraOffset);
    24032402
    2404             FloatRoundedRect pixelSnappedRectToClipOut = border.pixelSnappedRoundedRectForPainting(deviceScaleFactor);
    2405             FloatRoundedRect pixelSnappedFillRect = fillRect.pixelSnappedRoundedRectForPainting(deviceScaleFactor);
     2403            auto pixelSnappedRectToClipOut = borderRect.pixelSnappedRoundedRectForPainting(deviceScaleFactor);
     2404            auto pixelSnappedFillRect = fillRect.pixelSnappedRoundedRectForPainting(deviceScaleFactor);
    24062405           
    24072406            LayoutPoint shadowRectOrigin = fillRect.rect().location() + shadowOffset;
     
    24242423                    context.clipOutRoundedRect(pixelSnappedRectToClipOut);
    24252424
    2426                 RoundedRect influenceRect(LayoutRect(pixelSnappedShadowRect), border.radii());
     2425                RoundedRect influenceRect(LayoutRect(pixelSnappedShadowRect), borderRect.radii());
    24272426                influenceRect.expandRadii(2 * shadowPaintingExtent + shadowSpread);
    24282427
     
    24552454        } else {
    24562455            // Inset shadow.
    2457             LayoutRect holeRect = border.rect();
     2456            auto holeRect = borderRect.rect();
    24582457            holeRect.inflate(-shadowSpread);
    2459             FloatRect pixelSnappedHoleRect = snapRectToDevicePixels(holeRect, deviceScaleFactor);
    2460             FloatRoundedRect pixelSnappedBorderRect = border.pixelSnappedRoundedRectForPainting(deviceScaleFactor);
    2461 
     2458
     2459            bool isHorizontal = style.isHorizontalWritingMode();
     2460            if (!includeLogicalLeftEdge) {
     2461                if (isHorizontal)
     2462                    holeRect.shiftXEdgeBy(-(std::max<LayoutUnit>(shadowOffset.width(), 0) + shadowPaintingExtent + shadowSpread));
     2463                else
     2464                    holeRect.shiftYEdgeBy(-(std::max<LayoutUnit>(shadowOffset.height(), 0) + shadowPaintingExtent + shadowSpread));
     2465            }
     2466
     2467            if (!includeLogicalRightEdge) {
     2468                if (isHorizontal)
     2469                    holeRect.setWidth(holeRect.width() - std::min<LayoutUnit>(shadowOffset.width(), 0) + shadowPaintingExtent + shadowSpread);
     2470                else
     2471                    holeRect.setHeight(holeRect.height() - std::min<LayoutUnit>(shadowOffset.height(), 0) + shadowPaintingExtent + shadowSpread);
     2472            }
     2473
     2474            auto roundedHoleRect = RoundedRect { holeRect, borderRect.radii() };
     2475            if (shadowSpread && roundedHoleRect.isRounded()) {
     2476                auto rounedRectCorrectingForSpread = [&]() {
     2477                    bool horizontal = style.isHorizontalWritingMode();
     2478                    LayoutUnit leftWidth { (!horizontal || includeLogicalLeftEdge) ? style.borderLeftWidth() + shadowSpread : 0 };
     2479                    LayoutUnit rightWidth { (!horizontal || includeLogicalRightEdge) ? style.borderRightWidth() + shadowSpread : 0 };
     2480                    LayoutUnit topWidth { (horizontal || includeLogicalLeftEdge) ? style.borderTopWidth() + shadowSpread : 0 };
     2481                    LayoutUnit bottomWidth { (horizontal || includeLogicalRightEdge) ? style.borderBottomWidth() + shadowSpread : 0 };
     2482
     2483                    return style.getRoundedInnerBorderFor(paintRect, topWidth, bottomWidth, leftWidth, rightWidth, includeLogicalLeftEdge, includeLogicalRightEdge);
     2484                }();
     2485                roundedHoleRect.setRadii(rounedRectCorrectingForSpread.radii());
     2486            }
     2487
     2488            auto pixelSnappedHoleRect = roundedHoleRect.pixelSnappedRoundedRectForPainting(deviceScaleFactor);
     2489            auto pixelSnappedBorderRect = borderRect.pixelSnappedRoundedRectForPainting(deviceScaleFactor);
    24622490            if (pixelSnappedHoleRect.isEmpty()) {
    24632491                if (hasBorderRadius)
     
    24682496            }
    24692497
    2470             if (!includeLogicalLeftEdge) {
    2471                 if (isHorizontal) {
    2472                     holeRect.move(-std::max<LayoutUnit>(shadowOffset.width(), 0) - shadowPaintingExtent, 0);
    2473                     holeRect.setWidth(holeRect.width() + std::max<LayoutUnit>(shadowOffset.width(), 0) + shadowPaintingExtent);
    2474                 } else {
    2475                     holeRect.move(0, -std::max<LayoutUnit>(shadowOffset.height(), 0) - shadowPaintingExtent);
    2476                     holeRect.setHeight(holeRect.height() + std::max<LayoutUnit>(shadowOffset.height(), 0) + shadowPaintingExtent);
    2477                 }
    2478             }
    2479            
    2480             if (!includeLogicalRightEdge) {
    2481                 if (isHorizontal)
    2482                     holeRect.setWidth(holeRect.width() - std::min<LayoutUnit>(shadowOffset.width(), 0) + shadowPaintingExtent);
    2483                 else
    2484                     holeRect.setHeight(holeRect.height() - std::min<LayoutUnit>(shadowOffset.height(), 0) + shadowPaintingExtent);
    2485             }
    2486 
    2487             if (!includeLogicalLeftEdge || !includeLogicalRightEdge)
    2488                 pixelSnappedHoleRect = snapRectToDevicePixels(holeRect, deviceScaleFactor);
    2489 
    24902498            Color fillColor(shadowColor.red(), shadowColor.green(), shadowColor.blue(), 255);
    2491 
    2492             LayoutRect shadowCastingRect = areaCastingShadowInHole(border.rect(), shadowPaintingExtent, shadowSpread, shadowOffset);
    2493             RoundedRect roundedHoleRect(holeRect, border.radii());
    2494 
    2495             FloatRect pixelSnappedOuterRect = snapRectToDevicePixels(shadowCastingRect, deviceScaleFactor);
    2496             FloatRoundedRect pixelSnappedRoundedHole = roundedHoleRect.pixelSnappedRoundedRectForPainting(deviceScaleFactor);
     2499            auto shadowCastingRect = areaCastingShadowInHole(borderRect.rect(), shadowPaintingExtent, shadowSpread, shadowOffset);
     2500            auto pixelSnappedOuterRect = snapRectToDevicePixels(shadowCastingRect, deviceScaleFactor);
    24972501
    24982502            GraphicsContextStateSaver stateSaver(context);
    2499             if (hasBorderRadius) {
     2503            if (hasBorderRadius)
    25002504                context.clipRoundedRect(pixelSnappedBorderRect);
    2501                 pixelSnappedRoundedHole.shrinkRadii(shadowSpread);
    2502             } else
     2505            else
    25032506                context.clip(pixelSnappedBorderRect.rect());
    25042507
     
    25142517                context.setShadow(shadowOffset, shadowRadius, shadowColor);
    25152518
    2516             context.fillRectWithRoundedHole(pixelSnappedOuterRect, pixelSnappedRoundedHole, fillColor);
     2519            context.fillRectWithRoundedHole(pixelSnappedOuterRect, pixelSnappedHoleRect, fillColor);
    25172520        }
    25182521    }
Note: See TracChangeset for help on using the changeset viewer.