Merge pull request #182 from storytold/fix/146-151-152-153-project-text

Project and text: row clicks, item labels, tracking alignment, Delete on groups (#146, #151–#153)

crates/engine/src/commands/edit.rs

@@ -147,6 +147,12 @@ fn delete(s: &mut Session, p: &Value) -> Result<Value> {
     if p.get("layers").is_none() && !super::effect::selected_effects(s).is_empty() {
         return s.execute("effect.remove", json!({}));
     }
+    // Masks, shape items, text animators or trackers selected → remove them.
+    if p.get("layers").is_none()
+        && let Some(n) = super::prop_groups::remove_selected(s)?
+    {
+        return Ok(n);
+    }
     let (cid, ids) = layers_p(s, p)?;
     let ids = super::unlocked(s, cid, ids, "edit.clear")?;
     // Children of deleted layers are unparented where they are (like After Effects).

crates/engine/src/commands/prop_groups.rs

@@ -51,17 +51,38 @@ fn set_enabled(s: &mut Session, p: &Value) -> Result<Value> {
 }
 
 fn remove(s: &mut Session, p: &Value) -> Result<Value> {
-    let c = "prop.removeGroup";
-    let (cid, lid, uid) = group_ref(s, p, c, true)?;
+    let (cid, lid, uid) = group_ref(s, p, "prop.removeGroup", true)?;
+    remove_groups(s, cid, &[(lid, uid)])?;
+    Ok(Value::Null)
+}
+
+/// Remove groups of comp `cid` in one undo step.
+fn remove_groups(s: &mut Session, cid: ItemId, groups: &[(LayerId, Uid)]) -> Result<()> {
     s.edit("Delete", None, |proj, st| {
-        let l = layer_mut(proj, cid, lid)?;
-        let parent = l.props.parent_of_mut(uid).ok_or_else(|| bad(c, "group vanished"))?;
-        parent.children.retain(|n| n.uid() != uid);
-        st.selected_props.retain(|(_, u)| *u != uid);
-        Ok(Value::Null)
+        for (lid, uid) in groups {
+            // A group inside one removed before it went with it.
+            if let Some(parent) = layer_mut(proj, cid, *lid)?.props.parent_of_mut(*uid) {
+                parent.children.retain(|n| n.uid() != *uid);
+            }
+        }
+        st.selected_props.retain(|(_, u)| !groups.iter().any(|(_, g)| g == u));
+        Ok(())
     })
 }
 
+/// Edit ▸ Clear with masks, shape items, text animators or trackers selected in the active comp:
+/// removes those groups (not their layers) in one undo step. None when none are selected.
+pub(crate) fn remove_selected(s: &mut Session) -> Result<Option<Value>> {
+    let (Some(cid), Some(c)) = (s.active_comp_id(), s.active_comp()) else { return Ok(None) };
+    let groups: Vec<(LayerId, Uid)> =
+        s.state.selected_props.iter().filter(|(lid, uid)| c.layer(*lid).and_then(|l| l.props.find_group(*uid)).is_some_and(is_instance)).copied().collect();
+    if groups.is_empty() {
+        return Ok(None);
+    }
+    remove_groups(s, cid, &groups)?;
+    Ok(Some(json!(groups.len())))
+}
+
 fn duplicate(s: &mut Session, p: &Value) -> Result<Value> {
     let c = "prop.duplicateGroup";
     let (cid, lid, uid) = group_ref(s, p, c, true)?;

crates/engine/src/tests_project_items.rs

@@ -1,6 +1,6 @@
 //! Project panel item edits: move into folders, rename, label, comment, with undo.
 
-use effectcraft_project::ItemId;
+use effectcraft_project::{ItemId, LayerId};
 use serde_json::json;
 
 use crate::Session;
@@ -72,6 +72,34 @@ fn delete_and_duplicate_items() {
     assert!(s.project.item(dup).is_none());
 }
 
+/// A new layer takes its Project item's label (#153); layers made before keep their own when the
+/// item's label changes. Layers with no item keep their type's default.
+#[test]
+fn new_layers_take_their_items_label() {
+    use crate::color::Label;
+    use effectcraft_project::{Footage, FootageKind, ItemKind};
+    let mut s = Session::default();
+    let inner = s.execute("comp.new", json!({"name": "Inner", "width": 64, "height": 64, "duration": 1})).unwrap()["comp"].as_u64().unwrap();
+    s.execute("comp.new", json!({"name": "Main", "width": 64, "height": 64, "duration": 1})).unwrap();
+    s.execute("file.importPlaceholder", json!({"name": "Clip"})).unwrap();
+    let clip = s.state.project_selection[0].0;
+    let model = std::sync::Arc::make_mut(&mut s.project)
+        .add_item("Model", Label::Aqua, None, ItemKind::Footage(Footage { kind: FootageKind::Model, path: "missing.glb".into(), ..Default::default() }))
+        .0;
+    let solid = s.execute("layer.newSolid", json!({"name": "Solid", "color": "#406080"})).unwrap()["layer"].as_u64().unwrap();
+    let solid_item = s.active_comp().unwrap().layer(LayerId(solid)).unwrap().source.item().unwrap().0;
+    let label = |s: &Session, layer: &serde_json::Value| s.active_comp().unwrap().layer(LayerId(layer["layer"].as_u64().unwrap())).unwrap().label;
+    for (item, lab) in [(inner, "Blue"), (clip, "Green"), (model, "Orange"), (solid_item, "Fuchsia")] {
+        let before = s.execute("layer.addItem", json!({"item": item})).unwrap();
+        s.execute("project.setLabel", json!({"items": [item], "label": lab})).unwrap();
+        let after = s.execute("layer.addItem", json!({"item": item})).unwrap();
+        assert_eq!(label(&s, &after).name(), lab, "a new layer takes item {item}'s label");
+        assert_ne!(label(&s, &before).name(), lab, "a layer made before keeps its own");
+    }
+    let text = s.execute("layer.newText", json!({"text": "Hi"})).unwrap();
+    assert_eq!(label(&s, &text), Label::Red, "no item: the type's default");
+}
+
 #[test]
 fn property_group_edits() {
     let mut s = Session::default();
@@ -96,3 +124,46 @@ fn property_group_edits() {
     let tr = s.active_comp().unwrap().layers[0].props.sub("transform").unwrap().uid;
     assert!(s.execute("prop.removeGroup", json!({"layer": l, "prop": tr})).is_err());
 }
+
+/// Delete with a text animator, mask or shape group selected removes just those groups, in one
+/// undo step, and keeps their layers (#151). A fixed group selected still deletes the layer.
+#[test]
+fn delete_removes_selected_groups_not_their_layer() {
+    let mut s = Session::default();
+    s.execute("comp.new", json!({"name": "C", "width": 64, "height": 64, "duration": 1})).unwrap();
+    let t = s.execute("layer.newText", json!({"text": "ABCD"})).unwrap()["layer"].as_u64().unwrap();
+    s.execute("layer.addTextAnimator", json!({"layer": t, "properties": ["tracking"]})).unwrap();
+    let sh = s.execute("layer.newShape", json!({"kind": "star"})).unwrap()["layer"].as_u64().unwrap();
+    s.execute("layer.addMask", json!({"layer": sh, "shape": "ellipse"})).unwrap();
+    let groups = |s: &Session, l: u64, path: &str| -> Vec<u64> {
+        s.active_comp().unwrap().layer(LayerId(l)).map_or(vec![], |l| l.props.group(path).unwrap().groups().map(|g| g.uid).collect())
+    };
+    let anim = groups(&s, t, "text/animators")[0];
+    let (mask, shape) = (groups(&s, sh, "masks")[0], groups(&s, sh, "contents")[0]);
+    let delete = |s: &mut Session| {
+        let undo = s.history.undo.len();
+        s.execute("edit.clear", json!({})).unwrap();
+        assert_eq!(s.history.undo.len(), undo + 1, "one undo step");
+    };
+
+    s.execute("prop.select", json!({"layer": t, "prop": anim, "selectKeys": false})).unwrap();
+    let before = s.project.clone();
+    delete(&mut s);
+    assert!(groups(&s, t, "text/animators").is_empty(), "the animator is gone");
+    assert_eq!(s.active_comp().unwrap().layers.len(), 2, "its layer stays");
+    assert!(s.state.selected_props.is_empty());
+    s.execute("edit.undo", json!({})).unwrap();
+    assert_eq!(s.project, before);
+
+    s.execute("prop.select", json!({"layer": sh, "prop": mask, "selectKeys": false})).unwrap();
+    s.execute("prop.select", json!({"layer": sh, "prop": shape, "selectKeys": false, "add": true})).unwrap();
+    delete(&mut s);
+    assert!(groups(&s, sh, "masks").is_empty());
+    assert!(!groups(&s, sh, "contents").contains(&shape));
+    assert_eq!(s.active_comp().unwrap().layers.len(), 2);
+
+    let tr = s.active_comp().unwrap().layer(LayerId(t)).unwrap().props.sub("transform").unwrap().uid;
+    s.execute("prop.select", json!({"layer": t, "prop": tr, "selectKeys": false})).unwrap();
+    delete(&mut s);
+    assert!(s.active_comp().unwrap().layer(LayerId(t)).is_none(), "a fixed group selected: the layer goes");
+}

crates/project/src/build.rs

@@ -715,15 +715,15 @@ pub fn audio(ids: &mut Ids) -> PropGroup {
 
 // ---------------------------------------------------------------- layers
 
-/// Default label colour for a layer source (Labels preferences defaults).
+/// Label colour for a new layer: its Project item's label when it has one (later changes to the
+/// item's label leave existing layers alone), else the Labels preferences default for its type.
 pub fn default_label(src: &LayerSource, project: &Project) -> Label {
+    if let Some(it) = src.item().and_then(|i| project.item(i)) {
+        return it.label;
+    }
     match src {
         LayerSource::Comp { .. } => Label::Sandstone,
-        LayerSource::Footage { item } => match project.item(*item).map(|i| i.type_name()) {
-            Some("Image") | Some("Image Sequence") => Label::Lavender,
-            Some("Audio") => Label::SeaFoam,
-            _ => Label::Aqua,
-        },
+        LayerSource::Footage { .. } => Label::Aqua,
         LayerSource::Solid { .. } => Label::Red,
         LayerSource::Text => Label::Red,
         LayerSource::Shape => Label::Blue,

crates/render/src/text.rs

@@ -533,6 +533,15 @@ pub fn caret_maps(ctx: &EvalCtx, layer: &Layer, chars: usize) -> Vec<Mat3> {
     out
 }
 
+/// A paragraph's alignment: 0 left, 1 centre, 2 right (a justified paragraph by its last line).
+fn align_index(j: Justify) -> u8 {
+    match j {
+        Justify::Center | Justify::JustifyLastCenter => 1,
+        Justify::Right | Justify::JustifyLastRight => 2,
+        _ => 0,
+    }
+}
+
 /// Lay out and animate a text layer.
 pub fn text_geom(ctx: &EvalCtx, layer: &Layer) -> Option<TextGeom> {
     let doc = source_text(ctx, layer)?;
@@ -578,7 +587,9 @@ pub fn text_geom(ctx: &EvalCtx, layer: &Layer) -> Option<TextGeom> {
         })
         .collect();
     let advance = |gi: usize| lay.glyphs[gi].advance + vadv[gi];
-    // Tracking (before / after each character) and Line Anchor, per line.
+    // Tracking (before / after each character) and Line Anchor, per line. Without a Line Anchor
+    // a line grows from where its paragraph's alignment pins it (left, centre or right).
+    let paras = doc.paras();
     let mut shift = vec![0.0f64; n];
     let mut li_start = 0;
     while li_start < n {
@@ -588,7 +599,8 @@ pub fn text_geom(ctx: &EvalCtx, layer: &Layer) -> Option<TextGeom> {
             li_end += 1;
         }
         let mut pen = 0.0;
-        let (mut la, mut la_k) = (0.0, 0.0);
+        let justify = lay.layout.lines.get(line).and_then(|l| paras.get(l.para)).map_or(doc.justify, |p| p.justify);
+        let (mut la, mut la_k) = (50.0 * f64::from(align_index(justify)), 0.0);
         for gi in li_start..li_end {
             let x = &xfs[gi];
             let em = lay.styles.get(lay.glyphs[gi].run).map_or(doc.size, |s| s.size) / 1000.0;
@@ -639,11 +651,7 @@ pub fn text_geom(ctx: &EvalCtx, layer: &Layer) -> Option<TextGeom> {
 
     // Path Options.
     let on_path = text_path(ctx, layer, text);
-    let align = match doc.justify {
-        Justify::Center | Justify::JustifyLastCenter => 1,
-        Justify::Right | Justify::JustifyLastRight => 2,
-        _ => 0,
-    };
+    let align = align_index(doc.justify);
     let first_baseline = lay.line_boxes.first().map(|b| b[1]).unwrap_or(0.0);
     let mut arc = vec![0.0f64; n];
     if let Some((pm, _, force, fm, lm)) = &on_path {

crates/render/src/text/tests.rs

@@ -207,6 +207,39 @@ fn tracking_line_anchor_and_character_offset() {
     assert_eq!(substitute('a', &CharXf { char_value: Some((66.0, 1.0)), ..Default::default() }), Some('B'));
 }
 
+/// Without a Line Anchor, tracking grows each line from where its paragraph's alignment pins it:
+/// left text grows right, centred text around its centre, right text to the left (#146).
+#[test]
+fn tracking_grows_from_each_paragraphs_alignment() {
+    use effectcraft_keyframe::{Justify, ParaStyle};
+    let tracked = |d: TextDoc, amount: f64| {
+        let (mut p, cid, comp) = setup(600, 300);
+        let mut l = text_layer(&mut p, &comp, d);
+        animator(&mut p, &mut l, &["tracking"], &["range"]);
+        set(&mut l, "text/animators/#1/properties/tracking", Value::Scalar(amount));
+        p.comp_mut(cid).unwrap().layers.push(l);
+        glyph_paths(&ctx(&p, cid, 0.0), &p.comp(cid).unwrap().layers[0])
+    };
+    // 4 chars × 500/1000 em × 40 px: 20 px of tracking each, half before and half after.
+    for (justify, x0, centre, x1) in [(Justify::Left, 10.0, 40.0, 70.0), (Justify::Center, -30.0, 0.0, 30.0), (Justify::Right, -70.0, -40.0, -10.0)] {
+        let d = TextDoc { justify, ..doc("ABCD", 40.0) };
+        let (w0, w1) = (bounds_of(&tracked(d.clone(), 0.0)), bounds_of(&tracked(d, 500.0)));
+        let got = [w1.x0 - w0.x0, w1.center().x - w0.center().x, w1.x1 - w0.x1];
+        assert!(got.iter().zip([x0, centre, x1]).all(|(g, w)| (g - w).abs() < 1.0), "{justify:?}: moved {got:?}, want {:?}", [x0, centre, x1]);
+    }
+    // Each paragraph by its own alignment: a left paragraph above a right-aligned one.
+    let mut d = doc("ABCD\nABCD", 40.0);
+    let left = d.base_para();
+    d.set_paras(vec![left.clone(), ParaStyle { justify: Justify::Right, ..left }]);
+    let lines = |paths: Vec<(BezPath, CharXf)>| -> [kurbo::Rect; 2] {
+        let (a, b): (Vec<_>, Vec<_>) = paths.into_iter().partition(|g| bounds_of(std::slice::from_ref(g)).center().y < 10.0);
+        [bounds_of(&a), bounds_of(&b)]
+    };
+    let ([a0, b0], [a1, b1]) = (lines(tracked(d.clone(), 0.0)), lines(tracked(d, 500.0)));
+    assert!((a1.x0 - a0.x0 - 10.0).abs() < 1.0, "the left paragraph grows right: {} {}", a0.x0, a1.x0);
+    assert!((b1.x1 - b0.x1 + 10.0).abs() < 1.0, "the right paragraph grows left: {} {}", b0.x1, b1.x1);
+}
+
 #[test]
 fn anchor_grouping_line_rotates_around_line_centre() {
     let (mut p, cid, comp) = setup(600, 300);

crates/ui-egui/src/panels/project.rs

@@ -424,6 +424,10 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
             // Flat rows like After Effects' Project panel (no stripes).
             if selected { t.row_selected } else { t.row },
         );
+        // The row's click / drag area goes first, so the twirl, proxy, label and comment widgets
+        // on top of it take their own clicks.
+        let mut resp = ui.interact(r.intersect(list), egui::Id::new(("pitem", id.0)), Sense::click_and_drag());
+        app.auto.add(&format!("project.item.{}", id.0), r, &it.name);
         let _ = i;
         let x0 = r.min.x + 8.0 + 14.0 * *depth as f32;
         if it.is_folder() {
@@ -533,6 +537,9 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
                     app.auto.add(&format!("project.item.{}.comment", id.0), cell, &it.comment);
                     if cresp.double_clicked() {
                         ctx.data_mut(|d| d.insert_temp::<Editing>(edit_id(), (id.0, "comment".into(), it.comment.clone())));
+                    } else {
+                        // Other clicks select the row and open its context menu.
+                        resp = resp.union(cresp);
                     }
                     it.comment.clone()
                 }
@@ -545,8 +552,6 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
         if dragging.is_some_and(|d| d != id.0) && drop_row == Some(*id) && it.is_folder() {
             lp.rect_stroke(r.shrink(1.0), 2.0, Stroke::new(1.5, t.accent), egui::StrokeKind::Inside);
         }
-        let resp = ui.interact(r.intersect(list), egui::Id::new(("pitem", id.0)), Sense::click_and_drag());
-        app.auto.add(&format!("project.item.{}", id.0), r, &it.name);
         let nresp = ui.interact(name_clip, egui::Id::new(("pname", id.0)), Sense::click_and_drag());
         app.auto.add(&format!("project.item.{}.name", id.0), name_clip, &it.name);
         let resp = resp.union(nresp);

crates/ui-egui/tests/ui_project_rows.rs

@@ -0,0 +1,97 @@
+//! Project panel rows: a folder's twirl and an item's label swatch take their own clicks, over
+//! the row's click / drag area (#152).
+
+use effectcraft_engine::Session;
+use effectcraft_engine::color::Label;
+use effectcraft_engine::project::{ItemId, ItemKind};
+use effectcraft_ui_egui::EffectcraftApp;
+use egui::{Event, Modifiers, Pos2, pos2};
+use egui_kittest::Harness;
+use egui_kittest::kittest::Queryable;
+use serde_json::json;
+
+/// A closed folder holding a comp, and a solid at the project root: (harness, folder, comp, solid).
+fn harness() -> (Harness<'static, EffectcraftApp>, ItemId, ItemId, ItemId) {
+    let mut s = Session::default();
+    let comp = ItemId(s.execute("comp.new", json!({"name": "Main", "width": 320, "height": 180, "duration": 4})).unwrap()["comp"].as_u64().unwrap());
+    s.execute("layer.newSolid", json!({"name": "Solid", "color": "#406080"})).unwrap();
+    let solid = s.project.items.values().find(|i| matches!(i.kind, ItemKind::Solid(_))).unwrap().id;
+    let folder = ItemId(s.execute("project.newFolder", json!({"name": "Folder"})).unwrap()["item"].as_u64().unwrap());
+    s.execute("project.move", json!({"items": [comp.0], "folder": folder.0})).unwrap();
+    s.execute("project.move", json!({"items": [solid.0], "folder": null})).unwrap();
+    s.execute("project.select", json!({"items": []})).unwrap();
+    let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(s));
+    h.run_steps(3);
+    (h, folder, comp, solid)
+}
+
+fn center(h: &Harness<'_, EffectcraftApp>, id: &str) -> Pos2 {
+    let e = h.state().auto.find(id).unwrap_or_else(|| panic!("no {id}")).clone();
+    pos2(e.rect[0] + e.rect[2] / 2.0, e.rect[1] + e.rect[3] / 2.0)
+}
+
+fn button(h: &mut Harness<'_, EffectcraftApp>, p: Pos2, pressed: bool) {
+    h.input_mut().events.push(Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed, modifiers: Modifiers::NONE });
+}
+
+fn click_at(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) {
+    h.input_mut().events.push(Event::PointerMoved(p));
+    h.step();
+    button(h, p, true);
+    h.step();
+    button(h, p, false);
+    h.run_steps(3);
+}
+
+/// Both clicks in one frame: the harness' frame time is longer than a double-click's.
+fn double_click_at(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) {
+    h.input_mut().events.push(Event::PointerMoved(p));
+    h.step();
+    for _ in 0..2 {
+        button(h, p, true);
+        button(h, p, false);
+    }
+    h.run_steps(3);
+}
+
+fn click(h: &mut Harness<'_, EffectcraftApp>, id: &str) {
+    let p = center(h, id);
+    click_at(h, p);
+}
+
+#[test]
+fn folder_twirl_opens_and_closes_the_folder_on_one_click() {
+    let (mut h, folder, comp, _) = harness();
+    let twirl = format!("project.item.{}.twirl", folder.0);
+    let comp_row = format!("project.item.{}", comp.0);
+    assert!(!h.state().ui.project_open_folders.contains(&folder.0));
+    assert!(h.state().auto.find(&comp_row).is_none(), "the closed folder hides its comp");
+    click(&mut h, &twirl);
+    assert!(h.state().ui.project_open_folders.contains(&folder.0), "one click on the twirl opens the folder");
+    assert!(h.state().auto.find(&comp_row).is_some(), "the open folder shows its comp");
+    assert!(h.state().session.state.project_selection.is_empty(), "the twirl does not select the folder");
+    click(&mut h, &twirl);
+    assert!(!h.state().ui.project_open_folders.contains(&folder.0), "a second click closes it");
+    // The row under the twirl still selects on a click and twirls on a double-click.
+    let name = center(&h, &format!("project.item.{}.name", folder.0));
+    click_at(&mut h, name);
+    assert_eq!(h.state().session.state.project_selection, vec![folder]);
+    double_click_at(&mut h, name);
+    assert!(h.state().ui.project_open_folders.contains(&folder.0), "a double-click on the row opens the folder");
+}
+
+#[test]
+fn label_swatch_opens_the_label_menu() {
+    let (mut h, _, _, solid) = harness();
+    assert_ne!(h.state().session.project.item(solid).unwrap().label, Label::Green);
+    let green = h.state().session.prefs.label_name(Label::Green);
+    assert!(h.query_by_label(&green).is_none());
+    click(&mut h, &format!("project.item.{}.label", solid.0));
+    assert!(h.state().session.state.project_selection.is_empty(), "the swatch does not select the row");
+    let entry = h.query_by_label(&green).expect("the label menu is open").rect();
+    let undo = h.state().session.history.undo.len();
+    click_at(&mut h, entry.center());
+    assert_eq!(h.state().session.project.item(solid).unwrap().label, Label::Green);
+    assert_eq!(h.state().session.history.undo.len(), undo + 1);
+    assert!(h.query_by_label(&green).is_none(), "the menu closed after the choice");
+}