perf: edits, undo and redo recomposite only where added or removed layers draw (#3847)

Session::edit reported damage only for edits that changed no pixels (same_pixels) and left
every other edit to recomposite the whole document unless the command reported its own
damage. Duplicate and paste do (#2883, #2901), but Fill, Invert, filters, Delete Layer and
the undo/redo of a duplicate, a paste or a delete all recomposited everything.

- Session::edit now reports step_damage(before, after), the same bound undo and redo use.
- step_damage bounds layers only one side has in both directions, so a removed layer counts
  where it drew like an added one; the bounded_additions flag and insert_damage go, and
  added_bounds becomes one_sided_bounds. A layer that is (or was) the clipping base of
  clipped layers above it, an adjustment, a video layer or one excluding channels still
  recomposites everything (#3090).
- The subject-mask cache is dropped only when the step can have changed pixels.

perf_scenarios on the layered 20 MP document, edits on a 1 MP layer (p50, RTX 3090, Vulkan):
delete layer 9.1 -> 1.4 ms, undo a duplicate 9.3 -> 1.6 ms, fill a 500 px square
17.4 -> 5.5 ms, Invert 16.7 -> 9.2 ms. Duplicate and paste are unchanged (already bounded).

Tests: duplicate_paste_and_delete_report_where_the_layer_draws checks that each step, its
undo and its redo report the same bounded damage and that no pixel outside it changes.
new_layers_that_change_clipping_recomposite_everything (renamed) now expects bounded damage
for a painted or empty new layer and None for a new clipping base or an adjustment; deleting
and restoring a layer reports its bounds.
This commit is contained in:
Jakub 2026-10-12 14:47:05 +02:00 committed by GitHub
parent 05c4cd685e
commit 286ff73533
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 115 additions and 61 deletions

View File

@ -44,7 +44,11 @@ fn edits_preserve_mask_pixels_and_layer_at_all_depths_with_undo_redo() {
assert_eq!(*s.active().unwrap().doc, expected, "only density and feather change");
assert_eq!(s.active().unwrap().active_layer, Some(background));
assert_eq!(s.active().unwrap().history.entries().len(), count + 1);
assert!(s.active().unwrap().last_damage.is_none(), "properties invalidate the composite");
// Properties invalidate the composite as far as their undo would (the masked layer is
// empty here; the adjustment reaches everything).
let damage = s.active().unwrap().last_damage;
assert_eq!(damage, crate::layer_multi_cmds::step_damage(&before, &s.active().unwrap().doc));
assert_eq!(damage.is_none(), adjustment);
s.execute("edit.undo", json!({})).unwrap();
assert_eq!(*s.active().unwrap().doc, *before);
s.execute("edit.redo", json!({})).unwrap();

View File

@ -667,35 +667,31 @@ pub fn same_pixels(a: &Document, b: &Document) -> bool {
}
/// Pixels a single history step can have changed between `a` and `b` (the documents on either
/// side of an undo or redo): the union of the change bounds of every layer whose pixels
/// differ, so undoing a local edit recomposites only that area instead of the whole canvas.
/// `None` (composite everything, as before) when the step touched anything the layer walk
/// cannot bound: document-level fields the compositor reads (canvas size, mode, depth,
/// profile, global light, patterns, channels, palettes), the layer structure (order,
/// additions, removals, kind changes; adding or removing a layer that draws nothing, see
/// [`inert_at`], is no change), layer properties that reach outside their own bounds
/// (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
/// side of an edit, its undo or its redo): the union of the change bounds of every layer whose
/// pixels differ, so a local edit recomposites only that area instead of the whole canvas. A
/// layer only one side has (a duplicate, a paste, a deleted layer) counts where it draws (see
/// [`one_sided_bounds`]); one that draws nothing ([`inert_at`]) changes nothing. `None`
/// (composite everything) when the step touched anything the layer walk cannot bound:
/// document-level fields the compositor reads (canvas size, mode, depth, profile, global light,
/// patterns, channels, palettes), layer order and kind changes, layers added or removed that
/// reach outside their own bounds, layer properties that do (clipping, excluded channels —
/// what `layer.setProps` already reports as unbounded), and video layers. 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> {
pub fn step_damage(a: &Document, b: &Document) -> Option<Rect> {
if !doc_pixels_equal(a, b) {
return None;
}
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.
fn walk(xs: &[Layer], ys: &[Layer], a: &Document, b: &Document, out: &mut Rect) -> bool {
// Pair the layers by id, stepping over layers only one side has: one that draws nothing
// changes no pixels (a new empty layer or group, #1771), any other only where it draws.
let one_sided = |layers: &[Layer], k: usize, canvas: Rect, out: &mut Rect| {
let Some(r) = one_sided_bounds(layers, k, canvas) else { return false };
if !r.is_empty() {
*out = if out.is_empty() { r } else { out.union(&r) };
}
true
};
let (mut i, mut j) = (0, 0);
loop {
let (x, y) = match (xs.get(i), ys.get(j)) {
@ -705,19 +701,25 @@ fn damage_between(a: &Document, b: &Document, bounded_additions: bool) -> 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;
}
(_, Some(y)) if xs.iter().all(|x| x.id != y.id) => {
if !one_sided(ys, j, b.bounds(), out) {
return false;
}
j += 1;
continue;
}
(Some(x), _) if ys.iter().all(|y| y.id != x.id) => {
if !one_sided(xs, i, a.bounds(), out) {
return false;
}
i += 1;
continue;
}
// Both sides have the layer, in another order.
_ => return false,
};
i += 1;
@ -739,7 +741,7 @@ fn damage_between(a: &Document, b: &Document, bounded_additions: bool) -> Option
}
match (x.children(), y.children()) {
(Some(xc), Some(yc)) => {
if !walk(xc, yc, a, b, added, out) {
if !walk(xc, yc, a, b, out) {
return false;
}
}
@ -749,15 +751,15 @@ fn damage_between(a: &Document, b: &Document, bounded_additions: bool) -> Option
}
}
let mut out = Rect::EMPTY;
if walk(&a.layers, &b.layers, a, b, bounded_additions, &mut out) { Some(out) } else { None }
if walk(&a.layers, &b.layers, a, b, &mut out) { Some(out) } else { None }
}
/// The pixels adding `layers[k]` can change, when that is bounded: its change bounds
/// The pixels adding or removing `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> {
/// one), provided it isn't the clipping base of clipped layers above it (they would change base),
/// 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 one_sided_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)))
}
@ -804,11 +806,12 @@ 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.
/// Report the pixels an edit from `before` that added layers changed ([`step_damage`]), for
/// commands made of more than one edit, 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);
st.last_damage = step_damage(before, &st.doc);
}
/// Content bounds used by Align/Distribute: the layer's pixels (type and shape layers use their
@ -1588,7 +1591,7 @@ mod tests {
}
#[test]
fn new_layers_that_change_pixels_still_recomposite_everything() {
fn new_layers_that_change_clipping_recomposite_everything() {
let mut s = session(8);
let base = rect_layer(&mut s, Rect::new(10, 10, 20, 20));
let clipped = rect_layer(&mut s, Rect::new(0, 0, 100, 80));
@ -1612,21 +1615,33 @@ mod tests {
assert_eq!(step_damage(&before, &split), None);
assert_eq!(step_damage(&split, &before), None);
s.execute("edit.undo", json!({})).unwrap();
// A step that adds a layer with pixels (Layer via Copy, paste…) is not inert either.
// A step that adds a layer with pixels (Layer via Copy, paste…) changes where it draws.
let before = doc(&s).clone();
let mut after = before.clone();
let mut l = Layer::raster("painted", after.pixel_format());
l.surface_mut().unwrap().fill_rect(Rect::new(1, 1, 2, 2), &[0.0, 1.0, 0.0, 1.0]);
after.insert_above(None, l);
assert_eq!(step_damage(&before, &after), None);
assert_eq!(step_damage(&after, &before), None);
// Nor, conservatively, an empty layer in another blend mode.
assert_eq!(step_damage(&before, &after), Some(Rect::new(1, 1, 2, 2)));
assert_eq!(step_damage(&after, &before), Some(Rect::new(1, 1, 2, 2)));
// Unless it becomes the base of clipped layers above it.
let mut based = before.clone();
let mut l = Layer::raster("base", based.pixel_format());
l.surface_mut().unwrap().fill_rect(Rect::new(1, 1, 2, 2), &[0.0, 1.0, 0.0, 1.0]);
let under = based.layers.iter().position(|l| l.id == clipped).unwrap();
based.layers.insert(under, l);
assert_eq!(step_damage(&before, &based), None);
assert_eq!(step_damage(&based, &before), None);
// An adjustment reaches every pixel below it.
let mut adjusted = before.clone();
adjusted.insert_above(None, Layer::new("inv", LayerContent::Adjustment(photocraft_doc::Adjustment::Invert)));
assert_eq!(step_damage(&before, &adjusted), None);
assert_eq!(step_damage(&adjusted, &before), None);
// Empty layers draw nothing, in any blend mode.
let mut styled = before.clone();
let mut l = Layer::raster("styled", styled.pixel_format());
l.blend = photocraft_doc::BlendMode::Multiply;
styled.insert_above(None, l);
assert_eq!(step_damage(&before, &styled), None);
// An empty layer in Normal mode is.
assert_eq!(step_damage(&before, &styled), Some(Rect::EMPTY));
let mut plain = before.clone();
plain.insert_above(None, Layer::raster("plain", plain.pixel_format()));
assert_eq!(step_damage(&before, &plain), Some(Rect::EMPTY));
@ -1669,6 +1684,40 @@ mod tests {
}
}
/// The composite of `after` equals `before`'s outside `damage`.
fn unchanged_outside(before: &Document, after: &Document, damage: Rect) -> bool {
let (x, y) = (photocraft_compose::render(before, before.bounds()), photocraft_compose::render(after, after.bounds()));
let w = before.bounds().width() as i32;
x.px.iter().zip(&y.px).enumerate().all(|(i, (p, q))| p == q || damage.contains(i as i32 % w, i as i32 / w))
}
/// Duplicate, paste and delete, and their undo and redo, recomposite only where the layer
/// draws; undoing a duplicate used to recomposite the whole document.
#[test]
fn duplicate_paste_and_delete_report_where_the_layer_draws() {
let mut s = session(8);
let a = rect_layer(&mut s, Rect::new(10, 10, 20, 20));
let full = doc(&s).bounds();
let step = |s: &mut Session, id: &str, p: serde_json::Value| {
let before = doc(s).clone();
s.execute(id, p).unwrap();
let d = damage(s).unwrap_or_else(|| panic!("{id}: unbounded damage"));
assert!(d.width() < full.width(), "{id}: {d:?}");
assert!(unchanged_outside(&before, doc(s), d), "{id}: pixels changed outside {d:?}");
s.execute("edit.undo", json!({})).unwrap();
assert_eq!(damage(s), Some(d), "undo {id}");
s.execute("edit.redo", json!({})).unwrap();
assert_eq!(damage(s), Some(d), "redo {id}");
};
step(&mut s, "layer.duplicate", json!({"layer": a.0}));
s.execute("layer.select", json!({"layer": a.0})).unwrap();
s.execute("select.rect", json!({"x": 12, "y": 12, "width": 4, "height": 4})).unwrap();
s.execute("edit.copy", json!({})).unwrap();
s.execute("select.deselect", json!({})).unwrap();
step(&mut s, "edit.paste", json!({}));
step(&mut s, "layer.delete", json!({"layer": a.0}));
}
#[test]
fn the_move_preview_matches_the_move() {
let mut s = session(8);
@ -2205,12 +2254,12 @@ mod tests {
assert_ne!(doc(&s).size, photocraft_doc::Size::new(100, 80));
s.undo();
assert_eq!(damage(&s), None, "a size change recomposites the whole canvas");
// A structural change (a layer removed and restored) neither.
// A layer removed and restored changes only where it draws.
let b = rect_layer(&mut s, Rect::new(0, 0, 5, 5));
select_all(&mut s, &[b]);
s.execute("layer.delete", json!({})).unwrap();
s.undo();
assert_eq!(damage(&s), None);
assert_eq!(damage(&s), Some(Rect::new(0, 0, 5, 5)));
}
#[test]

View File

@ -644,11 +644,12 @@ impl Session {
st.color_samplers.retain(|p| bounds.contains(p[0].floor() as i32, p[1].floor() as i32));
samplers_removed = n - st.color_samplers.len();
}
// An edit that changes no pixels (a selection, guides, a layer's name) leaves the canvas
// as it is; anything else recomposites the whole document unless the command reports
// its own damage after this. A new lasso selection on a large document used to cost a
// full recomposite (seconds on the CPU path, #1773).
let unchanged = layer_multi_cmds::same_pixels(&before, &st.doc);
// The pixels the edit can have changed, as its undo would report them: nothing for a
// selection, guides or a layer's name (a new lasso selection used to recomposite a large
// document, seconds on the CPU path, #1773), a new or deleted layer's bounds (a
// duplicate or a paste), and everything when that can't be bounded. A command may still
// report its own, tighter damage after this.
let damage = layer_multi_cmds::step_damage(&before, &st.doc);
let key = self.coalesce_request.clone();
let st = self.active_mut().ok_or(EngineError::NoDocument)?;
let layers = st.layer_target();
@ -661,9 +662,9 @@ impl Session {
}
st.coalesce = key;
st.revision += 1;
st.last_damage = unchanged.then_some(photocraft_geom::Rect::EMPTY);
st.last_damage = damage;
self.samplers_removed += samplers_removed;
if !unchanged {
if damage.is_none_or(|d| !d.is_empty()) {
// Free the mask early; a stale entry would only miss (it is keyed by pixels).
self.subject_cache = Default::default();
}