diff --git a/crates/engine/src/history_cmds.rs b/crates/engine/src/history_cmds.rs index 652e2326b4..d9ca98f962 100644 --- a/crates/engine/src/history_cmds.rs +++ b/crates/engine/src/history_cmds.rs @@ -7,7 +7,7 @@ use serde_json::{Value, json}; use crate::commands::{CommandSpec, int}; -use crate::image_cmds::for_each_layer; +use crate::image_cmds::reidentify_copied_layers; use crate::{EngineError, Result, Session}; fn bad(cmd: &str, msg: impl Into) -> EngineError { @@ -66,7 +66,7 @@ fn new_document(s: &mut Session, p: &Value) -> Result { _ if !label.trim().is_empty() => label, _ => format!("{} copy", doc.name), }; - for_each_layer(&mut doc.layers, &mut |l| l.id = photocraft_doc::LayerId::fresh()); + reidentify_copied_layers(&mut doc); let name = doc.name.clone(); let index = s.add_document(doc, None); Ok(json!({ "document": index, "name": name })) diff --git a/crates/engine/src/image_cmds.rs b/crates/engine/src/image_cmds.rs index 077512047b..d18fc810ec 100644 --- a/crates/engine/src/image_cmds.rs +++ b/crates/engine/src/image_cmds.rs @@ -3,7 +3,7 @@ use photocraft_algo::resample::{Resample, crop_surface, resize_surface_in_canvas, translate_surface}; use photocraft_color::{ColorMode, SampleType}; -use photocraft_doc::{Document, Effect, Effects, FxPaint, Layer, LayerContent, Size}; +use photocraft_doc::{Document, Effect, Effects, FxPaint, Layer, LayerContent, LayerId, Size}; use photocraft_geom::Rect; use photocraft_raster::Surface; use serde_json::{Value, json}; @@ -65,6 +65,34 @@ pub(crate) fn for_each_layer(layers: &mut [Layer], f: &mut dyn FnMut(&mut Layer) } } +/// Assign new layer ids to a copied document and update every saved layer-comp reference. +/// +/// A document copy needs fresh identities, but its comps must still target the copied +/// layers. Deliberately leave references to already-deleted layers untouched so their +/// missing-layer warnings remain meaningful. The same applies to Last Document State. +pub(crate) fn reidentify_copied_layers(doc: &mut Document) { + let mut ids = std::collections::HashMap::new(); + for_each_layer(&mut doc.layers, &mut |layer| { + let previous = layer.id; + layer.id = LayerId::fresh(); + ids.insert(previous, layer.id); + }); + + let remap = |comp: &mut photocraft_doc::LayerComp| { + for state in &mut comp.states { + if let Some(&new_id) = ids.get(&state.layer) { + state.layer = new_id; + } + } + }; + for comp in &mut doc.layer_comps { + remap(comp); + } + if let Some(last) = &mut doc.last_document_state { + remap(last); + } +} + pub(crate) fn scale_effects(fx: &mut Effects, k: f32) { for e in &mut fx.items { match e { @@ -479,8 +507,13 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { surf.write_region(doc.bounds(), &vals); surf.prune(); doc.layers = vec![Layer::new("Background", LayerContent::Raster(surf))]; + // A merged-only copy has no original layers for saved comps to restore. + // Don't carry dangling comp state into a single-layer flattened document. + doc.layer_comps.clear(); + doc.last_applied_comp = None; + doc.last_document_state = None; } - for_each_layer(&mut doc.layers, &mut |l| l.id = photocraft_doc::LayerId::fresh()); + reidentify_copied_layers(&mut doc); let name = doc.name.clone(); let index = s.add_document(doc, None); Ok(json!({ "document": index, "name": name })) diff --git a/crates/engine/tests/duplicate_layer_comps.rs b/crates/engine/tests/duplicate_layer_comps.rs new file mode 100644 index 0000000000..ecd8b0230b --- /dev/null +++ b/crates/engine/tests/duplicate_layer_comps.rs @@ -0,0 +1,149 @@ +//! Layer comps must continue targeting their layers after a copied document gets fresh ids. +//! Cover current/old history states, nested layers, Last Document State and merged-only copies. + +use photocraft_doc::{CompLayerState, Layer, LayerId}; +use photocraft_engine::Session; +use serde_json::json; + +fn scene() -> (Session, LayerId) { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 16, "height": 16, "background": "white"})).unwrap(); + let id = LayerId(s.execute("layer.new.layer", json!({"name": "L"})).unwrap()["layer"].as_u64().unwrap()); + (s, id) +} + +fn copied_layer(s: &Session, name: &str) -> LayerId { + s.active().unwrap().doc.walk().into_iter().find(|(_, _, l)| l.name == name).unwrap().2.id +} + +#[test] +fn image_duplicate_and_history_copy_keep_saved_comp_visibility() { + for command in ["image.duplicate", "history.newDocument"] { + let (mut s, source) = scene(); + let original = s.active().unwrap().doc.id; + s.execute("layerComp.new", json!({"name": "Shown"})).unwrap(); + s.execute("layer.setProps", json!({"layer": source.0, "visible": false})).unwrap(); + s.execute(command, json!({})).unwrap(); + + let id = copied_layer(&s, "L"); + let doc = &s.active().unwrap().doc; + assert_ne!(doc.id, original); + assert_ne!(id, source, "{command}: layers must get new identities"); + assert!(!doc.layer(id).unwrap().visible, "{command}: copied current layout"); + let comp = doc.layer_comps.iter().find(|c| c.name == "Shown").unwrap(); + assert!(comp.missing_layers(doc).is_empty(), "{command}: all captured layers still exist"); + assert_eq!(comp.state(id).unwrap().visible, Some(true), "{command}: preserve recorded state"); + + let applied = s.execute("layerComp.apply", json!({"comp": "Shown"})).unwrap(); + assert_eq!(applied["missingLayers"], 0, "{command}: no spurious missing layers"); + assert!(s.active().unwrap().doc.layer(id).unwrap().visible, "{command}: recorded visibility restored"); + + s.set_active(0); + assert!(!s.active().unwrap().doc.layer(source).unwrap().visible, "{command}: source untouched"); + assert_eq!(s.execute("layerComp.apply", json!({"comp": "Shown"})).unwrap()["missingLayers"], 0); + assert!(s.active().unwrap().doc.layer(source).unwrap().visible, "{command}: source comp still works"); + } +} + +#[test] +fn a_historical_state_uses_its_own_saved_comp_and_fresh_layer_ids() { + let (mut s, source) = scene(); + s.execute("layerComp.new", json!({"name": "Shown"})).unwrap(); + s.execute("layer.setProps", json!({"layer": source.0, "visible": false})).unwrap(); + let saved_state = s.active().unwrap().history.past_len(); + s.execute("layer.setProps", json!({"layer": source.0, "visible": true})).unwrap(); + + s.execute("history.newDocument", json!({"state": saved_state, "name": "From History"})).unwrap(); + assert_eq!(s.active().unwrap().doc.name, "From History"); + let id = copied_layer(&s, "L"); + assert_ne!(id, source); + assert!(!s.active().unwrap().doc.layer(id).unwrap().visible, "the historical hidden layout survives"); + assert_eq!(s.execute("layerComp.apply", json!({"comp": "Shown"})).unwrap()["missingLayers"], 0); + assert!(s.active().unwrap().doc.layer(id).unwrap().visible, "historical comp can restore visibility"); + s.set_active(0); + assert!(s.active().unwrap().doc.layer(source).unwrap().visible, "current source state remains unchanged"); +} + +#[test] +fn nested_group_states_are_remapped_but_genuinely_missing_layers_stay_missing() { + let (mut s, _) = scene(); + let (group, child) = s + .edit("Add nested group", |doc, _| { + let child = Layer::raster("Child", doc.pixel_format()); + let child_id = child.id; + let group = Layer::group("Group", vec![child]); + let group_id = group.id; + doc.layers.push(group); + Ok((group_id, child_id)) + }) + .unwrap(); + s.execute("layerComp.new", json!({"name": "Nested"})).unwrap(); + // An old comp may already refer to a deleted layer before duplication. + // Copying should not silently rewrite that warning to an unrelated new id. + let missing = LayerId(u64::MAX); + s.edit("Record deleted layer", |doc, _| { + let comp = doc.layer_comps.iter_mut().find(|c| c.name == "Nested").unwrap(); + comp.states.push(CompLayerState { layer: missing, visible: Some(false), position: None, appearance: None }); + Ok(()) + }) + .unwrap(); + s.execute("layer.setProps", json!({"layer": child.0, "visible": false})).unwrap(); + s.execute("image.duplicate", json!({})).unwrap(); + + let new_group = copied_layer(&s, "Group"); + let new_child = copied_layer(&s, "Child"); + assert_ne!(new_group, group); + assert_ne!(new_child, child); + let doc = &s.active().unwrap().doc; + let comp = doc.layer_comps.iter().find(|c| c.name == "Nested").unwrap(); + assert!(comp.state(new_group).is_some()); + assert_eq!(comp.state(new_child).unwrap().visible, Some(true)); + assert_eq!(comp.missing_layers(doc), vec![missing]); + assert_eq!(s.execute("layerComp.apply", json!({"comp": "Nested"})).unwrap()["missingLayers"], 1); + assert!(s.active().unwrap().doc.layer(new_child).unwrap().visible); +} + +#[test] +fn duplicated_last_document_state_restores_the_copied_layout() { + for command in ["image.duplicate", "history.newDocument"] { + let (mut s, source) = scene(); + s.execute("layerComp.new", json!({"name": "Shown"})).unwrap(); + s.execute("layer.setProps", json!({"layer": source.0, "visible": false})).unwrap(); + s.execute("layerComp.apply", json!({"comp": "Shown"})).unwrap(); + assert!(s.active().unwrap().doc.last_document_state.is_some()); + s.execute(command, json!({})).unwrap(); + + let new_id = copied_layer(&s, "L"); + let doc = &s.active().unwrap().doc; + assert!(doc.layer(new_id).unwrap().visible); + let backup = doc.last_document_state.as_ref().unwrap(); + assert_eq!(backup.state(new_id).unwrap().visible, Some(false)); + assert!(backup.missing_layers(doc).is_empty(), "{command}: backup references copied ids"); + + s.execute("layerComp.restoreLastDocumentState", json!({})).unwrap(); + assert!(!s.active().unwrap().doc.layer(new_id).unwrap().visible, "{command}: backup restores hidden layout"); + s.set_active(0); + assert!(s.active().unwrap().doc.layer(source).unwrap().visible, "{command}: source remains visible"); + } +} + +#[test] +fn merged_only_duplicate_does_not_carry_comps_for_removed_source_layers() { + let (mut s, source) = scene(); + s.execute("layerComp.new", json!({"name": "Original"})).unwrap(); + s.execute("layer.setProps", json!({"layer": source.0, "visible": false})).unwrap(); + s.execute("layerComp.apply", json!({})).unwrap(); + assert!(s.active().unwrap().doc.last_document_state.is_some()); + + s.execute("image.duplicate", json!({"mergedOnly": true})).unwrap(); + let d = &s.active().unwrap().doc; + assert_eq!(d.layers.len(), 1, "merged-only copy is a single composite layer"); + assert!(d.layer_comps.is_empty(), "old comps cannot address discarded layers"); + assert_eq!(d.last_applied_comp, None); + assert!(d.last_document_state.is_none(), "no stale saved layout on a flattened copy"); + assert!(!s.is_enabled("layerComp.apply")); + + s.set_active(0); + assert_eq!(s.active().unwrap().doc.layer_comps.len(), 1, "original comps are preserved"); + assert!(s.active().unwrap().doc.last_document_state.is_some(), "original backup is preserved"); +}