egui037: Preserve text ownership across unrelated history changes

This commit is contained in:
skylar taylor-barrick
2026-10-25 13:03:14 +01:00
parent 18206c1014
commit dec327b98a
10 changed files with 698 additions and 42 deletions
+56 -4
View File
@@ -13,14 +13,23 @@ use super::{CommandSpec, always, b_p, bad, comp_id, has_comp, has_layers, has_pr
use crate::{EngineError, Event, Result, Session, cmd, query};
/// Run `f` (which may run several undoable commands) as one undo step named `label`; on error
/// the project is restored.
/// the project, editor state and undo history are restored.
///
/// Front-end events emitted by a failed `f` are discarded with the state. Pending-analysis
/// queues and the journal are not part of this rollback and keep their existing behaviour.
pub(crate) fn grouped<T>(s: &mut Session, label: &str, f: impl FnOnce(&mut Session) -> Result<T>) -> Result<T> {
// The steps `f` records go to an empty stack (so the undo limit can't drop the snapshot
// taken before them), then become one step on the real one.
// taken before them), then become one step on the real one. Keep the complete pre-`f`
// history and editor state for a failure: rollback must not leave a partially retargeted
// editor or a `f`-recorded redo/branch behind.
let saved_history = s.history.clone();
let saved = std::mem::take(&mut s.history.undo);
let before = s.project.clone();
let before_state = s.state.clone();
let event_mark = s.events.len();
let r = f(s);
let changed = !s.history.undo.is_empty();
let interim_revision = s.ime_revision;
s.history.undo = saved;
s.history.merge_key = None;
match r {
@@ -36,9 +45,22 @@ pub(crate) fn grouped<T>(s: &mut Session, label: &str, f: impl FnOnce(&mut Sessi
Ok(v)
}
Err(e) => {
// Snapshot the failing editor before restoring the pre-`f` state: an intermediate
// text.edit / text change / comp switch must invalidate the pre-`f` editor token
// instead of reviving it.
let edited = super::text_edit::active_editor_snapshot(s);
s.project = before;
s.state = before_state;
s.history = saved_history;
s.history.merge_key = None;
// A failed intermediate comp switch must not reach the front end through an
// `OpenComp` event while its state has been rolled back.
s.events.truncate(event_mark);
s.sanitize_after_replacement(edited.as_ref());
// As in the batch rollback, the IME revision is a high-water mark: leave the highest
// intermediate stamp rather than rewinding to before `f`.
s.ime_revision = s.ime_revision.max(interim_revision);
if changed {
s.project = before;
s.sanitize_after_replacement();
s.bump();
}
Err(e)
@@ -704,6 +726,36 @@ pub fn specs() -> Vec<CommandSpec> {
mod tests {
use serde_json::json;
#[test]
fn grouped_rollback_restores_the_editor_state_and_history_after_a_transient_text_change() {
let mut s = crate::Session::default();
s.execute("comp.new", json!({"name": "Main", "width": 320, "height": 180, "duration": 1})).unwrap();
let lid = s.execute("layer.newText", json!({"text": "abcdef"})).unwrap()["layer"].as_u64().unwrap();
s.execute("text.edit", json!({"layer": lid, "caret": 3})).unwrap();
let original_comp = s.active_comp_id().unwrap();
let owner = s.state.text_edit.as_ref().unwrap().session;
let other =
s.execute("comp.new", json!({"name": "Other", "width": 160, "height": 90, "duration": 1, "open": false})).unwrap()["comp"].as_u64().unwrap();
let undo = s.history.undo.len();
s.drain_events();
let r = super::grouped(&mut s, "Rollback", |s| -> crate::Result<()> {
// An intermediate document edit and state switch, both undone by the error path.
s.execute("layer.setText", json!({"layer": lid, "text": "ab"}))?;
s.execute("comp.open", json!({"comp": other}))?;
Err(crate::EngineError::Other("stop".into()))
});
assert!(r.is_err());
assert_eq!(s.active_comp_id(), Some(original_comp), "the pre-group comp is restored");
assert_eq!(super::super::text_edit::layer_doc(&s, effectcraft_project::LayerId(lid)).unwrap().text, "abcdef", "the document is restored");
assert_eq!(s.history.undo.len(), undo, "the partial grouped steps are not left in history");
assert_ne!(s.state.text_edit.as_ref().unwrap().session, owner, "a transient replacement does not revive the old IME token");
assert!(
!s.drain_events().iter().any(|e| matches!(e, crate::Event::OpenComp(_))),
"the failed intermediate comp switch is not published to the front end"
);
}
/// `File.execute()` opens the file as a well-formed `file://` URL (spaces, `#`, `%`).
#[test]
fn execute_file_opens_an_encoded_file_url() {
+11 -1
View File
@@ -75,10 +75,20 @@ fn batch(s: &mut Session, p: &Value) -> Result<Value> {
Err(e) => {
let id = step.get("command").or_else(|| step.get("id")).and_then(Value::as_str).unwrap_or("?");
let note = if atomic {
// The snapshot is the intermediate state the failing step left: comparing it
// with the restored editor detects a restart or cross-comp switch inside the
// batch, so the pre-batch token is never revived. Do not restore
// `ime_revision`: a transient text edit inside the batch already advanced
// this process-monotonic high-water mark, and rewinding it would reuse a
// stamp (letting a wide-selection composition look current). An unrelated
// failed batch did not advance it, so its value stays exactly as it was.
let edited = crate::commands::text_edit::active_editor_snapshot(s);
let interim_revision = s.ime_revision;
s.project = project;
s.history = history;
s.state = state;
s.sanitize_after_replacement();
s.sanitize_after_replacement(edited.as_ref());
s.ime_revision = s.ime_revision.max(interim_revision);
s.bump();
"; the batch was rolled back (nothing changed)".to_string()
} else {
+117 -17
View File
@@ -6,7 +6,7 @@
//! Positions are character indices. The selection is `anchor..caret` (either order).
use effectcraft_keyframe::{CharStyle, TextDoc, Value as KV};
use effectcraft_project::{LayerId, LayerSource};
use effectcraft_project::{ItemId, LayerId, LayerSource};
use serde::{Deserialize, Serialize};
use serde_json::{Value, json};
@@ -18,6 +18,41 @@ pub(super) fn next_edit_session() -> u64 {
NEXT_EDIT_SESSION.fetch_add(1, std::sync::atomic::Ordering::Relaxed)
}
/// Process-global checked high-water allocator for [`crate::Session::ime_revision`] stamps.
///
/// egui's unavailable-IME receipt identity must be process-monotonic. Allocating the stamp from
/// one process-global counter means neither a second session nor a rollback can reuse a value.
/// The counter fails closed at `u64::MAX`: it returns `None` without wrapping, and callers keep
/// their existing stamp. An unavailable before/after pair then compares equal, so it earns no
/// receipt rather than claiming a mutation with a reused revision.
static NEXT_IME_REVISION: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
pub(crate) fn next_ime_revision() -> Option<u64> {
allocate_ime_revision(&NEXT_IME_REVISION)
}
fn allocate_ime_revision(counter: &std::sync::atomic::AtomicU64) -> Option<u64> {
use std::sync::atomic::Ordering::Relaxed;
let mut current = counter.load(Relaxed);
loop {
let next = current.checked_add(1)?;
match counter.compare_exchange_weak(current, next, Relaxed, Relaxed) {
Ok(_) => return Some(next),
Err(observed) => current = observed,
}
}
}
#[test]
fn ime_revision_allocation_stops_at_exhaustion_without_reusing_a_stamp() {
use std::sync::atomic::{AtomicU64, Ordering::Relaxed};
let counter = AtomicU64::new(u64::MAX - 1);
assert_eq!(allocate_ime_revision(&counter), Some(u64::MAX));
assert_eq!(allocate_ime_revision(&counter), None);
assert_eq!(allocate_ime_revision(&counter), None);
assert_eq!(counter.load(Relaxed), u64::MAX);
}
use super::{CommandSpec, b_p, bad, comp_id, layer_mut, resolve_layer, str_p};
use crate::{EditorState, EngineError, Result, Session, cmd};
@@ -38,11 +73,13 @@ pub struct TextEdit {
pub goal_x: Option<f64>,
/// This editing session's identity.
///
/// It changes when the editor is started, its layer/document is replaced (including by
/// undo/redo), or the editor is restored from another state. It is deliberately not
/// serialized: a saved token belongs to a previous process and cannot identify a new
/// target. The IME owner uses it as its replacement epoch, so an old composition is never
/// inherited by a recreated layer or a replaced document.
/// It changes when the editor is started and when its layer or document is replaced, by
/// undo/redo, a history jump, a rollback, or a time change showing another document. A
/// restore that keeps the same composition, layer and byte/style-equal document (an opacity
/// undo while editing) keeps it, so an active IME composition survives an unrelated change.
/// It is deliberately not serialized: a saved token belongs to a previous process and
/// cannot identify a new target. The IME owner uses it as its replacement epoch, so an old
/// composition is never inherited by a recreated layer or a replaced document.
#[serde(skip, default = "next_edit_session")]
pub session: u64,
}
@@ -78,6 +115,15 @@ pub fn layer_doc(s: &Session, lid: LayerId) -> Option<TextDoc> {
/// reported as unavailable rather than silently truncated.
pub const MAX_IME_TEXT_BYTES: usize = 4000;
/// The editor's committed-text revision for an IME receipt whose surrounding text is
/// unavailable (a selection wider than [`MAX_IME_TEXT_BYTES`]).
///
/// Unlike [`Session::revision`], unrelated project edits (an opacity undo) do not advance it, so
/// they cannot make an active composition look stale.
pub fn ime_stamp_revision(s: &Session) -> u64 {
s.ime_revision
}
/// Bounded committed text around an IME selection, with UTF-8 byte offsets.
///
/// The Type tool keeps the input method's pre-edit in egui's temporary data, never in the
@@ -248,6 +294,7 @@ pub(crate) fn edit_doc(
) -> Result<()> {
let cid = s.active_comp_id().ok_or(EngineError::NoComp)?;
let t = s.time();
let revision = s.revision;
s.edit_text_doc(label, merge, |proj, st| {
let l = layer_mut(proj, cid, lid)?;
let lt = l.layer_time(t);
@@ -272,7 +319,13 @@ pub(crate) fn edit_doc(
}
}
Ok(())
})
})?;
// The editor's unavailable-text receipt identity follows only its own document: an edit to
// another text layer while this editor is open must not cancel the composition.
if s.revision != revision && s.state.text_edit.as_ref().is_some_and(|e| e.layer == lid) {
s.note_text_edit_change();
}
Ok(())
}
fn range_p(p: &Value) -> Option<std::ops::Range<usize>> {
@@ -675,9 +728,13 @@ fn paste_formatting(s: &mut Session, p: &Value) -> Result<Value> {
/// Give the current editor a fresh replacement epoch after its document/layer was replaced.
pub(crate) fn invalidate_session(s: &mut Session) {
let replaced = s.state.text_edit.is_some();
if let Some(e) = s.state.text_edit.as_mut() {
e.session = next_edit_session();
}
if replaced {
s.note_text_edit_change();
}
}
/// Clamp the editor selection and give it a fresh IME epoch after its document was replaced.
@@ -685,7 +742,8 @@ pub(crate) fn invalidate_session(s: &mut Session) {
/// This is the one replacement path for document changes outside `edit_doc`:
/// [`crate::Session::edit`] calls [`after_external_change`] after every edit, [`crate::Session::set_time`]
/// and [`crate::Session::sync_related_times`] call it after moving the CTI, and
/// [`crate::Session::sanitize_after_replacement`] calls it when it restores a whole document.
/// [`crate::Session::sanitize_after_replacement`] calls it with the editor document snapshotted
/// before a history restore or rollback.
/// Clamping first means a shortened document leaves the caret in range, so the next Backspace
/// deletes instead of being swallowed by an out-of-range selection.
pub(crate) fn document_replaced(s: &mut Session) {
@@ -693,27 +751,69 @@ pub(crate) fn document_replaced(s: &mut Session) {
invalidate_session(s);
}
/// The document the active on-canvas editor is showing, with its layer.
/// The identity and document of the active on-canvas editor at one moment.
///
/// The composition and the editor session are part of the identity: a restoration must not keep
/// a composition alive when the snapshot came from another comp, or from an intermediate editor
/// that was restarted during the operation being rolled back.
#[derive(Clone, Debug, PartialEq)]
pub struct EditorSnapshot {
/// The composition the editor was attached to.
pub comp: ItemId,
/// The edited layer.
pub layer: LayerId,
/// The editor session token, unique across the process.
pub session: u64,
/// The Source Text the editor was showing.
pub doc: TextDoc,
}
/// The identity and document the active on-canvas editor is showing.
///
/// [`crate::Session::edit`] and the time setters snapshot this before a change and compare it
/// afterwards: only a change that actually replaces this document starts a new epoch, while a
/// write to another property, layer or composition, a move to a time showing the same text, or a
/// write that leaves the text as it was, keeps the editor's owner. With no editor open this is a
/// single `Option` check, with no document copy.
pub(crate) fn active_editor_doc(s: &Session) -> Option<(LayerId, TextDoc)> {
pub fn active_editor_snapshot(s: &Session) -> Option<EditorSnapshot> {
let e = s.state.text_edit.as_ref()?;
Some((e.layer, layer_doc(s, e.layer)?))
Some(EditorSnapshot { comp: s.active_comp_id()?, layer: e.layer, session: e.session, doc: layer_doc(s, e.layer)? })
}
/// Finish a change outside the editor: if it replaced the document the editor owned, clamp the
/// Finish an edit outside the editor: if it replaced the document the editor owned, clamp the
/// selection and start a new IME epoch.
pub(crate) fn after_external_change(s: &mut Session, before: Option<&(LayerId, TextDoc)>) {
let Some((lid, before)) = before else { return };
// The change ended the editor or moved it to another layer: that path owns its own session.
if !s.state.text_edit.as_ref().is_some_and(|e| e.layer == *lid) {
///
/// An edit that itself moves or closes the editor owns the resulting session, so an identity
/// mismatch is not a replacement here.
pub(crate) fn after_external_change(s: &mut Session, before: Option<&EditorSnapshot>) {
let Some(before) = before else { return };
let Some(e) = s.state.text_edit.as_ref() else { return };
if s.state.active_comp != Some(before.comp) || e.layer != before.layer || e.session != before.session {
return;
}
if layer_doc(s, *lid).as_ref() != Some(before) {
if layer_doc(s, before.layer).as_ref() != Some(&before.doc) {
document_replaced(s);
}
}
/// Finish a project-state replacement (undo, redo, a history jump, a rollback): keep the editor
/// only when the restored state still has the same composition, layer and editor session and
/// shows byte/style-equal text.
///
/// The restored session token is the one saved in the replaced state, so an intermediate
/// restart or cross-comp switch must be detected against the snapshot instead of being assumed
/// to be the editor's own change.
pub(crate) fn after_replacement(s: &mut Session, before: Option<&EditorSnapshot>) {
let Some(before) = before else {
// No editor was open when the snapshot was taken: a restored editor must not inherit a
// composition from the state the replacement replaced.
if s.state.text_edit.is_some() {
document_replaced(s);
}
return;
};
let same = s.state.text_edit.as_ref().is_some_and(|e| s.state.active_comp == Some(before.comp) && e.layer == before.layer && e.session == before.session);
if !same || layer_doc(s, before.layer).as_ref() != Some(&before.doc) {
document_replaced(s);
}
}
+2 -1
View File
@@ -341,13 +341,14 @@ impl Session {
let k = new_line.iter().position(|&i| i == target).unwrap_or(0);
let undo: Vec<(String, Arc<Project>)> = (0..k).map(|i| (a.label[new_line[i + 1]].clone(), a.state[new_line[i]].clone())).collect();
let redo: Vec<(String, Arc<Project>)> = new_line[k + 1..].iter().rev().map(|&i| (a.label[i].clone(), a.state[i].clone())).collect();
let edited = crate::commands::text_edit::active_editor_snapshot(self);
let cur = std::mem::replace(&mut self.project, a.state[target].clone());
self.keep_view_settings(&cur);
self.history.undo = undo;
self.history.redo = redo;
self.history.branches = branches;
self.history.merge_key = None;
self.sanitize_after_replacement();
self.sanitize_after_replacement(edited.as_ref());
self.bump();
true
}
+46 -8
View File
@@ -394,6 +394,15 @@ pub struct Session {
pub project: Arc<Project>,
pub history: History,
pub revision: u64,
/// Process-unique monotonic revision of the on-canvas editor's committed text and session.
/// Unlike [`Session::revision`], unrelated project edits do not advance it; the IME receipt
/// uses it when surrounding text is unavailable. Read it with
/// [`commands::text_edit::ime_stamp_revision`].
///
/// Stamps come from a process-global checked allocator and are never rewound: a failed
/// atomic batch keeps the highest intermediate stamp. On allocator exhaustion no stamp is
/// issued, so an unavailable receipt cannot be accepted with a reused revision.
pub ime_revision: u64,
pub saved_revision: u64,
/// The project as last opened or saved: the project is unmodified while it is this very
/// snapshot, so undoing back to the saved state clears the modified mark.
@@ -562,6 +571,8 @@ impl Default for Session {
project,
history: History::default(),
revision: 0,
// A fresh session gets its own process-monotonic stamp even before its first edit.
ime_revision: commands::text_edit::next_ime_revision().unwrap_or(u64::MAX),
saved_revision: 0,
path: None,
state: EditorState { snapping: true, view_layout: 1, ..Default::default() },
@@ -692,7 +703,7 @@ impl Session {
/// epoch. The editor's own typing goes through `edit_text_doc`, which owns that
/// document and must not treat its own keystrokes as a replacement.
pub fn edit<T>(&mut self, label: &str, merge: Option<&str>, f: impl FnOnce(&mut Project, &mut EditorState) -> Result<T>) -> Result<T> {
let edited = commands::text_edit::active_editor_doc(self);
let edited = commands::text_edit::active_editor_snapshot(self);
let out = self.edit_inner(label, merge, f)?;
commands::text_edit::after_external_change(self, edited.as_ref());
Ok(out)
@@ -751,6 +762,19 @@ impl Session {
Ok(r)
}
/// Note a change to the on-canvas editor's committed text or session.
///
/// Frontends compare this with [`commands::text_edit::ime_stamp_revision`]; an unrelated
/// project change must not advance it.
pub(crate) fn note_text_edit_change(&mut self) {
// Checked process-wide allocation: a stamp is never wrapped or reused. On exhaustion
// keep the previous value; an unavailable before/after pair then compares equal and
// earns no receipt instead of accepting a stale revision.
if let Some(revision) = commands::text_edit::next_ime_revision() {
self.ime_revision = self.ime_revision.max(revision);
}
}
pub fn bump(&mut self) {
self.revision += 1;
// Back at the saved state (undo, redo, a history jump or a rolled-back batch).
@@ -823,32 +847,46 @@ impl Session {
pub fn undo(&mut self) -> bool {
let Some((label, p)) = self.history.undo.pop() else { return false };
let edited = commands::text_edit::active_editor_snapshot(self);
let cur = std::mem::replace(&mut self.project, p);
self.keep_view_settings(&cur);
self.history.redo.push((label, cur));
self.history.merge_key = None;
self.sanitize_after_replacement();
self.sanitize_after_replacement(edited.as_ref());
self.bump();
true
}
pub fn redo(&mut self) -> bool {
let Some((label, p)) = self.history.redo.pop() else { return false };
let edited = commands::text_edit::active_editor_snapshot(self);
let cur = std::mem::replace(&mut self.project, p);
self.keep_view_settings(&cur);
self.history.undo.push((label, cur));
self.history.merge_key = None;
self.sanitize_after_replacement();
self.sanitize_after_replacement(edited.as_ref());
self.bump();
true
}
/// Keep selections valid after the project was replaced by another state (undo, redo, a
/// history jump, a rollback): the same layer id can name a different document, so the
/// on-canvas editor gets a fresh IME epoch.
pub fn sanitize_after_replacement(&mut self) {
/// on-canvas editor gets a fresh IME epoch only when its own document changed.
///
/// Normally `before` is [`commands::text_edit::active_editor_snapshot`] taken from the
/// state this call is about to replace. Atomic batch rollback is the exception: it has
/// already restored the pre-batch state and passes the snapshot of the failing intermediate
/// state, so a transient restart or component switch inside the batch still rejects reviving
/// the old composition. A restored state that still has the same composition, layer and
/// editor session and shows byte/style-equivalent text (an opacity or other property undo)
/// keeps the editor's owner and selection; a different document, comp, layer or session clamps the
/// selection and starts a new epoch. A closed editor stays closed, and a fallback comp's
/// matching layer id never inherits the old comp's session. `None` means no editor was open
/// when the snapshot was taken; if one is open after the replacement it is treated as an
/// unverified document replacement, so it never inherits an old composition.
pub fn sanitize_after_replacement(&mut self, before: Option<&commands::text_edit::EditorSnapshot>) {
self.sanitize_state();
commands::text_edit::document_replaced(self);
commands::text_edit::after_replacement(self, before);
}
/// Drop selections that no longer exist. Each comp keeps its own selection (#1001); a layer id
@@ -936,7 +974,7 @@ impl Session {
pub fn set_time(&mut self, t: Tick) {
if let Some(c) = self.state.active_comp {
let t = self.project.comp(c).map_or(t, |comp| comp_frame_at(comp, t));
let edited = commands::text_edit::active_editor_doc(self);
let edited = commands::text_edit::active_editor_snapshot(self);
self.state.times.insert(c, t);
if self.prefs.general.sync_time_related_items {
// Direct call: this `set_time` owns the editor snapshot for both time changes,
@@ -953,7 +991,7 @@ impl Session {
pub fn sync_related_times(&mut self, comp: ItemId, t: Tick) {
// Moving a related comp can move the active one, so the editor's document is checked
// here too; the snapshot is skipped entirely when no text editor is open.
let edited = commands::text_edit::active_editor_doc(self);
let edited = commands::text_edit::active_editor_snapshot(self);
self.sync_related_times_inner(comp, t);
commands::text_edit::after_external_change(self, edited.as_ref());
}
+254 -2
View File
@@ -6,7 +6,7 @@ use effectcraft_keyframe::{BaselineOption, Justify, Kerning, TextDoc};
use serde_json::json;
use crate::Session;
use crate::commands::text_edit::{MAX_IME_TEXT_BYTES, ime_surrounding, layer_doc};
use crate::commands::text_edit::{MAX_IME_TEXT_BYTES, ime_stamp_revision, ime_surrounding, layer_doc};
fn session(text: &str) -> (Session, u64) {
let mut s = Session::default();
@@ -400,6 +400,257 @@ fn property_writes_replace_the_edited_document_only_when_the_text_changes() {
assert_ime_epoch_changed(&s, owner, "replacing the document changes the IME owner");
}
#[test]
fn undo_and_redo_of_an_unrelated_property_keep_the_ime_owner_and_selection() {
let (mut s, t) = editing("abcdef", 3);
let owner = ime_owner(&s);
s.execute("prop.set", json!({"layer": t, "path": "transform/opacity", "value": 50})).unwrap();
assert_ime_owner(&s, owner, "the opacity write keeps the owner");
assert_eq!(sel(&s), (3, 3), "the opacity write keeps the selection");
// The opacity step does not change Source Text: restoring it keeps the same editor.
s.execute("edit.undo", json!({})).unwrap();
assert_ime_owner(&s, owner, "undoing only opacity keeps the owner");
assert_eq!(sel(&s), (3, 3), "undoing only opacity keeps the selection");
s.execute("edit.redo", json!({})).unwrap();
assert_ime_owner(&s, owner, "redoing only opacity keeps the owner");
assert_eq!(sel(&s), (3, 3), "redoing only opacity keeps the selection");
}
#[test]
fn a_style_only_text_change_and_its_undo_replace_the_editor_epoch() {
let (mut s, t) = editing("abcdef", 3);
let owner = ime_owner(&s);
// The same characters with another run style are still a different document.
s.execute("layer.setText", json!({"layer": t, "range": [1, 4], "size": 12})).unwrap();
assert_eq!(doc(&s, t).text, "abcdef");
assert_eq!(doc(&s, t).style_at(2).size, 12.0);
assert_ime_epoch_changed(&s, owner, "a style-only change starts a new IME epoch");
assert_eq!(sel(&s), (3, 3), "a same-length style change keeps the caret in range");
let styled = ime_owner(&s);
s.execute("edit.undo", json!({})).unwrap();
assert_eq!(doc(&s, t).style_at(2).size, 60.0);
assert_ime_epoch_changed(&s, styled, "undoing a style change starts a new IME epoch");
}
#[test]
fn history_jumps_keep_the_owner_over_unrelated_steps_and_replace_it_when_the_text_changes() {
let (mut s, t) = editing("abcdef", 3);
let owner = ime_owner(&s);
s.execute("prop.set", json!({"layer": t, "path": "transform/opacity", "value": 50})).unwrap();
s.execute("prop.set", json!({"layer": t, "path": "transform/opacity", "value": 25})).unwrap();
let states = s.execute("edit.history.list", json!({})).unwrap()["states"].as_array().unwrap().clone();
let cur = states.iter().find(|n| n["current"] == true).unwrap()["index"].as_u64().unwrap();
let parent = states.iter().find(|n| n["index"].as_u64() == Some(cur)).unwrap()["parent"].as_u64().unwrap();
// Both states show the same Source Text: jumping between them keeps the editor.
s.execute("edit.history.goto", json!({"index": parent})).unwrap();
assert_ime_owner(&s, owner, "jumping to another opacity state keeps the owner");
assert_eq!(sel(&s), (3, 3), "and the selection");
s.execute("edit.history.goto", json!({"index": cur})).unwrap();
assert_ime_owner(&s, owner, "jumping back keeps the owner");
assert_eq!(sel(&s), (3, 3), "and the selection");
// A history state whose Source Text differs is a document replacement.
set_source(&mut s, t, "abc");
let changed = ime_owner(&s);
assert_ne!(changed, owner);
s.execute("edit.history.goto", json!({"index": parent})).unwrap();
assert_text(&s, t, "abcdef");
assert_ime_epoch_changed(&s, changed, "jumping to a different document starts a new epoch");
}
#[test]
fn atomic_batch_rollback_keeps_the_owner_for_unrelated_steps_and_replaces_it_for_text_changes() {
let (mut s, t) = editing("abcdef", 3);
let owner = ime_owner(&s);
let r = s.execute(
"engine.batch",
json!({"atomic": true, "steps": [
{"command": "prop.set", "params": {"layer": t, "path": "transform/opacity", "value": 50}},
{"command": "no.suchCommand", "params": {}}
]}),
);
assert!(r.is_err());
assert_ime_owner(&s, owner, "rolling back an unrelated step keeps the owner");
assert_eq!(sel(&s), (3, 3), "and the selection");
// A partially applied text replacement was still shown before the rollback restores the
// original document, so the editor must not revive the composition it had before the batch.
let r = s.execute(
"engine.batch",
json!({"atomic": true, "steps": [
{"command": "layer.setText", "params": {"layer": t, "text": "ab"}},
{"command": "no.suchCommand", "params": {}}
]}),
);
assert!(r.is_err());
assert_text(&s, t, "abcdef");
assert_ime_epoch_changed(&s, owner, "a rolled-back text change does not revive the old owner");
assert_eq!(sel(&s), (3, 3), "the restored document keeps the caret in range");
}
#[test]
fn atomic_batch_rollback_after_a_transient_editor_restart_bumps_the_epoch() {
let (mut s, t) = editing("abcdef", 3);
let owner = ime_owner(&s);
// `text.edit` restarts the session even on the layer that is already edited. The rollback
// restores the pre-batch TextEdit (with the old token), so the intermediate restart must be
// detected from the snapshot: neither the old composition nor its token may be revived.
let r = s.execute(
"engine.batch",
json!({"atomic": true, "steps": [
{"command": "text.edit", "params": {"layer": t, "caret": 1}},
{"command": "no.suchCommand", "params": {}}
]}),
);
assert!(r.is_err());
assert_eq!(sel(&s), (3, 3), "the restored selection is the pre-batch one");
assert_ime_epoch_changed(&s, owner, "a canceled intermediate editor must not revive the old token");
}
#[test]
fn atomic_batch_rollback_after_a_cross_comp_editor_switch_bumps_the_epoch() {
let (mut s, t) = session("abcdef");
let original = s.active_comp_id().unwrap();
// Give the other comp a layer with the same id and the same Source Text, so only the comp
// and editor-session identity distinguishes the two editors.
let layer = s.active_comp().unwrap().layer(effectcraft_project::LayerId(t)).unwrap().clone();
let other = new_other_comp(&mut s);
let other_id = effectcraft_project::ItemId(other);
std::sync::Arc::make_mut(&mut s.project).comp_mut(other_id).unwrap().layers.push(layer);
s.execute("comp.open", json!({"comp": original.0})).unwrap();
begin_edit_at(&mut s, t, 3);
let owner = ime_owner(&s);
let r = s.execute(
"engine.batch",
json!({"atomic": true, "steps": [
{"command": "comp.open", "params": {"comp": other}},
{"command": "text.edit", "params": {"layer": t, "caret": 1}},
{"command": "no.suchCommand", "params": {}}
]}),
);
assert!(r.is_err());
assert_eq!(s.active_comp_id(), Some(original), "the batch restored the original comp");
assert_eq!(sel(&s), (3, 3), "and the pre-batch selection");
assert_ime_epoch_changed(&s, owner, "an identical layer in another comp must not revive the old token");
}
#[test]
fn atomic_batch_rollback_after_closing_the_editor_does_not_revive_it() {
let (mut s, t) = session("abcdef");
let original = s.active_comp_id().unwrap();
let other = new_other_comp(&mut s);
s.execute("comp.open", json!({"comp": original.0})).unwrap();
begin_edit_at(&mut s, t, 3);
let owner = ime_owner(&s);
// The failing state has no editor at all: the restoration brings the pre-batch one back and
// must treat it as unverified rather than revive its composition.
let r = s.execute(
"engine.batch",
json!({"atomic": true, "steps": [
{"command": "comp.open", "params": {"comp": other}},
{"command": "no.suchCommand", "params": {}}
]}),
);
assert!(r.is_err());
assert!(s.state.text_edit.is_some(), "the batch restored the editor state");
assert_eq!(sel(&s), (3, 3));
assert_ime_epoch_changed(&s, owner, "a canceled editor close must not revive the old token");
}
#[test]
fn restoring_without_an_editor_snapshot_replaces_an_open_editor() {
let (mut s, _) = editing("abcdef", 3);
let owner = ime_owner(&s);
// `None` means no editor was open when the state was replaced; an editor that is open
// afterwards cannot prove it kept its document.
s.sanitize_after_replacement(None);
assert_ime_epoch_changed(&s, owner, "an unverified restored editor starts a new epoch");
assert_eq!(sel(&s), (3, 3));
}
#[test]
fn undo_to_a_shorter_document_clamps_the_caret() {
let (mut s, t) = editing("abc", 3);
replace_text(&mut s, t, "abcdef");
select_anchor(&mut s, 6, 6);
let owner = ime_owner(&s);
assert_eq!(sel(&s), (6, 6));
s.execute("edit.undo", json!({})).unwrap();
assert_text(&s, t, "abc");
assert_eq!(sel(&s), (3, 3), "undoing to a shorter document clamps the caret");
assert_ime_epoch_changed(&s, owner, "and starts a new epoch");
}
#[test]
fn the_ime_stamp_revision_follows_the_editor_not_unrelated_project_edits() {
let (mut s, t) = editing("abcdef", 6);
let revision = ime_stamp_revision(&s);
s.execute("prop.set", json!({"layer": t, "path": "transform/opacity", "value": 50})).unwrap();
assert_eq!(ime_stamp_revision(&s), revision, "an unrelated property write does not advance it");
s.execute("edit.undo", json!({})).unwrap();
assert_eq!(ime_stamp_revision(&s), revision, "undoing it does not either");
let r = s.execute(
"engine.batch",
json!({"atomic": true, "steps": [
{"command": "prop.set", "params": {"layer": t, "path": "transform/opacity", "value": 25}},
{"command": "no.suchCommand", "params": {}}
]}),
);
assert!(r.is_err());
assert_eq!(ime_stamp_revision(&s), revision, "a rolled-back unrelated batch restores it");
s.execute("text.insert", json!({"text": "x"})).unwrap();
assert!(ime_stamp_revision(&s) > revision, "typing advances it");
let typed = ime_stamp_revision(&s);
s.execute("layer.setText", json!({"layer": t, "text": "y"})).unwrap();
assert!(ime_stamp_revision(&s) > typed, "a replaced document advances it too");
}
#[test]
fn a_failed_atomic_batch_with_a_transient_edit_keeps_its_high_water_ime_revision() {
let (mut s, t) = editing("abcdef", 6);
let revision = ime_stamp_revision(&s);
let r = s.execute(
"engine.batch",
json!({"atomic": true, "steps": [
{"command": "text.insert", "params": {"text": "x"}},
{"command": "text.delete", "params": {}},
{"command": "no.suchCommand", "params": {}}
]}),
);
assert!(r.is_err());
assert_text(&s, t, "abcdef");
// The intermediate document edit is real even though the final document equals the
// pre-batch one: its stamp must remain the high-water mark.
assert!(ime_stamp_revision(&s) > revision, "a transient edit must not rewind the IME revision");
let after_transient = ime_stamp_revision(&s);
// An unrelated failed batch really did not edit the document: it must preserve the revision
// exactly, or every wide-selection composition would be canceled by unrelated work.
let r = s.execute(
"engine.batch",
json!({"atomic": true, "steps": [
{"command": "prop.set", "params": {"layer": t, "path": "transform/opacity", "value": 25}},
{"command": "no.suchCommand", "params": {}}
]}),
);
assert!(r.is_err());
assert_eq!(ime_stamp_revision(&s), after_transient, "an unrelated rolled-back batch preserves the revision");
}
#[test]
fn a_replacing_property_write_clamps_the_selection_so_the_next_backspace_deletes() {
let (mut s, t) = editing("abcdef", 6);
@@ -596,9 +847,10 @@ fn removing_the_active_comp_does_not_retarget_its_text_editor() {
// Reproduce cleanup after a project replacement, bypassing Session::edit's earlier
// document guard so this specifically exercises sanitize_state's fallback-comp branch.
let edited = crate::commands::text_edit::active_editor_snapshot(&s);
std::sync::Arc::make_mut(&mut s.project).items.remove(&original);
if replacement {
s.sanitize_after_replacement();
s.sanitize_after_replacement(edited.as_ref());
} else {
s.sanitize_state();
}
+2 -2
View File
@@ -1,5 +1,5 @@
//! The crate's integration tests, one binary (each module is a former `tests/<name>.rs`).
//! The crate's shared integration-test binary (each module is a former `tests/<name>.rs`).
//! `system_font_menu` is separate: it asserts that no system font scan has run in its process.
mod script_fallback;
mod system_font_menu;
mod vertical_punctuation;
@@ -5,6 +5,7 @@
//! edits made in the dialog back, OK keeps them (one undo step per control drag).
use crate::i18n::tr;
use effectcraft_engine::commands::text_edit::active_editor_snapshot;
use effectcraft_engine::keyframe::Value as KV;
use effectcraft_engine::project::styles::{BLENDING, STYLES};
use effectcraft_engine::project::{Layer, LayerId, ParamUi, Property};
@@ -258,15 +259,85 @@ pub fn finish(app: &mut EffectcraftApp, ok: bool) {
&& !ok
{
let changed = !std::sync::Arc::ptr_eq(&app.session.project, &checkpoint.project);
app.session.project = checkpoint.project;
app.session.history = checkpoint.history;
if changed {
app.session.sanitize_after_replacement();
// Snapshot only when Cancel has something to restore: opening and cancelling without
// edits must not copy the whole TextDoc.
let edited = active_editor_snapshot(&app.session);
app.session.project = checkpoint.project;
app.session.history = checkpoint.history;
app.session.sanitize_after_replacement(edited.as_ref());
app.session.bump();
} else {
app.session.sanitize_state();
// No edit was committed, so nothing can have become invalid. In particular, do not
// call sanitize_state here: its text-edit cleanup clones the whole TextDoc.
app.session.history = checkpoint.history;
}
}
app.session.history.merge_key = None;
app.dialog = None;
}
#[cfg(test)]
mod tests {
use super::*;
use effectcraft_engine::Session;
use effectcraft_engine::commands::text_edit::layer_doc;
use effectcraft_engine::project::LayerId;
use effectcraft_engine::project::styles::BLENDING;
use serde_json::json;
fn editing_app() -> (EffectcraftApp, u64) {
let mut s = Session::default();
s.execute("comp.new", json!({"name": "Styles", "width": 320, "height": 180, "duration": 1})).unwrap();
let lid = s.execute("layer.newText", json!({"text": "abcdef"})).unwrap()["layer"].as_u64().unwrap();
s.execute("text.edit", json!({"layer": lid, "caret": 3})).unwrap();
(EffectcraftApp::new(s), lid)
}
fn editor_state(app: &EffectcraftApp) -> Option<(LayerId, u64, usize, usize)> {
app.session.state.text_edit.as_ref().map(|e| (e.layer, e.session, e.anchor, e.caret))
}
#[test]
fn cancel_keeps_an_open_text_editor_when_the_dialog_did_not_change_its_text() {
let (mut app, lid) = editing_app();
open(&mut app, &json!({"layer": lid, "style": BLENDING})).unwrap();
// A live preview edit changes the project but not the edited Source Text.
app.session.execute("prop.set", json!({"layer": lid, "path": "transform/opacity", "value": 40})).unwrap();
let before = editor_state(&app);
assert!(before.is_some());
finish(&mut app, false);
assert_eq!(editor_state(&app), before, "Cancel keeps an editor whose document it did not replace");
}
#[test]
fn cancel_without_a_dialog_edit_keeps_the_open_editor_and_its_document_untouched() {
let (mut app, lid) = editing_app();
open(&mut app, &json!({"layer": lid, "style": BLENDING})).unwrap();
let before = editor_state(&app);
let project = app.session.project.clone();
let undo = app.session.history.undo.len();
assert!(before.is_some());
finish(&mut app, false);
assert!(std::sync::Arc::ptr_eq(&app.session.project, &project), "a no-op Cancel restores the same project");
assert_eq!(app.session.history.undo.len(), undo, "and the same undo history");
assert_eq!(editor_state(&app), before, "and leaves the open editor untouched");
}
#[test]
fn cancel_replaces_an_open_text_editor_when_the_preview_changed_its_text() {
let (mut app, lid) = editing_app();
open(&mut app, &json!({"layer": lid, "style": BLENDING})).unwrap();
app.session.execute("layer.setText", json!({"layer": lid, "text": "ab"})).unwrap();
let preview = editor_state(&app).expect("still editing");
assert_eq!(layer_doc(&app.session, LayerId(lid)).unwrap().text, "ab");
finish(&mut app, false);
let restored = editor_state(&app).expect("the editor survives the rollback");
assert_eq!(layer_doc(&app.session, LayerId(lid)).unwrap().text, "abcdef", "Cancel restores the pre-dialog text");
assert_ne!(restored.1, preview.1, "the restored editor starts a new epoch");
assert_eq!((restored.2, restored.3), (2, 2), "the caret stays in the restored document");
}
}
+135 -3
View File
@@ -14,7 +14,7 @@
//! Every change runs an engine command (`text.*`, `layer.setText`, `edit.*`), so it's undoable and
//! agents can do the same over the control channel.
use effectcraft_engine::commands::text_edit::{ImeSurrounding, MAX_IME_TEXT_BYTES, ime_surrounding, layer_doc};
use effectcraft_engine::commands::text_edit::{ImeSurrounding, MAX_IME_TEXT_BYTES, ime_stamp_revision, ime_surrounding, layer_doc};
use effectcraft_engine::geom::{Mat3, vec2 as gv2};
use effectcraft_engine::keyframe::{Kerning, TextDoc};
use effectcraft_engine::project::{Layer, LayerId, LayerSource};
@@ -334,7 +334,10 @@ fn ime_surrounding_cached(app: &EffectcraftApp, ctx: &egui::Context) -> Option<I
fn ime_stamp(app: &EffectcraftApp, ctx: &egui::Context) -> egui::ImeMutationStamp {
match ime_surrounding_cached(app, ctx) {
Some(w) => egui::ImeMutationStamp::from_committed(&w.text, w.cursor, w.anchor, None),
None => egui::ImeMutationStamp::unavailable_at_revision(app.session.revision),
// A global project revision would make an unrelated edit (an opacity undo) look like a
// mutation of a wide selection. The editor revision only moves with committed text and
// the editor session.
None => egui::ImeMutationStamp::unavailable_at_revision(ime_stamp_revision(&app.session)),
}
}
@@ -524,7 +527,11 @@ pub(crate) fn finish_ime(app: &mut EffectcraftApp, ui: &egui::Ui) {
(Some(target), Some((_, _rect, _cursor_rect))) => {
let window = ime_surrounding_cached(app, ctx);
let committed = window.as_ref().map(|w| egui::CommittedText::new(&w.text, w.cursor, w.anchor));
app.ime_tracker.observe(viewport, Some(egui::ImeObservation::new(frame, pass, target).committed(committed)))
// An unavailable observation still carries the editor revision: it is the only
// identity an IME receipt has when the selection is wider than the platform limit,
// and it must match `ime_stamp`'s revision exactly.
let observation = egui::ImeObservation::new(frame, pass, target).committed(committed).unavailable_revision(ime_stamp_revision(&app.session));
app.ime_tracker.observe(viewport, Some(observation))
}
_ => {
app.ime_tracker.observe(viewport, None);
@@ -1737,6 +1744,131 @@ mod tests {
assert_eq!(wide.owner, ime_target(&app).expect("an edit target").owner, "identity survives unavailable text");
}
#[test]
fn an_unrelated_undo_keeps_a_wide_selection_composition() {
// A selection wider than the IME limit reports unavailable surrounding text, whose
// receipt identity is a revision. That revision must follow the editor, not the whole
// project: undoing an unrelated opacity change used to look like an editor mutation and
// cancel an active composition.
let text = "a".repeat(5000);
let (mut app, lid) = editor(&text, 0, 5000);
let mut pass = ImePass::new(egui::ViewportId::ROOT);
let first = pass.frame(&mut app, vec![], true, false).expect("editing");
assert!(matches!(first.surrounding_text, egui::output::IMESurroundingText::Unavailable), "the wide selection is unavailable");
let composing = pass
.frame(&mut app, vec![egui::Event::Ime(egui::ImeEvent::Preedit { text: "に".into(), active_range_chars: None })], true, false)
.expect("still editing");
assert_eq!(composing.context_generation, first.context_generation, "the pre-edit keeps the context");
let owner = ime_owner(&app).expect("an editor owner");
assert!(matches!(
pass.ctx.data(|d| d.get_temp::<ImeComposition>(ime_composition_id(pass.viewport))),
Some(ImeComposition::Active(active)) if active == owner
));
app.session.execute("prop.set", json!({"layer": lid.0, "path": "transform/opacity", "value": 50})).unwrap();
app.session.execute("edit.undo", json!({})).unwrap();
let after = pass.frame(&mut app, vec![], true, false).expect("still editing");
assert_eq!(after.context_generation, first.context_generation, "an unrelated undo does not start a new context");
assert!(
matches!(
pass.ctx.data(|d| d.get_temp::<ImeComposition>(ime_composition_id(pass.viewport))),
Some(ImeComposition::Active(active)) if active == owner
),
"the wide-selection composition survives an unrelated undo"
);
assert!(read_reset(&pass.ctx).is_none(), "no IME reset was requested");
// The composition is still the live one: its commit is accepted and earns the
// input-method cause rather than being poisoned as leftover application work.
let commit = pass.frame(&mut app, vec![egui::Event::Ime(egui::ImeEvent::Commit("x".into()))], true, false).expect("still editing");
assert_eq!(doc_text(&app, lid), "x", "the preserved composition commits into the wide selection");
assert_eq!(commit.change_cause, egui::ImeChangeCause::InputMethod);
}
#[test]
fn a_failed_batch_with_a_transient_edit_does_not_revive_a_wide_selection_composition() {
// The final document equals the pre-batch one, but the intermediate insert/delete was a
// real document change. Its IME revision must stay the high-water mark; otherwise the
// active wide-selection composition would look current and accept its old commit.
let text = "a".repeat(5000);
let (mut app, lid) = editor(&text, 0, 5000);
let mut pass = ImePass::new(egui::ViewportId::ROOT);
pass.frame(&mut app, vec![], true, false).expect("editing");
pass.frame(&mut app, vec![egui::Event::Ime(egui::ImeEvent::Preedit { text: "に".into(), active_range_chars: None })], true, false)
.expect("still editing");
let owner = ime_owner(&app).expect("an editor owner");
assert!(matches!(
pass.ctx.data(|d| d.get_temp::<ImeComposition>(ime_composition_id(pass.viewport))),
Some(ImeComposition::Active(active)) if active == owner
));
let r = app.session.execute(
"engine.batch",
json!({"atomic": true, "steps": [
{"command": "text.insert", "params": {"text": "x", "range": [5000, 5000]}},
{"command": "text.delete", "params": {}},
{"command": "no.suchCommand", "params": {}}
]}),
);
assert!(r.is_err());
assert_eq!(doc_text(&app, lid), text, "the batch restored the final document");
pass.frame(&mut app, vec![], true, false).expect("still editing");
assert!(read_reset(&pass.ctx).is_some(), "the advanced revision retires the old IME context");
assert!(
matches!(pass.ctx.data(|d| d.get_temp::<ImeComposition>(ime_composition_id(pass.viewport))), Some(ImeComposition::Stale)),
"the failed batch's transient edit leaves the old composition stale"
);
let stale = pass.frame(&mut app, vec![egui::Event::Ime(egui::ImeEvent::Commit("z".into()))], true, false).expect("still editing");
assert_eq!(doc_text(&app, lid), text, "the stale composition cannot commit into the restored document");
assert_ne!(stale.change_cause, egui::ImeChangeCause::InputMethod, "a rejected commit earns no IME receipt");
}
#[test]
fn a_wide_content_change_invalidates_the_active_composition() {
let text = "a".repeat(5000);
let (mut app, lid) = editor(&text, 0, 5000);
let mut pass = ImePass::new(egui::ViewportId::ROOT);
pass.frame(&mut app, vec![], true, false).expect("editing");
pass.frame(&mut app, vec![egui::Event::Ime(egui::ImeEvent::Preedit { text: "に".into(), active_range_chars: None })], true, false)
.expect("still editing");
let replaced = "b".repeat(5000);
app.session.execute("layer.setText", json!({"layer": lid.0, "text": replaced})).unwrap();
let after = pass.frame(&mut app, vec![], true, false).expect("still editing");
assert_eq!(doc_text(&app, lid), replaced);
assert!(after.should_interrupt_composition, "replacing the wide document starts a new IME context");
assert!(
matches!(pass.ctx.data(|d| d.get_temp::<ImeComposition>(ime_composition_id(pass.viewport))), Some(ImeComposition::Stale)),
"the old composition is stale after the content change"
);
pass.frame(&mut app, vec![egui::Event::Ime(egui::ImeEvent::Commit("z".into()))], true, false).expect("still editing");
assert_eq!(doc_text(&app, lid), replaced, "a delayed commit from the old document is rejected");
}
#[test]
fn a_wide_selection_change_invalidates_the_active_composition() {
let text = "a".repeat(5000);
let (mut app, lid) = editor(&text, 0, 5000);
let mut pass = ImePass::new(egui::ViewportId::ROOT);
pass.frame(&mut app, vec![], true, false).expect("editing");
pass.frame(&mut app, vec![egui::Event::Ime(egui::ImeEvent::Preedit { text: "に".into(), active_range_chars: None })], true, false)
.expect("still editing");
// Still wider than the IME window limit, so this exercises the unavailable-text
// selection identity rather than merely making the surrounding text available.
app.session.execute("text.setSelection", json!({"start": 0, "end": 4500})).unwrap();
pass.frame(&mut app, vec![], true, false).expect("still editing");
assert!(read_reset(&pass.ctx).is_some(), "an application selection change retires the composition");
assert!(
matches!(pass.ctx.data(|d| d.get_temp::<ImeComposition>(ime_composition_id(pass.viewport))), Some(ImeComposition::Stale)),
"the old composition cannot follow the selection"
);
pass.frame(&mut app, vec![egui::Event::Ime(egui::ImeEvent::Commit("z".into()))], true, false).expect("still editing");
assert_eq!(doc_text(&app, lid), text, "the stale composition cannot mutate the wide selection");
}
#[test]
fn replaced_edit_session_and_hidden_passes_start_a_new_context() {
let (mut app, _) = editor("text", 4, 4);