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-shapes-2] Minor comments on shape() #5841

Closed
grorg opened this issue Jan 7, 2021 · 20 comments · Fixed by #9797
Closed

[css-shapes-2] Minor comments on shape() #5841

grorg opened this issue Jan 7, 2021 · 20 comments · Fixed by #9797

Comments

@grorg
Copy link
Contributor

grorg commented Jan 7, 2021

I was a few seconds late providing review feedback on #5711 . It's probably easier to read my comments inline there, but I'll repeat them here.

  1. Minor typo: corresponds
  2. The spec needs to describe animation, specifically that it works the same as SVG paths since the shape syntax is a mapping to SVG path primitives. It should probably call out the fact that the curve operator actually produces two segment types, so it may not be possible to smoothly animate between two curves.
  3. For that reason, I'd consider splitting cubic and quadratic into different operations rather than rely on the number of parameters
  4. Similarly, a curve and smooth might actually be the same segment type as far as animation is concerned! Confusing!
  5. I don't think via is a good term for bezier control points. The curve does not (typically) go through those points, which is what the definition of via would suggest. Maybe using is a better term?
  6. In SVG syntax you specify the control points before the end point. Is there a reason this was not followed here? The rationale behind that in SVG is that the point specified last in commands is where the pen ends up (where the next segment starts).
@grorg
Copy link
Contributor Author

grorg commented Jan 7, 2021

cc @noamr @tabatkins

@grorg
Copy link
Contributor Author

grorg commented Jan 7, 2021

produces two segment types

I meant "can produce either of two segment types".

@noamr
Copy link
Collaborator

noamr commented Jan 7, 2021

I was a few seconds late providing review feedback on #5711 . It's probably easier to read my comments inline there, but I'll repeat them here.

  1. Minor typo: corresponds
    Ack
  1. The spec needs to describe animation, specifically that it works the same as SVG paths since the shape syntax is a mapping to SVG path primitives. It should probably call out the fact that the curve operator actually produces two segment types, so it may not be possible to smoothly animate between two curves.
  2. For that reason, I'd consider splitting cubic and quadratic into different operations rather than rely on the number of parameters
  3. Similarly, a curve and smooth might actually be the same segment type as far as animation is concerned! Confusing!

