perf(undo): record entity deltas instead of cloning the whole document

Frequent entity-only edits (MOVE/ROTATE/SCALE/MIRROR/ALIGN, COPY/ARRAY,
draw-add, ERASE) captured a full ~800k-entity CadDocument clone (~46ms) on
every edit — the last big per-edit cost after the incremental-render work.

They now record only the touched entities' before/after images:

- Scene self-records through the five mutation primitives when a recording is
  open (UndoRecording + is_recording_undo guard, zero cost off-path).
- apply_entity_delta installs one side per handle (in-place / re-insert with
  the original handle / remove) plus a per-handle cache reseed, then a single
  bump_entities so the incremental caches patch instead of a full re-tess —
  undo/redo of a small edit is fast too, not just the edit.
- HistorySnapshot is now Full | Delta; a Delta is self-inverting and moves
  between the undo/redo stacks without re-capturing. Commands that touch
  non-entity state (new layer, group, *D block, viewport) fall back to the
  full snapshot via per-command predicates, and a poison flag debug-asserts
  any mis-scope.

remove_entity leaves the handle dangling in its owner block record's
entity_handles (harmless on its own, a lookup miss is skipped), so a
re-insert strips stale entries first — otherwise add_entity would list the
handle twice and the DXF/DWG writer would emit the entity twice.

Round-trip tests cover transform/erase/add/copy (do->undo->redo equality,
handle preservation, no duplication).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
Hakan SevenandClaude Opus 4.8 committed 2026-07-15 11:11:17 +03:00
1 parent aea1fd2caa
commit 1490fe4ea9
9 files changed
+761 -26

No files matched your search

