Bug 1849487. Futher improve miter joining. r=lsalzman

This avoids numerical stability problems when computing the
miter pointer intersection when the lines are nearly parallel.

Differential Revision: https://phabricator.services.mozilla.com/D187057

.cargo/config.in

@@ -5,9 +5,9 @@
 [source.crates-io]
 replace-with = "vendored-sources"
 
-[source."git+https://github.com/FirefoxGraphics/aa-stroke?rev=c7bc7190f6d3115bc81640c0433649c1fce9491c"]
+[source."git+https://github.com/FirefoxGraphics/aa-stroke?rev=ed4206ea11703580cd1d4fc63371a527b29d8252"]
 git = "https://github.com/FirefoxGraphics/aa-stroke"
-rev = "c7bc7190f6d3115bc81640c0433649c1fce9491c"
+rev = "ed4206ea11703580cd1d4fc63371a527b29d8252"
 replace-with = "vendored-sources"
 
 [source."git+https://github.com/FirefoxGraphics/wpf-gpu-raster?rev=5ab6fe33d00021325ee920b3c10526dc8301cf46"]

Cargo.lock

@@ -5,7 +5,7 @@ version = 3
 [[package]]
 name = "aa-stroke"
 version = "0.1.0"
-source = "git+https://github.com/FirefoxGraphics/aa-stroke?rev=c7bc7190f6d3115bc81640c0433649c1fce9491c#c7bc7190f6d3115bc81640c0433649c1fce9491c"
+source = "git+https://github.com/FirefoxGraphics/aa-stroke?rev=ed4206ea11703580cd1d4fc63371a527b29d8252#ed4206ea11703580cd1d4fc63371a527b29d8252"
 dependencies = [
  "euclid",
 ]

third_party/rust/aa-stroke/.cargo-checksum.json

@@ -1 +1 @@
-{"files":{".github/workflows/rust.yml":"e859b12cfed145b66e4fbf1571dde15880359717ca5b0a9720341229183f2c6f","Cargo.toml":"f507cac11c3c26af28420d68ec3748a5453322d51ef1379a340fdd3b1c9b187a","README.md":"60b34cfa653114d5054009696df2ed2ea1d4926a6bc312d0cac4b84845c2beff","examples/simple.rs":"c196e79568fe4be31a08374aa451c70c9377db5428aef924a985e069c12ed91e","src/bezierflattener.rs":"61687da22490cb1bd901d0b5eb1de3a98802b46c03719ded4163c7a4997f0ad9","src/c_bindings.rs":"23890f7bb5d40d87d72b28842821ada7b2e1509a3b37c9af8ed0055036ceadcc","src/lib.rs":"18480fd1f92519229d2439c226011df88a3abf9a6a12295685611cf2bddc8965","src/tri_rasterize.rs":"fb6f595ab9340d8ea6429b41638c378bbd772c8e4d8f7793e225624c12cd3a21"},"package":null}
+{"files":{".github/workflows/rust.yml":"6a9f1b122ea02367a2f1ff1fc7b9a728284ceb47fad12e1610cde9d760f4efc3","Cargo.toml":"f507cac11c3c26af28420d68ec3748a5453322d51ef1379a340fdd3b1c9b187a","README.md":"60b34cfa653114d5054009696df2ed2ea1d4926a6bc312d0cac4b84845c2beff","examples/simple.rs":"c196e79568fe4be31a08374aa451c70c9377db5428aef924a985e069c12ed91e","src/bezierflattener.rs":"61687da22490cb1bd901d0b5eb1de3a98802b46c03719ded4163c7a4997f0ad9","src/c_bindings.rs":"7cc3916665a89b8dc1eb1d1ddd2c966e6ece06dba04d228a8ebd807fb2270345","src/lib.rs":"9719ba050cc0ff6093c60646321f0cbedd06d6483106119bd9ca05d6e42ddaae","src/tri_rasterize.rs":"fb6f595ab9340d8ea6429b41638c378bbd772c8e4d8f7793e225624c12cd3a21"},"package":null}

third_party/rust/aa-stroke/.github/workflows/rust.yml

@@ -18,3 +18,15 @@ jobs:
       run: cargo build --verbose --features=c_bindings
     - name: Run tests
       run: cargo test --verbose --features=c_bindings
