Skip to content
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-backgrounds] The shape of box-shadow should be a circle for a box with border-radius:50% and big spread #7103

Open
wangxianzhu opened this issue Mar 5, 2022 · 52 comments
Labels
css-backgrounds-3 Current Work

Comments

@wangxianzhu
Copy link

wangxianzhu commented Mar 5, 2022

https://www.w3.org/TR/css-backgrounds-3/#shadow-shape

For the following case:
data:text/html,<div style="width: 40px; height: 40px; border-radius: 20px; box-shadow: black 0 0 0 80px; margin: 100px">
should we expect that the outer edge of the shadow is a circle (instead of a rounded rect with the 1+(r-1)^3 adjustment)?

Perhaps we should apply the adjustment only if the radius is smaller than e.g. 1/4 of the box size?

@tabatkins tabatkins added the css-backgrounds-3 Current Work label Mar 7, 2022
@tabatkins
Copy link
Member

Hm, I was about to say that browsers currently produce circles (testing Firefox and Chrome), but turns out that's because they don't implement the cubic reduction at all; if your example is changed to a 1px border-radius it still produces a huge rounded rect rather than a nearly-sharp square.

It does seem like it should maintain roundness when it's entirely round, I agree. How to achieve that smoothly and reasonably is an interesting question. /cc @fantasai @bradkemper

@wangxianzhu
Copy link
Author

wangxianzhu commented May 19, 2022

