Merge pull request #2876 from arnav-hiwarkar/fix/issue-2817-open-close-path

Fix opening and closing manually drawn paths

crates/engine/src/cmd.rs

@@ -49,7 +49,7 @@ mod nav_more;
 pub mod paint;
 pub mod panels_cmds;
 pub(crate) mod path_nulls;
-mod paths;
+pub(crate) mod paths;
 mod project_items;
 pub(crate) mod prop;
 mod prop_groups;

crates/engine/src/cmd/mask.rs

@@ -762,7 +762,7 @@ pub fn specs() -> Vec<CommandSpec> {
         cmd!("mask.new", "New Mask from Points", [], None, "{layer?, vertices: [[x,y]…], inTangents?, outTangents?, closed?, mode?}", has_layers, new_mask),
         cmd!("mask.addVertex", "Add Mask Vertex", [], None, "{layer?, mask, point: [x,y], in?, out?, index?}", has_layers, add_vertex),
         cmd!("mask.setVertex", "Set Mask Vertex", [], None, "{layer?, mask, index, point?, in?, out?, merge?}", has_layers, set_vertex),
-        cmd!("mask.setClosed", "Closed", ["Layer", "Mask and Shape Path"], None, "{layer?, mask, closed?}", has_layers, set_closed),
+        cmd!("mask.setClosed", "Closed", [], None, "{layer?, mask, closed?}", has_layers, set_closed),
         cmd!("mask.setRoundness", "Set Mask Roundness", [], None, "{layer?, mask, roundness (corner radius in px; 0 = square)}", has_layers, set_roundness),
         cmd!("mask.selectVertices", "Select Mask Vertices", [], None, "{vertices: [{layer, mask, index}], add?, toggle?}", has_comp, select_vertices),
         cmd!("mask.moveVertices", "Move Mask Vertices", [], None, "{vertices?: [{layer, mask, index}], delta: [dx,dy], merge?}", has_vertices, move_vertices),

crates/engine/src/cmd/paths.rs

@@ -12,6 +12,9 @@ use super::layer_menu::target_masks;
 use super::{CommandSpec, b_p, bad, f_p, has_comp, has_layers, layer_mut, layer_p, merge_p, str_p};
 use crate::{EngineError, Result, Session, VertexRef, cmd};
 
+mod closed;
+pub(crate) use closed::selected_paths_closed;
+
 // ---------------------------------------------------------------- RotoBezier
 
 /// Automatic (RotoBezier) tangents: each vertex's tangents follow the line through its
@@ -547,6 +550,7 @@ fn ungroup_shapes(s: &mut Session, p: &Value) -> Result<Value> {
 
 pub fn specs() -> Vec<CommandSpec> {
     vec![
+        cmd!("path.toggleClosed", "Closed", ["Layer", "Mask and Shape Path"], None, "{}", closed::has_paths, closed::toggle_closed),
         cmd!("path.rotoBezier", "RotoBezier", ["Layer", "Mask and Shape Path"], None, "{layer?, mask?, value?}", has_layers, roto_bezier),
         cmd!("path.convertToBezier", "Convert To Bezier Path", ["Layer", "Mask and Shape Path"], None, "{layer?, mask?}", has_layers, convert_to_bezier),
         cmd!("path.groupShapes", "Group Shapes", ["Layer"], Some("Cmd+G"), "{layer?, items?: [uid]}", has_shape_items, group_shapes),

crates/engine/src/cmd/paths/closed.rs

@@ -0,0 +1,130 @@
+//! The selection-aware Closed menu action. Explicit edits retain `mask.setClosed`'s API.
+
+use effectcraft_keyframe::Value as KV;
+use effectcraft_project::{GroupKind, Layer, LayerId, Node, PropGroup, Uid};
+use effectcraft_time::Tick;
+use serde_json::{Value, json};
+
+use super::super::{bad, layer_mut};
+use crate::{EngineError, Result, Session, VertexRef};
+
+const MAX_DEPTH: usize = 128;
+
+struct Target {
+    layer: LayerId,
+    route: Vec<usize>,
+    closed: bool,
+    editable: bool,
+}
+
+struct Selection<'a> {
+    vertices: &'a [VertexRef],
+    props: &'a [(LayerId, Uid)],
+    time: Tick,
+}
+
+/// A route uses checked child access and never recurses through an input-controlled tree.
+fn group_at_mut<'a>(mut group: &'a mut PropGroup, route: &[usize]) -> Option<&'a mut PropGroup> {
+    for &index in route {
+        group = group.children.get_mut(index)?.as_group_mut()?;
+    }
+    Some(group)
+}
+
+/// The selected paths on the selected layers, else every path of those layers (as the other
+/// Mask and Shape Path commands do). Locked layers and masks count as matches, so selecting one
+/// never falls back to its siblings, and are then left out.
+fn targets(s: &Session) -> Result<Vec<Target>> {
+    let comp = s.active_comp().ok_or(EngineError::NoComp)?;
+    let selection = Selection { vertices: &s.state.selected_vertices, props: &s.state.selected_props, time: s.time() };
+    let mut targets = Vec::new();
+    for whole_layers in [false, true] {
+        // Only selected layers: navigation and insertion can retain point/property selections
+        // on other layers, and every match below is per layer.
+        for layer in comp.layers.iter().filter(|l| s.state.selected_layers.contains(&l.id)) {
+            collect(&selection, layer, &layer.props, &mut Vec::new(), whole_layers, layer.switches.locked, &mut targets)?;
+        }
+        if !targets.is_empty() {
+            break;
+        }
+    }
+    targets.retain(|target| target.editable);
+    if targets.is_empty() {
+        return Err(bad("path.toggleClosed", "select an unlocked mask or Bezier shape path"));
+    }
+    Ok(targets)
+}
+
+fn collect(
+    selection: &Selection,
+    layer: &Layer,
+    group: &PropGroup,
+    route: &mut Vec<usize>,
+    inherited: bool,
+    locked: bool,
+    targets: &mut Vec<Target>,
+) -> Result<()> {
+    if route.len() > MAX_DEPTH {
+        return Err(bad("path.toggleClosed", "path group nesting exceeds 128 levels"));
+    }
+    let locked = locked || matches!(group.kind, GroupKind::Mask { locked: true, .. });
+    let selected = inherited || selection.props.contains(&(layer.id, group.uid));
+    if super::super::mask::is_path_group(group)
+        && let Some(prop) = group.get("path")
+        && let KV::Path(path) = prop.value_at(layer.layer_time(selection.time))
+    {
+        // Selecting any of a path group's properties (Mask Path, Feather, Opacity…) or its points
+        // selects the path.
+        let selected = selected
+            || group.children.iter().any(|c| matches!(c, Node::Prop(p) if selection.props.contains(&(layer.id, p.uid))))
+            || selection.vertices.iter().any(|v| v.layer == layer.id && v.mask == group.uid && v.index < path.vertices.len());
+        if selected {
+            targets.push(Target { layer: layer.id, route: route.clone(), closed: path.closed, editable: !locked });
+        }
+        return Ok(());
+    }
+    for (index, child) in group.children.iter().enumerate() {
+        let Some(child) = child.as_group() else { continue };
+        route.push(index);
+        collect(selection, layer, child, route, selected, locked, targets)?;
+        route.pop();
+    }
+    Ok(())
+}
+
+pub(crate) fn selected_paths_closed(s: &Session) -> Option<bool> {
+    targets(s).ok().map(|targets| targets.iter().all(|target| target.closed))
+}
+
+pub(super) fn has_paths(s: &Session) -> std::result::Result<(), String> {
+    targets(s).map(|_| ()).map_err(|error| error.to_string())
+}
+
+pub(super) fn toggle_closed(s: &mut Session, _: &Value) -> Result<Value> {
+    let targets = targets(s)?;
+    let closed = !targets.iter().all(|target| target.closed);
+    let cid = s.active_comp_id().ok_or(EngineError::NoComp)?;
+    let time = s.time();
+    s.edit("Closed", None, |project, _| {
+        for target in &targets {
+            let layer = layer_mut(project, cid, target.layer)?;
+            let time = layer.layer_time(time);
+            let group = group_at_mut(&mut layer.props, &target.route).ok_or_else(|| bad("path.toggleClosed", "the selected path no longer exists"))?;
+            let roto = super::is_roto(group);
+            let prop = group.get_mut("path").ok_or_else(|| bad("path.toggleClosed", "the selected group has no path"))?;
+            let KV::Path(mut path) = prop.value_at(time) else { return Err(bad("path.toggleClosed", "the selected value is not a path")) };
+            if path.closed == closed {
+                continue;
+            }
+            // As `mask.setClosed`: a path read from a file may have fewer tangents than vertices (#1250).
+            path.fit_tangents();
+            path.closed = closed;
+            if roto {
+                super::roto_smooth(&mut path);
+            }
+            prop.set_value_at(time, KV::Path(path));
+        }
+        Ok(())
+    })?;
+    Ok(json!({"paths": targets.len(), "closed": closed}))
+}

crates/engine/src/menus.rs

@@ -320,6 +320,7 @@ pub fn checked(s: &Session, command: &str, params: &Value) -> Option<bool> {
     let layer = s.state.selected_layers.first().and_then(|l| comp.layer(*l));
     let pstr = |k: &str| params.get(k).and_then(Value::as_str);
     match command {
+        "path.toggleClosed" => crate::cmd::paths::selected_paths_closed(s),
         "layer.setBlendMode" => {
             let want = effectcraft_color::BlendMode::from_name(pstr("mode")?)?;
             Some(layer?.blend_mode == want)
@@ -829,7 +830,7 @@ Layer
     Hide Locked Masks | layer.mask.hideLocked
   Mask and Shape Path
     RotoBezier | path.rotoBezier
-    Closed | mask.setClosed
+    Closed | path.toggleClosed
     Convert To Bezier Path | path.convertToBezier
     Set First Vertex | path.setFirstVertex
     Free Transform Points | path.freeTransform

crates/engine/src/menus/tests.rs

@@ -159,6 +159,44 @@ fn checked_reflects_layer_state() {
     assert_eq!(checked(&s, "file.save", &serde_json::json!({})), None);
 }
 
+#[test]
+fn closed_menu_opens_and_recloses_a_manual_shape() {
+    use serde_json::json;
+    let mut s = crate::Session::default();
+    s.execute("comp.new", json!({"width": 320, "height": 180})).unwrap();
+    let r = s
+        .execute(
+            "shape.newPath",
+            json!({"vertices": [[0, 0], [100, 0], [100, 100]], "closed": true,
+                "inTangents": [[0, -12], [-8, 0], [0, -6]], "outTangents": [[12, 0], [0, 8], [-6, 0]]}),
+        )
+        .unwrap();
+    let lid = effectcraft_project::LayerId(r["layer"].as_u64().unwrap());
+    let uid = r["path"].as_u64().unwrap();
+    let path =
+        |s: &crate::Session| s.active_comp().unwrap().layer(lid).unwrap().props.find_group(uid).unwrap().get("path").unwrap().value.as_path().unwrap().clone();
+    let original = path(&s);
+    for mac in [true, false] {
+        let (_, entry) =
+            platform_entries(mac).into_iter().find(|(p, e)| p.iter().map(String::as_str).eq(["Layer", "Mask and Shape Path"]) && e.label == "Closed").unwrap();
+        assert!(s.is_enabled(&entry.command));
+        assert_eq!(checked(&s, &entry.command, &entry.params), Some(true));
+        s.execute(&entry.command, entry.params.clone()).unwrap();
+        let mut opened = original.clone();
+        opened.closed = false;
+        assert_eq!(path(&s), opened);
+        assert!(!effectcraft_path::to_kurbo(&path(&s)).elements().contains(&effectcraft_path::PathEl::ClosePath));
+        assert_eq!(checked(&s, &entry.command, &entry.params), Some(false));
+        s.execute("edit.undo", json!({})).unwrap();
+        assert_eq!(path(&s), original);
+        s.execute("edit.redo", json!({})).unwrap();
+        assert_eq!(path(&s), opened);
+        s.execute(&entry.command, entry.params).unwrap();
+        assert_eq!(path(&s), original);
+        assert!(effectcraft_path::to_kurbo(&path(&s)).elements().contains(&effectcraft_path::PathEl::ClosePath));
+    }
+}
+
 /// `cargo test -p effectcraft-engine dump_specs -- --ignored --nocapture`: every command as TSV.
 #[test]
 #[ignore]

crates/engine/src/tests/viewer.rs

@@ -346,6 +346,263 @@ fn shape_path(s: &Session, lid: u64, uid: u64) -> effectcraft_keyframe::ShapePat
     l.props.find_group(uid).unwrap().get("path").unwrap().value.as_path().unwrap().clone()
 }
 
+#[test]
+fn toggle_closed_respects_path_and_vertex_selection() {
+    let mut s = comp();
+    let a = s.execute("shape.newPath", json!({"vertices": [[0, 0], [100, 0], [100, 100]], "closed": true})).unwrap();
+    let lid = a["layer"].as_u64().unwrap();
+    let a = a["path"].as_u64().unwrap();
+    let b = s.execute("shape.newPath", json!({"layer": lid, "vertices": [[10, 10], [90, 10], [90, 90]], "closed": true})).unwrap()["path"].as_u64().unwrap();
+    let leaf = layer(&s, lid).props.find_group(a).unwrap().get("path").unwrap().uid;
+    s.state.selected_vertices.clear();
+    s.state.selected_props = vec![(LayerId(lid), leaf)];
+    s.execute("path.toggleClosed", json!({})).unwrap();
+    assert!(!shape_path(&s, lid, a).closed);
+    assert!(shape_path(&s, lid, b).closed, "an unselected sibling stays closed");
+    s.state.selected_props = vec![(LayerId(lid), b)];
+    s.state.selected_vertices = vec![crate::VertexRef { layer: LayerId(lid), mask: a, index: 1 }];
+    s.execute("path.toggleClosed", json!({})).unwrap();
+    assert!(shape_path(&s, lid, a).closed, "selected points and properties both describe target paths");
+    assert!(shape_path(&s, lid, b).closed);
+    s.state.selected_vertices.clear();
+    s.state.selected_props = vec![(LayerId(lid), a), (LayerId(lid), leaf), (LayerId(lid), b)];
+    let r = s.execute("path.toggleClosed", json!({})).unwrap();
+    assert_eq!(r["paths"], json!(2));
+    assert!(!shape_path(&s, lid, a).closed);
+    assert!(!shape_path(&s, lid, b).closed);
+    s.execute("edit.undo", json!({})).unwrap();
+    assert!(shape_path(&s, lid, a).closed);
+    assert!(shape_path(&s, lid, b).closed, "all paths undo in one step");
+    s.execute("mask.setClosed", json!({"layer": lid, "mask": a, "closed": false})).unwrap();
+    assert_eq!(crate::menus::checked(&s, "path.toggleClosed", &json!({})), Some(false), "a mixed selection is unchecked");
+    s.execute("path.toggleClosed", json!({})).unwrap();
+    assert!(shape_path(&s, lid, a).closed);
+    assert!(shape_path(&s, lid, b).closed, "a mixed selection closes all paths");
+    s.state.selected_props.clear();
+    s.execute("path.toggleClosed", json!({})).unwrap();
+    assert!(!shape_path(&s, lid, a).closed);
+    assert!(!shape_path(&s, lid, b).closed, "layer selection addresses all editable paths");
+}
+
+#[test]
+fn toggle_closed_combines_property_and_shape_group_selections() {
+    let mut s = comp();
+    let a = s.execute("shape.newPath", json!({"vertices": [[0, 0], [100, 0], [100, 100]], "closed": true})).unwrap();
+    let (lid, a_path) = (a["layer"].as_u64().unwrap(), a["path"].as_u64().unwrap());
+    let b = s.execute("shape.newPath", json!({"layer": lid, "vertices": [[10, 10], [90, 10], [90, 90]], "closed": true})).unwrap();
+    let (b_path, b_group) = (b["path"].as_u64().unwrap(), b["group"].as_u64().unwrap());
+    let c_path =
+        s.execute("shape.newPath", json!({"layer": lid, "vertices": [[20, 20], [80, 20], [80, 80]], "closed": true})).unwrap()["path"].as_u64().unwrap();
+    s.execute("prop.select", json!({"layer": lid, "prop": a_path})).unwrap();
+    s.execute("prop.select", json!({"layer": lid, "prop": b_group, "add": true})).unwrap();
+    assert!(s.state.selected_vertices.iter().all(|v| v.mask == a_path), "only the selected path group implies selected vertices");
+    assert!(!s.state.selected_vertices.is_empty());
+    assert!(s.state.selected_props.contains(&(LayerId(lid), b_group)));
+    let result = s.execute("path.toggleClosed", json!({})).unwrap();
+    assert_eq!(result["paths"], json!(2));
+    assert!(!shape_path(&s, lid, a_path).closed);
+    assert!(!shape_path(&s, lid, b_path).closed, "the added shape group is included alongside implied vertices");
+    assert!(shape_path(&s, lid, c_path).closed, "unselected sibling stays unchanged");
+    s.execute("edit.undo", json!({})).unwrap();
+    assert!(shape_path(&s, lid, a_path).closed);
+    assert!(shape_path(&s, lid, b_path).closed, "both selected paths undo together");
+    assert!(shape_path(&s, lid, c_path).closed);
+}
+
+#[test]
+fn toggle_closed_ignores_selections_retained_on_other_layers() {
+    let mut s = comp();
+    let a = s.execute("shape.newPath", json!({"vertices": [[0, 0], [100, 0], [100, 100]], "closed": true})).unwrap();
+    let (a_layer, a_path) = (a["layer"].as_u64().unwrap(), a["path"].as_u64().unwrap());
+    let b_layer = s.execute("layer.newShape", json!({"kind": "none"})).unwrap()["layer"].as_u64().unwrap();
+    assert!(s.state.selected_vertices.iter().any(|v| v.layer == LayerId(a_layer)), "insertion retains old vertices");
+    assert!(!s.is_enabled("path.toggleClosed"));
+    assert!(s.execute("path.toggleClosed", json!({})).is_err());
+    assert!(shape_path(&s, a_layer, a_path).closed);
+
+    let b_path =
+        s.execute("shape.newPath", json!({"layer": b_layer, "vertices": [[0, 0], [50, 0], [50, 50]], "closed": true})).unwrap()["path"].as_u64().unwrap();
+    s.execute("prop.select", json!({"layer": b_layer, "prop": b_path})).unwrap();
+    s.execute("layer.selectNext", json!({})).unwrap();
+    assert_eq!(s.state.selected_layers, vec![LayerId(a_layer)]);
+    assert!(s.state.selected_vertices.iter().any(|v| v.layer == LayerId(b_layer)), "navigation retains old vertices");
+    assert!(s.state.selected_props.contains(&(LayerId(b_layer), b_path)));
+    assert_eq!(crate::menus::checked(&s, "path.toggleClosed", &json!({})), Some(true));
+    s.execute("path.toggleClosed", json!({})).unwrap();
+    assert!(!shape_path(&s, a_layer, a_path).closed);
+    assert!(shape_path(&s, b_layer, b_path).closed, "the deselected layer stays unchanged");
+    s.execute("layer.selectPrevious", json!({})).unwrap();
+    assert_eq!(s.state.selected_layers, vec![LayerId(b_layer)]);
+    s.execute("prop.select", json!({"layer": b_layer, "path": "transform/position"})).unwrap();
+    assert_eq!(crate::menus::checked(&s, "path.toggleClosed", &json!({})), Some(true), "a non-path property falls back to the layer's paths");
+    s.execute("path.toggleClosed", json!({})).unwrap();
+    assert!(!shape_path(&s, b_layer, b_path).closed);
+    s.execute("edit.undo", json!({})).unwrap();
+    s.execute("layer.selectNext", json!({})).unwrap();
+    s.execute("path.toggleClosed", json!({})).unwrap();
+    assert!(shape_path(&s, a_layer, a_path).closed, "a property retained on another layer cannot block this layer");
+    assert!(shape_path(&s, b_layer, b_path).closed);
+}
+
+#[test]
+fn toggle_closed_falls_back_from_stale_selection_and_rejects_locked_or_pathless_layers() {
+    let mut s = comp();
+    let r = s.execute("shape.newPath", json!({"vertices": [[0, 0], [100, 0], [100, 100]], "closed": true})).unwrap();
+    let (lid, uid) = (r["layer"].as_u64().unwrap(), r["path"].as_u64().unwrap());
+    let before = s.project.clone();
+    s.state.selected_vertices.clear();
+    s.state.selected_props = vec![(LayerId(lid), u64::MAX)];
+    s.execute("path.toggleClosed", json!({})).unwrap();
+    assert!(!shape_path(&s, lid, uid).closed, "a stale property selection falls back to the layer's paths");
+    s.execute("edit.undo", json!({})).unwrap();
+    assert!(s.project.same_content(&before));
+    s.state.selected_props.clear();
+    s.state.selected_vertices = vec![crate::VertexRef { layer: LayerId(lid), mask: uid, index: usize::MAX }];
+    s.execute("path.toggleClosed", json!({})).unwrap();
+    assert!(!shape_path(&s, lid, uid).closed, "a stale vertex selection falls back to the layer's paths");
+    s.execute("edit.undo", json!({})).unwrap();
+    s.state.selected_vertices.clear();
+    let cid = s.active_comp_id().unwrap();
+    std::sync::Arc::make_mut(&mut s.project).comp_mut(cid).unwrap().layer_mut(LayerId(lid)).unwrap().switches.locked = true;
+    assert!(!s.is_enabled("path.toggleClosed"));
+    assert!(s.execute("path.toggleClosed", json!({})).is_err());
+    assert!(shape_path(&s, lid, uid).closed);
+    s.execute("layer.newShape", json!({"kind": "rect"})).unwrap();
+    s.state.selected_props.clear();
+    assert!(!s.is_enabled("path.toggleClosed"));
+    assert!(s.execute("path.toggleClosed", json!({})).is_err());
+}
+
+#[test]
+fn toggle_closed_handles_masks_and_preserves_explicit_set_closed() {
+    let mut s = comp();
+    let lid = s.execute("layer.newSolid", json!({})).unwrap()["layer"].as_u64().unwrap();
+    let uid = s.execute("mask.new", json!({"layer": lid, "vertices": [[0, 0], [100, 0], [100, 100]], "closed": true})).unwrap()["mask"].as_u64().unwrap();
+    let path = |s: &Session| layer(s, lid).props.find_group(uid).unwrap().get("path").unwrap().value.as_path().unwrap().clone();
+    let original = path(&s);
+    let other = s.execute("mask.new", json!({"layer": lid, "vertices": [[10, 10], [90, 10], [90, 90]], "closed": true})).unwrap()["mask"].as_u64().unwrap();
+    let other_closed = |s: &Session| layer(s, lid).props.find_group(other).unwrap().get("path").unwrap().value.as_path().unwrap().closed;
+    let feather = layer(&s, lid).props.find_group(uid).unwrap().get("feather").unwrap().uid;
+    s.state.selected_vertices.clear();
+    s.state.selected_props = vec![(LayerId(lid), feather)];
+    s.execute("path.toggleClosed", json!({})).unwrap();
+    assert!(!path(&s).closed, "selecting Mask Feather selects its mask");
+    assert!(other_closed(&s), "an unselected mask stays closed");
+    assert!(layer(&s, lid).props.find_group(uid).unwrap().get("path").unwrap().keys.is_empty(), "a static path gains no keyframe");
+    s.state.selected_props = vec![(LayerId(lid), uid)];
+    s.execute("mask.setClosed", json!({"layer": lid, "mask": uid})).unwrap();
+    assert_eq!(path(&s), original, "explicit API still defaults to closed");
+    s.execute("mask.setClosed", json!({"layer": lid, "mask": uid, "closed": false})).unwrap();
+    assert!(!path(&s).closed);
+    s.execute("path.toggleClosed", json!({})).unwrap();
+    assert_eq!(path(&s), original);
+    let cid = s.active_comp_id().unwrap();
+    let g = std::sync::Arc::make_mut(&mut s.project).comp_mut(cid).unwrap().layer_mut(LayerId(lid)).unwrap().props.find_group_mut(uid).unwrap();
+    if let effectcraft_project::GroupKind::Mask { locked, .. } = &mut g.kind {
+        *locked = true;
+    }
+    assert!(!s.is_enabled("path.toggleClosed"));
+    assert!(s.execute("path.toggleClosed", json!({})).is_err());
+    assert_eq!(path(&s), original);
+    assert!(other_closed(&s), "a selected locked mask does not fall back to its unlocked siblings");
+}
+
+#[test]
+fn toggle_closed_recomputes_roto_bezier_tangents() {
+    let mut s = comp();
+    let lid = s.execute("layer.newSolid", json!({})).unwrap()["layer"].as_u64().unwrap();
+    let uid =
+        s.execute("mask.new", json!({"layer": lid, "vertices": [[0, 0], [100, 0], [100, 100], [0, 100]], "closed": true})).unwrap()["mask"].as_u64().unwrap();
+    s.execute("path.rotoBezier", json!({"layer": lid, "value": true})).unwrap();
+    let path = |s: &Session| layer(s, lid).props.find_group(uid).unwrap().get("path").unwrap().value.as_path().unwrap().clone();
+    let closed = path(&s);
+    assert_ne!(closed.out_tangents[0], [0.0; 2], "a closed RotoBezier path has tangents at every vertex");
+    s.state.selected_props = vec![(LayerId(lid), uid)];
+    s.execute("path.toggleClosed", json!({})).unwrap();
+    let open = path(&s);
+    assert!(!open.closed);
+    assert_eq!((open.in_tangents[0], open.out_tangents[3]), ([0.0; 2], [0.0; 2]), "open ends lose their automatic tangents");
+    assert_ne!(open.out_tangents[1], [0.0; 2]);
+    s.execute("path.toggleClosed", json!({})).unwrap();
+    assert_eq!(path(&s), closed, "reclosing restores the closed RotoBezier tangents");
+}
+
+#[test]
+fn toggle_closed_fits_short_tangent_lists() {
+    let mut s = comp();
+    let lid = s.execute("layer.newSolid", json!({})).unwrap()["layer"].as_u64().unwrap();
+    let uid = s.execute("mask.new", json!({"layer": lid, "vertices": [[0, 0], [100, 0], [100, 100]], "closed": true})).unwrap()["mask"].as_u64().unwrap();
+    let cid = s.active_comp_id().unwrap();
+    let layer_mut = std::sync::Arc::make_mut(&mut s.project).comp_mut(cid).unwrap().layer_mut(LayerId(lid)).unwrap();
+    let prop = layer_mut.props.find_group_mut(uid).unwrap().get_mut("path").unwrap();
+    let KV::Path(path) = &mut prop.value else { panic!("not a path") };
+    // A file can store fewer tangents than vertices (#1250).
+    path.in_tangents.clear();
+    path.out_tangents.truncate(1);
+    s.state.selected_props = vec![(LayerId(lid), uid)];
+    s.execute("path.toggleClosed", json!({})).unwrap();
+    let path = layer(&s, lid).props.find_group(uid).unwrap().get("path").unwrap().value.as_path().unwrap().clone();
+    assert!(!path.closed);
+    assert_eq!((path.in_tangents.len(), path.out_tangents.len()), (3, 3));
+}
+
+#[test]
+fn toggle_closed_edits_layer_time_and_roundtrips() {
+    use effectcraft_keyframe::Keyframe;
+    let mut s = comp();
+    let r = s.execute("shape.newPath", json!({"vertices": [[0, 0], [100, 0], [100, 100]], "closed": true})).unwrap();
+    let (lid, uid) = (r["layer"].as_u64().unwrap(), r["path"].as_u64().unwrap());
+    let cid = s.active_comp_id().unwrap();
+    let original = shape_path(&s, lid, uid);
+    let second = Tick::from_seconds_f64(1.0);
+    let layer = std::sync::Arc::make_mut(&mut s.project).comp_mut(cid).unwrap().layer_mut(LayerId(lid)).unwrap();
+    layer.start_time = second;
+    layer.stretch = 200.0;
+    let prop = layer.props.find_group_mut(uid).unwrap().get_mut("path").unwrap();
+    prop.keys = vec![Keyframe::new(Tick::ZERO, KV::Path(original.clone())), Keyframe::new(second, KV::Path(original.clone()))];
+    let keys = prop.keys.clone();
+    s.execute("time.set", json!({"time": 3.0})).unwrap();
+    s.execute("path.toggleClosed", json!({})).unwrap();
+    let prop = s.project.comp(cid).unwrap().layer(LayerId(lid)).unwrap().props.find_group(uid).unwrap().get("path").unwrap();
+    assert_eq!(prop.keys.len(), 2);
+    assert_eq!(prop.keys[0], keys[0], "the other keyframe is untouched");
+    let mut opened = original;
+    opened.closed = false;
+    assert_eq!(prop.value_at(second), KV::Path(opened.clone()));
+    assert!(prop.value_at(Tick::from_seconds_f64(0.5)).as_path().unwrap().closed, "the earlier segment keeps its closedness");
+    let bytes = serde_json::to_vec(&*s.project).unwrap();
+    let restored: effectcraft_project::Project = serde_json::from_slice(&bytes).unwrap();
+    let prop = restored.comp(cid).unwrap().layer(LayerId(lid)).unwrap().props.find_group(uid).unwrap().get("path").unwrap();
+    assert_eq!(prop.value_at(second), KV::Path(opened));
+    s.execute("time.set", json!({"time": 2.0})).unwrap();
+    assert_eq!(crate::menus::checked(&s, "path.toggleClosed", &json!({})), Some(true));
+    s.execute("time.set", json!({"time": 3.0})).unwrap();
+    assert_eq!(crate::menus::checked(&s, "path.toggleClosed", &json!({})), Some(false));
+}
+
+#[test]
+fn toggle_closed_rejects_excessive_group_nesting_without_an_edit() {
+    use effectcraft_project::build::{self, Ids};
+    let mut s = comp();
+    let r = s.execute("shape.newPath", json!({"vertices": [[0, 0], [100, 0], [100, 100]], "closed": true})).unwrap();
+    let lid = LayerId(r["layer"].as_u64().unwrap());
+    let cid = s.active_comp_id().unwrap();
+    let project = std::sync::Arc::make_mut(&mut s.project);
+    let mut next = project.next_id;
+    let contents = project.comp_mut(cid).unwrap().layer_mut(lid).unwrap().props.sub_mut("contents").unwrap();
+    let node = contents.children.first_mut().unwrap();
+    let mut nested = node.as_group().unwrap().clone();
+    for _ in 0..130 {
+        nested = build::shape_group(&mut Ids(&mut next), "Nested", vec![nested]);
+    }
+    *node = effectcraft_project::Node::Group(nested);
+    project.next_id = next;
+    let before = s.project.clone();
+    assert!(!s.is_enabled("path.toggleClosed"));
+    assert!(s.execute("path.toggleClosed", json!({})).is_err());
+    assert!(s.project.same_content(&before));
+}
+
 #[test]
 fn shape_pen_builds_shape_group_and_renders() {
     let mut s = comp();

crates/ui-egui/src/i18n.rs

@@ -701,7 +701,7 @@ const SIMPLIFIED_CHINESE: &[(&str, &str, &str)] = &[
     ("layer.mask.hideLocked", "Hide Locked Masks", "隐藏锁定的蒙版"),
     ("", "Mask and Shape Path", "蒙版和形状路径"),
     ("path.rotoBezier", "RotoBezier", "RotoBezier"),
-    ("mask.setClosed", "Closed", "已关闭"),
+    ("path.toggleClosed", "Closed", "已关闭"),
     ("path.convertToBezier", "Convert To Bezier Path", "转换为贝塞尔曲线路径"),
     ("path.setFirstVertex", "Set First Vertex", "设置第一个顶点"),
     ("path.freeTransform", "Free Transform Points", "自由变换点"),
@@ -1339,7 +1339,7 @@ const SWEDISH: &[(&str, &str, &str)] = &[
     ("layer.mask.hideLocked", "Hide Locked Masks", "Dölj låsta masker"),
     ("", "Mask and Shape Path", "Mask- och formbana"),
     ("path.rotoBezier", "RotoBezier", "RotoBezier"),
-    ("mask.setClosed", "Closed", "Sluten"),
+    ("path.toggleClosed", "Closed", "Sluten"),
     ("path.convertToBezier", "Convert To Bezier Path", "Konvertera till Bezierbana"),
     ("path.setFirstVertex", "Set First Vertex", "Ange första hörnpunkt"),
     ("path.freeTransform", "Free Transform Points", "Fri transformering av punkter"),
@@ -1983,7 +1983,7 @@ const TRADITIONAL_CHINESE: &[(&str, &str, &str)] = &[
     ("layer.mask.hideLocked", "Hide Locked Masks", "隱藏鎖定的遮色片"),
     ("", "Mask and Shape Path", "遮色片和形狀路徑"),
     ("path.rotoBezier", "RotoBezier", "RotoBezier"),
-    ("mask.setClosed", "Closed", "封閉"),
+    ("path.toggleClosed", "Closed", "封閉"),
     ("path.convertToBezier", "Convert To Bezier Path", "轉換為貝茲路徑"),
     ("path.setFirstVertex", "Set First Vertex", "設定第一個頂點"),
     ("path.freeTransform", "Free Transform Points", "自由變換點"),
@@ -2622,7 +2622,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[
     ("layer.mask.hideLocked", "Hide Locked Masks", "ロックしたマスクを非表示"),
     ("", "Mask and Shape Path", "マスクとシェイプのパス"),
     ("path.rotoBezier", "RotoBezier", "ロトベジェ"),
-    ("mask.setClosed", "Closed", "閉じたパス"),
+    ("path.toggleClosed", "Closed", "閉じたパス"),
     ("path.convertToBezier", "Convert To Bezier Path", "ベジェパスに変換"),
     ("path.setFirstVertex", "Set First Vertex", "最初の頂点を設定"),
     ("path.freeTransform", "Free Transform Points", "ポイントを自由変形"),
@@ -3264,7 +3264,7 @@ const UKRAINIAN: &[(&str, &str, &str)] = &[
     ("layer.mask.hideLocked", "Hide Locked Masks", "Приховати заблоковані маски"),
     ("", "Mask and Shape Path", "Контур маски та фігури"),
     ("path.rotoBezier", "RotoBezier", "RotoBezier"),
-    ("mask.setClosed", "Closed", "Замкнений"),
+    ("path.toggleClosed", "Closed", "Замкнений"),
     ("path.convertToBezier", "Convert To Bezier Path", "Перетворити на контур Безьє"),
     ("path.setFirstVertex", "Set First Vertex", "Задати першу вершину"),
     ("path.freeTransform", "Free Transform Points", "Вільне трансформування точок"),
@@ -3907,7 +3907,7 @@ const SPANISH: &[(&str, &str, &str)] = &[
     ("layer.mask.hideLocked", "Hide Locked Masks", "Ocultar máscaras bloqueadas"),
     ("", "Mask and Shape Path", "Trazado de máscara y de forma"),
     ("path.rotoBezier", "RotoBezier", "RotoBezier"),
-    ("mask.setClosed", "Closed", "Cerrado"),
+    ("path.toggleClosed", "Closed", "Cerrado"),
     ("path.convertToBezier", "Convert To Bezier Path", "Convertir en trazado Bézier"),
     ("path.setFirstVertex", "Set First Vertex", "Establecer primer vértice"),
     ("path.freeTransform", "Free Transform Points", "Transformar puntos libremente"),
@@ -4548,7 +4548,7 @@ const PORTUGUESE: &[(&str, &str, &str)] = &[
     ("layer.mask.hideLocked", "Hide Locked Masks", "Ocultar máscaras bloqueadas"),
     ("", "Mask and Shape Path", "Caminho da máscara e da forma"),
     ("path.rotoBezier", "RotoBezier", "RotoBezier"),
-    ("mask.setClosed", "Closed", "Fechado"),
+    ("path.toggleClosed", "Closed", "Fechado"),
     ("path.convertToBezier", "Convert To Bezier Path", "Converter em caminho Bézier"),
     ("path.setFirstVertex", "Set First Vertex", "Definir primeiro vértice"),
     ("path.freeTransform", "Free Transform Points", "Pontos de transformação livre"),
@@ -5196,7 +5196,7 @@ const RUSSIAN: &[(&str, &str, &str)] = &[
     ("layer.mask.hideLocked", "Hide Locked Masks", "Скрыть заблокированные маски"),
     ("", "Mask and Shape Path", "Контур маски и фигуры"),
     ("path.rotoBezier", "RotoBezier", "RotoBezier"),
-    ("mask.setClosed", "Closed", "Замкнутый"),
+    ("path.toggleClosed", "Closed", "Замкнутый"),
     ("path.convertToBezier", "Convert To Bezier Path", "Преобразовать в кривую Безье"),
     ("path.setFirstVertex", "Set First Vertex", "Задать первую вершину"),
     ("path.freeTransform", "Free Transform Points", "Свободное трансформирование точек"),
@@ -5837,7 +5837,7 @@ const FRENCH: &[(&str, &str, &str)] = &[
     ("layer.mask.hideLocked", "Hide Locked Masks", "Masquer les masques verrouillés"),
     ("", "Mask and Shape Path", "Trajectoire de masque et de forme"),
     ("path.rotoBezier", "RotoBezier", "RotoBezier"),
-    ("mask.setClosed", "Closed", "Fermé"),
+    ("path.toggleClosed", "Closed", "Fermé"),
     ("path.convertToBezier", "Convert To Bezier Path", "Convertir en trajectoire Bézier"),
     ("path.setFirstVertex", "Set First Vertex", "Définir le premier sommet"),
     ("path.freeTransform", "Free Transform Points", "Transformation libre des points"),
@@ -6478,7 +6478,7 @@ const GERMAN: &[(&str, &str, &str)] = &[
     ("layer.mask.hideLocked", "Hide Locked Masks", "Gesperrte Masken ausblenden"),
     ("", "Mask and Shape Path", "Masken- und Formpfad"),
     ("path.rotoBezier", "RotoBezier", "RotoBezier"),
-    ("mask.setClosed", "Closed", "Geschlossen"),
+    ("path.toggleClosed", "Closed", "Geschlossen"),
     ("path.convertToBezier", "Convert To Bezier Path", "In Bézier-Pfad konvertieren"),
     ("path.setFirstVertex", "Set First Vertex", "Ersten Scheitelpunkt festlegen"),
     ("path.freeTransform", "Free Transform Points", "Punkte frei transformieren"),
@@ -7118,7 +7118,7 @@ const POLISH: &[(&str, &str, &str)] = &[
     ("layer.mask.hideLocked", "Hide Locked Masks", "Ukryj zablokowane maski"),
     ("", "Mask and Shape Path", "Ścieżka maski i kształtu"),
     ("path.rotoBezier", "RotoBezier", "RotoBezier"),
-    ("mask.setClosed", "Closed", "Zamknięta"),
+    ("path.toggleClosed", "Closed", "Zamknięta"),
     ("path.convertToBezier", "Convert To Bezier Path", "Konwertuj na ścieżkę Béziera"),
     ("path.setFirstVertex", "Set First Vertex", "Ustaw pierwszy wierzchołek"),
     ("path.freeTransform", "Free Transform Points", "Swobodne przekształcanie punktów"),
@@ -7758,7 +7758,7 @@ const CZECH: &[(&str, &str, &str)] = &[
     ("layer.mask.hideLocked", "Hide Locked Masks", "Skrýt zamknuté masky"),
     ("", "Mask and Shape Path", "Cesta masky a tvaru"),
     ("path.rotoBezier", "RotoBezier", "RotoBezier"),
-    ("mask.setClosed", "Closed", "Uzavřená"),
+    ("path.toggleClosed", "Closed", "Uzavřená"),
     ("path.convertToBezier", "Convert To Bezier Path", "Převést na Bézierovu cestu"),
     ("path.setFirstVertex", "Set First Vertex", "Nastavit první vrchol"),
     ("path.freeTransform", "Free Transform Points", "Volně transformovat body"),
@@ -8395,7 +8395,7 @@ const GEORGIAN: &[(&str, &str, &str)] = &[
     ("layer.mask.hideLocked", "Hide Locked Masks", "ჩაკეტილი ნიღბების დამალვა"),
     ("", "Mask and Shape Path", "ნიღაბი და ფიგურის ტრაექტორია"),
     ("path.rotoBezier", "RotoBezier", "RotoBezier"),
-    ("mask.setClosed", "Closed", "შეკრული"),
+    ("path.toggleClosed", "Closed", "შეკრული"),
     ("path.convertToBezier", "Convert To Bezier Path", "ბეზიეს ტრაექტორიად გარდაქმნა"),
     ("path.setFirstVertex", "Set First Vertex", "პირველი წვეროს დაყენება"),
     ("path.freeTransform", "Free Transform Points", "წერტილების თავისუფალი გარდაქმნა"),
@@ -9308,12 +9308,16 @@ mod tests {
             alt_shortcuts: Vec::new(),
         };
         let new_comp = menu_entry("comp.new", "New Composition...");
+        let closed_path = menu_entry("path.toggleClosed", "Closed");
         let undo = menu_entry("edit.undo", "Undo");
         for code in CATALOG_CODES {
             app.session.execute("prefs.set", json!({"key":"general.language", "value":code})).unwrap();
             let drawn = entry(&app, &new_comp, new_comp.label.clone());
             assert_eq!(drawn, label(&app, "comp.new", "New Composition..."), "{code}");
             assert_ne!(drawn, new_comp.label, "{code}: the menu entry is drawn in English");
+            let closed_label = entry(&app, &closed_path, closed_path.label.clone());
+            assert_eq!(closed_label, label(&app, "path.toggleClosed", "Closed"), "{code}");
+            assert_ne!(closed_label, closed_path.label, "{code}: Closed is drawn in English");
         }
         app.session.execute("prefs.set", json!({"key":"general.language", "value":"ru"})).unwrap();
         assert_eq!(entry(&app, &undo, "Can't Undo".into()), "Нечего отменять");