From 6e3b5eb3e35510d36c456e634f7890a5060045db Mon Sep 17 00:00:00 2001 From: Anubhav-Rai Date: Fri, 23 Oct 2026 04:37:38 +0530 Subject: [PATCH] engine: deleting a layer clears the references to it (#2153) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing cleared a `Value::Layer` reference when its layer was deleted, so the id stayed behind: `env_pixels` resolved it to nothing and rendered no environment at all, while the UI showed "None", which is documented as "fall back to the comp's Environment Layer". The delete path already unparents children and drops a track matte that pointed at a deleted layer; a layer reference now goes the same way, in the same history step, so undo restores the layer and the reference together. This covers every layer-reference property, not only an Environment light's Source: 73 of them across the effects (Clone Source, Set Matte, Back Layer, Water Surface, Sky, Ground…). Keyframes carry the same value kind as the property, as in `essential.rs`, and are fixed too. Co-Authored-By: Claude Opus 5 --- crates/engine/src/commands/edit.rs | 16 ++++++++++++++++ crates/engine/src/tests_3d.rs | 17 +++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/crates/engine/src/commands/edit.rs b/crates/engine/src/commands/edit.rs index 019d56d1..4bfe6fa7 100644 --- a/crates/engine/src/commands/edit.rs +++ b/crates/engine/src/commands/edit.rs @@ -1,6 +1,7 @@ //! Edit menu. use effectcraft_color::Label; +use effectcraft_keyframe::Value as KV; use effectcraft_project::{Comp, Expression, ItemKind, Layer, LayerId, LayerSource, TrackMatte}; use effectcraft_time::Tick; use serde_json::{Value, json}; @@ -201,6 +202,21 @@ fn delete(s: &mut Session, p: &Value) -> Result { if l.track_matte.is_some_and(|m| ids.contains(&m.layer)) { l.track_matte = None; } + // A layer-reference property (an Environment light's Source, an effect's Clone Source, + // Back Layer…) must read as None once its layer is gone, in this same history step: a + // dead id renders as nothing while the UI shows "None", which means "fall back to the + // comp's Environment Layer" (#2153). Keys carry the same value kind as the property. + l.props.walk_mut(&mut |p| { + let dead = |v: &KV| matches!(v, KV::Layer(Some(id)) if ids.contains(&LayerId(*id))); + if dead(&p.value) { + p.value = KV::Layer(None); + } + for k in &mut p.keys { + if dead(&k.value) { + k.value = KV::Layer(None); + } + } + }); } st.selected_layers.clear(); st.selected_props.clear(); diff --git a/crates/engine/src/tests_3d.rs b/crates/engine/src/tests_3d.rs index cf9773f9..07a48af1 100644 --- a/crates/engine/src/tests_3d.rs +++ b/crates/engine/src/tests_3d.rs @@ -131,6 +131,23 @@ fn environment_light_is_listed_and_takes_a_source_layer() { assert_eq!(prop(&s, env, "lightOptions/source").as_layer(), None); } +/// Deleting the layer a Source points at leaves no dangling id: "None" means the comp's +/// Environment Layer, and a dead id rendered as nothing at all (#2153). +#[test] +fn deleting_a_source_layer_clears_the_reference() { + let mut s = session(); + let sky = s.execute("layer.newSolid", json!({"width": 256, "height": 128, "name": "Sky"})).unwrap()["layer"].as_u64().unwrap(); + let env = s.execute("layer.newLight", json!({"kind": "Environment", "name": "Env"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("prop.set", json!({"layer": env, "path": "lightOptions/source", "value": sky})).unwrap(); + assert_eq!(prop(&s, env, "lightOptions/source").as_layer(), Some(sky)); + s.execute("edit.clear", json!({"layers": [sky]})).unwrap(); + assert_eq!(prop(&s, env, "lightOptions/source").as_layer(), None, "the reference goes with the layer"); + // One history step with the delete, so undo brings both back. + s.undo(); + assert_eq!(prop(&s, env, "lightOptions/source").as_layer(), Some(sky)); + assert!(s.active_comp().unwrap().layer(effectcraft_project::LayerId(sky)).is_some()); +} + #[test] fn three_d_switch_undo_redo() { let mut s = session();