Chrome has changed to conform to the current spec. Since the change, people have filed bugs (e.g. https://crbug.com/1322942) about the case.

@SelenIT
Copy link
Collaborator

SelenIT commented May 19, 2022

I believe that anchoring the geometric adjustment of the shadow shape to the absolute value of corner radii regardless the proportions of the shape was probably not the best idea, too. The result when the corners of the shadow become relatively sharper than the corners of the original shape is far from intuitive and often undesired.

Maybe the shape of the shadow should simply be geomtrically similar to the original shape, i.e. roundings should take the same part of the side as they do on the original shape? At first glance, this would produce the smooth transition between sharp corners and complete roundness in the most natural way.
Screenshot 2022-05-19 at 15 55 28

@Loirooriol
Copy link
Contributor

I tend to agree that geometric similarity can sound good.

However, this would imply that the shadow wouldn't spread by the same amount in both axes, e.g.

<div style="width: 500px; height: 100px; background: cyan; border-radius: 250px / 50px; box-shadow: 0 0 0 125px blue"></div>

The border area is cyan, it's 500x100 with a border-radius of 250px / 50px, i.e. 50%. If the shadow spreads 125px horizontally, then border radius of the shadow should have an X component of 250+125 = 375, and the Y component should preserve the ratio in order to get a similar shape: 50 * 375/250 = 75. So the border radius of the shadow should be 375px / 75px.

However, while the shadow is 750px wide, so 375px is the 50%, the shadow should also spread 125px vertically, becoming 35px tall. This breaks the similarity: it should be 150px tall, so that 75px is the 50%. This difference is represented in red in the image.

So similarity would require changing the spread distance to apply in the biggest dimension, and resolving the other one proportionally. For continuity, this would need to happen regardless of whether there is a border-radius. But this would worsen usecases where you don't care about similarity, and instead just want a shadow of the same thickness in both axes, e.g. in order to have a secondary border.

@Loirooriol
Copy link
Contributor

Loirooriol commented May 23, 2022

I guess what could work is:

  1. Calculate the radius of the border corners in percentages of the border box size
  2. Set the radius of the shadow corners to be the same percentages, but now resolved with the shadow size.

Or in other words,

  1. Resolve the radius of the border corners as a pair of lengths x / y.
  2. Let the sizes of the border box be b_x and b_y
  3. Let the sizes of the shadow box be s_x and s_y
  4. Set the radius of the corresponding corner of the shadow box to calc(x * s_x / b_x) / calc(y * s_y / b_y), or to 0px to avoid dividing by zero.

So if you have the initial border-radius: 0px (i.e. 0%), then the shadow will keep being sharp with 0px.

And if you have border-radius: 50% then the shadow will keep being a full circle/ellipsis with 50% of its size.

@matthew-dean
Copy link

I’m seeing a lot of tweets about this breaking change in calculation like: https://twitter.com/alvaro_montoro/status/1532869371248394241?s=21&t=vhQ6aoBDItk9fALwfe5WDg

Isn’t it fair to say that a “wrong” calculation, but one done for years consistently across browsers and a calculation people rely on for illustrations is a “Don’t Break The Web” case?

IMO radical changes in CSS rendering should be avoided.

@bradkemper
Copy link
Contributor

I completely missed this. Was there a spec change? Doesn't seem like there should be breaking changes this late in the game.

@bradkemper
Copy link
Contributor

I would expect a circle shape to have a circle-shape box-shadow, no matter how big the spread.

@faceless2
Copy link

I don't think it's a spec change, just implementations catching up with the spec.

We had implemented the cubic reduction algorithm for box-shadow. I've now just tried patching in the proposed algorithm from @Loirooriol above for the three tests described in the comments above:

  • <div style="width: 40px; height: 40px; border-radius: 2px; box-shadow: black 0 0 0 80px">
  • <div style="width: 40px; height: 40px; border-radius: 20px; box-shadow: black 0 0 0 80px">
  • <div style="width: 500px; height: 100px; background: cyan; border-radius: 250px / 50px; box-shadow: 0 0 0 125px blue">

Current specification rendering on the left, revised algorithm on the right. Overall it looks like an improvement.

Untitled-1

@SelenIT
Copy link
Collaborator

SelenIT commented Jun 6, 2022

@fantasai @tabatkins what do you think? Wouldn't @Loirooriol's algorithm be a better solution for the zero/nonzero radius discontinuity problem than cubic (or likely any other) function of the absolute value of the radius that disregards its relative size (hence perceived shape)?

@fantasai
Copy link
Collaborator

fantasai commented Jun 9, 2022

Agenda+ to accept @Loirooriol's algorithm

@Loirooriol
Copy link
Contributor

Loirooriol commented Jun 10, 2022

I guess the potential surprising/undesirable outcome of my proposal is that is you set border-radius to a length, then you get circular corners. But if the width and height are different, the corners of the shadow will be elliptical instead of circular.

This e.g. makes the corners appear thicker when using a thin shadow spreading 1px:

demo

@Loirooriol
Copy link
Contributor

Another approach could be just keeping

To preserve the box’s shape when spread is applied, the corner radii of the shadow are also increased (decreased, for inner shadows) from the border-box (padding-box) radii by adding (subtracting) the spread distance (and flooring at zero).

with no further adjustment. This would imply that shadows would have rounded corners even with border-radius: 0px, but the trick would be that the initial value would be changed to border-radius: none. none would behave like 0px, but keeping shadows sharp.

Though this may have compat problems since probably there are various sites setting border-radius: 0px expecting a sharp shadow.

@faceless2
Copy link

Ideally someone (code for "not me" 😄) should update https://drafts.csswg.org/css-backgrounds-3/spread-radius to add the new proposed algorithm(s) and - most importantly - the ability to resize the box.

Clearly a lot of thought has gone into which algorithm is best, I think the only thing that was missed was how it performs on zero-size boxes.

@bradkemper
Copy link
Contributor

A box-shadow with no offsets or blur, and a large spread, should be indistinguishable from a border of the same width as the spread (aside from affecting layout, etc.). The shape should match, and the effect of having rounded corners should match. I guess I'm missing what the issue is that's causing it to be so different. A border of even width around a circle is still circular.

@Loirooriol
Copy link
Contributor

I guess I'm missing what the issue is that's causing it to be so different

The difference is that border-radius affects the border area, so no need to do anything for the border other than:

The padding edge (inner border) radius is the outer border radius minus the corresponding border thickness. In the case where this results in a negative value, the inner radius is zero.

But a spreading shadow goes beyond the border area, so its radius needs to be increased. And here there is a problem, because a sharp 0px should also produce a sharp shadow, but we should also have continuity.

@wangxianzhu
Copy link
Author

I think the outer edge of a rounded outline should follow the same rule as box-shadow spread. Filed #7378.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed box-shadow roundness/sharpness.

The full IRC log of that discussion <TabAtkins> Topic: box-shadow roundness/sharpness
<TabAtkins> github: https://github.com//issues/7103
<TabAtkins> [looking for Oriol on the call]
<TabAtkins> fantasai: Let's push to next week

@vmpstr
Copy link
Member

vmpstr commented Jun 28, 2022

Yet another alternative is to use Oriol's algorithm for one the larger of the corner dimensions and then use the initial border radius aspect ratio to compute the other smaller dimension. Since the percent increase of the shadow in different dimensions is not the same, it would at least preserve the corner shape.

It works nice for "typical" rounded corners of otherwise rectangular boxes, without changing the shape of the corner, and it works well for a circle.

However, it adds a straight edge to the side that has a larger percent increase in length. This isn't noticeable in rectangular boxes but a bad case I found is that it tends to not look too nice on ellipses
ellipses

@fantasai
Copy link
Collaborator

fantasai commented Jun 29, 2022

It sounds like maybe we need some kind of threshold formula? E.g. supposing the straight length of the side is L and the radius on that side is R, we'd do something like:

  • if L = 0 use the percentage formula
  • if L >= R use the spec’s cubic formula
  • if 0 < L < R interpolate between the two formulae

Although I think we need some experimentation to know what the right thresholds are...

@fantasai
Copy link
Collaborator

Alternatively, maybe treat percentage radii as percentages of the shadow, and use the existing formula for length radii?

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed box shadows and circles.

The full IRC log of that discussion <TabAtkins> Topic: box shadows and circles
<TabAtkins> github: https://github.com//issues/7103
<TabAtkins> oriol: When you specify border-radius, the length is the corner radius of the outer border edge
<TabAtkins> oriol: For inner edge we subtract the border width from that
<TabAtkins> oriol: Spread is the opposite
<TabAtkins> oriol: Initial vaule of spread is 0 tho, so this would make an ellipitical border radius ahve corners
<TabAtkins> oriol: Firefox has special case for spread radius of 0, keeping it sharp. But 0.1 makes rounded corners
<TabAtkins> oriol: Suggest adding a correction term that gives sharp corners at 0 and continouously transforms to round.
<TabAtkins> oriol: So the issue: if you say border-radius:50% you get an ellipse.
<TabAtkins> oriol: If you have a shadow you probably also expect it will be an ellipse.
<TabAtkins> oriol: Blink changed impl to match the spec, and people complained their shadows were no longer circles.
<TabAtkins> oriol: Proposal in the issue is, instead of adding the spread distance and subtracting a correction term, we could express the border-radius as a % and then, for the shadow, we use the same % but resolved agaisnt th esize of th eshadow
<TabAtkins> oriol: This seems to improve various cases, we have posted images in the thread
<TabAtkins> oriol: Shadows look better, stay circular
<astearns> images in this comment: https://github.com//issues/7103#issuecomment-1146870682
<TabAtkins> oriol: Downside is if you specify a length, and elemnet has circular corners, shadow will have circular corners. New change will instead make them elliptical if the box isn't square
<TabAtkins> oriol: Vlad suggested a variant where we only resolve to a % in one axis, then keep the same aspect ratio for the corner
<astearns> problem ellipse images in this comment: https://github.com//issues/7103#issuecomment-1168157338
<TabAtkins> oriol: Problem is if th eelement is a full ellipse, you won't get an ellipse shadow. Circles stay circles but in an ellipse you'll get flat edges on the short axis.
<TabAtkins> oriol: I proposed another idea - we could say that for the shadow we just add spread-distance to the border-radius
<TabAtkins> oriol: This would imply that for 0px you'd have rounded corners
<TabAtkins> oriol: But we'd also change the initial value for border-radius is "none", which would stay sharp. Unsure about compat tho.
<TabAtkins> oriol: fantasai proposed interpolating between some radius formulas.
<TabAtkins> oriol: Various options, not sure which is best.
<TabAtkins> fantasai: Two ideas. One is the problem is largely around circular/oval shapes. Many of those are done with %s.
<TabAtkins> fantasai: So we could say if the radius is a % we maintain it as a % in the spread shape.
<TabAtkins> fantasai: Dunno if that really works since the other way to do a circle is to do a huge px length and we scale it down.
<TabAtkins> fantasai: So it depends on how authors are specifying things.
<TabAtkins> fantasai: Another possibility is to interpolate based on how much of a straight side we have.
<TabAtkins> fantasai: So if the straight side is longer than the radius, we use existing formula which is good for rectangular shapes
<TabAtkins> fantasai: If straight side is 0 we use the % formula
<TabAtkins> fantasai: And between we can interpolate.
<fantasai> TabAtkins: I hadn't gone this deep before, I suspect I have opinions, but would like to take up at a later call, maybe at F2F
<fantasai> fantasai: F2F sounds good, can draw stuff
<fantasai> astearns: Leading up to F2F, we should have a set of things to test against
<fantasai> astearns: We absolutely need to fix the spec
<fantasai> astearns: I'm not sure that we have a fix that is good enough for all the edge cases yet
<TabAtkins> fantasai: We have a bunch of example scurrently, both working and not.
<fantasai> s/have/need to have/
<TabAtkins> I suspect we might need to address this semantically at the border-radius side with a keyword that does ellipse.
<TabAtkins> astearns: Can I ask Oriol to list out the cases to consider?
<TabAtkins> astearns: Summarizing them in the issue, and people can respond in the issue.
<TabAtkins> astearns: We'll tag this for f2f and come back to it
<fantasai> https://drafts.csswg.org/css-backgrounds-3/spread-radius
<TabAtkins> fantasai: We also have this which we can update

@bradkemper
Copy link
Contributor

The padding edge (inner border) radius is the outer border radius minus the corresponding border thickness. In the case where this results in a negative value, the inner radius is zero.
But a spreading shadow goes beyond the border area, so its radius needs to be increased. And here there is a problem, because a sharp 0px should also produce a sharp shadow, but we should also have continuity.

I'm still not getting it. How does the sharp corner at 0px harm continuity? Continuity of what? And how important is it to fix the issue, vs. having a break in continuity or whatever? Sorry, I'm sure I'm still missing something, because I'm still not seeing how a sharp corner at r:0 is such a problem that it warrants distorting the shape so severely at every other radius.

@bradkemper
Copy link
Contributor

If this is about animating a high-spread shadow between a small border-radius and a zero border-radius, and seeing a sharp corner suddenly appear at the end, then I think that is much, much, less bad than the distortions that are occurring from the remediation. This really seems like a cure that is much worse than the disease.

I vaguely recall a conversation about that sort of thing some time ago, but I didn't realize the effect of the fix would be so pronounced and bad. I'm sorry I wasn't more involved or thorough in gaining an understanding of the issue at the time and helping to work out a better solution.

@dbaron
Copy link
Member

dbaron commented Sep 19, 2022

I fixed the bug mentioned above (which was misplaced )s in the last change I made before our discussion) and added the additional variants mentioned above.

