Merge pull request #7 from storybored379/pr/invalid-dash-pattern

Stroke dashes: an invalid dash pattern draws a solid stroke instead of hanging the renderer

crates/doc/src/appearance.rs

@@ -39,6 +39,15 @@ pub struct Dash {
     pub align_corners: bool,
 }
 
+impl Dash {
+    /// Whether the pattern draws dashes. As in PDF (`d` operator) and SVG (`stroke-dasharray`), a
+    /// pattern with a negative or non-finite value, or whose values are all zero, is invalid and the
+    /// stroke is drawn solid.
+    pub fn is_dashed(&self) -> bool {
+        self.pattern.iter().all(|v| v.is_finite() && *v >= 0.0) && self.pattern.iter().any(|v| *v > 0.0)
+    }
+}
+
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub enum Arrowhead {
     Triangle,

crates/pdf/src/export.rs

@@ -545,8 +545,8 @@ impl Exporter<'_> {
             }
             _ => {}
         }
-        let dash = st.dash.as_ref().filter(|d| d.pattern.iter().any(|v| *v > 0.0)).map(|d| {
-            let mut pat: Vec<f32> = d.pattern.iter().map(|v| v.max(0.0) as f32).collect();
+        let dash = st.dash.as_ref().filter(|d| d.is_dashed()).map(|d| {
+            let mut pat: Vec<f32> = d.pattern.iter().map(|v| *v as f32).collect();
             if pat.len() % 2 == 1 {
                 pat.extend(pat.clone());
             }

crates/pdf/src/import.rs

@@ -189,13 +189,9 @@ fn stroke_layer(paint: Paint, opacity: f32, p: &StrokeProps, scale: f64) -> Stro
         kurbo::Join::Bevel => LineJoin::Bevel,
     };
     st.miter_limit = p.miter_limit as f64;
-    if p.dash_array.iter().any(|v| *v > 0.0) {
-        st.dash = Some(Dash {
-            pattern: p.dash_array.iter().map(|v| *v as f64 * scale).collect(),
-            offset: p.dash_offset as f64 * scale,
-            align_corners: false,
-        });
-    }
+    let dash = Dash { pattern: p.dash_array.iter().map(|v| *v as f64 * scale).collect(), offset: p.dash_offset as f64 * scale, align_corners: false };
+    // An invalid dash array (a negative value, or all zeros) strokes solid.
+    st.dash = dash.is_dashed().then_some(dash);
     st
 }
 

crates/pdf/src/tests.rs

@@ -427,6 +427,32 @@ fn import_handmade_pdf() {
     assert!(close(l[2].geometric_bounds().unwrap(), Rect::new(150.0, 270.0, 190.0, 300.0), 1e-6));
 }
 
+#[test]
+fn import_invalid_dash_array_strokes_solid() {
+    // A dash array with a negative value is invalid (PDF 32000-1 §8.4.3.6): the line is solid. A valid
+    // array next to it stays dashed.
+    let content = "0 0 1 RG 4 w [-5 3] 0 d 10 10 m 90 10 l S [6 2] 1 d 10 50 m 90 50 l S";
+    let d = import(&handmade_pdf(content, "[0 0 100 100]")).unwrap();
+    let l = leaves(&d);
+    assert_eq!(l.len(), 2);
+    let dash = |n: &Node| match &n.appearance.items[..] {
+        [.., AppearanceItem::Stroke(st)] => st.dash.clone(),
+        items => panic!("no stroke: {items:?}"),
+    };
+    assert_eq!(dash(l[0]), None);
+    assert_eq!(dash(l[1]).map(|d| d.pattern), Some(vec![6.0, 2.0]));
+    // And the exporter never writes an invalid array back.
+    let mut d2 = doc(100.0, 100.0);
+    let mut n = rect_node(Rect::new(10.0, 10.0, 90.0, 90.0), Color::rgb(1.0, 0.0, 0.0));
+    n.appearance = Appearance::basic(Paint::None, Paint::solid(Color::BLACK), 2.0);
+    if let Some(AppearanceItem::Stroke(st)) = n.appearance.items.last_mut() {
+        st.dash = Some(Dash { pattern: vec![-5.0, 3.0], offset: 0.0, align_corners: false });
+    }
+    add(&mut d2, n);
+    let s = uncompressed(&d2);
+    assert!(!s.contains("-5 3]") && !s.contains("[0 3]"), "invalid dash written:\n{s}");
+}
+
 #[test]
 fn import_clip_from_handmade_pdf() {
     let content = "q 0 0 50 50 re W n 1 0 0 rg 0 0 100 100 re f Q";

crates/render/src/lib.rs

@@ -669,7 +669,7 @@ impl Renderer {
             })
             .with_miter_limit(st.miter_limit);
         if let Some(d) = &st.dash
-            && d.pattern.iter().any(|v| *v > 0.0)
+            && d.is_dashed()
         {
             let mut pat = d.pattern.clone();
             if pat.len() % 2 == 1 {

crates/render/src/tests.rs

@@ -322,3 +322,29 @@ fn blend_mode_on_a_single_fill_is_exact_without_a_layer() {
         assert_eq!(img.pixel(10, 50), [255, 0, 0, 255]);
     }
 }
+
+#[test]
+fn invalid_dash_pattern_strokes_solid() {
+    // A negative dash value makes the pattern invalid (PDF `d`, SVG `stroke-dasharray`): the stroke
+    // is drawn solid. [-5, 3] used to send the dasher into an endless loop.
+    let line = |dash: Option<Vec<f64>>| {
+        let mut d = Document::new(100.0, 100.0);
+        let mut bp = BezPath::new();
+        bp.move_to((10.0, 50.0));
+        bp.line_to((90.0, 50.0));
+        let id = d.alloc_id();
+        let mut n = Node::path(id, vectorcraft_geom::PathData::from_bezpath(&bp), Appearance::basic(Paint::None, Paint::solid(Color::BLACK), 6.0));
+        n.appearance.stroke_mut().unwrap().dash = dash.map(|pattern| vectorcraft_doc::Dash { pattern, ..Default::default() });
+        let l = d.layers[0].id;
+        d.insert(Some(l), 0, n).unwrap();
+        d
+    };
+    let solid = render(&line(None));
+    for pattern in [vec![-5.0, 3.0], vec![3.0, -5.0], vec![4.0, f64::NAN], vec![0.0, 0.0]] {
+        let d = line(Some(pattern.clone()));
+        let (tx, rx) = std::sync::mpsc::channel();
+        std::thread::spawn(move || tx.send(render(&d)).ok());
+        let img = rx.recv_timeout(std::time::Duration::from_secs(20)).unwrap_or_else(|_| panic!("rendering dash {pattern:?} did not finish"));
+        assert_eq!(img.pixels, solid.pixels, "dash {pattern:?}");
+    }
+}

crates/render/src/width.rs

@@ -12,7 +12,7 @@ use vectorcraft_geom::kurbo::{self, BezPath, PathEl, Point, Vec2};
 /// The outline to fill instead of stroking, when `st` has a width profile (and no dashes).
 pub(crate) fn outline_for(bp: &BezPath, st: &StrokeLayer, tol: f64) -> Option<BezPath> {
     let profile = st.profile.as_ref()?;
-    if st.dash.as_ref().is_some_and(|d| d.pattern.iter().any(|v| *v > 0.0)) {
+    if st.dash.as_ref().is_some_and(|d| d.is_dashed()) {
         return None;
     }
     let closed = bp.elements().last().is_some_and(|e| matches!(e, PathEl::ClosePath));

crates/svg/src/export.rs

@@ -413,8 +413,7 @@ impl Writer<'_> {
             LineJoin::Bevel => p.push(("stroke-linejoin", "bevel".into())),
         }
         if let Some(d) = &s.dash
-            && !d.pattern.is_empty()
-            && d.pattern.iter().any(|v| *v > 0.0)
+            && d.is_dashed()
         {
             p.push(("stroke-dasharray", d.pattern.iter().map(|v| self.num(*v)).collect::<Vec<_>>().join(" ")));
             if d.offset != 0.0 {