Align path validity with SVG: only the first subpath requires a MoveTo

[tomcur]

Match SVG semantics:

A path data segment (if there is one) must begin with a "moveto" command.

And:

If a "closepath" is followed immediately by a "moveto", then the "moveto" identifies the start point of the next subpath. If a "closepath" is followed immediately by any other command, then the next subpath starts at the same initial point as the current subpath.

[dominikh]

This will likely lead to additional follow-up changes to add support for the implied MoveTo after ClosePath. I've noticed the following so far:

• kurbo/kurbo/src/simplify.rs

  Lines 335 to 340 in d3398c5

  	PathEl::ClosePath => {
  	state.flush(accuracy, options);
  	state.result.close_path();
  	state.needs_moveto = true;
  	last_seg = None;
  	continue;

• kurbo/kurbo/src/bezpath.rs

  Lines 330 to 336 in d3398c5

  	let last = match self.0[ix - 1] {
  	PathEl::MoveTo(p) => p,
  	PathEl::LineTo(p) => p,
  	PathEl::QuadTo(_, p2) => p2,
  	PathEl::CurveTo(_, _, p3) => p3,
  	PathEl::ClosePath => return None,
  	};

• kurbo/kurbo/src/bezpath.rs

  Lines 701 to 703 in d3398c5

  	PathEl::ClosePath => {
  	last_pt = None;
  	callback(PathEl::ClosePath);

It looks like we weren't very consistent in supporting this in the past.

[tomcur]

Subpaths started after a Close without a MoveTo need to know the where the previous subpath ends (or equivalently, where that subpath started). This perhaps would be the main argument in favor of not aligning with SVG, as always requiring an explicit MoveTo would allow taking &[PathEl] slices from the BezPath and having them be valid paths without further context.

Note that if we want such semantics, we may still want to update code (and improve docs) as Kurbo currently is not consistent and does different things in different places.

BezPath::segments does the SVG-aligned thing:

kurbo/kurbo/src/bezpath.rs

Lines 853 to 859 in 0c31f87

	PathEl::ClosePath => {
	if *last != *start {
	PathSeg::Line(Line::new(mem::replace(last, *start), *start))
	} else {
	continue;
	}
	}

BezPath::get_seg returns None (even though BezPath::segments does return the segment):

kurbo/kurbo/src/bezpath.rs

Lines 330 to 336 in 0c31f87

	let last = match self.0[ix - 1] {
	PathEl::MoveTo(p) => p,
	PathEl::LineTo(p) => p,
	PathEl::QuadTo(_, p2) => p2,
	PathEl::CurveTo(_, _, p3) => p3,
	PathEl::ClosePath => return None,
	};

BezPath::moments also does the SVG-aligned thing:

kurbo/kurbo/src/moments.rs

Lines 554 to 559 in 0c31f87

	PathEl::ClosePath => {
	if cur != start_pt {
	moments.handle_line(cur, start_pt);
	cur = start_pt;
	}
	}

BezPath::current_position requires all subpaths to start with MoveTo (otherwise it treats the first element as if it were a MoveTo:

kurbo/kurbo/src/bezpath.rs

Lines 402 to 422 in 0c31f87

	/// Returns current position in the path, if path is not empty.
	///
	/// This is different from calling [`PathEl::end_point`] on the last entry of [`BezPath::elements`]:
	/// this method handles [`PathEl::ClosePath`],
	/// by finding the first point of our last subpath, hence the time complexity is O(n).
	pub fn current_position(&self) -> Option<Point> {
	match self.0.last()? {
	PathEl::MoveTo(p) => Some(*p),
	PathEl::LineTo(p1) => Some(*p1),
	PathEl::QuadTo(_, p2) => Some(*p2),
	PathEl::CurveTo(_, _, p3) => Some(*p3),
	PathEl::ClosePath => self
	.elements()
	.iter()
	.rev()
	.skip(1)
	.take_while(|el| !matches!(el, PathEl::ClosePath))
	.last()
	.and_then(|el| el.end_point()),
	}
	}

Shape for &'a [PathEl] similarly treats the first drawing command in case of a missing MoveTos as a MoveTo:

kurbo/kurbo/src/bezpath.rs

Lines 1444 to 1448 in 0c31f87

	/// Implements [`Shape`] for a slice of [`PathEl`], provided that the first element of the slice is
	/// not a `PathEl::ClosePath`. If it is, several of these functions will panic.
	///
	/// If the slice starts with `LineTo`, `QuadTo`, or `CurveTo`, it will be treated as a `MoveTo`.
	impl<'a> Shape for &'a [PathEl] {

[tomcur]

I believe things are now consistent (or at least mostly consistent). There actually is an argument for keeping the requirement that subpaths start with MoveTo, as then each &[PathEl] subpath slice is itself a valid path, but that would still require changes: see #565 (comment).

I can see two possible behaviors in case of missing MoveTos that requires no further context of previous subpaths:

• treat the first drawing command as if it were a MoveTo (i.e., just using its end point); or
• implicitly start from (0, 0).

E.g., Skia does the latter https://api.skia.org/classSkPath.html#a27c6505cd16db954d6f0268e2de5d87f. On current main in some places we do the former or we already do the SVG-aligned thing of using the previous subpath's end point (see #565 (comment)).

[dominikh]

I think that being able to refer to subpaths without additional context is more valuable than the flexibility of omitting some MoveTos. Plus, paths are quite a bit easier to use if they don't require as much state tracking by the consumer.
SVG's model isn't necessarily an example of good design in this regard, anyway, considering it tries to simultaneously optimize the text format for writing documents by hand and making paths as succinct and unreadable as possible to save some bytes.

treat the first drawing command as if it were a MoveTo (i.e., just using its end point)

IMO that'll lead to annoying to debug issues in user code.

In addition to slicing paths, we should also consider the semantics of concatenating paths. But I am not sure that any model that permits omitting MoveTo makes intuitive sense in that context. However, starting at the previous subpath's start point seems to rank rather low on expectedness, IMHO.

[tomcur]

I think that being able to refer to subpaths without additional context is more valuable than the flexibility of omitting some MoveTos.

In a vacuum I completely agree with this statement.

SVG's model isn't necessarily an example of good design in this regard

While I agree, I think following SVG's model should be the "default" unless arguments against are strong enough (which they may be in this case).

But I am not sure that any model that permits omitting MoveTo makes intuitive sense in that context. However, starting at the previous subpath's start point seems to rank rather low on expectedness, IMHO.

Another way to think about SVG's model is that you always continue from the current point: a Close is a line from the current point to the subpath's start, that start point becoming the new current point, and the next draw command will continue from that start point.