I agree that consistency is of value, but I also dislike how SVG path animations are so restrictive, and I wish there was something we could do about it. Maybe we can word it in a way that releases this from this restriction? :(

My original thought was, instead, to treat all path segments (except move to/by) as cubic bezier curves - quadratic curves (and arcs?) would have the 2 control points at the same spot, and lines would have the two control points within the line, evenly distributed. This would allow nice smooth animations between lines/quadratic/cubic cutves/arcs without the author having to manually declare all of them as cubic bezier curves.

Maybe this can also be backported to SVG, with an opt-in attribute to maintain backwards compatibility.

  1. I don't think via is a good term for bezier control points. The curve does not (typically) go through those points, which is what the definition of via would suggest. Maybe using is a better term?

Right. using would be maybe more accurate.

  1. In SVG syntax you specify the control points before the end point. Is there a reason this was not followed here? The rationale behind that in SVG is that the point specified last in commands is where the pen ends up (where the next segment starts).

I specified it like this because the word by/to is important in order to understand the meaning of the rest of the coordinates. But I agree that it might make more sense to have the end point at the end of the phrase.

@tabatkins
Copy link
Member

Okay, I've done an editorial pass over the spec, and specified the animation behavior. As a first draft, I just made sure it works the same as path(), as far as I can tell per https://www.w3.org/TR/SVG2/paths.html#TheDProperty. It's not very clear what the actual rules are - should the three line commands interpolate? The two quadratics and the two cubics?

In SVG syntax you specify the control points before the end point. Is there a reason this was not followed here? The rationale behind that in SVG is that the point specified last in commands is where the pen ends up (where the next segment starts).

I actually prefer the current ordering, where all the commands immediately list their ending point and then have their extra info if needed; I think it reads well with the by/to keywords. SVG's ordering made more sense with its implicit command chaining, imo.

@tabatkins
Copy link
Member

Per animation, if we can change up the path interpolation rules, I'm all for letting lines/quadratics/cubics all interpolate together (as cubics), tho we'd have to choose where the control points are for the lines - I think they can go literally anywhere on the line without affecting anything? If so, I guess they should either both be in the middle, or maybe the first at the starting point and the second at the ending point?

But I'd really like to maintain the ability for path() and shape() to interpolate together, so this would change the path() interpolation rules too.

@tabatkins
Copy link
Member

Well, I guess we could let path() keep the SVG interpolation rules when interpolating with another path(), and use the looser rules only when interpolating shape() with itself or path().

@noamr
Copy link
Collaborator

noamr commented Jan 9, 2021

Per animation, if we can change up the path interpolation rules, I'm all for letting lines/quadratics/cubics all interpolate together (as cubics), tho we'd have to choose where the control points are for the lines - I think they can go literally anywhere on the line without affecting anything? If so, I guess they should either both be in the middle, or maybe the first at the starting point and the second at the ending point?

But I'd really like to maintain the ability for path() and shape() to interpolate together, so this would change the path() interpolation rules too.

I think both in the middle would interpolate in a more natural way with quadratic curves, staying quadratic throughout the animation, and would be easy to understand.

@noamr
Copy link
Collaborator

noamr commented Jan 9, 2021

Well, I guess we could let path() keep the SVG interpolation rules when interpolating with another path(), and use the looser rules only when interpolating shape() with itself or path().

I think this would work
Allowing ‘path()’ to follow SVG more closely for consistency and releasing ‘shape()’ from this burden.

@noamr
Copy link
Collaborator

noamr commented Jan 27, 2023

Apologies, I missed the latter part of the list.

I'm OK with these suggestions:

  • using instead of via
  • flipping the order
  • being explicit about smooth vs curve

It's more verbose but would make it closer to SVG.
@tabatkins WDYT?

@tabatkins
Copy link
Member

  • using seems fine
  • If we're changing the order I'd prefer to let it be flexible - curve [ [ <by-to> <coordinate-pair> ] && [ using <<coordinate-pair>{1,2} ] ]. I think both curve to 10px 20px using 0px 10px and curve using 0px 10px to 10px 20px read reasonably. (I do prefer the current "to-first" order, but Dean's complaint about it not matching the SVG path order is reasonable.)
  • Not entirely sure which of Dean's complaints you're referring to here.

@noamr
Copy link
Collaborator

noamr commented Jan 28, 2023

  • using seems fine

  • If we're changing the order I'd prefer to let it be flexible - curve [ [ <by-to> <coordinate-pair> ] && [ using <<coordinate-pair>{1,2} ] ]. I think both curve to 10px 20px using 0px 10px and curve using 0px 10px to 10px 20px read reasonably. (I do prefer the current "to-first" order, but Dean's complaint about it not matching the SVG path order is reasonable.)

  • Not entirely sure which of Dean's complaints you're referring to here.

Well the remaining complaint is about spelling out cubic vs quadratic, but I think that "smooth quadratic curve to" can maybe start going over the verbosity borderline?

@tabatkins
Copy link
Member

I think Dean's complaint is pretty much resolved by the idea that only string-to-string animation uses the strict already-defined SVG animation pairing, while animating to/from a shape() function can use a looser variant that allows animating the curve types together. We should maintain the current nice usability from the degree choice being based on the arguments.

noamr added a commit to noamr/csswg-drafts that referenced this issue Jan 15, 2024
noamr added a commit to noamr/csswg-drafts that referenced this issue Jan 15, 2024
@noamr noamr added the Agenda+ label Jan 15, 2024
@noamr noamr removed the Agenda+ label Jan 31, 2024
@nt1m nt1m added the Agenda+ label Jul 28, 2024
@nt1m
Copy link
Member

nt1m commented Jul 28, 2024

@astearns @atanassov Can we close this at editors discretion and merge @noamr's PR or should we put this at the discussion for the CSSWG?

WebKit is currently implementing shape: https://bugs.webkit.org/show_bug.cgi?id=238371

@smfr
Copy link
Contributor

smfr commented Jul 30, 2024

Another comment: the from parameter to shape() is specified as a coordinate-pair, but that doesn't seem to allow keywords like top left which is natural for the "from" point, which is always absolute. Maybe a <position> here would be better?

Similarly, you can't use <position> arguments to the various commands; maybe allow, and if we encounter a <position> keyword it's an error if it's not a to point? This would enable very natural shapes like shape(from center top, line to right center, line to center bottom, line to left center, close).

@noamr
Copy link
Collaborator

noamr commented Jul 30, 2024

Another comment: the from parameter to shape() is specified as a coordinate-pair, but that doesn't seem to allow keywords like top left which is natural for the "from" point, which is always absolute. Maybe a <position> here would be better?

Similarly, you can't use <position> arguments to the various commands; maybe allow, and if we encounter a <position> keyword it's an error if it's not a to point? This would enable very natural shapes like shape(from center top, line to right center, line to center bottom, line to left center, close).

It's a good idea, Perhaps we can open an issue it can be added on top of the current spec without a backwards-compat problem? Otherwise we'll just pile more things on this list of small issues

@noamr
Copy link
Collaborator

noamr commented Jul 31, 2024

Proposed resolution: merge PR #9797:

@LeaVerou
Copy link
Member

Per animation, if we can change up the path interpolation rules, I'm all for letting lines/quadratics/cubics all interpolate together (as cubics), tho we'd have to choose where the control points are for the lines - I think they can go literally anywhere on the line without affecting anything? If so, I guess they should either both be in the middle, or maybe the first at the starting point and the second at the ending point?

For animation, let's please not forget #10195 which is kind of a no-brainer.

But I'd really like to maintain the ability for path() and shape() to interpolate together, so this would change the path() interpolation rules too.

Or we can just re-use path() and leave shape() open to hosting something better than SVG paths in the future: #10647

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-shapes-2] Minor comments on shape(), and agreed to the following:

  • RESOLVED: Accept Noam's PR (switch to `using`, allow reordering grammar)
The full IRC log of that discussion <TabAtkins> noamr: A few years ago there were some comments about shape() syntax, wanted to reoslve them
<TabAtkins> noamr: One is we use keyword `via` to describe intermediate points in beziers
<TabAtkins> noamr: Proposal is `using` instead, which amkes sense - you dont' go via the points, you just use them to create the curve
<TabAtkins> noamr: Other was to allow reordering the arguments, since they're distinguished by keyword
<smfr> q+
<TabAtkins> noamr: Third is to also allow <position> rather than just <coord-pair>
<TabAtkins> noamr: Currently we use `by` and `to` - `by` is relative segment, `to` is absolute. So idea is if you use `to` you can use <position>, like `to top left`
<TabAtkins> noamr: These all make sense to me
<TabAtkins> noamr: There's also an animation question, I think we can defer that
<astearns> ack smfr
<TabAtkins> smfr: I agree with all of these
<TabAtkins> smfr: ONe thing, if you say `to` or `by` the end and control points are either all relative or all absolute
<TabAtkins> smfr: It makes sense to me to be able to specify control points relative to the end point
<TabAtkins> smfr: That would complicate the syntax, and needs something for quadratics
<TabAtkins> smfr: But in general it makes sense to allow moving beziers and leave control points alone becuase they're relative to the endpoints
<TabAtkins> noamr: I think we can address that in a separate issue
<TabAtkins> smfr: Right, just want to make sure we're not stopping my thing from working in the future
<TabAtkins> noamr: Yeah, it wont'
<TabAtkins> astearns: I suggest we move the <position> issue to the new issue about smfr's issue
<TabAtkins> astearns: And for now just resolve on what's in the PR - reordering grammar, and using instead of via
<TabAtkins> fantasai: I think we try to use prepositions, and this is a verb, so I'm not super into `using`
<fantasai> I don't particularly love 'using', we usually use prepositions not verbs
<TabAtkins> smfr: Nobody ships this yet, we can change things. It might change with the relative-control-points thing.
<TabAtkins> fantasai: I'm fine with the PR for now.
<TabAtkins> RESOLVED: Accept Noam's PR (switch to `using`, allow reordering grammar)

@fantasai
Copy link
Collaborator

Following up on using being a bit awkward, maybe around?

@fantasai
Copy link
Collaborator

Filed #10649

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

Successfully merging a pull request may close this issue.

9 participants