Skip to content

[css-overflow-3] overflow-clip-margin + border-radius continuity adjustment #5896

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

Closed
fantasai opened this issue Jan 26, 2021 · 11 comments
Closed

Comments

@fantasai
Copy link
Collaborator

border-radius and box-shadow have a continuity adjustment to ensure that zero radius is pointy. For overflow-clip-margin, do we apply this adjustment to the resulting offsetted position of the overflow clip edge, or do we apply it to the position of its origin edge (border-box/padding-box/content-box) and then apply an absolute offset from that curve?

@fantasai fantasai added the css-overflow-3 Current Work label Jan 26, 2021
@fantasai
Copy link
Collaborator Author

fantasai commented Jun 8, 2021

Here's an example of the question: suppose we have a border-radius that's less than the thickness of the border (which is, let's say, 10px). The border-edge is curved, the padding edge is square. If we specify overflow-clip-margin as '10px' in reference to the padding-box, do we get a square or round corner?

@frivoal
Copy link
Collaborator

frivoal commented Jun 8, 2021

I suspect the difference here is not use-case driven, so we should define, but it doesn't particularly matter what we pick as long as it's interoperable.

Do implementers have a preference? How was it done in Blink?

cc: @chrishtr

@chrishtr
Copy link
Contributor

chrishtr commented Jun 8, 2021

Re implementation: there's a related (not the same) bug I'm looking into at crbug.com/1200163.

@chrishtr
Copy link
Contributor

border-radius applies to the border edge, not to the overflow clip edge. See this comment where I discussed it with @sky2.

I think this means that the question in this issue is moot, unless we want to consider new ways to round corners at places other than the border. Perhaps that should be considered - I've already heard a request to have a way to apply the border radius to the padding or other boxes.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed overflow-clip-margin and border-radius.

The full IRC log of that discussion topic: overflow-clip-margin and border-radius
fantasai: The border radius applies to the border edge but it affects the padding contant edges. Are you offsetting from the shape of the offsetting box, or against the border edge?
s/contant/content/
github: https://github.com//issues/5896
fantasai: The border radius should not be affected by overflow-clip-margin.
Florian: the question is how these should interact
florian: There's a formula for rounding, it will give different results depending on how you apply it
florian: maybe we just need to spec what's implemented so far?
Rossen: we're at time
chrishtr: sounds like we should go back to issue
[note: smfr asked for pictures also]
fantasai: Why was https://github.com//issues/4791 on the adgenda?
Meeting closed.
iank_: good question... I think maye to confirm that we're editing in the 'zero area elements don't take up space' rule?
ok sg.

@astearns astearns removed the Agenda+ label Jun 23, 2021
@dbaron
Copy link
Member

dbaron commented Jun 28, 2021

A bit of background here:

The backgrounds spec describes how to map the rounding of the border edge to rounding of the padding edge, content edge, or margin edge. The last (margin edge) uses a more complicated formula because it has the goal of not creating a lot of curvature where there wasn't much curvature at the border. For example, with a 2px border-radius and a 10px margin, it produces curvature at the margin edge of 6.88px rather than the 12px produced by the naive formula.

The definition of box-shadow separately describes the same formula.

The overflow-clip-margin spec says:

The overflow clip edge is shaped in the corners exactly the same way as a box-shadow with a spread radius of the same cumulative offset from the box’s border edge. See CSS Backgrounds 3 §4.2 Corner Shaping.

which links to the previously-cited section about border-radius (but not the box-shadow section where the rules for box-shadow are defined, in the same way).


I think if this formula is generalized to apply to things involving more than one offset from the border edge, it would be best to (1) first combine the offsets and then (2) apply the border-radius adjustment formula a single time, to the combined offset. This will prevent inconsistent curvature resulting from multiple applications of the formula with different combinations of offsets that add to the same or similar results.

@dbaron
Copy link
Member

dbaron commented Jun 28, 2021

Oh, and the point @chrishtr makes is that §4.3 Corner Clipping defines border-radius combined with clipping as always clipping to the border-edge. In other words, if you interpret the specs as written, two different pieces of the spec are defining two different clips: one at the border edge and one at the overflow-clip-margin edge. The practical result of this is that the smaller clip region is the one that's visible.

I think the intent of the overflow-clip-margin wording is probably that it overrides this behavior, but I don't see a place the spec says to do that.

mfreed7 pushed a commit to mfreed7/csswg-drafts that referenced this issue Aug 4, 2021
@fantasai
Copy link
Collaborator Author

fantasai commented Aug 4, 2021

@dbaron OK, closed out the issue in line with your recommendation from #5896 (comment) in 7493d0c

As for #5896 (comment) the backgrounds spec doesn't define which edge is clipped to, it just requires that the clipped edges are curved and gives examples, so I'm not convinced there's an actual problem here.

Although border images are not affected by border-radius, other effects that clip painting or event handling to the border, padding, or content edge must clip to their respective curves.

Where overflow-clip-margin defines a different edge than one of these, its rules clearly override.

Agenda+ to confirm with the WG that we accumulate the offset from the border edge and apply the same formulas used for margin/padding/box-shadow edges.

@dbaron
Copy link
Member

dbaron commented Aug 16, 2021

Where overflow-clip-margin defines a different edge than one of these, its rules clearly override.

I think the problem is that it's not clear to many people. In particular, I think you're thinking of this in a manner similar to the way the color property works across elements in the tree -- the color on a descendant replaces the color on an ancestor. However, I think people who work with code that implements clipping almost always think of clips the way we (in the CSS world) think of background-color. When multiple things define a (clip / background-color), all of the (clips apply / backgrounds paint), and they then (intersect with each other / draw on top of each other). So if there's spec text that says X creates a clip, and that clip should be different in some cases, the spec text really needs to say so right there, since otherwise the general rule for combining clips is to intersect all the relevant ones.

@dbaron
Copy link
Member

dbaron commented Aug 16, 2021

I think it would also be very helpful to put the corner adjustment text in one place rather than two, and make it more direct about how it deals with offsets in each direction (rather than very specific to the properties talked about in the two current places it lives). (For example, the text on margins in Corner Shaping looks like it only handles negative margins correctly by accident, and the text in Shadow Shape, Spread, and Knockout relies heavily on shadow concepts in a way that makes it hard to easily apply it to other things.)

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed overflow-clip-margin + border-radius continuity, and agreed to the following:

  • RESOLVED: overflow-clip-margin uses the same corner-adjustment formula as margin/etc
  • RESOLVED: Accept dbaron's editorial feedback about making the "accidental" case more explicit.
The full IRC log of that discussion Topic: overflow-clip-margin + border-radius continuity
github: https://github.com//issues/5896#issuecomment-892995385
ScribeNick: TabAtkins
fantasai: I think dbaron's comments are mostly editorial
fantasai: So I'll ask the main question
fantasai: Do we want to accept that the overflow-clip-margin follows the same corner-adjustment rules as margin/border/padding edges (so pointy corners stay pointy, round corners stay round)
fantasai: When you have a curved border edge, and we calculate the equivalent padding or content-box edges, we subtract the border/padding widths, flooring at zero.
fantasai: Similarly when you're going out to the margin edge or box shadows, we *add* the margin/shadow size to the radius, but with a special adjustment that keeps zero radius at zero.
fantasai: Some math keeps it continuous between the two cases without causing noticeable changes in most cases
dbaron[m]: Another complexity is that a corner might be an inset on one side and outset on the other. already possible with negativemargins
dbaron[m]: This probably makes that case more common, since it's an offset from the padding edge
https://drafts.csswg.org/css-backgrounds-3/#corner-shaping
dbaron[m]: I think the spec actually does something reasonble for this right now, but it's by accident. I think it's right, tho
https://drafts.csswg.org/css-backgrounds-3/spread-radius
dbaron[m]: It probably should look more intentional to make sure impls think about that case
fantasai: I just posted the testcase we used when we were figuring out the continuity formula
fantasai: So proposal is to use the same formula for overflow-clip-margin
astearns: Any othe ropinions?
[no objections]
RESOLVED: overflow-clip-margin uses the same corner-adjustment formula as margin/etc
astearns: Should we also resolve about being more clear about the positive-and-negative margin case?
fantasai: sure
astearns: So proposal is to accept dbaron's feedback, making the "accidental" correctness more explicit.
RESOLVED: Accept dbaron's editorial feedback about making the "accidental" case more explicit.
dbaron[m]: I don't think there's more to discuss, but I did raise more substantive editorial issues that I think could confused implementors.
dbaron[m]: Do think we need to deal with those, but don't think we need group time for them.
astearns: So fantasai you'll go thru those and either ipmlement or bring back to the group if necessary?
yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants