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

Open
grorg opened this issue Jan 7, 2021 · 12 comments · May be fixed by #9797
Open

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

grorg opened this issue Jan 7, 2021 · 12 comments · May be 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 noamr linked a pull request Jan 15, 2024 that will close this issue
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
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.

4 participants