- Revision
- 223388
- Author
- [email protected]
- Date
- 2017-10-16 04:59:45 -0700 (Mon, 16 Oct 2017)
Log Message
Merge r222245 - 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):
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog (223387 => 223388)
--- releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog 2017-10-16 11:22:26 UTC (rev 223387)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog 2017-10-16 11:59:45 UTC (rev 223388)
@@ -1,3 +1,42 @@
+2017-09-19 Wenson Hsieh <[email protected]>
+
+ REGRESSION (r215613): Incorrect corners clipping with border-radius
+ https://bugs.webkit.org/show_bug.cgi?id=176498
+ <rdar://problem/34112607>
+
+ Reviewed by Tim Horton.
+
+ 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:
+
2017-09-19 Zalan Bujtas <[email protected]>
AXObjectCache::performDeferredCacheUpdate is called recursively through FrameView::layout.
Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/platform/graphics/GeometryUtilities.cpp (223387 => 223388)
--- releases/WebKitGTK/webkit-2.18/Source/WebCore/platform/graphics/GeometryUtilities.cpp 2017-10-16 11:22:26 UTC (rev 223387)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/platform/graphics/GeometryUtilities.cpp 2017-10-16 11:59:45 UTC (rev 223388)
@@ -154,16 +154,29 @@
bool ellipseContainsPoint(const FloatPoint& center, const FloatSize& radii, const FloatPoint& point)
{
+ if (radii.width() <= 0 || radii.height() <= 0)
+ return false;
+
+ // First, offset the query point so that the ellipse is effectively centered at the origin.
FloatPoint transformedPoint(point);
transformedPoint.move(-center.x(), -center.y());
+
+ // If the point lies outside of the bounding box determined by the radii of the ellipse, it can't possibly
+ // be contained within the ellipse, so bail early.
+ if (transformedPoint.x() < -radii.width() || transformedPoint.x() > radii.width() || transformedPoint.y() < -radii.height() || transformedPoint.y() > radii.height())
+ return false;
+
+ // Let (x, y) represent the translated point, and let (Rx, Ry) represent the radii of an ellipse centered at the origin.
+ // (x, y) is contained within the ellipse if, after scaling the ellipse to be a unit circle, the identically scaled
+ // point lies within that unit circle. In other words, the squared distance (x/Rx)^2 + (y/Ry)^2 of the transformed point
+ // to the origin is no greater than 1. This is equivalent to checking whether or not the point (xRy, yRx) lies within a
+ // circle of radius RxRy.
transformedPoint.scale(radii.height(), radii.width());
- float radius = radii.width() * radii.height();
+ auto transformedRadius = radii.width() * radii.height();
- if (transformedPoint.x() > radius || transformedPoint.y() > radius)
- return false;
- if (transformedPoint.x() + transformedPoint.y() <= radius)
- return true;
- return (transformedPoint.lengthSquared() <= radius * radius);
+ // We can bail early if |xRy| + |yRx| <= RxRy to avoid additional multiplications, since that means the Manhattan distance
+ // of the transformed point is less than the radius, so the point must lie within the transformed circle.
+ return std::abs(transformedPoint.x()) + std::abs(transformedPoint.y()) <= transformedRadius || transformedPoint.lengthSquared() <= transformedRadius * transformedRadius;
}
}
Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/platform/graphics/RoundedRect.cpp (223387 => 223388)
--- releases/WebKitGTK/webkit-2.18/Source/WebCore/platform/graphics/RoundedRect.cpp 2017-10-16 11:22:26 UTC (rev 223387)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/platform/graphics/RoundedRect.cpp 2017-10-16 11:59:45 UTC (rev 223388)
@@ -239,7 +239,7 @@
bool RoundedRect::contains(const LayoutRect& otherRect) const
{
- if (!rect().contains(otherRect))
+ if (!rect().contains(otherRect) || !isRenderable())
return false;
const LayoutSize& topLeft = m_radii.topLeft();
@@ -246,7 +246,7 @@
if (!topLeft.isEmpty()) {
FloatPoint center = { m_rect.x() + topLeft.width(), m_rect.y() + topLeft.height() };
if (otherRect.x() <= center.x() && otherRect.y() <= center.y()) {
- if (!ellipseContainsPoint(center, topLeft, otherRect.location()))
+ if (!ellipseContainsPoint(center, topLeft, otherRect.minXMinYCorner()))
return false;
}
}
@@ -255,7 +255,7 @@
if (!topRight.isEmpty()) {
FloatPoint center = { m_rect.maxX() - topRight.width(), m_rect.y() + topRight.height() };
if (otherRect.maxX() >= center.x() && otherRect.y() <= center.y()) {
- if (!ellipseContainsPoint(center, topRight, otherRect.location()))
+ if (!ellipseContainsPoint(center, topRight, otherRect.maxXMinYCorner()))
return false;
}
}
@@ -263,8 +263,8 @@
const LayoutSize& bottomLeft = m_radii.bottomLeft();
if (!bottomLeft.isEmpty()) {
FloatPoint center = { m_rect.x() + bottomLeft.width(), m_rect.maxY() - bottomLeft.height() };
- if (otherRect.maxX() >= center.x() && otherRect.maxY() >= center.y()) {
- if (!ellipseContainsPoint(center, bottomLeft, otherRect.location()))
+ if (otherRect.x() <= center.x() && otherRect.maxY() >= center.y()) {
+ if (!ellipseContainsPoint(center, bottomLeft, otherRect.minXMaxYCorner()))
return false;
}
}
@@ -272,8 +272,8 @@
const LayoutSize& bottomRight = m_radii.bottomRight();
if (!bottomRight.isEmpty()) {
FloatPoint center = { m_rect.maxX() - bottomRight.width(), m_rect.maxY() - bottomRight.height() };
- if (otherRect.x() <= center.x() && otherRect.maxY() >= center.y()) {
- if (!ellipseContainsPoint(center, bottomRight, otherRect.location()))
+ if (otherRect.maxX() >= center.x() && otherRect.maxY() >= center.y()) {
+ if (!ellipseContainsPoint(center, bottomRight, otherRect.maxXMaxYCorner()))
return false;
}
}
Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/platform/graphics/RoundedRect.h (223387 => 223388)
--- releases/WebKitGTK/webkit-2.18/Source/WebCore/platform/graphics/RoundedRect.h 2017-10-16 11:22:26 UTC (rev 223387)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/platform/graphics/RoundedRect.h 2017-10-16 11:59:45 UTC (rev 223388)
@@ -80,7 +80,7 @@
explicit RoundedRect(const LayoutRect&, const Radii& = Radii());
RoundedRect(const LayoutUnit&, const LayoutUnit&, const LayoutUnit& width, const LayoutUnit& height);
- RoundedRect(const LayoutRect&, const LayoutSize& topLeft, const LayoutSize& topRight, const LayoutSize& bottomLeft, const LayoutSize& bottomRight);
+ WEBCORE_EXPORT RoundedRect(const LayoutRect&, const LayoutSize& topLeft, const LayoutSize& topRight, const LayoutSize& bottomLeft, const LayoutSize& bottomRight);
const LayoutRect& rect() const { return m_rect; }
const Radii& radii() const { return m_radii; }
@@ -106,7 +106,7 @@
// Tests whether the quad intersects any part of this rounded rectangle.
// This only works for convex quads.
bool intersectsQuad(const FloatQuad&) const;
- bool contains(const LayoutRect&) const;
+ WEBCORE_EXPORT bool contains(const LayoutRect&) const;
FloatRoundedRect pixelSnappedRoundedRectForPainting(float deviceScaleFactor) const;
Modified: releases/WebKitGTK/webkit-2.18/Tools/ChangeLog (223387 => 223388)
--- releases/WebKitGTK/webkit-2.18/Tools/ChangeLog 2017-10-16 11:22:26 UTC (rev 223387)
+++ releases/WebKitGTK/webkit-2.18/Tools/ChangeLog 2017-10-16 11:59:45 UTC (rev 223388)
@@ -1,3 +1,18 @@
+2017-09-19 Wenson Hsieh <[email protected]>
+
+ REGRESSION (r215613): Incorrect corners clipping with border-radius
+ https://bugs.webkit.org/show_bug.cgi?id=176498
+ <rdar://problem/34112607>
+
+ Reviewed by Tim Horton.
+
+ Add WebCore API tests for RoundedRect::contains().
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebCore/RoundedRectTests.cpp: Added.
+ (TestWebKitAPI::layoutRect):
+ (TestWebKitAPI::TEST):
+
2017-09-15 Brent Fulgham <[email protected]>
Provide mechanism to immediately end tests
Added: releases/WebKitGTK/webkit-2.18/Tools/TestWebKitAPI/Tests/WebCore/RoundedRectTests.cpp (0 => 223388)
--- releases/WebKitGTK/webkit-2.18/Tools/TestWebKitAPI/Tests/WebCore/RoundedRectTests.cpp (rev 0)
+++ releases/WebKitGTK/webkit-2.18/Tools/TestWebKitAPI/Tests/WebCore/RoundedRectTests.cpp 2017-10-16 11:59:45 UTC (rev 223388)
@@ -0,0 +1,73 @@
+/*
+ * Copyright (C) 2017 Apple 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
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+
+#include <WebCore/RoundedRect.h>
+
+using namespace WebCore;
+
+namespace TestWebKitAPI {
+
+static LayoutRect layoutRect(float x, float y, float width, float height)
+{
+ return { LayoutPoint(x, y), LayoutSize(width, height) };
+}
+
+TEST(WebCore, RoundedRectContainsRect)
+{
+ auto roundedRectBounds = layoutRect(-10, -40, 40, 80);
+ RoundedRect roundedRect(roundedRectBounds, { 10, 10 }, { 10, 20 }, { 0, 0 }, { 20, 40 });
+
+ // Cases where the other rect is outside of the rounded rect's bounds.
+ EXPECT_FALSE(roundedRect.contains(layoutRect(-40, -10, 10, 10)));
+ EXPECT_FALSE(roundedRect.contains(layoutRect(-40, 10, 100, 10)));
+ EXPECT_FALSE(roundedRect.contains(layoutRect(20, 20, 60, 60)));
+ EXPECT_FALSE(roundedRect.contains(layoutRect(10, -80, 10, 200)));
+ EXPECT_FALSE(roundedRect.contains(layoutRect(-80, -80, 160, 160)));
+
+ // Cases where the other rect doesn't intersect with any corners.
+ EXPECT_TRUE(roundedRect.contains(layoutRect(-5, -5, 10, 10)));
+ EXPECT_TRUE(roundedRect.contains(layoutRect(20, -10, 5, 5)));
+ EXPECT_TRUE(roundedRect.contains(layoutRect(-5, 5, 5, 5)));
+ EXPECT_TRUE(roundedRect.contains(layoutRect(0, 0, 10, 10)));
+ EXPECT_TRUE(roundedRect.contains(layoutRect(0, -10, 10, 10)));
+
+ // Cases where the other rect intersects at one of the rounded rect's corners.
+ EXPECT_FALSE(roundedRect.contains(layoutRect(-9, -39, 20, 40)));
+ EXPECT_TRUE(roundedRect.contains(layoutRect(-1, -31, 20, 40)));
+ EXPECT_FALSE(roundedRect.contains(layoutRect(20, -30, 10, 10)));
+ EXPECT_TRUE(roundedRect.contains(layoutRect(10, -30, 11, 11)));
+ EXPECT_FALSE(roundedRect.contains(layoutRect(15, 25, 14, 14)));
+ EXPECT_TRUE(roundedRect.contains(layoutRect(15, 25, 3, 3)));
+ EXPECT_TRUE(roundedRect.contains(layoutRect(-10, 20, 10, 20)));
+
+ // The other rect is equal to the rounded rect's bounds.
+ EXPECT_FALSE(roundedRect.contains(roundedRectBounds));
+ RoundedRect nonRoundedRect(roundedRectBounds, { 0, 0 }, { 0, 0 }, { 0, 0 }, { 0, 0 });
+ EXPECT_TRUE(nonRoundedRect.contains(roundedRectBounds));
+}
+
+}