+  miri:
+    name: "Miri"
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Install Miri
+        run: |
+          rustup toolchain install nightly --component miri
+          rustup override set nightly
+          cargo miri setup
+      - name: Test with Miri
+        run: cargo miri test --lib --bins --tests --examples

third_party/rust/aa-stroke/src/c_bindings.rs

@@ -60,14 +60,17 @@ pub extern "C" fn aa_stroke_close(s: &mut Stroker) {
 pub extern "C" fn aa_stroke_finish(s: &mut Stroker) -> VertexBuffer {
     let stroked_path = s.get_stroked_path();
     if let Some(output_buffer_size) = stroked_path.get_output_buffer_size() {
+        // if let Some(output_buffer) = stroked_path.output_buffer {
+        //     dbg!(&output_buffer[0..output_buffer_size]);
+        // }
         VertexBuffer {
             data: std::ptr::null(),
             len: output_buffer_size,
         }
     } else {
-        let result = stroked_path.finish();
-        let vb = VertexBuffer { data: result.as_ptr(), len: result.len() };
-        std::mem::forget(result);
+        let result = s.finish();
+        let len = result.len();
+        let vb = VertexBuffer { data: Box::leak(result).as_ptr(), len };
         vb
     }
 }
@@ -99,3 +102,21 @@ fn simple() {
     aa_stroke_vertex_buffer_release(vb);
     unsafe { aa_stroke_release(s) } ;
 }
+
+#[test]
+fn output_buffer() {
+    let style = StrokeStyle::default();
+    let mut output = Vec::new();
+    output.resize_with(1000, || OutputVertex{x: 0., y: 0., coverage: 0.});
+    let s = unsafe { &mut *aa_stroke_new(&style, output.as_mut_ptr(), output.len()) } ;
+    aa_stroke_move_to(s, 10., 10., false);
+    aa_stroke_line_to(s, 100., 100., false);
+    aa_stroke_line_to(s, 100., 10., true);
+
+    let vb = aa_stroke_finish(s);
+    assert_ne!(vb.len, 0);
+    assert_eq!(vb.data, std::ptr::null());
+    aa_stroke_vertex_buffer_release(vb);
+    unsafe { aa_stroke_release(s) } ;
+}
+

third_party/rust/aa-stroke/src/lib.rs

@@ -156,8 +156,8 @@ impl<'z> PathBuilder<'z> {
     }
 
     /// Completes the current path
-    pub fn finish(self) -> Vec<Vertex> {
-        self.vertices
+    pub fn finish(self) -> Box<[Vertex]> {
+        self.vertices.into_boxed_slice()
     }
 
     pub fn get_output_buffer_size(&self) -> Option<usize> {
@@ -620,10 +620,14 @@ fn join_line(
                         let mid = bisect(s1_normal, s2_normal);
 
                         // cross = sin, dot = cos
-                        let s = cross(mid, s1_normal)/(1. + dot(s1_normal, mid));
+                        let cos = dot(s1_normal, mid);
+                        let s = cross(mid, s1_normal)/(1. + cos);
+
+                        // compute the intersection in a more stable way
+                        let intersection = pt + mid * (offset / cos);
 
                         let ramp_s1 = intersection + s1_normal * 1. + unperp(s1_normal) * s;
-                        let ramp_s2 = intersection + s2_normal * 1. + unperp(s2_normal) * s;
+                        let ramp_s2 = intersection + s2_normal * 1. + perp(s2_normal) * s;
 
                         dest.ramp(intersection.x, intersection.y,
                             ramp_s1.x, ramp_s1.y,
@@ -707,12 +711,16 @@ impl<'z> Stroker<'z> {
     }
 
     pub fn move_to(&mut self, pt: Point, closed_subpath: bool) {
+        //eprintln!("stroker.move_to(Point::new({}, {}), {});", pt.x, pt.y, closed_subpath);
+
         self.start_point = None;
         self.cur_pt = Some(pt);
         self.closed_subpath = closed_subpath;
     }
 
     pub fn line_to(&mut self, pt: Point) {
+        //eprintln!("stroker.line_to(Point::new({}, {}));", pt.x, pt.y);
+
         let cur_pt = self.cur_pt;
         let stroked_path = &mut self.stroked_path;
         let half_width = self.half_width;
@@ -784,6 +792,7 @@ impl<'z> Stroker<'z> {
     }
 
     pub fn curve_to(&mut self, cx1: Point, cx2: Point, pt: Point) {
+        //eprintln!("stroker.curve_to(Point::new({}, {}), Point::new({}, {}), Point::new({}, {}));", cx1.x, cx1.y, cx2.x, cx2.y, pt.x, pt.y);
         self.curve_to_internal(cx1, cx2, pt, false);
     }
 
@@ -890,7 +899,7 @@ impl<'z> Stroker<'z> {
         stroked_path
     }
 
-    pub fn finish(&mut self) -> Vec<Vertex> {
+    pub fn finish(&mut self) -> Box<[Vertex]> {
         self.get_stroked_path().finish()
     }
 }
@@ -939,7 +948,7 @@ fn butt_cap() {
     stroker.move_to(Point::new(20., 20.5), false);
     stroker.line_to_capped(Point::new(40., 20.5));
     let result = stroker.finish();
-    for v in result {
+    for v in result.iter() {
         assert!(v.y == 20.5 || v.y == 19.5 || v.y == 21.5);
     }
 }
@@ -1005,7 +1014,7 @@ fn degenerate_miter_join() {
 
     let result = stroker.finish();
     // make sure none of the verticies are wildly out of place
-    for v in result {
+    for v in result.iter() {
         assert!(v.y >= 527.);
     }
 
@@ -1025,8 +1034,41 @@ fn degenerate_miter_join() {
     stroker.line_to(Point::new(512.3874, 598.6111));
     stroker.line_to_capped(end);
     let result = stroker.finish();
-    for v in result {
-        assert!(dbg!(distance_from_line(start, end, Point::new(v.x, v.y))) <= 21.);
+    for v in result.iter() {
+        assert!(distance_from_line(start, end, Point::new(v.x, v.y)) <= 21.);
     }
+
+    // from https://stackoverflow.com/questions/849211/shortest-distance-between-a-point-and-a-line-segment
+    fn minimum_distance(v: Point, w: Point, p: Point) -> f32 {
+        // Return minimum distance between line segment vw and point p
+        let l2 = (v-w).length().powi(2);  // i.e. |w-v|^2 -  avoid a sqrt
+        if l2 == 0.0 { return (p - v).length(); }   // v == w case
+        // Consider the line extending the segment, parameterized as v + t (w - v).
+        // We find projection of point p onto the line.
+        // It falls where t = [(p-v) . (w-v)] / |w-v|^2
+        // We clamp t from [0,1] to handle points outside the segment vw.
+        let t = 0_f32.max(1_f32.min(dot(p - v, w - v) / l2));
+        let projection = v + (w - v) * t;  // Projection falls on the segment
+        (p - projection).length()
+    }
+
+    let mut stroker = Stroker::new(&StrokeStyle{
+        cap: LineCap::Square,
+        join: LineJoin::Miter,
+        width: 40.0,
+        miter_limit: 10.0,
+        ..Default::default()});
+    let start = Point::new(689.3504, 434.5446);
+    let end = Point::new(671.83203, 422.61914);
+    stroker.move_to(Point::new(689.3504, 434.5446), false);
+    stroker.line_to(Point::new(681.04254, 428.8891));
+    stroker.line_to_capped(Point::new(671.83203, 422.61914));
+
+    let result = stroker.finish();
+    let max_distance = (21_f32.powi(2) * 2.).sqrt();
+    for v in result.iter() {
+        assert!(minimum_distance(start, end, Point::new(v.x, v.y)) <= max_distance);
+    }
+
 }
 

toolkit/library/rust/shared/Cargo.toml

@@ -95,7 +95,7 @@ processtools = { path = "../../../components/processtools" }
 qcms = { path = "../../../../gfx/qcms", features = ["c_bindings", "neon"], default-features = false }
 
 wpf-gpu-raster = { git = "https://github.com/FirefoxGraphics/wpf-gpu-raster", rev = "5ab6fe33d00021325ee920b3c10526dc8301cf46" }
-aa-stroke = { git = "https://github.com/FirefoxGraphics/aa-stroke", rev = "c7bc7190f6d3115bc81640c0433649c1fce9491c" }
+aa-stroke = { git = "https://github.com/FirefoxGraphics/aa-stroke", rev = "ed4206ea11703580cd1d4fc63371a527b29d8252" }
 
 # Force url to stay at 2.1.0. See bug 1734538.
 url = "=2.1.0"