From 39a0c6aa547d6bad87bc22cc0824f45ee54ce0b1 Mon Sep 17 00:00:00 2001 From: Vikram Bhaskaran Date: Sat, 10 Oct 2026 21:42:03 +1100 Subject: [PATCH] Move a layer when Position is edited and its dimensions are separated. The renderer follows X, Y and Z Position once those channels exist. The Position command was writing only the combined property, so the layer stayed where it was. --- crates/engine/src/commands/layer_menu.rs | 26 ++++++++++++ crates/engine/src/tests_menu_cmds.rs | 53 ++++++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/crates/engine/src/commands/layer_menu.rs b/crates/engine/src/commands/layer_menu.rs index 5ad0c1fc..ba00db38 100644 --- a/crates/engine/src/commands/layer_menu.rs +++ b/crates/engine/src/commands/layer_menu.rs @@ -192,6 +192,32 @@ fn set_transform(s: &mut Session, p: &Value) -> Result { for lid in &ids { let l = layer_mut(proj, cid, *lid)?; let lt = l.layer_time(t); + // Separate Dimensions: the picture follows X/Y/Z Position, not the combined property. + if m == "position" && l.transform().is_some_and(|tr| tr.get("positionX").is_some()) { + let cur = l + .transform() + .map(|tr| { + [ + tr.get("positionX").map(|p| p.value_at(lt).as_f64()).unwrap_or(0.0), + tr.get("positionY").map(|p| p.value_at(lt).as_f64()).unwrap_or(0.0), + tr.get("positionZ").map(|p| p.value_at(lt).as_f64()).unwrap_or(0.0), + ] + }) + .unwrap_or([0.0; 3]); + // Two numbers keep the current z. + let j = match &v { + Value::Array(a) if a.len() == 2 => json!([a[0], a[1], cur[2]]), + _ => v.clone(), + }; + let nv = KV::Vec3(cur).coerce_json(&j).ok_or_else(|| bad("layer.setTransform", format!("can't use {v} for {label}")))?; + let xyz = nv.as_vec3(); + let Some(tr) = l.transform_mut() else { continue }; + super::layer::update_position(tr, lt, |_| xyz); + if let Some(pr) = tr.get_mut("position") { + pr.set_value_at(lt, KV::Vec3(xyz)); + } + continue; + } let Some(pr) = l.transform_mut().and_then(|tr| tr.get_mut(m)) else { continue }; let cur = pr.value_at(lt); // Two numbers for a 3D value keep the current z. diff --git a/crates/engine/src/tests_menu_cmds.rs b/crates/engine/src/tests_menu_cmds.rs index c342f72f..0d626dd8 100644 --- a/crates/engine/src/tests_menu_cmds.rs +++ b/crates/engine/src/tests_menu_cmds.rs @@ -3,6 +3,7 @@ use effectcraft_keyframe::Value as KV; use effectcraft_project::{FrameBlend, ItemKind, MatteKind, Quality, Sampling}; +use effectcraft_time::Tick; use serde_json::{Value, json}; use crate::{EngineError, Event, Session}; @@ -103,6 +104,58 @@ fn transform_dialog_values_and_center_anchor() { assert_eq!(layer(&s, a).auto_orient, effectcraft_project::AutoOrient::AlongPath); } +/// Pixel-centre of the opaque samples, so a 10×10 solid centred on (11, 22) reports (11, 22). +fn opaque_centroid(s: &Session) -> (f64, f64, u32) { + let img = s.render(s.active_comp_id().unwrap(), Tick::ZERO, Default::default()); + let mut n = 0u32; + let mut sx = 0.0; + let mut sy = 0.0; + for y in 0..img.height { + for x in 0..img.width { + if img.get(x as i64, y as i64)[3] > 0.5 { + n += 1; + sx += x as f64 + 0.5; + sy += y as f64 + 0.5; + } + } + } + assert!(n > 0, "the layer rendered nothing"); + (sx / n as f64, sy / n as f64, n) +} + +#[test] +fn separated_position_set_transform_moves_the_layer() { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Scene", "width": 100, "height": 100, "frameRate": 30, "duration": 2})).unwrap(); + let a = s.execute("layer.newSolid", json!({"name": "Square", "color": "#ff0000", "width": 10, "height": 10})).unwrap()["layer"].as_u64().unwrap(); + let (x, y, n) = opaque_centroid(&s); + assert_eq!(n, 100); + assert!((x - 50.0).abs() < 0.6 && (y - 50.0).abs() < 0.6, "starts centred, centroid ({x}, {y})"); + + s.execute("prop.separateDimensions", json!({"layer": a, "value": true})).unwrap(); + s.execute("layer.setTransform", json!({"layers": [a], "prop": "position", "value": [11, 22]})).unwrap(); + let l = layer(&s, a); + let tr = l.transform().unwrap(); + assert!((tr.get("positionX").unwrap().value.as_f64() - 11.0).abs() < 1e-6); + assert!((tr.get("positionY").unwrap().value.as_f64() - 22.0).abs() < 1e-6); + assert_eq!(tr.get("position").unwrap().value, KV::Vec3([11.0, 22.0, 0.0])); + let (x, y, n) = opaque_centroid(&s); + assert_eq!(n, 100); + assert!((x - 11.0).abs() < 0.6 && (y - 22.0).abs() < 0.6, "separated position renders at ({x}, {y})"); + + s.execute("edit.undo", json!({})).unwrap(); + let (x, y, _) = opaque_centroid(&s); + assert!((x - 50.0).abs() < 0.6 && (y - 50.0).abs() < 0.6, "undo puts it back at ({x}, {y})"); + + // Combined Position, the control, still moves. + s.execute("edit.undo", json!({})).unwrap(); + assert!(layer(&s, a).transform().unwrap().get("positionX").is_none()); + s.execute("layer.setTransform", json!({"prop": "position", "value": [11, 22]})).unwrap(); + let (x, y, n) = opaque_centroid(&s); + assert_eq!(n, 100); + assert!((x - 11.0).abs() < 0.6 && (y - 22.0).abs() < 0.6, "combined position renders at ({x}, {y})"); +} + #[test] fn mask_menu_on_existing_masks() { let mut s = comp();