-
Notifications
You must be signed in to change notification settings - Fork 664
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[css-shapes] Calculation of corner radii by margin-box and border-radius #675
Comments
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 |
@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 |
I don't think the cubic equation makes sense for negative margins; I think they should just use |
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. |
Looks good to me. It also matches what Gecko implements. |
What if
Note it checks the sign of margin before dividing by it, it's good practice. |
Thanks, @Loirooriol. I've added that case |
OK, now it's properly defined, but there is a discontinuity. If e.g.
|
@Loirooriol are you sure? For |
@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
|
You are correct - I've removed the outer parens. |
The CSS Working Group just discussed
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 |
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:My questions mainly related to the
ratio < 1
case.ratio < 1
andmargin > 0
. Why do we need to adjust margin bymargin * (1 + (ratio-1)^3)
which makes the margin even smaller? It seemsborder-radius + margin
is fine.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 is10 + (-5) * (1 + (-3)^3) = 140
. So it seems the spec needs some clarification whenmargin < 0
. Again simpleborder-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
The text was updated successfully, but these errors were encountered: