[css-shapes-2] Add a shape() syntax, which accepts shape-segments equivalent to SVG path #5711

noamr · 2020-11-10T13:16:33Z

Add a draw() shape syntax, which accepts shape-segments equivalent to SVG path segments.

astearns · 2020-11-10T13:40:41Z

Thanks for writing up the proposal! It might be better to put this into css-shapes-2, as everything in css-shapes-1 is a bit further along (with an implementation and tests), and we should likely get that finished in the process and push new work to the next level.

@tabatkins can you take a look, since you have been responding in the issue?

noamr · 2020-11-10T13:47:05Z

Thanks for writing up the proposal! It might be better to put this into css-shapes-2, as everything in css-shapes-1 is a bit further along (with an implementation and tests), and we should likely get that finished in the process and push new work to the next level.

Of course, will move to css-shapes-2.

SVG path segments. Fixes w3c#5674

noamr · 2020-11-10T17:06:07Z

@tabatkins this is an initial go at this, I'm sure it will require further examples and so, but wanted to align on the syntax.

tabatkins · 2020-11-13T19:19:20Z

Looks nice! I like the by/to distinction, seems like a reasonable way to handle the rel/abs distinction.

The only thing I don't like is the horizontal/vertical keywords, because they're so long; it's shorter to just type line 0 X instead of vertical X. I think we can get away with just shortening those to h and v.

noamr · 2020-11-13T19:27:12Z

Looks nice! I like the by/to distinction, seems like a reasonable way to handle the rel/abs distinction.

The only thing I don't like is the horizontal/vertical keywords, because they're so long; it's shorter to just type line 0 X instead of vertical X. I think we can get away with just shortening those to h and v.

The thing is that with absolute coordinates, line to X 0 is not the same as horizontal to X.
I was thinking as an alternative to use the auto keyword (line to X auto and line to auto Y). WDYT?

Something about the one-character EBNF feels less readable to me, but maybe just for h/v it's OK.

tabatkins · 2020-11-13T19:38:43Z

Yeah, the one-character bit bothered me too, since the other commands aren't doing it (and dont' need to).

What about hline and vline, then?

noamr · 2020-11-13T19:49:55Z

Yeah, the one-character bit bothered me too, since the other commands aren't doing it (and dont' need to).

What about hline and vline, then?

LGTM!

Crissov · 2020-11-14T10:04:21Z

I recently amended my last comment in #5674 to mention alternative keywords: horizontal level and vertical plummet.

`l`	`L`	`h`	`H`	`v`	`V`
`line by <length> <length>`	`line to <length> <length>`	`horizontal by <length>`	`horizontal to <length>`	`vertical by <length>`	`vertical to <length>`
`line by <length> <length>`	`line to <length> <length>`	`level by <length>`	`level to <length>`	`plummet by <length>`	`plummet to <length>`
`line by <length> <length>`	`line to <length> <length>`	`line by <length> 0`	`line to <length> auto`	`line by 0 <length>`	`line to auto <length>`
`line by <length> <length>`	`line to <length> <length>`	`[left\|right] by <length>`	`[left\|right] to <length>`	`[top\|bottom] by <length>`	`[top\|bottom] to <length>`
`line by <length> <length>`	`line to <length> <length>`	`[left\|right] by <length>`	`[left\|right] to <length>`	`[up\|down] by <length>`	`[up\|down] to <length>`

noamr · 2020-11-14T11:36:37Z

I recently amended my last comment in #5674 to mention alternative keywords: horizontal level and vertical plummet.

Oh, I missed that! Not sure I like level and plummet. But up/down/left/right sounds appealing.

noamr · 2020-11-14T11:42:43Z

A couple of things I could use feedback about:

WDYT of my suggestion to use draw() as the function keyword? It corresponds with SVG's d and is more specific than shape() and differentiates from path().
Not sure about including an arc syntax at all. It's been a long outstanding issue in SVG to try to find a better syntax for it, I feel that maybe we're making that clunky syntax last longer than it should. Perhaps bezier curves are sufficient for the first version (bezier curves and arcs are interchangeable);
Thinking that we can do a better job with animations than how path() is animated: enable animating between lines/curves/arcs by converting all of them to bezier curves.

noamr · 2020-11-14T11:47:48Z

I recently amended my last comment in #5674 to mention alternative keywords: horizontal level and vertical plummet.

Oh, I missed that! Not sure I like level and plummet. But up/down/left/right sounds appealing.

How about the algebra terms rise and run? I think they would be the most accurate but hline and vline still seem like the most understandable terms.

Or even x and y :)

noamr · 2020-11-14T15:40:28Z

New revision uses hline / vline. I think they're the most straightforward.

As for arc, since arcs are complex-ish and there's lots of tutorials out there around the existing SVG syntax, I kept it consistent for now.

ByteEater-pl · 2020-11-16T10:02:46Z

Additional advantages of down and left are that they obviate the need to multiply by -1.

noamr · 2020-11-16T10:06:24Z

Additional advantages of down and left are that they obviate the need to multiply by -1.

But they would be confusing in absolute coordinates... e.g. left to 100% would actually be going right.

noamr · 2020-11-17T13:24:52Z

New PR based on @smfr's comments and others:

Using shape rather than draw
Allowing logical properties by defining a potentially-logical origin corner rather than an arbitrary starting point.

noamr · 2020-11-29T14:54:14Z

@tabatkins I think it's ready for a review round.

tabatkins

Looks good. Lots of small editorial nits, but it's not worth going over in this review; I need to do an editorial pass over this entire spec.

tabatkins · 2020-12-01T23:14:09Z

Wait, spoke too soon, didn't notice the "corner" design. We should discuss that separately and make sure we're happy with the design there; I suggest dropping it from the initial proposal and just defaulting to the "standard" physical coordinate system that paths already use.

tabatkins · 2020-12-01T23:14:25Z

Anyway, Agenda+ to discuss this addition to the spec.

noamr · 2020-12-02T06:34:48Z

Wait, spoke too soon, didn't notice the "corner" design. We should discuss that separately and make sure we're happy with the design there; I suggest dropping it from the initial proposal and just defaulting to the "standard" physical coordinate system that paths already use.

OK, I'll switch to physical - and will propose a separate discussion about logical coordinates in shapes.
Thanks!

faceless2 · 2020-12-02T08:57:22Z

Won't make the call to discuss, but this LGTM too. I think a straight drop-in replacement for the existing SVG syntax is a good idea. For that reason I also agree with keeping logical coordinates well away is a good idea, at least or now. I'm also not sure about "corner" - given you can replace corner top right with move to 100% 0, I don't think it adds anything. Maybe it was more useful when logical coordinates were an option; I'd consider dropping it until they're readded.

Otherwise, great! It's definitely better than string concatenation :-)

