UI: Compact and align effect parameter rows

crates/engine/src/prefs.rs

@@ -145,6 +145,7 @@ page!(Appearance {
     brightness: f64 = 0.0,
     /// The size of the whole interface in percent (75–200), on top of the display's own scale.
     ui_scale: u32 = 100,
+    parameter_row_separators: bool = true,
     use_label_color_for_handles: bool = true,
     use_label_color_for_tabs: bool = true,
     cycle_mask_colors: bool = true,
@@ -1021,6 +1022,7 @@ pub fn pages() -> Vec<Page> {
                 s("appearance.useLabelColorForTabs", "Use Label Color for Related Tabs", B, true),
                 s("appearance.cycleMaskColors", "Cycle Mask Colors", B, true),
                 s("appearance.useGradients", "Use Gradients", B, true),
+                s("appearance.parameterRowSeparators", "Show Parameter Row Separators", B, true),
                 Section("Menu Bar"),
                 s("appearance.inWindowMenuBarMac", "Use In-Window Menu Bar on macOS", B, true),
             ],

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

@@ -30,8 +30,12 @@ use crate::{EffectcraftApp, widgets};
 
 type Actions = Vec<(String, serde_json::Value)>;
 
-const ROW: f32 = 24.0;
+const ROW: f32 = 20.0;
 const DIAL_ROW: f32 = 64.0;
+// The effect title starts at 44 pt. Each level's names sit one tree step farther in;
+// reserve the same icon gutter for properties and groups so sibling names align.
+const PARAM_NAME_X: f32 = 58.0;
+const TREE_INDENT: f32 = 14.0;
 
 fn selected_layer(app: &EffectcraftApp) -> Option<Layer> {
     let comp = app.session.active_comp()?;
@@ -183,7 +187,9 @@ fn prop_row(
         key_navigator(app, ui, p, layer, prop, ectx, r, actions);
     }
     let value = ectx.value(layer, prop);
-    let vx = (r.min.x + r.width() * 0.48).max(swr.max.x + 130.0);
+    // Keep a shared value column while names and their icons follow the group hierarchy.
+    // Extremely deep groups still leave a little name space before their value.
+    let vx = (r.min.x + r.width() * 0.48).max(r.min.x + PARAM_NAME_X + 124.0).max(swr.max.x + 40.0);
     // A vector's values end before the keyframe navigator: in a narrow panel they move left over
     // the name (cut short) instead of running under the navigator (#271).
     let vx = match &value {
@@ -200,8 +206,8 @@ fn prop_row(
         }
         _ => vx,
     };
-    let name_clip =
-        Rect::from_min_max(pos2(swr.max.x + 6.0, r.min.y), pos2(((r.min.x + r.width() * 0.48).max(swr.max.x + 124.0) - 4.0).min(vx - 6.0), r.max.y));
+    let name_clip = Rect::from_min_max(pos2(swr.max.x + 6.0, r.min.y), pos2(vx - 6.0, r.max.y));
+    app.auto.add(&format!("effectControls.prop.{uid}.name"), name_clip, &prop.name);
     p.with_clip_rect(name_clip.intersect(p.clip_rect())).text(pos2(swr.max.x + 6.0, cy), Align2::LEFT_CENTER, &prop.name, Tokens::ui(12.0), t.text);
     let merge = format!("ec-{uid}");
     let set =
@@ -538,9 +544,10 @@ fn group_rows(
                     continue;
                 }
                 *y += ROW;
-                let indent = 24.0 + 14.0 * depth as f32;
+                let indent = PARAM_NAME_X - 13.0 + TREE_INDENT * depth as f32;
                 let visible = !(r.max.y < rect.min.y || r.min.y > rect.max.y);
                 if visible {
+                    app.auto.add(&format!("effectControls.row.{}", pr.uid), r, &pr.name);
                     prop_row(app, ui, p, layer, g, pr, ectx, r, indent, actions);
                 }
                 if app.ui.fx_slider_open.contains(&pr.uid) {
@@ -558,6 +565,8 @@ fn group_rows(
                         }
                     }
                 }
+                // Expanded sliders/dials belong to their parameter; separate after the whole block.
+                widgets::parameter_separator(p, Rect::from_min_max(r.min, pos2(r.max.x, *y)), &t);
             }
             Node::Group(sg) => {
                 // Paint strokes live in the Timeline only (AE's Paint shows Paint on Transparent).
@@ -566,8 +575,11 @@ fn group_rows(
                 }
                 *y += ROW;
                 let open = !app.ui.fx_closed.contains(&sg.uid);
-                let tw = Rect::from_center_size(pos2(r.min.x + 12.0 + 14.0 * depth as f32, r.center().y), vec2(12.0, 12.0));
+                widgets::parameter_separator(p, r, &t);
+                let tw = Rect::from_center_size(pos2(r.min.x + PARAM_NAME_X - 12.0 + TREE_INDENT * depth as f32, r.center().y), vec2(12.0, 12.0));
                 if r.max.y >= rect.min.y && r.min.y <= rect.max.y {
+                    app.auto.add(&format!("effectControls.row.{}", sg.uid), r, &sg.name);
+                    app.auto.add(&format!("effectControls.group.{}.twirl", sg.uid), tw, &sg.name);
                     if widgets::twirl(ui, tw, open, egui::Id::new(("ec-g", sg.uid)), &t).clicked() {
                         if open {
                             app.ui.fx_closed.insert(sg.uid);
@@ -575,7 +587,8 @@ fn group_rows(
                             app.ui.fx_closed.remove(&sg.uid);
                         }
                     }
-                    p.text(pos2(tw.max.x + 6.0, r.center().y), Align2::LEFT_CENTER, &sg.name, Tokens::ui(12.0), t.text);
+                    let name = p.text(pos2(tw.max.x + 6.0, r.center().y), Align2::LEFT_CENTER, &sg.name, Tokens::ui(12.0), t.text);
+                    app.auto.add(&format!("effectControls.group.{}.name", sg.uid), name, &sg.name);
                 }
                 if open {
                     let sub = fx.sub(&sg.match_id);
@@ -963,9 +976,9 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
     let mut tops: Vec<(u64, f32)> = vec![];
     for (fi, g) in fx.iter().enumerate() {
         let GroupKind::Effect { effect } = &g.kind else { continue };
-        let r = Rect::from_min_size(pos2(body.min.x, y), vec2(body.width(), 26.0));
+        let r = Rect::from_min_size(pos2(body.min.x, y), vec2(body.width(), ROW + 2.0));
         tops.push((g.uid, r.min.y));
-        y += 26.0;
+        y += ROW + 2.0;
         let open = !app.ui.fx_closed.contains(&g.uid);
         let selected = app.session.state.selected_props.iter().any(|(l, u)| *l == layer.id && *u == g.uid);
         bp.rect_filled(r, 0.0, if selected { Color32::from_rgb(0x2f, 0x3a, 0x52) } else { Color32::from_rgb(0x2a, 0x2a, 0x2a) });
@@ -983,7 +996,8 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
             }
         }
         app.auto.add(&format!("effectControls.effect.{}.twirl", g.uid), tw, &g.name);
-        bp.text(pos2(tw.max.x + 6.0, r.center().y), Align2::LEFT_CENTER, &g.name, Tokens::semibold(12.0), t.text);
+        let name = bp.text(pos2(tw.max.x + 6.0, r.center().y), Align2::LEFT_CENTER, &g.name, Tokens::semibold(12.0), t.text);
+        app.auto.add(&format!("effectControls.effect.{}.name", g.uid), name, &g.name);
         let reset = Rect::from_min_size(pos2(r.max.x - 96.0, r.min.y + 4.0), vec2(40.0, 18.0));
         let rresp = ui.interact(reset, egui::Id::new(("ec-reset", g.uid)), Sense::click());
         bp.text(reset.center(), Align2::CENTER_CENTER, "Reset", Tokens::ui(11.5), if rresp.hovered() { t.hot_text } else { t.text_dim });
@@ -1070,7 +1084,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
     // Drag an effect header to reorder: insertion line between headers.
     if let Some(du) = dragging {
         if let Some(pos) = ctx.pointer_interact_pos() {
-            let slot = tops.iter().position(|(_, ty)| pos.y < *ty + 13.0).unwrap_or(tops.len());
+            let slot = tops.iter().position(|(_, ty)| pos.y < *ty + (ROW + 2.0) / 2.0).unwrap_or(tops.len());
             let ly = tops.get(slot).map(|(_, ty)| *ty).unwrap_or(y - 4.0);
             bp.line_segment([pos2(body.min.x + 4.0, ly), pos2(body.max.x - 4.0, ly)], Stroke::new(2.0, t.accent));
             if ctx.input(|i| i.pointer.any_released()) {

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

@@ -2196,6 +2196,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
             }
             RowKind::Prop { uid } => {
                 let Some(prop) = layer.props.find(*uid) else { continue };
+                app.auto.add(&format!("timeline.prop.{uid}.row"), r, &prop.name);
                 let sel_prop = app.session.state.selected_props.contains(&(layer.id, *uid));
                 lp.rect_filled(
                     left,
@@ -2451,6 +2452,10 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
                 }
             }
         }
+        if !matches!(row.kind, RowKind::Layer) {
+            widgets::parameter_separator(&lp, left, &t);
+            widgets::parameter_separator(&gp, Rect::from_min_max(pos2(graph_x0, r.min.y), r.max), &t);
+        }
     }
     ui.set_clip_rect(full_clip);
     // The background responses sit below rows, bars and editors, preserving their menus.

crates/ui-egui/src/theme.rs

@@ -75,6 +75,7 @@ pub struct Tokens {
     pub gap: f32,
     pub tab_h: f32,
     pub row_h: f32,
+    pub parameter_separators: bool,
     /// Settings ▸ Appearance ▸ Use Gradients: panel tab strips and buttons get a soft vertical
     /// gradient (off: flat fills).
     pub gradients: bool,
@@ -146,6 +147,7 @@ impl Tokens {
             gap: 4.0,
             tab_h: 30.0,
             row_h: 19.0,
+            parameter_separators: true,
             gradients: true,
         };
         match kind {
@@ -216,6 +218,7 @@ impl Tokens {
     pub fn from_prefs(p: &effectcraft_engine::prefs::Prefs) -> Tokens {
         let kind = ThemeKind::from_name(&p.appearance.theme).unwrap_or_default();
         let mut t = Tokens::for_kind(kind).with_brightness(p.appearance.brightness as f32);
+        t.parameter_separators = p.appearance.parameter_row_separators;
         for (i, l) in effectcraft_color::Label::ALL.iter().enumerate() {
             let [r, g, b] = p.label_rgb(*l);
             t.labels[i] = Color32::from_rgb(r, g, b);

crates/ui-egui/src/widgets.rs

@@ -112,6 +112,14 @@ pub fn select_all(ctx: &egui::Context, id: egui::Id, text: &str) {
     state.store(ctx, id);
 }
 
+/// A parameter boundary. It only paints: row layout owns both drawing and hit testing.
+pub fn parameter_separator(p: &egui::Painter, r: Rect, t: &Tokens) {
+    let y = r.max.y - 0.5;
+    if t.parameter_separators && p.clip_rect().y_range().contains(y) {
+        p.line_segment([pos2(r.min.x, y), pos2(r.max.x, y)], Stroke::new(1.0, t.app_bg));
+    }
+}
+
 /// A colour swatch; returns the response (click opens the picker in the caller).
 pub fn swatch(ui: &mut Ui, rect: Rect, c: [f32; 4], id: egui::Id, t: &Tokens) -> Response {
     let resp = ui.interact(rect, id, Sense::click());

crates/ui-egui/tests/ui_parameter_spacing.rs

@@ -0,0 +1,169 @@
+use effectcraft_engine::{Session, prefs::Prefs};
+use effectcraft_ui_egui::{
+    EffectcraftApp,
+    dock::PanelKind,
+    theme::{ThemeKind, Tokens},
+    widgets,
+};
+use egui::{Event, Modifiers, PointerButton, pos2, vec2};
+use egui_kittest::Harness;
+use serde_json::json;
+
+fn setup() -> (EffectcraftApp, u64, u64, u64) {
+    let mut s = Session::default();
+    s.execute("comp.new", json!({"width":640,"height":360,"duration":4})).unwrap();
+    let lid = s.execute("layer.newSolid", json!({"name":"Plate","color":"#406080"})).unwrap()["layer"].as_u64().unwrap();
+    let checkbox = s.execute("effect.apply", json!({"effect":"Checkbox Control"})).unwrap()["effects"][0].as_u64().unwrap();
+    let point = s.execute("effect.apply", json!({"effect":"Point Control"})).unwrap()["effects"][0].as_u64().unwrap();
+    let fx = s.active_comp().unwrap().layers[0].effects().unwrap();
+    let param = |uid, id| fx.groups().find(|g| g.uid == uid).unwrap().get(id).unwrap().uid;
+    let (checkbox, point) = (param(checkbox, "checkbox"), param(point, "point"));
+    let mut a = EffectcraftApp::new(s);
+    a.show_panel(PanelKind::EffectControls);
+    a.ui.timeline.search = "Checkbox".into();
+    (a, lid, checkbox, point)
+}
+
+fn click(h: &mut Harness<'_, EffectcraftApp>, id: &str) {
+    let r = h.state().auto.find(id).unwrap_or_else(|| panic!("no {id}")).rect;
+    let p = pos2(r[0] + r[2] / 2.0, r[1] + r[3] / 2.0);
+    h.event(Event::PointerMoved(p));
+    h.step();
+    for pressed in [true, false] {
+        h.event(Event::PointerButton { pos: p, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE });
+    }
+    h.run_steps(3);
+}
+
+#[test]
+fn fixed_compact_rows_preserve_checkbox_and_point_hit_targets() {
+    let (a, lid, checkbox, point) = setup();
+    let mut h = Harness::builder().with_size(vec2(1500.0, 1000.0)).build_eframe(|_| a);
+    h.run_steps(4);
+    let fx = h.state().auto.find(&format!("effectControls.row.{checkbox}")).unwrap().rect;
+    let tl = h.state().auto.find(&format!("timeline.prop.{checkbox}.row")).unwrap().rect;
+    assert_eq!(fx[3], 20.0);
+    assert_eq!(tl[3], 19.0);
+    let value = |h: &Harness<'_, EffectcraftApp>| h.state().session.active_comp().unwrap().layers[0].props.find(checkbox).unwrap().value.clone();
+    let before = value(&h);
+    click(&mut h, &format!("effectControls.prop.{checkbox}.value"));
+    assert_ne!(value(&h), before);
+    click(&mut h, &format!("effectControls.prop.{point}.crosshair"));
+    assert_eq!(h.state().ui.fx_pick.as_ref().map(|p| (p.layer, p.prop)), Some((lid, point)));
+    click(&mut h, &format!("effectControls.prop.{point}.crosshair"));
+    assert!(h.state().ui.fx_pick.is_none());
+}
+
+#[test]
+fn separator_preference_is_saved_and_resettable() {
+    let mut p = Prefs::default();
+    p.set("appearance.parameterRowSeparators", json!(false)).unwrap();
+    assert_eq!(Prefs::from_json(&p.to_json()), p);
+    assert!(!Tokens::from_prefs(&p).parameter_separators);
+    p.reset(Some("appearance")).unwrap();
+    assert!(p.appearance.parameter_row_separators);
+    assert!(Prefs::from_json(r#"{"appearance":{"theme":"light"}}"#).appearance.parameter_row_separators);
+}
+
+#[test]
+fn cancelling_separator_preview_preserves_fixed_row_geometry_and_project() {
+    let (a, _, checkbox, _) = setup();
+    let mut h = Harness::builder().with_size(vec2(1500.0, 1000.0)).build_eframe(|_| a);
+    h.run_steps(4);
+    let row = format!("effectControls.row.{checkbox}");
+    let before = h.state().auto.find(&row).unwrap().rect;
+    let project = h.state().session.project.clone();
+    let ctx = h.ctx.clone();
+    effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "app.settings", json!({"page":"appearance"})).unwrap();
+    h.state_mut().session.execute("prefs.set", json!({"key":"appearance.parameterRowSeparators","value":false})).unwrap();
+    h.run_steps(3);
+    assert!(!h.state().tokens.parameter_separators);
+    assert_eq!(h.state().auto.find(&row).unwrap().rect, before);
+    click(&mut h, "settings.cancel");
+    assert!(h.state().tokens.parameter_separators);
+    assert_eq!(h.state().auto.find(&row).unwrap().rect, before);
+    assert!(std::sync::Arc::ptr_eq(&project, &h.state().session.project));
+}
+
+#[test]
+fn row_separator_uses_the_theme_and_can_be_disabled() {
+    for kind in [ThemeKind::Dark, ThemeKind::Darker, ThemeKind::Light] {
+        for enabled in [true, false] {
+            let ctx = egui::Context::default();
+            let mut t = Tokens::for_kind(kind);
+            t.parameter_separators = enabled;
+            let mut out = ctx.run_ui(Default::default(), |_| {
+                let painter = ctx.layer_painter(egui::LayerId::background());
+                widgets::parameter_separator(&painter, egui::Rect::from_min_size(pos2(10.0, 20.0), vec2(200.0, 24.0)), &t);
+            });
+            let lines: Vec<_> = out
+                .shapes
+                .iter()
+                .filter_map(|s| match s.shape {
+                    egui::Shape::LineSegment { points, stroke } => Some((points, stroke)),
+                    _ => None,
+                })
+                .collect();
+            assert_eq!(lines.len(), usize::from(enabled));
+            if enabled {
+                assert_eq!(lines[0].0, [pos2(10.0, 43.5), pos2(210.0, 43.5)]);
+                assert_eq!(lines[0].1.color, t.app_bg);
+            }
+            out.textures_delta.clear();
+        }
+    }
+}
+
+#[test]
+fn nested_groups_keep_indentation_value_alignment_and_effect_gaps_at_fixed_compact_spacing() {
+    let mut s = Session::default();
+    s.execute("comp.new", json!({"width":64,"height":36,"duration":1})).unwrap();
+    s.execute("layer.newSolid", json!({"name":"Plate","color":"#406080"})).unwrap();
+    let fx = s.execute("effect.apply", json!({"effect":"Lumetri Color"})).unwrap()["effects"][0].as_u64().unwrap();
+    let next = s.execute("effect.apply", json!({"effect":"Checkbox Control"})).unwrap()["effects"][0].as_u64().unwrap();
+    let g = s.active_comp().unwrap().layers[0].props.find_group(fx).unwrap();
+    let basic = g.sub("basicCorrection").unwrap();
+    let white = basic.sub("whiteBalance").unwrap();
+    let (root, basic_id, white_id) = (g.get("highDynamicRange").unwrap().uid, basic.uid, white.uid);
+    let (temp, tint, sat) = (white.get("temperature").unwrap().uid, white.get("tint").unwrap().uid, basic.get("saturation").unwrap().uid);
+    let closed: Vec<_> = g.groups().filter(|g| g.uid != basic_id).chain(basic.groups().filter(|g| g.uid != white_id)).map(|g| g.uid).collect();
+    let mut a = EffectcraftApp::new(s);
+    a.show_panel(PanelKind::EffectControls);
+    a.ui.fx_closed.extend(closed);
+    a.ui.timeline.search = "Temperature".into();
+    let mut h = Harness::builder().with_size(vec2(1500.0, 1800.0)).build_eframe(|_| a);
+    h.run_steps(4);
+    let rect = |h: &Harness<'_, EffectcraftApp>, id: String| h.state().auto.find(&id).unwrap_or_else(|| panic!("no {id}")).rect;
+    let name_x = |kind: &str, uid| rect(&h, format!("effectControls.{kind}.{uid}.name"))[0];
+    assert!(name_x("effect", fx) < name_x("group", basic_id));
+    assert_eq!(name_x("prop", root), name_x("group", basic_id), "siblings align even when their icons differ");
+    assert!(name_x("group", basic_id) < name_x("group", white_id));
+    assert!(name_x("group", white_id) < name_x("prop", temp));
+    assert_eq!(name_x("prop", temp), name_x("prop", tint));
+    assert_eq!(rect(&h, format!("effectControls.prop.{temp}.value"))[0], rect(&h, format!("effectControls.prop.{sat}.value"))[0]);
+    let row = rect(&h, format!("effectControls.row.{temp}"));
+    assert_eq!(rect(&h, format!("effectControls.row.{tint}"))[1], row[1] + 20.0);
+    let top = rect(&h, format!("effectControls.effect.{next}"))[1];
+    let last = h
+        .state()
+        .auto
+        .elements
+        .iter()
+        .filter(|e| e.id.starts_with("effectControls.row.") && e.rect[1] < top)
+        .map(|e| e.rect[1] + e.rect[3])
+        .fold(0.0, f32::max);
+    assert_eq!(top - last, 4.0, "the gap between effects is preserved");
+    click(&mut h, &format!("effectControls.prop.{temp}.twirl"));
+    assert_eq!(rect(&h, format!("effectControls.row.{tint}"))[1], row[1] + 20.0 + 34.0);
+    click(&mut h, &format!("effectControls.prop.{temp}.twirl"));
+    click(&mut h, &format!("effectControls.group.{white_id}.twirl"));
+    assert!(h.state().auto.find(&format!("effectControls.row.{temp}")).is_none());
+    assert_eq!(rect(&h, format!("effectControls.effect.{next}"))[1], top - 2.0 * 20.0);
+    click(&mut h, &format!("effectControls.group.{white_id}.twirl"));
+    assert_eq!(rect(&h, format!("effectControls.row.{temp}")), row);
+    // Timeline search reveals the same nesting, with each child farther in than its parent.
+    let tl_x = |kind: &str, uid| rect(&h, format!("timeline.{kind}.{uid}.name"))[0];
+    assert!(tl_x("group", fx) < tl_x("group", basic_id));
+    assert!(tl_x("group", basic_id) < tl_x("group", white_id));
+    assert!(tl_x("group", white_id) < tl_x("prop", temp));
+}

docs/preferences.md

@@ -58,6 +58,7 @@ Commands (CLI, MCP, control channel):
 - `appearance.brightness`: Brightness
 - `appearance.uiScale`: UI Scale, the size of the whole interface (75–200 %, on top of the
   display's own scaling), applied as soon as it is chosen
+- `appearance.parameterRowSeparators`: thin dark boundaries between parameter/group rows in Effect Controls and the timeline (default on). Expanded sliders and dials are kept within their parameter block. Effect Controls indents each group level beneath its effect, aligns sibling names and values, and preserves the gap before the next effect.
 - `appearance.useLabelColorForHandles`: Use Label Color for Layer Handles and Paths
 - `grids.gridColor`: Color
 - `grids.gridSpacing`: Gridline Every
@@ -164,6 +165,8 @@ step with the schema.
 - `scripting.enableJsDebugger`: the JavaScript engine has no step debugger; script errors report
   their file and line in the Script Console.
 
+Effect Controls uses compact, fixed 20 pt parameter rows; the timeline keeps its fixed 19 pt rows. These logical sizes follow the existing whole-interface UI Scale. The separator preference previews immediately; Cancel restores it and OK saves it. Painting and mouse hit tests use the same row geometry. These UI changes do not alter project values or render output.
+
 ## Keyboard shortcuts
 
 Edit ▸ Keyboard Shortcuts (`Cmd+Alt+'`) opens the shortcut editor: an on-screen keyboard showing