diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index 83d529646..4f1a97c69 100644 --- a/crates/ui-egui/src/panels.rs +++ b/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; diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 70363d527..44349beb6 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/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(); diff --git a/crates/ui-egui/src/panels/dialogs_3d.rs b/crates/ui-egui/src/panels/dialogs_3d.rs index 92b9624f7..4b9524a3e 100644 --- a/crates/ui-egui/src/panels/dialogs_3d.rs +++ b/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(); diff --git a/crates/ui-egui/src/panels/gradient_editor.rs b/crates/ui-egui/src/panels/gradient_editor.rs index f6e7123ab..127d1f00f 100644 --- a/crates/ui-egui/src/panels/gradient_editor.rs +++ b/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, + /// The project as the preview left it, to tell edits made since from the preview's. + applied_at: Option, /// The project and undo history as the editor opened, for Cancel. checkpoint: Option, next_id: u64, @@ -244,12 +247,12 @@ pub fn open(app: &mut EffectcraftApp, p: &Json) -> Result { 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; } } diff --git a/crates/ui-egui/src/panels/layer_styles_dialog.rs b/crates/ui-egui/src/panels/layer_styles_dialog.rs index 70ba5b5b0..713dfe6d4 100644 --- a/crates/ui-egui/src/panels/layer_styles_dialog.rs +++ b/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, - history: effectcraft_engine::history::History, + pub(crate) project: std::sync::Arc, + pub(crate) history: effectcraft_engine::history::History, } impl std::fmt::Debug for Checkpoint { diff --git a/crates/ui-egui/src/panels/live_preview.rs b/crates/ui-egui/src/panels/live_preview.rs index 861f8eb9f..83d0d2836 100644 --- a/crates/ui-egui/src/panels/live_preview.rs +++ b/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, /// 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, } 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 diff --git a/crates/ui-egui/src/panels/preview_merge.rs b/crates/ui-egui/src/panels/preview_merge.rs new file mode 100644 index 000000000..4eb99f2f0 --- /dev/null +++ b/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, + 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 = 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(before: Option<&T>, after: Option<&T>, now: Option<&T>, both: impl FnOnce() -> Option) -> Option { + 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 { + 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 = 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"); + } +} diff --git a/crates/ui-egui/tests/it/ui_dialog_preview.rs b/crates/ui-egui/tests/it/ui_dialog_preview.rs index 861548012..ce5f2803e 100644 --- a/crates/ui-egui/tests/it/ui_dialog_preview.rs +++ b/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] diff --git a/crates/ui-egui/tests/it/ui_gradient_editor.rs b/crates/ui-egui/tests/it/ui_gradient_editor.rs index 3b5da558d..f5ee8634b 100644 --- a/crates/ui-egui/tests/it/ui_gradient_editor.rs +++ b/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() {