From 5586b40cf06303392dd9ecdecb569a56201b3461 Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Tue, 13 Oct 2026 11:43:31 +0000 Subject: [PATCH 01/23] #1190: clear existing STL Clippy warning --- crates/model/src/stl.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/model/src/stl.rs b/crates/model/src/stl.rs index 6684858ad..34e9a7f06 100644 --- a/crates/model/src/stl.rs +++ b/crates/model/src/stl.rs @@ -175,10 +175,8 @@ fn ascii(bytes: &[u8]) -> Result { let Some(key) = it.next() else { continue }; let bad = || perr(format!("STL line {}: expected three numbers", ln + 1)); match key.to_ascii_lowercase().as_str() { - "solid" => { - if name.is_none() { - name = Some(line.get(5..).unwrap_or("").trim().to_string()); - } + "solid" if name.is_none() => { + name = Some(line.get(5..).unwrap_or("").trim().to_string()); } "facet" => { // `facet normal nx ny nz` From ab692dce702abaf1a0cc4d31fc1f0ae3044046b4 Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Tue, 13 Oct 2026 11:43:31 +0000 Subject: [PATCH 02/23] #1190: enable drag toggling for built-in checkboxes --- crates/ui-egui/src/header.rs | 4 +- crates/ui-egui/src/panels/anim_tools.rs | 2 +- crates/ui-egui/src/panels/comp_settings.rs | 6 +- crates/ui-egui/src/panels/dialogs.rs | 8 +- crates/ui-egui/src/panels/dialogs_3d.rs | 6 +- crates/ui-egui/src/panels/effect_controls.rs | 2 +- crates/ui-egui/src/panels/essential.rs | 2 +- crates/ui-egui/src/panels/forms.rs | 16 +- crates/ui-egui/src/panels/key_dialogs.rs | 2 +- .../ui-egui/src/panels/layer_styles_dialog.rs | 4 +- crates/ui-egui/src/panels/markers_ui.rs | 4 +- crates/ui-egui/src/panels/misc.rs | 2 +- crates/ui-egui/src/panels/paint_panels.rs | 6 +- crates/ui-egui/src/panels/panel_kit.rs | 2 +- crates/ui-egui/src/panels/path_vr_panels.rs | 2 +- crates/ui-egui/src/panels/precomp.rs | 4 +- crates/ui-egui/src/panels/project.rs | 2 +- crates/ui-egui/src/panels/properties.rs | 2 +- crates/ui-egui/src/panels/render_queue.rs | 4 +- crates/ui-egui/src/panels/settings.rs | 2 +- crates/ui-egui/src/panels/text_panels.rs | 22 +- crates/ui-egui/src/panels/timeline.rs | 6 +- crates/ui-egui/src/panels/tracker.rs | 14 +- crates/ui-egui/src/widgets.rs | 191 +++++++++++++++++- crates/ui-egui/tests/ui_checkbox_drag.rs | 54 +++++ 25 files changed, 306 insertions(+), 63 deletions(-) create mode 100644 crates/ui-egui/tests/ui_checkbox_drag.rs diff --git a/crates/ui-egui/src/header.rs b/crates/ui-egui/src/header.rs index fb381cfd3..571229007 100644 --- a/crates/ui-egui/src/header.rs +++ b/crates/ui-egui/src/header.rs @@ -174,7 +174,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let snap = Rect::from_min_size(pos2(x, cy - 10.0), vec2(20.0, 20.0)); // The engine owns snapping (View ▸ Snapping); the checkbox mirrors it. app.ui.snapping = app.session.state.snapping; - if widgets::checkbox(ui, snap, app.ui.snapping, &t, egui::Id::new("snapping")).clicked() { + if widgets::checkbox(ui, snap, app.ui.snapping, &t, egui::Id::new("snapping")).changed() { let _ = app.session.execute("view.snapping", serde_json::json!({})); app.ui.snapping = app.session.state.snapping; } @@ -591,7 +591,7 @@ fn puppet_options(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter p.text(pos2(x, cy), Align2::LEFT_CENTER, tr("Mesh:"), Tokens::ui(12.0), t.text_dim); x += 40.0; let show = Rect::from_min_size(pos2(x, cy - 10.0), vec2(20.0, 20.0)); - if widgets::checkbox(ui, show, o.show_mesh, &t, egui::Id::new("puppet-show-mesh")).clicked() { + if widgets::checkbox(ui, show, o.show_mesh, &t, egui::Id::new("puppet-show-mesh")).changed() { let _ = app.session.execute("puppet.mesh", json!({"showMesh": !o.show_mesh})); } app.auto.add("header.puppet.showMesh", show, "Show mesh"); diff --git a/crates/ui-egui/src/panels/anim_tools.rs b/crates/ui-egui/src/panels/anim_tools.rs index 15a4f4f89..8d4066a7f 100644 --- a/crates/ui-egui/src/panels/anim_tools.rs +++ b/crates/ui-egui/src/panels/anim_tools.rs @@ -41,7 +41,7 @@ fn number(app: &mut EffectcraftApp, ui: &mut egui::Ui, at: egui::Pos2, auto: &st fn checkbox(app: &mut EffectcraftApp, ui: &mut egui::Ui, at: egui::Pos2, auto: &str, text: &str, on: bool) -> bool { let t = app.tokens; let r = Rect::from_min_size(at, vec2(16.0, 16.0)); - let clicked = widgets::checkbox(ui, r, on, &t, egui::Id::new(("animtool-cb", auto))).clicked(); + let clicked = widgets::checkbox(ui, r, on, &t, egui::Id::new(("animtool-cb", auto))).changed(); ui.painter().text(pos2(r.max.x + 6.0, r.center().y), Align2::LEFT_CENTER, text, Tokens::ui(12.0), t.text); app.auto.add(auto, r, text); if clicked { !on } else { on } diff --git a/crates/ui-egui/src/panels/comp_settings.rs b/crates/ui-egui/src/panels/comp_settings.rs index 2cd19df5d..14b1e7a9c 100644 --- a/crates/ui-egui/src/panels/comp_settings.rs +++ b/crates/ui-egui/src/panels/comp_settings.rs @@ -321,7 +321,7 @@ pub fn show( let r = ui.add(crate::widgets::drag_value(egui::DragValue::new(&mut d.width).range(4..=30000).suffix(" px"))); auto.add("dialog.comp.width", r.rect, "Width"); let lbl = format!("Lock Aspect Ratio to {}", aspect_label(ow as f64, oh as f64)); - let r = ui.checkbox(&mut d.lock_aspect, lbl); + let r = crate::widgets::labelled_checkbox(ui, &mut d.lock_aspect, lbl); auto.add("dialog.comp.lockAspect", r.rect, "Lock Aspect Ratio"); if d.lock_aspect && ow != d.width && ow > 0 { d.height = ((d.width as f64) * oh as f64 / ow as f64).round().max(4.0) as u32; @@ -416,11 +416,11 @@ pub fn show( }); ui.end_row(); ui.label(""); - let r = ui.checkbox(&mut d.preserve_frame_rate, tr("Preserve frame rate when nested or in render queue")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.preserve_frame_rate, tr("Preserve frame rate when nested or in render queue")); auto.add("dialog.comp.preserveFrameRate", r.rect, "Preserve frame rate when nested or in render queue"); ui.end_row(); ui.label(""); - let r = ui.checkbox(&mut d.preserve_resolution, tr("Preserve resolution when nested")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.preserve_resolution, tr("Preserve resolution when nested")); auto.add("dialog.comp.preserveResolution", r.rect, "Preserve resolution when nested"); ui.end_row(); ui.label(egui::RichText::new(tr("Motion Blur")).strong()); diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index d773fc433..b458701fc 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -480,7 +480,11 @@ fn solid(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { ui.horizontal(|ui| { let r = ui.add(crate::widgets::drag_value(egui::DragValue::new(&mut size[0]).range(1..=30000).suffix(" px"))); auto.add("dialog.solid.width", r.rect, "Width"); - let r = ui.checkbox(&mut lock, tr_args("Lock Aspect Ratio to {}", &[&(super::comp_settings::aspect_label(ow as f64, oh as f64))])); + let r = crate::widgets::labelled_checkbox( + ui, + &mut lock, + tr_args("Lock Aspect Ratio to {}", &[&(super::comp_settings::aspect_label(ow as f64, oh as f64))]), + ); auto.add("dialog.solid.lockAspect", r.rect, "Lock Aspect Ratio"); }); ui.end_row(); @@ -536,7 +540,7 @@ fn solid(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { if editing.is_some() { ui.add_space(8.0); let r = ui - .add_enabled(shared, egui::Checkbox::new(&mut affect_all, tr("Affect all layers that use this solid"))) + .add_enabled(shared, crate::widgets::DragCheckbox::new(&mut affect_all, tr("Affect all layers that use this solid"))) .on_disabled_hover_text(tr("Only this layer uses this solid.")); auto.add("dialog.solid.affectAll", r.rect, "Affect all layers that use this solid"); if shared && !affect_all { diff --git a/crates/ui-egui/src/panels/dialogs_3d.rs b/crates/ui-egui/src/panels/dialogs_3d.rs index a1a67bc60..84cf8d009 100644 --- a/crates/ui-egui/src/panels/dialogs_3d.rs +++ b/crates/ui-egui/src/panels/dialogs_3d.rs @@ -201,12 +201,12 @@ pub fn camera(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { }); ui.end_row(); ui.label(tr("Depth of Field")); - ui.checkbox(&mut d.dof, tr("Enable Depth of Field")); + crate::widgets::labelled_checkbox(ui, &mut d.dof, tr("Enable Depth of Field")); ui.end_row(); ui.label(tr("Focus Distance")); ui.horizontal(|ui| { ui.add_enabled(!d.lock_to_zoom, crate::widgets::drag_value(egui::DragValue::new(&mut d.focus).range(0.0..=100000.0).suffix(" px"))); - ui.checkbox(&mut d.lock_to_zoom, tr("Lock to Zoom")); + crate::widgets::labelled_checkbox(ui, &mut d.lock_to_zoom, tr("Lock to Zoom")); }); ui.end_row(); ui.label(tr("Aperture")); @@ -301,7 +301,7 @@ pub fn light(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { ui.end_row(); } ui.label(tr("Shadows")); - ui.checkbox(&mut d.casts_shadows, tr("Casts Shadows")); + crate::widgets::labelled_checkbox(ui, &mut d.casts_shadows, tr("Casts Shadows")); ui.end_row(); ui.label(tr("Shadow Darkness")); ui.add_enabled(d.casts_shadows, crate::widgets::drag_value(egui::DragValue::new(&mut d.shadow_darkness).range(0.0..=100.0).suffix(" %"))); diff --git a/crates/ui-egui/src/panels/effect_controls.rs b/crates/ui-egui/src/panels/effect_controls.rs index bad1db39f..87bc006c3 100644 --- a/crates/ui-egui/src/panels/effect_controls.rs +++ b/crates/ui-egui/src/panels/effect_controls.rs @@ -404,7 +404,7 @@ fn prop_row( } Value::Bool(b) => { let cr = Rect::from_min_size(pos2(vx, cy - 8.0), vec2(16.0, 16.0)); - if widgets::checkbox(ui, cr, *b, &t, egui::Id::new(("ec-b", uid))).clicked() { + if widgets::checkbox(ui, cr, *b, &t, egui::Id::new(("ec-b", uid))).changed() { set(actions, json!(!b)); } p.text(pos2(cr.max.x + 6.0, cy), Align2::LEFT_CENTER, &prop.name, Tokens::ui(12.0), t.text_dim); diff --git a/crates/ui-egui/src/panels/essential.rs b/crates/ui-egui/src/panels/essential.rs index 2e99c7d39..3dd9be36a 100644 --- a/crates/ui-egui/src/panels/essential.rs +++ b/crates/ui-egui/src/panels/essential.rs @@ -353,7 +353,7 @@ fn value_widget(app: &mut EffectcraftApp, ui: &mut egui::Ui, auto: &str, ty: Opt } ControlType::Checkbox => { let mut on = v.as_bool(); - let r = ui.checkbox(&mut on, ""); + let r = crate::widgets::labelled_checkbox(ui, &mut on, ""); if r.changed() { out = Some(json!(on)); } diff --git a/crates/ui-egui/src/panels/forms.rs b/crates/ui-egui/src/panels/forms.rs index 275e54b75..b37351336 100644 --- a/crates/ui-egui/src/panels/forms.rs +++ b/crates/ui-egui/src/panels/forms.rs @@ -814,7 +814,7 @@ fn form_grid(ui: &mut egui::Ui, f: &mut Form, regs: &mut Vec<(String, egui::Rect .inner .rect } - FieldKind::Bool(b) => ui.checkbox(b, "").rect, + FieldKind::Bool(b) => crate::widgets::labelled_checkbox(ui, b, "").rect, FieldKind::Choice { options, sel } => { let cur = options.get(*sel).map(|o| o.0.clone()).unwrap_or_default(); // The column keeps its width: a long choice (a GPU adapter's name) ends in an @@ -944,13 +944,13 @@ pub fn show_view_options(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Toke let mut close = false; super::dialogs::modal(ctx, "View Options", vec2(360.0, 330.0), t, |ui| { let v = &mut app.ui.viewer; - ui.checkbox(&mut v.show_layer_controls, tr("Layer controls")); - ui.checkbox(&mut v.show_masks, tr("Masks")); - ui.checkbox(&mut v.rulers, tr("Rulers")); - ui.checkbox(&mut v.guides, tr("Guides")); - ui.checkbox(&mut v.grid, tr("Grid")); - ui.checkbox(&mut v.safe_margins, tr("Title/action safe")); - ui.checkbox(&mut v.transparency_grid, tr("Transparency grid")); + crate::widgets::labelled_checkbox(ui, &mut v.show_layer_controls, tr("Layer controls")); + crate::widgets::labelled_checkbox(ui, &mut v.show_masks, tr("Masks")); + crate::widgets::labelled_checkbox(ui, &mut v.rulers, tr("Rulers")); + crate::widgets::labelled_checkbox(ui, &mut v.guides, tr("Guides")); + crate::widgets::labelled_checkbox(ui, &mut v.grid, tr("Grid")); + crate::widgets::labelled_checkbox(ui, &mut v.safe_margins, tr("Title/action safe")); + crate::widgets::labelled_checkbox(ui, &mut v.transparency_grid, tr("Transparency grid")); ui.add_space(12.0); ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { if ui.add(egui::Button::new(egui::RichText::new(tr(" OK ")).color(Color32::WHITE)).fill(t.accent)).clicked() { diff --git a/crates/ui-egui/src/panels/key_dialogs.rs b/crates/ui-egui/src/panels/key_dialogs.rs index 73f76aaaa..56fd5185a 100644 --- a/crates/ui-egui/src/panels/key_dialogs.rs +++ b/crates/ui-egui/src/panels/key_dialogs.rs @@ -160,7 +160,7 @@ pub fn velocity(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { }); ui.add_space(8.0); } - let r = ui.checkbox(&mut d.continuous, tr("Continuous (Lock Outgoing to Incoming)")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.continuous, tr("Continuous (Lock Outgoing to Incoming)")); app.auto.add("dialog.velocity.continuous", r.rect, "Continuous"); if d.continuous { d.out_speed = d.in_speed.clone(); diff --git a/crates/ui-egui/src/panels/layer_styles_dialog.rs b/crates/ui-egui/src/panels/layer_styles_dialog.rs index c0b03f77e..b3b78fb57 100644 --- a/crates/ui-egui/src/panels/layer_styles_dialog.rs +++ b/crates/ui-egui/src/panels/layer_styles_dialog.rs @@ -86,7 +86,7 @@ fn control(ui: &mut egui::Ui, p: &Property, v: &KV) -> Option { match (v, &p.ui) { (KV::Bool(b), _) => { let mut on = *b; - ui.checkbox(&mut on, "").changed().then(|| json!(on)) + crate::widgets::labelled_checkbox(ui, &mut on, "").changed().then(|| json!(on)) } (KV::Enum(i), ParamUi::Popup { options }) => { let mut sel = *i as usize; @@ -170,7 +170,7 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { if id != BLENDING { let g = styles.as_ref().and_then(|s| s.sub(id)); let mut on = g.is_some_and(|g| g.enabled); - if ui.checkbox(&mut on, "").changed() { + if crate::widgets::labelled_checkbox(ui, &mut on, "").changed() { if g.is_none() { actions.push(("layer.style.add".into(), json!({"layers": [lid], "style": id}))); } else { diff --git a/crates/ui-egui/src/panels/markers_ui.rs b/crates/ui-egui/src/panels/markers_ui.rs index 2fff7e1c8..102309765 100644 --- a/crates/ui-egui/src/panels/markers_ui.rs +++ b/crates/ui-egui/src/panels/markers_ui.rs @@ -155,7 +155,7 @@ pub fn dialog(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { ui.end_row(); }); ui.add_space(8.0); - let r = ui.checkbox(&mut d.cue, tr("Cue Point")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.cue, tr("Cue Point")); app.auto.add("dialog.marker.cue", r.rect, "Cue Point"); ui.add_enabled_ui(d.cue, |ui| { ui.horizontal(|ui| { @@ -191,7 +191,7 @@ pub fn dialog(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { }); ui.add_space(8.0); ui.horizontal(|ui| { - let r = ui.checkbox(&mut d.protected, tr("Protected Region")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.protected, tr("Protected Region")); app.auto.add("dialog.marker.protected", r.rect, "Protected Region"); ui.add_space(20.0); ui.label(tr("Label:")); diff --git a/crates/ui-egui/src/panels/misc.rs b/crates/ui-egui/src/panels/misc.rs index cd0ab6897..5a1b77c07 100644 --- a/crates/ui-egui/src/panels/misc.rs +++ b/crates/ui-egui/src/panels/misc.rs @@ -240,7 +240,7 @@ fn fmt_rate(r: f64) -> String { fn check_row(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, at: egui::Pos2, key: &str, text: &str, on: bool) -> bool { let t = app.tokens; let r = Rect::from_min_size(pos2(at.x, at.y - 8.0), vec2(16.0, 16.0)); - let clicked = widgets::checkbox(ui, r, on, &t, egui::Id::new(("pv-check", key))).clicked(); + let clicked = widgets::checkbox(ui, r, on, &t, egui::Id::new(("pv-check", key))).changed(); p.text(pos2(r.max.x + 5.0, at.y), Align2::LEFT_CENTER, text, Tokens::ui(12.0), t.text); app.auto.add(&format!("preview.{key}"), r, text); clicked diff --git a/crates/ui-egui/src/panels/paint_panels.rs b/crates/ui-egui/src/panels/paint_panels.rs index 675bc1e16..3c9332aea 100644 --- a/crates/ui-egui/src/panels/paint_panels.rs +++ b/crates/ui-egui/src/panels/paint_panels.rs @@ -210,7 +210,7 @@ pub fn paint(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { .enumerate() { let r = Rect::from_min_size(pos2(x0 + i as f32 * (w / 2.0), y), vec2(18.0, 20.0)); - if widgets::checkbox(ui, r, on, &t, egui::Id::new(key)).clicked() { + if widgets::checkbox(ui, r, on, &t, egui::Id::new(key)).changed() { set(app, json!({k: !on})); } p.text(pos2(r.max.x + 4.0, r.center().y), Align2::LEFT_CENTER, label, Tokens::ui(11.5), t.text_dim); @@ -237,7 +237,7 @@ pub fn paint(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } y += 26.0; let r = Rect::from_min_size(pos2(x0, y), vec2(18.0, 20.0)); - if widgets::checkbox(ui, r, o.show_overlay, &t, egui::Id::new("paint.showOverlay")).clicked() { + if widgets::checkbox(ui, r, o.show_overlay, &t, egui::Id::new("paint.showOverlay")).changed() { set(app, json!({"showOverlay": !o.show_overlay})); } app.auto.add("paint.showOverlay", r, "Clone Source Overlay"); @@ -246,7 +246,7 @@ pub fn paint(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { set(app, json!({"overlayOpacity": nv})); } let dr = Rect::from_min_size(pos2(x0 + w - 18.0, y), vec2(18.0, 20.0)); - if widgets::checkbox(ui, dr, o.overlay_difference, &t, egui::Id::new("paint.overlayDifference")).clicked() { + if widgets::checkbox(ui, dr, o.overlay_difference, &t, egui::Id::new("paint.overlayDifference")).changed() { set(app, json!({"overlayDifference": !o.overlay_difference})); } app.auto.add("paint.overlayDifference", dr, "Difference mode overlay"); diff --git a/crates/ui-egui/src/panels/panel_kit.rs b/crates/ui-egui/src/panels/panel_kit.rs index cbf28c8ac..06dc8b4db 100644 --- a/crates/ui-egui/src/panels/panel_kit.rs +++ b/crates/ui-egui/src/panels/panel_kit.rs @@ -23,7 +23,7 @@ pub fn dropdown(app: &mut EffectcraftApp, ui: &mut egui::Ui, r: Rect, auto: &str pub fn checkbox(app: &mut EffectcraftApp, ui: &mut egui::Ui, at: egui::Pos2, auto: &str, text: &str, on: bool) -> bool { let t = app.tokens; let r = Rect::from_min_size(at, vec2(16.0, 16.0)); - let clicked = widgets::checkbox(ui, r, on, &t, egui::Id::new(("kit-cb", auto))).clicked(); + let clicked = widgets::checkbox(ui, r, on, &t, egui::Id::new(("kit-cb", auto))).changed(); ui.painter().text(pos2(r.max.x + 6.0, r.center().y), Align2::LEFT_CENTER, text, Tokens::ui(12.0), t.text); let full = Rect::from_min_size(at, vec2(22.0 + text.len() as f32 * 6.5, 16.0)); app.auto.add(auto, full, text); diff --git a/crates/ui-egui/src/panels/path_vr_panels.rs b/crates/ui-egui/src/panels/path_vr_panels.rs index 9dba0dd59..b9d747a3b 100644 --- a/crates/ui-egui/src/panels/path_vr_panels.rs +++ b/crates/ui-egui/src/panels/path_vr_panels.rs @@ -54,7 +54,7 @@ pub fn create_nulls(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { acts.push((id, if id == "paths.tracePath" { json!({"loop": looping}) } else { json!({}) })); } } - let r = ui.checkbox(&mut looping, tr("Loop (Trace Path)")); + let r = crate::widgets::labelled_checkbox(&mut ui, &mut looping, tr("Loop (Trace Path)")); app.auto.add("createNulls.loop", r.rect, "Loop"); ui.data_mut(|d| d.insert_temp(loop_id, looping)); run(app, &ctx, acts); diff --git a/crates/ui-egui/src/panels/precomp.rs b/crates/ui-egui/src/panels/precomp.rs index 8a40c56e9..f6d3068b1 100644 --- a/crates/ui-egui/src/panels/precomp.rs +++ b/crates/ui-egui/src/panels/precomp.rs @@ -141,11 +141,11 @@ pub fn dialog(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { ); ui.add_space(4.0); ui.add_enabled_ui(!d.leave, |ui| { - let r = ui.checkbox(&mut d.adjust, tr("Adjust composition duration to the time span of the selected layers")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.adjust, tr("Adjust composition duration to the time span of the selected layers")); app.auto.add("dialog.precompose.adjust", r.rect, "Adjust composition duration"); }); ui.add_space(8.0); - let r = ui.checkbox(&mut d.open, tr("Open New Composition")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.open, tr("Open New Composition")); app.auto.add("dialog.precompose.open", r.rect, "Open New Composition"); ui.add_space(14.0); ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { diff --git a/crates/ui-egui/src/panels/project.rs b/crates/ui-egui/src/panels/project.rs index 46187fa8c..62938e635 100644 --- a/crates/ui-egui/src/panels/project.rs +++ b/crates/ui-egui/src/panels/project.rs @@ -1094,7 +1094,7 @@ fn column_menu(app: &mut EffectcraftApp, resp: &egui::Response) { ui.label(egui::RichText::new(crate::i18n::tr("Columns")).weak()); for (key, label, _) in COLUMNS { let mut on = app.ui.project_columns.iter().any(|c| c == key); - if ui.checkbox(&mut on, label).changed() { + if crate::widgets::labelled_checkbox(ui, &mut on, label).changed() { set_column(&mut app.ui.project_columns, key, on); } } diff --git a/crates/ui-egui/src/panels/properties.rs b/crates/ui-egui/src/panels/properties.rs index 2649a38ef..9939b56fa 100644 --- a/crates/ui-egui/src/panels/properties.rs +++ b/crates/ui-egui/src/panels/properties.rs @@ -402,7 +402,7 @@ fn text_section( for (i, (key, label, on, c)) in [("fill", "Fill", doc.apply_fill, doc.fill), ("stroke", "Stroke", doc.apply_stroke, doc.stroke)].into_iter().enumerate() { let ry = y + i as f32 * 24.0; let cr = Rect::from_min_size(pos2(x0, ry + 2.0), vec2(14.0, 14.0)); - if widgets::checkbox(ui, cr, on, &t, egui::Id::new(("props-apply", key))).clicked() { + if widgets::checkbox(ui, cr, on, &t, egui::Id::new(("props-apply", key))).changed() { let k = if key == "fill" { "applyFill" } else { "applyStroke" }; set(actions, json!({k: !on})); } diff --git a/crates/ui-egui/src/panels/render_queue.rs b/crates/ui-egui/src/panels/render_queue.rs index e07efe69e..539fb2af1 100644 --- a/crates/ui-egui/src/panels/render_queue.rs +++ b/crates/ui-egui/src/panels/render_queue.rs @@ -463,7 +463,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { // Notify (sound + notification when the queue finishes). let notify = app.session.project.render_prefs.notify; let nb = Rect::from_min_size(pos2(render_b.min.x - 8.0 - 2.0 * bw - 8.0, render_b.min.y + 4.0), vec2(18.0, 18.0)); - if widgets::checkbox(ui, nb, notify, &t, egui::Id::new("rq-notify")).clicked() { + if widgets::checkbox(ui, nb, notify, &t, egui::Id::new("rq-notify")).changed() { actions.push(("renderQueue.setNotify", json!({"notify": !notify}))); } p.text(pos2(nb.max.x + 5.0, nb.center().y), Align2::LEFT_CENTER, tr("Notify"), Tokens::ui(11.5), t.text_dim); @@ -589,7 +589,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } app.auto.add(&aid("twirl"), tw, "Twirl"); let cb = Rect::from_center_size(pos2(cx_render + 20.0, row.center().y), vec2(18.0, 18.0)); - if widgets::checkbox(ui, cb, it.render, &t, egui::Id::new(("rq-cb", it.id))).clicked() && !rendering { + if ui.add_enabled_ui(!rendering, |ui| widgets::checkbox(ui, cb, it.render, &t, egui::Id::new(("rq-cb", it.id)))).inner.changed() && !rendering { actions.push(("renderQueue.setRender", json!({"item": it.id, "render": !it.render}))); } app.auto.add(&aid("render"), cb, if it.render { "Render: on" } else { "Render: off" }); diff --git a/crates/ui-egui/src/panels/settings.rs b/crates/ui-egui/src/panels/settings.rs index b2d2ca435..77eb7e7c0 100644 --- a/crates/ui-egui/src/panels/settings.rs +++ b/crates/ui-egui/src/panels/settings.rs @@ -106,7 +106,7 @@ fn page_ui(app: &mut EffectcraftApp, ui: &mut egui::Ui, page: &Page, cur: &Value match kind { Kind::Bool => { let mut b = v.as_bool().unwrap_or(false); - let r = ui.checkbox(&mut b, tr(label)); + let r = crate::widgets::labelled_checkbox(ui, &mut b, tr(label)); if r.changed() { acts.push(Act::Set(key.into(), json!(b))); } diff --git a/crates/ui-egui/src/panels/text_panels.rs b/crates/ui-egui/src/panels/text_panels.rs index ae2294f1f..fd5e4e17b 100644 --- a/crates/ui-egui/src/panels/text_panels.rs +++ b/crates/ui-egui/src/panels/text_panels.rs @@ -188,7 +188,7 @@ pub fn character(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { for (i, (key, label, on, c)) in [("fill", "Fill", doc.apply_fill, doc.fill), ("stroke", "Stroke", doc.apply_stroke, doc.stroke)].into_iter().enumerate() { let fx = x0 + i as f32 * (w / 2.0); let cr = Rect::from_min_size(pos2(fx, y + 3.0), vec2(14.0, 14.0)); - if widgets::checkbox(ui, cr, on, &t, egui::Id::new(("char-apply", key))).clicked() && enabled { + if ui.add_enabled_ui(enabled, |ui| widgets::checkbox(ui, cr, on, &t, egui::Id::new(("char-apply", key)))).inner.changed() && enabled { actions.push(json!({if key == "fill" { "applyFill" } else { "applyStroke" }: !on})); } app.auto.add(&format!("character.{key}.enabled"), cr, label); @@ -240,7 +240,7 @@ pub fn character(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { y += 32.0; // Ligatures. let lr = Rect::from_min_size(pos2(x0, y), vec2(14.0, 14.0)); - if widgets::checkbox(ui, lr, doc.ligatures, &t, egui::Id::new("char-ligatures")).clicked() && enabled { + if ui.add_enabled_ui(enabled, |ui| widgets::checkbox(ui, lr, doc.ligatures, &t, egui::Id::new("char-ligatures"))).inner.changed() && enabled { actions.push(json!({"ligatures": !doc.ligatures})); } app.auto.add("character.ligatures", lr, "Ligatures"); @@ -264,7 +264,7 @@ pub fn character(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { .enumerate() { let cr = Rect::from_min_size(pos2(x0 + i as f32 * 120.0, y), vec2(14.0, 14.0)); - if widgets::checkbox(ui, cr, on, &t, egui::Id::new(("char-vert", key))).clicked() && enabled { + if ui.add_enabled_ui(enabled, |ui| widgets::checkbox(ui, cr, on, &t, egui::Id::new(("char-vert", key)))).inner.changed() && enabled { actions.push(json!({key: !on})); } app.auto.add(&format!("character.{key}"), cr, label); @@ -544,7 +544,7 @@ pub fn paragraph(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } y += 30.0; let hr = Rect::from_min_size(pos2(x0, y), vec2(14.0, 14.0)); - if widgets::checkbox(ui, hr, doc.hanging_punctuation, &t, egui::Id::new("para-hanging")).clicked() && enabled { + if ui.add_enabled_ui(enabled, |ui| widgets::checkbox(ui, hr, doc.hanging_punctuation, &t, egui::Id::new("para-hanging"))).inner.changed() && enabled { actions.push(json!({"hangingPunctuation": !doc.hanging_punctuation})); } app.auto.add("paragraph.hangingPunctuation", hr, "Roman Hanging Punctuation"); @@ -629,17 +629,15 @@ pub(super) fn font_picker(app: &mut EffectcraftApp, ui: &mut egui::Ui, id: egui: ui.set_width(rect.width().max(180.0)); // Filter row: a checkbox that keeps only the variable fonts. let filter_h = 22.0; - let row = ui.add_sized([ui.available_width(), filter_h], egui::Button::selectable(false, "")); - let box_rect = Rect::from_min_size(row.rect.min, vec2(18.0, filter_h)); - let label_rect = Rect::from_min_max(pos2(box_rect.max.x + 2.0, row.rect.min.y), row.rect.max); - let boxed = widgets::checkbox(ui, box_rect, only_variable, &t, id.with("variable-check")); - ui.painter().text(label_rect.left_center(), Align2::LEFT_CENTER, tr("Variable fonts only"), Tokens::ui(11.5), t.text); + let row = ui.add_sized([ui.available_width(), filter_h], widgets::DragCheckbox::new(&mut only_variable, tr("Variable fonts only"))); app.auto.add(&format!("{auto}.variable"), row.rect, "Variable fonts only"); - let toggled = row.clicked() || boxed.clicked(); + let toggled = row.changed(); if toggled { - only_variable = !only_variable; ui.data_mut(|d| d.insert_temp(variable_id, only_variable)); - // Typing goes on into the search field. + } + // The native checkbox can take focus on release after changing on press. + // Typing goes on into the search field after either event. + if toggled || row.clicked() { ui.memory_mut(|m| m.request_focus(search_id)); } let query = state.query.trim().to_lowercase(); diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 163e6507d..79bfe7174 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -2146,7 +2146,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { actions.push(("layer.setBlendMode".into(), json!({"layers": [layer.id.0], "mode": opts[i]}))); } let tr = Rect::from_center_size(pos2(cw.mode + 76.0 + 6.0, cy), vec2(14.0, 14.0)); - if widgets::checkbox(ui, tr, layer.preserve_transparency, &t, egui::Id::new(("pt", layer.id.0))).clicked() { + if widgets::checkbox(ui, tr, layer.preserve_transparency, &t, egui::Id::new(("pt", layer.id.0))).changed() { actions.push(("layer.setSwitch".into(), json!({"layers": [layer.id.0], "switch": "preserveTransparency"}))); } // Track matte: layer picker + kind. @@ -2476,7 +2476,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { actions.push(("layer.setMask".into(), json!({"layer": layer.id.0, "mask": uid, "mode": opts[i]}))); } let ir = Rect::from_min_size(pos2(mr.max.x + 8.0, cy - 8.0), vec2(16.0, 16.0)); - if widgets::checkbox(ui, ir, inverted, &t, egui::Id::new(("minv", uid))).clicked() { + if widgets::checkbox(ui, ir, inverted, &t, egui::Id::new(("minv", uid))).changed() { actions.push(("layer.setMask".into(), json!({"layer": layer.id.0, "mask": uid, "inverted": !inverted}))); } lp.text(pos2(ir.max.x + 4.0, cy), Align2::LEFT_CENTER, crate::i18n::tr("Inverted"), Tokens::ui(11.0), t.text_dim); @@ -4098,7 +4098,7 @@ fn value_editor( } Value::Bool(b) => { let r = Rect::from_min_size(pos2(x, at.y - 8.0), vec2(16.0, 16.0)); - if widgets::checkbox(ui, r, *b, &t, egui::Id::new(("cb", uid))).clicked() { + if widgets::checkbox(ui, r, *b, &t, egui::Id::new(("cb", uid))).changed() { set(actions, json!(!b)); } app.auto.add(&format!("timeline.prop.{uid}.value"), r, &prop.name); diff --git a/crates/ui-egui/src/panels/tracker.rs b/crates/ui-egui/src/panels/tracker.rs index bd54625b8..aa537ff8c 100644 --- a/crates/ui-egui/src/panels/tracker.rs +++ b/crates/ui-egui/src/panels/tracker.rs @@ -246,10 +246,10 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { ("scale", "Scale", settings.as_ref().is_some_and(|s| s.scale)), ] { let cr = Rect::from_min_size(pos2(xx, y + 2.0), vec2(16.0, 16.0)); - let resp = widgets::checkbox(ui, cr, on && transformish, &t, egui::Id::new(("tracker-cb", key))); + let resp = ui.add_enabled_ui(transformish, |ui| widgets::checkbox(ui, cr, on && transformish, &t, egui::Id::new(("tracker-cb", key)))).inner; app.auto.add(&format!("tracker.{key}"), cr, text); p.text(pos2(cr.max.x + 4.0, cr.center().y), Align2::LEFT_CENTER, text, Tokens::ui(12.0), if transformish { t.text } else { t.text_faint }); - if resp.clicked() && transformish { + if resp.changed() && transformish { run(app, &ctx, "track.setType", with(json!({ key: !on }))); } xx += 84.0; @@ -506,20 +506,20 @@ pub fn options_dialog(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) ui.add_space(8.0); ui.label(egui::RichText::new(crate::i18n::tr("Process Before Match")).font(Tokens::medium(12.5))); ui.horizontal(|ui| { - let r = ui.checkbox(&mut d.blur_on, crate::i18n::tr("Blur")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.blur_on, crate::i18n::tr("Blur")); app.auto.add("dialog.trackOptions.blurOn", r.rect, "Blur"); let r = ui.add_enabled(d.blur_on, crate::widgets::drag_value(egui::DragValue::new(&mut d.opts.blur).speed(0.1).range(0.5..=50.0).suffix(" pixels"))); app.auto.add("dialog.trackOptions.blur", r.rect, "Blur pixels"); - let r = ui.checkbox(&mut d.opts.enhance, crate::i18n::tr("Enhance")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.opts.enhance, crate::i18n::tr("Enhance")); app.auto.add("dialog.trackOptions.enhance", r.rect, "Enhance"); }); ui.add_space(8.0); - let r = ui.checkbox(&mut d.opts.subpixel, crate::i18n::tr("Subpixel Positioning")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.opts.subpixel, crate::i18n::tr("Subpixel Positioning")); app.auto.add("dialog.trackOptions.subpixel", r.rect, "Subpixel Positioning"); - let r = ui.checkbox(&mut d.opts.adapt_every_frame, crate::i18n::tr("Adapt Feature On Every Frame")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.opts.adapt_every_frame, crate::i18n::tr("Adapt Feature On Every Frame")); app.auto.add("dialog.trackOptions.adaptEveryFrame", r.rect, "Adapt Feature On Every Frame"); - let r = ui.checkbox(&mut d.opts.track_shape, crate::i18n::tr("Follow Feature Rotation and Scale")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.opts.track_shape, crate::i18n::tr("Follow Feature Rotation and Scale")); app.auto.add("dialog.trackOptions.trackShape", r.rect, "Follow Feature Rotation and Scale"); ui.add_space(8.0); ui.horizontal(|ui| { diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index c005224d0..1ab352730 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -382,9 +382,96 @@ pub fn text_button(ui: &mut Ui, rect: Rect, text: &str, primary: bool, t: &Token resp } -/// Checkbox in the AE look (small square). +/// A primary-button gesture shared by custom and labelled checkboxes in this context. +#[derive(Clone, Default)] +struct CheckboxDrag { + press_frame: u64, + target: Option, + visited: std::collections::HashSet, +} + +/// Returns the requested value. Mixed checkboxes (`None`) and disabled controls are +/// skipped. Each checkbox is visited once, so crossing it again cannot toggle it back. +fn drag_toggle(ui: &Ui, response: &Response, value: Option) -> Option { + let (pressed, down, released) = ui.input(|i| (i.pointer.primary_pressed(), i.pointer.primary_down(), i.pointer.primary_released())); + let frame = ui.ctx().cumulative_frame_nr(); + // clicked() reads Context input; do not call it while Context data is locked. + let clicked = response.clicked(); + ui.data_mut(|data| { + let drag = data.get_temp_mut_or_default::(egui::Id::new("checkbox-drag")); + if (!down && !released) || (pressed && drag.press_frame != frame) { + *drag = CheckboxDrag { press_frame: frame, ..Default::default() }; + } + let on = value?; + if !response.enabled() { + return None; + } + if pressed && drag.target.is_none() && response.is_pointer_button_down_on() && response.contains_pointer() { + drag.target = Some(!on); + } + if down + && response.contains_pointer() + && let Some(target) = drag.target + { + if drag.visited.insert(response.id) && on != target { + return Some(target); + } + return None; + } + // The source already changed on press. Suppress its ordinary release click. + if released && drag.target.is_some() { + return None; + } + clicked.then_some(!on) + }) +} + +/// A labelled checkbox with the same drag behavior as the custom panel checkbox. +/// Use through `ui.add`, `ui.put`, or `ui.add_enabled` like `egui::Checkbox`. +pub struct DragCheckbox<'a> { + value: &'a mut bool, + text: egui::WidgetText, +} + +impl<'a> DragCheckbox<'a> { + pub fn new(value: &'a mut bool, text: impl Into) -> Self { + Self { value, text: text.into() } + } +} + +impl egui::Widget for DragCheckbox<'_> { + fn ui(self, ui: &mut Ui) -> Response { + let before = *self.value; + let mut response = ui.add(egui::Checkbox::new(self.value, self.text)); + // egui changes on release; our shared gesture changes on press and entry. + *self.value = before; + response.flags.remove(egui::response::Flags::CHANGED); + if response.clicked() { + ui.ctx().request_repaint(); + } + if let Some(value) = drag_toggle(ui, &response, Some(before)) { + *self.value = value; + response.mark_changed(); + ui.ctx().request_repaint(); + } + response + } +} + +/// A labelled checkbox that allocates its own rectangle. +pub fn labelled_checkbox(ui: &mut Ui, value: &mut bool, text: impl Into) -> Response { + ui.add(DragCheckbox::new(value, text)) +} + +/// Checkbox in the AE look (small square). `changed()` requests a toggle, including +/// keyboard activation and entry during a drag begun on another checkbox. pub fn checkbox(ui: &mut Ui, rect: Rect, on: bool, t: &Tokens, id: egui::Id) -> Response { - let resp = ui.interact(rect, id, Sense::click()); + let mut resp = ui.interact(rect, id, Sense::click()); + let next = drag_toggle(ui, &resp, Some(on)); + if next.is_some() { + resp.mark_changed(); + } + let on = next.unwrap_or(on); let b = Rect::from_center_size(rect.center(), vec2(12.0, 12.0)); ui.painter().rect_filled(b, 2.0, if on { t.accent } else { t.field_bg }); ui.painter().rect_stroke(b, 2.0, Stroke::new(1.0, if on { t.accent } else { t.field_border }), StrokeKind::Inside); @@ -666,6 +753,106 @@ pub(crate) fn constrained(c: &[f64], d: usize, v: f64) -> Vec { mod tests { use super::*; + fn checkbox_frame(ctx: &egui::Context, values: &mut [Option; 5], events: Vec) { + let t = Tokens::for_kind(Default::default()); + let input = egui::RawInput { events, ..Default::default() }; + let mut output = ctx.run_ui(input, |ui| { + for (index, value) in values.iter_mut().enumerate() { + let rect = Rect::from_min_size(pos2(20.0, 20.0 + index as f32 * 30.0), vec2(120.0, 20.0)); + ui.add_enabled_ui(index != 3, |ui| { + if index == 4 { + if let Some(on) = value { + ui.put(rect, DragCheckbox::new(on, crate::i18n::tr("Grid"))); + } + } else if let Some(on) = value { + if checkbox(ui, rect, *on, &t, egui::Id::new(index)).changed() { + *on = !*on; + } + } else { + let response = ui.interact(rect, egui::Id::new(index), Sense::click()); + assert_eq!(drag_toggle(ui, &response, None), None); + } + }); + } + }); + output.textures_delta.clear(); + } + + fn checkbox_pointer(y: f32, pressed: Option) -> Vec { + let pos = pos2(28.0, y); + let mut events = vec![egui::Event::PointerMoved(pos)]; + if let Some(pressed) = pressed { + events.push(egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }); + } + events + } + + #[test] + fn checkbox_drag_sets_one_state_skips_mixed_and_disabled_and_ends_on_release() { + let ctx = egui::Context::default(); + let mut values = [Some(false), Some(true), None, Some(false), Some(false)]; + checkbox_frame(&ctx, &mut values, vec![]); + checkbox_frame(&ctx, &mut values, checkbox_pointer(28.0, Some(true))); + assert_eq!(values[0], Some(true)); + // An already-matching neighbour must not invert, even after revisiting. + for y in [58.0, 88.0, 118.0, 148.0, 58.0, 28.0] { + checkbox_frame(&ctx, &mut values, checkbox_pointer(y, None)); + } + assert_eq!(values, [Some(true), Some(true), None, Some(false), Some(true)]); + checkbox_frame(&ctx, &mut values, checkbox_pointer(28.0, Some(false))); + assert_eq!(values[0], Some(true), "release must not toggle the source again"); + // Begin on the labelled wrapper and drag back into custom checkboxes. + checkbox_frame(&ctx, &mut values, checkbox_pointer(148.0, Some(true))); + assert_eq!(values[4], Some(false)); + for y in [58.0, 28.0] { + checkbox_frame(&ctx, &mut values, checkbox_pointer(y, None)); + } + checkbox_frame(&ctx, &mut values, checkbox_pointer(28.0, Some(false))); + assert_eq!(values, [Some(false), Some(false), None, Some(false), Some(false)]); + checkbox_frame(&ctx, &mut values, checkbox_pointer(58.0, None)); + assert_eq!(values[1], Some(false), "hover after release must not paint"); + } + + #[test] + fn checkbox_click_and_keyboard_toggle_once_and_drag_must_start_on_a_checkbox() { + let ctx = egui::Context::default(); + let mut values = [Some(false), Some(false), None, Some(false), Some(false)]; + checkbox_frame(&ctx, &mut values, vec![]); + // Pressing outside the checkboxes must not start a gesture on entry. + checkbox_frame(&ctx, &mut values, checkbox_pointer(200.0, Some(true))); + checkbox_frame(&ctx, &mut values, checkbox_pointer(28.0, None)); + checkbox_frame(&ctx, &mut values, checkbox_pointer(28.0, Some(false))); + assert_eq!(values[0], Some(false)); + for source_y in [88.0, 118.0] { + checkbox_frame(&ctx, &mut values, checkbox_pointer(source_y, Some(true))); + checkbox_frame(&ctx, &mut values, checkbox_pointer(28.0, None)); + checkbox_frame(&ctx, &mut values, checkbox_pointer(28.0, Some(false))); + assert_eq!(values[0], Some(false), "mixed and disabled sources cannot begin a drag"); + } + for y in [28.0, 148.0] { + checkbox_frame(&ctx, &mut values, checkbox_pointer(y, Some(true))); + checkbox_frame(&ctx, &mut values, checkbox_pointer(y, Some(false))); + } + assert_eq!(values[0], Some(true)); + assert_eq!(values[4], Some(true)); + ctx.memory_mut(|m| m.request_focus(egui::Id::new(0usize))); + checkbox_frame( + &ctx, + &mut values, + vec![egui::Event::Key { key: egui::Key::Space, physical_key: None, pressed: true, repeat: false, modifiers: Default::default() }], + ); + assert_eq!(values[0], Some(false), "keyboard activation still works"); + // Fast clicks may deliver press and release together in one input frame. + // This also exercises clicked() without an active drag (outside the data lock). + for (index, y) in [(0, 28.0), (4, 148.0)] { + let before = values[index]; + let mut events = checkbox_pointer(y, Some(true)); + events.extend(checkbox_pointer(y, Some(false))); + checkbox_frame(&ctx, &mut values, events); + assert_eq!(values[index], before.map(|on| !on)); + } + } + /// #488: an inline editor selects its whole text only as it opens. Should it be without the /// keyboard for a frame later (an input method, another handler), it takes the keyboard back /// with the caret where the user put it instead of selecting everything again, which made diff --git a/crates/ui-egui/tests/ui_checkbox_drag.rs b/crates/ui-egui/tests/ui_checkbox_drag.rs new file mode 100644 index 000000000..ba65e54dc --- /dev/null +++ b/crates/ui-egui/tests/ui_checkbox_drag.rs @@ -0,0 +1,54 @@ +//! #1190: the shared drag gesture also reaches real dialog controls. +use effectcraft_engine::Session; +use effectcraft_ui_egui::{EffectcraftApp, menus}; +use egui::{Event, Modifiers, PointerButton, Pos2, vec2}; +use egui_kittest::{Harness, kittest::Queryable as _}; +use serde_json::json; + +fn pointer(h: &mut Harness<'_, EffectcraftApp>, at: Pos2, pressed: Option) { + h.event(Event::PointerMoved(at)); + h.step(); + if let Some(pressed) = pressed { + h.event(Event::PointerButton { pos: at, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }); + h.step(); + } + h.run_steps(2); +} + +#[test] +fn view_options_drag_checks_and_unchecks_neighbours() { + let mut session = Session::default(); + session.prefs.disk.disk_cache_enabled = false; + session.execute("comp.new", json!({"width":32,"height":32,"duration":1})).unwrap(); + let mut h = Harness::builder().with_size(vec2(1200.0, 900.0)).build_eframe(|_| EffectcraftApp::new(session)); + h.state_mut().ui.viewer.rulers = false; + h.state_mut().ui.viewer.guides = true; + h.state_mut().ui.viewer.grid = false; + h.run_steps(3); + let ctx = h.ctx.clone(); + menus::invoke(h.state_mut(), &ctx, "view.options", json!({})).unwrap(); + h.run_steps(3); + let rulers = h.get_by_label("Rulers").rect().center(); + let guides = h.get_by_label("Guides").rect().center(); + let grid = h.get_by_label("Grid").rect().center(); + pointer(&mut h, rulers, Some(true)); + assert!(h.state().ui.viewer.rulers); + pointer(&mut h, guides, None); + assert!(h.state().ui.viewer.guides, "an already-checked neighbour stays checked"); + pointer(&mut h, grid, None); + assert!(h.state().ui.viewer.grid); + pointer(&mut h, rulers, None); + pointer(&mut h, rulers, Some(false)); + assert!(h.state().ui.viewer.rulers, "revisiting and releasing does not undo the change"); + if let Ok(path) = std::env::var("CHECKBOX_REVIEW_PNG") { + h.render().unwrap().save(path).unwrap(); + } + pointer(&mut h, grid, Some(true)); + pointer(&mut h, guides, None); + pointer(&mut h, rulers, None); + pointer(&mut h, rulers, Some(false)); + let viewer = &h.state().ui.viewer; + assert!(!viewer.rulers && !viewer.guides && !viewer.grid); + pointer(&mut h, guides, None); + assert!(!h.state().ui.viewer.guides, "hover after release does not change a checkbox"); +} From 32296d6f653f3efbaf99df06f17f01495e61be07 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 19:28:22 +0000 Subject: [PATCH 03/23] #1284: Cmd+W closes the Render Queue while it has focus With the Render Queue active, Cmd+W ran comp.close on the active composition, so the composition next to the queue closed and the queue stayed open. Close now follows panel focus, as Clear does: the queue closes and the composition stays open. Covered by menus::tests::cmd_w_in_the_render_queue_closes_the_queue_not_the_comp. Claude-Session: https://claude.ai/code/session_0136kZtXrYo5VUWEFxWduwUD --- crates/ui-egui/src/menus.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 5c61b4977..eb89e6ab0 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -291,6 +291,12 @@ pub fn invoke(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: V _ => {} } } + // Close follows panel focus too: with the Render Queue active, Cmd+W closes that panel and + // leaves the active composition open (it used to close the composition next to it). + if id == "file.close" && params.as_object().is_some_and(|p| p.is_empty()) && app.ui.focused == PanelKind::RenderQueue { + app.close_panel(PanelKind::RenderQueue); + return Ok(Value::Null); + } // Clear follows panel focus, including the Edit menu and keyboard shortcut. if id == "edit.clear" && params.as_object().is_some_and(|p| p.is_empty()) && app.ui.focused == PanelKind::Project { // Through `invoke`, so deleting items that compositions use still asks first (M3.16). @@ -1986,6 +1992,21 @@ mod tests { } } + /// Cmd+W with the Render Queue focused closes the queue; the composition next to it stays + /// open (it used to be closed, and the queue stayed). + #[test] + fn cmd_w_in_the_render_queue_closes_the_queue_not_the_comp() { + let ctx = egui::Context::default(); + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + let a = app.session.execute("comp.new", json!({"name": "A", "duration": 4})).unwrap()["comp"].as_u64(); + app.show_panel(PanelKind::RenderQueue); + assert_eq!(app.ui.focused, PanelKind::RenderQueue); + invoke(&mut app, &ctx, "file.close", json!({})).unwrap(); + assert!(!app.ui.dock.contains(PanelKind::RenderQueue)); + assert_eq!(app.session.state.active_comp.map(|c| c.0), a); + assert_eq!(app.session.state.open_comps.len(), 1); + } + /// #336: Ctrl+\ (Cmd+\ on macOS) maximizes the application window, and again restores /// it, as in After Effects. #[test] From a7cad22c13a0f7e05708c2279ce88185e3afd7de Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 19:28:22 +0000 Subject: [PATCH 04/23] #1284: right-clicking a Graph Editor keyframe opens its key menu The Graph Editor had no menu for keyframes. Right-click now opens the same key menu as the Timeline, for the commands that apply from the graph: Edit Value, Copy and Paste, selecting equal or neighbouring keys, Hold, Rove Across Time (spatial properties only), Easy Ease, interpolation and Delete. The items run the existing keys.* commands, so undo and the automation ids are unchanged. A right-click on an unselected key selects it first, as a click does. Not covered by an automated test; checked by compiling and by reading the menu against the Timeline's. Claude-Session: https://claude.ai/code/session_0136kZtXrYo5VUWEFxWduwUD --- crates/ui-egui/src/panels/graph.rs | 44 +++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/panels/graph.rs b/crates/ui-egui/src/panels/graph.rs index 2dbb98268..41e2c5263 100644 --- a/crates/ui-egui/src/panels/graph.rs +++ b/crates/ui-egui/src/panels/graph.rs @@ -453,7 +453,7 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte } } else if kresp.clicked() && mods.shift { actions.push(("keys.select".into(), json!({"keys": [kjson.clone()], "toggle": true}))); - } else if (kresp.clicked() || kresp.drag_started()) && !k.sel { + } else if (kresp.clicked() || kresp.drag_started() || kresp.secondary_clicked()) && !k.sel { actions.push(("keys.select".into(), json!({"keys": [kjson.clone()], "add": mods.shift}))); } // A drag moves every selected key, by how far the pointer went from where the button @@ -485,6 +485,48 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte ctx.data_mut(|dd| dd.insert_temp(drag_id, false)); actions.push(("__endMerge".into(), json!({}))); } + // Right-click: the Timeline's key menu, for the commands that apply from the Graph Editor. + kresp.context_menu(|ui| { + let editable_value = matches!(key.value, Value::Scalar(_) | Value::Vec2(_) | Value::Vec3(_)); + for (lbl, cmd, params) in [ + ("Edit Value…", "keys.set", kjson.clone()), + ("-", "", json!({})), + ("Copy", "keys.copy", json!({})), + ("Paste", "keys.paste", json!({})), + ("-", "", json!({})), + ("Select Equal Keyframes", "keys.selectEqual", json!({})), + ("Select Previous Keyframes", "keys.selectPrevious", json!({})), + ("Select Following Keyframes", "keys.selectFollowing", json!({})), + ("-", "", json!({})), + ("Toggle Hold Keyframe", "keys.toggleHold", json!({})), + ("Rove Across Time", "keys.interpolation", json!({"roving": !key.roving})), + ("-", "", json!({})), + ("Easy Ease", "keys.easyEase", json!({})), + ("Easy Ease In", "keys.easyEaseIn", json!({})), + ("Easy Ease Out", "keys.easyEaseOut", json!({})), + ("-", "", json!({})), + ("Linear", "keys.interpolation", json!({"interpolation": "linear"})), + ("Bezier", "keys.interpolation", json!({"interpolation": "bezier"})), + ("Auto Bezier", "keys.interpolation", json!({"interpolation": "autoBezier"})), + ("-", "", json!({})), + ("Delete", "keys.delete", json!({})), + ] { + if lbl == "-" { + ui.separator(); + continue; + } + if lbl == "Rove Across Time" && !c.prop.spatial { + continue; + } + if lbl == "Edit Value…" && !editable_value { + continue; + } + if ui.button(lbl).clicked() { + actions.push((cmd.into(), params)); + ui.close(); + } + } + }); } } From 84589dc28804dd112c7474a1af6282ff99b2d3ba Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 19:28:22 +0000 Subject: [PATCH 05/23] #1284: double-clicking a shape tool adds a comp-sized shape layer Double-click a shape tool in the toolbar to add a shape layer the size of the composition, at its centre, for backgrounds, mattes and overlays, as After Effects does. It uses layer.newShape with the tool's kind, so the undo label and the command are the same as drawing one. Covered by tests::a_comp_sized_shape_layer_sits_on_the_comp_centre; the double-click itself is not covered by an automated test. Claude-Session: https://claude.ai/code/session_0136kZtXrYo5VUWEFxWduwUD --- crates/engine/src/tests.rs | 12 ++++++++++++ crates/ui-egui/src/header.rs | 9 +++++++++ crates/ui-egui/src/panels/viewer.rs | 2 +- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 587379a85..680cbff22 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -246,6 +246,18 @@ fn text_shape_mask_matte_parent() { assert_eq!(info["layers"].as_array().unwrap().len(), 2); } +/// A shape layer the size of the comp and placed at its default position covers the frame: the +/// Shape Tool's double-click (#1284) makes one this way, for backgrounds, mattes and overlays. +#[test] +fn a_comp_sized_shape_layer_sits_on_the_comp_centre() { + let mut s = Session::default(); + s.execute("comp.new", json!({"width": 400, "height": 300, "duration": 2})).unwrap(); + let sh = s.execute("layer.newShape", json!({"kind": "rect", "size": [400, 300], "fill": "#ff0000"})).unwrap()["layer"].as_u64().unwrap(); + let l = s.active_comp().unwrap().layer(effectcraft_project::LayerId(sh)).unwrap().clone(); + let pos = l.props.prop("transform/position").unwrap().value.clone(); + assert!(matches!(pos, effectcraft_keyframe::Value::Vec3(p) if (p[0] - 200.0).abs() < 1e-9 && (p[1] - 150.0).abs() < 1e-9), "{pos:?}"); +} + /// The toolbar's Fill / Stroke change a selected shape layer's paint (#205): a shape drawn /// without a stroke gets one, later edits change it, and the Contents "Add:" items (#206) include /// an empty Path and a Gradient Stroke that render. diff --git a/crates/ui-egui/src/header.rs b/crates/ui-egui/src/header.rs index 1260c9c9e..0c6a9d169 100644 --- a/crates/ui-egui/src/header.rs +++ b/crates/ui-egui/src/header.rs @@ -82,6 +82,15 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { { app.ui.status = e; } + // Double-click a shape tool: a shape layer filling the composition, centred, for + // backgrounds, mattes and overlays (as After Effects does). + if cur.is_shape() + && resp.double_clicked() + && let Some((w, h)) = app.session.active_comp().map(|c| (c.width, c.height)) + && let Err(e) = crate::menus::invoke(app, ui.ctx(), "layer.newShape", json!({"kind": crate::panels::viewer::shape_kind(cur), "size": [w, h]})) + { + app.ui.status = e; + } if slot.len() > 1 { let held_for = resp.is_pointer_button_down_on().then(|| ui.input(|i| i.pointer.press_start_time().map(|s| i.time - s))).flatten(); if let Some(seconds) = held_for diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index 5fe7697c9..8410bb78c 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -2138,7 +2138,7 @@ struct ShapeDrag { } /// The kind `tool` draws, as `shape.newShape` / `layer.addMask` name it. -fn shape_kind(tool: Tool) -> &'static str { +pub(crate) fn shape_kind(tool: Tool) -> &'static str { match tool { Tool::Rectangle => "rect", Tool::RoundedRect => "rounded", From f9c3547e57001ee9d876c1620e598a992a6d6b74 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 19:28:22 +0000 Subject: [PATCH 06/23] #1284: keep Rove Across Time covered through edits to other keys The report says a roving key goes back to its original time after other keys are edited. Editing the last key's value and then its time keeps the roving key at its share of the path, as the Rove Across Time description says, so this adds a regression test for both edits. It passes on this branch; the reported sequence is not reproduced yet, so the report still needs the exact steps. Claude-Session: https://claude.ai/code/session_0136kZtXrYo5VUWEFxWduwUD --- crates/engine/src/tests_timeline.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/crates/engine/src/tests_timeline.rs b/crates/engine/src/tests_timeline.rs index 6a74a23b3..94566240a 100644 --- a/crates/engine/src/tests_timeline.rs +++ b/crates/engine/src/tests_timeline.rs @@ -219,6 +219,25 @@ fn interpolation_dialog_temporal_spatial_and_roving() { assert_ne!(prop(&s, l, "transform/position").keys[0].out_interp, Interp::Hold); } +/// Editing another key's value or time keeps the roving key at its share of the path (#1284: the +/// roving key went back to where it was after edits to other keys). +#[test] +fn roving_key_follows_value_and_time_edits_to_other_keys() { + let (mut s, l) = setup(); + animate(&mut s, l, "transform/position", &[(0.0, json!([0, 0])), (0.3, json!([300, 0])), (2.0, json!([400, 0]))]); + let uid = prop(&s, l, "transform/position").uid; + s.execute("keys.select", json!({"keys": [{"layer": l, "prop": uid, "time": 0.3}]})).unwrap(); + s.execute("keys.interpolation", json!({"roving": true})).unwrap(); + let roving_at = |s: &Session| prop(s, l, "transform/position").keys[1].time.seconds(); + assert!((roving_at(&s) - 1.5).abs() < 0.01, "start: {}", roving_at(&s)); + // The last key's value moves to 800 px: the path is 300 + 500 px, so the roving key is at 300 / 800. + s.execute("keys.set", json!({"layer": l, "path": "transform/position", "time": 2.0, "value": [800, 0]})).unwrap(); + assert!((roving_at(&s) - 0.75).abs() < 0.02, "value edit: {}", roving_at(&s)); + // The last key moves to 3 s: the same 300 / 800 share of 3 s. + s.execute("keys.set", json!({"layer": l, "path": "transform/position", "time": 2.0, "newTime": 3.0})).unwrap(); + assert!((roving_at(&s) - 1.125).abs() < 0.02, "time edit: {}", roving_at(&s)); +} + #[test] fn graph_editor_key_and_handle_edits() { let (mut s, l) = setup(); From f9d514bb217ba89b1c9d65336347821f6f3bffa3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 12:29:10 +0000 Subject: [PATCH 07/23] #1284: fix the shape double-click's size and paint, focus after closing the queue, the Graph Editor key menu - The comp-sized shape is made by shape.newShape (new newLayer option) so it takes the Tools bar's Fill and Stroke, and a polygon or star fits the shorter comp side. - Cmd+W on the Render Queue moves the focus to a panel that is still open. - The Graph Editor's key menu is the Timeline's (one shared function): all its commands, labels in the interface language, edits disabled on a locked layer. - keys.delete and keys.set leave a locked layer's keys alone. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01RDvxCS8xcR28VArjzn1x7U --- crates/engine/src/cmd/anim.rs | 3 + crates/engine/src/cmd/prop.rs | 6 +- crates/engine/src/cmd/shape_tool.rs | 6 +- crates/engine/src/tests/delete_keys.rs | 21 ++++ crates/ui-egui/src/header.rs | 61 +++++++++- crates/ui-egui/src/menus/invoke.rs | 4 + crates/ui-egui/src/menus/tests.rs | 4 + crates/ui-egui/src/panels/graph.rs | 44 +------ crates/ui-egui/src/panels/timeline.rs | 121 +++++++++++--------- crates/ui-egui/src/panels/timeline/tests.rs | 43 +++++++ 10 files changed, 214 insertions(+), 99 deletions(-) diff --git a/crates/engine/src/cmd/anim.rs b/crates/engine/src/cmd/anim.rs index 325845115..76bfb5a63 100644 --- a/crates/engine/src/cmd/anim.rs +++ b/crates/engine/src/cmd/anim.rs @@ -296,6 +296,9 @@ fn set_key(s: &mut Session, p: &Value) -> Result { let base = KeyTimeBase::from_params(p, "keys.set")?; let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; let layer = comp.layer(lid).ok_or(EngineError::NoComp)?; + if layer.switches.locked { + return Err(bad("keys.set", "the layer is locked")); + } let t = key_time(layer, comp.frame_rate, p, "time", base, "keys.set", false)?.ok_or_else(|| bad("keys.set", "missing `time` (layer time by default)"))?; let new_t = key_time(layer, comp.frame_rate, p, "newTime", base, "keys.set", true)?; let value = p.get("value").cloned(); diff --git a/crates/engine/src/cmd/prop.rs b/crates/engine/src/cmd/prop.rs index 00232967e..36f76706f 100644 --- a/crates/engine/src/cmd/prop.rs +++ b/crates/engine/src/cmd/prop.rs @@ -1046,6 +1046,10 @@ fn delete_keys(s: &mut Session, _: &Value) -> Result { let comp = proj.comp_mut(cid).ok_or(EngineError::NoComp)?; let mut removed = 0usize; for k in &sel { + // A layer locked after its keys were selected keeps them (the selection stays too). + if comp.layer(k.layer).is_some_and(|l| l.switches.locked) { + continue; + } if let Some(pr) = comp.layer_mut(k.layer).and_then(|l| l.props.find_mut(k.prop)) && let Some(i) = key_at(&pr.keys, k.time) { @@ -1056,7 +1060,7 @@ fn delete_keys(s: &mut Session, _: &Value) -> Result { removed += 1; } } - st.selected_keys.clear(); + st.selected_keys.retain(|k| comp.layer(k.layer).is_some_and(|l| l.switches.locked)); // The keys actually removed: a selected key that is gone (or listed twice) isn't counted, // so a caller can tell that a delete changed nothing. Ok(json!(removed)) diff --git a/crates/engine/src/cmd/shape_tool.rs b/crates/engine/src/cmd/shape_tool.rs index e1f3607b1..3db347e60 100644 --- a/crates/engine/src/cmd/shape_tool.rs +++ b/crates/engine/src/cmd/shape_tool.rs @@ -318,7 +318,9 @@ fn new_shape(s: &mut Session, p: &Value) -> Result { }; let pos = position_p(p, c)?.unwrap_or([comp.width as f64 / 2.0, comp.height as f64 / 2.0]); let paint = paint_p(p, &s.state.shape_tool, c)?; - let target = draw_target(s, &comp, p, c)?; + // `newLayer` makes a layer of its own even with a shape layer selected (a shape that fills the + // comp for a background or matte, whatever is selected). + let target = if b_p(p, "newLayer") == Some(true) { None } else { draw_target(s, &comp, p, c)? }; // In the target layer's space: the centre maps through the layer's transform, the size by // its scale. let (centre, size) = match target.and_then(|l| comp.layer(l)) { @@ -374,7 +376,7 @@ pub fn specs() -> Vec { "Shape Tool", [], None, - "{layer?, kind?: rect|rounded|ellipse|polygon|star, size? [w,h], position? [x,y] (the centre), space?: comp|layer, name? (a new layer's), fill?, fillType?, fillBlend?, fillOpacity?, stroke?, strokeType?, strokeBlend?, strokeOpacity?, strokeWidth? (default the Tools bar's, shape.toolOptions)} → {layer, group}", + "{layer?, newLayer? (a new shape layer even with a shape layer selected), kind?: rect|rounded|ellipse|polygon|star, size? [w,h], position? [x,y] (the centre), space?: comp|layer, name? (a new layer's), fill?, fillType?, fillBlend?, fillOpacity?, stroke?, strokeType?, strokeBlend?, strokeOpacity?, strokeWidth? (default the Tools bar's, shape.toolOptions)} → {layer, group}", has_comp, new_shape ), diff --git a/crates/engine/src/tests/delete_keys.rs b/crates/engine/src/tests/delete_keys.rs index b1b9af316..029104497 100644 --- a/crates/engine/src/tests/delete_keys.rs +++ b/crates/engine/src/tests/delete_keys.rs @@ -43,3 +43,24 @@ fn delete_keys_reports_zero_when_no_selected_key_exists() { assert_eq!(removed, json!(0)); assert_eq!(keys_of(&s, layer, prop), keys); } + +/// A layer locked after its keys were selected keeps them: Delete and Edit Value leave them alone. +#[test] +fn a_locked_layers_keys_cannot_be_deleted_or_edited() { + let (mut s, layer, prop) = setup(); + let keys = keys_of(&s, layer, prop); + s.state.selected_keys = keys.clone(); + s.execute_checked("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": true})).unwrap(); + let removed = s.execute_checked("keys.delete", json!({})).unwrap(); + assert_eq!(removed, json!(0)); + assert_eq!(keys_of(&s, layer, prop), keys); + assert_eq!(s.state.selected_keys, keys, "the selection stays"); + let e = s.execute("keys.set", json!({"layer": layer, "prop": prop, "time": 0, "value": 90})).unwrap_err().to_string(); + assert!(e.contains("locked"), "{e}"); + let value = s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.find(prop).unwrap().keys[0].value.as_f64(); + assert_eq!(value, 0.0); + // Unlocked again, both work. + s.execute_checked("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": false})).unwrap(); + s.execute_checked("keys.set", json!({"layer": layer, "prop": prop, "time": 0, "value": 90})).unwrap(); + assert_eq!(s.execute_checked("keys.delete", json!({})).unwrap(), json!(3)); +} diff --git a/crates/ui-egui/src/header.rs b/crates/ui-egui/src/header.rs index 45caa86de..8a336fcbb 100644 --- a/crates/ui-egui/src/header.rs +++ b/crates/ui-egui/src/header.rs @@ -57,6 +57,22 @@ fn brand_and_home(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter x + 6.0 } +/// The `shape.newShape` parameters for a shape that fills a `w` × `h` composition with `tool`: a new +/// layer, painted with the Tools bar's Fill and Stroke. A polygon or star is round, so it takes the +/// shorter side; the other kinds take the whole frame. +pub(crate) fn comp_sized_shape_params(tool: Tool, w: u32, h: u32) -> serde_json::Value { + let kind = crate::panels::viewer::shape_kind(tool); + let (w, h) = (f64::from(w), f64::from(h)); + let size = if matches!(kind, "polygon" | "star") { [w.min(h); 2] } else { [w, h] }; + json!({"kind": kind, "size": size, "position": [w / 2.0, h / 2.0], "newLayer": true}) +} + +/// Make the shape layer that fills the composition for a double-clicked shape `tool`. +fn comp_sized_shape(app: &mut EffectcraftApp, ctx: &egui::Context, tool: Tool) -> Result { + let (w, h) = app.session.active_comp().map(|c| (c.width, c.height)).ok_or("no composition is open")?; + crate::menus::invoke(app, ctx, "shape.newShape", comp_sized_shape_params(tool, w, h)) +} + /// The tool slots (with group separators after camera tools and pan-behind); returns the x after them. fn tool_slots(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, mut x: f32, cy: f32) -> f32 { let t = app.tokens; @@ -114,8 +130,7 @@ fn tool_slot(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, si: // backgrounds, mattes and overlays (as After Effects does). if cur.is_shape() && resp.double_clicked() - && let Some((w, h)) = app.session.active_comp().map(|c| (c.width, c.height)) - && let Err(e) = crate::menus::invoke(app, ui.ctx(), "layer.newShape", json!({"kind": crate::panels::viewer::shape_kind(cur), "size": [w, h]})) + && let Err(e) = comp_sized_shape(app, ui.ctx(), cur) { app.ui.status = e; } @@ -843,4 +858,46 @@ mod tests { frame(&mut app, &ctx, 2.1, vec![]); assert!(!app.auto.elements.iter().any(|w| w.id == "tools.select.TypeVertical")); } + + /// A polygon or star fills the shorter side; the other kinds fill the frame; all make a new layer. + #[test] + fn a_comp_sized_shape_fits_the_frame_for_every_kind() { + for (tool, size) in [ + (Tool::Rectangle, [1920.0, 1080.0]), + (Tool::RoundedRect, [1920.0, 1080.0]), + (Tool::Ellipse, [1920.0, 1080.0]), + (Tool::Polygon, [1080.0, 1080.0]), + (Tool::Star, [1080.0, 1080.0]), + ] { + let p = comp_sized_shape_params(tool, 1920, 1080); + assert_eq!(p["size"], json!(size), "{tool:?}"); + assert_eq!(p["position"], json!([960.0, 540.0])); + assert_eq!(p["newLayer"], json!(true)); + } + } + + /// The double-click shape takes the Tools bar's Fill, and is a new layer even with a shape layer + /// selected (not a group in that layer). + #[test] + fn the_double_clicked_shape_uses_the_toolbar_paint_on_a_new_layer() { + let mut s = effectcraft_engine::Session::default(); + s.execute("comp.new", json!({"width": 400, "height": 300, "duration": 2})).unwrap(); + let first = s.execute("layer.newShape", json!({"kind": "rect"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("shape.toolOptions", json!({"fill": "#00ff00", "stroke": false})).unwrap(); + let mut app = EffectcraftApp::new(s); + let ctx = egui::Context::default(); + assert_eq!(app.session.state.selected_layers.first().map(|l| l.0), Some(first), "a shape layer is selected"); + comp_sized_shape(&mut app, &ctx, Tool::Star).unwrap(); + let comp = app.session.active_comp().unwrap(); + assert_eq!(comp.layers.len(), 2, "a new layer, not a group in the selected one"); + let layer = comp.layers.iter().find(|l| l.id.0 != first).unwrap(); + let mut fill = None; + layer.props.walk("", &mut |_, pr| { + if fill.is_none() && matches!(pr.value, effectcraft_engine::keyframe::Value::Color(_)) { + fill = Some(pr.value.clone()); + } + }); + let fill = fill.expect("a fill colour"); + assert_eq!(fill, effectcraft_engine::keyframe::Value::Color([0.0, 1.0, 0.0, 1.0]), "the Tools bar's Fill, not the default blue"); + } } diff --git a/crates/ui-egui/src/menus/invoke.rs b/crates/ui-egui/src/menus/invoke.rs index 2c99274ed..f1958fb5e 100644 --- a/crates/ui-egui/src/menus/invoke.rs +++ b/crates/ui-egui/src/menus/invoke.rs @@ -218,6 +218,10 @@ pub fn invoke(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: V // leaves the active composition open (it used to close the composition next to it). if id == "file.close" && params.as_object().is_some_and(|p| p.is_empty()) && app.ui.focused == PanelKind::RenderQueue { app.close_panel(PanelKind::RenderQueue); + // The focus goes to a panel that is still there, so the next Close acts on the composition + // instead of on the queue that is gone. + app.ui.focused = + [PanelKind::Timeline, PanelKind::Composition, PanelKind::Project].into_iter().find(|p| app.ui.dock.is_visible(*p)).unwrap_or(PanelKind::Project); return Ok(Value::Null); } // Edit ▸ Paste with SVG markup on the system clipboard pastes it as a shape layer (After diff --git a/crates/ui-egui/src/menus/tests.rs b/crates/ui-egui/src/menus/tests.rs index ed35e727f..5af50f0fc 100644 --- a/crates/ui-egui/src/menus/tests.rs +++ b/crates/ui-egui/src/menus/tests.rs @@ -431,6 +431,10 @@ fn cmd_w_in_the_render_queue_closes_the_queue_not_the_comp() { assert!(!app.ui.dock.is_visible(PanelKind::RenderQueue)); assert_eq!(app.session.state.active_comp.map(|c| c.0), a); assert_eq!(app.session.state.open_comps.len(), 1); + // The focus left the closed queue: the next Close closes the composition. + assert_ne!(app.ui.focused, PanelKind::RenderQueue); + invoke(&mut app, &ctx, "file.close", json!({})).unwrap(); + assert_eq!(app.session.state.open_comps.len(), 0, "a second Close acts on the composition"); } #[test] diff --git a/crates/ui-egui/src/panels/graph.rs b/crates/ui-egui/src/panels/graph.rs index 3bd46738e..a87a909a0 100644 --- a/crates/ui-egui/src/panels/graph.rs +++ b/crates/ui-egui/src/panels/graph.rs @@ -573,48 +573,8 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte ctx.data_mut(|dd| dd.insert_temp(drag_id, false)); actions.push(("__endMerge".into(), json!({}))); } - // Right-click: the Timeline's key menu, for the commands that apply from the Graph Editor. - kresp.context_menu(|ui| { - let editable_value = matches!(key.value, Value::Scalar(_) | Value::Vec2(_) | Value::Vec3(_)); - for (lbl, cmd, params) in [ - ("Edit Value…", "keys.set", kjson.clone()), - ("-", "", json!({})), - ("Copy", "keys.copy", json!({})), - ("Paste", "keys.paste", json!({})), - ("-", "", json!({})), - ("Select Equal Keyframes", "keys.selectEqual", json!({})), - ("Select Previous Keyframes", "keys.selectPrevious", json!({})), - ("Select Following Keyframes", "keys.selectFollowing", json!({})), - ("-", "", json!({})), - ("Toggle Hold Keyframe", "keys.toggleHold", json!({})), - ("Rove Across Time", "keys.interpolation", json!({"roving": !key.roving})), - ("-", "", json!({})), - ("Easy Ease", "keys.easyEase", json!({})), - ("Easy Ease In", "keys.easyEaseIn", json!({})), - ("Easy Ease Out", "keys.easyEaseOut", json!({})), - ("-", "", json!({})), - ("Linear", "keys.interpolation", json!({"interpolation": "linear"})), - ("Bezier", "keys.interpolation", json!({"interpolation": "bezier"})), - ("Auto Bezier", "keys.interpolation", json!({"interpolation": "autoBezier"})), - ("-", "", json!({})), - ("Delete", "keys.delete", json!({})), - ] { - if lbl == "-" { - ui.separator(); - continue; - } - if lbl == "Rove Across Time" && !c.prop.spatial { - continue; - } - if lbl == "Edit Value…" && !editable_value { - continue; - } - if ui.button(lbl).clicked() { - actions.push((cmd.into(), params)); - ui.close(); - } - } - }); + // Right-click: the Timeline's key menu, with the same commands. + crate::widgets::context_menu(&kresp, |ui| super::timeline::key_menu(ui, app, c.layer, c.prop, key, k.sel, actions)); } } diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index a45d8e708..7fa5881a4 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -4006,71 +4006,88 @@ fn key_double_click(g: &mut Rw, rc: &RowCtx<'_>, prop: &Property, kh: &KeyHit<'_ /// The keyframe right-click menu. fn key_context_menu(app: &mut EffectcraftApp, g: &mut Rw, rc: &RowCtx<'_>, prop: &Property, kh: &KeyHit<'_>) { let KeyHit { k, ks, ref kresp, .. } = *kh; - let t = g.tl.t; let actions = &mut g.tl.actions; let layer = rc.layer; - let uid = &prop.uid; + crate::widgets::context_menu(kresp, |ui| key_menu(ui, app, layer, prop, k, ks, actions)); +} - crate::widgets::context_menu(kresp, |ui| { - for (lbl, cmd, params) in [ - ("Edit Value…", "keys.set", json!({"layer": layer.id.0, "prop": uid, "time": k.time.seconds()})), - ("-", "", json!({})), - ("Copy", "keys.copy", json!({})), - ("Paste", "keys.paste", json!({})), - ("-", "", json!({})), - ("Select Equal Keyframes", "keys.selectEqual", json!({})), - ("Select Previous Keyframes", "keys.selectPrevious", json!({})), - ("Select Following Keyframes", "keys.selectFollowing", json!({})), - ("-", "", json!({})), - ("Keyframe Interpolation…", "keys.interpolation", json!({})), - ("Keyframe Velocity…", "keys.velocity", json!({})), - ("Toggle Hold Keyframe", "keys.toggleHold", json!({})), - ("Rove Across Time", "keys.interpolation", json!({"roving": !k.roving})), - ("-", "", json!({})), - ("Easy Ease", "keys.easyEase", json!({})), - ("Easy Ease In", "keys.easyEaseIn", json!({})), - ("Easy Ease Out", "keys.easyEaseOut", json!({})), - ("Time-Reverse Keyframes", "keys.timeReverse", json!({})), - ("-", "", json!({})), - ("Linear", "keys.interpolation", json!({"interpolation": "linear"})), - ("Bezier", "keys.interpolation", json!({"interpolation": "bezier"})), - ("Auto Bezier", "keys.interpolation", json!({"interpolation": "autoBezier"})), - ("Select All Keyframes", "keys.selectAll", json!({})), - ("Delete", "keys.delete", json!({})), - ] { - if lbl == "Select All Keyframes" { - // Keyframe colour labels. +/// The commands of a keyframe's right-click menu, for the Timeline and the Graph Editor alike. They act +/// on the selected keys, so a menu opened on an unselected key selects it first. A locked layer's keys +/// can still be copied and selected, but not edited. +pub(crate) fn key_menu( + ui: &mut egui::Ui, + app: &EffectcraftApp, + layer: &Layer, + prop: &Property, + k: &effectcraft_engine::keyframe::Keyframe, + selected: bool, + actions: &mut Vec<(String, serde_json::Value)>, +) { + let t = app.tokens; + let locked = layer.switches.locked; + let uid = &prop.uid; + let this_key = json!({"layer": layer.id.0, "prop": uid, "time": k.time.seconds()}); + for (lbl, cmd, params) in [ + ("Edit Value…", "keys.set", this_key.clone()), + ("-", "", json!({})), + ("Copy", "keys.copy", json!({})), + ("Paste", "keys.paste", json!({})), + ("-", "", json!({})), + ("Select Equal Keyframes", "keys.selectEqual", json!({})), + ("Select Previous Keyframes", "keys.selectPrevious", json!({})), + ("Select Following Keyframes", "keys.selectFollowing", json!({})), + ("-", "", json!({})), + ("Keyframe Interpolation…", "keys.interpolation", json!({})), + ("Keyframe Velocity…", "keys.velocity", json!({})), + ("Toggle Hold Keyframe", "keys.toggleHold", json!({})), + ("Rove Across Time", "keys.interpolation", json!({"roving": !k.roving})), + ("-", "", json!({})), + ("Easy Ease", "keys.easyEase", json!({})), + ("Easy Ease In", "keys.easyEaseIn", json!({})), + ("Easy Ease Out", "keys.easyEaseOut", json!({})), + ("Time-Reverse Keyframes", "keys.timeReverse", json!({})), + ("-", "", json!({})), + ("Linear", "keys.interpolation", json!({"interpolation": "linear"})), + ("Bezier", "keys.interpolation", json!({"interpolation": "bezier"})), + ("Auto Bezier", "keys.interpolation", json!({"interpolation": "autoBezier"})), + ("Select All Keyframes", "keys.selectAll", json!({})), + ("Delete", "keys.delete", json!({})), + ] { + if lbl == "Select All Keyframes" { + // Keyframe colour labels. + ui.add_enabled_ui(!locked, |ui| { ui.menu_button(crate::i18n::tr("Label"), |ui| { for (i, l) in effectcraft_engine::color::Label::ALL.into_iter().enumerate() { if widgets::label_entry(ui, l, &app.session.prefs.label_name(l), k.label as usize == i, &t).clicked() { - if !ks { - actions.push(("keys.select".into(), json!({"keys": [{"layer": layer.id.0, "prop": uid, "time": k.time.seconds()}]}))); + if !selected { + actions.push(("keys.select".into(), json!({"keys": [this_key.clone()]}))); } actions.push(("keys.setLabel".into(), json!({"label": i}))); ui.close(); } } }); - } - if lbl == "-" { - ui.separator(); - continue; - } - if lbl == "Rove Across Time" && !prop.spatial { - continue; - } - if lbl == "Edit Value…" && !matches!(k.value, Value::Scalar(_) | Value::Vec2(_) | Value::Vec3(_)) { - continue; - } - if ui.button(lbl).clicked() { - if !ks { - actions.push(("keys.select".into(), json!({"keys": [{"layer": layer.id.0, "prop": uid, "time": k.time.seconds()}]}))); - } - actions.push((cmd.into(), params)); - ui.close(); - } + }); } - }); + if lbl == "-" { + ui.separator(); + continue; + } + if lbl == "Rove Across Time" && !prop.spatial { + continue; + } + if lbl == "Edit Value…" && !matches!(k.value, Value::Scalar(_) | Value::Vec2(_) | Value::Vec3(_)) { + continue; + } + let edits = !matches!(cmd, "keys.copy" | "keys.selectEqual" | "keys.selectPrevious" | "keys.selectFollowing" | "keys.selectAll"); + if ui.add_enabled(!(locked && edits), egui::Button::new(crate::i18n::context_item(cmd, ¶ms, lbl))).clicked() { + if !selected { + actions.push(("keys.select".into(), json!({"keys": [this_key.clone()]}))); + } + actions.push((cmd.into(), params)); + ui.close(); + } + } } /// Starts a key drag (or Alt-scale). diff --git a/crates/ui-egui/src/panels/timeline/tests.rs b/crates/ui-egui/src/panels/timeline/tests.rs index 0355ea5ed..ee0e39ca3 100644 --- a/crates/ui-egui/src/panels/timeline/tests.rs +++ b/crates/ui-egui/src/panels/timeline/tests.rs @@ -535,3 +535,46 @@ fn disk_bar_range_covers_partial_frames() { let above_limit = TMap { start: 100_001.0, ..tm }; assert_eq!(disk_bar_range(&above_limit, clip, 1.0, i64::MAX), 100_000..100_000); } + +/// The keyframe menu is one for the Timeline and the Graph Editor: its commands are drawn in the +/// interface language, and a locked layer's keys can be copied and selected but not edited. +#[test] +fn the_key_menu_is_translated_and_cannot_edit_a_locked_layers_keys() { + use egui_kittest::Harness; + use egui_kittest::kittest::{NodeT, Queryable}; + let mut s = effectcraft_engine::Session::default(); + s.execute("comp.new", json!({"name": "C", "width": 64, "height": 64, "duration": 2})).unwrap(); + let id = s.execute("layer.newSolid", json!({"name": "Box", "color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + for (t, v) in [(0.0, 0), (1.0, 50)] { + s.execute("prop.addKey", json!({"layer": id, "path": "transform/opacity", "time": t, "value": v})).unwrap(); + } + let app = EffectcraftApp::new(s); + let layer = app.session.active_comp().unwrap().layer(LayerId(id)).unwrap().clone(); + let prop = layer.props.prop("transform/opacity").unwrap().clone(); + let key = prop.keys[0].clone(); + let enabled = |locked: bool, label: &str| { + let mut layer = layer.clone(); + layer.switches.locked = locked; + let mut actions = vec![]; + let mut h = Harness::new_ui(|ui| key_menu(ui, &app, &layer, &prop, &key, true, &mut actions)); + h.run(); + !h.get_by_label(label).accesskit_node().is_disabled() + }; + for label in + ["Copy", "Select Equal Keyframes", "Select All Keyframes", "Delete", "Edit Value…", "Easy Ease", "Time-Reverse Keyframes", "Keyframe Velocity…"] + { + assert!(enabled(false, label), "{label} on an unlocked layer"); + } + for label in ["Copy", "Select Equal Keyframes", "Select All Keyframes"] { + assert!(enabled(true, label), "{label} is still available on a locked layer"); + } + for label in ["Delete", "Edit Value…", "Easy Ease", "Time-Reverse Keyframes", "Keyframe Velocity…", "Linear"] { + assert!(!enabled(true, label), "{label} is off on a locked layer"); + } + // The labels follow the interface language. + crate::i18n::set_current("es"); + let copy = crate::i18n::context_item("keys.copy", &json!({}), "Copy"); + assert_ne!(copy, "Copy", "the catalog translates Copy"); + assert!(enabled(false, ©)); + crate::i18n::set_current("en"); +} From 5b452a90fe6b617adaa3aa42ec84e59b9bc441c4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 12:40:09 +0000 Subject: [PATCH 08/23] #1190: find the pressed checkbox and the ones crossed from the frame's ordered pointer events Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01RDvxCS8xcR28VArjzn1x7U --- crates/ui-egui/src/widgets.rs | 136 +++++++++++++++++++++++++++++++++- 1 file changed, 132 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index cc6da8ebb..f8e2933ab 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -421,15 +421,50 @@ struct CheckboxDrag { press_frame: u64, target: Option, visited: std::collections::HashSet, + /// Checkboxes the pointer crossed in the frame of the press, before the checkbox it pressed on + /// had been drawn (so the value to set was not known): they take it in the next frame. + pending: std::collections::HashSet, +} + +/// A primary-button event of the frame, in the order it happened. +#[derive(Clone, Copy, PartialEq)] +enum Step { + Press, + Move, + Release, +} + +/// The frame's primary-button events in order, each with its position. egui hit-tests the whole +/// frame at the last pointer position, so a press followed by a move in one frame would otherwise +/// be taken for a press where the pointer ended. +fn pointer_steps(ui: &Ui) -> Vec<(Step, egui::Pos2)> { + ui.input(|i| { + i.events + .iter() + .filter_map(|e| match e { + egui::Event::PointerMoved(pos) => Some((Step::Move, *pos)), + egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, .. } => { + Some((if *pressed { Step::Press } else { Step::Release }, *pos)) + } + _ => None, + }) + .collect() + }) } /// Returns the requested value. Mixed checkboxes (`None`) and disabled controls are -/// skipped. Each checkbox is visited once, so crossing it again cannot toggle it back. +/// skipped. Each checkbox is visited once, so crossing it again cannot toggle it back. The +/// checkbox a press lands on, and the ones the pointer passes over after it, are found from the +/// frame's ordered pointer events, not from where the pointer ended. fn drag_toggle(ui: &Ui, response: &Response, value: Option) -> Option { let (pressed, down, released) = ui.input(|i| (i.pointer.primary_pressed(), i.pointer.primary_down(), i.pointer.primary_released())); let frame = ui.ctx().cumulative_frame_nr(); // clicked() reads Context input; do not call it while Context data is locked. let clicked = response.clicked(); + // Whether the pointer at `pos` is over this checkbox itself (not a window above it). + let over = |pos: egui::Pos2| response.rect.contains(pos) && ui.clip_rect().contains(pos) && ui.ctx().layer_id_at(pos) == Some(response.layer_id); + // Resolved before the data lock below: layer_id_at reads the Context. + let steps: Vec<(Step, bool)> = pointer_steps(ui).into_iter().map(|(step, pos)| (step, over(pos))).collect(); ui.data_mut(|data| { let drag = data.get_temp_mut_or_default::(egui::Id::unique("checkbox-drag")); if (!down && !released) || (pressed && drag.press_frame != frame) { @@ -439,10 +474,43 @@ fn drag_toggle(ui: &Ui, response: &Response, value: Option) -> Option { + press_seen = true; + if drag.target.is_none() && over { + drag.target = Some(!on); + drag.visited.insert(response.id); + return Some(!on); + } + } + Step::Move if press_seen || down => match drag.target { + Some(target) if over => { + if drag.visited.insert(response.id) && on != target { + return Some(target); + } + } + None if press_seen && over => { + drag.pending.insert(response.id); + } + _ => {} + }, + _ => {} + } + } + // The pointer held still over this checkbox in a later frame of the gesture. + if down + && !pressed && response.contains_pointer() && let Some(target) = drag.target { @@ -971,6 +1039,66 @@ mod tests { assert_eq!(values[1], Some(false), "hover after release must not paint"); } + fn button(y: f32, pressed: bool) -> Vec { + let pos = pos2(28.0, y); + vec![egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }] + } + + fn moved(y: f32) -> egui::Event { + egui::Event::PointerMoved(pos2(28.0, y)) + } + + /// A press and the moves after it can arrive in one frame (a quick flick): the press belongs to + /// the checkbox it landed on, not to the one the pointer ended over, and the ones passed over + /// follow the pressed one's new state, whether they are drawn before it or after it. + #[test] + fn a_press_and_a_move_in_one_frame_start_on_the_checkbox_pressed() { + // Rows are 30 apart, from y = 28; index 3 is disabled. + let (a, b) = (28.0, 58.0); + // Pressed on the first, moved to the second (drawn after it). + let ctx = egui::Context::default(); + let mut values = [Some(false); 5]; + values[2] = None; + checkbox_frame(&ctx, &mut values, vec![]); + let mut events = vec![moved(a)]; + events.extend(button(a, true)); + events.push(moved(b)); + checkbox_frame(&ctx, &mut values, events); + assert_eq!((values[0], values[1]), (Some(true), Some(true)), "the pressed one is the source, the one passed over follows"); + let mut events = vec![moved(b)]; + events.extend(button(b, false)); + checkbox_frame(&ctx, &mut values, events); + assert_eq!((values[0], values[1]), (Some(true), Some(true)), "release changes nothing"); + // Pressed on the second, moved to the first (drawn before it): the first takes the state in + // the next frame, while the button is still down. + let ctx = egui::Context::default(); + let mut values = [Some(false); 5]; + values[2] = None; + checkbox_frame(&ctx, &mut values, vec![]); + let mut events = vec![moved(b)]; + events.extend(button(b, true)); + events.push(moved(a)); + checkbox_frame(&ctx, &mut values, events); + assert_eq!(values[1], Some(true), "the pressed one is the source"); + checkbox_frame(&ctx, &mut values, vec![]); + assert_eq!(values[0], Some(true), "the one passed over takes the state"); + let mut events = vec![moved(a)]; + events.extend(button(a, false)); + checkbox_frame(&ctx, &mut values, events); + assert_eq!((values[0], values[1]), (Some(true), Some(true))); + // Pressed outside every checkbox, then moved onto one in the same frame: nothing starts. + let ctx = egui::Context::default(); + let mut values = [Some(false); 5]; + values[2] = None; + checkbox_frame(&ctx, &mut values, vec![]); + let mut events = vec![moved(200.0)]; + events.extend(button(200.0, true)); + events.push(moved(a)); + checkbox_frame(&ctx, &mut values, events); + checkbox_frame(&ctx, &mut values, vec![]); + assert_eq!(values, [Some(false), Some(false), None, Some(false), Some(false)], "a press outside starts no gesture"); + } + #[test] fn checkbox_click_and_keyboard_toggle_once_and_drag_must_start_on_a_checkbox() { let ctx = egui::Context::default(); From c4ca4dc0e9e220e3834e10846517dc5390d1e547 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 13:01:42 +0000 Subject: [PATCH 09/23] #1284: disable key-menu commands that need selecting an unselected key on a locked layer Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01RDvxCR4xUtUfBJwkhwaFuD --- crates/ui-egui/src/panels/timeline.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 7fa5881a4..b94519713 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -4080,7 +4080,9 @@ pub(crate) fn key_menu( continue; } let edits = !matches!(cmd, "keys.copy" | "keys.selectEqual" | "keys.selectPrevious" | "keys.selectFollowing" | "keys.selectAll"); - if ui.add_enabled(!(locked && edits), egui::Button::new(crate::i18n::context_item(cmd, ¶ms, lbl))).clicked() { + // An unselected key is selected before the command runs, and locked keys cannot be selected. + let unselectable = locked && !selected && cmd != "keys.selectAll"; + if ui.add_enabled(!(locked && edits) && !unselectable, egui::Button::new(crate::i18n::context_item(cmd, ¶ms, lbl))).clicked() { if !selected { actions.push(("keys.select".into(), json!({"keys": [this_key.clone()]}))); } From 90874c3dd2b0122740355337bd0103200111d1ce Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 13:28:09 +0000 Subject: [PATCH 10/23] #1190: apply checkbox crossings recorded before the target was known, even after release Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01RDvxCR4xUtUfBJwkhwaFuD --- crates/ui-egui/src/widgets.rs | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index f8e2933ab..cd00bffb1 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -467,7 +467,9 @@ fn drag_toggle(ui: &Ui, response: &Response, value: Option) -> Option = pointer_steps(ui).into_iter().map(|(step, pos)| (step, over(pos))).collect(); ui.data_mut(|data| { let drag = data.get_temp_mut_or_default::(egui::Id::unique("checkbox-drag")); - if (!down && !released) || (pressed && drag.press_frame != frame) { + // A release in the press frame leaves crossings pending: keep them for the next frame. + let flushing = !drag.pending.is_empty() && drag.target.is_some() && frame <= drag.press_frame.saturating_add(1); + if (!down && !released && !flushing) || (pressed && drag.press_frame != frame) { *drag = CheckboxDrag { press_frame: frame, ..Default::default() }; } let on = value?; @@ -475,8 +477,7 @@ fn drag_toggle(ui: &Ui, response: &Response, value: Option) -> Option Date: Thu, 29 Oct 2026 14:16:36 +0000 Subject: [PATCH 11/23] #1190: a move after the release in the same frame does not carry the checkbox drag Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01RDvxCS8xcR28VArjzn1x7U --- crates/ui-egui/src/widgets.rs | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index cd00bffb1..9c4f76e72 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -484,18 +484,26 @@ fn drag_toggle(ui: &Ui, response: &Response, value: Option) -> Option *step == Step::Release, + None => down, + }; let mut press_seen = false; for (step, over) in steps { match step { Step::Press => { press_seen = true; + held = true; if drag.target.is_none() && over { drag.target = Some(!on); drag.visited.insert(response.id); return Some(!on); } } - Step::Move if press_seen || down => match drag.target { + Step::Release => held = false, + Step::Move if held => match drag.target { Some(target) if over => { if drag.visited.insert(response.id) && on != target { return Some(target); @@ -1113,6 +1121,24 @@ mod tests { assert_eq!((values[0], values[1]), (Some(true), Some(true)), "a crossing before the release still takes the state"); } + /// A move that arrives after the release, in the frame of the click, is the pointer travelling without the button: + /// it must not carry the gesture onto the checkbox it ends over. + #[test] + fn a_move_after_the_release_in_the_same_frame_paints_nothing() { + let (a, b) = (28.0, 58.0); + let ctx = egui::Context::default(); + let mut values = [Some(false); 5]; + values[2] = None; + checkbox_frame(&ctx, &mut values, vec![]); + let mut events = vec![moved(a)]; + events.extend(button(a, true)); + events.extend(button(a, false)); + events.push(moved(b)); + checkbox_frame(&ctx, &mut values, events); + checkbox_frame(&ctx, &mut values, vec![]); + assert_eq!((values[0], values[1]), (Some(true), Some(false)), "the clicked one changes, the one moved onto afterwards does not"); + } + #[test] fn checkbox_click_and_keyboard_toggle_once_and_drag_must_start_on_a_checkbox() { let ctx = egui::Context::default(); From 1dc5d2db69b65e1227009a4d2dd9502292949df7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 15:53:52 +0000 Subject: [PATCH 12/23] #1284: Easy Ease, interpolation, Toggle Hold, labels and Time-Reverse skip a locked layer's keys and keep them selected Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01RDvxCS8xcR28VArjzn1x7U --- crates/engine/src/cmd/anim.rs | 8 +++++++- crates/engine/src/cmd/prop.rs | 11 ++++++++++- crates/engine/src/tests/delete_keys.rs | 20 ++++++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/anim.rs b/crates/engine/src/cmd/anim.rs index 76bfb5a63..1aecf3b5d 100644 --- a/crates/engine/src/cmd/anim.rs +++ b/crates/engine/src/cmd/anim.rs @@ -498,7 +498,13 @@ fn time_reverse(s: &mut Session, _: &Value) -> Result { } let mut new_sel = vec![]; for ((lid, uid), times) in groups { - let Some(pr) = comp.layer_mut(lid).and_then(|l| l.props.find_mut(uid)) else { continue }; + let Some(layer) = comp.layer_mut(lid) else { continue }; + // A locked layer's keys stay as they are, and stay selected. + if layer.switches.locked { + new_sel.extend(times.iter().map(|time| KeyRef { layer: lid, prop: uid, time: *time })); + continue; + } + let Some(pr) = layer.props.find_mut(uid) else { continue }; let (mut picked, rest): (Vec, Vec) = pr.keys.drain(..).partition(|k| times.contains(&k.time)); effectcraft_keyframe::time_reverse(&mut picked); new_sel.extend(picked.iter().map(|k| KeyRef { layer: lid, prop: uid, time: k.time })); diff --git a/crates/engine/src/cmd/prop.rs b/crates/engine/src/cmd/prop.rs index 36f76706f..0eff1dd09 100644 --- a/crates/engine/src/cmd/prop.rs +++ b/crates/engine/src/cmd/prop.rs @@ -895,7 +895,16 @@ pub(crate) fn edit_keys(s: &mut Session, label: &str, merge: Option<&str>, f: im groups.entry((k.layer, k.prop)).or_default().push(k.time); } for ((lid, uid), times) in groups { - let Some(pr) = comp.layer_mut(lid).and_then(|l| l.props.find_mut(uid)) else { continue }; + let Some(layer) = comp.layer_mut(lid) else { continue }; + // A locked layer's keys stay as they are, and stay selected. + if layer.switches.locked { + for time in times { + let r = KeyRef { layer: lid, prop: uid, time }; + remapped.insert(r, r); + } + continue; + } + let Some(pr) = layer.props.find_mut(uid) else { continue }; // Process each stored key once, in an order that avoids collisions when moving. let mut idx: Vec<(usize, Tick)> = times.iter().filter_map(|t| key_at(&pr.keys, *t).map(|i| (i, *t))).collect(); idx.sort_by_key(|x| x.0); diff --git a/crates/engine/src/tests/delete_keys.rs b/crates/engine/src/tests/delete_keys.rs index 029104497..829331fd7 100644 --- a/crates/engine/src/tests/delete_keys.rs +++ b/crates/engine/src/tests/delete_keys.rs @@ -64,3 +64,23 @@ fn a_locked_layers_keys_cannot_be_deleted_or_edited() { s.execute_checked("keys.set", json!({"layer": layer, "prop": prop, "time": 0, "value": 90})).unwrap(); assert_eq!(s.execute_checked("keys.delete", json!({})).unwrap(), json!(3)); } + +/// Easy Ease, Toggle Hold and Time-Reverse leave a locked layer's keys as they are and keep them +/// selected; unlocked, they change them. +#[test] +fn selection_based_key_edits_skip_a_locked_layer() { + let (mut s, layer, prop) = setup(); + let keys = keys_of(&s, layer, prop); + let stored = |s: &Session| s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.find(prop).unwrap().keys.clone(); + let before = stored(&s); + s.state.selected_keys = keys.clone(); + s.execute_checked("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": true})).unwrap(); + for command in ["keys.easyEase", "keys.toggleHold", "keys.timeReverse"] { + s.execute_checked(command, json!({})).unwrap(); + assert_eq!(stored(&s), before, "{command} changed a locked layer's keys"); + assert_eq!(s.state.selected_keys, keys, "{command} dropped the selection of a locked layer's keys"); + } + s.execute_checked("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": false})).unwrap(); + s.execute_checked("keys.easyEase", json!({})).unwrap(); + assert_ne!(stored(&s), before, "unlocked, Easy Ease changes the keys"); +} From 37e0c5dff62c8b375db80b86b849781b4a4c798a Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Thu, 29 Oct 2026 19:38:48 +0100 Subject: [PATCH 13/23] #1284: reject Graph Editor ease edits on locked layers --- crates/engine/src/cmd/anim.rs | 3 +++ crates/engine/src/tests/timeline.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/crates/engine/src/cmd/anim.rs b/crates/engine/src/cmd/anim.rs index 1aecf3b5d..11aad5e8b 100644 --- a/crates/engine/src/cmd/anim.rs +++ b/crates/engine/src/cmd/anim.rs @@ -352,6 +352,9 @@ fn set_ease(s: &mut Session, p: &Value) -> Result { let influence = f_p(p, "influence").map(|v| (v / 100.0).clamp(0.001, 1.0)); s.edit("Keyframe Velocity", merge_p(p), |proj, _| { let l = super::layer_mut(proj, cid, lid)?; + if l.switches.locked { + return Err(bad("keys.setEase", "the layer is locked")); + } let pr = l.props.find_mut(uid).ok_or_else(|| bad("keys.setEase", "no property"))?; let i = nearest_key(&pr.keys, t).ok_or_else(|| bad("keys.setEase", "no keyframes"))?; let spatial = pr.spatial; diff --git a/crates/engine/src/tests/timeline.rs b/crates/engine/src/tests/timeline.rs index 1330f0b09..c96053bf2 100644 --- a/crates/engine/src/tests/timeline.rs +++ b/crates/engine/src/tests/timeline.rs @@ -344,6 +344,34 @@ fn graph_editor_key_and_handle_edits() { assert_eq!(prop(&s, l, "transform/opacity").keys[0].out_interp, Interp::Linear); } +#[test] +fn graph_editor_ease_edits_reject_a_locked_layer() { + let (mut s, l) = setup(); + animate(&mut s, l, "transform/opacity", &[(0.0, json!(0)), (1.0, json!(100)), (2.0, json!(0))]); + select_prop_keys(&mut s, l, "transform/opacity"); + s.execute("layer.setSwitch", json!({"layers": [l], "switch": "locked", "value": true})).unwrap(); + let before = prop(&s, l, "transform/opacity"); + let selected = s.state.selected_keys.clone(); + let undo = s.history.undo.len(); + for side in ["in", "out"] { + let error = + s.execute("keys.setEase", json!({"layer": l, "path": "transform/opacity", "time": 1.0, "side": side, "speed": 200, "influence": 60})).unwrap_err(); + assert!(error.to_string().contains("locked"), "{error}"); + assert_eq!(prop(&s, l, "transform/opacity").keys, before.keys); + assert_eq!(s.state.selected_keys, selected); + assert_eq!(s.history.undo.len(), undo); + } + s.execute("layer.setSwitch", json!({"layers": [l], "switch": "locked", "value": false})).unwrap(); + s.execute("keys.setEase", json!({"layer": l, "path": "transform/opacity", "time": 1.0, "side": "out", "speed": 200, "influence": 60})).unwrap(); + let after = prop(&s, l, "transform/opacity"); + assert_eq!(after.keys[1].out_interp, Interp::Bezier); + assert_eq!(after.keys[1].out_ease[0].speed, 200.0); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(prop(&s, l, "transform/opacity").keys, before.keys); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(prop(&s, l, "transform/opacity").keys, after.keys); +} + #[test] fn convert_expression_to_keyframes_on_a_reversed_layer_keeps_values() { let (mut s, l) = setup(); From 5d02db47408f5c4facf68250d7c3cf05371315be Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Thu, 29 Oct 2026 20:01:08 +0100 Subject: [PATCH 14/23] #1284: preserve locked keys during group transforms and cover unselected menus --- crates/engine/src/cmd/key_transform.rs | 4 +++ crates/engine/src/tests/delete_keys.rs | 39 +++++++++++++++++++++ crates/ui-egui/src/panels/timeline/tests.rs | 17 +++++---- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/key_transform.rs b/crates/engine/src/cmd/key_transform.rs index b78439266..641759095 100644 --- a/crates/engine/src/cmd/key_transform.rs +++ b/crates/engine/src/cmd/key_transform.rs @@ -72,6 +72,10 @@ fn transform(s: &mut Session, p: &Value) -> Result { let mut new_sel = vec![]; for ((lid, uid), times) in groups { let Some(layer) = comp.layer_mut(lid) else { continue }; + if layer.switches.locked { + new_sel.extend(times.into_iter().map(|time| KeyRef { layer: lid, prop: uid, time })); + continue; + } let (start, stretch) = (layer.start_time, layer.stretch); let to_comp = |lt: Tick| if (stretch - 100.0).abs() < 1e-9 { start + lt } else { start + Tick((lt.0 as f64 * stretch / 100.0) as i64) }; let to_layer = |ct: Tick| { diff --git a/crates/engine/src/tests/delete_keys.rs b/crates/engine/src/tests/delete_keys.rs index 829331fd7..fb8162855 100644 --- a/crates/engine/src/tests/delete_keys.rs +++ b/crates/engine/src/tests/delete_keys.rs @@ -84,3 +84,42 @@ fn selection_based_key_edits_skip_a_locked_layer() { s.execute_checked("keys.easyEase", json!({})).unwrap(); assert_ne!(stored(&s), before, "unlocked, Easy Ease changes the keys"); } + +#[test] +fn key_transform_preserves_locked_keys_in_a_mixed_selection() { + let (mut s, locked, locked_prop) = setup(); + let unlocked = s.execute("layer.newSolid", json!({"name": "Unlocked", "color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + for t in 0..3 { + s.execute("prop.addKey", json!({"layer": unlocked, "path": "transform/opacity", "time": t, "value": t * 25})).unwrap(); + } + let unlocked_prop = s.active_comp().unwrap().layer(LayerId(unlocked)).unwrap().props.prop("transform/opacity").unwrap().uid; + let locked_keys = keys_of(&s, locked, locked_prop); + s.state.selected_keys = locked_keys.iter().chain(keys_of(&s, unlocked, unlocked_prop).iter()).copied().collect(); + s.execute("layer.setSwitch", json!({"layers": [locked], "switch": "locked", "value": true})).unwrap(); + let stored = |s: &Session, layer, prop| s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.find(prop).unwrap().keys.clone(); + let before = stored(&s, locked, locked_prop); + let undo = s.history.undo.len(); + for offset in [0.5, 1.0] { + s.execute("keys.transform", json!({"timeOffset": offset, "valueOffset": offset * 10.0, "merge": "mixed-transform", "fromStart": true})).unwrap(); + assert_eq!(stored(&s, locked, locked_prop), before); + assert_eq!(s.state.selected_keys.len(), 6); + assert!(locked_keys.iter().all(|k| s.state.selected_keys.contains(k)), "locked selections stay at their original times"); + assert!(keys_of(&s, unlocked, unlocked_prop).iter().all(|k| s.state.selected_keys.contains(k)), "unlocked selections follow the keys"); + let moved = stored(&s, unlocked, unlocked_prop); + for (i, key) in moved.iter().enumerate() { + assert_eq!(key.time.seconds(), i as f64 + offset); + assert_eq!(key.value.as_f64(), i as f64 * 25.0 + offset * 10.0); + } + } + assert_eq!(s.history.undo.len(), undo + 1, "one merged drag"); + let after = stored(&s, unlocked, unlocked_prop); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(stored(&s, locked, locked_prop), before); + assert_eq!(stored(&s, unlocked, unlocked_prop), before); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(stored(&s, locked, locked_prop), before); + assert_eq!(stored(&s, unlocked, unlocked_prop), after); + s.execute("layer.setSwitch", json!({"layers": [locked], "switch": "locked", "value": false})).unwrap(); + s.execute("keys.transform", json!({"timeOffset": 0.5, "valueOffset": 10})).unwrap(); + assert_ne!(stored(&s, locked, locked_prop), before, "unlocking makes the keys editable again"); +} diff --git a/crates/ui-egui/src/panels/timeline/tests.rs b/crates/ui-egui/src/panels/timeline/tests.rs index ee0e39ca3..4b7bdefd5 100644 --- a/crates/ui-egui/src/panels/timeline/tests.rs +++ b/crates/ui-egui/src/panels/timeline/tests.rs @@ -552,29 +552,34 @@ fn the_key_menu_is_translated_and_cannot_edit_a_locked_layers_keys() { let layer = app.session.active_comp().unwrap().layer(LayerId(id)).unwrap().clone(); let prop = layer.props.prop("transform/opacity").unwrap().clone(); let key = prop.keys[0].clone(); - let enabled = |locked: bool, label: &str| { + let enabled = |locked: bool, selected: bool, label: &str| { let mut layer = layer.clone(); layer.switches.locked = locked; let mut actions = vec![]; - let mut h = Harness::new_ui(|ui| key_menu(ui, &app, &layer, &prop, &key, true, &mut actions)); + let mut h = Harness::new_ui(|ui| key_menu(ui, &app, &layer, &prop, &key, selected, &mut actions)); h.run(); !h.get_by_label(label).accesskit_node().is_disabled() }; for label in ["Copy", "Select Equal Keyframes", "Select All Keyframes", "Delete", "Edit Value…", "Easy Ease", "Time-Reverse Keyframes", "Keyframe Velocity…"] { - assert!(enabled(false, label), "{label} on an unlocked layer"); + assert!(enabled(false, true, label), "{label} on an unlocked layer"); } for label in ["Copy", "Select Equal Keyframes", "Select All Keyframes"] { - assert!(enabled(true, label), "{label} is still available on a locked layer"); + assert!(enabled(true, true, label), "{label} is still available on a locked layer"); } for label in ["Delete", "Edit Value…", "Easy Ease", "Time-Reverse Keyframes", "Keyframe Velocity…", "Linear"] { - assert!(!enabled(true, label), "{label} is off on a locked layer"); + assert!(!enabled(true, true, label), "{label} is off on a locked layer"); + assert!(!enabled(true, false, label), "{label} is off on an unselected locked key"); } + for label in ["Copy", "Select Equal Keyframes", "Select Previous Keyframes", "Select Following Keyframes"] { + assert!(!enabled(true, false, label), "{label} requires selecting the clicked key"); + } + assert!(enabled(true, false, "Select All Keyframes")); // The labels follow the interface language. crate::i18n::set_current("es"); let copy = crate::i18n::context_item("keys.copy", &json!({}), "Copy"); assert_ne!(copy, "Copy", "the catalog translates Copy"); - assert!(enabled(false, ©)); + assert!(enabled(false, true, ©)); crate::i18n::set_current("en"); } From 0c4668df85cb4cd9e839ef8aa0c635fe98824c74 Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Thu, 29 Oct 2026 20:59:48 +0100 Subject: [PATCH 15/23] Graph Editor: preserve locked keys across gestures (#1342) --- crates/engine/src/cmd/key_transform.rs | 3 + crates/engine/src/tests/delete_keys.rs | 29 +++++++++ crates/ui-egui/src/header.rs | 22 ++++++- crates/ui-egui/src/panels/graph.rs | 19 ++++-- crates/ui-egui/src/panels/timeline/tests.rs | 31 +++++++++ crates/ui-egui/tests/it/ui_graph_fit.rs | 72 +++++++++++++++++++++ 6 files changed, 168 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/cmd/key_transform.rs b/crates/engine/src/cmd/key_transform.rs index 641759095..397db4de5 100644 --- a/crates/engine/src/cmd/key_transform.rs +++ b/crates/engine/src/cmd/key_transform.rs @@ -126,6 +126,9 @@ fn spatial_tangents(s: &mut Session, p: &Value) -> Result { let brk = b_p(p, "break").unwrap_or(false); s.edit("Edit Spatial Tangents", merge_p(p), |proj, _| { let l = super::layer_mut(proj, cid, lid)?; + if l.switches.locked { + return Err(bad("keys.setSpatialTangents", "the layer is locked")); + } let pr = l.props.find_mut(uid).ok_or_else(|| bad("keys.setSpatialTangents", "no property"))?; if !pr.spatial { return Err(bad("keys.setSpatialTangents", "not a spatial property")); diff --git a/crates/engine/src/tests/delete_keys.rs b/crates/engine/src/tests/delete_keys.rs index fb8162855..457808a92 100644 --- a/crates/engine/src/tests/delete_keys.rs +++ b/crates/engine/src/tests/delete_keys.rs @@ -85,6 +85,35 @@ fn selection_based_key_edits_skip_a_locked_layer() { assert_ne!(stored(&s), before, "unlocked, Easy Ease changes the keys"); } +#[test] +fn spatial_handles_cannot_edit_a_locked_layer() { + let (mut s, layer, _) = setup(); + for (time, value) in [(0, [10, 20, 0]), (1, [80, 60, 0])] { + s.execute("prop.addKey", json!({"layer": layer, "path": "transform/position", "time": time, "value": value})).unwrap(); + } + let prop = s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.prop("transform/position").unwrap().uid; + s.execute("keys.select", json!({"keys": [{"layer": layer, "prop": prop, "time": 0}]})).unwrap(); + s.execute("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": true})).unwrap(); + let before = s.project.to_json(); + let selected = s.state.selected_keys.clone(); + let undo_len = s.history.undo.len(); + let params = json!({"layer": layer, "prop": prop, "time": 0, "out": [15, 25, 0]}); + let error = s.execute("keys.setSpatialTangents", params.clone()).unwrap_err().to_string(); + assert!(error.contains("locked"), "{error}"); + assert_eq!(s.project.to_json(), before); + assert_eq!(s.state.selected_keys, selected); + assert_eq!(s.history.undo.len(), undo_len); + s.execute("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": false})).unwrap(); + let unlocked = s.project.to_json(); + s.execute("keys.setSpatialTangents", params).unwrap(); + let edited = s.project.to_json(); + assert_ne!(edited, unlocked); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), unlocked); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), edited); +} + #[test] fn key_transform_preserves_locked_keys_in_a_mixed_selection() { let (mut s, locked, locked_prop) = setup(); diff --git a/crates/ui-egui/src/header.rs b/crates/ui-egui/src/header.rs index 8a336fcbb..55659e8b1 100644 --- a/crates/ui-egui/src/header.rs +++ b/crates/ui-egui/src/header.rs @@ -883,21 +883,41 @@ mod tests { let mut s = effectcraft_engine::Session::default(); s.execute("comp.new", json!({"width": 400, "height": 300, "duration": 2})).unwrap(); let first = s.execute("layer.newShape", json!({"kind": "rect"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("shape.fillStroke", json!({"layers": [first], "fill": "#00ff00", "stroke": false})).unwrap(); s.execute("shape.toolOptions", json!({"fill": "#00ff00", "stroke": false})).unwrap(); let mut app = EffectcraftApp::new(s); let ctx = egui::Context::default(); assert_eq!(app.session.state.selected_layers.first().map(|l| l.0), Some(first), "a shape layer is selected"); - comp_sized_shape(&mut app, &ctx, Tool::Star).unwrap(); + crate::theme::install(&ctx, &app.tokens, crate::i18n::language(&app)); + frame(&mut app, &ctx, 0.0, vec![]); + frame(&mut app, &ctx, 0.1, vec![]); + assert_eq!(app.session.state.shape_tool.fill.color, [0.0, 1.0, 0.0], "the visible toolbar paint"); + let [x, y, w, h] = app.auto.find("tools.Rectangle").unwrap().rect; + let at = egui::pos2(x + w / 2.0, y + h / 2.0); + let pointer = |pressed| egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }; + frame(&mut app, &ctx, 1.0, vec![egui::Event::PointerMoved(at), pointer(true)]); + frame(&mut app, &ctx, 1.05, vec![pointer(false)]); + assert_eq!(app.session.active_comp().unwrap().layers.len(), 1, "a single click only selects the tool"); + // A separate double-click must route through the toolbar response, not just its helper. + for (time, pressed) in [(2.0, true), (2.05, false), (2.1, true), (2.15, false)] { + frame(&mut app, &ctx, time, vec![pointer(pressed)]); + } let comp = app.session.active_comp().unwrap(); assert_eq!(comp.layers.len(), 2, "a new layer, not a group in the selected one"); let layer = comp.layers.iter().find(|l| l.id.0 != first).unwrap(); + assert_eq!(layer.props.prop("transform/position").unwrap().value.components(), vec![200.0, 150.0, 0.0]); let mut fill = None; + let mut size = None; layer.props.walk("", &mut |_, pr| { + if pr.name == "Size" { + size = Some(pr.value.clone()); + } if fill.is_none() && matches!(pr.value, effectcraft_engine::keyframe::Value::Color(_)) { fill = Some(pr.value.clone()); } }); let fill = fill.expect("a fill colour"); + assert_eq!(size, Some(effectcraft_engine::keyframe::Value::Vec2([400.0, 300.0]))); assert_eq!(fill, effectcraft_engine::keyframe::Value::Color([0.0, 1.0, 0.0, 1.0]), "the Tools bar's Fill, not the default blue"); } } diff --git a/crates/ui-egui/src/panels/graph.rs b/crates/ui-egui/src/panels/graph.rs index a87a909a0..24b48bc86 100644 --- a/crates/ui-egui/src/panels/graph.rs +++ b/crates/ui-egui/src/panels/graph.rs @@ -456,6 +456,7 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte let mut key_screens: Vec<(serde_json::Value, Pos2)> = vec![]; for c in &cs { let (lid, uid, d) = (c.layer.id, c.prop.uid, c.dim); + let unlocked = !c.layer.switches.locked; for k in &c.keys { let kp = pos2(tm.x(k.t), ymap(k.v)); // In the speed graph an end key rises with the handle being dragged; skipping it once it @@ -477,9 +478,10 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte app.auto.add(&format!("timeline.graph.handle.{uid}.{d}.{}.{side}", k.idx), hr, "Bezier handle"); // The same side's handles of every selected key move with it, by as much as the // pointer moved (After Effects), in one undo step. - if hresp.drag_started() { + if unlocked && hresp.drag_started() { let grabs: Vec = cs .iter() + .filter(|c2| !c2.layer.switches.locked) .flat_map(|c2| { c2.keys.iter().flat_map(move |k2| { k2.handles.iter().filter(|h2| h2.out == h.out).map(move |h2| HandleGrab { @@ -499,7 +501,8 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte let press = ui.input(|i| i.pointer.press_origin()).unwrap_or(hp); ctx.data_mut(|dd| dd.insert_temp::(handle_drag_id(), (press, grabs))); } - if hresp.dragged() + if unlocked + && hresp.dragged() && let Some(pt) = hresp.interact_pointer_pos() && let Some((press, grabs)) = ctx.data(|dd| dd.get_temp::(handle_drag_id())) { @@ -530,7 +533,7 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte app.auto.add(&format!("timeline.graph.key.{uid}.{d}.{}", k.idx), kr, &c.prop.name); let mods = ui.input(|i| i.modifiers); let key = &c.prop.keys[k.idx]; - if kresp.clicked() && mods.command { + if unlocked && kresp.clicked() && mods.command { // Ctrl+click: Linear ↔ Auto Bezier; Ctrl+Alt+click: Hold on / off (as in the Timeline). actions.push(("keys.select".into(), json!({"keys": [kjson.clone()]}))); if mods.alt { @@ -539,20 +542,22 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte let linear = key.in_interp == Interp::Linear && key.out_interp == Interp::Linear; actions.push(("keys.interpolation".into(), json!({"interpolation": if linear { "autoBezier" } else { "linear" }}))); } - } else if kresp.clicked() && mods.shift { + } else if unlocked && kresp.clicked() && mods.shift { actions.push(("keys.select".into(), json!({"keys": [kjson.clone()], "toggle": true}))); - } else if (kresp.clicked() || kresp.drag_started() || kresp.secondary_clicked()) && !k.sel { + } else if unlocked && (kresp.clicked() || kresp.drag_started() || kresp.secondary_clicked()) && !k.sel { actions.push(("keys.select".into(), json!({"keys": [kjson.clone()], "add": mods.shift}))); } // A drag moves every selected key, by how far the pointer went from where the button // went down (the key doesn't jump under the pointer); Shift keeps it to the time or // the value axis, whichever moved more. - if kresp.drag_started() + if unlocked + && kresp.drag_started() && let Some(o) = ui.input(|i| i.pointer.press_origin()) { ctx.data_mut(|dd| dd.insert_temp(key_grab_id(), KeyGrab { press: o, t: k.t })); } - if kresp.dragged() + if unlocked + && kresp.dragged() && let Some(pt) = kresp.interact_pointer_pos() && let Some(g) = ctx.data(|dd| dd.get_temp::(key_grab_id())) { diff --git a/crates/ui-egui/src/panels/timeline/tests.rs b/crates/ui-egui/src/panels/timeline/tests.rs index 4b7bdefd5..c46699eeb 100644 --- a/crates/ui-egui/src/panels/timeline/tests.rs +++ b/crates/ui-egui/src/panels/timeline/tests.rs @@ -583,3 +583,34 @@ fn the_key_menu_is_translated_and_cannot_edit_a_locked_layers_keys() { assert!(enabled(false, true, ©)); crate::i18n::set_current("en"); } + +#[test] +fn a_key_menu_edit_selects_the_clicked_key_before_applying_the_command() { + use egui_kittest::{Harness, kittest::Queryable}; + let mut s = effectcraft_engine::Session::default(); + s.execute("comp.new", json!({"width": 64, "height": 64, "duration": 2})).unwrap(); + let id = s.execute("layer.newSolid", json!({"color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + for (time, value) in [(0.0, 0), (1.0, 50)] { + s.execute("prop.addKey", json!({"layer": id, "path": "transform/opacity", "time": time, "value": value})).unwrap(); + } + let layer = s.active_comp().unwrap().layer(LayerId(id)).unwrap().clone(); + let prop = layer.props.prop("transform/opacity").unwrap().clone(); + s.execute("keys.select", json!({"keys": [{"layer": id, "prop": prop.uid, "time": 1.0}]})).unwrap(); + let mut app = EffectcraftApp::new(s); + let mut actions = vec![]; + { + let mut h = Harness::new_ui(|ui| key_menu(ui, &app, &layer, &prop, &prop.keys[0], false, &mut actions)); + h.run(); + h.get_by_label("Easy Ease").click(); + h.run(); + } + assert_eq!(actions.iter().map(|(command, _)| command.as_str()).collect::>(), ["keys.select", "keys.easyEase"]); + for (command, params) in actions { + app.session.execute(&command, params).unwrap(); + } + let keys = &app.session.active_comp().unwrap().layer(LayerId(id)).unwrap().props.find(prop.uid).unwrap().keys; + assert_ne!(keys[0], prop.keys[0], "the clicked key was eased"); + assert_eq!(keys[1], prop.keys[1], "the previously selected key was not edited"); + assert_eq!(app.session.state.selected_keys.len(), 1); + assert_eq!(app.session.state.selected_keys[0].time, prop.keys[0].time); +} diff --git a/crates/ui-egui/tests/it/ui_graph_fit.rs b/crates/ui-egui/tests/it/ui_graph_fit.rs index c161dfef6..d80ae315e 100644 --- a/crates/ui-egui/tests/it/ui_graph_fit.rs +++ b/crates/ui-egui/tests/it/ui_graph_fit.rs @@ -77,3 +77,75 @@ fn open_graph_toggle_twice(h: &mut Harness<'_, EffectcraftApp>) { assert!(!h.state().ui.timeline.graph_editor); open_graph(h); } + +#[test] +fn locked_graph_keys_preserve_the_existing_selection_and_keyframes() { + use egui::{Event, Modifiers, PointerButton, pos2, vec2}; + for (button, modifiers, drag) in [ + (PointerButton::Primary, Modifiers::NONE, false), + (PointerButton::Primary, Modifiers::SHIFT, false), + (PointerButton::Primary, Modifiers::COMMAND, false), + (PointerButton::Secondary, Modifiers::NONE, false), + (PointerButton::Primary, Modifiers::NONE, true), + ] { + let (mut h, layer, uid) = harness(true); + let s = &mut h.state_mut().session; + let locked = s.execute("layer.newSolid", json!({"name": "Locked", "color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + for (time, value) in [(9.0, 100.0), (11.0, 200.0)] { + s.execute("prop.addKey", json!({"layer": locked, "path": "transform/rotation", "time": time, "value": value})).unwrap(); + } + let locked_id = effectcraft_engine::project::LayerId(locked); + let locked_uid = s.active_comp().unwrap().layer(locked_id).unwrap().props.prop("transform/rotation").unwrap().uid; + s.execute("layer.setSwitch", json!({"layers": [locked], "switch": "locked", "value": true})).unwrap(); + s.execute("keys.select", json!({"keys": [{"layer": layer, "prop": uid, "time": 12.0}]})).unwrap(); + s.state.selected_props = vec![(effectcraft_engine::project::LayerId(layer), uid), (locked_id, locked_uid)]; + let selection = s.state.selected_keys.clone(); + let before = s.project.clone(); + open_graph(&mut h); + h.state_mut().ui.timeline.graph_auto_zoom = false; + h.state_mut().ui.timeline.graph_range = Some((0.0, 800.0)); + h.run_steps(3); + let [x, y, w, height] = h.state().auto.find(&format!("timeline.graph.key.{locked_uid}.0.0")).expect("locked key drawn").rect; + let point = pos2(x + w / 2.0, y + height / 2.0); + h.input_mut().events.push(Event::ModifiersChanged(modifiers)); + h.input_mut().events.push(Event::PointerMoved(point)); + h.input_mut().events.push(Event::PointerButton { pos: point, button, pressed: true, modifiers }); + h.step(); + let end = if drag { point + vec2(30.0, -20.0) } else { point }; + h.input_mut().events.push(Event::PointerMoved(end)); + h.step(); + h.input_mut().events.push(Event::PointerButton { pos: end, button, pressed: false, modifiers }); + h.run_steps(3); + assert_eq!(h.state().session.state.selected_keys, selection, "{button:?}, {modifiers:?}, drag={drag}"); + assert_eq!( + serde_json::to_value(&h.state().session.project).unwrap(), + serde_json::to_value(&before).unwrap(), + "locked gestures must not edit another key" + ); + } +} + +#[test] +fn an_unlocked_graph_keys_menu_edits_the_clicked_key() { + use egui::{Event, Modifiers, PointerButton, pos2}; + use egui_kittest::kittest::Queryable; + let (mut h, layer, uid) = harness(true); + h.state_mut().session.execute("keys.select", json!({"keys": [{"layer": layer, "prop": uid, "time": 12.0}]})).unwrap(); + open_graph(&mut h); + let [x, y, w, height] = h.state().auto.find(&format!("timeline.graph.key.{uid}.0.0")).unwrap().rect; + let at = pos2(x + w / 2.0, y + height / 2.0); + for pressed in [true, false] { + h.input_mut().events.push(Event::PointerMoved(at)); + h.input_mut().events.push(Event::PointerButton { pos: at, button: PointerButton::Secondary, pressed, modifiers: Modifiers::NONE }); + h.step(); + } + h.run_steps(3); + let selected = &h.state().session.state.selected_keys; + assert_eq!(selected.len(), 1); + assert_eq!(selected[0].time.seconds(), 8.0); + h.get_by_label("Easy Ease").click(); + h.run_steps(3); + let prop = h.state().session.active_comp().unwrap().layer(effectcraft_engine::project::LayerId(layer)).unwrap().props.find(uid).unwrap(); + assert_eq!(prop.keys[0].out_interp, effectcraft_engine::keyframe::Interp::Bezier); + assert_eq!(prop.keys[1].out_interp, effectcraft_engine::keyframe::Interp::Linear); +} From e32eaa03ad8adef265588717a55de1c7d8311c28 Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Thu, 29 Oct 2026 21:31:23 +0100 Subject: [PATCH 16/23] Graph Editor: close remaining lock and menu gaps (#1342) --- crates/engine/src/cmd/anim.rs | 14 ++++-- crates/engine/src/cmd/shape_stroke.rs | 4 +- crates/engine/src/tests/delete_keys.rs | 67 ++++++++++++++++++++++++++ crates/ui-egui/src/header.rs | 36 ++++++++++++++ crates/ui-egui/src/menus/entries.rs | 3 ++ crates/ui-egui/src/menus/invoke.rs | 7 +-- crates/ui-egui/src/menus/tests.rs | 36 ++++++++++++++ 7 files changed, 160 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/cmd/anim.rs b/crates/engine/src/cmd/anim.rs index 11aad5e8b..073262d0f 100644 --- a/crates/engine/src/cmd/anim.rs +++ b/crates/engine/src/cmd/anim.rs @@ -89,6 +89,9 @@ fn paste(s: &mut Session, p: &Value) -> Result { let mut plan: Vec<(LayerId, Uid, usize)> = vec![]; for (ti, lid) in targets.iter().enumerate() { let Some(l) = comp.layer(*lid) else { continue }; + if l.switches.locked { + continue; + } for (ci, c) in clip.iter().enumerate() { if paired && sources.get(ti) != Some(&c.layer) { continue; @@ -110,11 +113,12 @@ fn paste(s: &mut Session, p: &Value) -> Result { } } if plan.is_empty() { - return Err(bad("keys.paste", "no compatible property to paste into (select a property of the same type)")); + return Err(bad("keys.paste", "no compatible unlocked property to paste into (unlock the target or select a property of the same type)")); } s.edit("Paste Keyframes", merge_p(p), |proj, st| { let comp = proj.comp_mut(cid).ok_or(EngineError::NoComp)?; - let mut sel = vec![]; + let mut sel: Vec = st.selected_keys.iter().filter(|k| comp.layer(k.layer).is_some_and(|l| l.switches.locked)).copied().collect(); + let retained = sel.len(); for (lid, uid, ci) in &plan { let Some(l) = comp.layer_mut(*lid) else { continue }; let times: Vec = clip[*ci].keys.iter().map(|k| l.layer_time(t + k.time)).collect(); @@ -136,8 +140,9 @@ fn paste(s: &mut Session, p: &Value) -> Result { } retime_roving(&mut pr.keys, pr.spatial); } + let pasted = sel.len().saturating_sub(retained); st.selected_keys = sel; - Ok(json!(st.selected_keys.len())) + Ok(json!(pasted)) }) } @@ -175,6 +180,9 @@ pub(super) fn paste_path(s: &mut Session, p: &Value, (cid, lid, uid): (ItemId, L let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; let t = time_p(s, p, Some(comp)); let target = comp.layer(lid).ok_or(EngineError::NoComp)?; + if target.switches.locked { + return Err(bad("keys.paste", "the target layer is locked")); + } let mut order: Vec = (0..path.vertices.len()).collect(); if path.closed && !order.is_empty() { order.push(0); diff --git a/crates/engine/src/cmd/shape_stroke.rs b/crates/engine/src/cmd/shape_stroke.rs index ecd3ed8e0..e81103cdf 100644 --- a/crates/engine/src/cmd/shape_stroke.rs +++ b/crates/engine/src/cmd/shape_stroke.rs @@ -216,10 +216,12 @@ pub fn first_paint(layer: &Layer, lt: Tick) -> (Option<[f64; 4]>, Option<[f64; 4 "fill" if fill.is_none() => fill = at("color").map(rgba), "stroke" if stroke.is_none() => { stroke = at("color").map(rgba); - width = at("width").map(|v| v.as_f64()); } _ => {} } + if width.is_none() && matches!(g.match_id.as_str(), "stroke" | "gstroke") { + width = at("width").map(|v| v.as_f64()); + } }); } (fill, stroke, width) diff --git a/crates/engine/src/tests/delete_keys.rs b/crates/engine/src/tests/delete_keys.rs index 457808a92..a6ca08bed 100644 --- a/crates/engine/src/tests/delete_keys.rs +++ b/crates/engine/src/tests/delete_keys.rs @@ -114,6 +114,73 @@ fn spatial_handles_cannot_edit_a_locked_layer() { assert_eq!(s.project.to_json(), edited); } +#[test] +fn pasting_keys_or_motion_paths_cannot_edit_locked_targets() { + for path_clip in [false, true] { + for explicit in [false, true] { + let (mut s, layer, opacity) = setup(); + let target_path = if path_clip { "transform/position" } else { "transform/opacity" }; + let source = if path_clip { + s.execute("mask.new", json!({"layer": layer, "vertices": [[0, 0], [20, 10], [40, 0]], "closed": false})).unwrap(); + s.execute("prop.toggleAnimation", json!({"layer": layer, "path": "masks/#1/path"})).unwrap(); + s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.prop("masks/#1/path").unwrap().uid + } else { + opacity + }; + s.execute("keys.select", json!({"keys": [{"layer": layer, "prop": source, "time": 0}]})).unwrap(); + s.execute("keys.copy", json!({})).unwrap(); + s.execute("prop.select", json!({"layer": layer, "path": target_path})).unwrap(); + s.execute("time.set", json!({"time": 3})).unwrap(); + s.execute("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": true})).unwrap(); + let params = if explicit { json!({"layer": layer, "path": target_path}) } else { json!({}) }; + let before = s.project.to_json(); + let selected = s.state.selected_keys.clone(); + let undo_len = s.history.undo.len(); + let result = s.execute("keys.paste", params.clone()); + assert!(result.is_err(), "path={path_clip}, explicit={explicit}: {result:?}"); + assert_eq!(s.project.to_json(), before); + assert_eq!(s.state.selected_keys, selected); + assert_eq!(s.history.undo.len(), undo_len); + s.execute("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": false})).unwrap(); + let unlocked = s.project.to_json(); + s.execute("keys.paste", params).unwrap(); + let pasted = s.project.to_json(); + assert_ne!(pasted, unlocked); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), unlocked); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), pasted); + } + } +} + +#[test] +fn key_paste_skips_locked_targets_without_changing_layer_pairing() { + let (mut s, locked, locked_prop) = setup(); + let other = s.execute("layer.newSolid", json!({"name": "Other", "color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + for (time, value) in [(0, 100), (1, 80), (2, 60)] { + s.execute("prop.addKey", json!({"layer": other, "path": "transform/opacity", "time": time, "value": value})).unwrap(); + } + let other_prop = s.active_comp().unwrap().layer(LayerId(other)).unwrap().props.prop("transform/opacity").unwrap().uid; + let locked_keys = keys_of(&s, locked, locked_prop); + s.state.selected_keys = locked_keys.iter().chain(keys_of(&s, other, other_prop).iter()).copied().collect(); + s.execute("keys.copy", json!({})).unwrap(); + s.execute("layer.setSwitch", json!({"layers": [locked], "switch": "locked", "value": true})).unwrap(); + s.execute("time.set", json!({"time": 3})).unwrap(); + let before = s.project.to_json(); + assert_eq!(s.execute("keys.paste", json!({"layers": [locked, other]})).unwrap(), json!(3), "only newly pasted keys are counted"); + assert_eq!(keys_of(&s, locked, locked_prop), locked_keys); + assert!(locked_keys.iter().all(|k| s.state.selected_keys.contains(k))); + assert_eq!(s.state.selected_keys.len(), 6); + let keys = &s.active_comp().unwrap().layer(LayerId(other)).unwrap().props.find(other_prop).unwrap().keys; + assert_eq!(keys.iter().filter(|k| k.time.seconds() >= 3.0).map(|k| k.value.as_f64()).collect::>(), [100.0, 80.0, 60.0]); + let pasted = s.project.to_json(); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), pasted); +} + #[test] fn key_transform_preserves_locked_keys_in_a_mixed_selection() { let (mut s, locked, locked_prop) = setup(); diff --git a/crates/ui-egui/src/header.rs b/crates/ui-egui/src/header.rs index 55659e8b1..2c14e188b 100644 --- a/crates/ui-egui/src/header.rs +++ b/crates/ui-egui/src/header.rs @@ -920,4 +920,40 @@ mod tests { assert_eq!(size, Some(effectcraft_engine::keyframe::Value::Vec2([400.0, 300.0]))); assert_eq!(fill, effectcraft_engine::keyframe::Value::Color([0.0, 1.0, 0.0, 1.0]), "the Tools bar's Fill, not the default blue"); } + + #[test] + fn double_clicked_shapes_keep_the_selected_gradient_stroke_width() { + for kind in ["solid", "linear", "radial"] { + let mut s = effectcraft_engine::Session::default(); + s.execute("comp.new", json!({"width": 400, "height": 300, "duration": 2})).unwrap(); + s.execute("layer.newShape", json!({"kind": "rect"})).unwrap(); + s.execute("shape.fillStroke", json!({"fill": false, "strokeType": kind, "strokeWidth": 9})).unwrap(); + let mut app = EffectcraftApp::new(s); + let ctx = egui::Context::default(); + crate::theme::install(&ctx, &app.tokens, crate::i18n::language(&app)); + frame(&mut app, &ctx, 0.0, vec![]); + frame(&mut app, &ctx, 0.1, vec![]); + assert_eq!(app.session.state.shape_tool.stroke_width, 9.0, "selected {kind} stroke"); + let [x, y, w, h] = app.auto.find("tools.Rectangle").unwrap().rect; + let at = egui::pos2(x + w / 2.0, y + h / 2.0); + for (time, pressed) in [(1.0, true), (1.05, false), (1.1, true), (1.15, false)] { + frame( + &mut app, + &ctx, + time, + vec![ + egui::Event::PointerMoved(at), + egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }, + ], + ); + } + let comp = app.session.active_comp().unwrap(); + assert_eq!(comp.layers.len(), 2); + let layer = comp.layer(app.session.state.selected_layers[0]).unwrap(); + let (_, _, width) = effectcraft_engine::commands::shape_stroke::first_paint(layer, effectcraft_engine::time::Tick::ZERO); + assert_eq!(width, Some(9.0), "new {kind} stroke"); + let (_, stroke_kind) = effectcraft_engine::commands::shape_stroke::first_paint_kinds(layer, effectcraft_engine::time::Tick::ZERO); + assert_eq!(stroke_kind.id(), kind); + } + } } diff --git a/crates/ui-egui/src/menus/entries.rs b/crates/ui-egui/src/menus/entries.rs index 8fee44a72..e9a99522e 100644 --- a/crates/ui-egui/src/menus/entries.rs +++ b/crates/ui-egui/src/menus/entries.rs @@ -124,6 +124,9 @@ pub(crate) fn entry_enabled(app: &EffectcraftApp, e: &MenuEntry) -> bool { if e.command == "edit.clear" && app.ui.focused == PanelKind::RenderQueue { return !app.session.is_rendering() && !app.session.project.render_queue.is_empty(); } + if e.command == "file.close" && super::invoke::no_params(&e.params) && app.ui.focused == PanelKind::RenderQueue { + return true; + } if e.command == "window.panel" { return true; } diff --git a/crates/ui-egui/src/menus/invoke.rs b/crates/ui-egui/src/menus/invoke.rs index f1958fb5e..fbd4fa1fe 100644 --- a/crates/ui-egui/src/menus/invoke.rs +++ b/crates/ui-egui/src/menus/invoke.rs @@ -159,7 +159,7 @@ fn vertex_delta(app: &EffectcraftApp, layer: effectcraft_engine::project::LayerI }) } -fn no_params(p: &Value) -> bool { +pub(super) fn no_params(p: &Value) -> bool { p.as_object().is_none_or(|m| m.is_empty()) } @@ -216,12 +216,13 @@ pub fn invoke(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: V } // Close follows panel focus too: with the Render Queue active, Cmd+W closes that panel and // leaves the active composition open (it used to close the composition next to it). - if id == "file.close" && params.as_object().is_some_and(|p| p.is_empty()) && app.ui.focused == PanelKind::RenderQueue { + if id == "file.close" && no_params(¶ms) && app.ui.focused == PanelKind::RenderQueue { app.close_panel(PanelKind::RenderQueue); // The focus goes to a panel that is still there, so the next Close acts on the composition // instead of on the queue that is gone. - app.ui.focused = + let next = [PanelKind::Timeline, PanelKind::Composition, PanelKind::Project].into_iter().find(|p| app.ui.dock.is_visible(*p)).unwrap_or(PanelKind::Project); + app.show_panel(next); return Ok(Value::Null); } // Edit ▸ Paste with SVG markup on the system clipboard pastes it as a shape layer (After diff --git a/crates/ui-egui/src/menus/tests.rs b/crates/ui-egui/src/menus/tests.rs index 5af50f0fc..a043f67d1 100644 --- a/crates/ui-egui/src/menus/tests.rs +++ b/crates/ui-egui/src/menus/tests.rs @@ -437,6 +437,42 @@ fn cmd_w_in_the_render_queue_closes_the_queue_not_the_comp() { assert_eq!(app.session.state.open_comps.len(), 0, "a second Close acts on the composition"); } +#[test] +fn file_close_is_available_for_an_empty_render_queue_without_a_comp() { + for params in [Value::Null, json!({})] { + let ctx = egui::Context::default(); + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + assert!(app.session.active_comp().is_none()); + app.show_panel(PanelKind::RenderQueue); + assert!(menu_items(&app).iter().find(|item| item.id == "file.close").unwrap().enabled); + invoke(&mut app, &ctx, "file.close", params).unwrap(); + assert!(!app.ui.dock.is_open(PanelKind::RenderQueue)); + } +} + +#[test] +fn closing_the_render_queue_reveals_the_panel_receiving_focus() { + for maximized in [false, true] { + let ctx = egui::Context::default(); + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + app.close_panel(PanelKind::Timeline); + app.close_panel(PanelKind::Composition); + app.show_panel(PanelKind::EffectControls); + assert!(!app.ui.dock.is_visible(PanelKind::Project)); + app.show_panel(PanelKind::RenderQueue); + if maximized { + app.apply_dock(craft_ui::docking::Action::Float { panel: PanelKind::RenderQueue, rect: [50.0, 60.0, 400.0, 200.0] }).unwrap(); + app.toggle_maximize(PanelKind::EffectControls); + app.ui.focused = PanelKind::RenderQueue; + assert!(app.ui.dock.is_floating(PanelKind::RenderQueue)); + } + invoke(&mut app, &ctx, "file.close", json!({})).unwrap(); + assert_eq!(app.ui.focused, PanelKind::Project); + assert!(app.ui.dock.is_visible(app.ui.focused), "focus must belong to a front tab"); + assert_eq!(app.ui.maximized, None, "the focused docked panel must be on screen"); + } +} + #[test] fn menu_items_cover_the_tree() { assert_eq!(menus().last(), Some(&"Help")); From b3887c226f93813d16873c2f706ae91086c36adc Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Thu, 29 Oct 2026 21:58:14 +0100 Subject: [PATCH 17/23] Graph Editor: keep close focus and locked selections consistent (#1342) --- crates/engine/src/cmd/anim.rs | 15 +++++++-- crates/engine/src/tests/delete_keys.rs | 28 ++++++++++++++++ crates/ui-egui/src/dock_ui.rs | 2 +- crates/ui-egui/src/menus/entries.rs | 2 +- crates/ui-egui/src/menus/invoke.rs | 3 +- crates/ui-egui/src/menus/tests.rs | 44 ++++++++++++++++++++++++++ 6 files changed, 88 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/anim.rs b/crates/engine/src/cmd/anim.rs index 073262d0f..e2bbab62b 100644 --- a/crates/engine/src/cmd/anim.rs +++ b/crates/engine/src/cmd/anim.rs @@ -55,6 +55,10 @@ fn compatible(a: &Property, b: &effectcraft_keyframe::Value) -> bool { a.value.kind_name() == b.kind_name() && a.value.components().len() == b.components().len() } +fn locked_selection(comp: &effectcraft_project::Comp, selected: &[KeyRef]) -> Vec { + selected.iter().filter(|k| comp.layer(k.layer).is_some_and(|l| l.switches.locked)).copied().collect() +} + /// Paste copied keyframes at the current time into the target layers' properties. fn paste(s: &mut Session, p: &Value) -> Result { let clip = s.state.key_clipboard.clone(); @@ -117,7 +121,7 @@ fn paste(s: &mut Session, p: &Value) -> Result { } s.edit("Paste Keyframes", merge_p(p), |proj, st| { let comp = proj.comp_mut(cid).ok_or(EngineError::NoComp)?; - let mut sel: Vec = st.selected_keys.iter().filter(|k| comp.layer(k.layer).is_some_and(|l| l.switches.locked)).copied().collect(); + let mut sel = locked_selection(comp, &st.selected_keys); let retained = sel.len(); for (lid, uid, ci) in &plan { let Some(l) = comp.layer_mut(*lid) else { continue }; @@ -210,6 +214,9 @@ pub(super) fn paste_path(s: &mut Session, p: &Value, (cid, lid, uid): (ItemId, L }) .collect(); s.edit("Paste Keyframes", merge_p(p), |proj, st| { + let comp = proj.comp(cid).ok_or(EngineError::NoComp)?; + let mut selected = locked_selection(comp, &st.selected_keys); + let retained = selected.len(); let l = super::layer_mut(proj, cid, lid)?; let pr = l.props.find_mut(uid).ok_or_else(|| bad("keys.paste", "no property"))?; let mut sel = vec![]; @@ -224,8 +231,10 @@ pub(super) fn paste_path(s: &mut Session, p: &Value, (cid, lid, uid): (ItemId, L // Re-timing keeps the keys' order, so the pasted keys are found again by index. let pasted: Vec = pr.keys.iter().enumerate().filter(|(_, k)| sel.contains(&k.time)).map(|(i, _)| i).collect(); retime_roving(&mut pr.keys, pr.spatial); - st.selected_keys = pasted.iter().filter_map(|&i| pr.keys.get(i)).map(|k| KeyRef { layer: lid, prop: uid, time: k.time }).collect(); - Ok(json!(st.selected_keys.len())) + selected.extend(pasted.iter().filter_map(|&i| pr.keys.get(i)).map(|k| KeyRef { layer: lid, prop: uid, time: k.time })); + let count = selected.len().saturating_sub(retained); + st.selected_keys = selected; + Ok(json!(count)) }) } diff --git a/crates/engine/src/tests/delete_keys.rs b/crates/engine/src/tests/delete_keys.rs index a6ca08bed..ec6ec888e 100644 --- a/crates/engine/src/tests/delete_keys.rs +++ b/crates/engine/src/tests/delete_keys.rs @@ -181,6 +181,34 @@ fn key_paste_skips_locked_targets_without_changing_layer_pairing() { assert_eq!(s.project.to_json(), pasted); } +#[test] +fn motion_path_paste_preserves_locked_source_selection() { + for explicit in [false, true] { + let (mut s, source, _) = setup(); + s.execute("mask.new", json!({"layer": source, "vertices": [[0, 0], [20, 10], [40, 0]], "closed": false})).unwrap(); + s.execute("prop.toggleAnimation", json!({"layer": source, "path": "masks/#1/path"})).unwrap(); + let uid = s.active_comp().unwrap().layer(LayerId(source)).unwrap().props.prop("masks/#1/path").unwrap().uid; + s.execute("keys.select", json!({"keys": [{"layer": source, "prop": uid, "time": 0}]})).unwrap(); + s.execute("keys.copy", json!({})).unwrap(); + let locked_keys = s.state.selected_keys.clone(); + let target = s.execute("layer.newSolid", json!({"name": "Target", "color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("prop.select", json!({"layer": target, "path": "transform/position"})).unwrap(); + s.execute("layer.setSwitch", json!({"layers": [source], "switch": "locked", "value": true})).unwrap(); + s.state.selected_keys = locked_keys.clone(); + let params = if explicit { json!({"layer": target, "path": "transform/position"}) } else { json!({}) }; + let before = s.project.to_json(); + assert_eq!(s.execute("keys.paste", params).unwrap(), json!(3)); + assert!(locked_keys.iter().all(|k| s.state.selected_keys.contains(k))); + assert_eq!(s.state.selected_keys.len(), 4); + assert_eq!(s.state.selected_keys.iter().filter(|k| k.layer == LayerId(target)).count(), 3); + let pasted = s.project.to_json(); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), pasted); + } +} + #[test] fn key_transform_preserves_locked_keys_in_a_mixed_selection() { let (mut s, locked, locked_prop) = setup(); diff --git a/crates/ui-egui/src/dock_ui.rs b/crates/ui-egui/src/dock_ui.rs index 30710392a..750764b84 100644 --- a/crates/ui-egui/src/dock_ui.rs +++ b/crates/ui-egui/src/dock_ui.rs @@ -398,7 +398,7 @@ impl EffectcraftApp { /// A press in a panel (its body, tab or group header) focuses it; another Composition viewer /// also becomes the active one. - fn focus_panel(&mut self, ctx: &egui::Context, p: PanelKind) { + pub(crate) fn focus_panel(&mut self, ctx: &egui::Context, p: PanelKind) { self.ui.focused = p; panels::viewer_text::panel_clicked(self, ctx, p); if let Some(id) = panels::viewers::id_of(p) { diff --git a/crates/ui-egui/src/menus/entries.rs b/crates/ui-egui/src/menus/entries.rs index e9a99522e..cb3a0042b 100644 --- a/crates/ui-egui/src/menus/entries.rs +++ b/crates/ui-egui/src/menus/entries.rs @@ -125,7 +125,7 @@ pub(crate) fn entry_enabled(app: &EffectcraftApp, e: &MenuEntry) -> bool { return !app.session.is_rendering() && !app.session.project.render_queue.is_empty(); } if e.command == "file.close" && super::invoke::no_params(&e.params) && app.ui.focused == PanelKind::RenderQueue { - return true; + return !app.ui.dock.locked; } if e.command == "window.panel" { return true; diff --git a/crates/ui-egui/src/menus/invoke.rs b/crates/ui-egui/src/menus/invoke.rs index fbd4fa1fe..b3a29d66a 100644 --- a/crates/ui-egui/src/menus/invoke.rs +++ b/crates/ui-egui/src/menus/invoke.rs @@ -217,12 +217,13 @@ pub fn invoke(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: V // Close follows panel focus too: with the Render Queue active, Cmd+W closes that panel and // leaves the active composition open (it used to close the composition next to it). if id == "file.close" && no_params(¶ms) && app.ui.focused == PanelKind::RenderQueue { - app.close_panel(PanelKind::RenderQueue); + app.apply_dock(craft_ui::docking::Action::Close { panel: PanelKind::RenderQueue })?; // The focus goes to a panel that is still there, so the next Close acts on the composition // instead of on the queue that is gone. let next = [PanelKind::Timeline, PanelKind::Composition, PanelKind::Project].into_iter().find(|p| app.ui.dock.is_visible(*p)).unwrap_or(PanelKind::Project); app.show_panel(next); + app.focus_panel(ctx, next); return Ok(Value::Null); } // Edit ▸ Paste with SVG markup on the system clipboard pastes it as a shape layer (After diff --git a/crates/ui-egui/src/menus/tests.rs b/crates/ui-egui/src/menus/tests.rs index a043f67d1..83ba78dc1 100644 --- a/crates/ui-egui/src/menus/tests.rs +++ b/crates/ui-egui/src/menus/tests.rs @@ -473,6 +473,50 @@ fn closing_the_render_queue_reveals_the_panel_receiving_focus() { } } +#[test] +fn queue_close_preserves_state_when_workspace_is_locked() { + let ctx = egui::Context::default(); + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + app.session.execute("comp.new", json!({"name": "A", "duration": 2})).unwrap(); + app.show_panel(PanelKind::RenderQueue); + app.ui.dock.locked = true; + let workspace = app.ui.dock.clone(); + let comp = app.session.active_comp_id(); + for _ in 0..2 { + assert!(invoke(&mut app, &ctx, "file.close", Value::Null).is_err()); + assert_eq!(app.ui.dock, workspace); + assert_eq!(app.ui.focused, PanelKind::RenderQueue); + assert_eq!(app.session.active_comp_id(), comp); + } + assert!(!menu_items(&app).iter().find(|item| item.id == "file.close").unwrap().enabled); + app.ui.dock.locked = false; + invoke(&mut app, &ctx, "file.close", Value::Null).unwrap(); + assert!(!app.ui.dock.is_open(PanelKind::RenderQueue)); +} + +#[test] +fn queue_close_preserves_viewer_command_target() { + let ctx = egui::Context::default(); + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + let a = app.session.execute("comp.new", json!({"name": "A", "duration": 2})).unwrap()["comp"].as_u64().unwrap(); + let b = app.session.execute("comp.new", json!({"name": "B", "duration": 2})).unwrap()["comp"].as_u64().unwrap(); + app.show_panel(PanelKind::Viewer(1)); + app.ui.viewers.insert(0, Some(a)); + app.ui.viewers.insert(1, Some(b)); + app.ui.active_viewer = 1; + app.close_panel(PanelKind::Timeline); + app.show_panel(PanelKind::Composition); + app.show_panel(PanelKind::RenderQueue); + assert_eq!(app.session.active_comp_id().map(|id| id.0), Some(b)); + invoke(&mut app, &ctx, "file.close", json!({})).unwrap(); + assert_eq!(app.ui.focused, PanelKind::Composition); + assert_eq!(app.ui.active_viewer, 0); + assert_eq!(app.session.active_comp_id().map(|id| id.0), Some(a)); + invoke(&mut app, &ctx, "file.close", json!({})).unwrap(); + assert!(!app.session.state.open_comps.iter().any(|id| id.0 == a)); + assert!(app.session.state.open_comps.iter().any(|id| id.0 == b)); +} + #[test] fn menu_items_cover_the_tree() { assert_eq!(menus().last(), Some(&"Help")); From f93d336f9d7bc0dbd61822a4651c7dfd903f3b7c Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Thu, 29 Oct 2026 22:38:53 +0100 Subject: [PATCH 18/23] Graph Editor: protect selection tools and restore queue-close focus --- crates/engine/src/cmd/anim.rs | 2 +- crates/engine/src/cmd/anim_tools.rs | 5 +- crates/engine/src/cmd/animation.rs | 3 +- crates/engine/src/cmd/keys_more.rs | 2 + crates/engine/src/cmd/mask_interp.rs | 12 +++-- crates/engine/src/cmd/prop.rs | 9 ++++ crates/engine/src/tests/anim_tools.rs | 74 +++++++++++++++++++++++++++ crates/engine/src/tests/mask_warp.rs | 45 ++++++++++++++++ crates/ui-egui/src/app/frame.rs | 2 +- crates/ui-egui/src/menus/invoke.rs | 4 ++ crates/ui-egui/src/menus/tests.rs | 46 +++++++++++++++++ 11 files changed, 194 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/cmd/anim.rs b/crates/engine/src/cmd/anim.rs index e2bbab62b..1064ba021 100644 --- a/crates/engine/src/cmd/anim.rs +++ b/crates/engine/src/cmd/anim.rs @@ -55,7 +55,7 @@ fn compatible(a: &Property, b: &effectcraft_keyframe::Value) -> bool { a.value.kind_name() == b.kind_name() && a.value.components().len() == b.components().len() } -fn locked_selection(comp: &effectcraft_project::Comp, selected: &[KeyRef]) -> Vec { +pub(crate) fn locked_selection(comp: &effectcraft_project::Comp, selected: &[KeyRef]) -> Vec { selected.iter().filter(|k| comp.layer(k.layer).is_some_and(|l| l.switches.locked)).copied().collect() } diff --git a/crates/engine/src/cmd/anim_tools.rs b/crates/engine/src/cmd/anim_tools.rs index 06e0d341b..33ee39ae2 100644 --- a/crates/engine/src/cmd/anim_tools.rs +++ b/crates/engine/src/cmd/anim_tools.rs @@ -213,7 +213,8 @@ fn sample_curve( fn grouped_selection(s: &Session) -> std::collections::BTreeMap<(LayerId, Uid), Vec> { let mut g: std::collections::BTreeMap<(LayerId, Uid), Vec> = Default::default(); - for k in &s.state.selected_keys { + let Some(comp) = s.active_comp() else { return g }; + for k in super::prop::editable_keys(comp, &s.state.selected_keys) { g.entry((k.layer, k.prop)).or_default().push(k.time); } for v in g.values_mut() { @@ -273,7 +274,7 @@ fn run_on_selection( s.edit(label, None, |proj, st| { let comp = proj.comp_mut(cid).ok_or(EngineError::NoComp)?; let fr = comp.frame_rate; - let mut sel = vec![]; + let mut sel = super::anim::locked_selection(comp, &st.selected_keys); for ((lid, uid), times) in groups { let Some(l) = comp.layer_mut(lid) else { continue }; let lc = l.clone(); diff --git a/crates/engine/src/cmd/animation.rs b/crates/engine/src/cmd/animation.rs index 1bdf3fed7..c781e9da1 100644 --- a/crates/engine/src/cmd/animation.rs +++ b/crates/engine/src/cmd/animation.rs @@ -209,7 +209,8 @@ fn add_keyframe(s: &mut Session, _: &Value) -> Result { /// Selected keys grouped by (layer, property). fn keys_by_prop(s: &Session) -> BTreeMap<(LayerId, Uid), Vec> { let mut m: BTreeMap<(LayerId, Uid), Vec> = BTreeMap::new(); - for k in &s.state.selected_keys { + let Some(comp) = s.active_comp() else { return m }; + for k in super::prop::editable_keys(comp, &s.state.selected_keys) { m.entry((k.layer, k.prop)).or_default().push(k.time); } m diff --git a/crates/engine/src/cmd/keys_more.rs b/crates/engine/src/cmd/keys_more.rs index c7785d08c..4edef885a 100644 --- a/crates/engine/src/cmd/keys_more.rs +++ b/crates/engine/src/cmd/keys_more.rs @@ -37,6 +37,8 @@ pub(crate) fn label_keys(s: &mut Session, p: &Value) -> Result { return Err(bad("keys.label", "select keyframes first")); } let cid = comp_id(s, p)?; + let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; + let keys = super::prop::editable_keys(comp, &keys); let n = s.edit("Keyframe Label", None, |proj, _| { let mut n = 0; for k in &keys { diff --git a/crates/engine/src/cmd/mask_interp.rs b/crates/engine/src/cmd/mask_interp.rs index 982468c8e..bbcc9f99d 100644 --- a/crates/engine/src/cmd/mask_interp.rs +++ b/crates/engine/src/cmd/mask_interp.rs @@ -437,20 +437,22 @@ fn interpolate(s: &mut Session, p: &Value) -> Result { // The layer: given, else the layer of selected Mask Path keys, else the selection. let cid = super::comp_id(s, p)?; let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; + let selected = super::prop::editable_keys(comp, &s.state.selected_keys); let lid = match p.get("layer") { Some(v) => super::resolve_layer(comp, v).ok_or_else(|| bad(CMD, format!("no layer {v}")))?, - None => match s.state.selected_keys.iter().find(|k| comp.layer(k.layer).is_some_and(|l| l.masks().is_some_and(|m| m.find(k.prop).is_some()))) { + None => match selected.iter().find(|k| comp.layer(k.layer).is_some_and(|l| l.masks().is_some_and(|m| m.find(k.prop).is_some()))) { Some(k) => k.layer, None => layer_p(s, p, CMD)?.1, }, }; let layer = comp.layer(lid).ok_or(EngineError::NoComp)?; + if !super::prop::key_layer_editable(comp, lid) { + return Err(bad(CMD, "the layer is locked")); + } let masks = layer.masks().ok_or_else(|| bad(CMD, "the layer has no masks"))?; let uid = match p.get("mask") { Some(_) => mask_uid(s, layer, p), - None => s - .state - .selected_keys + None => selected .iter() .filter(|k| k.layer == lid) .find_map(|k| masks.groups().find(|g| g.get("path").is_some_and(|pr| pr.uid == k.prop)).map(|g| g.uid)) @@ -465,7 +467,7 @@ fn interpolate(s: &mut Session, p: &Value) -> Result { let ts: Vec = a.iter().filter_map(Value::as_f64).map(|t| layer.layer_time(comp.frame_rate.snap_nearest(Tick::from_seconds_f64(t)))).collect(); ts.into_iter().filter_map(|t| path_prop.keys.iter().find(|k| (k.time.0 - t.0).abs() <= comp.frame_duration().0 / 2).map(|k| k.time)).collect() } - None => s.state.selected_keys.iter().filter(|k| k.layer == lid && k.prop == path_prop.uid).map(|k| k.time).collect(), + None => selected.iter().filter(|k| k.layer == lid && k.prop == path_prop.uid).map(|k| k.time).collect(), }; times.sort(); times.dedup(); diff --git a/crates/engine/src/cmd/prop.rs b/crates/engine/src/cmd/prop.rs index 0eff1dd09..b08021b66 100644 --- a/crates/engine/src/cmd/prop.rs +++ b/crates/engine/src/cmd/prop.rs @@ -883,6 +883,15 @@ fn select_keys(s: &mut Session, p: &Value) -> Result { Ok(out) } +/// References that can be edited, including explicit key lists supplied by commands. +pub(crate) fn editable_keys(comp: &effectcraft_project::Comp, selected: &[KeyRef]) -> Vec { + selected.iter().filter(|key| key_layer_editable(comp, key.layer)).copied().collect() +} + +pub(crate) fn key_layer_editable(comp: &effectcraft_project::Comp, layer: LayerId) -> bool { + comp.layer(layer).is_some_and(|layer| !layer.switches.locked) +} + /// Apply `f` to every selected key; keeps the selection pointing at moved keys. pub(crate) fn edit_keys(s: &mut Session, label: &str, merge: Option<&str>, f: impl Fn(&mut Vec, usize, &mut Tick) -> bool) -> Result { let cid = s.active_comp_id().ok_or(EngineError::NoComp)?; diff --git a/crates/engine/src/tests/anim_tools.rs b/crates/engine/src/tests/anim_tools.rs index 06d194001..96b861972 100644 --- a/crates/engine/src/tests/anim_tools.rs +++ b/crates/engine/src/tests/anim_tools.rs @@ -9,6 +9,80 @@ fn select(s: &mut crate::Session, layer: u64, path: &str) { s.execute("prop.select", json!({"layer": layer, "path": path})).unwrap(); } +fn selection_command_skips_locked_keys(command: &str, path: &str, values: [serde_json::Value; 3], mut params: serde_json::Value) { + let (mut s, locked) = setup(); + let unlocked = s.execute("layer.newSolid", json!({"name": "Editable", "width": 100, "height": 100})).unwrap()["layer"].as_u64().unwrap(); + for layer in [locked, unlocked] { + animate(&mut s, layer, path, &[(0.0, values[0].clone()), (0.5, values[1].clone()), (1.0, values[2].clone())]); + } + let before = prop(&s, locked, path).keys; + let selected: Vec<_> = [locked, unlocked] + .into_iter() + .flat_map(|layer| { + let pr = prop(&s, layer, path); + pr.keys.into_iter().map(move |key| crate::KeyRef { layer: effectcraft_project::LayerId(layer), prop: pr.uid, time: key.time }) + }) + .collect(); + s.execute("layer.setSwitch", json!({"layers": [locked], "switch": "locked", "value": true})).unwrap(); + s.state.selected_keys = selected.clone(); + if params.get("keys").is_some() { + params["keys"] = serde_json::to_value(&selected).unwrap(); + } + s.execute_checked(command, params.clone()).unwrap(); + assert_eq!(prop(&s, locked, path).keys, before, "{command} changed a locked layer"); + let after = prop(&s, unlocked, path).keys; + assert_ne!(after, before, "{command} must still edit unlocked keys"); + let after_selection = s.state.selected_keys.clone(); + assert!(selected.iter().filter(|k| k.layer.0 == locked).all(|key| after_selection.contains(key))); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(prop(&s, unlocked, path).keys, before); + assert!(selected.iter().filter(|key| key.layer.0 == locked).all(|key| s.state.selected_keys.contains(key))); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(prop(&s, unlocked, path).keys, after); + assert_eq!(s.state.selected_keys, after_selection); + // All-locked input must preserve the selection and document as well. + s.state.selected_keys.retain(|k| k.layer.0 == locked); + if params.get("keys").is_some() { + params["keys"] = serde_json::to_value(&s.state.selected_keys).unwrap(); + } + let locked_selection = s.state.selected_keys.clone(); + let project = serde_json::to_value(&*s.project).unwrap(); + let _ = s.execute_checked(command, params); + assert_eq!(serde_json::to_value(&*s.project).unwrap(), project); + assert_eq!(s.state.selected_keys, locked_selection); +} + +#[test] +fn wiggler_preserves_locked_keys_in_mixed_selection() { + selection_command_skips_locked_keys( + "keys.wiggle", + "transform/opacity", + [json!(10), json!(90), json!(20)], + json!({"apply": "temporal", "frequency": 8, "magnitude": 10}), + ); +} + +#[test] +fn smoother_preserves_locked_keys_in_mixed_selection() { + selection_command_skips_locked_keys("keys.smooth", "transform/opacity", [json!(10), json!(50), json!(90)], json!({"tolerance": 1})); +} + +#[test] +fn exponential_scale_preserves_locked_keys_in_mixed_selection() { + selection_command_skips_locked_keys( + "keys.exponentialScale", + "transform/scale", + [json!([100, 100, 100]), json!([150, 150, 150]), json!([400, 400, 400])], + json!({}), + ); +} + +#[test] +fn edit_label_preserves_locked_keys_in_mixed_selection() { + selection_command_skips_locked_keys("edit.label", "transform/opacity", [json!(10), json!(50), json!(90)], json!({"label": "Red"})); + selection_command_skips_locked_keys("edit.label", "transform/opacity", [json!(10), json!(50), json!(90)], json!({"label": "Red", "keys": true})); +} + #[test] fn wiggle_adds_keys_at_frequency_within_magnitude() { let (mut s, l) = setup(); diff --git a/crates/engine/src/tests/mask_warp.rs b/crates/engine/src/tests/mask_warp.rs index adcea44be..5387fe562 100644 --- a/crates/engine/src/tests/mask_warp.rs +++ b/crates/engine/src/tests/mask_warp.rs @@ -184,6 +184,51 @@ fn mask_interpolation_creates_matching_keys() { assert!(s.execute_checked("mask.interpolate", json!({"matchingMethod": "magic", "times": [0, 0.8], "layer": clip.0})).is_err()); } +#[test] +fn mask_interpolation_skips_locked_selection_and_rejects_explicit_locked_targets() { + let (mut s, locked, _) = setup(patch_frames(rotating_pose)); + let unlocked = effectcraft_project::LayerId(s.execute("layer.newSolid", json!({"width": 100, "height": 100})).unwrap()["layer"].as_u64().unwrap()); + let a = ShapePath::rect([20.0, 20.0], 10.0, 10.0); + let b = ShapePath::rect([40.0, 30.0], 20.0, 20.0); + let cid = s.active_comp_id().unwrap(); + let mut selected = vec![]; + let mut masks = vec![]; + for layer in [locked, unlocked] { + let uid = new_mask(&mut s, layer, &a); + masks.push(uid); + s.edit("keys", None, |project, _| { + let pr = project.comp_mut(cid).unwrap().layer_mut(layer).unwrap().props.find_group_mut(uid).unwrap().get_mut("path").unwrap(); + pr.keys = vec![Keyframe::new(frame_time(0), KV::Path(a.clone())), Keyframe::new(frame_time(20), KV::Path(b.clone()))]; + selected.extend(pr.keys.iter().map(|key| crate::KeyRef { layer, prop: pr.uid, time: key.time })); + Ok(()) + }) + .unwrap(); + } + s.execute("layer.setSwitch", json!({"layers": [locked.0], "switch": "locked", "value": true})).unwrap(); + s.state.selected_keys = selected.clone(); + let before = mask_path(&s, locked, masks[0]).keys; + let options = serde_json::to_value(&s.state.mask_interp).unwrap(); + let project = serde_json::to_value(&*s.project).unwrap(); + let error = s.execute_checked("mask.interpolate", json!({"layer": locked.0, "mask": masks[0], "times": [0.0, 0.8], "quality": 75})).unwrap_err(); + assert!(error.to_string().contains("locked"), "{error}"); + assert_eq!(serde_json::to_value(&*s.project).unwrap(), project); + assert_eq!(serde_json::to_value(&s.state.mask_interp).unwrap(), options); + assert_eq!(s.state.selected_keys, selected); + s.execute_checked("mask.interpolate", json!({})).unwrap(); + assert_eq!(mask_path(&s, locked, masks[0]).keys, before); + assert_eq!(mask_path(&s, unlocked, masks[1]).keys.len(), 21); + assert_eq!(s.state.selected_keys, selected); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(mask_path(&s, unlocked, masks[1]).keys.len(), 2); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(mask_path(&s, unlocked, masks[1]).keys.len(), 21); + s.state.selected_keys.retain(|key| key.layer == locked); + let selection = s.state.selected_keys.clone(); + assert!(s.execute_checked("mask.interpolate", json!({})).is_err()); + assert_eq!(mask_path(&s, locked, masks[0]).keys, before); + assert_eq!(s.state.selected_keys, selection); +} + // ---------------------------------------------------------------- Warp Stabilizer /// Camera offset on frame `f`: a slow pan plus jitter. diff --git a/crates/ui-egui/src/app/frame.rs b/crates/ui-egui/src/app/frame.rs index d41e0a9d0..a76e7dce1 100644 --- a/crates/ui-egui/src/app/frame.rs +++ b/crates/ui-egui/src/app/frame.rs @@ -90,7 +90,7 @@ impl EffectcraftApp { /// Apply the engine events drained this tick: open a comp, show a toast, open a URL, run a frontend command, /// purge frame caches or copy an image; command and clipboard failures go to `app.ui.status`. - pub(super) fn handle_events(&mut self, ctx: &egui::Context) { + pub(crate) fn handle_events(&mut self, ctx: &egui::Context) { for ev in self.session.drain_events() { match ev { effectcraft_engine::Event::OpenComp(c) => { diff --git a/crates/ui-egui/src/menus/invoke.rs b/crates/ui-egui/src/menus/invoke.rs index b3a29d66a..446513f05 100644 --- a/crates/ui-egui/src/menus/invoke.rs +++ b/crates/ui-egui/src/menus/invoke.rs @@ -224,6 +224,10 @@ pub fn invoke(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: V [PanelKind::Timeline, PanelKind::Composition, PanelKind::Project].into_iter().find(|p| app.ui.dock.is_visible(*p)).unwrap_or(PanelKind::Project); app.show_panel(next); app.focus_panel(ctx, next); + // Activating a passive viewer emits OpenComp, which raises Timeline. Finish that routing + // before restoring the destination panel, including when they share the same tab group. + app.handle_events(ctx); + app.show_panel(next); return Ok(Value::Null); } // Edit ▸ Paste with SVG markup on the system clipboard pastes it as a shape layer (After diff --git a/crates/ui-egui/src/menus/tests.rs b/crates/ui-egui/src/menus/tests.rs index 83ba78dc1..63fac9879 100644 --- a/crates/ui-egui/src/menus/tests.rs +++ b/crates/ui-egui/src/menus/tests.rs @@ -504,6 +504,12 @@ fn queue_close_preserves_viewer_command_target() { app.ui.viewers.insert(0, Some(a)); app.ui.viewers.insert(1, Some(b)); app.ui.active_viewer = 1; + app.apply_dock(craft_ui::docking::Action::Move { + panel: PanelKind::Timeline, + anchor: PanelKind::Composition, + placement: craft_ui::docking::Placement::Tab { before: None }, + }) + .unwrap(); app.close_panel(PanelKind::Timeline); app.show_panel(PanelKind::Composition); app.show_panel(PanelKind::RenderQueue); @@ -517,6 +523,46 @@ fn queue_close_preserves_viewer_command_target() { assert!(app.session.state.open_comps.iter().any(|id| id.0 == b)); } +#[test] +fn queue_close_keeps_passive_viewer_focus_across_frames() { + let mut session = effectcraft_engine::Session::default(); + let a = session.execute("comp.new", json!({"name": "A", "width": 32, "height": 32, "duration": 2})).unwrap()["comp"].as_u64().unwrap(); + let b = session.execute("comp.new", json!({"name": "B", "width": 32, "height": 32, "duration": 2})).unwrap()["comp"].as_u64().unwrap(); + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(|_| EffectcraftApp::new(session)); + h.run_steps(3); + let ctx = h.ctx.clone(); + let app = h.state_mut(); + app.show_panel(PanelKind::Viewer(1)); + app.ui.viewers.insert(0, Some(a)); + app.ui.viewers.insert(1, Some(b)); + app.ui.active_viewer = 1; + app.apply_dock(craft_ui::docking::Action::Move { + panel: PanelKind::Timeline, + anchor: PanelKind::Composition, + placement: craft_ui::docking::Placement::Tab { before: None }, + }) + .unwrap(); + app.close_panel(PanelKind::Timeline); + app.show_panel(PanelKind::Composition); + app.show_panel(PanelKind::RenderQueue); + invoke(app, &ctx, "file.close", Value::Null).unwrap(); + h.run_steps(4); + let app = h.state_mut(); + assert_eq!(app.ui.focused, PanelKind::Composition); + assert_eq!(app.ui.active_viewer, 0); + assert_eq!(app.session.active_comp_id().map(|id| id.0), Some(a)); + assert!(app.ui.dock.is_visible(PanelKind::Composition)); + assert!(!app.ui.dock.is_open(PanelKind::RenderQueue)); + invoke(app, &ctx, "file.close", json!({})).unwrap(); + assert!(!app.session.state.open_comps.iter().any(|id| id.0 == a)); + assert!(app.session.state.open_comps.iter().any(|id| id.0 == b)); + // A later explicit Open still routes normally, rather than inheriting a deferred focus override. + invoke(app, &ctx, "comp.open", json!({"comp": b})).unwrap(); + h.run_steps(3); + assert!(h.state().ui.dock.is_visible(PanelKind::Timeline)); + assert_eq!(h.state().session.active_comp_id().map(|id| id.0), Some(b)); +} + #[test] fn menu_items_cover_the_tree() { assert_eq!(menus().last(), Some(&"Help")); From c51b4618604fde69fb46fdfa358615c35f95b6ca Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Fri, 30 Oct 2026 03:52:38 +0100 Subject: [PATCH 19/23] #1284: drop superseded shape tool double-click changes --- crates/engine/src/cmd/shape_tool.rs | 6 +- crates/engine/src/tests/session.rs | 12 --- crates/ui-egui/src/header.rs | 122 ---------------------------- crates/ui-egui/src/panels/viewer.rs | 2 +- 4 files changed, 3 insertions(+), 139 deletions(-) diff --git a/crates/engine/src/cmd/shape_tool.rs b/crates/engine/src/cmd/shape_tool.rs index 3db347e60..e1f3607b1 100644 --- a/crates/engine/src/cmd/shape_tool.rs +++ b/crates/engine/src/cmd/shape_tool.rs @@ -318,9 +318,7 @@ fn new_shape(s: &mut Session, p: &Value) -> Result { }; let pos = position_p(p, c)?.unwrap_or([comp.width as f64 / 2.0, comp.height as f64 / 2.0]); let paint = paint_p(p, &s.state.shape_tool, c)?; - // `newLayer` makes a layer of its own even with a shape layer selected (a shape that fills the - // comp for a background or matte, whatever is selected). - let target = if b_p(p, "newLayer") == Some(true) { None } else { draw_target(s, &comp, p, c)? }; + let target = draw_target(s, &comp, p, c)?; // In the target layer's space: the centre maps through the layer's transform, the size by // its scale. let (centre, size) = match target.and_then(|l| comp.layer(l)) { @@ -376,7 +374,7 @@ pub fn specs() -> Vec { "Shape Tool", [], None, - "{layer?, newLayer? (a new shape layer even with a shape layer selected), kind?: rect|rounded|ellipse|polygon|star, size? [w,h], position? [x,y] (the centre), space?: comp|layer, name? (a new layer's), fill?, fillType?, fillBlend?, fillOpacity?, stroke?, strokeType?, strokeBlend?, strokeOpacity?, strokeWidth? (default the Tools bar's, shape.toolOptions)} → {layer, group}", + "{layer?, kind?: rect|rounded|ellipse|polygon|star, size? [w,h], position? [x,y] (the centre), space?: comp|layer, name? (a new layer's), fill?, fillType?, fillBlend?, fillOpacity?, stroke?, strokeType?, strokeBlend?, strokeOpacity?, strokeWidth? (default the Tools bar's, shape.toolOptions)} → {layer, group}", has_comp, new_shape ), diff --git a/crates/engine/src/tests/session.rs b/crates/engine/src/tests/session.rs index e0f3e9cd8..b5d1cbb5f 100644 --- a/crates/engine/src/tests/session.rs +++ b/crates/engine/src/tests/session.rs @@ -246,18 +246,6 @@ fn text_shape_mask_matte_parent() { assert_eq!(info["layers"].as_array().unwrap().len(), 2); } -/// A shape layer the size of the comp and placed at its default position covers the frame: the -/// Shape Tool's double-click (#1284) makes one this way, for backgrounds, mattes and overlays. -#[test] -fn a_comp_sized_shape_layer_sits_on_the_comp_centre() { - let mut s = Session::default(); - s.execute("comp.new", json!({"width": 400, "height": 300, "duration": 2})).unwrap(); - let sh = s.execute("layer.newShape", json!({"kind": "rect", "size": [400, 300], "fill": "#ff0000"})).unwrap()["layer"].as_u64().unwrap(); - let l = s.active_comp().unwrap().layer(effectcraft_project::LayerId(sh)).unwrap().clone(); - let pos = l.props.prop("transform/position").unwrap().value.clone(); - assert!(matches!(pos, effectcraft_keyframe::Value::Vec3(p) if (p[0] - 200.0).abs() < 1e-9 && (p[1] - 150.0).abs() < 1e-9), "{pos:?}"); -} - /// The toolbar's Fill / Stroke change a selected shape layer's paint (#205): a shape drawn /// without a stroke gets one, later edits change it, and the Contents "Add:" items (#206) include /// an empty Path and a Gradient Stroke that render. diff --git a/crates/ui-egui/src/header.rs b/crates/ui-egui/src/header.rs index 2c14e188b..3eee89453 100644 --- a/crates/ui-egui/src/header.rs +++ b/crates/ui-egui/src/header.rs @@ -57,22 +57,6 @@ fn brand_and_home(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter x + 6.0 } -/// The `shape.newShape` parameters for a shape that fills a `w` × `h` composition with `tool`: a new -/// layer, painted with the Tools bar's Fill and Stroke. A polygon or star is round, so it takes the -/// shorter side; the other kinds take the whole frame. -pub(crate) fn comp_sized_shape_params(tool: Tool, w: u32, h: u32) -> serde_json::Value { - let kind = crate::panels::viewer::shape_kind(tool); - let (w, h) = (f64::from(w), f64::from(h)); - let size = if matches!(kind, "polygon" | "star") { [w.min(h); 2] } else { [w, h] }; - json!({"kind": kind, "size": size, "position": [w / 2.0, h / 2.0], "newLayer": true}) -} - -/// Make the shape layer that fills the composition for a double-clicked shape `tool`. -fn comp_sized_shape(app: &mut EffectcraftApp, ctx: &egui::Context, tool: Tool) -> Result { - let (w, h) = app.session.active_comp().map(|c| (c.width, c.height)).ok_or("no composition is open")?; - crate::menus::invoke(app, ctx, "shape.newShape", comp_sized_shape_params(tool, w, h)) -} - /// The tool slots (with group separators after camera tools and pan-behind); returns the x after them. fn tool_slots(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, mut x: f32, cy: f32) -> f32 { let t = app.tokens; @@ -126,14 +110,6 @@ fn tool_slot(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, si: { app.ui.status = e; } - // Double-click a shape tool: a shape layer filling the composition, centred, for - // backgrounds, mattes and overlays (as After Effects does). - if cur.is_shape() - && resp.double_clicked() - && let Err(e) = comp_sized_shape(app, ui.ctx(), cur) - { - app.ui.status = e; - } if slot.len() > 1 { slot_flyout(app, ui, si, slot, &resp, held_id); } @@ -858,102 +834,4 @@ mod tests { frame(&mut app, &ctx, 2.1, vec![]); assert!(!app.auto.elements.iter().any(|w| w.id == "tools.select.TypeVertical")); } - - /// A polygon or star fills the shorter side; the other kinds fill the frame; all make a new layer. - #[test] - fn a_comp_sized_shape_fits_the_frame_for_every_kind() { - for (tool, size) in [ - (Tool::Rectangle, [1920.0, 1080.0]), - (Tool::RoundedRect, [1920.0, 1080.0]), - (Tool::Ellipse, [1920.0, 1080.0]), - (Tool::Polygon, [1080.0, 1080.0]), - (Tool::Star, [1080.0, 1080.0]), - ] { - let p = comp_sized_shape_params(tool, 1920, 1080); - assert_eq!(p["size"], json!(size), "{tool:?}"); - assert_eq!(p["position"], json!([960.0, 540.0])); - assert_eq!(p["newLayer"], json!(true)); - } - } - - /// The double-click shape takes the Tools bar's Fill, and is a new layer even with a shape layer - /// selected (not a group in that layer). - #[test] - fn the_double_clicked_shape_uses_the_toolbar_paint_on_a_new_layer() { - let mut s = effectcraft_engine::Session::default(); - s.execute("comp.new", json!({"width": 400, "height": 300, "duration": 2})).unwrap(); - let first = s.execute("layer.newShape", json!({"kind": "rect"})).unwrap()["layer"].as_u64().unwrap(); - s.execute("shape.fillStroke", json!({"layers": [first], "fill": "#00ff00", "stroke": false})).unwrap(); - s.execute("shape.toolOptions", json!({"fill": "#00ff00", "stroke": false})).unwrap(); - let mut app = EffectcraftApp::new(s); - let ctx = egui::Context::default(); - assert_eq!(app.session.state.selected_layers.first().map(|l| l.0), Some(first), "a shape layer is selected"); - crate::theme::install(&ctx, &app.tokens, crate::i18n::language(&app)); - frame(&mut app, &ctx, 0.0, vec![]); - frame(&mut app, &ctx, 0.1, vec![]); - assert_eq!(app.session.state.shape_tool.fill.color, [0.0, 1.0, 0.0], "the visible toolbar paint"); - let [x, y, w, h] = app.auto.find("tools.Rectangle").unwrap().rect; - let at = egui::pos2(x + w / 2.0, y + h / 2.0); - let pointer = |pressed| egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }; - frame(&mut app, &ctx, 1.0, vec![egui::Event::PointerMoved(at), pointer(true)]); - frame(&mut app, &ctx, 1.05, vec![pointer(false)]); - assert_eq!(app.session.active_comp().unwrap().layers.len(), 1, "a single click only selects the tool"); - // A separate double-click must route through the toolbar response, not just its helper. - for (time, pressed) in [(2.0, true), (2.05, false), (2.1, true), (2.15, false)] { - frame(&mut app, &ctx, time, vec![pointer(pressed)]); - } - let comp = app.session.active_comp().unwrap(); - assert_eq!(comp.layers.len(), 2, "a new layer, not a group in the selected one"); - let layer = comp.layers.iter().find(|l| l.id.0 != first).unwrap(); - assert_eq!(layer.props.prop("transform/position").unwrap().value.components(), vec![200.0, 150.0, 0.0]); - let mut fill = None; - let mut size = None; - layer.props.walk("", &mut |_, pr| { - if pr.name == "Size" { - size = Some(pr.value.clone()); - } - if fill.is_none() && matches!(pr.value, effectcraft_engine::keyframe::Value::Color(_)) { - fill = Some(pr.value.clone()); - } - }); - let fill = fill.expect("a fill colour"); - assert_eq!(size, Some(effectcraft_engine::keyframe::Value::Vec2([400.0, 300.0]))); - assert_eq!(fill, effectcraft_engine::keyframe::Value::Color([0.0, 1.0, 0.0, 1.0]), "the Tools bar's Fill, not the default blue"); - } - - #[test] - fn double_clicked_shapes_keep_the_selected_gradient_stroke_width() { - for kind in ["solid", "linear", "radial"] { - let mut s = effectcraft_engine::Session::default(); - s.execute("comp.new", json!({"width": 400, "height": 300, "duration": 2})).unwrap(); - s.execute("layer.newShape", json!({"kind": "rect"})).unwrap(); - s.execute("shape.fillStroke", json!({"fill": false, "strokeType": kind, "strokeWidth": 9})).unwrap(); - let mut app = EffectcraftApp::new(s); - let ctx = egui::Context::default(); - crate::theme::install(&ctx, &app.tokens, crate::i18n::language(&app)); - frame(&mut app, &ctx, 0.0, vec![]); - frame(&mut app, &ctx, 0.1, vec![]); - assert_eq!(app.session.state.shape_tool.stroke_width, 9.0, "selected {kind} stroke"); - let [x, y, w, h] = app.auto.find("tools.Rectangle").unwrap().rect; - let at = egui::pos2(x + w / 2.0, y + h / 2.0); - for (time, pressed) in [(1.0, true), (1.05, false), (1.1, true), (1.15, false)] { - frame( - &mut app, - &ctx, - time, - vec![ - egui::Event::PointerMoved(at), - egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }, - ], - ); - } - let comp = app.session.active_comp().unwrap(); - assert_eq!(comp.layers.len(), 2); - let layer = comp.layer(app.session.state.selected_layers[0]).unwrap(); - let (_, _, width) = effectcraft_engine::commands::shape_stroke::first_paint(layer, effectcraft_engine::time::Tick::ZERO); - assert_eq!(width, Some(9.0), "new {kind} stroke"); - let (_, stroke_kind) = effectcraft_engine::commands::shape_stroke::first_paint_kinds(layer, effectcraft_engine::time::Tick::ZERO); - assert_eq!(stroke_kind.id(), kind); - } - } } diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index 9efa9d940..bcae99bc5 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -1808,7 +1808,7 @@ use geometry::{ pub(crate) use geometry::{from_effect_space, fx2c, l2c, layer_outline, layer_quad}; use gesture::{Gesture, HANDLE, ROTO_HINT, pen_id, roi_drag, roi_handles, vertex_grab_radius}; use shapes::{create_shape, draw_shape_ghost, shape_drag}; -pub(crate) use shapes::{layer_at, layers_at, pick, selectable_layers, shape_kind}; +pub(crate) use shapes::{layer_at, layers_at, pick, selectable_layers}; #[cfg(test)] use tools::constrain_45; use tools::{add_motion_key, axis_at, axis_drag, drag_axes, draw_rigs, parent_world_inverse, pen_cursor, pen_cursor_icon, pen_press, select_motion_key}; From bc1734f59ef5ae59e0b44503390823d9153404dd Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Fri, 30 Oct 2026 03:56:25 +0100 Subject: [PATCH 20/23] #1284: remove remaining double-click paint helpers --- crates/engine/src/cmd/shape_stroke.rs | 4 +--- crates/ui-egui/src/panels/viewer/shapes.rs | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/shape_stroke.rs b/crates/engine/src/cmd/shape_stroke.rs index e81103cdf..ecd3ed8e0 100644 --- a/crates/engine/src/cmd/shape_stroke.rs +++ b/crates/engine/src/cmd/shape_stroke.rs @@ -216,12 +216,10 @@ pub fn first_paint(layer: &Layer, lt: Tick) -> (Option<[f64; 4]>, Option<[f64; 4 "fill" if fill.is_none() => fill = at("color").map(rgba), "stroke" if stroke.is_none() => { stroke = at("color").map(rgba); + width = at("width").map(|v| v.as_f64()); } _ => {} } - if width.is_none() && matches!(g.match_id.as_str(), "stroke" | "gstroke") { - width = at("width").map(|v| v.as_f64()); - } }); } (fill, stroke, width) diff --git a/crates/ui-egui/src/panels/viewer/shapes.rs b/crates/ui-egui/src/panels/viewer/shapes.rs index 1de85e46a..395219a05 100644 --- a/crates/ui-egui/src/panels/viewer/shapes.rs +++ b/crates/ui-egui/src/panels/viewer/shapes.rs @@ -78,7 +78,7 @@ pub(super) struct ShapeDrag { } /// The kind `tool` draws, as `shape.newShape` / `layer.addMask` name it. -pub(crate) fn shape_kind(tool: Tool) -> &'static str { +fn shape_kind(tool: Tool) -> &'static str { match tool { Tool::Rectangle => "rect", Tool::RoundedRect => "rounded", From eef204a79e46947fdcd6ab914fdec4ec6618196e Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Fri, 30 Oct 2026 04:20:49 +0100 Subject: [PATCH 21/23] #1284: preserve maximized viewer focus when closing the queue --- crates/ui-egui/src/menus/invoke.rs | 3 +-- crates/ui-egui/src/menus/tests.rs | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/menus/invoke.rs b/crates/ui-egui/src/menus/invoke.rs index 446513f05..87f6a750a 100644 --- a/crates/ui-egui/src/menus/invoke.rs +++ b/crates/ui-egui/src/menus/invoke.rs @@ -220,8 +220,7 @@ pub fn invoke(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: V app.apply_dock(craft_ui::docking::Action::Close { panel: PanelKind::RenderQueue })?; // The focus goes to a panel that is still there, so the next Close acts on the composition // instead of on the queue that is gone. - let next = - [PanelKind::Timeline, PanelKind::Composition, PanelKind::Project].into_iter().find(|p| app.ui.dock.is_visible(*p)).unwrap_or(PanelKind::Project); + let next = [PanelKind::Timeline, PanelKind::Composition, PanelKind::Project].into_iter().find(|p| panel_shown(app, *p)).unwrap_or(PanelKind::Project); app.show_panel(next); app.focus_panel(ctx, next); // Activating a passive viewer emits OpenComp, which raises Timeline. Finish that routing diff --git a/crates/ui-egui/src/menus/tests.rs b/crates/ui-egui/src/menus/tests.rs index 63fac9879..36d667a72 100644 --- a/crates/ui-egui/src/menus/tests.rs +++ b/crates/ui-egui/src/menus/tests.rs @@ -473,6 +473,30 @@ fn closing_the_render_queue_reveals_the_panel_receiving_focus() { } } +/// Closing a floating queue must focus the viewer that is actually shown, retaining maximization. +#[test] +fn closing_a_floating_queue_preserves_the_maximized_composition() { + let mut session = effectcraft_engine::Session::default(); + session.execute("comp.new", json!({"name": "A", "width": 32, "height": 32, "duration": 2})).unwrap(); + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(|_| EffectcraftApp::new(session)); + h.run_steps(3); + let ctx = h.ctx.clone(); + let app = h.state_mut(); + app.show_panel(PanelKind::Composition); + app.show_panel(PanelKind::RenderQueue); + app.apply_dock(craft_ui::docking::Action::Float { panel: PanelKind::RenderQueue, rect: [50.0, 60.0, 400.0, 200.0] }).unwrap(); + app.toggle_maximize(PanelKind::Composition); + app.ui.focused = PanelKind::RenderQueue; + assert!(app.ui.dock.is_visible(PanelKind::Timeline), "the hidden underlying tab triggers the regression"); + assert!(!panel_shown(app, PanelKind::Timeline)); + invoke(app, &ctx, "file.close", json!({})).unwrap(); + h.run_steps(3); + assert_eq!(h.state().ui.focused, PanelKind::Composition); + assert_eq!(h.state().ui.maximized, Some(PanelKind::Composition)); + assert!(!h.state().ui.dock.is_open(PanelKind::RenderQueue)); + assert_eq!(h.state().session.state.open_comps.len(), 1); +} + #[test] fn queue_close_preserves_state_when_workspace_is_locked() { let ctx = egui::Context::default(); From b04fe30baef76aca8011f6df8a02504fc7964336 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Fri, 30 Oct 2026 10:46:48 +0700 Subject: [PATCH 22/23] rustfmt --- crates/ui-egui/src/panels/timeline/tests.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/panels/timeline/tests.rs b/crates/ui-egui/src/panels/timeline/tests.rs index 3263a14ed..ebcbfa8b9 100644 --- a/crates/ui-egui/src/panels/timeline/tests.rs +++ b/crates/ui-egui/src/panels/timeline/tests.rs @@ -565,9 +565,17 @@ fn the_key_menu_is_translated_and_cannot_edit_a_locked_layers_keys() { h.run(); !h.get_by_label(label).accesskit_node().is_disabled() }; - for label in - ["Copy", "Select Equal Keyframes", "Select All Keyframes", "Delete", "Edit Value…", "Easy Ease", "Time-Reverse Keyframes", "Keyframe Velocity…", "Mute Keyframes"] - { + for label in [ + "Copy", + "Select Equal Keyframes", + "Select All Keyframes", + "Delete", + "Edit Value…", + "Easy Ease", + "Time-Reverse Keyframes", + "Keyframe Velocity…", + "Mute Keyframes", + ] { assert!(enabled(false, true, label), "{label} on an unlocked layer"); } for label in ["Copy", "Select Equal Keyframes", "Select All Keyframes"] { From 3a7c4fbbc7053fb3acefdc94acbec7b1c50c2758 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Fri, 30 Oct 2026 11:02:17 +0700 Subject: [PATCH 23/23] Timeline key menu: clippy's simpler enabled condition --- crates/ui-egui/src/panels/timeline.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index df3c9a801..7c47dccfb 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -4177,7 +4177,7 @@ pub(crate) fn key_menu( let edits = !matches!(cmd, "keys.copy" | "keys.selectEqual" | "keys.selectPrevious" | "keys.selectFollowing" | "keys.selectAll"); // An unselected key is selected before the command runs, and locked keys cannot be selected. let unselectable = locked && !selected && cmd != "keys.selectAll"; - if ui.add_enabled(!(locked && edits) && !unselectable, egui::Button::new(crate::i18n::context_item(cmd, ¶ms, lbl))).clicked() { + if ui.add_enabled(!(unselectable || locked && edits), egui::Button::new(crate::i18n::context_item(cmd, ¶ms, lbl))).clicked() { if !selected { actions.push(("keys.select".into(), json!({"keys": [this_key.clone()]}))); }