Match CPU/GPU logic for start tangents (#705)

When encoding a start tangent for an end cap or closed segment, use logic designed to match the GPU computed tangent for the first segment. Note: this changes the GPU calculation to write out the lerp calculation explicitly rather than use the mix intrinsic, so we can rely on the rounding behavior. In the presence of fastmath, the rounding behavior is not guaranteed, but it is verified to work on M1. Fixes #704, unless we get bitten by fastmath. --------- Co-authored-by: Daniel McNab <[email protected]>
linebender · Oct 3, 2024 · 30c5e8e · 30c5e8e
1 parent b57d930
commit 30c5e8e
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 18 deletions.
diff --git a/vello_encoding/src/path.rs b/vello_encoding/src/path.rs
@@ -524,7 +524,7 @@ impl<'a> PathEncoder<'a> {
         }
         if self.state == PathState::MoveTo {
             // Ensure that we don't end up with a zero-length start tangent.
-            let Some((x, y)) = self.start_tangent_for_curve((x, y), None, None) else {
+            let Some((x, y)) = self.start_tangent_for_line((x, y)) else {
                 return;
             };
             self.first_start_tangent_end = [x, y];
@@ -552,7 +552,7 @@ impl<'a> PathEncoder<'a> {
         }
         if self.state == PathState::MoveTo {
             // Ensure that we don't end up with a zero-length start tangent.
-            let Some((x, y)) = self.start_tangent_for_curve((x1, y1), Some((x2, y2)), None) else {
+            let Some((x, y)) = self.start_tangent_for_quad((x1, y1), (x2, y2)) else {
                 return;
             };
             self.first_start_tangent_end = [x, y];
@@ -580,9 +580,7 @@ impl<'a> PathEncoder<'a> {
         }
         if self.state == PathState::MoveTo {
             // Ensure that we don't end up with a zero-length start tangent.
-            let Some((x, y)) =
-                self.start_tangent_for_curve((x1, y1), Some((x2, y2)), Some((x3, y3)))
-            else {
+            let Some((x, y)) = self.start_tangent_for_curve((x1, y1), (x2, y2), (x3, y3)) else {
                 return;
             };
             self.first_start_tangent_end = [x, y];
@@ -748,20 +746,17 @@ impl<'a> PathEncoder<'a> {
     }
 
     // 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`.
+    // curve is degenerate / has zero-length. The inputs are a sequence of control points that
+    // represent a cubic Bezier.
     //
     // `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)>,
+        p2: (f32, f32),
+        p3: (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 {
@@ -773,6 +768,39 @@ impl<'a> PathEncoder<'a> {
         };
         Some(pt)
     }
+
+    /// Similar to [`Self::start_tangent_for_curve`] but for a line.
+    fn start_tangent_for_line(&self, p1: (f32, f32)) -> Option<(f32, f32)> {
+        let p0 = (self.first_point[0], self.first_point[1]);
+        let pt = if (p1.0 - p0.0).abs() > EPSILON || (p1.1 - p0.1).abs() > EPSILON {
+            (
+                p0.0 + 1. / 3. * (p1.0 - p0.0),
+                p0.1 + 1. / 3. * (p1.1 - p0.1),
+            )
+        } else {
+            return None;
+        };
+        Some(pt)
+    }
+
+    // Similar to `start_tangent_for_curve` but for a quadratic Bézier.
+    fn start_tangent_for_quad(&self, p1: (f32, f32), p2: (f32, f32)) -> Option<(f32, f32)> {
+        let p0 = (self.first_point[0], self.first_point[1]);
+        let pt = if (p1.0 - p0.0).abs() > EPSILON || (p1.1 - p0.1).abs() > EPSILON {
+            (
+                p1.0 + 1. / 3. * (p0.0 - p1.0),
+                p1.1 + 1. / 3. * (p0.1 - p1.1),
+            )
+        } else if (p2.0 - p0.0).abs() > EPSILON || (p2.1 - p0.1).abs() > EPSILON {
+            (
+                p1.0 + 1. / 3. * (p2.0 - p1.0),
+                p1.1 + 1. / 3. * (p2.1 - p1.1),
+            )
+        } else {
+            return None;
+        };
+        Some(pt)
+    }
 }
 
 #[cfg(feature = "full")]

diff --git a/vello_shaders/shader/flatten.wgsl b/vello_shaders/shader/flatten.wgsl
@@ -649,7 +649,9 @@ fn read_transform(transform_base: u32, ix: u32) -> Transform {
 }
 
 fn transform_apply(transform: Transform, p: vec2f) -> vec2f {
-    return transform.mat.xy * p.x + transform.mat.zw * p.y + transform.translate;
+    let px = fma(transform.mat.x, p.x, fma(transform.mat.z, p.y, transform.translate.x));
+    let py = fma(transform.mat.y, p.x, fma(transform.mat.w, p.y, transform.translate.y));
+    return vec2(px, py);
 }
 
 fn round_down(x: f32) -> i32 {
@@ -735,12 +737,12 @@ fn read_path_segment(tag: PathTagData, is_stroke: bool) -> CubicPoints {
     // Degree-raise
     if seg_type == PATH_TAG_LINETO {
         p3 = p1;
-        p2 = mix(p3, p0, 1.0 / 3.0);
-        p1 = mix(p0, p3, 1.0 / 3.0);
+        p2 = p3 + (1.0 / 3.0) * (p0 - p3);
+        p1 = p0 + (1.0 / 3.0) * (p3 - p0);
     } else if seg_type == PATH_TAG_QUADTO {
         p3 = p2;
-        p2 = mix(p1, p2, 1.0 / 3.0);
-        p1 = mix(p1, p0, 1.0 / 3.0);
+        p2 = p1 + (1.0 / 3.0) * (p2 - p1);
+        p1 = p1 + (1.0 / 3.0) * (p0 - p1);
     }
 
     return CubicPoints(p0, p1, p2, p3);

diff --git a/vello_tests/snapshots/longpathdash_butt.png b/vello_tests/snapshots/longpathdash_butt.png
diff --git a/vello_tests/tests/snapshot_test_scenes.rs b/vello_tests/tests/snapshot_test_scenes.rs
@@ -11,7 +11,7 @@ fn snapshot_test_scene(test_scene: ExampleScene, mut params: TestParams) {
     let scene = encode_test_scene(test_scene, &mut params);
     snapshot_test_sync(scene, &params)
         .unwrap()
-        .assert_mean_less_than(0.01);
+        .assert_mean_less_than(0.0095);
 }
 
 #[test]
@@ -97,3 +97,11 @@ fn snapshot_blurred_rounded_rect() {
     let params = TestParams::new("blurred_rounded_rect", 400, 400);
     snapshot_test_scene(test_scene, params);
 }
+
+#[test]
+#[cfg_attr(skip_gpu_tests, ignore)]
+fn snapshot_longpathdash_butt() {
+    let test_scene = test_scenes::longpathdash_butt();
+    let params = TestParams::new("longpathdash_butt", 440, 80);
+    snapshot_test_scene(test_scene, params);
+}