I also added an additional testcase that I think @fantasai's variant performs poorly on... at least assuming I'm understanding that variant correctly. (The variant is a 25px radius on a narrow rectangle whose short dimension is 50px.)

@dbaron
Copy link
Member

dbaron commented Sep 19, 2022

Oh, and I should link to the current version of the demos.

@fantasai
Copy link
Collaborator

fantasai commented Oct 4, 2022

@dbaron For clarification, if using the formula in #7103 (comment) , if the short side of the rectangle is 50px and it has a 25px border radius, then its spread radii in the short axis should be 50% and the spread radii in the long axis should be as dictated by the current spec formula.

(Same case except the short side of the rectangle being 75px instead of 50px, the spread radii would be as dictated by the current spec formula in both axes.)

@LeaVerou
Copy link
Member

FYI @fantasai and I just committed a trimmed down set of algorithms with a correct implementation of Elika's proposal, and made the testcases editable: https://drafts.csswg.org/css-backgrounds-3/radius-expansion.html

@Loirooriol
Copy link
Contributor

return Math.min(ret, straight + value + testCase.spread);

I'm not sure about straight. Should it be just Math.min(ret, value + testCase.spread), i.e. clamp by "increase-by-spread"?

Some difference can be seen with {"width":500,"height":60,"spread":30,"borderRadius":"1px 1px 49px 49px"}.

