Avoid hardcoding a tolerance in Vello API, instead require the user to provide it

[DJMcNab]

This was one of the key points raised for follow-up in #1360, to make this API harder to misuse.

The implementation strategy is to have a new ExactPathElements trait, which is implemented for Shapes which don't require approximation.
To allow shapes which do require a tolerance to be used without allocation, the within function can be used, which creates an exact shape from an inexact shape within the provided tolerance.

This ensures that the common-case of rectangles and similar is ergonomic, whilst the case where tolerance is needed is handled by the user. This still completely avoids per-path allocations.

The ExactPathElements is based on code by @tomcur from #vello > Determining correct `Shape` tolerance. There are reasonable arguments that ExactPathElements actually belongs in Kurbo, but I also think that, if this is the direction we decide to go, migrating that code into Kurbo would (relatively) straightforward.

[taj-p]

Looks much better!! 🎉

The only comment is the use of the Shape super trait.

[taj-p]

Edited by @DJMcNab to make suggestion take up less space.

Already applied suggestion

Suggested change

	/// Use an approximated shape where an [`ExactPathElements`] is required, by approximating it to
	/// within the given tolerance.
	///
	/// This is useful for drawing shapes such as [`Circle`](kurbo::Circle)s and [`RoundedRect`](kurbo::RoundedRect)s
	/// to renderers which use Vello API.
	///
	/// As the user of this function, you are responsible for determining the correct tolerance for your use case.
	/// A reasonable approach might be to select a tolerance which allows scaling up ("zooming in") by 4x to be
	/// within your intended tolernace bound, then recomputing the [`Scene`](crate::Scene) from your base data
	/// representation once that is exceeded.
	/// If you know that the shape will not be scaled, you can use [`UNSCALED_TOLERANCE`].
	/// The resulting path will be within 1/10th of a pixel of the actual shape, which is a negligible
	/// difference for rendering.
	/// Use an approximated shape where an [`ExactPathElements`] is required, by approximating it to
	/// within the given tolerance.
	///
	/// *WARNING*: Unlike [`ExactPathElements`], which will produce correct renderings for any scale, this
	/// approximation is only valid for a fixed range of transforms.
	///
	/// As the user of this function, you are responsible for determining the correct tolerance for your use case.
	/// A reasonable approach might be to select a tolerance which allows scaling up ("zooming in") by 4x (for example, 0.00001) to be
	/// within your intended tolerance bound, then recomputing the [`Scene`](crate::Scene) from your base data
	/// representation once that is exceeded.
	/// If you know that the shape will not be scaled, you can use [`UNSCALED_TOLERANCE`].
	/// The resulting path will be within 1/10th of a pixel of the actual shape, which is a negligible
	/// difference for rendering.
	///
	/// This is useful for drawing shapes such as [`Circle`](kurbo::Circle)s and [`RoundedRect`](kurbo::RoundedRect)s.

[DJMcNab]

I have one concern here, which is the 0.00001 bound. I believe that would support a $$10^4$$ times zoom, rather than a 4x zoom, right?

[DJMcNab]

I've removed the 0.00001 for now; we can always restore it in a follow-up.

[taj-p]

It is a little confusing that we take an owned ExactPathElements instead of a borrowed type. For hybrid and single threaded CPU, we only require borrowed data. For multithreading, we still only require borrowed data IIUC since we copy the path into the allocation group.

Should this be &impl ExactPathElements to better reflect that only borrowed data is required from the caller? I believe Vello Classic took this approach - is there context for why impl ExactPathElements is better?

[DJMcNab]

I thought I was copying Vello Classic, but a revisit of the docs for vello::Scene shows that my memory was mistaken. I'll change this; I think it makes as much sense to do it in this PR.

[DJMcNab]

In applying this change, it raises a small ergonomics question; for the vast majority of non-BezPath shapes, the extra & isn't doing anything meaningful here. A conceivable compromise here would be impl ExactPathElements + Copy, which would not require the & for shapes where it isn't consuming, but would force it for e.g. BezPath.

In this PR, I've changed it to be & for simplicity/to avoid blocking the rest of this work on that question. But it's something I'll consider for a follow-up.

[taj-p]

Nice work!!! 🎉

[taj-p]

We probably want to just pass the iterator so we don't monomorphise across every trait impl

[DJMcNab]

I believe it would still in most cases be a separate impl for each Shape, as each Shape has their own iterator type (unless I guess we used something like &mut dyn Iterator?)

[nicoburns]

Which shapes cannot implement ExactPathElements?

Is it primarily those based on circular geometry? (Arc, Circle, Ellipse, etc?). If so, might it make sense to create an extended version of PathEl that is able to encode those losslessly (i.e. by adding an ArcTo variant to the enum) and then use that in Scene (we could add an extended_path_elements method or similar directly to the Shape trait). Then the conversion to BezPath could be deferred into the renderers that would have context about absolute scale, and would be able to determine a sensible value for the tolerance.

Or are (infinitely many) other shape types that we may need to support such that this wouldn't work?

[tomcur]

Which shapes cannot implement ExactPathElements?

Is it primarily those based on circular geometry? (Arc, Circle, Ellipse, etc?).

That's right. More specifically, only shapes that can be represented by cubic polynomial segments (including line and quadratic segments) can be ExactPathElements (given the current path element kinds).

If so, might it make sense to create an extended version of PathEl that is able to encode those losslessly (i.e. by adding an ArcTo variant to the enum)

I don't think so. The issue with something like a circular ArcTo is that it cannot be preserved under affine transforms. Béziers (including lines) are nice, because the affine transform of a Bézier path is just again a regular Bézier with simply the original control points transformed.

Edit: with your edits I now understand you meant keeping those extended path elements around until such time the actual transforming is done. Something like that is possible, but at that point may be quite similar to just keeping an enumeration of shapes or a dyn Shape around. For extending path elements to allow arcs while still doing lossless transformation anywhere in the pipeline, an elliptic arc path element could work, as it's also closed under affine transformation.

[nicoburns]

Edit: with your edits I now understand you meant keeping those extended path elements around until such time the actual transforming is done. Something like that is possible, but at that point may be quite similar to just keeping an enumeration of shapes or a dyn Shape around. For extending path elements to allow arcs while still doing lossless transformation anywhere in the pipeline, an elliptic arc path element could work, as it's also closed under affine transformation.

Yeah, that's the idea.

Ellitical arcs (and/or perhaps even hyper-elliptical arcs - people love their squircles) would make sense to me.

It would be similar to keeping around Shape, but a more practical way of implementing it I think. Storing dyn Shape would require allocating a Box<dyn Shape> for every single shape. And I guess the idea is that it does still somewhat simplify an arbitrary Shape down into simpler primitives (e.g. this would support RoundedRectangle without having to have a special case for it.

[tomcur]

Skia has e.g. SkPath::conicTo.