Merge pull request #1318 from DWAA1660/fix/1250-mask-tangent-lengths

Masks: addVertex and setVertex fill in missing tangents of a loaded path (#1250)

crates/engine/src/commands/mask.rs

@@ -132,6 +132,9 @@ fn edit_path_counted<T>(s: &mut Session, p: &Value, label: &str, f: impl FnOnce(
         let roto = super::paths::is_roto(g);
         let pr = g.get_mut("path").ok_or_else(|| bad(label, "mask has no path"))?;
         let KV::Path(mut sp) = pr.value_at(lt) else { return Err(bad(label, "not a path")) };
+        // A path read from a file may have fewer tangents than vertices; the edits below index
+        // all three arrays by vertex (#1250).
+        sp.fit_tangents();
         let (r, op) = f(&mut sp)?;
         if roto {
             super::paths::roto_smooth(&mut sp);
@@ -287,6 +290,7 @@ fn edit_vertices_counted(s: &mut Session, p: &Value, label: &str, removes: bool,
             let roto = super::paths::is_roto(g);
             let Some(pr) = g.get_mut("path") else { continue };
             let KV::Path(mut sp) = pr.value_at(lt) else { continue };
+            sp.fit_tangents();
             f(&mut sp, idx);
             if roto {
                 super::paths::roto_smooth(&mut sp);

crates/engine/src/tests_timeline.rs

@@ -410,6 +410,77 @@ fn pen_tool_draws_edits_and_closes_a_mask() {
     assert_eq!(s.active_comp().unwrap().layers[0].masks().unwrap().children.len(), 1);
 }
 
+/// Cut every mask path's tangent arrays in the saved project to one entry (static values and
+/// keyframes alike), then open the result: the shape of a hand-edited or foreign file (#1250).
+fn reopen_with_short_tangents(s: &mut Session) {
+    fn cut(v: &mut Value) {
+        match v {
+            Value::Object(o) => {
+                if o.contains_key("vertices") {
+                    for k in ["in_tangents", "out_tangents"] {
+                        if let Some(Value::Array(a)) = o.get_mut(k) {
+                            a.truncate(1);
+                        }
+                    }
+                }
+                o.values_mut().for_each(cut);
+            }
+            Value::Array(a) => a.iter_mut().for_each(cut),
+            _ => {}
+        }
+    }
+    let mut j: Value = serde_json::from_str(&s.project.to_json()).unwrap();
+    cut(&mut j);
+    let p = effectcraft_project::Project::from_json(&j.to_string()).unwrap();
+    s.replace_project(p, None);
+}
+
+#[test]
+fn vertex_commands_fill_in_missing_tangents_of_a_loaded_path() {
+    let (mut s, l) = setup();
+    let m = s.execute("mask.new", json!({"layer": l, "vertices": [[10, 10], [90, 10], [90, 90]]})).unwrap()["mask"].as_u64().unwrap();
+    reopen_with_short_tangents(&mut s);
+    let path = |s: &Session| prop(s, l, "masks/#1/path").value.as_path().unwrap().clone();
+    assert_eq!(path(&s).in_tangents.len(), 1, "the file really has short tangents");
+    // Both used to index the tangent arrays by vertex and panic.
+    s.execute("mask.setVertex", json!({"layer": l, "mask": m, "index": 2, "in": [0, -20], "out": [0, 20]})).unwrap();
+    s.execute("mask.addVertex", json!({"layer": l, "mask": m, "point": [10, 90]})).unwrap();
+    let p = path(&s);
+    assert_eq!(p.vertices.len(), 4);
+    assert_eq!((p.in_tangents.len(), p.out_tangents.len()), (4, 4));
+    assert_eq!((p.in_tangents[2], p.out_tangents[2]), ([0.0, -20.0], [0.0, 20.0]));
+    assert_eq!(p.in_tangents[1], [0.0, 0.0], "a missing tangent is a corner");
+}
+
+#[test]
+fn deleting_vertices_of_a_loaded_path_with_short_tangents_does_not_panic() {
+    let (mut s, l) = setup();
+    let m = s.execute("mask.new", json!({"layer": l, "vertices": [[10, 10], [90, 10], [90, 90]]})).unwrap()["mask"].as_u64().unwrap();
+    reopen_with_short_tangents(&mut s);
+    s.execute("mask.selectVertices", json!({"vertices": [{"layer": l, "mask": m, "index": 2}]})).unwrap();
+    s.execute("mask.deleteVertices", json!({})).unwrap();
+    let p = prop(&s, l, "masks/#1/path").value.as_path().unwrap().clone();
+    assert_eq!((p.vertices.len(), p.in_tangents.len(), p.out_tangents.len()), (2, 2, 2));
+}
+
+#[test]
+fn vertex_commands_fill_in_missing_tangents_of_a_loaded_animated_path() {
+    let (mut s, l) = setup();
+    let m = s.execute("mask.new", json!({"layer": l, "vertices": [[10, 10], [90, 10], [90, 90]]})).unwrap()["mask"].as_u64().unwrap();
+    s.execute("prop.toggleAnimation", json!({"layer": l, "path": "masks/#1/path"})).unwrap();
+    s.execute("time.set", json!({"time": 1.0})).unwrap();
+    s.execute("mask.setVertex", json!({"layer": l, "mask": m, "index": 0, "point": [0, 0]})).unwrap();
+    reopen_with_short_tangents(&mut s);
+    s.execute("time.set", json!({"time": 1.0})).unwrap();
+    s.execute("mask.setVertex", json!({"layer": l, "mask": m, "index": 2, "in": [5, 5]})).unwrap();
+    s.execute("mask.addVertex", json!({"layer": l, "mask": m, "index": 3, "point": [10, 90]})).unwrap();
+    for k in &prop(&s, l, "masks/#1/path").keys {
+        let p = k.value.as_path().unwrap();
+        assert_eq!(p.vertices.len(), 4, "the other key gains the vertex too");
+        assert!(p.in_tangents.len() == 4 && p.out_tangents.len() == 4, "{p:?}");
+    }
+}
+
 #[test]
 fn alt_click_stopwatch_toggles_a_self_reference_expression() {
     let (mut s, l) = setup();

crates/keyframe/src/value.rs

@@ -42,6 +42,16 @@ impl ShapePath {
     pub fn len(&self) -> usize {
         self.vertices.len()
     }
+    /// Give both tangent arrays one entry per vertex: missing tangents become zero (a corner),
+    /// extra ones are dropped. The three arrays are read independently from a project file, so a
+    /// loaded path can disagree with itself (#1250). Returns whether anything changed.
+    pub fn fit_tangents(&mut self) -> bool {
+        let n = self.vertices.len();
+        let changed = self.in_tangents.len() != n || self.out_tangents.len() != n;
+        self.in_tangents.resize(n, [0.0; 2]);
+        self.out_tangents.resize(n, [0.0; 2]);
+        changed
+    }
     /// Number of segments (vertex to next vertex, the closing one included).
     pub fn segment_count(&self) -> usize {
         let n = self.vertices.len();