Make tangents at subpath end watertight

[raphlinus]

At subpath end, the last path segment is encoded with the end point chosen so that the vector from the current point to the end point is the tangent, ie the same vector as cubic_start_tangent applied to the first segment in the subpath. In those cases, compute the tangent by subtracting those two points, rather than cubic_start_tangent applied to the line. These are mathematically identical, but may give different results because of roundoff.

There are two cases: an open subpath, in which case the case is applied to the tangent at draw_start_cap, or a closed subpath, where the logic is in the tangent calculation in read_neighboring_segment.

Both GPU and CPU shaders are updated. It hasn't been carefully validated.

Hopefully fixes #616 and #650.

[dominikh]

There are two cases: an open subpath, in which case the case is applied to the tangent at draw_start_cap, or an open subpath, where the logic is in the tangent calculation in read_neighboring_segment.

I think this is erroneously saying "open subpath" twice (in the PR description as well as the commit message).

[dominikh]

I can confirm that with the new flatten.wgsl, #650 is fixed for me. I also didn't notice any incorrect rendering in any of my (manual) testing.

[raphlinus]

I think this is erroneously saying "open subpath" twice (in the PR description as well as the commit message).

Thanks, the second one should be "closed," and I've fixed that in the comment above. Glad to hear this works!

[DJMcNab]

This works for me on the regression test. I'd like to see an actual test added. I.e.:

diff --git a/vello_tests/src/lib.rs b/vello_tests/src/lib.rs
index e97910593..f9b3c1c1e 100644
--- a/vello_tests/src/lib.rs
+++ b/vello_tests/src/lib.rs
@@ -16,7 +16,7 @@ use vello::wgpu::{
     self, BufferDescriptor, BufferUsages, CommandEncoderDescriptor, Extent3d, ImageCopyBuffer,
     TextureDescriptor, TextureFormat, TextureUsages,
 };
-use vello::{block_on_wgpu, util::RenderContext, RendererOptions, Scene};
+use vello::{block_on_wgpu, util::RenderContext, AaConfig, RendererOptions, Scene};
 
 mod compare;
 mod snapshot;
@@ -32,6 +32,7 @@ pub struct TestParams {
     pub base_colour: Option<Color>,
     pub use_cpu: bool,
     pub name: String,
+    pub anti_aliasing: AaConfig,
 }
 
 impl TestParams {
@@ -42,6 +43,7 @@ impl TestParams {
             base_colour: None,
             use_cpu: false,
             name: name.into(),
+            anti_aliasing: AaConfig::Area,
         }
     }
 }
@@ -87,7 +89,7 @@ pub async fn get_scene_image(params: &TestParams, scene: &Scene) -> Result<Image
             surface_format: None,
             use_cpu: params.use_cpu,
             num_init_threads: NonZeroUsize::new(1),
-            antialiasing_support: vello::AaSupport::area_only(),
+            antialiasing_support: std::iter::once(params.anti_aliasing).collect(),
         },
     )
     .or_else(|_| bail!("Got non-Send/Sync error from creating renderer"))?;
@@ -97,7 +99,7 @@ pub async fn get_scene_image(params: &TestParams, scene: &Scene) -> Result<Image
         base_color: params.base_colour.unwrap_or(Color::BLACK),
         width,
         height,
-        antialiasing_method: vello::AaConfig::Area,
+        antialiasing_method: params.anti_aliasing,
         debug: vello::DebugLayers::none(),
     };
     let size = Extent3d {
diff --git a/vello_tests/tests/regression.rs b/vello_tests/tests/regression.rs
new file mode 100644
index 000000000..46451216b
--- /dev/null
+++ b/vello_tests/tests/regression.rs
@@ -0,0 +1,20 @@
+use vello::{
+    kurbo::{Affine, RoundedRect, Stroke},
+    peniko::Color,
+    AaConfig, Scene,
+};
+use vello_tests::{snapshot_test_sync, TestParams};
+
+#[test]
+#[cfg_attr(skip_gpu_tests, ignore)]
+fn rounded_rectangle_watertight() {
+    let mut scene = Scene::new();
+    let rect = RoundedRect::new(60.0, 10.0, 80.0, 30.0, 10.0);
+    let stroke = Stroke::new(2.0);
+    scene.stroke(&stroke, Affine::IDENTITY, Color::WHITE, None, &rect);
+    let mut params = TestParams::new("rounded_rectangle_watertight", 70, 30);
+    params.anti_aliasing = AaConfig::Msaa16;
+    snapshot_test_sync(scene, &params)
+        .unwrap()
+        .assert_mean_less_than(0.001);
+}

[raphlinus]

I agree we could use better testing, but I'm on the road right now so not in a great position to write and validate tests.