Skip to content

[css-shapes] Calculation of corner radii by margin-box and border-radius #675

Closed

Description

@aethanyc

When implementing shape-outside in Firefox in Bug 1309467, I found the calculation of corner radii by margin-box and border-radius might need more clarification. Quote from the spec:

If the ratio of border-radius/margin is 1 or more, then the margin box corner radius is border-radius + margin. If the ratio of border-radius/margin is less than 1, then the margin box corner radius is border-radius + (margin * (1 + (ratio-1)^3)).

My questions mainly related to the ratio < 1 case.

  1. Suppose ratio < 1 and margin > 0. Why do we need to adjust margin by margin * (1 + (ratio-1)^3) which makes the margin even smaller? It seems border-radius + margin is fine.
  2. Suppose margin < 0, and we allow negative margin to reduce the final corner radii from border-radius. However based on the formula, it makes the final corner radii bigger! For example, suppose border-radius is 10px and margin is -5px, so the ratio is -2. The final corner radii is 10 + (-5) * (1 + (-3)^3) = 140. So it seems the spec needs some clarification when margin < 0. Again simple border-radius + margin seems fine with negative margin. If we do allow negative margin, the spec should explicit say that the corner radii is the larger of 0 and the final result.

cc @astearns @dbaron

Activity

  1. astearns commented on Nov 3, 2016

    @astearns
    Member

    I don't recall all of the details, but what I do remember is that this was added to shapes and to box-shadow to avoid a discontinuity as border-radius interpolates from zero to a positive value. Please see the last paragraph of this section https://drafts.csswg.org/css-backgrounds-3/#shadow-shape and the thread here https://lists.w3.org/Archives/Public/www-style/2013Nov/0290.html

  2. astearns commented on Nov 7, 2016

    @astearns
    Member

    @aethanyc is that enough to explain why the ratio calculation is there? Does the spec need more clarification on that and/or the case of negative margins?

  3. aethanyc commented on Nov 8, 2016

    @aethanyc
    ContributorAuthor

    @astearns Yes. The links you provided explain the reason behind the need of the cubic function. However I think it'll be good if the spec could clarify the ratio under the case of negative margins as well as the final corner radii cannot be less than 0 (since the border-radius cannot be negative). For example, we could define radio = abs(border-radius/margin) so that the cubic equation make sense.

  4. dbaron commented on Nov 10, 2016

    @dbaron
    Member

    I don't think the cubic equation makes sense for negative margins; I think they should just use max(border-radius + margin, 0).

  5. dbaron commented on Nov 10, 2016

    @dbaron
    Member

    I believe that would also match the spec for box-shadow spread, since that spec says:

    However, in order to create a sharper corner when the border radius is small, when the border radius is less than the spread distance, the spread distance is multiplied by the proportion 1 + (r-1)3, where r is the ratio of the border radius to the spread distance, in calculating the corner radii of the spread shadow shape.

    ... and since border-radius must be nonnegative, it can only be less than the spread distance when the spread distance is positive.

  6. astearns commented on Oct 15, 2019

    @astearns
    Member

    I added the max(border-radius + margin, 0) bit in 6792d49
    @dbaron @aethanyc let me know if this works for you.

  7. aethanyc commented on Oct 16, 2019

    @aethanyc
    ContributorAuthor

    Looks good to me. It also matches what Gecko implements.

  8. Loirooriol commented on Oct 23, 2019

    @Loirooriol
    Contributor

    What if margin=0? Then the ratio border-radius/margin is mathematically undefined, so I would handle it with max(border-radius + margin, 0):

    If the margin is non-positive, or if the ratio of border-radius/margin is 1 or more, then the margin box corner radius is max(border-radius + margin, 0). If the margin is positive, and the ratio of border-radius/margin is less than 1, then the margin box corner radius is border-radius + (margin * (1 + (ratio-1)^3)).

    Note it checks the sign of margin before dividing by it, it's good practice.

  9. added a commit that references this issue on Oct 23, 2019
  10. astearns commented on Oct 23, 2019

    @astearns
    Member

    Thanks, @Loirooriol. I've added that case

  11. Loirooriol commented on Oct 24, 2019

    @Loirooriol
    Contributor

    OK, now it's properly defined, but there is a discontinuity. If e.g. borderRadius is 0.5,

    • For margin = 0, we get
      max(borderRadius + margin, 0)
      = max(0.5, 0)
      = 0.5
      
    • For margin -> 0⁺, we get
      lim_(margin -> 0⁺) borderRadius + margin * (1 + (borderRadius/margin-1)^3)
      = lim_(margin -> 0⁺) 0.5 + margin * (1 + (0.5/margin-1)^3)
      = lim_(margin -> 0⁺) 0.5 + margin * ((0.5/margin)^3 - 3*(0.5/margin)^2 + 3*0.5/margin)
      = lim_(margin -> 0⁺) 0.5 + 0.5 * ((0.5/margin)^2 - 3*0.5/margin + 3)
      = +∞
      
  12. astearns commented on Oct 24, 2019

    @astearns
    Member

    @Loirooriol are you sure? For borderRadius of 0.5, as soon as margin is 0.5, we go back to the max(border-radius + margin, 0) calculation.

  13. Loirooriol commented on Oct 24, 2019

    @Loirooriol
    Contributor

    @astearns Oops, sorry, you are right, of course! I wasn't paying enough attention to the definition. The discontinuity would be when borderRadius is negative and the margin is 0, but borderRadius can't be negative, so there is no problem :)

    Then just a last thought: are the extra parentheses deliberate?

    border-radius + (margin * (1 + (ratio-1)^3))
    

    Couldn't it just be

    border-radius + margin * (1 + (ratio-1)^3)
    
  14. astearns commented on Oct 24, 2019

    @astearns
    Member

    You are correct - I've removed the outer parens.

  15. dbaron commented on Nov 6, 2019

    @dbaron
    Member

    These changes seem to make sense to me modulo the concerns in #1900 about lack of implementation. (Though they led me to find what might be an error in the changes in #1900; see my comment there.)

  16. css-meeting-bot commented on Nov 7, 2019

    @css-meeting-bot
    Member

    The CSS Working Group just discussed [css-shapes] Calculation of corner radii by margin-box and border-radius, and agreed to the following:

    • RESOLVED: accept the change that astearns put into the ED, handling the case where margin = 0 for very small values of shape-margin
    The full IRC log of that discussion <myles> Topic: [css-shapes] Calculation of corner radii by margin-box and border-radius
    <myles> GitHub: https://github.com//issues/675
    <myles> astearns: This is a small change to existing change dealing with an edge case when margins hit 0. There was some discussion. The person who raised the issue seems okay with the change, as does dbaron. I wanted to have a WG resolution.
    <myles> astearns: Anyone have anything to add?
    <myles> astearns: The proposed resolution is we accept the change that I put into the ED, handling the case where margin = 0 for very small values of shape-margin
    <myles> astearns: objections?
    <myles> RESOLVED: accept the change that astearns put into the ED, handling the case where margin = 0 for very small values of shape-margin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions