mirror of
https://github.com/storytold/photocraft.git
synced 2026-10-14 19:26:51 -04:00
Duplicating a layer reported no damage, so the canvas recomposited the whole document after every Duplicate Layer (P8/P9). The copy itself was already cheap (tiles are shared Arcs). Report the copy's change bounds instead, through the step_damage walk extended to bounded additions: self-contained added layers (not an adjustment, not a new clipping base, no video or excluded channels) contribute their change bounds; anything else still recomposites everything. Co-authored-by: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
51f0a287b1
commit
3e90c81e3c
2 files changed
+89
-3
No files matched your search
@@ -560,6 +560,7 @@ fn build() -> Vec<CommandSpec> {
|
||||
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<CommandSpec> {
|
||||
*active = Some(nid);
|
||||
Ok(nid)
|
||||
})?;
|
||||
crate::layer_multi_cmds::note_insert(s, &before);
|
||||
Ok(json!({ "layer": nid.0 }))
|
||||
}
|
||||
),
|
||||
|
||||
@@ -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<Rect> {
|
||||
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<Rect> {
|
||||
damage_between(before, after, true)
|
||||
}
|
||||
|
||||
fn damage_between(a: &Document, b: &Document, bounded_additions: bool) -> Option<Rect> {
|
||||
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<Rect> {
|
||||
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<Rect> {
|
||||
}
|
||||
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<Rect> {
|
||||
}
|
||||
}
|
||||
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<Rect> {
|
||||
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<Rect> {
|
||||
@@ -1216,6 +1262,7 @@ pub fn delete_selected(s: &mut Session) -> Result<Value> {
|
||||
/// this path, #2777).
|
||||
pub fn duplicate_selected(s: &mut Session, in_place: bool, label: &str) -> Result<Value> {
|
||||
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::<Vec<_>>();
|
||||
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::<Vec<_>>();
|
||||
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);
|
||||
|
||||
Reference in new issue
Block a user