#1436: take a preview back without taking other edits with it; sync the draft before OK keeps a preview

Cancel and Preview-off merge the checkpoint, the preview's result and the project now (new
preview_merge module), so an edit made while the dialog was open stays. OK first brings the
preview up to the draft it finds, and Composition Settings OK applies the Resolution after a kept
preview. The Gradient Editor's OK syncs and sets the draft when it differs from the last preview.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RDvxCS8xcR28VArjzn1x7U

crates/ui-egui/src/panels.rs

@@ -36,6 +36,7 @@ pub mod paint_panels;
 pub mod panel_kit;
 pub mod path_vr_panels;
 pub mod precomp;
+pub(crate) mod preview_merge;
 pub mod primitive_dialog;
 pub mod project;
 pub mod properties;

crates/ui-egui/src/panels/dialogs.rs

@@ -472,7 +472,13 @@ fn comp_settings(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens, exis
                 m.insert("comp".into(), json!(cid.0));
             }
             // A preview already made the edit (one undo step): OK keeps it.
-            if existing && super::live_preview::keep(app) {
+            if existing && super::live_preview::keep(app, "comp.settings", params.clone()) {
+                // The Resolution is not part of the command, so the preview never applied it.
+                if let Some(cid) = app.dialog_state.editing_comp
+                    && let Err(e) = set_resolution(app, cid.0, app.dialog_state.comp.resolution)
+                {
+                    app.ui.status = e.to_string();
+                }
                 app.dialog = None;
                 return;
             }
@@ -657,7 +663,7 @@ fn solid(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) {
     let (command, params) = (solid_command(ds), solid_params(ds));
     if ok || ui_enter(ctx) {
         // A preview already made the edit (one undo step): OK keeps it.
-        if !super::live_preview::keep(app)
+        if !super::live_preview::keep(app, command, params.clone())
             && let Err(e) = app.session.execute(command, params)
         {
             app.ui.status = e.to_string();

crates/ui-egui/src/panels/dialogs_3d.rs

@@ -255,7 +255,7 @@ pub fn camera(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) {
     let (id, p) = camera_command(&d);
     if ok || super::dialogs::ui_enter(ctx) {
         // A preview already made the edit (one undo step): OK keeps it.
-        if !super::live_preview::keep(app)
+        if !super::live_preview::keep(app, id, p.clone())
             && let Err(e) = app.session.execute(id, p)
         {
             app.ui.status = e.to_string();
@@ -372,7 +372,7 @@ pub fn light(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) {
     let (id, p) = light_command(&d);
     if ok || super::dialogs::ui_enter(ctx) {
         // A preview already made the edit (one undo step): OK keeps it.
-        if !super::live_preview::keep(app)
+        if !super::live_preview::keep(app, id, p.clone())
             && let Err(e) = app.session.execute(id, p)
         {
             app.ui.status = e.to_string();

crates/ui-egui/src/panels/gradient_editor.rs

@@ -22,7 +22,8 @@ use egui::{Color32, Rect, Sense, Stroke, StrokeKind, pos2, vec2};
 use serde_json::{Value as Json, json};
 
 use crate::i18n::tr;
-use crate::panels::layer_styles_dialog::{Checkpoint, checkpoint, restore};
+use crate::panels::layer_styles_dialog::{Checkpoint, checkpoint};
+use crate::panels::preview_merge::{self, Applied};
 use crate::theme::Tokens;
 use crate::{Dialog, EffectcraftApp};
 
@@ -67,6 +68,8 @@ pub struct Draft {
     pub preview: bool,
     /// The gradient the property was last given by the preview, if it has been.
     pub applied: Option<Gradient>,
+    /// The project as the preview left it, to tell edits made since from the preview's.
+    applied_at: Option<Applied>,
     /// The project and undo history as the editor opened, for Cancel.
     checkpoint: Option<Checkpoint>,
     next_id: u64,
@@ -244,12 +247,12 @@ pub fn open(app: &mut EffectcraftApp, p: &Json) -> Result<Json, String> {
 fn commit(app: &mut EffectcraftApp, ctx: &egui::Context) {
     app.dialog = None;
     let d = std::mem::take(&mut app.dialog_state.gradient);
-    // The preview already set the property, in one merged undo step: OK keeps it.
-    if d.applied.is_some() {
+    let g = d.gradient();
+    // The preview already set the property to this draft, in one merged undo step: OK keeps it.
+    if d.applied.as_ref() == Some(&g) {
         app.session.history.merge_key = None;
         return;
     }
-    let g = d.gradient();
     let Some(tg) = d.target else { return };
     if g == d.baseline {
         return;
@@ -264,6 +267,8 @@ fn commit(app: &mut EffectcraftApp, ctx: &egui::Context) {
 /// were when it opened (nothing happens when the preview never changed the property).
 pub fn finish(app: &mut EffectcraftApp, ok: bool, ctx: &egui::Context) {
     if ok {
+        // An edit made in the same frame as OK is not in the last preview yet.
+        sync_preview(app, ctx);
         commit(app, ctx);
         return;
     }
@@ -271,7 +276,7 @@ pub fn finish(app: &mut EffectcraftApp, ok: bool, ctx: &egui::Context) {
     if d.applied.is_some()
         && let Some(cp) = d.checkpoint
     {
-        restore(app, cp);
+        preview_merge::rollback(app, cp, d.applied_at.as_ref());
     }
     app.session.history.merge_key = None;
     app.dialog = None;
@@ -291,14 +296,18 @@ fn sync_preview(app: &mut EffectcraftApp, ctx: &egui::Context) {
             "value": Value::Gradient(g.clone()).to_json(), "merge": format!("gradient-editor-{}", tg.prop),
         });
         match crate::menus::invoke(app, ctx, "prop.set", params) {
-            Ok(_) => app.dialog_state.gradient.applied = Some(g),
+            Ok(_) => {
+                app.dialog_state.gradient.applied = Some(g);
+                app.dialog_state.gradient.applied_at = Some(preview_merge::applied(app));
+            }
             Err(e) => app.ui.status = e,
         }
     } else if !want
         && d.applied.is_some()
         && let Some(cp) = d.checkpoint.clone()
     {
-        restore(app, cp);
+        let at = app.dialog_state.gradient.applied_at.take();
+        preview_merge::rollback(app, cp, at.as_ref());
         app.dialog_state.gradient.applied = None;
     }
 }

crates/ui-egui/src/panels/layer_styles_dialog.rs

@@ -27,8 +27,8 @@ pub struct LayerStyleState {
 /// The project and undo history as a dialog found them, for a Cancel that rolls its live edits back.
 #[derive(Clone)]
 pub(crate) struct Checkpoint {
-    project: std::sync::Arc<effectcraft_engine::project::Project>,
-    history: effectcraft_engine::history::History,
+    pub(crate) project: std::sync::Arc<effectcraft_engine::project::Project>,
+    pub(crate) history: effectcraft_engine::history::History,
 }
 
 impl std::fmt::Debug for Checkpoint {

crates/ui-egui/src/panels/live_preview.rs

@@ -16,7 +16,8 @@ use egui::vec2;
 use serde_json::Value;
 
 use crate::panels::dialogs::Placement;
-use crate::panels::layer_styles_dialog::{Checkpoint, checkpoint, restore};
+use crate::panels::layer_styles_dialog::{Checkpoint, checkpoint};
+use crate::panels::preview_merge::{self, Applied};
 use crate::{Dialog, EffectcraftApp};
 
 /// Where a previewing dialog opens: the lower right, undimmed.
@@ -34,6 +35,8 @@ pub struct LivePreview {
     baseline: Option<Value>,
     /// The command and parameters the preview last applied.
     applied: Option<(&'static str, Value)>,
+    /// The project as that command left it, to tell edits made since from the preview's.
+    applied_at: Option<Applied>,
 }
 
 impl LivePreview {
@@ -78,7 +81,10 @@ pub fn sync(app: &mut EffectcraftApp, command: &'static str, params: Value) {
             put_back(app);
         }
         match app.session.execute(command, params.clone()) {
-            Ok(_) => app.dialog_state.live.applied = Some((command, params)),
+            Ok(_) => {
+                app.dialog_state.live.applied = Some((command, params));
+                app.dialog_state.live.applied_at = Some(preview_merge::applied(app));
+            }
             Err(e) => app.ui.status = e.to_string(),
         }
     } else if is_applied {
@@ -89,14 +95,20 @@ pub fn sync(app: &mut EffectcraftApp, command: &'static str, params: Value) {
 /// The project as the dialog found it.
 fn put_back(app: &mut EffectcraftApp) {
     if let Some(cp) = app.dialog_state.live.checkpoint.clone() {
-        restore(app, cp);
+        let at = app.dialog_state.live.applied_at.take();
+        // Only what the preview did is taken back: an edit made while the dialog was open stays.
+        preview_merge::rollback(app, cp, at.as_ref());
     }
     app.dialog_state.live.applied = None;
+    app.dialog_state.live.applied_at = None;
 }
 
-/// OK: the preview's result is the edit. Returns whether it already is (the caller then must not
-/// run the command again); otherwise the caller runs it, as without a preview.
-pub fn keep(app: &mut EffectcraftApp) -> bool {
+/// OK: the preview's result is the edit. The preview is first brought up to `command` with
+/// `params`, the draft as OK found it (an edit made in the same frame as OK is not in the last
+/// preview). Returns whether the project then already has the edit (the caller must not run the
+/// command again); otherwise the caller runs it, as without a preview.
+pub fn keep(app: &mut EffectcraftApp, command: &'static str, params: Value) -> bool {
+    sync(app, command, params);
     let kept = active(app) && app.dialog_state.live.applied.is_some();
     app.dialog_state.live = LivePreview::default();
     kept

crates/ui-egui/src/panels/preview_merge.rs

@@ -0,0 +1,204 @@
+//! Taking a dialog's Preview back without taking other edits with it.
+//!
+//! A previewing dialog (Camera, Light, Solid, Composition Settings, the Gradient Editor) puts the
+//! project and undo history back to a checkpoint when it is cancelled or Preview goes off. If the
+//! project changed in the meantime for another reason (a camera analysis finishing, a script, the
+//! control channel), swapping the checkpoint in would throw that edit away. This module undoes
+//! only what the preview did: a three-way merge of the checkpoint (before the preview), the project
+//! right after the preview (`Applied`) and the project now. Whatever the preview did not touch
+//! stays as it is now; what only the preview touched goes back to the checkpoint; what both touched
+//! keeps the other edit.
+
+use std::collections::BTreeSet;
+use std::sync::Arc;
+
+use effectcraft_engine::commands::text_edit::active_editor_snapshot;
+use effectcraft_engine::project::{Comp, Item, ItemId, ItemKind, Layer, LayerId, Project};
+
+use crate::EffectcraftApp;
+use crate::panels::layer_styles_dialog::{Checkpoint, restore};
+
+/// The project right after a preview was applied, and the undo depth then.
+#[derive(Clone)]
+pub(crate) struct Applied {
+    project: Arc<Project>,
+    undo_len: usize,
+}
+
+impl std::fmt::Debug for Applied {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("Applied").field("undo_len", &self.undo_len).finish_non_exhaustive()
+    }
+}
+
+/// Note the project as the preview that has just been applied left it.
+pub(crate) fn applied(app: &EffectcraftApp) -> Applied {
+    Applied { project: app.session.project.clone(), undo_len: app.session.history.undo.len() }
+}
+
+/// Put the project back as `checkpoint` has it, minus nothing but the preview: when the project is
+/// still what the preview made (or nothing says what that was) the checkpoint is simply restored.
+pub(crate) fn rollback(app: &mut EffectcraftApp, checkpoint: Checkpoint, applied: Option<&Applied>) {
+    let Some(applied) = applied.filter(|a| !Arc::ptr_eq(&a.project, &app.session.project)) else {
+        restore(app, checkpoint);
+        return;
+    };
+    let merged = merge(&checkpoint.project, &applied.project, &app.session.project);
+    // One step for what was done meanwhile, from the project before the dialog: Undo takes it back.
+    let label = app.session.history.undo.get(applied.undo_len).map(|(label, _)| label.clone()).unwrap_or_else(|| "Edit".to_string());
+    let mut history = checkpoint.history;
+    history.undo.push((label, checkpoint.project));
+    history.redo.clear();
+    history.merge_key = None;
+    let edited = active_editor_snapshot(&app.session);
+    app.session.project = Arc::new(merged);
+    app.session.history = history;
+    app.session.sanitize_after_replacement(edited.as_ref());
+    app.session.bump();
+}
+
+/// `now` without the changes between `before` and `after`, keeping any other change.
+fn merge(before: &Project, after: &Project, now: &Project) -> Project {
+    let mut merged = now.clone();
+    let ids: BTreeSet<ItemId> = before.items.keys().chain(after.items.keys()).chain(now.items.keys()).copied().collect();
+    for id in ids {
+        match item(before.items.get(&id), after.items.get(&id), now.items.get(&id)) {
+            Some(it) => {
+                merged.items.insert(id, it);
+            }
+            None => {
+                merged.items.remove(&id);
+            }
+        }
+    }
+    merged
+}
+
+/// The three-way choice for one value: untouched by the preview keeps `now`, touched only by the
+/// preview goes back to `before`, touched by both is `both`.
+fn pick<T: PartialEq + Clone>(before: Option<&T>, after: Option<&T>, now: Option<&T>, both: impl FnOnce() -> Option<T>) -> Option<T> {
+    if after == before {
+        now.cloned()
+    } else if now == after {
+        before.cloned()
+    } else {
+        both()
+    }
+}
+
+fn item(before: Option<&Item>, after: Option<&Item>, now: Option<&Item>) -> Option<Item> {
+    pick(before, after, now, || {
+        if let (Some(b), Some(a), Some(n)) = (before, after, now)
+            && let (ItemKind::Comp(bc), ItemKind::Comp(ac), ItemKind::Comp(nc)) = (&b.kind, &a.kind, &n.kind)
+        {
+            let mut merged = n.clone();
+            // Name, folder and the like: only a preview that changed the item itself and nothing else
+            // is taken back; the comp's own contents are merged below.
+            merged.kind = ItemKind::Comp(Arc::new(comp(bc, ac, nc)));
+            return Some(merged);
+        }
+        now.cloned()
+    })
+}
+
+fn bare(c: &Comp) -> Comp {
+    let mut b = c.clone();
+    b.layers.clear();
+    b
+}
+
+fn layer_of(c: &Comp, id: LayerId) -> Option<&Layer> {
+    c.layers.iter().find(|l| l.id == id)
+}
+
+fn comp(before: &Comp, after: &Comp, now: &Comp) -> Comp {
+    // The comp's own settings, apart from its layers.
+    let mut merged = pick(Some(&bare(before)), Some(&bare(after)), Some(&bare(now)), || Some(bare(now))).unwrap_or_else(|| bare(now));
+    let merge_layer = |id: LayerId| pick(layer_of(before, id), layer_of(after, id), layer_of(now, id), || layer_of(now, id).cloned());
+    // Layers as they stand now, in their order, then the ones the preview removed put back.
+    let mut layers: Vec<Layer> = now.layers.iter().filter_map(|l| merge_layer(l.id)).collect();
+    for (at, l) in before.layers.iter().enumerate() {
+        if layer_of(now, l.id).is_none()
+            && let Some(back) = merge_layer(l.id)
+        {
+            layers.insert(at.min(layers.len()), back);
+        }
+    }
+    merged.layers = layers;
+    merged
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::Dialog;
+    use crate::panels::live_preview;
+    use effectcraft_engine::Session;
+    use serde_json::json;
+
+    fn app_with_light() -> (EffectcraftApp, u64) {
+        let mut s = Session::default();
+        s.execute("comp.new", json!({"name": "Scene", "width": 320, "height": 180, "duration": 1})).unwrap();
+        let light = s.execute("layer.newLight", json!({"kind": "Point"})).unwrap()["layer"].as_u64().unwrap();
+        (EffectcraftApp::new(s), light)
+    }
+
+    fn name_of(app: &EffectcraftApp, layer: u64) -> String {
+        app.session.active_comp().unwrap().layer(LayerId(layer)).unwrap().name.clone()
+    }
+
+    fn layer_count(app: &EffectcraftApp) -> usize {
+        app.session.active_comp().unwrap().layers.len()
+    }
+
+    /// The preview renames the light; a null is added meanwhile (what a finished background job
+    /// would do). Cancel takes the rename back and keeps the null, with one undo step for it.
+    #[test]
+    fn cancelling_a_preview_keeps_an_edit_made_while_it_was_open() {
+        let (mut app, light) = app_with_light();
+        let original = name_of(&app, light);
+        app.dialog = Some(Dialog::LightSettings);
+        live_preview::begin(&mut app, Dialog::LightSettings);
+        let rename = |name: &str| json!({"layer": light, "name": name});
+        live_preview::sync(&mut app, "layer.rename", rename(&original));
+        live_preview::sync(&mut app, "layer.rename", rename("Previewed"));
+        assert_eq!(name_of(&app, light), "Previewed");
+        let layers = layer_count(&app);
+        app.session.execute("layer.newNull", json!({})).unwrap();
+        live_preview::cancel(&mut app);
+        assert_eq!(name_of(&app, light), original, "the preview is taken back");
+        assert_eq!(layer_count(&app), layers + 1, "the other edit stays");
+        assert!(app.session.undo());
+        assert_eq!(layer_count(&app), layers, "and has its own undo step");
+        assert_eq!(name_of(&app, light), original);
+    }
+
+    /// With nothing else done meanwhile the checkpoint is restored as before.
+    #[test]
+    fn cancelling_an_undisturbed_preview_restores_the_checkpoint() {
+        let (mut app, light) = app_with_light();
+        let original = name_of(&app, light);
+        let steps = app.session.history.undo.len();
+        app.dialog = Some(Dialog::LightSettings);
+        live_preview::begin(&mut app, Dialog::LightSettings);
+        live_preview::sync(&mut app, "layer.rename", json!({"layer": light, "name": original}));
+        live_preview::sync(&mut app, "layer.rename", json!({"layer": light, "name": "Previewed"}));
+        live_preview::cancel(&mut app);
+        assert_eq!(name_of(&app, light), original);
+        assert_eq!(app.session.history.undo.len(), steps);
+    }
+
+    /// An item both the preview and another edit changed keeps the other edit.
+    #[test]
+    fn a_layer_both_edits_changed_keeps_the_other_edit() {
+        let (mut app, light) = app_with_light();
+        let original = name_of(&app, light);
+        app.dialog = Some(Dialog::LightSettings);
+        live_preview::begin(&mut app, Dialog::LightSettings);
+        live_preview::sync(&mut app, "layer.rename", json!({"layer": light, "name": original}));
+        live_preview::sync(&mut app, "layer.rename", json!({"layer": light, "name": "Previewed"}));
+        app.session.execute("layer.rename", json!({"layer": light, "name": "Renamed elsewhere"})).unwrap();
+        live_preview::cancel(&mut app);
+        assert_eq!(name_of(&app, light), "Renamed elsewhere");
+    }
+}

crates/ui-egui/tests/it/ui_dialog_preview.rs

@@ -6,7 +6,7 @@
 //! at the lower right; the same dialogs creating something new keep the middle.
 
 use effectcraft_engine::Session;
-use effectcraft_engine::project::{ItemKind, LayerId};
+use effectcraft_engine::project::{ItemKind, LayerId, Resolution};
 use effectcraft_ui_egui::{Dialog, EffectcraftApp, menus};
 use egui::{Event, Key, Modifiers, PointerButton, Pos2, pos2};
 use egui_kittest::Harness;
@@ -218,6 +218,41 @@ fn composition_settings_preview() {
     assert_eq!(undo_len(&h), undo + 1, "in one undo step");
 }
 
+/// The Resolution is no part of the Composition Settings command, so a preview never applies it:
+/// OK has to, also when it keeps a preview of another field.
+#[test]
+fn composition_settings_ok_after_a_preview_still_applies_the_resolution() {
+    let mut h = harness(|_| {});
+    let before = comp_width(&h);
+    open(&mut h, "comp.settings", json!({}));
+    h.state_mut().dialog_state_mut().comp.width = before + 80;
+    h.run_steps(3);
+    assert_eq!(comp_width(&h), before + 80, "the preview applied the width");
+    h.state_mut().dialog_state_mut().comp.resolution = Resolution::Half;
+    h.run_steps(3);
+    click(&mut h, "dialog.comp.ok");
+    assert_eq!(comp_width(&h), before + 80);
+    assert_eq!(h.state().session.active_comp().unwrap().resolution, Resolution::Half, "OK applied the Resolution");
+}
+
+/// An edit made in the same frame as OK (Enter) is not in the last preview yet: OK brings the
+/// project up to the draft it finds instead of keeping the older preview.
+#[test]
+fn ok_in_the_frame_of_an_edit_keeps_the_edit_not_the_older_preview() {
+    let mut h = harness(solid_setup);
+    let l = solid_layer(&h);
+    open(&mut h, "layer.settings", json!({"layer": l}));
+    h.state_mut().dialog_state_mut().solid_size = [200, 150];
+    h.run_steps(3);
+    assert_eq!(solid_size(&h, l), [200, 150]);
+    h.state_mut().dialog_state_mut().solid_size = [240, 120];
+    h.input_mut().events.push(Event::Key { key: Key::Enter, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE });
+    h.step();
+    h.run_steps(2);
+    assert_eq!(h.state().dialog, None);
+    assert_eq!(solid_size(&h, l), [240, 120], "OK kept the draft it found, not the earlier preview");
+}
+
 /// A previewing dialog opens at the lower right so the canvas stays in view; the same dialogs
 /// creating something new keep the middle and have no Preview.
 #[test]

crates/ui-egui/tests/it/ui_gradient_editor.rs

@@ -466,6 +466,21 @@ fn the_preview_applies_edits_live_and_cancel_rolls_them_back() {
     assert_eq!(undo_len(&h), undo);
 }
 
+/// An edit made in the same frame as OK is not in the last preview: OK sets the draft it finds.
+#[test]
+fn ok_in_the_frame_of_an_edit_sets_the_draft_not_the_older_preview() {
+    let (mut h, l, colors) = setup(&[]);
+    open_editor(&mut h, colors);
+    let p = along(&h, "dialog.gradient.addColor", 0.25);
+    click_at(&mut h, p);
+    assert_eq!(gradient(&colors_prop(&h, l, colors).value).colors.len(), 3, "the preview has three stops");
+    h.state_mut().gradient_draft_mut().add(Track::Color, 0.75);
+    let ctx = h.ctx.clone();
+    effectcraft_ui_egui::panels::gradient_editor::finish(h.state_mut(), true, &ctx);
+    assert_eq!(h.state().dialog, None);
+    assert_eq!(gradient(&colors_prop(&h, l, colors).value).colors.len(), 4, "OK sets the draft it found");
+}
+
 /// With Preview off nothing is set until OK, and switching it off takes back what it applied.
 #[test]
 fn with_preview_off_nothing_is_set_until_ok() {