From ab35ce5a8e66469f306e5b112cdfee86939ead49 Mon Sep 17 00:00:00 2001 From: Arman Uguray Date: Tue, 14 Nov 2023 12:10:21 -0800 Subject: [PATCH 1/4] [encoding] CPU-side dashing for GPU strokes When GPU-side stroking is enbled, a stroke with a dash style now gets converted to a series of undashed strokes which then get encoded separately and in sequence. To support this, `Encoding` now accepts an iterator over `PathEl` as an alternative to `Shape`. --- crates/encoding/src/encoding.rs | 12 +++++++++ crates/encoding/src/path.rs | 7 ++++- src/scene.rs | 46 +++++++++++++++++++++------------ 3 files changed, 48 insertions(+), 17 deletions(-) diff --git a/crates/encoding/src/encoding.rs b/crates/encoding/src/encoding.rs index 1e16b70cf..6ec93dd7b 100644 --- a/crates/encoding/src/encoding.rs +++ b/crates/encoding/src/encoding.rs @@ -218,6 +218,18 @@ impl Encoding { encoder.finish(true) != 0 } + /// Encodes a path element iterator. If `is_fill` is true, all subpaths will be automatically + /// closed. Returns true if a non-zero number of segments were encoded. + pub fn encode_path_elements( + &mut self, + path: impl Iterator, + is_fill: bool, + ) -> bool { + let mut encoder = self.encode_path(is_fill); + encoder.path_elements(path); + encoder.finish(true) != 0 + } + /// Encodes a brush with an optional alpha modifier. #[allow(unused_variables)] pub fn encode_brush<'b>(&mut self, brush: impl Into>, alpha: f32) { diff --git a/crates/encoding/src/path.rs b/crates/encoding/src/path.rs index ed9cd99e5..a4a8905cf 100644 --- a/crates/encoding/src/path.rs +++ b/crates/encoding/src/path.rs @@ -628,8 +628,13 @@ impl<'a> PathEncoder<'a> { /// Encodes a shape. pub fn shape(&mut self, shape: &impl Shape) { + self.path_elements(shape.path_elements(0.1)); + } + + /// Encodes a path iterator + pub fn path_elements(&mut self, path: impl Iterator) { use peniko::kurbo::PathEl; - for el in shape.path_elements(0.1) { + for el in path { match el { PathEl::MoveTo(p0) => self.move_to(p0.x as f32, p0.y as f32), PathEl::LineTo(p0) => self.line_to(p0.x as f32, p0.y as f32), diff --git a/src/scene.rs b/src/scene.rs index 80f876792..ce0af74d2 100644 --- a/src/scene.rs +++ b/src/scene.rs @@ -149,13 +149,40 @@ impl<'a> SceneBuilder<'a> { brush_transform: Option, shape: &impl Shape, ) { - const GPU_STROKES: bool = false; + // The setting for tolerance are a compromise. For most applications, + // shape tolerance doesn't matter, as the input is likely Bézier paths, + // which is exact. Note that shape tolerance is hard-coded as 0.1 in + // the encoding crate. + // + // Stroke tolerance is a different matter. Generally, the cost scales + // with inverse O(n^6), so there is moderate rendering cost to setting + // too fine a value. On the other hand, error scales with the transform + // applied post-stroking, so may exceed visible threshold. When we do + // GPU-side stroking, the transform will be known. In the meantime, + // this is a compromise. + const SHAPE_TOLERANCE: f64 = 0.01; + const STROKE_TOLERANCE: f64 = SHAPE_TOLERANCE; + + const GPU_STROKES: bool = false; // Set this to `true` to enable GPU-side stroking if GPU_STROKES { - // TODO: handle dashing by using a DashIterator self.scene .encode_transform(Transform::from_kurbo(&transform)); self.scene.encode_stroke_style(style); - if self.scene.encode_shape(shape, false) { + + // We currently don't support dashing on the GPU. If the style has a dash pattern, then + // we convert it into stroked paths on the CPU and encode those as individual draw + // objects. + let encode_result = if style.dash_pattern.is_empty() { + self.scene.encode_shape(shape, false) + } else { + let dashed = peniko::kurbo::dash( + shape.path_elements(SHAPE_TOLERANCE), + style.dash_offset, + &style.dash_pattern, + ); + self.scene.encode_path_elements(dashed, false) + }; + if encode_result { if let Some(brush_transform) = brush_transform { if self .scene @@ -167,19 +194,6 @@ impl<'a> SceneBuilder<'a> { self.scene.encode_brush(brush, 1.0); } } else { - // The setting for tolerance are a compromise. For most applications, - // shape tolerance doesn't matter, as the input is likely Bézier paths, - // which is exact. Note that shape tolerance is hard-coded as 0.1 in - // the encoding crate. - // - // Stroke tolerance is a different matter. Generally, the cost scales - // with inverse O(n^6), so there is moderate rendering cost to setting - // too fine a value. On the other hand, error scales with the transform - // applied post-stroking, so may exceed visible threshold. When we do - // GPU-side stroking, the transform will be known. In the meantime, - // this is a compromise. - const SHAPE_TOLERANCE: f64 = 0.01; - const STROKE_TOLERANCE: f64 = SHAPE_TOLERANCE; let stroked = peniko::kurbo::stroke( shape.path_elements(SHAPE_TOLERANCE), style, From c5b08d034e13ee16893136ae3d3fa7b40f6379c2 Mon Sep 17 00:00:00 2001 From: Arman Uguray Date: Wed, 15 Nov 2023 14:19:17 -0800 Subject: [PATCH 2/4] Add a dashing column to stroke_styles scene --- examples/scenes/src/test_scenes.rs | 36 ++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/examples/scenes/src/test_scenes.rs b/examples/scenes/src/test_scenes.rs index c617e5d78..ac74a4828 100644 --- a/examples/scenes/src/test_scenes.rs +++ b/examples/scenes/src/test_scenes.rs @@ -70,6 +70,7 @@ pub fn test_scenes() -> SceneSet { fn funky_paths(sb: &mut SceneBuilder, _: &mut SceneParams) { use PathEl::*; let missing_movetos = [ + MoveTo((0., 0.).into()), LineTo((100.0, 100.0).into()), LineTo((100.0, 200.0).into()), ClosePath, @@ -171,8 +172,36 @@ fn stroke_styles(transform: Affine) -> impl FnMut(&mut SceneBuilder, &mut SceneP } } + // Dashed strokes with cap combinations + let t = Affine::translate((450., 0.)) * t; + y = 0.; + for start in cap_styles { + for end in cap_styles { + params.text.add( + sb, + None, + 12., + None, + Affine::translate((0., y)) * t, + &format!("Dashing - Start cap: {:?}, End cap: {:?}", start, end), + ); + sb.stroke( + &Stroke::new(20.) + .with_start_cap(start) + .with_end_cap(end) + .with_dashes(0., [10.0, 21.0]), + Affine::translate((0., y + 30.)) * t * transform, + colors[color_idx], + None, + &simple_stroke, + ); + y += 180.; + color_idx = (color_idx + 1) % colors.len(); + } + } + // Cap and join combinations - let t = Affine::translate((500., 0.)) * t; + let t = Affine::translate((550., 0.)) * t; y = 0.; for cap in cap_styles { for join in join_styles { @@ -463,7 +492,10 @@ fn longpathdash(cap: Cap) -> impl FnMut(&mut SceneBuilder, &mut SceneParams) { x += 16; } sb.stroke( - &Stroke::new(1.0).with_caps(cap).with_dashes(0.0, [1.0, 1.0]), + &Stroke::new(1.0) + .with_caps(cap) + .with_join(Join::Bevel) + .with_dashes(0.0, [1.0, 1.0]), Affine::translate((50.0, 50.0)), Color::YELLOW, None, From b36051315206187db5117042956b3cf055e91b5a Mon Sep 17 00:00:00 2001 From: Arman Uguray Date: Wed, 15 Nov 2023 14:20:32 -0800 Subject: [PATCH 3/4] [encoding] Do not encode zero-length path segments Zero-length segments lead to numerical errors and lead to extra complexity and wasted work (in the form of thread allocation) for GPU flattening. We now detect and drop them at encoding time. Currently, a path segment is considered zero-length if all of its control points (in local coordinates) are within EPSILON (1e-12) of each other. This may not be the desired behavior under transform. Also since the length here isn't in terms of the actual arc length, this won't detect all curves that are within an EPSILON sized bounding box (which may have a larger distance between their control points). For stroking purposes, the distance between the control points is what matters most as that's what's used to compute a curve's tangent. --- crates/encoding/src/path.rs | 77 +++++++++++++++++++++++++------------ 1 file changed, 53 insertions(+), 24 deletions(-) diff --git a/crates/encoding/src/path.rs b/crates/encoding/src/path.rs index a4a8905cf..91bca2c0b 100644 --- a/crates/encoding/src/path.rs +++ b/crates/encoding/src/path.rs @@ -525,13 +525,15 @@ impl<'a> PathEncoder<'a> { if self.state == PathState::MoveTo { let p0 = (self.first_point[0], self.first_point[1]); // Ensure that we don't end up with a zero-length start tangent. - let Some((x, y)) = start_tangent_for_curve(p0, (x, y), p0, p0) else { - // Drop the segment if its length is zero - // TODO: do this for all not segments, not just start. + let Some((x, y)) = start_tangent_for_curve(p0, (x, y), None, None) else { return; }; self.first_start_tangent_end = [x, y]; } + // Drop the segment if its length is zero + if self.is_zero_length_segment((x, y), None, None) { + return; + } let buf = [x, y]; let bytes = bytemuck::bytes_of(&buf); self.data.extend_from_slice(bytes); @@ -552,13 +554,15 @@ impl<'a> PathEncoder<'a> { if self.state == PathState::MoveTo { let p0 = (self.first_point[0], self.first_point[1]); // Ensure that we don't end up with a zero-length start tangent. - let Some((x, y)) = start_tangent_for_curve(p0, (x1, y1), (x2, y2), p0) else { - // Drop the segment if its length is zero - // TODO: do this for all not segments, not just start. + let Some((x, y)) = start_tangent_for_curve(p0, (x1, y1), Some((x2, y2)), None) else { return; }; self.first_start_tangent_end = [x, y]; } + // Drop the segment if its length is zero + if self.is_zero_length_segment((x1, y1), Some((x2, y2)), None) { + return; + } let buf = [x1, y1, x2, y2]; let bytes = bytemuck::bytes_of(&buf); self.data.extend_from_slice(bytes); @@ -579,13 +583,17 @@ impl<'a> PathEncoder<'a> { if self.state == PathState::MoveTo { let p0 = (self.first_point[0], self.first_point[1]); // Ensure that we don't end up with a zero-length start tangent. - let Some((x, y)) = start_tangent_for_curve(p0, (x1, y1), (x2, y2), (x3, y3)) else { - // Drop the segment if its length is zero - // TODO: do this for all not segments, not just start. + let Some((x, y)) = + start_tangent_for_curve(p0, (x1, y1), Some((x2, y2)), Some((x3, y3))) + else { return; }; self.first_start_tangent_end = [x, y]; } + // Drop the segment if its length is zero + if self.is_zero_length_segment((x1, y1), Some((x2, y2)), Some((x3, y3))) { + return; + } let buf = [x1, y1, x2, y2, x3, y3]; let bytes = bytemuck::bytes_of(&buf); self.data.extend_from_slice(bytes); @@ -703,6 +711,33 @@ impl<'a> PathEncoder<'a> { ); } } + + fn last_point(&self) -> Option<(f32, f32)> { + let len = self.data.len(); + if len < 8 { + return None; + } + let pts: &[f32; 2] = bytemuck::from_bytes(&self.data[len - 8..len]); + Some((pts[0], pts[1])) + } + + fn is_zero_length_segment( + &self, + p1: (f32, f32), + p2: Option<(f32, f32)>, + p3: Option<(f32, f32)>, + ) -> bool { + let p0 = self.last_point().unwrap(); + let p2 = p2.unwrap_or(p1); + let p3 = p3.unwrap_or(p1); + + let x_min = p0.0.min(p1.0.min(p2.0.min(p3.0))); + let x_max = p0.0.max(p1.0.max(p2.0.max(p3.0))); + let y_min = p0.1.min(p1.1.min(p2.1.min(p3.1))); + let y_max = p0.1.max(p1.1.max(p2.1.max(p3.1))); + + !(x_max - x_min > EPSILON || y_max - y_min > EPSILON) + } } #[cfg(feature = "full")] @@ -728,6 +763,8 @@ impl fello::scale::Pen for PathEncoder<'_> { } } +const EPSILON: f32 = 1e-12; + // Returns the end point of the start tangent of a curve starting at `(x0, y0)`, or `None` if the // curve is degenerate / has zero-length. The inputs are a sequence of control points that can // represent a line, a quadratic Bezier, or a cubic Bezier. Lines and quadratic Beziers can be @@ -735,24 +772,16 @@ impl fello::scale::Pen for PathEncoder<'_> { fn start_tangent_for_curve( p0: (f32, f32), p1: (f32, f32), - p2: (f32, f32), - p3: (f32, f32), + p2: Option<(f32, f32)>, + p3: Option<(f32, f32)>, ) -> Option<(f32, f32)> { - debug_assert!(!p0.0.is_nan()); - debug_assert!(!p0.1.is_nan()); - debug_assert!(!p1.0.is_nan()); - debug_assert!(!p1.1.is_nan()); - debug_assert!(!p2.0.is_nan()); - debug_assert!(!p2.1.is_nan()); - debug_assert!(!p3.0.is_nan()); - debug_assert!(!p3.1.is_nan()); - - const EPS: f32 = 1e-12; - let pt = if (p1.0 - p0.0).abs() > EPS || (p1.1 - p0.1).abs() > EPS { + let p2 = p2.unwrap_or(p0); + let p3 = p3.unwrap_or(p0); + let pt = if (p1.0 - p0.0).abs() > EPSILON || (p1.1 - p0.1).abs() > EPSILON { p1 - } else if (p2.0 - p0.0).abs() > EPS || (p2.1 - p0.1).abs() > EPS { + } else if (p2.0 - p0.0).abs() > EPSILON || (p2.1 - p0.1).abs() > EPSILON { p2 - } else if (p3.0 - p0.0).abs() > EPS || (p3.1 - p0.1).abs() > EPS { + } else if (p3.0 - p0.0).abs() > EPSILON || (p3.1 - p0.1).abs() > EPSILON { p3 } else { return None; From 8001151ea2aa2076c1f19336e51a03c29c59aa57 Mon Sep 17 00:00:00 2001 From: Arman Uguray Date: Tue, 21 Nov 2023 16:30:12 -0800 Subject: [PATCH 4/4] Address review comments --- crates/encoding/src/path.rs | 60 ++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/crates/encoding/src/path.rs b/crates/encoding/src/path.rs index 91bca2c0b..69ca2cce9 100644 --- a/crates/encoding/src/path.rs +++ b/crates/encoding/src/path.rs @@ -523,9 +523,8 @@ impl<'a> PathEncoder<'a> { self.move_to(self.first_point[0], self.first_point[1]); } if self.state == PathState::MoveTo { - let p0 = (self.first_point[0], self.first_point[1]); // Ensure that we don't end up with a zero-length start tangent. - let Some((x, y)) = start_tangent_for_curve(p0, (x, y), None, None) else { + let Some((x, y)) = self.start_tangent_for_curve((x, y), None, None) else { return; }; self.first_start_tangent_end = [x, y]; @@ -552,9 +551,8 @@ impl<'a> PathEncoder<'a> { self.move_to(self.first_point[0], self.first_point[1]); } if self.state == PathState::MoveTo { - let p0 = (self.first_point[0], self.first_point[1]); // Ensure that we don't end up with a zero-length start tangent. - let Some((x, y)) = start_tangent_for_curve(p0, (x1, y1), Some((x2, y2)), None) else { + let Some((x, y)) = self.start_tangent_for_curve((x1, y1), Some((x2, y2)), None) else { return; }; self.first_start_tangent_end = [x, y]; @@ -581,10 +579,9 @@ impl<'a> PathEncoder<'a> { self.move_to(self.first_point[0], self.first_point[1]); } if self.state == PathState::MoveTo { - let p0 = (self.first_point[0], self.first_point[1]); // Ensure that we don't end up with a zero-length start tangent. let Some((x, y)) = - start_tangent_for_curve(p0, (x1, y1), Some((x2, y2)), Some((x3, y3))) + self.start_tangent_for_curve((x1, y1), Some((x2, y2)), Some((x3, y3))) else { return; }; @@ -738,6 +735,33 @@ impl<'a> PathEncoder<'a> { !(x_max - x_min > EPSILON || y_max - y_min > EPSILON) } + + // Returns the end point of the start tangent of a curve starting at `(x0, y0)`, or `None` if the + // curve is degenerate / has zero-length. The inputs are a sequence of control points that can + // represent a line, a quadratic Bezier, or a cubic Bezier. Lines and quadratic Beziers can be + // passed to this function by simply setting the invalid control point degrees equal to `None`. + // + // `self.first_point` is always treated as the first control point of the curve. + fn start_tangent_for_curve( + &self, + p1: (f32, f32), + p2: Option<(f32, f32)>, + p3: Option<(f32, f32)>, + ) -> Option<(f32, f32)> { + let p0 = (self.first_point[0], self.first_point[1]); + let p2 = p2.unwrap_or(p0); + let p3 = p3.unwrap_or(p0); + let pt = if (p1.0 - p0.0).abs() > EPSILON || (p1.1 - p0.1).abs() > EPSILON { + p1 + } else if (p2.0 - p0.0).abs() > EPSILON || (p2.1 - p0.1).abs() > EPSILON { + p2 + } else if (p3.0 - p0.0).abs() > EPSILON || (p3.1 - p0.1).abs() > EPSILON { + p3 + } else { + return None; + }; + Some(pt) + } } #[cfg(feature = "full")] @@ -765,30 +789,6 @@ impl fello::scale::Pen for PathEncoder<'_> { const EPSILON: f32 = 1e-12; -// Returns the end point of the start tangent of a curve starting at `(x0, y0)`, or `None` if the -// curve is degenerate / has zero-length. The inputs are a sequence of control points that can -// represent a line, a quadratic Bezier, or a cubic Bezier. Lines and quadratic Beziers can be -// passed to this function by simply setting the invalid control point degrees equal to `(x0, y0)`. -fn start_tangent_for_curve( - p0: (f32, f32), - p1: (f32, f32), - p2: Option<(f32, f32)>, - p3: Option<(f32, f32)>, -) -> Option<(f32, f32)> { - let p2 = p2.unwrap_or(p0); - let p3 = p3.unwrap_or(p0); - let pt = if (p1.0 - p0.0).abs() > EPSILON || (p1.1 - p0.1).abs() > EPSILON { - p1 - } else if (p2.0 - p0.0).abs() > EPSILON || (p2.1 - p0.1).abs() > EPSILON { - p2 - } else if (p3.0 - p0.0).abs() > EPSILON || (p3.1 - p0.1).abs() > EPSILON { - p3 - } else { - return None; - }; - Some(pt) -} - #[cfg(test)] mod tests { use super::*;