@lobo-tuerto

This comment was marked as off-topic.

@fantasai

This comment was marked as off-topic.

@lobo-tuerto

This comment was marked as off-topic.

@Loirooriol
Copy link
Contributor

Loirooriol commented Feb 2, 2023

Elika's option doesn't round at all with {"width":500,"height":50,"spread":30,"borderRadius":"0px 0px 50px 50px"}

And I'm not convinced about {"width":500,"height":70,"spread":100,"borderRadius":"0px 0px 50px 50px"}:

With the change from #7103 (comment) it seems better

But still elliptical, should it be circular instead?

@Loirooriol
Copy link
Contributor

I have updated https://drafts.csswg.org/css-backgrounds-3/radius-expansion.html with more testcases and a new proposal. To summarize the context of this issue:

  • When reducing a border radius, we just subtract the distance (and clamp at zero)
  • So the obvious thing to do when expanding a radius, would be to add the distance
  • But that's problematic, because non-rounded corners (radius of 0) should stay non-rounded.
  • So the current spec adds the distance multiplied by an adjustment factor. This factor is 1 if the radius is greater than the spread distance, it's zero if the radius is 0, and it's continuous between these cases.
  • The problem is that, if the original shape is a full ellipsis/circle, then the expanded shape may stop being a full ellipsis/circle.