(oh, a quick addendum re. one of your earlier comments - arcs are not directly replaceable with beziers, as they are treated differently when used with SVG markers: a 270° arc with end markers has just the two, but drawing the same curve with beziers would result in more. So the concept needs to remain in the grammar. Yes the syntax is awkward, but it's well established, and using small|large and cw|ccw will certainly help)

noamr · 2020-12-02T10:16:42Z

Won't make the call to discuss, but this LGTM too. I think a straight drop-in replacement for the existing SVG syntax is a good idea. For that reason I also agree with keeping logical coordinates well away is a good idea, at least or now. I'm also not sure about "corner" - given you can replace corner top right with move to 100% 0, I don't think it adds anything. Maybe it was more useful when logical coordinates were an option; I'd consider dropping it until they're readded.

Yes, the corner thing makes sense only in the context of logical coordinates. Will wait with that.
As a Hebrew speaker I'd love to help out figure how shapes could become logical-property friendly (but no in this PR).

(oh, a quick addendum re. one of your earlier comments - arcs are not directly replaceable with beziers, as they are treated differently when used with SVG markers: a 270° arc with end markers has just the two, but drawing the same curve with beziers would result in more. So the concept needs to remain in the grammar. Yes the syntax is awkward, but it's well established, and using small|large and cw|ccw will certainly help)

Good point, I didn't knot at! Though SVG markers don't play a role in CSS shapes. Do they? In which case, arcs and curves are interchangeable.

faceless2 · 2020-12-02T11:29:31Z

Good point, I didn't knot at! Though SVG markers don't play a role in CSS shapes. Do they? In which case, arcs and curves are interchangeable.

True, currently. But (from w3c/svgwg#119 and w3c/svgwg#320) the "d" property uses the path() function (see also PR w3c/svgwg#374). Although it doesn't yet reference css-shapes, it seems to me that where path() applies, shape() would always apply too. And this looks pretty good to me:

<svg>
 <style>
   path {
     d: shape(move to 0 0 arc to 100% 100% cw small 90deg);
     marker-start: url(#marker);
     marker-end: url(#marker);
   }
 </style>
 <marker id="marker" markerWidth="10" markerHeight="10" refX="5" refY="5">
  <circle cx="5" cy="5" r="5"/>
 </marker>
 <path></path>
</svg>

Actually, that's another point - ideally the grammar for "arc" would allow the radius, arc-sweep arc-large and angle to be reordered, as this can be done without ambiguity. I think that means it should look like arc <by-to> <coordinate-pair> [<radius> || <arc-sweep> || <arc-large> || <angle>], but don't quote me on that. No doubt these are some of the editorial nits Tab will be picking.

noamr · 2020-12-02T11:32:22Z

Good point, I didn't knot at! Though SVG markers don't play a role in CSS shapes. Do they? In which case, arcs and curves are interchangeable.

True, currently. But (from w3c/svgwg#119 and w3c/svgwg#320) the "d" property uses the path() function (see also PR w3c/svgwg#374). Although it doesn't yet reference css-shapes, it seems to me that where path() applies, shape() would always apply too. And this looks pretty good to me:
<svg>
 <style>
   path {
     d: shape(move to 0 0 arc to 100% 100% cw small 90deg);
     marker-start: url(#marker);
     marker-end: url(#marker);
   }
 </style>
 <marker id="marker" markerWidth="10" markerHeight="10" refX="5" refY="5">
  <circle cx="5" cy="5" r="5"/>
 </marker>
 <path></path>
</svg>
Actually, that's another point - ideally the grammar for "arc" would allow the radius, arc-sweep arc-large and angle to be reordered, as this can be done without ambiguity. I think that means it should look like arc <by-to> <coordinate-pair> [<radius> || <arc-sweep> || <arc-large> || <angle>], but don't quote me on that. No doubt these are some of the editorial nits Tab will be picking.

Yes I think making arc parameters order-agnostic would be better. Let me revise.

noamr · 2020-12-03T07:52:15Z

Anyway, Agenda+ to discuss this addition to the spec.

Should I wait till it's discussed in the agenda or merge it in?

astearns · 2020-12-03T18:28:04Z

Anyway, Agenda+ to discuss this addition to the spec.

Should I wait till it's discussed in the agenda or merge it in?

Let’s wait for the discussion. There’s also the IPR commitment that needs to be made in some form or other. Are you talking to @svgeesus about that?

svgeesus · 2020-12-03T18:35:51Z

There’s also the IPR commitment that needs to be made in some form or other. Are you talking to @svgeesus about that?

I asked for the IPR commitment, and it has been made. I don't understand why the flag hasn't cleared. Ah! I had to press the "revalidate" button. All good now.

css-shapes-2/Overview.bs

fantasai · 2020-12-09T16:38:31Z

Overall looks great!!!

You need to also add yourself to the acknowledgements, though. =)

noamr · 2020-12-09T16:40:29Z

Overall looks great!!!

Thanks!

You need to also add yourself to the acknowledgements, though. =)

How do I do that?

fantasai · 2020-12-09T16:41:57Z

Good point. Seems there isn't one yet. Need to add one, and add you to it. :) I suppose we can make that a separate issue.

This reverts commit ec435e7.

grorg

Typo Corresponds.

grorg · 2021-01-07T00:21:54Z

css-shapes-2/Overview.bs

+			If it is not set, the automatic value is used.
+		<dt>close
+		<dd>
+			Corrsepponds to a <a href="https://www.w3.org/TR/SVG/paths.html#PathDataClosePathCommand">closepath</a> command.


Typo. Corresponds.

grorg · 2021-01-07T00:22:49Z

css-shapes-2/Overview.bs

+		<dt><dfn><<hv-line-command>></dfn> = [hline | vline] <<by-to>> <<length-percentage>>
+		<dd>
+			Corresponds to a horizontal or vertical <a href="https://www.w3.org/TR/SVG/paths.html#PathDataLinetoCommands">lineto</a> command.
+			<dt><dfn><<curve-command>></dfn> = curve <<by-to>> <<coordinate-pair>> via <<coordinate-pair>>{1,2}


would prefer using over via. The curve does not go via the control points.

Also, I wonder if the end point should come after the control points.

Lastly (hopefully), additional spec text should point out that an animation of the shape would work the same as SVG, but that curve is a bit different since the type of curve is defined by the number of parameters (rather than an identifier like SVG).

grorg · 2021-01-07T00:28:02Z

Ha. @tabatkins merged before I pressed the "Submit Review" button.

tabatkins · 2021-01-07T01:22:25Z

No worries, I'm gonna do a comprehensive edit pass over the spec anyway, so I'll make sure your comments are addressed.

astearns requested a review from tabatkins November 10, 2020 13:40

noamr added 2 commits November 10, 2020 19:04

Add a draw() shape syntax, which accepts shape-segments equivalent to

3002754

SVG path segments. Fixes w3c#5674

Move to css-shapes-2

7998c5e

noamr force-pushed the draw-shape branch from 67fcc4a to 7998c5e Compare November 10, 2020 17:05

noamr changed the title ~~[css-shapes-1] WIP: Add a draw() shape syntax, which accepts shape-segments equivalent to~~ [css-shapes-2] WIP: Add a draw() shape syntax, which accepts shape-segments equivalent to Nov 11, 2020

noamr added 2 commits November 14, 2020 17:36

Amendments

d916425

Amendments

bffdc46

Switched back to shape, and allow logical coordinates

54bdc29

noamr mentioned this pull request Nov 29, 2020

[css-shapes] Allow CSS grammar for path shapes #5674

Closed

tabatkins approved these changes Dec 1, 2020

View reviewed changes

Ditch logical coordinates for now

d48d769

Make arc parameters order independent

2e8b244

noamr changed the title ~~[css-shapes-2] WIP: Add a draw() shape syntax, which accepts shape-segments equivalent to~~ [css-shapes-2] Add a shape() syntax, which accepts shape-segments equivalent to SVG path Dec 2, 2020

fantasai reviewed Dec 9, 2020

View reviewed changes

css-shapes-2/Overview.bs Outdated Show resolved Hide resolved

fantasai reviewed Dec 9, 2020

View reviewed changes

css-shapes-2/Overview.bs Show resolved Hide resolved

fantasai reviewed Dec 9, 2020

View reviewed changes

css-shapes-2/Overview.bs Outdated Show resolved Hide resolved

fantasai mentioned this pull request Dec 9, 2020

[css-shapes-2] Add Acknowledgements section #5773

Open

Minor spec changes

5f4b6d5

fantasai approved these changes Dec 9, 2020

View reviewed changes

noamr added 2 commits December 9, 2020 20:43

Allow <<position>>

ec435e7

Revert "Allow <<position>>"

e2a14d6

This reverts commit ec435e7.

grorg reviewed Jan 7, 2021

View reviewed changes

tabatkins merged commit e8404d1 into w3c:master Jan 7, 2021

grorg requested changes Jan 7, 2021

View reviewed changes

grorg mentioned this pull request Jan 7, 2021

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

Open

noamr deleted the draw-shape branch January 7, 2021 05:46

[css-shapes-2] Add a shape() syntax, which accepts shape-segments equivalent to SVG path #5711

[css-shapes-2] Add a shape() syntax, which accepts shape-segments equivalent to SVG path #5711

Conversation

noamr commented Nov 10, 2020

astearns commented Nov 10, 2020

noamr commented Nov 10, 2020

noamr commented Nov 10, 2020

tabatkins commented Nov 13, 2020

noamr commented Nov 13, 2020

tabatkins commented Nov 13, 2020

noamr commented Nov 13, 2020

Crissov commented Nov 14, 2020 • edited Loading

noamr commented Nov 14, 2020 • edited Loading

noamr commented Nov 14, 2020

noamr commented Nov 14, 2020 • edited Loading

noamr commented Nov 14, 2020

ByteEater-pl commented Nov 16, 2020

noamr commented Nov 16, 2020

noamr commented Nov 17, 2020

noamr commented Nov 29, 2020

tabatkins left a comment

Choose a reason for hiding this comment

tabatkins commented Dec 1, 2020

tabatkins commented Dec 1, 2020

noamr commented Dec 2, 2020

faceless2 commented Dec 2, 2020

noamr commented Dec 2, 2020

faceless2 commented Dec 2, 2020

noamr commented Dec 2, 2020

noamr commented Dec 3, 2020 • edited Loading

astearns commented Dec 3, 2020

svgeesus commented Dec 3, 2020

fantasai commented Dec 9, 2020 • edited Loading

noamr commented Dec 9, 2020

fantasai commented Dec 9, 2020 • edited Loading

grorg left a comment

Choose a reason for hiding this comment

grorg Jan 7, 2021

Choose a reason for hiding this comment

grorg Jan 7, 2021

Choose a reason for hiding this comment

grorg Jan 7, 2021

Choose a reason for hiding this comment

grorg Jan 7, 2021

Choose a reason for hiding this comment

grorg commented Jan 7, 2021

tabatkins commented Jan 7, 2021

Crissov commented Nov 14, 2020 •

edited

Loading

noamr commented Nov 14, 2020 •

edited

Loading

noamr commented Nov 14, 2020 •

edited

Loading

noamr commented Dec 3, 2020 •

edited

Loading

fantasai commented Dec 9, 2020 •

edited

Loading

fantasai commented Dec 9, 2020 •

edited

Loading