diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index 8cee83468..9692dba4a 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -560,6 +560,7 @@ fn build() -> Vec { return crate::layer_multi_cmds::duplicate_selected(s, in_place, "Duplicate Layers"); } let id = layer_param(s, p)?; + let before = s.active().ok_or(EngineError::NoDocument)?.doc.clone(); let nid = s.edit("Duplicate Layer", |doc, active| { let dup = layer_copy(doc, id)?; let nid = doc.insert_above(Some(id), dup); @@ -569,6 +570,7 @@ fn build() -> Vec { *active = Some(nid); Ok(nid) })?; + crate::layer_multi_cmds::note_insert(s, &before); Ok(json!({ "layer": nid.0 })) } ), diff --git a/crates/engine/src/layer_multi_cmds.rs b/crates/engine/src/layer_multi_cmds.rs index 4b9aeffa2..27424e41b 100644 --- a/crates/engine/src/layer_multi_cmds.rs +++ b/crates/engine/src/layer_multi_cmds.rs @@ -648,10 +648,23 @@ pub fn same_pixels(a: &Document, b: &Document) -> bool { /// (clipping, excluded channels — what `layer.setProps` already reports as unbounded), and /// video layers. pub fn step_damage(a: &Document, b: &Document) -> Option { + damage_between(a, b, false) +} + +/// [`step_damage`] for an edit from `before` to `after` that may also have added layers which +/// change pixels only within their own bounds (see [`added_bounds`]), as Duplicate Layer does: +/// each contributes its change bounds instead of making the whole canvas recomposite. Inserting a +/// copy of a 12 MP layer used to recomposite every layer of the document (#2883, P8/P9 in +/// `perf/budgets.toml`). +pub(crate) fn insert_damage(before: &Document, after: &Document) -> Option { + damage_between(before, after, true) +} + +fn damage_between(a: &Document, b: &Document, bounded_additions: bool) -> Option { if !doc_pixels_equal(a, b) { return None; } - fn walk(xs: &[Layer], ys: &[Layer], a: &Document, b: &Document, out: &mut Rect) -> bool { + fn walk(xs: &[Layer], ys: &[Layer], a: &Document, b: &Document, added: bool, out: &mut Rect) -> bool { // Pair the layers by id, stepping over layers only one side has when they draw nothing // (a new empty layer or group, #1771): adding or deleting one changes no pixels. let (mut i, mut j) = (0, 0); @@ -663,6 +676,15 @@ pub fn step_damage(a: &Document, b: &Document) -> Option { j += 1; continue; } + _ if added && ys.get(j).is_some_and(|y| xs.iter().all(|x| x.id != y.id)) => { + // A layer only `b` has: bounded when it is self-contained. + let Some(r) = added_bounds(ys, j, b.bounds()) else { return false }; + if !r.is_empty() { + *out = if out.is_empty() { r } else { out.union(&r) }; + } + j += 1; + continue; + } _ if inert_at(xs, i) => { i += 1; continue; @@ -688,7 +710,7 @@ pub fn step_damage(a: &Document, b: &Document) -> Option { } match (x.children(), y.children()) { (Some(xc), Some(yc)) => { - if !walk(xc, yc, a, b, out) { + if !walk(xc, yc, a, b, added, out) { return false; } } @@ -698,7 +720,24 @@ pub fn step_damage(a: &Document, b: &Document) -> Option { } } let mut out = Rect::EMPTY; - if walk(&a.layers, &b.layers, a, b, &mut out) { Some(out) } else { None } + if walk(&a.layers, &b.layers, a, b, bounded_additions, &mut out) { Some(out) } else { None } +} + +/// The pixels adding `layers[k]` can change, when that is bounded: its change bounds +/// (`photocraft_compose::change_bounds`: not for an adjustment, or a pass-through group holding +/// one), provided it doesn't become the clipping base of clipped layers above it, and neither it +/// nor anything inside it is a video layer or excludes channels (both reach outside the layer's +/// bounds, see [`step_damage`]). +fn added_bounds(layers: &[Layer], k: usize, canvas: Rect) -> Option { + fn contained(l: &Layer, depth: usize) -> bool { + depth < 256 && l.video.is_none() && l.excluded_channels == 0 && l.children().is_none_or(|c| c.iter().all(|c| contained(c, depth + 1))) + } + let l = layers.get(k)?; + let new_base = !l.clipped && layers.get(k + 1).is_some_and(|above| above.clipped); + if new_base || !contained(l, 0) { + return None; + } + photocraft_compose::change_bounds(l, canvas) } /// Whether `layers[k]` draws nothing wherever it sits, so adding or removing it leaves the @@ -736,6 +775,13 @@ pub(crate) fn note_inert_insert(s: &mut Session, id: LayerId) { } } +/// Report the pixels an edit from `before` that added layers changed ([`insert_damage`]), so +/// the canvas recomposites only where the new layers draw instead of the whole document. +pub(crate) fn note_insert(s: &mut Session, before: &Document) { + let Some(st) = s.active_mut() else { return }; + st.last_damage = insert_damage(before, &st.doc); +} + /// Content bounds used by Align/Distribute: the layer's pixels (type and shape layers use their /// rendered pixels), or the union of a group's children. Fill and adjustment layers have none. pub fn layer_bounds(l: &Layer) -> Option { @@ -1216,6 +1262,7 @@ pub fn delete_selected(s: &mut Session) -> Result { /// this path, #2777). pub fn duplicate_selected(s: &mut Session, in_place: bool, label: &str) -> Result { let sel = selected(s); + let before = s.active().ok_or(EngineError::NoDocument)?.doc.clone(); let old_active = s.active().and_then(|d| d.active_layer); let (copies, active) = s.edit(label, |doc, active| { let mut copies = Vec::new(); @@ -1237,6 +1284,7 @@ pub fn duplicate_selected(s: &mut Session, in_place: bool, label: &str) -> Resul *active = a; Ok((copies, a)) })?; + note_insert(s, &before); let out = copies.iter().map(|l| l.0).collect::>(); reselect(s, copies, active); Ok(json!({"layers": out, "layer": active.map(|l| l.0)})) @@ -1495,6 +1543,42 @@ mod tests { assert_eq!(step_damage(&plain, &before), Some(Rect::EMPTY)); } + /// #2883 (P8/P9): a duplicate shares the source's tiles (no pixel copy) and recomposites only + /// where it draws; undo brings back exactly the old pixels. A copy that becomes the clipping + /// base of the layers above still recomposites everything. + #[test] + fn duplicate_shares_tiles_and_damages_only_the_copy() { + let render = |s: &Session| photocraft_compose::render(doc(s), doc(s).bounds()); + let tiles = + |s: &Session, id: LayerId| doc(s).layer(id).unwrap().surface().unwrap().tiles().map(|(c, t)| (*c, std::sync::Arc::as_ptr(t))).collect::>(); + for depth in [8, 16, 32] { + let mut s = session(depth); + let a = rect_layer(&mut s, Rect::new(10, 10, 20, 20)); + s.execute("layer.setProps", json!({"layer": a.0, "opacity": 0.5})).unwrap(); + let shown = render(&s); + let copy = LayerId(s.execute("layer.duplicate", json!({"layer": a.0})).unwrap()["layer"].as_u64().unwrap()); + assert_eq!(damage(&s), Some(Rect::new(10, 10, 20, 20)), "{depth}-bit: only where the copy draws"); + assert!(!tiles(&s, a).is_empty()); + assert_eq!(tiles(&s, copy), tiles(&s, a), "{depth}-bit: the copy shares the source's tiles"); + let now = render(&s); + assert_ne!(now, shown, "the half-transparent copy darkens its area"); + let w = shown.rect.width() as usize; + for (i, (p, q)) in shown.px.iter().zip(&now.px).enumerate() { + if !Rect::new(10, 10, 20, 20).contains((i % w) as i32, (i / w) as i32) { + assert_eq!(p, q, "{depth}-bit: pixel {i} outside the damage changed"); + } + } + s.execute("edit.undo", json!({})).unwrap(); + assert!(doc(&s).layer(copy).is_none()); + assert_eq!(render(&s), shown); + // A clipping base's copy goes between it and its clipped layer and becomes their base. + let clipped = rect_layer(&mut s, Rect::new(0, 0, 50, 50)); + s.execute("layer.setProps", json!({"layer": clipped.0, "clipped": true})).unwrap(); + s.execute("layer.duplicate", json!({"layer": a.0})).unwrap(); + assert_eq!(damage(&s), None, "{depth}-bit: a new clipping base"); + } + } + #[test] fn the_move_preview_matches_the_move() { let mut s = session(8);