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

sparse_strips/vello_api/src/exact.rs

@@ -0,0 +1,141 @@
+// Copyright 2025 the Vello Authors
+// SPDX-License-Identifier: Apache-2.0 OR MIT
+
+//! Turning shapes into Bézier paths without approximation.
+
+use kurbo::{BezPath, CubicBez, Line, QuadBez, Rect, Segments, Shape, Triangle, segments};
+use peniko::kurbo::{self, PathEl};
+
+/// A generic trait for shapes that can be mapped exactly to Bézier path elements (i.e., without
+/// approximation).
+///
+/// The methods on [`PaintScene`](crate::PaintScene) use this trait ensure that consistent behaviour
+/// is maintained when Vello API is used to render content which might be rescaled.
+///
+/// This is implemented for [`Shape`]s from Kurbo that can be exactly mapped to Bézier path elements.
+/// To convert a [`Shape`] which requires approximation (such as [`Circle`](kurbo::Circle) or
+/// [`RoundedRect`](kurbo::RoundedRect)), you can use [`within`].
+/// This however requires you to provide the tolerance.
+/// See the docs of `within` for more details.
+///
+/// It is a requirement of this trait that [`Shape::path_elements`] returns the same iterator
+/// as [`ExactPathElements::exact_path_elements`] for any provided tolerance value.
+pub trait ExactPathElements {
+    /// The iterator returned by the [`Self::exact_path_elements`] method.
+    ///
+    /// In many cases, this will be the same iterator as [`Shape::path_elements`]
+    type ExactPathElementsIter<'iter>: Iterator<Item = PathEl> + 'iter
+    where
+        Self: 'iter;
+
+    /// Returns an iterator over this shape expressed as exact [`PathEl`]s;
+    /// that is, as exact Bézier path _elements_.
+    ///
+    /// These path elements are exact, in the sense that no approximation is required
+    /// to calculate them. This is not possible for all shapes, but is possible for all
+    /// finite polygons and other piecewise-cubic parametric curves. Some shapes will
+    /// need to be approximated, which [`Shape::path_elements`] does instead.
+    /// The [`within`] helper function allows the approximated shape to be used
+    /// where an exact value is required, by providing the tolerance used.
+    ///
+    /// In many cases, shapes are able to iterate their elements without
+    /// allocating; however creating a [`BezPath`] object always allocates.
+    /// If you need an owned [`BezPath`] you can use [`BezPath::from_iter`] (or
+    /// [`Iterator::collect`]).
+    fn exact_path_elements(&self) -> Self::ExactPathElementsIter<'_>;
+
+    /// Returns an iterator over this shape expressed as exact Bézier path
+    /// _segments_ ([`PathSeg`]s).
+    ///
+    /// The allocation behaviour is the same as for [`ExactPathElements::exact_path_elements`].
+    ///
+    /// [`PathSeg`]: kurbo::PathSeg
+    #[inline]
+    fn exact_path_segments(&self) -> Segments<Self::ExactPathElementsIter<'_>> {
+        segments(self.exact_path_elements())
+    }
+}
+
+impl<T: ExactPathElements> ExactPathElements for &T {
+    type ExactPathElementsIter<'iter>
+        = T::ExactPathElementsIter<'iter>
+    where
+        Self: 'iter;
+
+    #[inline]
+    fn exact_path_elements(&self) -> Self::ExactPathElementsIter<'_> {
+        (*self).exact_path_elements()
+    }
+}
+
+/// 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;
+/// you should evaluate the correct value yourself) and remaining within your intended tolerance bound.
+/// If the user zoomed past that limit, you would 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.
+// TODO: Provide as an extension trait?
+pub fn within<S: Shape>(shape: S, tolerance: f64) -> WithTolerance<S> {
+    WithTolerance { shape, tolerance }
+}
+
+/// The type used to implement [`within`].
+#[derive(Debug, Clone, Copy)]
+pub struct WithTolerance<S: Shape> {
+    /// The shape to be approximated.
+    pub shape: S,
+    /// The tolerance to use when approximating it.
+    pub tolerance: f64,
+}
+
+impl<S: Shape> ExactPathElements for WithTolerance<S> {
+    type ExactPathElementsIter<'iter>
+        = S::PathElementsIter<'iter>
+    where
+        S: 'iter;
+    fn exact_path_elements(&self) -> Self::ExactPathElementsIter<'_> {
+        self.shape.path_elements(self.tolerance)
+    }
+}
+
+/// The recommended tolerance for approximating shapes which won't be rescaled as Bezier paths.
+///
+/// This can be used as the tolerance parameter to [`within`], when you know that
+/// a shape will be drawn at its natural size, without scaling or skew.
+pub const UNSCALED_TOLERANCE: f64 = 0.1;
+
+// TODO: Provide an `fn ideal_tolerance(transform: Affine) -> f64`
+
+/// Implement `ExactPathElements` for an existing [`Shape`], which we know will not be approximated in Kurbo.
+// In theory, the impl is the wrong way around; instead the `Shape` impl should be in terms of `ExactPathElements`.
+macro_rules! passthrough {
+    ($ty: ty) => {
+        impl ExactPathElements for $ty {
+            type ExactPathElementsIter<'iter>
+                = <$ty as Shape>::PathElementsIter<'iter>
+            where
+                $ty: 'iter;
+
+            fn exact_path_elements(&self) -> Self::ExactPathElementsIter<'_> {
+                // We use a tolerance of zero here because we know this to be exact.
+                self.path_elements(0.)
+            }
+        }
+    };
+}
+
+passthrough!(BezPath);
+passthrough!(CubicBez);
+passthrough!(Line);
+passthrough!(QuadBez);
+passthrough!(Rect);
+passthrough!(Triangle);

