From e24ea4cb3b1858230ac00be475c680b9ab796d23 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 2 Nov 2026 22:13:44 -0800 Subject: [PATCH 1/7] chore(quest): claim quest/m0/outside-edits.md Quest-Claim: cda75ac205cf1421 From 12b486807682c395c462fa2951fd83d60db2983e Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 3 Nov 2026 06:37:20 -0800 Subject: [PATCH 2/7] feat(studio): reload or ask on an outside edit to the scene file Autosave compares the scene file with the bytes it last read or wrote, every second, before each write, and right before each write's rename. With nothing unsaved it takes the outside edit; otherwise, or when the edit cannot be loaded, it pauses and asks whether to keep the studio's version or take the file's. Co-Authored-By: Claude Opus 5.5 --- README.md | 13 +- quest/m0/README.md | 1 - quest/m0/outside-edits.md | 31 -- rs/moq-studio/src/autosave.rs | 627 +++++++++++++++++++++++++--------- rs/moq-studio/src/engine.rs | 35 +- rs/moq-studio/src/ui.rs | 58 +++- 6 files changed, 557 insertions(+), 208 deletions(-) delete mode 100644 quest/m0/outside-edits.md diff --git a/README.md b/README.md index 0cf243cd..a450c47d 100644 --- a/README.md +++ b/README.md @@ -147,9 +147,16 @@ leaving half of it, and a collection that has not changed is not rewritten, so a hand-written file keeps its comments and layout until the next edit in the studio. A failed write shows in the status bar and is retried every second; until one succeeds, a crash loses every edit since the -last write. To edit the file by hand while the studio runs, save it and pick -**File > Reload**; an edit in the studio before that overwrites it. A new -canvas size or frame rate takes a restart. +last write. A new canvas size or frame rate takes a restart. + +The file can be edited by hand, or by another studio, while the studio runs. +The studio looks at it every second and before each write. With no edits of +its own unsaved, it reloads the change, as **File > Reload** does, which +replaces even unsaved edits. If both sides changed, or the change cannot be +loaded, the studio saves nothing until you pick **Keep the studio's**, which +writes its version over the file, or **Take the file's**; quitting before you +pick saves nothing. One overwrite remains possible: a change saved in the +instant between the studio's last look and its write replacing the file. The last 100 edits are written the same way beside the scene file, as `show.history.toml` for `show.toml`, after the scene file, and opening the diff --git a/quest/m0/README.md b/quest/m0/README.md index c21f3500..dad87156 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -84,7 +84,6 @@ repository. - [Viewer-side composition](/quest/m0/compose/README.md) - sources as their own tracks plus a layout track, composed by viewers, with a server-composed program as the fallback - [Worker threads fail loud at startup](/quest/m0/thread-start.md) - a worker that cannot start fails the studio's start instead of logging - [The program encoder drains](/quest/m0/program-drain.md) - a hardware encoder never loses the program's last frames at exit or on pause -- [Outside edits to the scene file](/quest/m0/outside-edits.md) - autosave reloads or asks instead of overwriting a change made outside the studio - [Scene editing](/quest/m0/scene-editing.md) - rename and reorder scenes, and resize the canvas while live through a new rendition - [Broadcast sources play in sync](/quest/m0/broadcast-sync.md) - subscribed broadcasts and guests play audio and video on the publisher's timestamps, with `moq play`'s delay - [Wayland display and window sources](/quest/m0/wayland-sources.md) - portal-picked sources that reopen without the picker diff --git a/quest/m0/outside-edits.md b/quest/m0/outside-edits.md deleted file mode 100644 index a9a16d93..00000000 --- a/quest/m0/outside-edits.md +++ /dev/null @@ -1,31 +0,0 @@ -# [S] Outside edits to the scene file - -## Goal - -Autosave does not silently overwrite a change made to the scene file outside -the studio, by hand or by another studio on the same file, except in the short -window between its last check and its rename, which `README.md` names. -When the studio has nothing unsaved, it reloads the outside change without -asking. -When both sides changed, autosave pauses and the studio asks whether to keep -its own version or take the file's; nothing is written until the user picks. - -## Plan - -Decided 2026-11-02 (maintainer, follow-up planning after -[#4](https://github.com/moq-dev/studio/pull/4)): the silent reload and the -keep-mine or take-theirs prompt above. Today the last writer wins, which -`README.md` documents; that note goes away with this quest. - -- Detect a change by what the studio last wrote or read, not by timing, so the - studio's own atomic rename never reads as an outside edit. -- Check again right before the autosave's rename, not only before its write, - since another studio can write in between. A window remains between that - check and the rename. No lock binds an arbitrary editor, so this is - best-effort; an advisory lock between studios is worth it only if it works - on every platform and removes the studio-to-studio half of the window. -- An outside edit that does not parse is refused like File > Reload refuses - it: the studio keeps its state, says why, and does not overwrite the file - until the user picks. -- The undo history file follows the scene file: a reload already replays or - drops its entries, and "take theirs" goes through that path. diff --git a/rs/moq-studio/src/autosave.rs b/rs/moq-studio/src/autosave.rs index 6adde967..4845089e 100644 --- a/rs/moq-studio/src/autosave.rs +++ b/rs/moq-studio/src/autosave.rs @@ -5,6 +5,12 @@ //! unsaved one if the edits never pause. A write compares each serialized file //! with what it holds, so a change undone before it is written rewrites only //! the history, and an undo that leaves the history as written writes nothing. +//! +//! The scene file can also change outside the studio, by hand or by another +//! studio. The studio looks for that every [`POLL`], before each write, and +//! again right before each write's rename. It takes such a change when it has +//! nothing unsaved, and otherwise holds it as a [`Conflict`], writing nothing +//! until the user keeps the studio's version or takes the file's. use std::io::Write; use std::path::{Path, PathBuf}; @@ -24,6 +30,9 @@ pub const QUIET: Duration = Duration::from_secs(1); /// drag is still written: the most edits a crash loses. pub const LIMIT: Duration = Duration::from_secs(5); +/// How often the scene file is read for a change made outside the studio. +pub const POLL: Duration = Duration::from_secs(1); + /// The edits not yet written, shared by the studio's edit path and the autosave. #[derive(Clone, Default)] pub struct Edits(kio::Shared>); @@ -69,6 +78,19 @@ impl Edits { .await; unsaved.expect("waited for an edit").deadline() } + + /// Wait until the unsaved edits are due. + async fn due(&self) { + let mut deadline = self.next().await; + // Another edit only moves the deadline later, so sleep until it holds. + loop { + kio::clock::sleep_until(deadline).await; + match self.deadline() { + Some(next) if next > deadline => deadline = next, + _ => return, + } + } + } } /// The scene file and the history beside it, serialized as the studio writes @@ -96,6 +118,24 @@ pub struct Loaded { pub dropped: usize, } +/// The collection and history the autosave writes, and replaces with what the +/// files hold on a reload. +pub trait Document { + fn text(&self) -> anyhow::Result; + + /// Swap in what the files hold, or refuse it and keep what is there. + fn replace(&mut self, loaded: Loaded) -> anyhow::Result<()>; +} + +/// A change to the scene file made outside the studio that it has not taken, +/// since it has unsaved edits of its own or cannot load the file. The +/// autosave writes nothing until the user picks a version. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Conflict { + /// Why the file's version cannot be loaded, if it cannot. + pub refused: Option, +} + pub struct Autosave { path: PathBuf, /// Where the history is saved, beside the scene file. @@ -106,36 +146,49 @@ pub struct Autosave { edits: Edits, /// Why the last write failed, until one succeeds. error: Mutex>, + conflict: Mutex>, } struct Written { - /// `None` until the scene file exists. + /// `None` until the scene file exists, or when it must be rewritten. collection: Option, /// An empty history when there is no history file, so none is created - /// until there is an edit to undo. - history: String, + /// until there is an edit to undo. `None` when it must be rewritten. + history: Option, + /// Exactly what the scene file held when the studio last read or wrote + /// it, so anything else there is an outside edit. Compared by content, + /// not time, so the studio's own write never reads as one. + disk: Option>, +} + +impl Written { + /// Whether the files hold `text`, so the studio has nothing unsaved. + fn holds(&self, text: &Text) -> bool { + self.collection.as_ref() == Some(&text.collection) && self.history.as_ref() == Some(&text.history) + } } impl Autosave { /// Read the scene file at `path` and its history, or start a new /// collection if there is no scene file yet; the first save creates it. pub fn open(path: PathBuf) -> anyhow::Result<(Self, Loaded)> { - let (collection, written) = match std::fs::read_to_string(&path) { - Ok(text) => { - let collection = parse(&path, &text)?; + let disk = read(&path)?; + let (collection, written) = match &disk { + Some(bytes) => { + let collection = parse(&path, bytes)?; // Kept as the studio would write it, not as the file spells it, so // a hand-written file is rewritten only once the collection changes. let written = collection.to_toml()?; (collection, Some(written)) } - Err(err) if err.kind() == std::io::ErrorKind::NotFound => (Collection::new(), None), - Err(err) => return Err(err).with_context(|| format!("reading {}", path.display())), + None => (Collection::new(), None), }; let history = history_path(&path); let loaded = load_history(&history, collection)?; let written = Written { collection: written, - history: loaded.history.to_toml()?, + history: Some(loaded.history.to_toml()?), + disk, }; let autosave = Self { path, @@ -143,6 +196,7 @@ impl Autosave { written: Mutex::new(written), edits: Edits::default(), error: Mutex::new(None), + conflict: Mutex::new(None), }; Ok((autosave, loaded)) } @@ -161,25 +215,26 @@ impl Autosave { self.error.lock().unwrap().clone() } - /// Write each file `serialize` returns that does not already hold it: the - /// scene file, then the history, so a history never describes edits the - /// scene file lacks. A failure stays in [`Self::error`] until a write - /// succeeds. - pub fn save(&self, serialize: impl FnOnce() -> anyhow::Result) -> anyhow::Result<()> { + /// The outside edit holding up the autosave, until the user picks a + /// version or the studio can take it. + pub fn conflict(&self) -> Option { + self.conflict.lock().unwrap().clone() + } + + /// Write each file of `doc` that does not already hold it: the scene file, + /// then the history, so a history never describes edits the scene file + /// lacks. An outside edit to the scene file is taken instead when `doc` + /// has nothing unsaved, and otherwise fails the save as a [`Conflict`]. A + /// failure stays in [`Self::error`] until a write succeeds. + pub fn save(&self, doc: &Mutex) -> anyhow::Result<()> { let mut written = self.written.lock().unwrap(); + self.save_locked(&mut written, doc) + } + + fn save_locked(&self, written: &mut Written, doc: &Mutex) -> anyhow::Result<()> { // Before serializing, so an edit made after it is recorded anew. self.edits.0.lock().take(); - let result = serialize().and_then(|text| { - if written.collection.as_ref() != Some(&text.collection) { - write(&self.path, &text.collection).with_context(|| format!("writing {}", self.path.display()))?; - written.collection = Some(text.collection); - } - if written.history != text.history { - write(&self.history, &text.history).with_context(|| format!("writing {}", self.history.display()))?; - written.history = text.history; - } - Ok(()) - }); + let result = self.write(written, doc); let error = result.as_ref().err().map(|err| format!("{err:#}")); let mut last = self.error.lock().unwrap(); @@ -195,42 +250,159 @@ impl Autosave { result } - /// Write the files that `serialize` returns now, which creates a missing - /// scene file, and then whenever the recorded edits are due. - pub async fn run(&self, serialize: impl Fn() -> anyhow::Result) { + fn write(&self, written: &mut Written, doc: &Mutex) -> anyhow::Result<()> { loop { - // Kept for the status bar until a save succeeds. - let _ = self.save(&serialize); - let mut deadline = self.edits.next().await; - // Another edit only moves the deadline later, so sleep until it holds. - loop { - kio::clock::sleep_until(deadline).await; - match self.edits.deadline() { - Some(next) if next > deadline => deadline = next, - _ => break, + self.look(written, doc)?; + let text = doc.lock().unwrap().text()?; + if written.collection.as_ref() != Some(&text.collection) { + // Checked again right before the rename, since another studio may + // have written the file since the look above. + let unchanged = |path: &Path| Ok(outside(path, written.disk.as_deref())?.is_none()); + let wrote = write(&self.path, &text.collection, unchanged) + .with_context(|| format!("writing {}", self.path.display()))?; + if !wrote { + // Taken or held as a conflict by the next look. + continue; } + written.disk = Some(text.collection.clone().into_bytes()); + written.collection = Some(text.collection); + } + if written.history.as_ref() != Some(&text.history) { + write(&self.history, &text.history, |_| Ok(true)) + .with_context(|| format!("writing {}", self.history.display()))?; + written.history = Some(text.history); } + return Ok(()); } } - /// Read the scene file and its history and hand them to `replace`. Once - /// they are accepted, the files count as holding them. - pub fn reload(&self, replace: impl FnOnce(Loaded) -> anyhow::Result<()>) -> anyhow::Result<()> { + /// Look for an outside edit to the scene file now, without writing. + pub fn check(&self, doc: &Mutex) -> anyhow::Result<()> { let mut written = self.written.lock().unwrap(); - let text = std::fs::read_to_string(&self.path).with_context(|| format!("reading {}", self.path.display()))?; - let collection = parse(&self.path, &text)?; - let loaded = load_history(&self.history, collection)?; + self.look(&mut written, doc) + } + + /// Take an outside edit to the scene file if `doc` has nothing unsaved and + /// accepts it, or else hold it as a [`Conflict`] and fail. + fn look(&self, written: &mut Written, doc: &Mutex) -> anyhow::Result<()> { + let Some(bytes) = + outside(&self.path, written.disk.as_deref()).with_context(|| format!("reading {}", self.path.display()))? + else { + *self.conflict.lock().unwrap() = None; + return Ok(()); + }; + + let loaded = parse(&self.path, &bytes).and_then(|collection| load_history(&self.history, collection)); + let refused = match loaded { + Ok(loaded) => { + let mut doc = doc.lock().unwrap(); + if written.holds(&doc.text()?) { + match self.take(written, &mut *doc, loaded, bytes) { + Ok(()) => return Ok(()), + Err(err) => Some(err), + } + } else { + None + } + } + Err(err) => Some(err), + }; + + let conflict = Conflict { + refused: refused.map(|err| format!("{err:#}")), + }; + let mut last = self.conflict.lock().unwrap(); + if last.as_ref() != Some(&conflict) { + tracing::warn!(refused = conflict.refused.as_deref(), path = %self.path.display(), "the scene file changed outside the studio"); + } + *last = Some(conflict); + anyhow::bail!("{} changed outside the studio", self.path.display()) + } + + /// Hand `loaded`, read as `bytes` from the scene file, to `doc`. Once it + /// is accepted, the files count as holding it. + fn take( + &self, + written: &mut Written, + doc: &mut impl Document, + loaded: Loaded, + bytes: Vec, + ) -> anyhow::Result<()> { let text = Text::new(&loaded.collection, &loaded.history)?; - replace(loaded)?; + doc.replace(loaded)?; *written = Written { collection: Some(text.collection), - history: text.history, + history: Some(text.history), + disk: Some(bytes), }; + *self.conflict.lock().unwrap() = None; Ok(()) } + + /// Replace `doc` with what the scene file and its history hold, whatever + /// it has unsaved: the user taking the file's version. + pub fn reload(&self, doc: &Mutex) -> anyhow::Result<()> { + let mut written = self.written.lock().unwrap(); + let bytes = std::fs::read(&self.path).with_context(|| format!("reading {}", self.path.display()))?; + let collection = parse(&self.path, &bytes)?; + let loaded = load_history(&self.history, collection)?; + self.take(&mut written, &mut *doc.lock().unwrap(), loaded, bytes) + } + + /// Write `doc` over whatever the scene file and its history hold now: the + /// user keeping the studio's version. + pub fn keep(&self, doc: &Mutex) -> anyhow::Result<()> { + let mut written = self.written.lock().unwrap(); + written.disk = read(&self.path)?; + // The history beside an outside edit may be another studio's. + written.collection = None; + written.history = None; + self.save_locked(&mut written, doc) + } + + /// Write `doc`'s files now, which creates a missing scene file, and then + /// whenever the recorded edits are due, looking for an outside edit every + /// [`POLL`] in between. + pub async fn run(&self, doc: &Mutex) { + // Kept for the status bar until a save succeeds. + let _ = self.save(doc); + loop { + tokio::select! { + () = self.edits.due() => { + let _ = self.save(doc); + } + () = kio::clock::sleep(POLL) => { + // A conflict shows for itself. + let _ = self.check(doc); + } + } + } + } } -fn parse(path: &Path, text: &str) -> anyhow::Result { +/// Read the file at `path`, or `None` if there is none. +fn read(path: &Path) -> anyhow::Result>> { + match std::fs::read(path) { + Ok(bytes) => Ok(Some(bytes)), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(err) => Err(err).with_context(|| format!("reading {}", path.display())), + } +} + +/// What the scene file at `path` holds if it is not `disk`, what the studio +/// last read or wrote there. A file deleted since holds nothing to keep, so +/// the next write recreates it. +fn outside(path: &Path, disk: Option<&[u8]>) -> std::io::Result>> { + match std::fs::read(path) { + Ok(bytes) if Some(bytes.as_slice()) != disk => Ok(Some(bytes)), + Ok(_) => Ok(None), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(err) => Err(err), + } +} + +fn parse(path: &Path, bytes: &[u8]) -> anyhow::Result { + let text = std::str::from_utf8(bytes).with_context(|| format!("loading {}", path.display()))?; Collection::from_toml(text).with_context(|| format!("loading {}", path.display())) } @@ -262,8 +434,10 @@ fn load_history(path: &Path, collection: Collection) -> anyhow::Result { } /// Replace the file at `path` with `text` all at once, so a crash leaves -/// either the old file or the new one, never part of either. -fn write(path: &Path, text: &str) -> std::io::Result<()> { +/// either the old file or the new one, never part of either. Returns `false`, +/// leaving the file as it is, if `unchanged` says otherwise of the file it +/// would replace right before the rename. +fn write(path: &Path, text: &str, unchanged: impl FnOnce(&Path) -> std::io::Result) -> std::io::Result { let path = resolve(path)?; let existing = match std::fs::metadata(&path) { Ok(metadata) => { @@ -302,11 +476,16 @@ fn write(path: &Path, text: &str) -> std::io::Result<()> { // On disk before the rename, so a power loss cannot leave the new name // pointing at an empty file. file.as_file().sync_all()?; + // As late as it can be. An edit landing between this and the rename is + // still lost, since nothing binds every editor to a lock. + if !unchanged(&path)? { + return Ok(false); + } file.persist(&path)?; // And the rename itself, so a power loss cannot undo the save. #[cfg(unix)] std::fs::File::open(dir)?.sync_all()?; - Ok(()) + Ok(true) } /// The file a chain of symlinks at `path` ends in, which need not exist yet, @@ -344,22 +523,59 @@ mod tests { collection } - /// A new collection with scene `scene` added, and the history of that edit. - fn edited(scene: &str) -> (Collection, History) { - let mut collection = Collection::new(); - let mut history = History::new(); - let mut draft = collection.draft(format!("Add scene `{scene}`")); - draft.insert_scene(1, moq_scene::Scene::new(scene)).unwrap(); - let edit = draft.finish().unwrap(); - collection.apply(&edit).unwrap(); - history.record(edit, None); - (collection, history) + /// A collection and its history as the studio holds them, which can be + /// told to refuse a reload as the studio refuses a new canvas. + struct Doc { + collection: Collection, + history: History, + refuse: bool, } - /// `collection` with no history, which leaves the history file unwritten. - fn serialize(collection: &Collection) -> impl FnOnce() -> anyhow::Result { - let text = Text::new(collection, &History::new()); - move || text + impl Doc { + fn new(collection: Collection) -> Mutex { + Mutex::new(Self { + collection, + history: History::new(), + refuse: false, + }) + } + + /// Add scene `scene`, as an edit in the studio would. + fn edit(&mut self, scene: &str) { + let mut draft = self.collection.draft(format!("Add scene `{scene}`")); + draft + .insert_scene(self.collection.scenes.len(), moq_scene::Scene::new(scene)) + .unwrap(); + let edit = draft.finish().unwrap(); + self.collection.apply(&edit).unwrap(); + self.history.record(edit, None); + } + } + + impl Document for Doc { + fn text(&self) -> anyhow::Result { + Text::new(&self.collection, &self.history) + } + + fn replace(&mut self, loaded: Loaded) -> anyhow::Result<()> { + anyhow::ensure!(!self.refuse, "refused"); + self.collection = loaded.collection; + self.history = loaded.history; + Ok(()) + } + } + + fn read(path: &Path) -> String { + std::fs::read_to_string(path).unwrap() + } + + fn names(dir: &Path) -> Vec { + let mut names: Vec<_> = std::fs::read_dir(dir) + .unwrap() + .map(|e| e.unwrap().file_name()) + .collect(); + names.sort(); + names } #[test] @@ -369,23 +585,15 @@ mod tests { let (autosave, Loaded { collection: opened, .. }) = Autosave::open(path.clone()).unwrap(); assert_eq!(opened, Collection::new()); - autosave.save(serialize(&opened)).unwrap(); - assert_eq!(std::fs::read_to_string(&path).unwrap(), opened.to_toml().unwrap()); - - // The file is not touched while the collection matches what was written. - std::fs::write(&path, "elsewhere").unwrap(); - autosave.save(serialize(&opened)).unwrap(); - assert_eq!(std::fs::read_to_string(&path).unwrap(), "elsewhere"); + let doc = Doc::new(opened); + autosave.save(&doc).unwrap(); + assert_eq!(read(&path), Collection::new().to_toml().unwrap()); - let edited = collection("Second"); - autosave.save(serialize(&edited)).unwrap(); - assert_eq!(std::fs::read_to_string(&path).unwrap(), edited.to_toml().unwrap()); - let names: Vec<_> = std::fs::read_dir(dir.path()) - .unwrap() - .map(|e| e.unwrap().file_name()) - .collect(); + doc.lock().unwrap().collection = collection("Second"); + autosave.save(&doc).unwrap(); + assert_eq!(read(&path), collection("Second").to_toml().unwrap()); assert_eq!( - names, + names(dir.path()), ["show.toml"], "the temporary file is renamed into place, and an empty history is not written" ); @@ -403,31 +611,32 @@ mod tests { std::fs::File::create(&plain).unwrap(); let path = dir.path().join("show.toml"); let (autosave, Loaded { collection: opened, .. }) = Autosave::open(path.clone()).unwrap(); - autosave.save(serialize(&opened)).unwrap(); + autosave.save(&Doc::new(opened)).unwrap(); let mode = |path: &Path| std::fs::metadata(path).unwrap().permissions().mode() & 0o777; assert_eq!(mode(&path), mode(&plain)); } + const HAND_WRITTEN: &str = "# my show\nversion = 1\ncanvas = { width = 1280, height = 720, framerate = 30 }\n\n[[scenes]]\nname = \"Main\"\n"; + /// A hand-written file keeps its comments and layout until the collection /// it describes changes. #[test] fn leaves_a_hand_written_file_alone_until_an_edit() { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("show.toml"); - let text = "# my show\nversion = 1\ncanvas = { width = 1280, height = 720, framerate = 30 }\n\n[[scenes]]\nname = \"Main\"\n"; - std::fs::write(&path, text).unwrap(); + std::fs::write(&path, HAND_WRITTEN).unwrap(); let (autosave, Loaded { collection: opened, .. }) = Autosave::open(path.clone()).unwrap(); - autosave.save(serialize(&opened)).unwrap(); - assert_eq!(std::fs::read_to_string(&path).unwrap(), text); + autosave.save(&Doc::new(opened)).unwrap(); + assert_eq!(read(&path), HAND_WRITTEN); } /// An autosave running over a collection the test edits, as the engine's /// does over the studio's. struct Running { autosave: Arc, - state: Arc>, + doc: Arc>, task: tokio::task::JoinHandle<()>, } @@ -435,52 +644,44 @@ mod tests { async fn start(path: PathBuf) -> Self { let (autosave, opened) = Autosave::open(path).unwrap(); let autosave = Arc::new(autosave); - let state = Arc::new(Mutex::new(opened)); + let doc = Arc::new(Mutex::new(Doc { + collection: opened.collection, + history: opened.history, + refuse: false, + })); let task = tokio::spawn({ let autosave = autosave.clone(); - let state = state.clone(); - async move { - autosave - .run(|| { - let state = state.lock().unwrap(); - Text::new(&state.collection, &state.history) - }) - .await - } + let doc = doc.clone(); + async move { autosave.run(&doc).await } }); // Its first save, at once. tokio::task::yield_now().await; - Self { autosave, state, task } + Self { autosave, doc, task } } /// Add a scene, as an edit in the studio would, and return the /// collection it leaves serialized. fn edit(&self, scene: &str) -> String { - let mut state = self.state.lock().unwrap(); - let Loaded { - collection, history, .. - } = &mut *state; - let mut draft = collection.draft(format!("Add scene `{scene}`")); - draft - .insert_scene(collection.scenes.len(), moq_scene::Scene::new(scene)) - .unwrap(); - let edit = draft.finish().unwrap(); - collection.apply(&edit).unwrap(); - history.record(edit, None); + let mut doc = self.doc.lock().unwrap(); + doc.edit(scene); self.autosave.edits().record(); - collection.to_toml().unwrap() + doc.collection.to_toml().unwrap() } /// Undo the newest edit, as the studio would, and return the files it /// leaves serialized. fn undo(&self) -> Text { - let mut state = self.state.lock().unwrap(); - let Loaded { + let mut doc = self.doc.lock().unwrap(); + let Doc { collection, history, .. - } = &mut *state; + } = &mut *doc; history.undo(collection).unwrap().expect("an edit to undo"); self.autosave.edits().record(); - Text::new(collection, history).unwrap() + doc.text().unwrap() + } + + fn text(&self) -> Text { + self.doc.lock().unwrap().text().unwrap() } } @@ -490,11 +691,8 @@ mod tests { } } - fn read(path: &Path) -> String { - std::fs::read_to_string(path).unwrap() - } - - /// A burst of edits is written once, a quiet second after the last. + /// A burst of edits is written once, a quiet second after the last, and + /// the studio's own writes never read as an outside edit. #[tokio::test(start_paused = true)] async fn writes_once_the_edits_pause() { let dir = tempfile::tempdir().unwrap(); @@ -510,6 +708,12 @@ mod tests { assert_eq!(read(&path), created, "written before the edits paused"); kio::clock::sleep(Duration::from_millis(200)).await; assert_eq!(read(&path), last); + + let last = running.edit("C"); + kio::clock::sleep(QUIET + POLL * 2).await; + assert_eq!(read(&path), last); + assert_eq!(running.autosave.conflict(), None); + assert_eq!(running.autosave.error(), None); } /// Edits that never pause are still written, [`LIMIT`] after the first. @@ -550,33 +754,142 @@ mod tests { assert!(path.exists()); } - /// A reload the studio refuses leaves the file as it is, so the next save - /// does not overwrite the hand edit that was refused. - #[test] - fn a_refused_reload_keeps_the_file() { + /// With nothing unsaved, an outside edit is taken within a poll, its + /// history with it, and left as written until the next edit. + #[tokio::test(start_paused = true)] + async fn takes_an_outside_edit_with_nothing_unsaved() { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("show.toml"); - let original = Collection::new(); - std::fs::write(&path, original.to_toml().unwrap()).unwrap(); - let (autosave, _) = Autosave::open(path.clone()).unwrap(); + let running = Running::start(path.clone()).await; - let edited = collection("Edited"); - std::fs::write(&path, edited.to_toml().unwrap()).unwrap(); - assert!(autosave.reload(|_| anyhow::bail!("refused")).is_err()); - autosave.save(serialize(&original)).unwrap(); - assert_eq!(std::fs::read_to_string(&path).unwrap(), edited.to_toml().unwrap()); - - // Accepted, the reloaded collection is what the file holds. - let mut reloaded = None; - autosave - .reload(|loaded| { - reloaded = Some(loaded.collection); - Ok(()) - }) + std::fs::write(&path, HAND_WRITTEN).unwrap(); + // Another studio's history, describing the edit that made the file. + let mut theirs = Doc::new(Collection::from_toml(HAND_WRITTEN).unwrap()) + .into_inner() .unwrap(); - std::fs::write(&path, "elsewhere").unwrap(); - autosave.save(serialize(&reloaded.unwrap())).unwrap(); - assert_eq!(std::fs::read_to_string(&path).unwrap(), "elsewhere"); + theirs.edit("Theirs"); + theirs.history.undo(&mut theirs.collection).unwrap(); + std::fs::write(history_path(&path), theirs.history.to_toml().unwrap()).unwrap(); + + kio::clock::sleep(POLL + Duration::from_millis(100)).await; + assert_eq!(running.text(), theirs.text().unwrap()); + assert_eq!(running.autosave.conflict(), None); + assert_eq!(read(&path), HAND_WRITTEN, "taken, not rewritten"); + + let edited = running.edit("Mine"); + kio::clock::sleep(QUIET + Duration::from_millis(100)).await; + assert_eq!(read(&path), edited); + } + + /// When both the studio and the file changed, nothing is written until + /// the user keeps the studio's version or takes the file's. + #[tokio::test(start_paused = true)] + async fn holds_an_outside_edit_against_unsaved_edits() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("show.toml"); + let history = history_path(&path); + let running = Running::start(path.clone()).await; + + let mine = running.edit("Mine"); + std::fs::write(&path, HAND_WRITTEN).unwrap(); + kio::clock::sleep(QUIET * 3).await; + assert_eq!(read(&path), HAND_WRITTEN); + assert!(!history.exists()); + assert_eq!(running.autosave.conflict(), Some(Conflict { refused: None })); + assert!(running.autosave.error().unwrap().contains("changed outside the studio")); + + // Keeping the studio's version writes it, and its history, over the file. + running.autosave.keep(&*running.doc).unwrap(); + assert_eq!(read(&path), mine); + assert_eq!(read(&history), running.text().history); + assert_eq!((running.autosave.conflict(), running.autosave.error()), (None, None)); + + // Taking the file's version replaces the studio's unsaved edit. + running.edit("Again"); + std::fs::write(&path, HAND_WRITTEN).unwrap(); + kio::clock::sleep(POLL + Duration::from_millis(100)).await; + assert!(running.autosave.conflict().is_some()); + running.autosave.reload(&*running.doc).unwrap(); + assert_eq!( + running.text().collection, + Collection::from_toml(HAND_WRITTEN).unwrap().to_toml().unwrap() + ); + assert_eq!(running.autosave.conflict(), None); + kio::clock::sleep(QUIET * 2).await; + assert_eq!(read(&path), HAND_WRITTEN); + } + + /// An outside edit the studio cannot load is held even with nothing + /// unsaved, so the file is not overwritten, and taken once it loads. + #[tokio::test(start_paused = true)] + async fn holds_an_outside_edit_it_cannot_load() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("show.toml"); + let running = Running::start(path.clone()).await; + let created = running.text(); + + std::fs::write(&path, "not a scene file").unwrap(); + kio::clock::sleep(POLL + Duration::from_millis(100)).await; + let refused = running.autosave.conflict().unwrap().refused.unwrap(); + assert!(refused.contains("show.toml"), "{refused}"); + assert!(running.autosave.reload(&*running.doc).is_err()); + assert_eq!(running.text(), created); + assert_eq!(read(&path), "not a scene file"); + + // One the studio refuses, as it does a new canvas, is held the same way. + running.doc.lock().unwrap().refuse = true; + std::fs::write(&path, HAND_WRITTEN).unwrap(); + kio::clock::sleep(POLL + Duration::from_millis(100)).await; + let conflict = running.autosave.conflict().unwrap(); + assert_eq!(conflict.refused.as_deref(), Some("refused")); + assert_eq!(read(&path), HAND_WRITTEN); + + running.doc.lock().unwrap().refuse = false; + kio::clock::sleep(POLL + Duration::from_millis(100)).await; + assert_eq!(running.autosave.conflict(), None); + assert_eq!( + running.text().collection, + Collection::from_toml(HAND_WRITTEN).unwrap().to_toml().unwrap() + ); + } + + /// The file is checked again after the save serializes the studio's + /// version, so an outside edit landing as the save starts is not lost. + #[test] + fn checks_again_before_replacing_the_file() { + /// Another studio writes the file just as this one serializes. + struct Racing { + doc: Doc, + path: PathBuf, + } + + impl Document for Racing { + fn text(&self) -> anyhow::Result { + std::fs::write(&self.path, HAND_WRITTEN).unwrap(); + self.doc.text() + } + + fn replace(&mut self, loaded: Loaded) -> anyhow::Result<()> { + self.doc.replace(loaded) + } + } + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("show.toml"); + std::fs::write(&path, Collection::new().to_toml().unwrap()).unwrap(); + let (autosave, Loaded { collection, .. }) = Autosave::open(path.clone()).unwrap(); + let mut doc = Doc::new(collection).into_inner().unwrap(); + doc.edit("Mine"); + let racing = Mutex::new(Racing { + doc, + path: path.clone(), + }); + + let err = format!("{:#}", autosave.save(&racing).unwrap_err()); + assert!(err.contains("changed outside the studio"), "{err}"); + assert_eq!(read(&path), HAND_WRITTEN); + assert_eq!(names(dir.path()), ["show.toml"], "the temporary file is removed"); + assert_eq!(autosave.conflict(), Some(Conflict { refused: None })); } #[cfg(unix)] @@ -592,11 +905,13 @@ mod tests { std::os::unix::fs::symlink(&target, &link).unwrap(); let (autosave, _) = Autosave::open(link.clone()).unwrap(); - let edited = collection("Edited"); - autosave.save(serialize(&edited)).unwrap(); + let doc = Doc::new(collection("Edited")); + autosave.save(&doc).unwrap(); + // Written through the link, so its own write reads as no outside edit. + autosave.check(&doc).unwrap(); assert!(std::fs::symlink_metadata(&link).unwrap().file_type().is_symlink()); - assert_eq!(std::fs::read_to_string(&target).unwrap(), edited.to_toml().unwrap()); + assert_eq!(read(&target), collection("Edited").to_toml().unwrap()); let mode = std::fs::metadata(&target).unwrap().permissions().mode(); assert_eq!(mode & 0o777, 0o640); } @@ -614,11 +929,11 @@ mod tests { std::os::unix::fs::symlink(&middle, &link).unwrap(); let (autosave, Loaded { collection: opened, .. }) = Autosave::open(link.clone()).unwrap(); - autosave.save(serialize(&opened)).unwrap(); + autosave.save(&Doc::new(opened)).unwrap(); assert!(std::fs::symlink_metadata(&link).unwrap().file_type().is_symlink()); assert!(std::fs::symlink_metadata(&middle).unwrap().file_type().is_symlink()); - assert_eq!(std::fs::read_to_string(&target).unwrap(), opened.to_toml().unwrap()); + assert_eq!(read(&target), Collection::new().to_toml().unwrap()); } #[cfg(unix)] @@ -637,9 +952,10 @@ mod tests { // Nor is a history of edits the scene file lacks written beside it. let (autosave, _) = Autosave::open(path.clone()).unwrap(); - let (collection, history) = edited("Edited"); - assert!(autosave.save(|| Text::new(&collection, &history)).is_err()); - assert_eq!(std::fs::read_to_string(&path).unwrap(), text); + let doc = Doc::new(Collection::new()); + doc.lock().unwrap().edit("Edited"); + assert!(autosave.save(&doc).is_err()); + assert_eq!(read(&path), text); assert!(!history_path(&path).exists()); } @@ -650,15 +966,14 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("show.toml"); let history = dir.path().join("show.history.toml"); + std::fs::write(&path, HAND_WRITTEN).unwrap(); let running = Running::start(path.clone()).await; assert!(!history.exists(), "no history until an edit"); - // The file is not touched while the collection matches what was written. - std::fs::write(&path, "elsewhere").unwrap(); running.edit("A"); let undone = running.undo(); kio::clock::sleep(QUIET + Duration::from_millis(100)).await; - assert_eq!(read(&path), "elsewhere"); + assert_eq!(read(&path), HAND_WRITTEN); assert_eq!(read(&history), undone.history); } @@ -672,16 +987,18 @@ mod tests { let history = dir.path().join("show.history.toml"); assert_eq!(history_path(&path), history); - let (collection, saved) = edited("Two"); - std::fs::write(&path, collection.to_toml().unwrap()).unwrap(); - std::fs::write(&history, saved.to_toml().unwrap()).unwrap(); + let saved = Doc::new(Collection::new()); + saved.lock().unwrap().edit("Two"); + let saved = saved.into_inner().unwrap(); + std::fs::write(&path, saved.collection.to_toml().unwrap()).unwrap(); + std::fs::write(&history, saved.history.to_toml().unwrap()).unwrap(); let (autosave, mut loaded) = Autosave::open(path.clone()).unwrap(); - assert_eq!((&loaded.collection, loaded.dropped), (&collection, 0)); + assert_eq!((&loaded.collection, loaded.dropped), (&saved.collection, 0)); loaded.history.undo(&mut loaded.collection).unwrap(); assert_eq!(loaded.collection, Collection::new()); std::fs::write(&history, "not a history").unwrap(); - let err = format!("{:#}", autosave.reload(|_| Ok(())).unwrap_err()); + let err = format!("{:#}", autosave.reload(&Doc::new(Collection::new())).unwrap_err()); assert!(err.contains("show.history.toml"), "{err}"); let err = format!("{:#}", Autosave::open(path).err().unwrap()); assert!(err.contains("show.history.toml"), "{err}"); diff --git a/rs/moq-studio/src/engine.rs b/rs/moq-studio/src/engine.rs index d4d466b4..6d5e0428 100644 --- a/rs/moq-studio/src/engine.rs +++ b/rs/moq-studio/src/engine.rs @@ -10,7 +10,7 @@ //! - **source-***: one per running source, since a capture stream may not //! leave the thread that opened it. //! - **autosave** writes the collection and its history to their files once -//! edits to them settle. +//! edits to them settle, and takes or holds an outside edit to the scene file. //! - The UI runs on the main thread and edits [`State`] under its lock. use std::path::Path; @@ -75,6 +75,9 @@ pub struct State { /// History entries the last load dropped, since they no longer applied to /// the scene file. pub dropped: usize, + /// How many times the collection was reloaded from its files, so the UI + /// can let go of what a reload may have removed. + pub reloads: u64, pub sources: Sources, pub compositor: Compositor, /// Edited since the sources were last synced. @@ -134,6 +137,13 @@ impl State { self.stale = true; self.edits.record(); } +} + +impl autosave::Document for State { + /// Both under one lock, so the history describes the collection beside it. + fn text(&self) -> anyhow::Result { + autosave::Text::new(&self.collection, &self.history) + } /// Swap in a collection and history reloaded from their files. fn replace(&mut self, loaded: autosave::Loaded) -> anyhow::Result<()> { @@ -154,6 +164,7 @@ impl State { self.collection = collection; self.history = history; self.dropped = dropped; + self.reloads += 1; self.stale = true; Ok(()) } @@ -287,6 +298,7 @@ impl Engine { collection, history, dropped, + reloads: 0, sources, compositor, stale: true, @@ -347,28 +359,27 @@ impl Engine { let save = engine.clone(); std::thread::Builder::new() .name("autosave".into()) - .spawn(move || clocked(save.autosave.run(|| save.serialize())))?; + .spawn(move || clocked(save.autosave.run(&save.state)))?; tokio::spawn(engine.clone().grade(health::Sampler::new(publisher))); Ok(engine) } /// Write the collection and its history to their files unless they - /// already hold them. + /// already hold them, or the scene file changed outside the studio. pub fn save(&self) -> anyhow::Result<()> { - self.autosave.save(|| self.serialize()) + self.autosave.save(&self.state) } - /// Both under one lock, so the history describes the collection beside it. - fn serialize(&self) -> anyhow::Result { - let state = self.state.lock().unwrap(); - autosave::Text::new(&state.collection, &state.history) + /// Replace the collection and its history with what the files hold now, + /// dropping any unsaved edit. + pub fn reload(&self) -> anyhow::Result<()> { + self.autosave.reload(&self.state) } - /// Replace the collection and its history with what the files hold now. - pub fn reload(&self) -> anyhow::Result<()> { - self.autosave - .reload(|loaded| self.state.lock().unwrap().replace(loaded)) + /// Write the collection and its history over what the files hold now. + pub fn keep(&self) -> anyhow::Result<()> { + self.autosave.keep(&self.state) } /// Grade the stream's health every interval, for the Health panel. diff --git a/rs/moq-studio/src/ui.rs b/rs/moq-studio/src/ui.rs index 74e76e97..c6d63a8e 100644 --- a/rs/moq-studio/src/ui.rs +++ b/rs/moq-studio/src/ui.rs @@ -10,6 +10,7 @@ use egui_dock::{DockArea, DockState, NodeIndex, Style, TabViewer}; use moq_scene::source::{Feed, Status}; use moq_scene::{Clip, Collection, Draft, Filter, Item, Layer, Rect, Scene, Source, Video, wgpu}; +use crate::autosave::Conflict; use crate::engine::{Engine, State}; use crate::health::{self, Verdict}; @@ -103,6 +104,7 @@ impl Studio { add: AddSource::default(), devices: Devices::default(), message: (dropped > 0).then(|| Err(dropped_message(dropped))), + reloads: 0, }, }) } @@ -114,8 +116,12 @@ impl eframe::App for Studio { let framerate = self.panels.engine.state.lock().unwrap().collection().canvas.framerate; ui.ctx().request_repaint_after(Duration::from_secs(1) / framerate); + self.panels.follow_reloads(); self.panels.shortcuts(ui.ctx()); egui::Panel::top("menu").show(ui, |ui| self.panels.menu(ui)); + if let Some(conflict) = self.panels.engine.autosave.conflict() { + egui::Panel::top("conflict").show(ui, |ui| self.panels.conflict(ui, conflict)); + } egui::Panel::bottom("status").show(ui, |ui| self.panels.status(ui)); DockArea::new(&mut self.dock) .style(Style::from_egui(ui.style())) @@ -141,6 +147,8 @@ struct Panels { devices: Devices, /// The outcome of the last action, shown in the status bar. message: Option>, + /// The engine's reloads seen so far. + reloads: u64, } impl TabViewer for Panels { @@ -291,13 +299,8 @@ impl Panels { egui::MenuBar::new().ui(ui, |ui| { ui.menu_button("File", |ui| { if ui.button("Reload").clicked() { - self.deselect_item(); let result = self.engine.reload(); - let result = result.map(|()| match self.engine.state.lock().unwrap().dropped { - 0 => "reloaded".to_string(), - dropped => format!("reloaded; {}", dropped_message(dropped)), - }); - self.report(result); + self.check(result); } }); ui.menu_button("Edit", |ui| { @@ -319,6 +322,49 @@ impl Panels { }); } + /// Report a reload, from File > Reload or an outside edit the autosave + /// took, and let go of a selected item it may have removed. + fn follow_reloads(&mut self) { + let (reloads, dropped) = { + let state = self.engine.state.lock().unwrap(); + (state.reloads, state.dropped) + }; + if reloads == self.reloads { + return; + } + self.reloads = reloads; + self.deselect_item(); + let path = self.engine.autosave.path().display(); + self.report(Ok(match dropped { + 0 => format!("reloaded {path}"), + dropped => format!("reloaded {path}; {}", dropped_message(dropped)), + })); + } + + /// An outside edit to the scene file the studio has not taken, and the + /// choice of which version to keep. + fn conflict(&mut self, ui: &mut Ui, conflict: Conflict) { + ui.horizontal_wrapped(|ui| { + let path = self.engine.autosave.path().display().to_string(); + let why = match &conflict.refused { + None => "while the studio has unsaved edits".to_string(), + Some(refused) => format!("and cannot be loaded: {refused}"), + }; + ui.colored_label( + ui.visuals().warn_fg_color, + format!("{path} changed outside the studio {why}. Nothing is saved until you pick a version."), + ); + if ui.button("Keep the studio's").clicked() { + let result = self.engine.keep(); + self.report(result.map(|()| format!("saved the studio's version over {path}"))); + } + if ui.button("Take the file's").clicked() { + let result = self.engine.reload(); + self.check(result); + } + }); + } + fn status(&mut self, ui: &mut Ui) { ui.horizontal(|ui| { let autosave = &self.engine.autosave; From 367903ae1b53a406b411001e882facb6ac6458a7 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 3 Nov 2026 12:26:05 -0800 Subject: [PATCH 3/7] fix(studio): watch the history file too, and show a scene file the poll cannot read Co-Authored-By: Claude Opus 5.5 --- rs/moq-studio/src/autosave.rs | 191 ++++++++++++++++++++++------------ 1 file changed, 122 insertions(+), 69 deletions(-) diff --git a/rs/moq-studio/src/autosave.rs b/rs/moq-studio/src/autosave.rs index 4845089e..1d92fd58 100644 --- a/rs/moq-studio/src/autosave.rs +++ b/rs/moq-studio/src/autosave.rs @@ -155,10 +155,26 @@ struct Written { /// An empty history when there is no history file, so none is created /// until there is an edit to undo. `None` when it must be rewritten. history: Option, - /// Exactly what the scene file held when the studio last read or wrote - /// it, so anything else there is an outside edit. Compared by content, - /// not time, so the studio's own write never reads as one. - disk: Option>, + disk: Disk, +} + +/// Exactly what the scene file and its history held when the studio last read +/// or wrote them, so anything else there is an outside edit. Compared by +/// content, not time, so the studio's own write never reads as one. `None` +/// for a missing file. +#[derive(Clone, Debug, PartialEq, Eq)] +struct Disk { + scene: Option>, + history: Option>, +} + +impl Disk { + fn read(scene: &Path, history: &Path) -> std::io::Result { + Ok(Self { + scene: read(scene)?, + history: read(history)?, + }) + } } impl Written { @@ -172,8 +188,9 @@ impl Autosave { /// Read the scene file at `path` and its history, or start a new /// collection if there is no scene file yet; the first save creates it. pub fn open(path: PathBuf) -> anyhow::Result<(Self, Loaded)> { - let disk = read(&path)?; - let (collection, written) = match &disk { + let history = history_path(&path); + let disk = Disk::read(&path, &history)?; + let (collection, written) = match &disk.scene { Some(bytes) => { let collection = parse(&path, bytes)?; // Kept as the studio would write it, not as the file spells it, so @@ -183,8 +200,7 @@ impl Autosave { } None => (Collection::new(), None), }; - let history = history_path(&path); - let loaded = load_history(&history, collection)?; + let loaded = load_history(&history, disk.history.as_deref(), collection)?; let written = Written { collection: written, history: Some(loaded.history.to_toml()?), @@ -256,48 +272,50 @@ impl Autosave { let text = doc.lock().unwrap().text()?; if written.collection.as_ref() != Some(&text.collection) { // Checked again right before the rename, since another studio may - // have written the file since the look above. - let unchanged = |path: &Path| Ok(outside(path, written.disk.as_deref())?.is_none()); + // have written the files since the look above. + let unchanged = || Ok(self.outside(&written.disk)?.is_none()); let wrote = write(&self.path, &text.collection, unchanged) .with_context(|| format!("writing {}", self.path.display()))?; if !wrote { // Taken or held as a conflict by the next look. continue; } - written.disk = Some(text.collection.clone().into_bytes()); + written.disk.scene = Some(text.collection.clone().into_bytes()); written.collection = Some(text.collection); } if written.history.as_ref() != Some(&text.history) { - write(&self.history, &text.history, |_| Ok(true)) + let unchanged = || Ok(self.outside(&written.disk)?.is_none()); + let wrote = write(&self.history, &text.history, unchanged) .with_context(|| format!("writing {}", self.history.display()))?; + if !wrote { + continue; + } + written.disk.history = Some(text.history.clone().into_bytes()); written.history = Some(text.history); } return Ok(()); } } - /// Look for an outside edit to the scene file now, without writing. + /// Look for an outside edit to the files now, without writing. pub fn check(&self, doc: &Mutex) -> anyhow::Result<()> { let mut written = self.written.lock().unwrap(); self.look(&mut written, doc) } - /// Take an outside edit to the scene file if `doc` has nothing unsaved and + /// Take an outside edit to the files if `doc` has nothing unsaved and /// accepts it, or else hold it as a [`Conflict`] and fail. fn look(&self, written: &mut Written, doc: &Mutex) -> anyhow::Result<()> { - let Some(bytes) = - outside(&self.path, written.disk.as_deref()).with_context(|| format!("reading {}", self.path.display()))? - else { + let Some(disk) = self.outside(&written.disk)? else { *self.conflict.lock().unwrap() = None; return Ok(()); }; - let loaded = parse(&self.path, &bytes).and_then(|collection| load_history(&self.history, collection)); - let refused = match loaded { + let refused = match self.load(&disk) { Ok(loaded) => { let mut doc = doc.lock().unwrap(); if written.holds(&doc.text()?) { - match self.take(written, &mut *doc, loaded, bytes) { + match self.take(written, &mut *doc, loaded, disk) { Ok(()) => return Ok(()), Err(err) => Some(err), } @@ -319,21 +337,36 @@ impl Autosave { anyhow::bail!("{} changed outside the studio", self.path.display()) } - /// Hand `loaded`, read as `bytes` from the scene file, to `doc`. Once it - /// is accepted, the files count as holding it. - fn take( - &self, - written: &mut Written, - doc: &mut impl Document, - loaded: Loaded, - bytes: Vec, - ) -> anyhow::Result<()> { + /// What the files hold now, if an edit outside the studio changed them + /// since `disk`. The history counts on its own, since another studio + /// writes it after the scene file. A file deleted since holds nothing to + /// keep, so the next write recreates it. + fn outside(&self, disk: &Disk) -> std::io::Result> { + let now = Disk::read(&self.path, &self.history)?; + let changed = |now: &Option>, then: &Option>| now.is_some() && now != then; + let changed = now.scene.is_some() && (changed(&now.scene, &disk.scene) || changed(&now.history, &disk.history)); + Ok(changed.then_some(now)) + } + + /// The collection and history `disk` holds. + fn load(&self, disk: &Disk) -> anyhow::Result { + let scene = disk + .scene + .as_deref() + .with_context(|| format!("{} does not exist", self.path.display()))?; + let collection = parse(&self.path, scene)?; + load_history(&self.history, disk.history.as_deref(), collection) + } + + /// Hand `loaded`, read from `disk`, to `doc`. Once it is accepted, the + /// files count as holding it. + fn take(&self, written: &mut Written, doc: &mut impl Document, loaded: Loaded, disk: Disk) -> anyhow::Result<()> { let text = Text::new(&loaded.collection, &loaded.history)?; doc.replace(loaded)?; *written = Written { collection: Some(text.collection), history: Some(text.history), - disk: Some(bytes), + disk, }; *self.conflict.lock().unwrap() = None; Ok(()) @@ -343,17 +376,16 @@ impl Autosave { /// it has unsaved: the user taking the file's version. pub fn reload(&self, doc: &Mutex) -> anyhow::Result<()> { let mut written = self.written.lock().unwrap(); - let bytes = std::fs::read(&self.path).with_context(|| format!("reading {}", self.path.display()))?; - let collection = parse(&self.path, &bytes)?; - let loaded = load_history(&self.history, collection)?; - self.take(&mut written, &mut *doc.lock().unwrap(), loaded, bytes) + let disk = Disk::read(&self.path, &self.history)?; + let loaded = self.load(&disk)?; + self.take(&mut written, &mut *doc.lock().unwrap(), loaded, disk) } /// Write `doc` over whatever the scene file and its history hold now: the /// user keeping the studio's version. pub fn keep(&self, doc: &Mutex) -> anyhow::Result<()> { let mut written = self.written.lock().unwrap(); - written.disk = read(&self.path)?; + written.disk = Disk::read(&self.path, &self.history)?; // The history beside an outside edit may be another studio's. written.collection = None; written.history = None; @@ -372,8 +404,11 @@ impl Autosave { let _ = self.save(doc); } () = kio::clock::sleep(POLL) => { - // A conflict shows for itself. - let _ = self.check(doc); + if self.check(doc).is_err() { + // Retried as a save, so a file that cannot be read shows in the + // status bar until a save succeeds. + self.edits.record(); + } } } } @@ -381,23 +416,14 @@ impl Autosave { } /// Read the file at `path`, or `None` if there is none. -fn read(path: &Path) -> anyhow::Result>> { +fn read(path: &Path) -> std::io::Result>> { match std::fs::read(path) { Ok(bytes) => Ok(Some(bytes)), Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(None), - Err(err) => Err(err).with_context(|| format!("reading {}", path.display())), - } -} - -/// What the scene file at `path` holds if it is not `disk`, what the studio -/// last read or wrote there. A file deleted since holds nothing to keep, so -/// the next write recreates it. -fn outside(path: &Path, disk: Option<&[u8]>) -> std::io::Result>> { - match std::fs::read(path) { - Ok(bytes) if Some(bytes.as_slice()) != disk => Ok(Some(bytes)), - Ok(_) => Ok(None), - Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(None), - Err(err) => Err(err), + Err(err) => Err(std::io::Error::new( + err.kind(), + format!("reading {}: {err}", path.display()), + )), } } @@ -414,14 +440,16 @@ fn history_path(scene: &Path) -> PathBuf { scene.with_file_name(name) } -/// Read the history at `path` for `collection`, keeping the entries that -/// still apply to it, or start a new one if there is no history file. -fn load_history(path: &Path, collection: Collection) -> anyhow::Result { - let (history, dropped) = match std::fs::read_to_string(path) { - Ok(text) => History::from_toml(&text, &collection) +/// Load the history file at `path`, which holds `bytes`, for `collection`, +/// keeping the entries that still apply to it, or start a new one if there is +/// no history file. +fn load_history(path: &Path, bytes: Option<&[u8]>, collection: Collection) -> anyhow::Result { + let (history, dropped) = match bytes { + Some(bytes) => std::str::from_utf8(bytes) + .map_err(anyhow::Error::from) + .and_then(|text| Ok(History::from_toml(text, &collection)?)) .with_context(|| format!("loading {}; delete it to start a new history", path.display()))?, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => (History::new(), 0), - Err(err) => return Err(err).with_context(|| format!("reading {}", path.display())), + None => (History::new(), 0), }; if dropped > 0 { tracing::warn!(dropped, path = %path.display(), "history entries no longer apply to the scene file"); @@ -435,9 +463,9 @@ fn load_history(path: &Path, collection: Collection) -> anyhow::Result { /// Replace the file at `path` with `text` all at once, so a crash leaves /// either the old file or the new one, never part of either. Returns `false`, -/// leaving the file as it is, if `unchanged` says otherwise of the file it -/// would replace right before the rename. -fn write(path: &Path, text: &str, unchanged: impl FnOnce(&Path) -> std::io::Result) -> std::io::Result { +/// leaving the file as it is, if `unchanged` says the files changed outside +/// the studio, asked right before the rename. +fn write(path: &Path, text: &str, unchanged: impl FnOnce() -> std::io::Result) -> std::io::Result { let path = resolve(path)?; let existing = match std::fs::metadata(&path) { Ok(metadata) => { @@ -478,7 +506,7 @@ fn write(path: &Path, text: &str, unchanged: impl FnOnce(&Path) -> std::io::Resu file.as_file().sync_all()?; // As late as it can be. An edit landing between this and the rename is // still lost, since nothing binds every editor to a lock. - if !unchanged(&path)? { + if !unchanged()? { return Ok(false); } file.persist(&path)?; @@ -755,7 +783,8 @@ mod tests { } /// With nothing unsaved, an outside edit is taken within a poll, its - /// history with it, and left as written until the next edit. + /// history with it even when it lands a poll later, and left as written + /// until the next edit. #[tokio::test(start_paused = true)] async fn takes_an_outside_edit_with_nothing_unsaved() { let dir = tempfile::tempdir().unwrap(); @@ -763,15 +792,16 @@ mod tests { let running = Running::start(path.clone()).await; std::fs::write(&path, HAND_WRITTEN).unwrap(); - // Another studio's history, describing the edit that made the file. - let mut theirs = Doc::new(Collection::from_toml(HAND_WRITTEN).unwrap()) - .into_inner() - .unwrap(); + kio::clock::sleep(POLL + Duration::from_millis(100)).await; + let hand = Collection::from_toml(HAND_WRITTEN).unwrap(); + assert_eq!(running.text().collection, hand.to_toml().unwrap()); + + // Another studio's history, written after its scene file. + let mut theirs = Doc::new(hand).into_inner().unwrap(); theirs.edit("Theirs"); theirs.history.undo(&mut theirs.collection).unwrap(); std::fs::write(history_path(&path), theirs.history.to_toml().unwrap()).unwrap(); - - kio::clock::sleep(POLL + Duration::from_millis(100)).await; + kio::clock::sleep(POLL).await; assert_eq!(running.text(), theirs.text().unwrap()); assert_eq!(running.autosave.conflict(), None); assert_eq!(read(&path), HAND_WRITTEN, "taken, not rewritten"); @@ -853,6 +883,29 @@ mod tests { ); } + /// A scene file that cannot be read shows as a failed save until it can. + #[cfg(unix)] + #[tokio::test(start_paused = true)] + async fn shows_a_file_it_cannot_read() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("show.toml"); + let running = Running::start(path.clone()).await; + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o000)).unwrap(); + if std::fs::File::open(&path).is_ok() { + return; // Running as root, which may read it anyway. + } + + kio::clock::sleep(POLL + QUIET + Duration::from_millis(100)).await; + let error = running.autosave.error().unwrap(); + assert!(error.contains("show.toml"), "{error}"); + + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)).unwrap(); + kio::clock::sleep(QUIET + Duration::from_millis(100)).await; + assert_eq!(running.autosave.error(), None); + } + /// The file is checked again after the save serializes the studio's /// version, so an outside edit landing as the save starts is not lost. #[test] From 3d68ef74257a9d6c2bfd5fe083ae114e6c57e0d4 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 3 Nov 2026 12:59:38 -0800 Subject: [PATCH 4/7] fix(studio): a failed poll does not put off a pending save Co-Authored-By: Claude Opus 5.5 --- rs/moq-studio/src/autosave.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/rs/moq-studio/src/autosave.rs b/rs/moq-studio/src/autosave.rs index 6ae224ba..3aba774b 100644 --- a/rs/moq-studio/src/autosave.rs +++ b/rs/moq-studio/src/autosave.rs @@ -62,6 +62,17 @@ impl Edits { } } + /// Ask for a save [`QUIET`] from now, without delaying one already + /// waiting, which a repeated failure would otherwise put off until + /// [`LIMIT`]. + fn retry(&self) { + let mut unsaved = self.0.lock(); + if unsaved.is_none() { + let now = kio::clock::now(); + *unsaved = Some(Unsaved { first: now, last: now }); + } + } + /// When the unsaved edits are due, if there are any. fn deadline(&self) -> Option { self.0.read().as_ref().map(Unsaved::deadline) @@ -414,7 +425,7 @@ impl Autosave { if self.check(doc).is_err() { // Retried as a save, so a file that cannot be read shows in the // status bar until a save succeeds. - self.edits.record(); + self.edits.retry(); } } } From 0276f02f5d6b99a5d1075d2c156c0f768b9361fc Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 3 Nov 2026 16:56:34 -0800 Subject: [PATCH 5/7] fix(studio): write a deleted scene file again, remember a rename whose sync failed, and drop a drag a reload outdated - A scene file or history deleted outside the studio is written again within a poll, or by the save on quit, rather than left gone until the next edit. - A rename that succeeded before its directory sync failed is recorded as the file's contents, so it never reads as an outside edit, and is written again until the sync succeeds. - A preview drag is dropped under the same lock that applies it when the autosave reloaded the collection since it started. Co-Authored-By: Claude Opus 5.5 --- README.md | 5 +- rs/moq-studio/src/autosave.rs | 187 +++++++++++++++++++++++++++------- rs/moq-studio/src/ui.rs | 11 +- 3 files changed, 164 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index d2ed9003..4bd66101 100644 --- a/README.md +++ b/README.md @@ -312,8 +312,9 @@ its own unsaved, it reloads the change, as **File > Reload** does, which replaces even unsaved edits. If both sides changed, or the change cannot be loaded, the studio saves nothing until you pick **Keep the studio's**, which writes its version over the file, or **Take the file's**; quitting before you -pick saves nothing. One overwrite remains possible: a change saved in the -instant between the studio's last look and its write replacing the file. +pick saves nothing. A file deleted while the studio runs is written again. One +overwrite remains possible: a change saved in the instant between the studio's +last look and its write replacing the file. The last 100 edits are written the same way beside the scene file, as `show.history.toml` for `show.toml`, after the scene file, and opening the diff --git a/rs/moq-studio/src/autosave.rs b/rs/moq-studio/src/autosave.rs index 3aba774b..1b09ab34 100644 --- a/rs/moq-studio/src/autosave.rs +++ b/rs/moq-studio/src/autosave.rs @@ -10,7 +10,8 @@ //! studio. The studio looks for that every [`POLL`], before each write, and //! again right before each write's rename. It takes such a change when it has //! nothing unsaved, and otherwise holds it as a [`Conflict`], writing nothing -//! until the user keeps the studio's version or takes the file's. +//! until the user keeps the studio's version or takes the file's. A file +//! deleted outside the studio is written again. use std::io::Write; use std::path::{Path, PathBuf}; @@ -186,6 +187,16 @@ impl Disk { history: read(history)?, }) } + + /// `now`, what the files hold, if an edit outside the studio changed them + /// since they held `self`. The history counts on its own, since another + /// studio writes it after the scene file. A deleted file is no edit to + /// take, as the studio writes it again. + fn changed(&self, now: Self) -> Option { + let changed = |now: &Option>, then: &Option>| now.is_some() && now != then; + let changed = now.scene.is_some() && (changed(&now.scene, &self.scene) || changed(&now.history, &self.history)); + changed.then_some(now) + } } impl Written { @@ -233,8 +244,7 @@ impl Autosave { &self.path } - /// Whether the scene file exists: it was read when opened, or a save has - /// created it since. + /// Whether the scene file exists, as the studio last read or wrote it. pub fn exists(&self) -> bool { self.written.lock().unwrap().disk.scene.is_some() } @@ -292,39 +302,63 @@ impl Autosave { // Checked again right before the rename, since another studio may // have written the files since the look above. let unchanged = || Ok(self.outside(&written.disk)?.is_none()); - let wrote = write(&self.path, &text.collection, written.disk.scene.is_none(), unchanged) - .with_context(|| format!("writing {}", self.path.display()))?; - if !wrote { + let context = || format!("writing {}", self.path.display()); + let renamed = write(&self.path, &text.collection, written.disk.scene.is_none(), unchanged) + .with_context(context)?; + let Some(dir) = renamed else { // Taken or held as a conflict by the next look. continue; - } + }; + // Renamed into place, so the studio's own file never reads as an + // outside edit, but written again until its rename is synced. written.disk.scene = Some(text.collection.clone().into_bytes()); + written.collection = None; + sync(&dir).with_context(context)?; written.collection = Some(text.collection); } if written.history.as_ref() != Some(&text.history) { let unchanged = || Ok(self.outside(&written.disk)?.is_none()); - let wrote = write(&self.history, &text.history, false, unchanged) - .with_context(|| format!("writing {}", self.history.display()))?; - if !wrote { + let context = || format!("writing {}", self.history.display()); + let renamed = write(&self.history, &text.history, false, unchanged).with_context(context)?; + let Some(dir) = renamed else { continue; - } + }; written.disk.history = Some(text.history.clone().into_bytes()); + written.history = None; + sync(&dir).with_context(context)?; written.history = Some(text.history); } return Ok(()); } } - /// Look for an outside edit to the files now, without writing. + /// Look for an outside edit to the files now, without writing, and ask + /// for a save if a file must be written again, such as one deleted. pub fn check(&self, doc: &Mutex) -> anyhow::Result<()> { let mut written = self.written.lock().unwrap(); - self.look(&mut written, doc) + self.look(&mut written, doc)?; + if written.collection.is_none() || written.history.is_none() { + self.edits.retry(); + } + Ok(()) } /// Take an outside edit to the files if `doc` has nothing unsaved and - /// accepts it, or else hold it as a [`Conflict`] and fail. + /// accepts it, or else hold it as a [`Conflict`] and fail. A file deleted + /// outside the studio holds nothing to keep, so it is written again. fn look(&self, written: &mut Written, doc: &Mutex) -> anyhow::Result<()> { - let Some(disk) = self.outside(&written.disk)? else { + let now = Disk::read(&self.path, &self.history)?; + if now.scene.is_none() && written.disk.scene.is_some() { + tracing::warn!(path = %self.path.display(), "the scene file was deleted outside the studio; writing it again"); + written.disk.scene = None; + written.collection = None; + } + if now.history.is_none() && written.disk.history.is_some() { + tracing::warn!(path = %self.history.display(), "the history was deleted outside the studio; writing it again"); + written.disk.history = None; + written.history = None; + } + let Some(disk) = written.disk.changed(now) else { *self.conflict.lock().unwrap() = None; return Ok(()); }; @@ -356,14 +390,9 @@ impl Autosave { } /// What the files hold now, if an edit outside the studio changed them - /// since `disk`. The history counts on its own, since another studio - /// writes it after the scene file. A file deleted since holds nothing to - /// keep, so the next write recreates it. + /// since `disk`. fn outside(&self, disk: &Disk) -> std::io::Result> { - let now = Disk::read(&self.path, &self.history)?; - let changed = |now: &Option>, then: &Option>| now.is_some() && now != then; - let changed = now.scene.is_some() && (changed(&now.scene, &disk.scene) || changed(&now.history, &disk.history)); - Ok(changed.then_some(now)) + Ok(disk.changed(Disk::read(&self.path, &self.history)?)) } /// The collection and history `disk` holds. @@ -480,19 +509,20 @@ fn load_history(path: &Path, bytes: Option<&[u8]>, collection: Collection) -> an } /// Replace the file at `path` with `text` all at once, so a crash leaves -/// either the old file or the new one, never part of either. Returns `false`, -/// leaving the file as it is, if the files changed outside the studio: a -/// `new` file, one the studio last found missing, exists, or `unchanged`, -/// asked right before the rename, says otherwise. +/// either the old file or the new one, never part of either. Returns the +/// directory to [`sync`] once the caller notes the rename, or `None`, leaving +/// the file as it is, if the files changed outside the studio: a `new` file, +/// one the studio last found missing, exists, or `unchanged`, asked right +/// before the rename, says otherwise. fn write( path: &Path, text: &str, new: bool, unchanged: impl FnOnce() -> std::io::Result, -) -> std::io::Result { +) -> std::io::Result> { let path = resolve(path)?; let existing = match std::fs::metadata(&path) { - Ok(_) if new => return Ok(false), + Ok(_) if new => return Ok(None), Ok(metadata) => { // Refused as a write in place would be, so a read-only scene file // is not replaced. @@ -503,8 +533,8 @@ fn write( Err(err) => return Err(err), }; let dir = match path.parent() { - Some(dir) if !dir.as_os_str().is_empty() => dir, - _ => Path::new("."), + Some(dir) if !dir.as_os_str().is_empty() => dir.to_path_buf(), + _ => PathBuf::from("."), }; // Named uniquely and created exclusively, so another studio saving the @@ -520,7 +550,7 @@ fn write( use std::os::unix::fs::PermissionsExt; builder.permissions(std::fs::Permissions::from_mode(0o666)); } - let mut file = builder.tempfile_in(dir)?; + let mut file = builder.tempfile_in(&dir)?; // It replaces the old file, so it takes its permissions. if let Some(permissions) = existing { file.as_file().set_permissions(permissions)?; @@ -533,22 +563,29 @@ fn write( // still lost, since nothing binds every editor to a lock, except that a // new file is never replaced. if !unchanged()? { - return Ok(false); + return Ok(None); } match new { true => match file.persist_noclobber(&path) { Ok(_) => {} - Err(err) if err.error.kind() == std::io::ErrorKind::AlreadyExists => return Ok(false), + Err(err) if err.error.kind() == std::io::ErrorKind::AlreadyExists => return Ok(None), Err(err) => return Err(err.error), }, false => { file.persist(&path)?; } } - // And the rename itself, so a power loss cannot undo the save. + Ok(Some(dir)) +} + +/// Sync the directory `dir` a file was renamed into, so a power loss cannot +/// undo the rename. +fn sync(dir: &Path) -> std::io::Result<()> { #[cfg(unix)] std::fs::File::open(dir)?.sync_all()?; - Ok(true) + #[cfg(not(unix))] + let _ = dir; + Ok(()) } /// The file a chain of symlinks at `path` ends in, which need not exist yet, @@ -959,6 +996,86 @@ mod tests { assert_eq!(running.autosave.error(), None); } + /// A file deleted outside the studio holds nothing to keep, so it is + /// written again within a poll, or by the save on quit, without waiting + /// for an edit. + #[tokio::test(start_paused = true)] + async fn writes_a_deleted_file_again() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("show.toml"); + let history = history_path(&path); + let running = Running::start(path.clone()).await; + running.edit("A"); + kio::clock::sleep(QUIET + Duration::from_millis(100)).await; + let saved = running.text(); + assert_eq!(read(&history), saved.history); + + std::fs::remove_file(&path).unwrap(); + std::fs::remove_file(&history).unwrap(); + kio::clock::sleep(POLL + QUIET + Duration::from_millis(100)).await; + assert_eq!(read(&path), saved.collection); + assert_eq!(read(&history), saved.history); + + // Saved on quit before a poll sees it. + std::fs::remove_file(&path).unwrap(); + running.autosave.save(&*running.doc).unwrap(); + assert_eq!(read(&path), saved.collection); + assert_eq!(running.autosave.conflict(), None); + } + + /// A rename whose directory cannot be synced still replaced the file, so + /// once the directory can be synced, the studio's own file is no outside + /// edit, and it is written again so the save is durable. + #[cfg(unix)] + #[test] + fn remembers_a_rename_it_could_not_sync() { + use std::os::unix::fs::{MetadataExt, PermissionsExt}; + + let dir = tempfile::tempdir().unwrap(); + let show = dir.path().join("show"); + std::fs::create_dir(&show).unwrap(); + let path = show.join("show.toml"); + let (autosave, Loaded { collection, .. }) = Autosave::open(path.clone()).unwrap(); + let doc = Doc::new(collection); + autosave.save(&doc).unwrap(); + + // Files can be created and renamed in it, but it cannot be opened to sync. + let mode = |mode| std::fs::set_permissions(&show, std::fs::Permissions::from_mode(mode)).unwrap(); + mode(0o333); + if std::fs::File::open(&show).is_ok() { + return mode(0o755); // Running as root, which may open it anyway. + } + doc.lock().unwrap().edit("Mine"); + let err = autosave.save(&doc); + mode(0o755); + let err = format!("{:#}", err.unwrap_err()); + assert!(err.contains("show.toml"), "{err}"); + let mine = doc.lock().unwrap().text().unwrap(); + assert_eq!(read(&path), mine.collection, "renamed before the sync failed"); + + autosave.check(&doc).unwrap(); + assert_eq!(autosave.conflict(), None); + let inode = || std::fs::metadata(&path).unwrap().ino(); + let renamed = inode(); + autosave.save(&doc).unwrap(); + assert_eq!(autosave.error(), None); + assert_ne!(inode(), renamed, "written again, so its rename is synced"); + assert_eq!(read(&path), mine.collection); + assert_eq!(read(&history_path(&path)), mine.history); + } + + /// A retry asks for a save without putting off one already waiting, so a + /// failure repeating every poll cannot hold it back until [`LIMIT`]. + #[tokio::test(start_paused = true)] + async fn a_retry_does_not_put_off_a_waiting_save() { + let edits = Edits::default(); + edits.record(); + let deadline = edits.deadline(); + kio::clock::sleep(QUIET / 2).await; + edits.retry(); + assert_eq!(edits.deadline(), deadline); + } + /// The file is checked again after the save serializes the studio's /// version, so an outside edit landing as the save starts is not lost. #[test] diff --git a/rs/moq-studio/src/ui.rs b/rs/moq-studio/src/ui.rs index 0733ff22..a67210cd 100644 --- a/rs/moq-studio/src/ui.rs +++ b/rs/moq-studio/src/ui.rs @@ -592,8 +592,12 @@ impl Panels { /// by dragging them in the preview, snapping as they go. fn arrange(&mut self, ui: &Ui, state: &mut State, response: &egui::Response, canvas: Canvas) -> anyhow::Result<()> { let gesture = self.gestures.of(response); - // The release still moves the item to where the pointer let go. - if !response.dragged() && !response.drag_stopped() { + // The release still moves the item to where the pointer let go. A + // reload since the drag started, which the autosave can make between + // frames, may have put another item at its index. + if (!response.dragged() && !response.drag_stopped()) + || self.drag.as_ref().is_some_and(|drag| drag.reloads != state.reloads) + { self.drag = None; } // A panel squeezed to nothing has no picture to map the pointer onto. @@ -636,6 +640,7 @@ impl Panels { origin: view.point(origin), gesture, hints: None, + reloads: state.reloads, }); } @@ -2044,6 +2049,8 @@ struct Drag { gesture: Option, /// The snapped rect, its guides, and its spacing, unless snapping is off. hints: Option, + /// The collection's reloads when the drag started. + reloads: u64, } /// How far, in points on screen, the pointer reaches to an edge or a line. From a9bc485f96d35773ed4abee68a0cb397903ee4a6 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 3 Nov 2026 17:08:53 -0800 Subject: [PATCH 6/7] fix(studio): let go of an outdated selection under each lock, and keep rewrites apart from the saved baseline - Every lock that resolves the selected item or the preview drag first lets go of them if the autosave reloaded since, so a paste, filter edit, or drag never lands on another item at the same index. - A file deleted outside the studio, or renamed without a sync, is marked to be written again without forgetting what it held, so the studio still has nothing unsaved and takes an outside edit landing beside the deletion. Co-Authored-By: Claude Opus 5.5 --- rs/moq-studio/src/autosave.rs | 40 +++++++++++++++++++++++++-------- rs/moq-studio/src/ui.rs | 42 +++++++++++++++++------------------ 2 files changed, 52 insertions(+), 30 deletions(-) diff --git a/rs/moq-studio/src/autosave.rs b/rs/moq-studio/src/autosave.rs index 1b09ab34..13d08b63 100644 --- a/rs/moq-studio/src/autosave.rs +++ b/rs/moq-studio/src/autosave.rs @@ -168,6 +168,16 @@ struct Written { /// until there is an edit to undo. `None` when it must be rewritten. history: Option, disk: Disk, + rewrite: Rewrite, +} + +/// Files to write again though they hold the collection and history: deleted +/// outside the studio, or renamed into place without a sync. Kept apart from +/// what they hold, so the studio still has nothing unsaved. +#[derive(Default)] +struct Rewrite { + scene: bool, + history: bool, } /// Exactly what the scene file and its history held when the studio last read @@ -228,6 +238,7 @@ impl Autosave { collection: written, history: Some(loaded.history.to_toml()?), disk, + rewrite: Rewrite::default(), }; let autosave = Self { path, @@ -298,7 +309,7 @@ impl Autosave { loop { self.look(written, doc)?; let text = doc.lock().unwrap().text()?; - if written.collection.as_ref() != Some(&text.collection) { + if written.rewrite.scene || written.collection.as_ref() != Some(&text.collection) { // Checked again right before the rename, since another studio may // have written the files since the look above. let unchanged = || Ok(self.outside(&written.disk)?.is_none()); @@ -312,11 +323,12 @@ impl Autosave { // Renamed into place, so the studio's own file never reads as an // outside edit, but written again until its rename is synced. written.disk.scene = Some(text.collection.clone().into_bytes()); - written.collection = None; - sync(&dir).with_context(context)?; written.collection = Some(text.collection); + written.rewrite.scene = true; + sync(&dir).with_context(context)?; + written.rewrite.scene = false; } - if written.history.as_ref() != Some(&text.history) { + if written.rewrite.history || written.history.as_ref() != Some(&text.history) { let unchanged = || Ok(self.outside(&written.disk)?.is_none()); let context = || format!("writing {}", self.history.display()); let renamed = write(&self.history, &text.history, false, unchanged).with_context(context)?; @@ -324,9 +336,10 @@ impl Autosave { continue; }; written.disk.history = Some(text.history.clone().into_bytes()); - written.history = None; - sync(&dir).with_context(context)?; written.history = Some(text.history); + written.rewrite.history = true; + sync(&dir).with_context(context)?; + written.rewrite.history = false; } return Ok(()); } @@ -337,7 +350,7 @@ impl Autosave { pub fn check(&self, doc: &Mutex) -> anyhow::Result<()> { let mut written = self.written.lock().unwrap(); self.look(&mut written, doc)?; - if written.collection.is_none() || written.history.is_none() { + if written.rewrite.scene || written.rewrite.history { self.edits.retry(); } Ok(()) @@ -351,12 +364,12 @@ impl Autosave { if now.scene.is_none() && written.disk.scene.is_some() { tracing::warn!(path = %self.path.display(), "the scene file was deleted outside the studio; writing it again"); written.disk.scene = None; - written.collection = None; + written.rewrite.scene = true; } if now.history.is_none() && written.disk.history.is_some() { tracing::warn!(path = %self.history.display(), "the history was deleted outside the studio; writing it again"); written.disk.history = None; - written.history = None; + written.rewrite.history = true; } let Some(disk) = written.disk.changed(now) else { *self.conflict.lock().unwrap() = None; @@ -414,6 +427,7 @@ impl Autosave { collection: Some(text.collection), history: Some(text.history), disk, + rewrite: Rewrite::default(), }; *self.conflict.lock().unwrap() = None; Ok(()) @@ -1020,7 +1034,15 @@ mod tests { std::fs::remove_file(&path).unwrap(); running.autosave.save(&*running.doc).unwrap(); assert_eq!(read(&path), saved.collection); + + // The studio still has nothing unsaved, so an outside edit landing + // beside a deletion is taken, not held. + std::fs::remove_file(&history).unwrap(); + std::fs::write(&path, HAND_WRITTEN).unwrap(); + kio::clock::sleep(POLL + Duration::from_millis(100)).await; assert_eq!(running.autosave.conflict(), None); + let hand = Collection::from_toml(HAND_WRITTEN).unwrap(); + assert_eq!(running.text().collection, hand.to_toml().unwrap()); } /// A rename whose directory cannot be synced still replaced the file, so diff --git a/rs/moq-studio/src/ui.rs b/rs/moq-studio/src/ui.rs index a67210cd..0e47ef6a 100644 --- a/rs/moq-studio/src/ui.rs +++ b/rs/moq-studio/src/ui.rs @@ -114,10 +114,13 @@ impl Studio { impl eframe::App for Studio { fn ui(&mut self, ui: &mut Ui, _frame: &mut eframe::Frame) { // Redraw at the canvas rate, so preview and program are live. - let framerate = self.panels.engine.state.lock().unwrap().collection().canvas.framerate; + let engine = self.panels.engine.clone(); + let state = engine.state.lock().unwrap(); + let framerate = state.collection().canvas.framerate; ui.ctx().request_repaint_after(Duration::from_secs(1) / framerate); + self.panels.follow_reloads(&state); + drop(state); - self.panels.follow_reloads(); self.panels.shortcuts(ui.ctx()); egui::Panel::top("menu").show(ui, |ui| self.panels.menu(ui)); if let Some(conflict) = self.panels.engine.autosave.conflict() { @@ -376,19 +379,17 @@ impl Panels { } /// Report a reload, from File > Reload or an outside edit the autosave - /// took, and let go of a selected item it may have removed. - fn follow_reloads(&mut self) { - let (reloads, dropped) = { - let state = self.engine.state.lock().unwrap(); - (state.reloads, state.dropped) - }; - if reloads == self.reloads { + /// took, and let go of a selected item and a drag whose index it may have + /// given to another item. Called under every lock that uses either, since + /// the autosave can reload between two. + fn follow_reloads(&mut self, state: &State) { + if state.reloads == self.reloads { return; } - self.reloads = reloads; + self.reloads = state.reloads; self.deselect_item(); let path = self.engine.autosave.path().display(); - self.report(Ok(match dropped { + self.report(Ok(match state.dropped { 0 => format!("reloaded {path}"), dropped => format!("reloaded {path}; {}", dropped_message(dropped)), })); @@ -501,7 +502,9 @@ impl Panels { /// Copy the selected item, with what it shows, to the clipboard. fn copy(&mut self, ctx: &egui::Context) { - let state = self.engine.state.lock().unwrap(); + let engine = self.engine.clone(); + let state = engine.state.lock().unwrap(); + self.follow_reloads(&state); let collection = state.collection(); let item = match &self.selection { Some(Selection::Item(layer, index)) => collection.items(layer).and_then(|items| items.get(*index)), @@ -527,6 +530,7 @@ impl Panels { fn paste(&mut self, text: &str) { let engine = self.engine.clone(); let mut state = engine.state.lock().unwrap(); + self.follow_reloads(&state); let result = (|| { let clip = Clip::from_toml(text).context("the clipboard holds no items or filters")?; match clip { @@ -562,6 +566,7 @@ impl Panels { fn preview(&mut self, ui: &mut Ui) { let engine = self.engine.clone(); let mut state = engine.state.lock().unwrap(); + self.follow_reloads(&state); let canvas = state.collection().canvas; ui.horizontal(|ui| { ui.label(format!( @@ -592,12 +597,8 @@ impl Panels { /// by dragging them in the preview, snapping as they go. fn arrange(&mut self, ui: &Ui, state: &mut State, response: &egui::Response, canvas: Canvas) -> anyhow::Result<()> { let gesture = self.gestures.of(response); - // The release still moves the item to where the pointer let go. A - // reload since the drag started, which the autosave can make between - // frames, may have put another item at its index. - if (!response.dragged() && !response.drag_stopped()) - || self.drag.as_ref().is_some_and(|drag| drag.reloads != state.reloads) - { + // The release still moves the item to where the pointer let go. + if !response.dragged() && !response.drag_stopped() { self.drag = None; } // A panel squeezed to nothing has no picture to map the pointer onto. @@ -640,7 +641,6 @@ impl Panels { origin: view.point(origin), gesture, hints: None, - reloads: state.reloads, }); } @@ -834,6 +834,7 @@ impl Panels { fn items(&mut self, ui: &mut Ui) { let engine = self.engine.clone(); let mut state = engine.state.lock().unwrap(); + self.follow_reloads(&state); let scene = state.compositor.preview_scene().unwrap_or("no scene").to_string(); ui.horizontal(|ui| { ui.selectable_value(&mut self.top, false, format!("Scene: {scene}")); @@ -1274,6 +1275,7 @@ impl Panels { fn filters(&mut self, ui: &mut Ui) { let engine = self.engine.clone(); let mut state = engine.state.lock().unwrap(); + self.follow_reloads(&state); let collection = state.collection(); let Some(selection) = self.selection.clone() else { ui.label("Select a source or an item to filter it."); @@ -2049,8 +2051,6 @@ struct Drag { gesture: Option, /// The snapped rect, its guides, and its spacing, unless snapping is off. hints: Option, - /// The collection's reloads when the drag started. - reloads: u64, } /// How far, in points on screen, the pointer reaches to an edge or a line. From 621981891a83728316054f4a619b3bafec5ade14 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 3 Nov 2026 17:21:09 -0800 Subject: [PATCH 7/7] fix(studio): keep a pending sync when taking an outside edit to the other file A rename whose directory sync failed stays marked to be written again when the studio takes an outside edit that left that file as it was, so the save is still made durable. Co-Authored-By: Claude Opus 5.5 --- rs/moq-studio/src/autosave.rs | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/rs/moq-studio/src/autosave.rs b/rs/moq-studio/src/autosave.rs index 13d08b63..6b39846c 100644 --- a/rs/moq-studio/src/autosave.rs +++ b/rs/moq-studio/src/autosave.rs @@ -423,11 +423,19 @@ impl Autosave { fn take(&self, written: &mut Written, doc: &mut impl Document, loaded: Loaded, disk: Disk) -> anyhow::Result<()> { let text = Text::new(&loaded.collection, &loaded.history)?; doc.replace(loaded)?; + // A rename of the studio's still waits on its sync, unless the edit + // replaced that file too. + let unsynced = + |rewrite: bool, then: &Option>, now: &Option>| rewrite && now.is_some() && now == then; + let rewrite = Rewrite { + scene: unsynced(written.rewrite.scene, &written.disk.scene, &disk.scene), + history: unsynced(written.rewrite.history, &written.disk.history, &disk.history), + }; *written = Written { collection: Some(text.collection), history: Some(text.history), disk, - rewrite: Rewrite::default(), + rewrite, }; *self.conflict.lock().unwrap() = None; Ok(()) @@ -1084,6 +1092,22 @@ mod tests { assert_ne!(inode(), renamed, "written again, so its rename is synced"); assert_eq!(read(&path), mine.collection); assert_eq!(read(&history_path(&path)), mine.history); + + // Still written again after taking an outside edit to the history alone. + std::fs::remove_file(&path).unwrap(); + mode(0o333); + let err = autosave.save(&doc); + mode(0o755); + assert!(err.is_err()); + let mut theirs = Doc::new(doc.lock().unwrap().collection.clone()).into_inner().unwrap(); + theirs.edit("Theirs"); + theirs.history.undo(&mut theirs.collection).unwrap(); + std::fs::write(history_path(&path), theirs.history.to_toml().unwrap()).unwrap(); + autosave.check(&doc).unwrap(); + assert_eq!(doc.lock().unwrap().text().unwrap(), theirs.text().unwrap()); + let renamed = inode(); + autosave.save(&doc).unwrap(); + assert_ne!(inode(), renamed, "written again, so its rename is synced"); } /// A retry asks for a save without putting off one already waiting, so a