Bug 232650

Summary: Verify borderRect is Renderable
Product: WebKit Reporter: Brandon <brandonstewart>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: changseok, esprehn+autocc, ews-watchlist, glenn, koivisto, kondapallykalyan, pdr, simon.fraser, webkit-bug-importer, wenson_hsieh, zalan
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
simon.fraser: review-, simon.fraser: commit-queue-
Patch
simon.fraser: review+, simon.fraser: commit-queue-
Patch
simon.fraser: review+, simon.fraser: commit-queue-
Patch
ews-feeder: commit-queue-
patch
none
patch none

Description Brandon 2021-11-02 18:38:19 PDT
Update whether an object is renderable
Comment 1 Brandon 2021-11-02 18:44:19 PDT
<rdar://82980915>
Comment 2 Brandon 2021-11-02 18:45:06 PDT
Created attachment 443157 [details]
Patch
Comment 3 Simon Fraser (smfr) 2021-11-03 10:32:13 PDT
Comment on attachment 443157 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        No new tests needed.

You need some more words here for why this change makes sense. It looks like it was copied from FloatRoundedRect::isRenderable(), so say that.

What's the consequence of the different behaviors of RoundedRect and FloatRoundedRect? Explain here.
Comment 4 Brandon 2021-11-03 14:08:27 PDT
Created attachment 443237 [details]
Patch
Comment 5 Simon Fraser (smfr) 2021-11-03 16:14:16 PDT
Comment on attachment 443237 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Update whether an object is renderable

You can be more specific than "on object". It's about rounded rects.

> Source/WebCore/ChangeLog:8
> +        No new tests needed.

Just remove this line.
Comment 6 Brandon 2021-11-03 19:39:46 PDT
Created attachment 443268 [details]
Patch
Comment 7 Simon Fraser (smfr) 2021-11-08 08:33:49 PST
Comment on attachment 443268 [details]
Patch

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

> Source/WebCore/ChangeLog:5
> +        https://bugs.webkit.org/show_bug.cgi?id=232650
> +

Please put the radar number here too.
Comment 8 Brandon 2021-11-29 11:57:41 PST
Created attachment 445318 [details]
Patch
Comment 9 Simon Fraser (smfr) 2021-11-29 12:12:45 PST
Comment on attachment 445318 [details]
Patch

How do we know in blendMixedTypes that negative values should not be allowed? For example, negative margins, or top/left are perfectly valid.
Comment 10 Brandon 2021-12-01 14:31:51 PST
Created attachment 445616 [details]
patch
Comment 11 zalan 2021-12-02 08:39:55 PST
Comment on attachment 445616 [details]
patch

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

> Source/WebCore/platform/graphics/RoundedRect.cpp:175
> +    return m_radii.topLeft().width()     >= 0 && m_radii.topLeft().height()     >= 0
> +        && m_radii.bottomLeft().width()  >= 0 && m_radii.bottomLeft().height()  >= 0
> +        && m_radii.topRight().width()    >= 0 && m_radii.topRight().height()    >= 0
> +        && m_radii.bottomRight().width() >= 0 && m_radii.bottomRight().height() >= 0
> +        && m_radii.topLeft().width()    + m_radii.topRight().width()     <= m_rect.width()
> +        && m_radii.bottomLeft().width() + m_radii.bottomRight().width()  <= m_rect.width()
> +        && m_radii.topLeft().height()   + m_radii.bottomLeft().height()  <= m_rect.height()
> +        && m_radii.topRight().height()  + m_radii.bottomRight().height() <= m_rect.height();

some weird spacing going on here.
Comment 12 Brandon 2021-12-02 09:25:13 PST
Created attachment 445726 [details]
patch
Comment 13 EWS 2021-12-02 13:26:47 PST
Committed r286449 (244791@main): <https://commits.webkit.org/244791@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 445726 [details].