So, my idea is: in the adjustment factor, instead of only considering the ratio between the radius and the spread distance, also consider the ratio of the element sizes covered by the radius. If the horizontal radius is at least 50% of the width of the element, or the vertical radius is at least 50% of the height, then we want the factor to be 1, even if the spread distance is much bigger than the radius.

BTW, this approach guarantees that, if a corner is circular (same horizontal and vertical radii), the expanded corner will stay circular instead of becoming elliptical.

This is the code in the demo:

for (let corner in radii) {
  let coverage = Math.max(
    2 * radii[corner][0] / testCase.width,
    2 * radii[corner][1] / testCase.height,
  ) || 0;
  r[corner] = radii[corner].map(value => {
    if (value >= testCase.spread || coverage >= 1) {
      return value + testCase.spread;
    }
    let r = (1 - value / testCase.spread) * (1 - coverage);
    return value + testCase.spread * (1 - r**3);
  });
}

I think it produces better results than Elika's proposal for {"width":500,"height":60,"spread":30,"borderRadius":"20px 20px 40px 40px"} and {"width":250,"height":35,"spread":50,"borderRadius":"0px 0px 25px 25px"}.

@fantasai
Copy link
Collaborator

fantasai commented Feb 8, 2023

Elika's option doesn't round at all with {"width":500,"height":50,"spread":30,"borderRadius":"0px 0px 50px 50px"}

It should? It should result in a bottom left radius of y=50% x=80px ...

@Loirooriol
Copy link
Contributor

