From 4d1eb16424ef189c5a9e7c54740ddf73f0102424 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Thu, 1 Oct 2026 10:32:58 -0400 Subject: [PATCH] M1: fix paste into a compound path when undo made a new object reuse the active layer's id (active layer / isolation must still be a layer / group); regression test Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/clipboard.rs | 19 +++++++++++++++++++ crates/engine/src/lib.rs | 8 +++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/clipboard.rs b/crates/engine/src/cmd/clipboard.rs index 6a9c7c06..110ab49d 100644 --- a/crates/engine/src/cmd/clipboard.rs +++ b/crates/engine/src/cmd/clipboard.rs @@ -115,6 +115,25 @@ mod tests { assert!((b.width() - 30.0).abs() < 1e-6 && (b.height() - 40.0).abs() < 1e-6); } + #[test] + fn paste_never_lands_in_an_object_that_reused_an_undone_layers_id() { + // Found by the model-based test: undo restores the id counter, so a compound path made + // after undoing New Layer gets the dead layer's id, which was still the active layer. + let mut s = session(); + let r = s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap()["id"].clone(); + s.execute("select.set", &json!({"ids": [r]})).unwrap(); + s.execute("edit.copy", &json!({})).unwrap(); + let layer = s.execute("layer.new", &json!({})).unwrap()["id"].as_u64().unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + s.execute("select.set", &json!({"ids": [r]})).unwrap(); + let c = s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap(); + assert_eq!(c, layer, "the scenario needs the id to be reused"); + s.execute("edit.paste", &json!({})).unwrap(); + let d = &s.doc().unwrap().doc; + let compound = d.node(NodeId(c)).unwrap(); + assert!(compound.children().unwrap().iter().all(|ch| matches!(ch.kind, drawcraft_doc::NodeKind::Path { .. }))); + } + #[test] fn foreign_svg_and_non_svg_text() { let mut s = session(); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 398872cd..b2e17596 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -13,7 +13,7 @@ mod tooling; use std::sync::Arc; use drawcraft_color::{Color, Paint}; -use drawcraft_doc::{Document, NodeId, Selection}; +use drawcraft_doc::{Document, NodeId, NodeKind, Selection}; use drawcraft_geom::Affine; use drawcraft_tools::{PaintDefaults, Tool}; use serde_json::Value; @@ -130,12 +130,14 @@ impl DocState { } /// Where new art is inserted: the isolation container, else the active layer. pub fn insertion_parent(&self) -> Option { + // Ids are reused after undo (the id counter is part of the document), so a remembered + // layer or isolated group must still be one: a pasted group must never land in a path. if let Some(i) = self.isolation - && self.doc.node(i).is_some() + && self.doc.node(i).is_some_and(|n| matches!(n.kind, NodeKind::Group { .. } | NodeKind::Layer { .. })) { return Some(i); } - self.active_layer.filter(|l| self.doc.node(*l).is_some_and(|n| !n.locked)).or_else(|| self.doc.default_layer()) + self.active_layer.filter(|l| self.doc.node(*l).is_some_and(|n| n.is_layer() && !n.locked)).or_else(|| self.doc.default_layer()) } }