You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[css-shapes] Calculation of corner radii by margin-box and border-radius #675
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.
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.
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.
@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?
@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.
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.
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.
@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?
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.)
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
Activity
astearns commentedon Nov 3, 2016
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
astearns commentedon Nov 7, 2016
@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?
aethanyc commentedon Nov 8, 2016
@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.dbaron commentedon Nov 10, 2016
I don't think the cubic equation makes sense for negative margins; I think they should just use
max(border-radius + margin, 0).dbaron commentedon Nov 10, 2016
I believe that would also match the spec for box-shadow spread, since that spec says:
... and since border-radius must be nonnegative, it can only be less than the spread distance when the spread distance is positive.
astearns commentedon Oct 15, 2019
I added the
max(border-radius + margin, 0)bit in 6792d49@dbaron @aethanyc let me know if this works for you.
aethanyc commentedon Oct 16, 2019
Looks good to me. It also matches what Gecko implements.
Loirooriol commentedon Oct 23, 2019
What if
margin=0? Then the ratioborder-radius/marginis mathematically undefined, so I would handle it withmax(border-radius + margin, 0):Note it checks the sign of margin before dividing by it, it's good practice.
add margin=0 for #675
astearns commentedon Oct 23, 2019
Thanks, @Loirooriol. I've added that case
Loirooriol commentedon Oct 24, 2019
OK, now it's properly defined, but there is a discontinuity. If e.g.
borderRadiusis 0.5,margin = 0, we getmargin -> 0⁺, we getastearns commentedon Oct 24, 2019
@Loirooriol are you sure? For
borderRadiusof 0.5, as soon asmarginis 0.5, we go back to themax(border-radius + margin, 0)calculation.Loirooriol commentedon Oct 24, 2019
@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?
Couldn't it just be
astearns commentedon Oct 24, 2019
You are correct - I've removed the outer parens.
dbaron commentedon Nov 6, 2019
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.)
css-meeting-bot commentedon Nov 7, 2019
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-marginThe 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