Fix pasted shape group bounds and effects across documents (#8992)

* fix(clipboard): render copied shapes in destination canvas space

When a copied shape group is placed beyond its source canvas, rebuild its vector caches against the destination canvas and pixel format. Previously a centered paste into a larger document could clip the caches to an empty source-sized frame, leaving no transform bounds and incorrect effects until a point edit refreshed each shape.

Restore missing shape caches and refresh source-clipped caches when canvas dimensions change. Cover Paste and Copy to Document, nested group contents, 8/16-bit destinations, rendered cache equality, transform bounds and movement Undo.

* fix(clipboard): measure shape placement before destination clipping

Restore missing source-space shape caches before measuring copy bounds, then adapt and translate for the destination. Re-render empty caches when shapes move back onto the canvas instead of shifting an empty raster.

Add focused integration coverage for 32 combinations of partial and full destination clipping, grouped shapes, missing caches, 8/16-bit output, centered and point placement, and Undo/Redo. Clippy and WASM checks pass.

---------

Co-authored-by: kotulsky <7805437+kotulsky@users.noreply.github.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
kotulsky
2026-10-23 17:54:26 -04:00
committed by GitHub
co-authored by kotulsky Brandon Thomas
parent 5004d96acb
commit 5750ed40f9
5 changed files with 177 additions and 8 deletions
+73 -1
View File
@@ -416,7 +416,7 @@ fn paste_layers(s: &mut Session, layers: &Document, dx: i32, dy: i32) -> Result<
*l = l.duplicate();
}
crate::layer_copy_cmds::adapt_copies(s, &mut copies, &d.doc)?;
crate::layer_copy_cmds::translate_copies(&mut copies, dx, dy);
crate::layer_copy_cmds::translate_copies(&mut copies, &d.doc, dx, dy);
let patterns: Vec<_> = copies.patterns.iter().filter(|pat| !d.doc.patterns.iter().any(|q| q.id == pat.id)).cloned().collect();
// A copied smart object may name a PSD-embedded linked file; land its `lnk` block too (#3584).
let mut linked = Vec::new();
@@ -1932,6 +1932,78 @@ mod tests {
assert_eq!(shape.surface().map(|c| c.format().sample), Some(s.active().unwrap().doc.depth), "converted to 16 bits");
}
#[test]
fn cross_document_group_paste_renders_shapes_on_destination_canvas() {
for (copy_to_document, depth) in [(false, 8), (true, 16)] {
let mut s = Session::new();
s.execute("file.new", json!({"width":100,"height":100})).unwrap();
let mut ids = Vec::new();
for x in [10, 50] {
s.execute("shape.create", json!({"kind":"rect","rect":[x,20,20,30],"fill":"#ff0000"})).unwrap();
s.execute("layer.layerStyle.dropShadow", json!({"size":4,"distance":3})).unwrap();
ids.push(s.active().unwrap().active_layer.unwrap());
}
s.active_mut().unwrap().selected_layers = ids;
s.execute("layer.groupLayers", json!({})).unwrap();
s.execute("edit.copy", json!({})).unwrap();
s.execute("file.new", json!({"width":800,"height":600,"depth":depth})).unwrap();
if copy_to_document {
s.execute("layer.copyToDocument", json!({"source":0,"document":1,"center":true})).unwrap();
} else {
s.execute("edit.paste", json!({"center":[400,300]})).unwrap();
}
let st = s.active().unwrap();
let group = st.doc.layer(st.active_layer.unwrap()).unwrap();
assert!(!crate::transform_cmds::transform_bounds(&st.doc, group).is_empty());
assert_eq!(group.children().unwrap().len(), 2);
for child in group.children().unwrap() {
let LayerContent::Shape(shape) = &child.content else { panic!("shape") };
let cache = shape.cache.as_ref().unwrap();
assert_eq!(cache.format().sample, st.doc.depth);
assert!(!cache.content_bounds().is_empty(), "destination cache must not be clipped to source canvas");
assert!(cache.content_bounds().x0 > 100);
let fresh = photocraft_vector::render_shape(shape, st.doc.pixel_format(), st.doc.bounds());
assert_eq!(cache, &fresh);
}
let before = st.doc.clone();
s.execute("layer.translate", json!({"dx":10,"dy":10})).unwrap();
assert_ne!(s.active().unwrap().doc, before);
assert!(s.undo());
assert_eq!(s.active().unwrap().doc, before);
}
}
#[test]
fn paste_repairs_uncached_shapes_before_transform_and_effect_rendering() {
let mut s = session();
s.execute("shape.create", json!({"kind":"rect","rect":[10,10,40,20],"fill":"#ff0000"})).unwrap();
s.execute("layer.layerStyle.dropShadow", json!({"size":4,"distance":3})).unwrap();
s.execute("edit.copy", json!({})).unwrap();
let layers = s.clipboard.as_mut().unwrap().layers.as_mut().unwrap();
let LayerContent::Shape(sh) = &mut layers.layers[0].content else { panic!("shape") };
sh.cache = None;
s.execute("edit.pasteSpecial.pasteInPlace", json!({})).unwrap();
let st = s.active().unwrap();
let layer = st.doc.layer(st.active_layer.unwrap()).unwrap();
let LayerContent::Shape(sh) = &layer.content else { panic!("shape") };
assert!(sh.cache.is_some());
assert!(!crate::transform_cmds::transform_bounds(&st.doc, layer).is_empty());
let expected = photocraft_vector::render_shape(sh, st.doc.pixel_format(), st.doc.bounds());
assert_eq!(sh.cache.as_ref().unwrap(), &expected);
let rendered = photocraft_compose::render_layer_in(&st.doc, layer, st.doc.bounds());
let mut fresh_doc = (*st.doc).clone();
let snapshot = fresh_doc.clone();
if let LayerContent::Shape(shape) = &mut fresh_doc.layer_mut(layer.id).unwrap().content {
crate::vector_cmds::refresh_shape(&snapshot, shape);
}
let fresh = photocraft_compose::render_layer_in(&fresh_doc, fresh_doc.layer(layer.id).unwrap(), fresh_doc.bounds());
assert_eq!(rendered.px, fresh.px, "pasted effects match a freshly rebuilt shape");
assert!(s.undo());
assert!(s.redo());
let st = s.active().unwrap();
assert!(st.doc.layer(st.active_layer.unwrap()).unwrap().surface().is_some());
}
/// #2344: fill and adjustment layers have no pixels of their own, so with no selection Copy
/// takes them whole and Paste recreates them, in the same or another document, in one step.
#[test]
+27 -6
View File
@@ -11,7 +11,7 @@
use std::collections::{HashMap, HashSet};
use photocraft_color::ColorMode;
use photocraft_doc::{Document, LayerContent, LayerId, SmartContentsId};
use photocraft_doc::{Document, Layer, LayerContent, LayerId, SmartContentsId};
use photocraft_geom::Rect;
use serde_json::{Value, json};
@@ -59,6 +59,17 @@ fn center(r: Rect) -> [f64; 2] {
[(f64::from(r.x0) + f64::from(r.x1)) / 2.0, (f64::from(r.y0) + f64::from(r.y1)) / 2.0]
}
/// Restore missing shape caches, or refresh every shape for a changed canvas, including groups.
fn prepare_shapes(layers: &mut [Layer], doc: &Document, new_canvas: bool) {
for layer in layers {
match &mut layer.content {
LayerContent::Shape(shape) if new_canvas || shape.cache.is_none() => crate::vector_cmds::refresh_shape(doc, shape),
LayerContent::Group(group) => prepare_shapes(&mut group.children, doc, new_canvas),
_ => {}
}
}
}
/// Readies `copies` (a scratch document of the source's colour holding duplicated layers) for
/// landing in `dest`: smart objects get fresh contents ids, keeping only the sharing within the
/// batch (contents ids belong to a document, so the copies never join objects already in `dest`,
@@ -107,13 +118,20 @@ pub(crate) fn adapt_copies(s: &Session, copies: &mut Document, dest: &Document)
*surf = surf.convert(f);
});
}
let new_canvas = copies.bounds() != dest.bounds();
prepare_shapes(&mut copies.layers, dest, new_canvas);
Ok(())
}
/// Moves every layer of `copies` by whole pixels (content, masks, vectors and smart placement).
pub(crate) fn translate_copies(copies: &mut Document, dx: i32, dy: i32) {
pub(crate) fn translate_copies(copies: &mut Document, destination: &Document, dx: i32, dy: i32) {
if dx != 0 || dy != 0 {
let snapshot = copies.clone();
let mut snapshot = copies.clone();
// Vector caches are clipped and quantized for the destination, not the clipboard's source.
snapshot.size = destination.size;
snapshot.mode = destination.mode;
snapshot.depth = destination.depth;
snapshot.icc_profile = destination.icc_profile.clone();
for l in &mut copies.layers {
crate::commands::translate_layer(&snapshot, l, dx, dy);
crate::vector_cmds::translate_vectors(&snapshot, l, f64::from(dx), f64::from(dy));
@@ -172,11 +190,14 @@ fn copy_to_document(s: &mut Session, p: &Value) -> Result<Value> {
if let (Some(name), [l]) = (p.get("name").and_then(Value::as_str).filter(|n| !n.is_empty()), copies.layers.as_mut_slice()) {
l.name = name.to_string();
}
// Measure in source space: destination refresh may clip a shape before it is moved.
// Missing caches must participate too, including shapes nested inside groups.
prepare_shapes(&mut copies.layers, &sdoc, false);
let bounds = copies.layers.iter().filter_map(crate::layer_multi_cmds::layer_bounds).fold(Rect::EMPTY, |a, b| a.union(&b));
let content = if bounds.is_empty() { sdoc.bounds() } else { bounds };
adapt_copies(s, &mut copies, &ddoc)?;
// Placement: centred on the destination's canvas, centred on a point, or offset from where
// the layers are in the source (default: the same canvas position).
let bounds = copies.layers.iter().filter_map(crate::layer_multi_cmds::layer_bounds).fold(Rect::EMPTY, |a, b| a.union(&b));
let content = if bounds.is_empty() { sdoc.bounds() } else { bounds };
let (dx, dy) = if p.get("center").and_then(Value::as_bool).unwrap_or(false) {
let (c, d) = (center(content), center(ddoc.bounds()));
(d[0] - c[0], d[1] - c[1])
@@ -187,7 +208,7 @@ fn copy_to_document(s: &mut Session, p: &Value) -> Result<Value> {
point(p, "offset")?.map_or((0.0, 0.0), |o| (o[0], o[1]))
};
let (dx, dy) = (dx.round() as i32, dy.round() as i32);
translate_copies(&mut copies, dx, dy);
translate_copies(&mut copies, &ddoc, dx, dy);
let patterns: Vec<_> = sdoc.patterns.iter().filter(|pat| !ddoc.patterns.iter().any(|d| d.id == pat.id)).cloned().collect();
s.set_active(dest);
let label = if ids.len() == 1 { "Duplicate Layer" } else { "Duplicate Layers" };
+2 -1
View File
@@ -506,7 +506,8 @@ pub(crate) fn shift_shape(doc: &Document, sh: &mut ShapeLayer, dx: f64, dy: f64)
let canvas = doc.bounds();
let inside = sh.cache.as_ref().is_some_and(|c| {
let b = photocraft_compose::bounds::content_bounds(c);
b.is_empty() || (b.x0 > canvas.x0 && b.y0 > canvas.y0 && b.x1 < canvas.x1 && b.y1 < canvas.y1)
// An empty cache can be an off-canvas shape moving back into view.
!b.is_empty() && b.x0 > canvas.x0 && b.y0 > canvas.y0 && b.x1 < canvas.x1 && b.y1 < canvas.y1
});
let stroke_pad = sh.stroke.as_ref().filter(|s| s.opacity > 0.0).map_or(0.0, |s| f64::from(s.width.ceil()));
let stays_inside = sh.path.control_bounds().is_some_and(|(x0, y0, x1, y1)| {
@@ -0,0 +1,74 @@
//! Copy placement must use source-space bounds before destination cache clipping.
use photocraft_doc::LayerContent;
use photocraft_engine::Session;
use serde_json::json;
/// Cover source clipping, groups, missing caches, destination depth, placement and Undo/Redo.
#[test]
fn copying_shapes_to_smaller_documents_preserves_requested_center() {
for x in [90, 700] {
for grouped in [false, true] {
for missing_cache in [false, true] {
for depth in [8, 16] {
for at in [None, Some([40, 60])] {
let mut s = Session::new();
s.execute("file.new", json!({"width":800,"height":600,"background":"transparent"})).unwrap();
let mut ids = Vec::new();
for offset in if grouped { vec![0, 30] } else { vec![0] } {
s.execute("shape.create", json!({"kind":"rect","rect":[x+offset,20,20,20],"fill":"#ff0000"})).unwrap();
s.execute("layer.layerStyle.dropShadow", json!({"size":4,"distance":3})).unwrap();
ids.push(s.active().unwrap().active_layer.unwrap());
}
if grouped {
s.active_mut().unwrap().selected_layers = ids.clone();
s.execute("layer.groupLayers", json!({})).unwrap();
}
if missing_cache {
s.edit("drop cached shapes", |doc, _| {
for id in &ids {
let LayerContent::Shape(shape) = &mut doc.layer_mut(*id).unwrap().content else { panic!("shape") };
shape.cache = None;
}
Ok(())
})
.unwrap();
}
s.execute("file.new", json!({"width":100,"height":100,"depth":depth,"background":"transparent"})).unwrap();
let before = s.active().unwrap().doc.clone();
let mut params = json!({"source":0,"document":1,"center":true});
if let Some(at) = at {
params["center"] = json!(false);
params["at"] = json!(at);
}
let result = s.execute("layer.copyToDocument", params).unwrap();
let target = at.unwrap_or([50, 50]);
let dx = target[0] - (x + if grouped { 25 } else { 10 });
let dy = target[1] - 30;
assert_eq!(result["offset"], json!([dx, dy]), "x={x}, grouped={grouped}, cache missing={missing_cache}");
let st = s.active().unwrap();
let shapes: Vec<_> = st
.doc
.walk()
.into_iter()
.filter_map(|(_, _, layer)| if let LayerContent::Shape(shape) = &layer.content { Some(shape) } else { None })
.collect();
assert_eq!(shapes.len(), if grouped { 2 } else { 1 });
for shape in shapes {
let fresh = photocraft_vector::render_shape(shape, st.doc.pixel_format(), st.doc.bounds());
assert!(
shape.cache.as_ref() == Some(&fresh),
"cache mismatch: x={x}, grouped={grouped}, cache missing={missing_cache}, depth={depth}, at={at:?}"
);
assert!(!fresh.content_bounds().is_empty());
}
let after = st.doc.clone();
assert!(s.undo());
assert_eq!(s.active().unwrap().doc, before);
assert!(s.redo());
assert_eq!(s.active().unwrap().doc, after);
}
}
}
}
}
}
+1
View File
@@ -14,6 +14,7 @@ mod clipping_semantics;
mod close_document;
mod command_lookup;
mod convert_keeps_references;
mod copy_shape_placement;
mod corpus_atn;
mod crop_content_aware;
mod current_affinity_artboards;