sparse_strips/vello_api/src/lib.rs

@@ -137,6 +137,7 @@ extern crate alloc;
 
 mod painter;
 
+pub mod exact;
 pub mod paths;
 pub mod scene;
 pub mod texture;

sparse_strips/vello_api/src/painter.rs

@@ -3,9 +3,10 @@
 
 use core::any::Any;
 
-use peniko::kurbo::{Affine, BezPath, Rect, Shape, Stroke};
+use peniko::kurbo::{Affine, BezPath, Rect, Stroke};
 use peniko::{BlendMode, Brush, Color, Fill, ImageBrush};
 
+use crate::exact::ExactPathElements;
 use crate::scene::Scene;
 use crate::texture::TextureId;
 
@@ -86,13 +87,18 @@ pub trait PaintScene: Any {
     /// Both `shape` and the current brush will be transformed using the 2d affine `transform`.
     /// See the documentation on [`Fill`]'s variants for details on when you might choose a given fill rule.
     // It would be really nice to have images of nested paths here, to explain winding numbers.
-    fn fill_path(&mut self, transform: Affine, fill_rule: Fill, path: impl Shape);
+    fn fill_path(&mut self, transform: Affine, fill_rule: Fill, path: &impl ExactPathElements);
 
     /// Stroke along `path` with the current brush, following the given stroke parameters.
     ///
     /// Both the stroked area and the current brush will be transformed using the 2d affine `transform`.
     /// Dashes configured in the stroke parameter will be expanded.
-    fn stroke_path(&mut self, transform: Affine, stroke_params: &Stroke, path: impl Shape);
+    fn stroke_path(
+        &mut self,
+        transform: Affine,
+        stroke_params: &Stroke,
+        path: &impl ExactPathElements,
+    );
 
     /// Set the current brush to `brush`.
     ///
@@ -174,7 +180,7 @@ pub trait PaintScene: Any {
         let kernel_size = (2.5 * std_dev) as f64;
 
         let shape: Rect = rect.inflate(kernel_size, kernel_size);
-        self.fill_path(transform, Fill::EvenOdd, shape);
+        self.fill_path(transform, Fill::EvenOdd, &shape);
     }
 
     /// Pushes a new layer clipped by the specified shape (if provided) and composed with
@@ -198,7 +204,7 @@ pub trait PaintScene: Any {
     fn push_layer(
         &mut self,
         clip_transform: Affine,
-        clip_path: Option<impl Shape>,
+        clip_path: Option<&impl ExactPathElements>,
         blend_mode: Option<BlendMode>,
         opacity: Option<f32>,
     );
@@ -212,7 +218,7 @@ pub trait PaintScene: Any {
     ///
     /// **However, the transforms are *not* saved or modified by the layer stack.**
     /// That is, the `transform` argument to this function only applies a transform to the `clip` shape.
-    fn push_clip_layer(&mut self, clip_transform: Affine, path: impl Shape) {
+    fn push_clip_layer(&mut self, clip_transform: Affine, path: &impl ExactPathElements) {
         self.push_layer(clip_transform, Some(path), None, None);
     }
 
@@ -224,7 +230,7 @@ pub trait PaintScene: Any {
         self.push_layer(
             Affine::IDENTITY,
             // This needing to be turbofished shows a real footgun with the proposed "push_layer" API here.
-            None::<BezPath>,
+            None::<&BezPath>,
             Some(blend_mode),
             None,
         );
@@ -239,7 +245,7 @@ pub trait PaintScene: Any {
     /// Every drawing command after this call will be composed as described
     /// until the layer is [popped](PaintScene::pop_layer).
     fn push_opacity_layer(&mut self, opacity: f32) {
-        self.push_layer(Affine::IDENTITY, None::<BezPath>, None, Some(opacity));
+        self.push_layer(Affine::IDENTITY, None::<&BezPath>, None, Some(opacity));
     }
 
     /// Pop the most recently pushed layer.

sparse_strips/vello_api/src/paths.rs

@@ -15,10 +15,9 @@
 //! for each path).
 
 use alloc::vec::Vec;
-use peniko::{
-    Style,
-    kurbo::{PathEl, Shape},
-};
+use peniko::{Style, kurbo::PathEl};
+
+use crate::exact::ExactPathElements;
 
 /// The id for a single path within a given [`PathSet`].
 /// This is an index into the [`meta`](PathSet::meta) field.
@@ -87,13 +86,13 @@ impl PathSet {
     /// See the docs on [`append`](PathSet::append) for how this changes when path sets are combined.
     ///
     /// This method is generally only expected to be used by [`Scene`](crate::Scene).
-    pub fn prepare_shape(&mut self, shape: &impl Shape, style: impl Into<Style>) -> PathId {
+    pub fn prepare_shape(
+        &mut self,
+        shape: &impl ExactPathElements,
+        style: impl Into<Style>,
+    ) -> PathId {
         let start_index = self.elements.len();
-        // TODO: We hard-code this tolerance to be 0.1, as every other call does so.
-        // We should maybe change that at some point?
-        // https://xi.zulipchat.com/#narrow/channel/197075-vello/topic/Determining.20correct.20.60Shape.60.20tolerance/with/565793178
-        // If you need a different tolerance, you should pass in a `BezPath` (i.e. using Shape::to_path with the tolerance you require)
-        self.elements.extend(shape.path_elements(0.1));
+        self.elements.extend(shape.exact_path_elements());
         let meta_index = self.meta.len();
         self.meta.push(PathMeta {
             start_index,

sparse_strips/vello_api/src/scene.rs

@@ -17,6 +17,7 @@ use peniko::{
 
 use crate::{
     PaintScene, StandardBrush,
+    exact::ExactPathElements,
     paths::{PathId, PathSet},
 };
 
@@ -230,7 +231,12 @@ impl PaintScene for Scene {
         Ok(())
     }
 
-    fn fill_path(&mut self, transform: Affine, fill_rule: peniko::Fill, path: impl kurbo::Shape) {
+    fn fill_path(
+        &mut self,
+        transform: Affine,
+        fill_rule: peniko::Fill,
+        path: &impl ExactPathElements,
+    ) {
         let idx = self.paths.prepare_shape(&path, fill_rule);
         self.commands.push(RenderCommand::DrawPath(transform, idx));
     }
@@ -239,7 +245,7 @@ impl PaintScene for Scene {
         &mut self,
         transform: Affine,
         stroke_params: &kurbo::Stroke,
-        path: impl kurbo::Shape,
+        path: &impl ExactPathElements,
     ) {
         let idx = self.paths.prepare_shape(&path, stroke_params.clone());
         self.commands.push(RenderCommand::DrawPath(transform, idx));
@@ -273,7 +279,7 @@ impl PaintScene for Scene {
     fn push_layer(
         &mut self,
         clip_transform: Affine,
-        clip_path: Option<impl kurbo::Shape>,
+        clip_path: Option<&impl ExactPathElements>,
         blend_mode: Option<BlendMode>,
         opacity: Option<f32>,
         // mask: Option<Mask>,

sparse_strips/vello_cpu/src/api.rs

@@ -12,12 +12,13 @@
 
 use vello_api::{
     PaintScene, Scene,
+    exact::ExactPathElements,
     peniko::Style,
     scene::{RenderCommand, extract_integer_translation},
     texture::TextureId,
 };
 use vello_common::{
-    kurbo::{self, Affine, BezPath, Shape},
+    kurbo::{self, Affine, BezPath},
     paint::{ImageId, ImageSource},
     peniko::{BlendMode, Brush, Color, Fill, ImageBrush},
 };
@@ -86,6 +87,7 @@ impl PaintScene for CPUScenePainter {
                         .set_transform(push_layer_command.clip_transform);
                     let clip_path = if let Some(path_id) = push_layer_command.clip_path {
                         let path = &input_paths.meta[usize::try_from(path_id.0).unwrap()];
+                        // TODO: Also correctly support the case where the meta has a `Style::Stroke`
                         let path_end = &input_paths
                             .meta
                             .get(usize::try_from(path_id.0).unwrap() + 1)
@@ -133,17 +135,27 @@ impl PaintScene for CPUScenePainter {
         Ok(())
     }
 
-    fn fill_path(&mut self, transform: Affine, fill_rule: Fill, path: impl Shape) {
+    fn fill_path(&mut self, transform: Affine, fill_rule: Fill, path: &impl ExactPathElements) {
         self.render_context.set_transform(transform);
         self.render_context.set_fill_rule(fill_rule);
-        // TODO: Tweak inner `fill_path` API to either take a `Shape` or an &[PathEl]
-        self.render_context.fill_path(&path.into_path(0.1));
+        // However, using `to_path` avoids allocation in some cases.
+        // TODO: Tweak inner API to accept an `ExactPathElements` (or at least, the resultant iterator)
+        // That would avoid the superfluous allocation here.
+        self.render_context
+            .fill_path(&path.exact_path_elements().collect());
     }
 
-    fn stroke_path(&mut self, transform: Affine, stroke_params: &kurbo::Stroke, path: impl Shape) {
+    fn stroke_path(
+        &mut self,
+        transform: Affine,
+        stroke_params: &kurbo::Stroke,
+        path: &impl ExactPathElements,
+    ) {
         self.render_context.set_transform(transform);
         self.render_context.set_stroke(stroke_params.clone());
-        self.render_context.stroke_path(&path.into_path(0.1));
+        // TODO: As in `fill_path`
+        self.render_context
+            .stroke_path(&path.exact_path_elements().collect());
     }
 
     fn set_brush(
@@ -198,7 +210,7 @@ impl PaintScene for CPUScenePainter {
     fn push_layer(
         &mut self,
         clip_transform: Affine,
-        clip_path: Option<impl Shape>,
+        clip_path: Option<&impl ExactPathElements>,
         blend_mode: Option<BlendMode>,
         opacity: Option<f32>,
         // mask: Option<Mask>,
@@ -208,21 +220,23 @@ impl PaintScene for CPUScenePainter {
         self.render_context.set_fill_rule(Fill::NonZero);
         self.render_context.set_transform(clip_transform);
         self.render_context.push_layer(
-            clip_path.map(|it| it.into_path(0.1)).as_ref(),
+            // TODO: As in `fill_path`
+            clip_path
+                .map(|it| it.exact_path_elements().collect())
+                .as_ref(),
             blend_mode,
             opacity,
             None,
             None,
         );
     }
 
-    fn push_clip_layer(&mut self, clip_transform: Affine, path: impl Shape) {
+    fn push_clip_layer(&mut self, clip_transform: Affine, path: &impl ExactPathElements) {
         self.render_context.set_fill_rule(Fill::NonZero);
         self.render_context.set_transform(clip_transform);
-        self.render_context.push_clip_layer(
-            // TODO: Not allocate
-            &path.into_path(0.1),
-        );
+        // TODO: As in `fill_path`
+        self.render_context
+            .push_clip_layer(&path.exact_path_elements().collect());
     }
 
     fn pop_layer(&mut self) {