Bug 176498 - REGRESSION (r215613): Incorrect corners clipping with border-radius
Summary: REGRESSION (r215613): Incorrect corners clipping with border-radius
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: All Unspecified
: P1 Normal
Assignee: Wenson Hsieh
URL: http://jsfiddle.net/5k1bcL3s/3/
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2017-09-07 01:29 PDT by Sergii Ostroverkhov
Modified: 2017-09-19 19:28 PDT (History)
9 users (show)

See Also:


Attachments
Corrupted corners (8.55 KB, image/png)
2017-09-07 01:29 PDT, Sergii Ostroverkhov
no flags Details
Test case (taken from jsfiddle) (962 bytes, text/html)
2017-09-07 21:25 PDT, Wenson Hsieh
no flags Details
Patch (18.57 KB, patch)
2017-09-19 16:48 PDT, Wenson Hsieh
thorton: review+
Details | Formatted Diff | Diff
Patch for landing (18.58 KB, patch)
2017-09-19 18:38 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sergii Ostroverkhov 2017-09-07 01:29:18 PDT
Created attachment 320104 [details]
Corrupted corners

Steps to Reproduce:
1. Open page: https://jsfiddle.net/5k1bcL3s/3/
2. Click on Yes/No buttons
3. Observe the issue with corners (see screenshot enclosed)

This issue occurs on specific widths of the browser window.
Also is reproducible on iPhone with iOS 11.

Please note, it works well on previous versions of Safari and iOS.
Comment 1 Radar WebKit Bug Importer 2017-09-07 17:53:39 PDT
<rdar://problem/34322011>
Comment 2 Simon Fraser (smfr) 2017-09-07 19:13:11 PDT
rdar://problem/34112607
Comment 3 Wenson Hsieh 2017-09-07 21:15:15 PDT
From an SVN bisection, this broke in http://trac.webkit.org/changeset/215613.
Comment 4 Wenson Hsieh 2017-09-07 21:25:21 PDT
Created attachment 320233 [details]
Test case (taken from jsfiddle)
Comment 5 Wenson Hsieh 2017-09-19 10:10:46 PDT
Some logging in RenderBoxModelObject::paintBorder, before the early return if `innerBorder.contains(info.rect)`:

2017-09-19 10:06:18.267116-0700 0xa4a5     Default     0x0                  619    com.apple.WebKit.WebContent: (JavaScriptCore) <WK>: The paint rect is: {{9.0, 784.0}, {160.0, 32.0}}
2017-09-19 10:06:18.267340-0700 0xa4a5     Default     0x0                  619    com.apple.WebKit.WebContent: (JavaScriptCore) <WK>: The inner border rect is: {{9.0, 784.0}, {160.0, 32.0}}
2017-09-19 10:06:18.267486-0700 0xa4a5     Default     0x0                  619    com.apple.WebKit.WebContent: (JavaScriptCore) <WK>: The top left radius is: (6.0, 6.0)
2017-09-19 10:06:18.267621-0700 0xa4a5     Default     0x0                  619    com.apple.WebKit.WebContent: (JavaScriptCore) <WK>: The top right radius is: (6.0, 6.0)
2017-09-19 10:06:18.267756-0700 0xa4a5     Default     0x0                  619    com.apple.WebKit.WebContent: (JavaScriptCore) <WK>: The bottom left radius is: (6.0, 6.0)
2017-09-19 10:06:18.267888-0700 0xa4a5     Default     0x0                  619    com.apple.WebKit.WebContent: (JavaScriptCore) <WK>: The bottom right radius is: (6.0, 6.0)
2017-09-19 10:06:18.268018-0700 0xa4a5     Default     0x0                  619    com.apple.WebKit.WebContent: (JavaScriptCore) <WK>: Does the inner border contain the paint rect? YES

...this doesn't look right to me. If the paint rect is the same as the border rect, but the border has non-zero radii, I don't see how the border rounded rect can fully contain the paint rect. Could there be a bug in the math checking whether a rounded rect contains a non-rounded rect?
Comment 6 Wenson Hsieh 2017-09-19 16:48:42 PDT
Created attachment 321265 [details]
Patch
Comment 7 Tim Horton 2017-09-19 17:17:56 PDT
Comment on attachment 321265 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=321265&action=review

> Source/WebCore/platform/graphics/GeometryUtilities.cpp:178
> +    // We can bail early if |xRy| + |yRx| <= RxRy do avoid additional multiplications, since that means the Manhattan distance

I think that "do" is meant to be a "to"?
Comment 8 Wenson Hsieh 2017-09-19 18:22:39 PDT
Comment on attachment 321265 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=321265&action=review

From EWS results, accessibility/crash-table-recursive-layout.html appears to be failing; this was added very recently, and it doesn't seem related, but I'll double check.

> Source/WebCore/ChangeLog:21
> +        This function attempts to return early if the Manhattan distance if the transformed point is less than the

Whoops, s/if the transformed/of the transformed/

>> Source/WebCore/platform/graphics/GeometryUtilities.cpp:178
>> +    // We can bail early if |xRy| + |yRx| <= RxRy do avoid additional multiplications, since that means the Manhattan distance
> 
> I think that "do" is meant to be a "to"?

Ah, that's correct. Fixed.
Comment 9 Wenson Hsieh 2017-09-19 18:38:02 PDT
Created attachment 321273 [details]
Patch for landing
Comment 10 WebKit Commit Bot 2017-09-19 19:20:32 PDT
Comment on attachment 321273 [details]
Patch for landing

Clearing flags on attachment: 321273

Committed r222245: <http://trac.webkit.org/changeset/222245>