Changeset 222245 in webkit


Ignore:
Timestamp:
Sep 19, 2017 7:20:28 PM (7 years ago)
Author:
Wenson Hsieh
Message:

REGRESSION (r215613): Incorrect corners clipping with border-radius
​https://bugs.webkit.org/show_bug.cgi?id=176498
<rdar://problem/34112607>

Reviewed by Tim Horton.

Source/WebCore:

​http://trac.webkit.org/r215613 introduced an optimization to bail out of repainting borders if the invalidated
rect to paint is fully contained within the inner rounded rect of the border. However, due to issues with
coordinate and intersection math in RoundedRect::contains() and ellipseContainsPoint(), this causes
RenderBoxModelObject::paintBorder to return early even in circumstances where the border requires a repaint.
This patch fixes the contains() helper in RoundedRect and adds a new API test suite for RoundedRect that covers
these changes.

Test: WebCore.RoundedRectContainsRect

  • platform/graphics/GeometryUtilities.cpp:

(WebCore::ellipseContainsPoint):

This function attempts to return early if the Manhattan distance of the transformed point is less than the
radius of the circle that results from applying the same transformation to the ellipse. However, this bails and
returns true if x + y <= R, but this means that if x and y are negative, we'll always end up returning true.
We fix this by adding the absolute values instead, so the check becomes: |x| + |y| <= R.

  • platform/graphics/RoundedRect.cpp:

(WebCore::RoundedRect::contains const):

Before this patch, otherRect's upper left location was being used to hit-test against the ellipses formed from
each of the 4 corners of the rounded rect. Instead, this should use (x, y), (maxX, y), (x, maxY), (maxX, maxY)
for the top left, top right, bottom left, and bottom right corners, respectively.

Additionally, the checks for the bottom left and bottom right to determine whether the rect corner should be
checked for intersection against the ellipse's corner are incorrect. In the bottom left corner, the check for
otherRect.maxX() >= center.x() should instead be otherRect.x() <= center.x(), and the check for
otherRect.x() <= center.x() should instead be otherRect.maxX() >= center.x().

  • platform/graphics/RoundedRect.h:

Tools:

Add WebCore API tests for RoundedRect::contains().

  • TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
  • TestWebKitAPI/Tests/WebCore/RoundedRectTests.cpp: Added.

(TestWebKitAPI::layoutRect):
(TestWebKitAPI::TEST):

