From 9992fa0bf62ecabcf7451849fef2d10ceb8c90c1 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 4 Nov 2026 06:39:39 -0800 Subject: [PATCH 1/2] Symbols: saving leaves out a nested editing layer and keeps layers named in file data Leaving opacity-mask or symbol editing now removes the editing layer wherever it is, so a save or export after Layers moved it into another layer no longer writes it. An editing id that names anything other than a layer is left alone. symbol_edit is no longer read from files: a value in file data is dropped on load, so the layer it names is kept and saving does not write the value again. --- crates/doc/src/document_ext.rs | 14 ++++---- crates/doc/src/lib.rs | 5 +-- crates/doc/src/skip.rs | 5 +++ crates/doc/src/tests.rs | 61 ++++++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 8 deletions(-) diff --git a/crates/doc/src/document_ext.rs b/crates/doc/src/document_ext.rs index c9a6c5f56..efd7f80b2 100644 --- a/crates/doc/src/document_ext.rs +++ b/crates/doc/src/document_ext.rs @@ -99,13 +99,15 @@ impl Document { } /// Leave opacity-mask editing and symbol editing: drop the temporary editing layer, a working - /// copy of art the mask or the symbol already holds (the engine syncs it after every edit). + /// copy of art the mask or the symbol already holds (the engine syncs it after every edit). The + /// layer is removed wherever it is, also as a sublayer of another layer; an id that names + /// anything other than a layer is left alone. pub fn drop_edit_modes(&mut self) { - if let Some(me) = self.mask_edit.take() { - self.layers.retain(|l| l.id != me.layer); - } - if let Some(se) = self.symbol_edit.take() { - self.layers.retain(|l| l.id != se.layer); + let layers = [self.mask_edit.take().map(|me| me.layer), self.symbol_edit.take().map(|se| se.layer)]; + for id in layers.into_iter().flatten() { + if self.node(id).is_some_and(Node::is_layer) { + let _ = self.remove(id); + } } } diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index 84a66220f..133bc4120 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -228,8 +228,9 @@ pub struct Document { #[serde(default, skip_serializing)] pub mask_edit: Option, /// Symbol editing mode, while active. Never saved (see [`Document::without_edit_modes`]): the - /// symbol already holds the art of its editing layer. - #[serde(default, skip_serializing)] + /// symbol already holds the art of its editing layer. Never read either: a value in file data + /// is dropped, so the layer it names is kept and saving does not write the value again. + #[serde(default, skip_serializing, deserialize_with = "skip::read_as_none")] pub symbol_edit: Option, next_id: u64, /// Foreign data preserved on round-trip. diff --git a/crates/doc/src/skip.rs b/crates/doc/src/skip.rs index 15f294c00..b26a1a7ac 100644 --- a/crates/doc/src/skip.rs +++ b/crates/doc/src/skip.rs @@ -13,3 +13,8 @@ pub fn is_empty_vec(v: &[T]) -> bool { pub fn is_empty_map(v: &serde_json::Map) -> bool { v.is_empty() } +/// Read a field's value and drop it, giving `None`: for state that is never taken from a file. +pub fn read_as_none<'de, D: serde::Deserializer<'de>, T>(d: D) -> Result, D::Error> { + ::deserialize(d)?; + Ok(None) +} diff --git a/crates/doc/src/tests.rs b/crates/doc/src/tests.rs index 37e2d39bc..4b75b0149 100644 --- a/crates/doc/src/tests.rs +++ b/crates/doc/src/tests.rs @@ -451,3 +451,64 @@ fn pica_form_rejects_exponent_notation() { assert_eq!(Unit::Points.parse(s), None, "{s}"); } } + +/// Symbol editing of symbol "S" on `layer`. +fn symbol_edit_on(layer: NodeId) -> SymbolEdit { + SymbolEdit { + symbol: "S".into(), + layer, + instance: None, + xf: Affine::IDENTITY, + group: false, + changed: false, + back_isolation: None, + back_layer: None, + } +} + +/// The opacity-mask and symbol editing layers are left out of the saved document wherever they +/// are, also when `layer.move` made them sublayers of another layer. +#[test] +fn edit_mode_layers_are_dropped_as_sublayers_too() { + let mut d = Document::new(612.0, 792.0); + let host = d.layers[0].id; + let mask = d.add_layer(Some("Mask")); + let symbol = d.add_layer(Some("Symbol")); + d.move_node(mask, Some(host), 0).unwrap(); + d.move_node(symbol, Some(host), usize::MAX).unwrap(); + let object = d.alloc_id(); + d.mask_edit = Some(MaskEdit { object, layer: mask }); + d.symbol_edit = Some(symbol_edit_on(symbol)); + let saved = d.without_edit_modes(); + assert!(saved.node(mask).is_none(), "the mask editing layer is left out"); + assert!(saved.node(symbol).is_none(), "the symbol editing layer is left out"); + assert!(saved.node(host).is_some_and(|l| l.children().is_some_and(|c| c.is_empty()))); + assert!(saved.mask_edit.is_none() && saved.symbol_edit.is_none()); +} + +/// Symbol editing is never read from a file, so a layer that file data names as the editing layer +/// loads and stays in the document, and saving does not write the value again. +#[test] +fn symbol_edit_is_not_read_from_a_file() { + let mut d = Document::new(612.0, 792.0); + let layer = d.layers[0].id; + let mut v = serde_json::to_value(&d).unwrap(); + v["symbol_edit"] = serde_json::to_value(symbol_edit_on(layer)).unwrap(); + d = serde_json::from_value(v).unwrap(); + assert_eq!(d.symbol_edit, None); + assert!(d.extra.is_empty(), "not kept as an unknown key"); + d.drop_edit_modes(); + assert!(d.node(layer).is_some(), "the layer is kept"); + assert!(serde_json::to_value(&d).unwrap().get("symbol_edit").is_none(), "not saved again"); +} + +/// An opacity-mask editing id in file data that names art inside a layer, not a layer, leaves the +/// art in place. +#[test] +fn mask_edit_naming_art_leaves_it() { + let (mut d, a, b) = doc_with_rects(); + d.mask_edit = Some(MaskEdit { object: b, layer: a }); + d.drop_edit_modes(); + assert!(d.node(a).is_some() && d.node(b).is_some()); + assert_eq!(d.mask_edit, None); +} From 32456839ebb3dfd3d6f08ca06d4a8880655528e9 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 4 Nov 2026 06:53:00 -0800 Subject: [PATCH 2/2] Native format: a saved mask-editing id drops only a top-level layer Files saved mid-edit by older versions carry the opacity-mask editing layer at the top level. An id there that names a sublayer is ignored on load, so that sublayer and its art are kept. --- crates/format/src/lib.rs | 6 +++++- crates/format/tests/it/native_v3.rs | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 6ed3e6840..65c10ba80 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -537,7 +537,11 @@ fn load_file_within(bytes: &[u8], max_nodes: usize, max_values: usize) -> Result } doc.images.insert(k, blob); } - // Saved mid-edit by an older version: drop the opacity-mask editing layer. + // Saved mid-edit by an older version: drop the opacity-mask editing layer. Those versions + // removed it only as a top-level layer, so an id naming anything else is ignored. + if doc.mask_edit.is_some_and(|me| !doc.layers.iter().any(|l| l.id == me.layer)) { + doc.mask_edit = None; + } doc.drop_edit_modes(); // Saved before per-fill/stroke overprint: the Overprint Black list becomes overprint flags. doc.migrate_overprint_black(); diff --git a/crates/format/tests/it/native_v3.rs b/crates/format/tests/it/native_v3.rs index 5475f39fb..6a96fae79 100644 --- a/crates/format/tests/it/native_v3.rs +++ b/crates/format/tests/it/native_v3.rs @@ -157,6 +157,24 @@ fn keys_from_newer_versions_round_trip() { check_native_roundtrip(&d).unwrap(); } +/// An opacity-mask editing state in a file drops the editing layer it names only when that is a +/// top-level layer, as files saved mid-edit have it: a sublayer it names loads and stays. +#[test] +fn saved_mask_editing_drops_only_a_top_level_layer() { + let mut d = Document::new(612.0, 792.0); + let host = d.layers[0].id; + let sub = d.add_layer(Some("Sub")); + d.move_node(sub, Some(host), 0).unwrap(); + let top = d.add_layer(Some("Mask")); + for (layer, kept) in [(sub, true), (top, false)] { + let mut v = json_of(&save(&d, false)); + v["document"]["mask_edit"] = json!({"object": host, "layer": layer}); + let back = load(&serde_json::to_vec(&v).unwrap()).unwrap(); + assert_eq!(back.node(layer).is_some(), kept, "layer {layer:?}"); + assert!(back.mask_edit.is_none() && back.node(host).is_some()); + } +} + /// The paragraph composer is saved only when it isn't the default (Every-line) and reads back. #[test] fn text_composer_round_trips() {