Bug 232650 - Verify borderRect is Renderable
Summary: Verify borderRect is Renderable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-11-02 18:38 PDT by Brandon
Modified: 2021-12-02 13:26 PST (History)
11 users (show)

See Also:

Attachments
Patch (2.12 KB, patch)
2021-11-02 18:45 PDT, Brandon 		simon.fraser: review-
simon.fraser: commit-queue-
Details | Formatted Diff | Diff
Patch (2.21 KB, patch)
2021-11-03 14:08 PDT, Brandon 		simon.fraser: review+
simon.fraser: commit-queue-
Details | Formatted Diff | Diff
Patch (2.18 KB, patch)
2021-11-03 19:39 PDT, Brandon 		simon.fraser: review+
simon.fraser: commit-queue-
Details | Formatted Diff | Diff
Patch (1.31 KB, patch)
2021-11-29 11:57 PST, Brandon 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (3.13 KB, patch)
2021-12-01 14:31 PST, Brandon 		no flags Details | Formatted Diff | Diff
patch (2.79 KB, patch)
2021-12-02 09:25 PST, Brandon 		no flags Details | Formatted Diff | Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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].