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

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].