+85
View File
@@ -106,6 +106,12 @@ impl Scene {
// (e.g. a plugin-supplied layer) so it survives a DWG save instead of
// collapsing to layer 0 in the reopened file (#252).
let layer = entity.common().layer.clone();
// Delta-undo poison inputs (captured before the mutations below): an
// add that also creates a new layer, adds a block, or inserts an image
// definition mutates non-entity state a pure-entity delta can't undo.
let creates_layer =
self.is_recording_undo() && !layer.trim().is_empty() && !self.document.layers.contains(&layer);
let is_image = matches!(&entity, EntityType::RasterImage(_));
self.ensure_layer(&layer);
// Route to the correct block based on current editing mode:
@@ -135,6 +141,16 @@ impl Scene {
if let Some(model) = mesh_seed {
self.meshes.insert(handle, model);
}
// Delta-undo: the new handle's before-image is "nothing" (it did not
// exist). Poison the recording if this add also mutated non-entity
// state (a new layer / block / image definition) so the app knows a
// pure-entity delta would be incomplete.
if self.is_recording_undo() {
self.record_undo_before(handle, None);
if creates_layer || affects_blocks || is_image {
self.poison_undo_recording();
}
}
if affects_blocks {
self.bump_geometry();
} else {
@@ -210,6 +226,9 @@ impl Scene {
// so the edited entity keeps that layer on save instead of collapsing
// to layer 0 in the reopened file (#252).
let new_layer = entity.common().layer.clone();
let creates_layer = self.is_recording_undo()
&& !new_layer.trim().is_empty()
&& !self.document.layers.contains(&new_layer);
self.ensure_layer(&new_layer);
// Rebuild the derived-model seeds from the new entity (as add_entity).
@@ -239,6 +258,17 @@ impl Scene {
None
};
// Delta-undo: capture the entity's pre-edit image (before the slot is
// overwritten) so an undo can restore it, and poison if this replace
// also created a layer or crossed a block boundary.
if self.is_recording_undo() {
let before = self.document.get_entity(handle).cloned();
self.record_undo_before(handle, before);
if creates_layer || affects_blocks {
self.poison_undo_recording();
}
}
// Write the new entity into the live slot.
let Some(slot) = self.document.get_entity_mut(handle) else {
return false;
@@ -273,6 +303,61 @@ impl Scene {
true
}
/// Rebuild the per-entity derived caches (hatch fill / raster image / solid
/// mesh) for a single handle from whatever entity currently lives at it —
/// or drop them all if the handle is now absent. Mirrors the reseed block in
/// [`Scene::update_entity`]; used by delta-undo when it re-applies an
/// entity's before / after image so the fills and meshes follow.
pub(crate) fn reseed_derived_caches(&mut self, handle: Handle) {
let (hatch_seed, image_seed, mesh_seed) = match self.document.get_entity(handle) {
None => (None, None, None),
Some(entity) => {
let hatch_seed = if let EntityType::Hatch(dxf) = entity {
let color = self.render_style(entity).0;
Self::hatch_model_from_dxf(dxf, color)
} else if let EntityType::Solid(solid) = entity {
let color = self.render_style(entity).0;
Some(Self::solid_hatch_model(solid, color))
} else {
None
};
let image_seed = if let EntityType::RasterImage(img) = entity {
ImageModel::from_raster_image(img)
} else {
None
};
let facet_res = self.document.header.facet_resolution;
let mesh_seed = if matches!(
entity,
EntityType::Solid3D(_)
| EntityType::Region(_)
| EntityType::Body(_)
| EntityType::Surface(_)
) {
let color = self.render_style(entity).0;
crate::entities::solid3d::tessellate_volume(entity, color, facet_res)
.map(offset_mesh_lod_set)
} else {
None
};
(hatch_seed, image_seed, mesh_seed)
}
};
self.hatches.remove(&handle);
self.images.remove(&handle);
self.meshes.remove(&handle);
self.solid_models.remove(&handle);
if let Some(model) = hatch_seed {
self.hatches.insert(handle, model);
}
if let Some(model) = image_seed {
self.images.insert(handle, model);
}
if let Some(model) = mesh_seed {
self.meshes.insert(handle, model);
}
}
/// Returns the RGBA color for the given layer name.
pub fn layer_color(&self, layer: &str) -> [f32; 4] {
let layer_entry = self.document.layers.get(layer);
+329
View File
@@ -105,6 +105,48 @@ pub enum ChangeKind {
Modified,
}
/// While an entity-only undoable command runs, this captures the pre-mutation
/// image of every entity the five mutation primitives (add / update / erase /
/// transform / copy) touch, so the app can build a cheap **delta**-undo entry
/// instead of cloning the whole ~800k-entity document (~46 ms). `poisoned` is
/// set when a primitive also mutated non-entity state (a brand-new layer, a
/// group, or a `*D` block record) that a pure-entity delta cannot restore — the
/// app's per-command predicate keeps that from happening, and a debug assertion
/// fires if it ever does.
#[derive(Default)]
pub struct UndoRecording {
/// First-touch before-image per handle (the first write for a handle wins,
/// so repeated touches within one command keep the true pre-command state).
/// A value of `None` marks an entity *added* by the command (no prior
/// state). `order` preserves first-touch order for a deterministic delta.
before: HashMap<Handle, Option<EntityType>>,
order: Vec<Handle>,
poisoned: bool,
}
impl UndoRecording {
/// The command touched non-entity state a pure-entity delta can't restore.
pub fn is_poisoned(&self) -> bool {
self.poisoned
}
/// No entity was recorded (nothing to undo).
pub fn is_empty(&self) -> bool {
self.order.is_empty()
}
/// Consume the recording into `(handle, before-image)` pairs in first-touch
/// order. A `None` before-image means the entity was added by the command.
/// The app pairs each with the entity's current (after) state to build the
/// invertible delta entry.
pub fn into_before_images(mut self) -> Vec<(Handle, Option<EntityType>)> {
self.order
.drain(..)
.map(|h| (h, self.before.remove(&h).flatten()))
.collect()
}
}
/// One geometry mutation's delta record. `changes` names the exact handles that
/// changed; `full` marks a mutation that can't enumerate its handles (undo/redo,
/// file open, style/display change, any bulk op) and forces every consumer that
@@ -1019,6 +1061,12 @@ pub struct Scene {
/// render layer matches `new_gen` against the viewport's content id and
/// `prev_gen` against what the GPU currently holds; a mismatch just rebuilds.
model_wire_gpu_patch: RefCell<Option<(u64, u64, Arc<Vec<(Handle, ChangeKind)>>)>>,
/// Active delta-undo recording, or `None` when no entity-only command is
/// capturing. Populated by the five mutation primitives via
/// [`Scene::record_undo_before`]; consumed by the app (`take_undo_recording`)
/// to build a cheap history delta instead of a full document clone. See
/// [`UndoRecording`].
undo_recording: Option<UndoRecording>,
}
impl Scene {
@@ -1105,6 +1153,7 @@ impl Scene {
geometry_journal_floor: std::cell::Cell::new(0),
resident_patch_hits: std::cell::Cell::new(0),
model_wire_gpu_patch: RefCell::new(None),
undo_recording: None,
}
}
@@ -1342,6 +1391,136 @@ impl Scene {
/// changed — the delta lets every derived cache patch per-handle instead of
/// re-walking the whole drawing. Blocks are untouched (`block_epoch` stays),
/// so this is for top-level entity add / edit / erase, not block-defn edits.
/// Begin capturing before-images for a delta-undo entry. Pair with
/// [`Scene::take_undo_recording`]. Only entity-only commands — whose
/// mutations flow through the five primitives (add / update / erase /
/// transform / copy) and that touch no layers / objects / block records —
/// may use this; anything else must fall back to a full history snapshot.
pub fn begin_undo_recording(&mut self) {
self.undo_recording = Some(UndoRecording::default());
}
/// Whether a delta-undo recording is currently open.
pub fn is_recording_undo(&self) -> bool {
self.undo_recording.is_some()
}
/// Finish and return the open recording (if any) for the app to build a
/// history delta from.
pub fn take_undo_recording(&mut self) -> Option<UndoRecording> {
self.undo_recording.take()
}
/// Record the pre-mutation image of `handle` (first touch wins). `before`
/// is `None` for a freshly added entity. No-op when not recording — the
/// callers guard with [`Scene::is_recording_undo`] so the clone is skipped
/// entirely on the common (no-recording) path.
pub(crate) fn record_undo_before(&mut self, handle: Handle, before: Option<EntityType>) {
if let Some(rec) = self.undo_recording.as_mut() {
if !rec.before.contains_key(&handle) {
rec.order.push(handle);
rec.before.insert(handle, before);
}
}
}
/// Flag the open recording as touching non-entity state (a new layer, a
/// group edit, a `*D` block record) that a pure-entity delta cannot
/// restore. The app's per-command predicate keeps this from firing.
pub(crate) fn poison_undo_recording(&mut self) {
if let Some(rec) = self.undo_recording.as_mut() {
rec.poisoned = true;
}
}
/// Re-apply one side of an entity delta to the document. For each
/// `(handle, before, after)`, install the chosen `target` — `before` when
/// `undo`, `after` on redo: overwrite the entity in place, re-insert it with
/// its original handle, or remove it — reseeding the per-handle derived
/// caches so fills/meshes follow. Returns the exact per-handle change list
/// for the caller to report via [`Scene::bump_entities`]; it does not bump,
/// touch the selection, or rebuild any whole-document cache.
///
/// `remove_entity` leaves a handle dangling in its owner block record's
/// `entity_handles` (harmless — a render/save lookup miss is skipped), so a
/// re-insert's `add_entity` push would make it appear twice and emit the
/// entity twice on save. Every to-be-re-inserted handle is therefore
/// stripped from the block records in one pass first, leaving exactly one
/// entry after the re-insert.
pub(crate) fn apply_entity_delta(
&mut self,
entities: &[(Handle, Option<EntityType>, Option<EntityType>)],
undo: bool,
) -> Vec<(Handle, ChangeKind)> {
let reinsert: HashSet<Handle> = entities
.iter()
.filter_map(|(h, before, after)| {
let target = if undo { before } else { after };
(target.is_some() && self.document.get_entity(*h).is_none()).then_some(*h)
})
.collect();
if !reinsert.is_empty() {
for br in self.document.block_records.iter_mut() {
br.entity_handles.retain(|h| !reinsert.contains(h));
}
}
let mut changes: Vec<(Handle, ChangeKind)> = Vec::with_capacity(entities.len());
for (h, before, after) in entities {
let target = if undo { before } else { after };
let existed = self.document.get_entity(*h).is_some();
match target {
Some(ent) => {
if existed {
if let Some(slot) = self.document.get_entity_mut(*h) {
*slot = ent.clone();
}
changes.push((*h, ChangeKind::Modified));
} else {
// Re-insert with the original handle: the stored image
// keeps it, and document.add_entity honours a preset,
// non-null handle (routing to its owner block record).
let _ = self.document.add_entity(ent.clone());
changes.push((*h, ChangeKind::Added));
}
self.reseed_derived_caches(*h);
}
None => {
if existed {
self.document.remove_entity(*h);
// Drops the now-absent entity's hatch/image/mesh caches.
self.reseed_derived_caches(*h);
changes.push((*h, ChangeKind::Removed));
}
}
}
}
// Integrity net: no re-inserted handle may appear more than once across
// the block records (a duplicate would emit the entity twice on save).
// Debug-only, and skipped entirely when nothing was re-inserted (the
// common transform/property-edit case).
#[cfg(debug_assertions)]
if !reinsert.is_empty() {
let mut counts: HashMap<Handle, u32> = HashMap::default();
for br in self.document.block_records.iter() {
for h in &br.entity_handles {
if reinsert.contains(h) {
*counts.entry(*h).or_default() += 1;
}
}
}
for (h, c) in counts {
debug_assert!(
c <= 1,
"delta re-insert duplicated handle {h:?} in block records ({c}×)"
);
}
}
changes
}
pub fn bump_entities(&mut self, changes: &[(Handle, ChangeKind)]) {
let epoch = GEOMETRY_EPOCH.fetch_add(1, Ordering::Relaxed);
self.geometry_epoch = epoch;
@@ -4988,3 +5167,153 @@ mod journal_tests {
}
}
/// Delta-undo round-trips: a recorded entity-only edit must be exactly
/// invertible (undo restores the pre-edit images) and re-appliable (redo
/// restores the post-edit images), including handle preservation and no
/// duplication of a re-inserted handle in its owning block record.
#[cfg(test)]
mod delta_undo_tests {
use super::*;
use crate::command::EntityTransform;
use acadrust::entities::Line;
use acadrust::types::Vector3;
use glam::DVec3;
fn line(x1: f64, y1: f64, x2: f64, y2: f64) -> EntityType {
EntityType::Line(Line::from_points(
Vector3::new(x1, y1, 0.0),
Vector3::new(x2, y2, 0.0),
))
}
/// Build the delta the way `commit_undo_delta` does: pair each recorded
/// before-image with the entity's current (after) state.
fn build_delta(
scene: &Scene,
rec: UndoRecording,
) -> Vec<(Handle, Option<EntityType>, Option<EntityType>)> {
rec.into_before_images()
.into_iter()
.map(|(h, before)| {
let after = scene.document.get_entity(h).cloned();
(h, before, after)
})
.collect()
}
/// Occurrences of a handle in the *Model_Space block record's entity list.
fn ms_occurrences(scene: &Scene, handle: Handle) -> usize {
scene
.document
.block_records
.get("*Model_Space")
.map(|br| br.entity_handles.iter().filter(|&&x| x == handle).count())
.unwrap_or(0)
}
#[test]
fn transform_delta_round_trips() {
let mut scene = Scene::new();
let h = scene.add_entity(line(0.0, 0.0, 10.0, 0.0));
let orig = scene.document.get_entity(h).cloned().unwrap();
scene.begin_undo_recording();
scene.transform_entities(&[h], &EntityTransform::Translate(DVec3::new(5.0, 3.0, 0.0)));
let rec = scene.take_undo_recording().unwrap();
assert!(!rec.is_poisoned(), "a plain move must not poison");
let delta = build_delta(&scene, rec);
let moved = scene.document.get_entity(h).cloned().unwrap();
assert_ne!(orig, moved, "the move must actually change the entity");
// Undo restores the pre-move image exactly, in place (same handle).
scene.apply_entity_delta(&delta, true);
assert_eq!(scene.document.get_entity(h).cloned().unwrap(), orig);
assert_eq!(ms_occurrences(&scene, h), 1);
// Redo re-applies the post-move image.
scene.apply_entity_delta(&delta, false);
assert_eq!(scene.document.get_entity(h).cloned().unwrap(), moved);
assert_eq!(ms_occurrences(&scene, h), 1);
}
#[test]
fn erase_delta_round_trips_with_handle_and_no_dup() {
let mut scene = Scene::new();
let h1 = scene.add_entity(line(0.0, 0.0, 1.0, 0.0));
let h2 = scene.add_entity(line(2.0, 2.0, 3.0, 3.0));
let orig1 = scene.document.get_entity(h1).cloned().unwrap();
let orig2 = scene.document.get_entity(h2).cloned().unwrap();
let count = scene.document.entities().count();
scene.begin_undo_recording();
scene.erase_entities(&[h2]);
let rec = scene.take_undo_recording().unwrap();
assert!(!rec.is_poisoned(), "erasing an ungrouped entity must not poison");
let delta = build_delta(&scene, rec);
assert!(scene.document.get_entity(h2).is_none(), "h2 must be erased");
// Undo re-inserts h2 with its ORIGINAL handle and image, exactly once in
// the block record (the strip prevents a duplicated dangling entry),
// leaving h1 untouched.
scene.apply_entity_delta(&delta, true);
assert_eq!(scene.document.get_entity(h2).cloned().unwrap(), orig2);
assert_eq!(scene.document.get_entity(h1).cloned().unwrap(), orig1);
assert_eq!(scene.document.entities().count(), count);
assert_eq!(ms_occurrences(&scene, h2), 1, "h2 must appear once, not duplicated");
// Redo erases h2 again. (remove_entity leaves the handle dangling in
// the block record — harmless, a lookup miss is skipped on render/save
// — so we assert absence from the document, not from entity_handles.)
scene.apply_entity_delta(&delta, false);
assert!(scene.document.get_entity(h2).is_none());
}
#[test]
fn add_delta_round_trips() {
let mut scene = Scene::new();
scene.begin_undo_recording();
let h = scene.add_entity(line(0.0, 0.0, 4.0, 4.0));
let rec = scene.take_undo_recording().unwrap();
assert!(!rec.is_poisoned(), "a plain add on an existing layer must not poison");
let added = scene.document.get_entity(h).cloned().unwrap();
let delta = build_delta(&scene, rec);
// Undo removes the freshly added entity from the document (a dangling
// block-record entry may remain — see erase test — that's fine).
scene.apply_entity_delta(&delta, true);
assert!(scene.document.get_entity(h).is_none());
// Redo re-adds it with the same handle, exactly once (the strip clears
// any dangling entry so add_entity doesn't duplicate it).
scene.apply_entity_delta(&delta, false);
assert_eq!(scene.document.get_entity(h).cloned().unwrap(), added);
assert_eq!(ms_occurrences(&scene, h), 1);
}
#[test]
fn copy_delta_round_trips() {
let mut scene = Scene::new();
let src = scene.add_entity(line(0.0, 0.0, 1.0, 0.0));
scene.begin_undo_recording();
let new_handles =
scene.copy_entities(&[src], &EntityTransform::Translate(DVec3::new(10.0, 0.0, 0.0)));
let rec = scene.take_undo_recording().unwrap();
assert!(!rec.is_poisoned(), "copying a plain entity must not poison");
assert_eq!(new_handles.len(), 1);
let copy_h = new_handles[0];
let copy_img = scene.document.get_entity(copy_h).cloned().unwrap();
let delta = build_delta(&scene, rec);
// Undo erases the copy from the document, leaving the source intact.
scene.apply_entity_delta(&delta, true);
assert!(scene.document.get_entity(copy_h).is_none());
assert!(scene.document.get_entity(src).is_some());
// Redo restores the copy with its handle, once.
scene.apply_entity_delta(&delta, false);
assert_eq!(scene.document.get_entity(copy_h).cloned().unwrap(), copy_img);
assert_eq!(ms_occurrences(&scene, copy_h), 1);
}
}
+19
View File
@@ -132,6 +132,11 @@ impl Scene {
.flatten()
.collect();
for &h in handles {
// Delta-undo: capture the pre-transform image before mutating.
if self.is_recording_undo() {
let before = self.document.get_entity(h).cloned();
self.record_undo_before(h, before);
}
if let Some(entity) = self.document.get_entity_mut(h) {
view::dispatch::apply_transform(entity, t);
if mirror_true {
@@ -164,7 +169,13 @@ impl Scene {
}
}
// Move the baked dimension-block sub-entities too (collected above).
// These are entities not named in the reported change set, so a delta
// must capture their before-images here or a dimension move won't undo.
for h in &dim_block_subs {
if self.is_recording_undo() {
let before = self.document.get_entity(*h).cloned();
self.record_undo_before(*h, before);
}
if let Some(entity) = self.document.get_entity_mut(*h) {
view::dispatch::apply_transform(entity, t);
}
@@ -317,6 +328,10 @@ impl Scene {
let bn = d.base().block_name.clone();
if !bn.trim().is_empty() {
if let Some(new_bn) = self.clone_transformed_block(&bn, t) {
// Delta-undo: a copied dimension adds a fresh anonymous
// *D block record — non-entity state a pure-entity delta
// can't restore. Poison so the app keeps a full snapshot.
self.poison_undo_recording();
if let EntityType::Dimension(d) = &mut entity {
d.base_mut().block_name = new_bn;
}
@@ -327,6 +342,10 @@ impl Scene {
entity.common_mut().handle = Handle::NULL;
let h = self.document.add_entity(entity).unwrap_or(Handle::NULL);
if !h.is_null() {
// Delta-undo: a copy's before-image is "nothing" (undo erases it).
if self.is_recording_undo() {
self.record_undo_before(h, None);
}
let new_model = match self.document.get_entity(h) {
Some(EntityType::Hatch(dxf)) => {
let color = convert::tess_util::aci_to_rgba(&dxf.common.color);
+15
View File
@@ -335,6 +335,11 @@ impl Scene {
if self.is_layer_locked(h) {
continue;
}
// Delta-undo: capture the removed entity so an undo can re-insert it.
if self.is_recording_undo() {
let before = self.document.get_entity(h).cloned();
self.record_undo_before(h, before);
}
self.document.remove_entity(h);
self.selected.remove(&h);
self.hatches.remove(&h);
@@ -344,13 +349,18 @@ impl Scene {
}
// Remove erased handles from all groups; delete groups that become empty.
let group_dict_handle = self.document.header.acad_group_dict_handle;
let mut groups_changed = false;
let to_remove: Vec<Handle> = self
.document
.objects
.values_mut()
.filter_map(|obj| match obj {
ObjectType::Group(g) => {
let before = g.entities.len();
g.entities.retain(|h| !handles.contains(h));
if g.entities.len() != before {
groups_changed = true;
}
if g.entities.is_empty() {
Some(g.handle)
} else {
@@ -360,6 +370,11 @@ impl Scene {
_ => None,
})
.collect();
// Delta-undo: editing a group (or deleting an emptied one) mutates
// document.objects — non-entity state a pure-entity delta can't restore.
if groups_changed {
self.poison_undo_recording();
}
for gh in &to_remove {
if let Some(ObjectType::Dictionary(dict)) =
self.document.objects.get_mut(&group_dict_handle)