The demo produces 0px 0px 80px 80px / NaNpx NaNpx 80px 80px because of straight / value both being 0. I guess the ratio just needs to turn NaN into 0 or 1, probably not much important.

@yarusome
Copy link

yarusome commented Feb 28, 2023

I'd like to make new proposals consisting of two weakly related parts:

  • a spread-adjustment formula based on geometric means;
  • two variants of formulae for better appoximating the actual shape of the spread of an elliptic border.

Here's the demo based on Loirooriol's with two more test cases added:

https://yarusome.github.io/box-shadow-proposal/


Summary for the spread-adjustment formula:

If the spread is less than the radius, the spread is not adjusted. Otherwise the adjusted spread satisfies:

radius + adjusted_spread = C * (radius + spread) ^ coverage * radius ^ (1 - coverage),

where

coverage = min(radius * 2 / side_length, 1),

and the constant C = 2 ^ (1 - coverage) guarantees that the formula is continuous at radius = spread.


Summary for the elliptic correction formulae:

For an elliptic border, both variants ensure that the approximate spread passes through a particular point on the actual spread. (A side effect is that circular borders automatically produce circular spreads.) But to fix the radii of the approximate spread:

  • The first variant makes a requirement on the aspect ratio of the approximate spread.
  • The second variant requires that the longer axis of the approximate spread “take as much length as possible.”

Personally I find the second elliptic correction formula aesthetically better.


If the WG is interested, I could also write a mathematical note explaining the derivations of the two correction formulae.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed border-radius adjustment formula.

The full IRC log of that discussion <fantasai> Topic: border-radius adjustment formula
<fantasai> github: https://github.com//issues/7103
<fantasai> fantasai: not prepared to present; suggest taking at F2F where we have better visuals

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-backgrounds] The shape of box-shadow should be a circle for a box with border-radius:50% and big spread.

The full IRC log of that discussion <emilio> oriol: IIRC we had a webpage where you could test the various algorithms
<fantasai> -> https://yarusome.github.io/box-shadow-proposal/
<oriol> https://drafts.csswg.org/css-backgrounds-3/radius-expansion.html
<emilio> oriol: summary, when going inwards with border-radius you can just reduce the amount of radius
<emilio> ... but when going outwards if we just add the spread-distance then we break border-radius: 0
<emilio> ... browsers checked if border-radius is zero, but that's not great because it's not continuous
<emilio> ... spec tries to use a cubic formula that keeps being zero if it was zero but that has an issue but that deforms the inner shape with circles etc
<emilio> ... we had various ideas for ways to try to approach these
<emilio> ... one of the ideas was to try to keep percentages
<emilio> ... so we'd express the border radius as percentages and apply it to the inner and outer box
<emilio> ... but that doesn't work if the aspect ratio of the inner and outer didn't match
<astearns> two more options added from the latest comment: https://yarusome.github.io/box-shadow-proposal/
<emilio> ... then fantasai proposed an interpolation between the two
<emilio> ... I found some cases when this was not behaving that welll
<emilio> ... I proposed a modification to the current spec but with the addition of another factor
<astearns> s/two more/three more
<emilio> ... that also considers the ratio of the element size that's covered by the radius
<emilio> ... so if the horiz radius is >50% of the width of the element then we want this factor to be 1
<emilio> ... even if the spread is much bigger than the radius
<emilio> ... you can see the test-case, it seems it was better in some cases
<emilio> ... then someone else (yarusome) proposed some other ideas
<Rossen_> q?
<emilio> ... not sure if we should try with a whiteboard or some brainstorming to try to come up with a solution
<Rossen_> ack oriol
<emilio> ... short version is take a look at the test-case
<emilio> ... I don't think we have the right answer yet
<emilio> ... someone added more funny cases
<fantasai> scribenick: fantasai

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-backgrounds-3 Current Work
Projects
Status: Wednesday
Development

No branches or pull requests