Location:
trunk
Files:
1 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r222239 r222245  
     12017-09-19  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        REGRESSION (r215613): Incorrect corners clipping with border-radius
     4        https://bugs.webkit.org/show_bug.cgi?id=176498
     5        <rdar://problem/34112607>
     6
     7        Reviewed by Tim Horton.
     8
     9        http://trac.webkit.org/r215613 introduced an optimization to bail out of repainting borders if the invalidated
     10        rect to paint is fully contained within the inner rounded rect of the border. However, due to issues with
     11        coordinate and intersection math in RoundedRect::contains() and ellipseContainsPoint(), this causes
     12        RenderBoxModelObject::paintBorder to return early even in circumstances where the border requires a repaint.
     13        This patch fixes the contains() helper in RoundedRect and adds a new API test suite for RoundedRect that covers
     14        these changes.
     15
     16        Test: WebCore.RoundedRectContainsRect
     17
     18        * platform/graphics/GeometryUtilities.cpp:
     19        (WebCore::ellipseContainsPoint):
     20
     21        This function attempts to return early if the Manhattan distance of the transformed point is less than the
     22        radius of the circle that results from applying the same transformation to the ellipse. However, this bails and
     23        returns true if `x + y <= R`, but this means that if x and y are negative, we'll always end up returning true.
     24        We fix this by adding the absolute values instead, so the check becomes: |x| + |y| <= R.
     25
     26        * platform/graphics/RoundedRect.cpp:
     27        (WebCore::RoundedRect::contains const):
     28
     29        Before this patch, otherRect's upper left location was being used to hit-test against the ellipses formed from
     30        each of the 4 corners of the rounded rect. Instead, this should use (x, y), (maxX, y), (x, maxY), (maxX, maxY)
     31        for the top left, top right, bottom left, and bottom right corners, respectively.
     32
     33        Additionally, the checks for the bottom left and bottom right to determine whether the rect corner should be
     34        checked for intersection against the ellipse's corner are incorrect. In the bottom left corner, the check for
     35        `otherRect.maxX() >= center.x()` should instead be `otherRect.x() <= center.x()`, and the check for
     36        `otherRect.x() <= center.x()` should instead be `otherRect.maxX() >= center.x()`.
     37
     38        * platform/graphics/RoundedRect.h:
     39
    1402017-09-19  Alexey Proskuryakov  <ap@apple.com>
    241

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

    r222113 r222245  
    156156bool ellipseContainsPoint(const FloatPoint& center, const FloatSize& radii, const FloatPoint& point)
    157157{
     158    if (radii.width() <= 0 || radii.height() <= 0)
     159        return false;
     160
     161    // First, offset the query point so that the ellipse is effectively centered at the origin.
    158162    FloatPoint transformedPoint(point);
    159163    transformedPoint.move(-center.x(), -center.y());
     164
     165    // If the point lies outside of the bounding box determined by the radii of the ellipse, it can't possibly
     166    // be contained within the ellipse, so bail early.
     167    if (transformedPoint.x() < -radii.width() || transformedPoint.x() > radii.width() || transformedPoint.y() < -radii.height() || transformedPoint.y() > radii.height())
     168        return false;
     169
     170    // Let (x, y) represent the translated point, and let (Rx, Ry) represent the radii of an ellipse centered at the origin.
     171    // (x, y) is contained within the ellipse if, after scaling the ellipse to be a unit circle, the identically scaled
     172    // point lies within that unit circle. In other words, the squared distance (x/Rx)^2 + (y/Ry)^2 of the transformed point
     173    // to the origin is no greater than 1. This is equivalent to checking whether or not the point (xRy, yRx) lies within a
     174    // circle of radius RxRy.
    160175    transformedPoint.scale(radii.height(), radii.width());
    161     float radius = radii.width() * radii.height();
     176    auto transformedRadius = radii.width() * radii.height();
    162177
    163     if (transformedPoint.x() > radius || transformedPoint.y() > radius)
    164         return false;
    165     if (transformedPoint.x() + transformedPoint.y() <= radius)
    166         return true;
    167     return (transformedPoint.lengthSquared() <= radius * radius);
     178    // We can bail early if |xRy| + |yRx| <= RxRy to avoid additional multiplications, since that means the Manhattan distance
     179    // of the transformed point is less than the radius, so the point must lie within the transformed circle.
     180    return std::abs(transformedPoint.x()) + std::abs(transformedPoint.y()) <= transformedRadius || transformedPoint.lengthSquared() <= transformedRadius * transformedRadius;
    168181}
    169182

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

    r215613 r222245  
    240240bool RoundedRect::contains(const LayoutRect& otherRect) const
    241241{
    242     if (!rect().contains(otherRect))
     242    if (!rect().contains(otherRect) || !isRenderable())
    243243        return false;
    244244
    … …  
    247247        FloatPoint center = { m_rect.x() + topLeft.width(), m_rect.y() + topLeft.height() };
    248248        if (otherRect.x() <= center.x() && otherRect.y() <= center.y()) {
    249             if (!ellipseContainsPoint(center, topLeft, otherRect.location()))
     249            if (!ellipseContainsPoint(center, topLeft, otherRect.minXMinYCorner()))
    250250                return false;
    251251        }
    … …  
    256256        FloatPoint center = { m_rect.maxX() - topRight.width(), m_rect.y() + topRight.height() };
    257257        if (otherRect.maxX() >= center.x() && otherRect.y() <= center.y()) {
    258             if (!ellipseContainsPoint(center, topRight, otherRect.location()))
     258            if (!ellipseContainsPoint(center, topRight, otherRect.maxXMinYCorner()))
    259259                return false;
    260260        }
    … …  
    264264    if (!bottomLeft.isEmpty()) {
    265265        FloatPoint center = { m_rect.x() + bottomLeft.width(), m_rect.maxY() - bottomLeft.height() };
    266         if (otherRect.maxX() >= center.x() && otherRect.maxY() >= center.y()) {
    267             if (!ellipseContainsPoint(center, bottomLeft, otherRect.location()))
     266        if (otherRect.x() <= center.x() && otherRect.maxY() >= center.y()) {
     267            if (!ellipseContainsPoint(center, bottomLeft, otherRect.minXMaxYCorner()))
    268268                return false;
    269269        }
    … …  
    273273    if (!bottomRight.isEmpty()) {
    274274        FloatPoint center = { m_rect.maxX() - bottomRight.width(), m_rect.maxY() - bottomRight.height() };
    275         if (otherRect.x() <= center.x() && otherRect.maxY() >= center.y()) {
    276             if (!ellipseContainsPoint(center, bottomRight, otherRect.location()))
     275        if (otherRect.maxX() >= center.x() && otherRect.maxY() >= center.y()) {
     276            if (!ellipseContainsPoint(center, bottomRight, otherRect.maxXMaxYCorner()))
    277277                return false;
    278278        }

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

    r215613 r222245  
    8181    explicit RoundedRect(const LayoutRect&, const Radii& = Radii());
    8282    RoundedRect(const LayoutUnit&, const LayoutUnit&, const LayoutUnit& width, const LayoutUnit& height);
    83     RoundedRect(const LayoutRect&, const LayoutSize& topLeft, const LayoutSize& topRight, const LayoutSize& bottomLeft, const LayoutSize& bottomRight);
     83    WEBCORE_EXPORT RoundedRect(const LayoutRect&, const LayoutSize& topLeft, const LayoutSize& topRight, const LayoutSize& bottomLeft, const LayoutSize& bottomRight);
    8484
    8585    const LayoutRect& rect() const { return m_rect; }
    … …  
    107107    // This only works for convex quads.
    108108    bool intersectsQuad(const FloatQuad&) const;
    109     bool contains(const LayoutRect&) const;
     109    WEBCORE_EXPORT bool contains(const LayoutRect&) const;
    110110
    111111    FloatRoundedRect pixelSnappedRoundedRectForPainting(float deviceScaleFactor) const;

  • trunk/Tools/ChangeLog

    r222238 r222245  
     12017-09-19  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        REGRESSION (r215613): Incorrect corners clipping with border-radius
     4        https://bugs.webkit.org/show_bug.cgi?id=176498
     5        <rdar://problem/34112607>
     6
     7        Reviewed by Tim Horton.
     8
     9        Add WebCore API tests for RoundedRect::contains().
     10
     11        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     12        * TestWebKitAPI/Tests/WebCore/RoundedRectTests.cpp: Added.
     13        (TestWebKitAPI::layoutRect):
     14        (TestWebKitAPI::TEST):
     15
    1162017-09-19  Youenn Fablet  <youenn@apple.com>
    217

  • trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

    r222013 r222245  
    663663                F407FE391F1D0DFC0017CF25 /* enormous.svg in Copy Resources */ = {isa = PBXBuildFile; fileRef = F407FE381F1D0DE60017CF25 /* enormous.svg */; };
    664664                F415086D1DA040C50044BE9B /* play-audio-on-click.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F415086C1DA040C10044BE9B /* play-audio-on-click.html */; };
     665                F418BE151F71B7DC001970E6 /* RoundedRectTests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = F418BE141F71B7DC001970E6 /* RoundedRectTests.cpp */; };
    665666                F4194AD11F5A320100ADD83F /* drop-targets.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4194AD01F5A2EA500ADD83F /* drop-targets.html */; };
    666667                F41AB99F1EF4696B0083FA08 /* autofocus-contenteditable.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F41AB9981EF4692C0083FA08 /* autofocus-contenteditable.html */; };
    … …  
    17021703                F407FE381F1D0DE60017CF25 /* enormous.svg */ = {isa = PBXFileReference; lastKnownFileType = text; path = enormous.svg; sourceTree = "<group>"; };
    17031704                F415086C1DA040C10044BE9B /* play-audio-on-click.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "play-audio-on-click.html"; sourceTree = "<group>"; };
     1705                F418BE141F71B7DC001970E6 /* RoundedRectTests.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = RoundedRectTests.cpp; sourceTree = "<group>"; };
    17041706                F4194AD01F5A2EA500ADD83F /* drop-targets.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "drop-targets.html"; sourceTree = "<group>"; };
    17051707                F41AB9931EF4692C0083FA08 /* image-and-textarea.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "image-and-textarea.html"; sourceTree = "<group>"; };
    … …  
    20862088                                A5B149DD1F5A19DC00C6DAFF /* MIMETypeRegistry.cpp */,
    20872089                                CD225C071C45A69200140761 /* ParsedContentRange.cpp */,
     2090                                F418BE141F71B7DC001970E6 /* RoundedRectTests.cpp */,
    20882091                                CDCFA7A91E45122F00C2433D /* SampleMap.cpp */,
    20892092                                CE06DF9A1E1851F200E570C9 /* SecurityOrigin.cpp */,
    … …  
    33403343                                835CF9671D25FCD6001A65D4 /* RestoreSessionStateWithoutNavigation.cpp in Sources */,
    33413344                                46E816F81E79E29C00375ADC /* RestoreStateAfterTermination.mm in Sources */,
     3345                                F418BE151F71B7DC001970E6 /* RoundedRectTests.cpp in Sources */,
    33423346                                A180C0FA1EE67DF000468F47 /* RunOpenPanel.mm in Sources */,
    33433347                                CDCFA7AA1E45183200C2433D /* SampleMap.cpp in Sources */,
    … …  
    34743478                        files = (
    34753479                                37E7DD671EA071F3009B396D /* AdditionalReadAccessAllowedURLsPlugin.mm in Sources */,
     3480                                754CEC811F6722F200D0039A /* AutoFillAvailable.mm in Sources */,
    34763481                                374B7A611DF371CF00ACCB6C /* BundleEditingDelegatePlugIn.mm in Sources */,
    34773482                                A13EBBB01B87436F00097110 /* BundleParametersPlugIn.mm in Sources */,
    … …  
    34823487                                A13EBBAB1B87434600097110 /* PlatformUtilitiesCocoa.mm in Sources */,
    34833488                                1A4F81CF1BDFFD53004E672E /* RemoteObjectRegistryPlugIn.mm in Sources */,
    3484                                 754CEC811F6722F200D0039A /* AutoFillAvailable.mm in Sources */,
    34853489                                A12DDC021E837C2400CF6CAE /* RenderedImageWithOptionsPlugIn.mm in Sources */,
    34863490                                7C882E091C80C630006BF731 /* UserContentWorldPlugIn.mm in Sources */,
Note: See TracChangeset for help on using the changeset viewer.