From 538a3299353b8b676ec94b46c9086e2454c341ee Mon Sep 17 00:00:00 2001 From: Ahmed Hosny Date: Thu, 8 Oct 2026 08:32:08 +0300 Subject: [PATCH 01/35] linux: install a dev desktop entry so the taskbar shows the app icon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dev run (cargo run) has no installed .desktop entry or hicolor icon for the app id the window sets (ai.storyteller.filmcraft), so KDE (Wayland and X11) falls back to the generic Wayland icon in the taskpanel and the window topbar. Packaging installs both; development did not. At startup on Linux, when no ai.storyteller.filmcraft.desktop exists in any XDG data dir, install one into $XDG_DATA_HOME/~/.local/share (applications/) plus the 256/512 hicolor PNGs, with Exec pointing at the running binary. A packaged entry is never overridden; failures print a warning and the app carries on (AGENTS.md §0). 🤖 Generated with [Kimi Code](https://github.com/storyteller/kimi-code) Co-Authored-By: Kimi --- apps/filmcraft/src/dev_icon.rs | 213 +++++++++++++++++++++++++++++++++ apps/filmcraft/src/main.rs | 4 + 2 files changed, 217 insertions(+) create mode 100644 apps/filmcraft/src/dev_icon.rs diff --git a/apps/filmcraft/src/dev_icon.rs b/apps/filmcraft/src/dev_icon.rs new file mode 100644 index 00000000..a00986d2 --- /dev/null +++ b/apps/filmcraft/src/dev_icon.rs @@ -0,0 +1,213 @@ +//! Dev-time Linux desktop entry: the shell icon fallback. +//! +//! The Linux shell (Wayland *and* X11) resolves the window to a `.desktop` file by app id (set in +//! `main.rs` via `eframe`'s `with_app_id`) and takes the taskbar / window-list icon from its +//! `Icon=` entry in the hicolor theme. Packaging installs both +//! (`packaging/linux/ai.storyteller.filmcraft.desktop` and the hicolor PNGs) — but a dev machine +//! running `cargo run` has neither, so KDE/GNOME fall back to the generic Wayland icon. The fix is +//! to ensure a user-level entry at startup: when no entry for the app id exists in any XDG data +//! dir, install one (plus the hicolor icon PNGs, embedded so it works whatever the working +//! directory is) into `$XDG_DATA_HOME`/`~/.local/share`. An existing packaged entry is never +//! overridden; the user-owned dev copy is rewritten when it points at another binary (a dev build +//! moves between checkouts). The shell picks the icon up at the *next* app start; +//! `update-desktop-database`/`kbuildsycoca` is not needed for icon lookup. +//! +//! Every failure is reported and skipped — a missing dev icon must never stop the app +//! ([`AGENTS.md`](/AGENTS.md) §0 "never crash"). + +/// Install the user-level desktop entry and icons when the app id has no entry anywhere (Linux). +pub fn ensure_dev_desktop_entry() { + if let Err(e) = ensure() { + eprintln!("filmcraft: desktop entry (taskbar icon) not installed: {e}"); + } +} + +/// Install missing files; `Ok` whether anything was written or an entry already existed. +fn ensure() -> Result<(), DevIconError> { + let file_name = format!("{}.desktop", crate::APP_ID); + let user_dir = user_data_dir()?; + let exe = std::env::current_exe().map_err(DevIconError::Io)?; + let exe = exe.to_string_lossy(); + for dir in search_dirs(&user_dir) { + let candidate = dir.join("applications").join(&file_name); + if !candidate.is_file() { + continue; + } + // A packaged entry (any system dir) already gives the shell its icon; only our own + // user-level copy is kept fresh. + if dir != user_dir { + return Ok(()); + } + let body = std::fs::read_to_string(&candidate).map_err(DevIconError::Io)?; + if desktop_file_exec_is_ours(&body, &exe) { + return Ok(()); + } + break; // a dev entry from another checkout: fall through and rewrite it + } + for (size, png) in ICONS { + let dest = user_dir.join("icons").join("hicolor").join(format!("{size}x{size}")).join("apps").join(format!("{}.png", crate::APP_ID)); + if !dest.is_file() + && let Some(parent) = dest.parent() + { + std::fs::create_dir_all(parent).map_err(DevIconError::Io)?; + std::fs::write(&dest, png).map_err(DevIconError::Io)?; + } + } + let applications = user_dir.join("applications"); + std::fs::create_dir_all(&applications).map_err(DevIconError::Io)?; + let body = desktop_file_content(crate::APP_ID, "FilmCraft", &exe); + std::fs::write(applications.join(&file_name), body).map_err(DevIconError::Io)?; + Ok(()) +} + +/// The hicolor icon sizes installed for the taskbar (a subset large enough for panel and topbar; +/// the shell picks the nearest size, so 256 and 512 cover both). +const ICONS: &[(u16, &[u8])] = &[ + (256, include_bytes!("../../../assets/app-icon/hicolor/256x256/apps/ai.storyteller.filmcraft.png")), + (512, include_bytes!("../../../assets/app-icon/hicolor/512x512/apps/ai.storyteller.filmcraft.png")), +]; + +/// The user data dir we install into: `$XDG_DATA_HOME`, else `$HOME/.local/share` (XDG base +/// directory spec). Untrusted environment: an empty value means the default, and a relative or +/// missing one is an error — never a guess that would write into the working directory. +fn user_data_dir() -> Result { + if let Some(v) = std::env::var_os("XDG_DATA_HOME") + && !v.is_empty() + && std::path::Path::new(&v).is_absolute() + { + return Ok(std::path::PathBuf::from(v)); + } + match std::env::var_os("HOME").filter(|h| !h.is_empty()) { + Some(home) => Ok(std::path::PathBuf::from(home).join(".local").join("share")), + None => Err(DevIconError::NoHome), + } +} + +/// The XDG data dirs to search for an existing entry, user dir first, then `$XDG_DATA_DIRS` (or +/// the spec default `/usr/local/share:/usr/share`). Only absolute paths are taken; the spec +/// requires them and a relative one would read the working directory. +fn search_dirs(user: &std::path::Path) -> Vec { + let mut dirs = vec![user.to_path_buf()]; + let system = std::env::var_os("XDG_DATA_DIRS") + .filter(|v| !v.is_empty()) + .map(|v| v.to_string_lossy().into_owned()) + .unwrap_or_else(|| "/usr/local/share:/usr/share".into()); + for dir in system.split(':') { + if std::path::Path::new(dir).is_absolute() { + dirs.push(std::path::PathBuf::from(dir)); + } + } + dirs +} + +/// Whether the `.desktop` body's `Exec` line points at our binary (a stale dev entry from another +/// checkout does not, and is rewritten; a packaged `Exec=filmcraft` never matches a dev path). +fn desktop_file_exec_is_ours(body: &str, exe: &str) -> bool { + body.lines().any(|l| l.strip_prefix("Exec=").is_some_and(|rest| rest.contains(exe))) +} + +/// The `.desktop` body: same fields as `packaging/linux/ai.storyteller.filmcraft.desktop`, with +/// `Exec` pointing at this exact binary (a dev build isn't on `$PATH`, so `TryExec` is left out) +/// and the icon by theme name. `exe` is shell-quoted to survive spaces in the path. +fn desktop_file_content(app_id: &str, name: &str, exe: &str) -> String { + let mut out = String::new(); + out.push_str("[Desktop Entry]\n"); + out.push_str("Type=Application\n"); + out.push_str(&format!("Name={name}\n")); + out.push_str("GenericName=Video Editor\n"); + out.push_str("Comment=Edit video, color and sound\n"); + out.push_str(&format!("Exec={} %F\n", quote(exe))); + out.push_str(&format!("Icon={app_id}\n")); + out.push_str("Terminal=false\n"); + out.push_str("StartupNotify=true\n"); + out.push_str(&format!("StartupWMClass={app_id}\n")); + out.push_str("Categories=AudioVideo;Video;AudioVideoEditing;\n"); + out.push_str("Keywords=video;editor;film;timeline;color;grading;nle;\n"); + out +} + +/// Quote `s` for a shell `Exec=` line (Desktop Entry spec §"Exec string": an argument with +/// reserved characters is wrapped in double quotes, escaping `\`, `"`, `` ` `` and `$`). +fn quote(s: &str) -> String { + let needs = s.is_empty() || s.bytes().any(|b| b" \"'\\`$<>&~|;()*?[]#".contains(&b)); + if !needs { + return s.into(); + } + let mut out = String::with_capacity(s.len() + 2); + out.push('"'); + for c in s.chars() { + if matches!(c, '\\' | '"' | '`' | '$') { + out.push('\\'); + } + out.push(c); + } + out.push('"'); + out +} + +/// Everything that can keep the dev icon out of the taskbar; printed, never fatal. +#[derive(Debug)] +enum DevIconError { + Io(std::io::Error), + NoHome, +} + +impl std::fmt::Display for DevIconError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + DevIconError::Io(e) => write!(f, "{e}"), + DevIconError::NoHome => write!(f, "no home directory for the user data dir"), + } + } +} + +impl From for DevIconError { + fn from(e: std::io::Error) -> Self { + DevIconError::Io(e) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The entry mirrors the packaged one (minus `TryExec`, plus the dev `Exec` path). + #[test] + fn desktop_file_matches_the_packaged_fields() { + let body = desktop_file_content("ai.storyteller.filmcraft", "FilmCraft", "/target/debug/filmcraft"); + assert_eq!( + body, + "[Desktop Entry]\n\ + Type=Application\n\ + Name=FilmCraft\n\ + GenericName=Video Editor\n\ + Comment=Edit video, color and sound\n\ + Exec=/target/debug/filmcraft %F\n\ + Icon=ai.storyteller.filmcraft\n\ + Terminal=false\n\ + StartupNotify=true\n\ + StartupWMClass=ai.storyteller.filmcraft\n\ + Categories=AudioVideo;Video;AudioVideoEditing;\n\ + Keywords=video;editor;film;timeline;color;grading;nle;\n" + ); + } + + /// A path with shell-reserved characters stays one `Exec` argument. + #[test] + fn exec_quotes_shell_reserved_paths() { + assert_eq!(quote("/opt/film craft/filmcraft"), "\"/opt/film craft/filmcraft\""); + assert_eq!(quote("/bin/filmcraft"), "/bin/filmcraft"); + assert_eq!(quote("/a$b\"c\\d"), "\"/a\\$b\\\"c\\\\d\""); + } + + /// Only an `Exec` line that contains our binary marks the entry as ours — a packaged + /// `Exec=filmcraft` or another checkout's path does not. + #[test] + fn desktop_file_exec_is_ours_matches_only_our_binary() { + let body = desktop_file_content("ai.storyteller.filmcraft", "FilmCraft", "/a/filmcraft"); + assert!(desktop_file_exec_is_ours(&body, "/a/filmcraft")); + assert!(!desktop_file_exec_is_ours(&body, "/b/filmcraft")); + assert!(!desktop_file_exec_is_ours("Exec=filmcraft %F\n", "/a/filmcraft")); + assert!(!desktop_file_exec_is_ours("Name=filmcraft\n", "/a/filmcraft")); + } +} diff --git a/apps/filmcraft/src/main.rs b/apps/filmcraft/src/main.rs index 52454830..ad3e9f5e 100644 --- a/apps/filmcraft/src/main.rs +++ b/apps/filmcraft/src/main.rs @@ -24,6 +24,8 @@ mod args; mod audio; mod audio_in; mod control_server; +#[cfg(target_os = "linux")] +mod dev_icon; #[cfg(target_os = "macos")] mod native_menu; mod window_raise; @@ -84,6 +86,8 @@ fn main() -> eframe::Result { } }; app_nap::disable(); + #[cfg(target_os = "linux")] + dev_icon::ensure_dev_desktop_entry(); // Panics anywhere go to /Logs/crash-.log with a backtrace; the UI pass and // frame workers catch them and keep running (see filmcraft_ui_egui::crash). filmcraft_ui_egui::crash::install(data_dir.clone().or_else(default_data_dir).map(|d| d.join("Logs"))); From 58ce158ed0ad9f40b279f5552584644b3c259a10 Mon Sep 17 00:00:00 2001 From: Ahmed Hosny Date: Thu, 8 Oct 2026 11:38:57 +0300 Subject: [PATCH 02/35] contributors: credit ahhosnyas (people.ahmedhosnypro) as Ahmed Hosny MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Own entry in contributors/people.toml for the About window, per AGENTS.md. 🤖 Generated with [Kimi Code](https://github.com/storyteller/kimi-code) Co-Authored-By: Kimi --- contributors/people.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/contributors/people.toml b/contributors/people.toml index 89c6db20..77d991e7 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -21,3 +21,8 @@ real_name = "Brandon Thomas" display_name = "Brandon Thomas" sync_github_name = false + +[people.ahmedhosnypro] +real_name = "Ahmed Hosny" +display_name = "Ahmed Hosny" +sync_github_name = false From 9bf6595ccd53eeb2591320f8db19bcd983f9da11 Mon Sep 17 00:00:00 2001 From: Ahmed Hosny Date: Thu, 8 Oct 2026 13:59:59 +0300 Subject: [PATCH 03/35] Playback: fall back to a working output device when the ALSA default cannot be opened MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a machine whose ALSA default PCM is misconfigured (missing 99-pipewire-default.conf symlink, #23/#106) the host default could not be opened: playback ran without sound and without any user-facing message. The device picker now falls back to the first output device that can open, logs why, and a new defaulted AudioOut::note() surfaces the note in the status bar (first frame and when playback starts); a machine with no device at all reports it too. 🤖 Generated with [Kimi Code](https://github.com/storyteller/kimi-code) Co-Authored-By: Kimi --- apps/filmcraft/src/audio.rs | 53 +++++++++++++++++++---- crates/ui-egui/src/lib.rs | 15 ++++++- crates/ui-egui/tests/playback_clock_ui.rs | 26 ++++++++++- 3 files changed, 83 insertions(+), 11 deletions(-) diff --git a/apps/filmcraft/src/audio.rs b/apps/filmcraft/src/audio.rs index c8fe8b7f..b8d164bb 100644 --- a/apps/filmcraft/src/audio.rs +++ b/apps/filmcraft/src/audio.rs @@ -17,6 +17,9 @@ pub struct CpalOut { hw: AudioHardwarePrefs, /// Sequence sample rate for "Attempt to force hardware to document sample rate". document_rate: Option, + /// Status-bar note: the host's default device could not be opened and a fallback device is + /// in use, or no output device was found (playing without sound). + note: Option, } /// The host named in the settings (empty or unknown = the default host). @@ -30,21 +33,47 @@ fn host(name: &str) -> cpal::Host { cpal::default_host() } -/// The output device named in the settings (empty or missing = the host's default). -fn output_device(h: &cpal::Host, name: &str) -> Option { +/// The output device the settings ask for, with a status-bar note when a fallback is in use. +struct DeviceChoice { + device: cpal::Device, + /// The host's default device could not be opened: this device is the fallback. + fallback: Option, +} + +/// The output device named in the settings (empty or missing = the host's default). When the +/// default cannot be opened — a broken ALSA `default` (missing `99-pipewire-default.conf`, +/// #23/#106) — the first device that can, so the app is not silently mute. +fn output_device(h: &cpal::Host, name: &str) -> Option { if !name.is_empty() && let Ok(mut devs) = h.output_devices() && let Some(d) = devs.find(|d| d.name().is_ok_and(|n| n == name)) { - return Some(d); + return Some(DeviceChoice { device: d, fallback: None }); } - h.default_output_device() + if let Some(d) = h.default_output_device() + && d.default_output_config().is_ok() + { + return Some(DeviceChoice { device: d, fallback: None }); + } + let mut devs = h.output_devices().ok()?; + let d = devs.find(|d| d.default_output_config().is_ok())?; + let found = d.name().unwrap_or_default(); + log::warn!("the default audio output device cannot be opened; falling back to '{found}'"); + Some(DeviceChoice { device: d, fallback: Some(format!("Audio hardware: the default output device cannot be opened; using '{found}'")) }) } impl CpalOut { pub fn new() -> Self { let host = cpal::default_host(); - let cfg = host.default_output_device().and_then(|dev| dev.default_output_config().ok()); + let choice = output_device(&host, ""); + let cfg = choice.as_ref().and_then(|c| c.device.default_output_config().ok()); + let note = match choice { + Some(c) => c.fallback, + None => { + log::warn!("no audio output device found"); + Some("No audio output device found: playing without sound (check Settings ▸ Audio Hardware)".into()) + } + }; Self { stream: None, played: Arc::new(AtomicU64::new(0)), @@ -53,6 +82,7 @@ impl CpalOut { channels: cfg.as_ref().map_or(2, |c| c.channels()), hw: AudioHardwarePrefs::default(), document_rate: None, + note, } } @@ -124,7 +154,8 @@ impl AudioOut for CpalOut { fn start(&mut self, mut fill: Box) -> Result { self.stop(); let host = host(&self.hw.device_class); - let dev = output_device(&host, &self.hw.default_output).ok_or("no output device")?; + let DeviceChoice { device: dev, fallback } = output_device(&host, &self.hw.default_output).ok_or("no output device")?; + self.note = fallback; let cfg = self.config(&dev)?; let channels = cfg.channels() as usize; if channels == 0 || cfg.sample_rate().0 == 0 { @@ -185,7 +216,7 @@ impl AudioOut for CpalOut { let h = host(&self.hw.device_class); let outputs = h.output_devices().map(|d| d.filter_map(|x| x.name().ok()).collect()).unwrap_or_default(); let inputs = h.input_devices().map(|d| d.filter_map(|x| x.name().ok()).collect()).unwrap_or_default(); - let output_channels = output_device(&h, &self.hw.default_output).and_then(|d| self.config(&d).ok()).map(|c| c.channels()).unwrap_or(0); + let output_channels = output_device(&h, &self.hw.default_output).and_then(|c| self.config(&c.device).ok()).map(|cfg| cfg.channels()).unwrap_or(0); AudioDevices { hosts: cpal::available_hosts().iter().map(|h| h.name().to_string()).collect(), inputs, outputs, output_channels } } fn configure(&mut self, hw: &AudioHardwarePrefs, document_rate: Option) { @@ -193,13 +224,17 @@ impl AudioOut for CpalOut { self.document_rate = document_rate; // the rate playback mixes at must be known before `start` let host = host(&self.hw.device_class); - if let Some(dev) = output_device(&host, &self.hw.default_output) - && let Ok(cfg) = self.config(&dev) + if let Some(choice) = output_device(&host, &self.hw.default_output) + && let Ok(cfg) = self.config(&choice.device) { self.rate = cfg.sample_rate().0; self.channels = cfg.channels(); + self.note = choice.fallback; } } + fn note(&self) -> Option { + self.note.clone() + } } #[cfg(test)] diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 461aa36e..ed7ad14c 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -62,6 +62,11 @@ pub trait AudioOut { /// effect at the next `start`; `document_rate` is the sequence sample rate for "Attempt to /// force hardware to document sample rate". fn configure(&mut self, _hw: &filmcraft_engine::settings::AudioHardwarePrefs, _document_rate: Option) {} + /// A hardware note for the status bar — e.g. the default output device could not be opened + /// and a fallback device is in use, or no output device was found — or None. + fn note(&self) -> Option { + None + } } /// What the platform audio layer can open (Settings ▸ Audio Hardware). @@ -494,6 +499,9 @@ impl FilmcraftApp { self.start_audio(); } } + if let Some(note) = self.audio.as_ref().and_then(|a| a.note()) { + self.ui.status = note; + } if prev.is_none() && !self.workspace_restored { // reopen the workspace in use when the app last closed self.workspace_restored = true; @@ -758,7 +766,12 @@ impl FilmcraftApp { }) }; match a.start(fill) { - Ok(_) => self.playback.audio_clock = true, + Ok(_) => { + self.playback.audio_clock = true; + if let Some(note) = self.audio.as_ref().and_then(|audio| audio.note()) { + self.ui.status = note; + } + } Err(e) => { log::warn!("audio output unavailable: {e}"); self.playback.audio_clock = false; diff --git a/crates/ui-egui/tests/playback_clock_ui.rs b/crates/ui-egui/tests/playback_clock_ui.rs index 81570d16..3fb7b3e6 100644 --- a/crates/ui-egui/tests/playback_clock_ui.rs +++ b/crates/ui-egui/tests/playback_clock_ui.rs @@ -16,6 +16,7 @@ struct FakeOut { played: Arc, per_poll: u64, started: bool, + note: Option, } impl AudioOut for FakeOut { @@ -33,6 +34,9 @@ impl AudioOut for FakeOut { fn played_frames(&self) -> Option { self.started.then(|| self.played.fetch_add(self.per_poll, Ordering::SeqCst) + self.per_poll) } + fn note(&self) -> Option { + self.note.clone() + } } /// Play for `steps` × 0.25 s of simulated time with the given output; the playhead in seconds. @@ -41,7 +45,7 @@ fn play_with(per_poll: u64, steps: usize) -> (f64, bool) { s.execute("file.openDemoProject", json!({})).unwrap(); s.set_playhead(Tick::ZERO); let mut app = FilmcraftApp::new(s); - app.audio = Some(Box::new(FakeOut { played: Arc::new(AtomicU64::new(0)), per_poll, started: false })); + app.audio = Some(Box::new(FakeOut { played: Arc::new(AtomicU64::new(0)), per_poll, started: false, note: None })); let mut h = Harness::builder().with_size(egui::vec2(1280.0, 800.0)).with_step_dt(0.25).build_eframe(move |_cc| app); h.step(); h.state_mut().play(1.0); @@ -68,3 +72,23 @@ fn a_playing_audio_device_drives_the_clock() { assert!(t > 0.2, "{t:.2} s"); assert!(audio_clock); } + +/// A fallback note from the audio device (broken ALSA `default`, #23/#106) reaches the status +/// bar on the first frame, and survives while playback runs. +#[test] +fn a_fallback_note_from_the_audio_device_reaches_the_status_bar() { + let mut s = Session::default(); + s.execute("file.openDemoProject", json!({})).unwrap(); + s.set_playhead(Tick::ZERO); + let note = "Audio hardware: the default output device cannot be opened; using 'sofhdadsp'".to_string(); + let mut app = FilmcraftApp::new(s); + app.audio = Some(Box::new(FakeOut { played: Arc::new(AtomicU64::new(0)), per_poll: 4_800, started: false, note: Some(note.clone()) })); + let mut h = Harness::builder().with_size(egui::vec2(1280.0, 800.0)).with_step_dt(0.25).build_eframe(move |_cc| app); + h.step(); + assert_eq!(h.state_mut().ui.status, note, "the note surfaces on the first frame (apply_prefs)"); + h.state_mut().play(1.0); + for _ in 0..16 { + h.step(); + } + assert_eq!(h.state_mut().ui.status, note, "the note survives while playback runs"); +} From 0bfea22e1ba938a642149b8166b71edd52c51c1b Mon Sep 17 00:00:00 2001 From: jnohclee-rgb <336857373+jnohclee-rgb@users.noreply.github.com> Date: Fri, 9 Oct 2026 10:14:06 +0800 Subject: [PATCH 04/35] cli: make --help print usage successfully Fixes #344. Handle the leading help flag before command parsing and decoder registration; cover output and exit status with a native CLI regression. --- apps/filmcraft-cli/src/main.rs | 4 ++++ apps/filmcraft-cli/tests/cli.rs | 13 +++++++++++++ docs/agents.md | 3 ++- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/apps/filmcraft-cli/src/main.rs b/apps/filmcraft-cli/src/main.rs index 13bebb85..5597c9bb 100644 --- a/apps/filmcraft-cli/src/main.rs +++ b/apps/filmcraft-cli/src/main.rs @@ -162,6 +162,10 @@ fn register_hardware_decoders() -> filmcraft_platform::Availability { #[tokio::main] async fn main() { + if matches!(std::env::args().nth(1).as_deref(), Some("--help")) { + print!("{HELP}"); + return; + } if matches!(std::env::args().nth(1).as_deref(), Some("--version" | "-V")) { println!("filmcraft-cli {}", env!("CARGO_PKG_VERSION")); return; diff --git a/apps/filmcraft-cli/tests/cli.rs b/apps/filmcraft-cli/tests/cli.rs index b7fedfc2..2ff45e20 100644 --- a/apps/filmcraft-cli/tests/cli.rs +++ b/apps/filmcraft-cli/tests/cli.rs @@ -24,6 +24,19 @@ fn help_and_usage_errors() { assert_eq!(cli(&["exec", "file.newBin", "notkv"]).status.code(), Some(2)); } +#[test] +fn long_help_matches_help_command() { + let reference = cli(&["help"]); + assert!(reference.status.success()); + let long_help = cli(&["--help"]); + assert!(long_help.status.success(), "--help failed: {}", String::from_utf8_lossy(&long_help.stderr)); + assert_eq!(long_help.stdout, reference.stdout); + assert!(long_help.stderr.is_empty()); + let short_help = cli(&["-h"]); + assert!(short_help.status.success()); + assert_eq!(short_help.stdout, reference.stdout); +} + #[test] fn probe_reports_mpeg_transport_and_program_streams() { let Some(ffmpeg) = ["/opt/homebrew/bin/ffmpeg", "/usr/local/bin/ffmpeg", "/usr/bin/ffmpeg"].into_iter().find(|p| std::path::Path::new(p).exists()) else { diff --git a/docs/agents.md b/docs/agents.md index 2f02bc91..3cc79391 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -165,7 +165,8 @@ Notes: ## 2b. Command-line interface Every command is also one shell call away. Options go anywhere; output is JSON; exit status is 0 on -success, 1 when a command fails and 2 on a usage error. `filmcraft-cli help` prints the reference. +success, 1 when a command fails and 2 on a usage error. `filmcraft-cli help`, `filmcraft-cli --help` +and `filmcraft-cli -h` print the reference and exit successfully. ```sh filmcraft-cli commands razor # find ids (add --json for machine output) From 5f3682fe7ceba1cc9fe51a31b84875091b6d2254 Mon Sep 17 00:00:00 2001 From: txiki739 <111882715+txiki739@users.noreply.github.com> Date: Fri, 9 Oct 2026 10:06:35 +0200 Subject: [PATCH 05/35] #262: file dialogs list extensions in any capitalisation on Linux rfd hands filter extensions to the XDG portal, GTK and Zenity as case-sensitive `*.ext` globs, so `CLIP.MP4` and `clip.Mp4` were hidden. `file_filters::extensions` keeps the given extensions first (the save dialogs' default suffix) and, outside Windows and macOS, adds one bracket pattern per extension (`mp4` -> `[mM][pP]4`), the form the portal's FileChooser docs recommend. Every filtered dialog uses it: import, relink, Open file, Open Project, Save Project and Save As. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 7 ++++ apps/filmcraft/Cargo.toml | 3 ++ apps/filmcraft/README.md | 18 +++++++++ apps/filmcraft/src/file_filters.rs | 62 ++++++++++++++++++++++++++++++ apps/filmcraft/src/main.rs | 36 +++++++++++++---- 5 files changed, 118 insertions(+), 8 deletions(-) create mode 100644 apps/filmcraft/README.md create mode 100644 apps/filmcraft/src/file_filters.rs diff --git a/Cargo.lock b/Cargo.lock index 900806b9..1562edb5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1674,6 +1674,7 @@ dependencies = [ "filmcraft-engine", "filmcraft-platform", "filmcraft-ui-egui", + "glob", "log", "muda", "objc2 0.6.4", @@ -2816,6 +2817,12 @@ dependencies = [ "vello_common", ] +[[package]] +name = "glob" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e4eba85ea1d0a966a983acd07deee566e67395d2d96b6fb39e62b5a833f1eb0b" + [[package]] name = "glow" version = "0.17.0" diff --git a/apps/filmcraft/Cargo.toml b/apps/filmcraft/Cargo.toml index 8095c787..e3f67a28 100644 --- a/apps/filmcraft/Cargo.toml +++ b/apps/filmcraft/Cargo.toml @@ -27,6 +27,9 @@ rfd = { workspace = true } cpal = "0.16" chrono = { version = "0.4", default-features = false, features = ["clock"] } +[dev-dependencies] +glob = "0.3" + [target.'cfg(target_os = "macos")'.dependencies] muda = "0.21" objc2-foundation = { version = "0.3", default-features = false, features = ["std", "NSProcessInfo", "NSString", "NSArray"] } diff --git a/apps/filmcraft/README.md b/apps/filmcraft/README.md new file mode 100644 index 00000000..02813d0c --- /dev/null +++ b/apps/filmcraft/README.md @@ -0,0 +1,18 @@ +# FilmCraft desktop host + +The native host connects the shared egui UI to audio devices, file dialogs, the control +server and platform media APIs. See [contributing](../../docs/contributing.md) for running +the app and [the control protocol](../../docs/control-protocol.md) for automation. + +## File dialogs + +All filtered file dialogs use `file_filters::extensions`. On Linux and the BSDs, rfd's +XDG portal and Zenity backends use case-sensitive globs. The helper adds bracket patterns +so camera files such as `shot.MP4` and `shot.Mp4` appear alongside `shot.mp4`. This also +applies to relinking, opening projects and presets, and save dialogs. Literal extensions +remain first for the default save suffix; macOS and Windows receive ordinary extensions. + +`cargo test --release -p filmcraft --bin filmcraft` checks matching and rejection of +filenames, including mixed case and misleading suffixes, and preserves default save +extensions. The filters follow the +[XDG FileChooser contract](https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.FileChooser.html). diff --git a/apps/filmcraft/src/file_filters.rs b/apps/filmcraft/src/file_filters.rs new file mode 100644 index 00000000..516eb598 --- /dev/null +++ b/apps/filmcraft/src/file_filters.rs @@ -0,0 +1,62 @@ +//! Extension filters for native file dialogs. + +pub fn extensions(exts: &[&str]) -> Vec { + let extensions: Vec<_> = exts.iter().map(|ext| (*ext).to_owned()).collect(); + // XDG portal/Zenity globs are case-sensitive. Keep the literal extensions first so + // Save dialogs can still infer a normal default suffix, then cover mixed case too. + // https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.FileChooser.html + #[cfg(not(any(target_os = "windows", target_os = "macos")))] + { + let mut extensions = extensions; + for ext in exts { + if !ext.bytes().any(|b| b.is_ascii_alphabetic()) { + continue; + } + let mut pattern = String::new(); + for c in ext.chars() { + if c.is_ascii_alphabetic() { + pattern.extend(['[', c.to_ascii_lowercase(), c.to_ascii_uppercase(), ']']); + } else { + pattern.push(c); + } + } + extensions.push(pattern); + } + extensions + } + // Native macOS and Windows filters take ordinary extensions. + #[cfg(any(target_os = "windows", target_os = "macos"))] + extensions +} + +#[cfg(test)] +mod tests { + use super::extensions; + + // rfd's XDG portal and Zenity backends turn each extension into `*.{ext}`. + // Exercise those case-sensitive patterns against filenames, as the picker does. + #[cfg(not(any(target_os = "windows", target_os = "macos")))] + #[test] + fn camera_files_are_visible_in_every_capitalization() { + let filters: Vec<_> = extensions(&["mp4", "mov", "m2ts", "wav", "fcproj"]).iter().map(|ext| glob::Pattern::new(&format!("*.{ext}")).unwrap()).collect(); + for name in ["camera.mp4", "camera.MP4", "camera.Mp4", "clip.MOV", "clip.moV", "take.M2TS", "sound.WaV", "Edit.FCPROJ", "Edit.FcProj"] { + assert!(filters.iter().any(|f| f.matches(name)), "the picker hides {name}"); + } + for name in ["camera.mp4.bak", "camera.mp44", "camera.xmp4", "notes.txt", "mp4"] { + assert!(!filters.iter().any(|f| f.matches(name)), "the picker admits {name}"); + } + } + + #[test] + fn save_filters_keep_the_literal_default_extension() { + assert_eq!(extensions(&["mp4", "mov"]).first().map(String::as_str), Some("mp4")); + assert!(extensions(&[]).is_empty()); + assert_eq!(extensions(&["*", "", "123"]), ["*", "", "123"]); + } + + #[cfg(any(target_os = "windows", target_os = "macos"))] + #[test] + fn native_filters_receive_only_literal_extensions() { + assert_eq!(extensions(&["mp4", "mov", "fcproj"]), ["mp4", "mov", "fcproj"]); + } +} diff --git a/apps/filmcraft/src/main.rs b/apps/filmcraft/src/main.rs index b9b279a1..73734cf9 100644 --- a/apps/filmcraft/src/main.rs +++ b/apps/filmcraft/src/main.rs @@ -24,6 +24,7 @@ mod args; mod audio; mod audio_in; mod control_server; +mod file_filters; #[cfg(target_os = "macos")] mod native_menu; mod window_raise; @@ -168,16 +169,31 @@ fn main() -> eframe::Result { // Settings ▸ Audio Hardware is applied on the first frame (`apply_prefs`). app.audio = Some(Box::new(audio::CpalOut::new())); app.hooks.pick_files = Some(Box::new(|exts: &[&str]| { - rfd::FileDialog::new().add_filter("Media", exts).pick_files().unwrap_or_default().into_iter().map(|p| p.to_string_lossy().to_string()).collect() + rfd::FileDialog::new() + .add_filter("Media", &file_filters::extensions(exts)) + .pick_files() + .unwrap_or_default() + .into_iter() + .map(|p| p.to_string_lossy().to_string()) + .collect() })); // Link Media ▸ Locate…, Attach Proxies, Reconnect Full Resolution: one path, not imported. - app.hooks.pick_file_for_relink = - Some(Box::new(|exts: &[&str], _hint| rfd::FileDialog::new().add_filter("Media", exts).pick_file().map(|p| p.to_string_lossy().to_string()))); + app.hooks.pick_file_for_relink = Some(Box::new(|exts: &[&str], _hint| { + rfd::FileDialog::new().add_filter("Media", &file_filters::extensions(exts)).pick_file().map(|p| p.to_string_lossy().to_string()) + })); app.hooks.pick_save = Some(Box::new(|name: &str| { - rfd::FileDialog::new().add_filter("FilmCraft Project", &["fcproj"]).set_file_name(name).save_file().map(|p| p.to_string_lossy().to_string()) + rfd::FileDialog::new() + .add_filter("FilmCraft Project", &file_filters::extensions(&["fcproj"])) + .set_file_name(name) + .save_file() + .map(|p| p.to_string_lossy().to_string()) })); app.hooks.pick_save_as = Some(Box::new(|filter: &str, exts: &[&str], name: &str| { - rfd::FileDialog::new().add_filter(filter, exts).set_file_name(name).save_file().map(|p| p.to_string_lossy().to_string()) + rfd::FileDialog::new() + .add_filter(filter, &file_filters::extensions(exts)) + .set_file_name(name) + .save_file() + .map(|p| p.to_string_lossy().to_string()) })); app.hooks.pick_folder = Some(Box::new(|| rfd::FileDialog::new().pick_folder().map(|p| p.to_string_lossy().to_string()))); app.hooks.open_path = Some(Box::new(open_path)); @@ -185,10 +201,14 @@ fn main() -> eframe::Result { window_raise::raise_without_focus(); })); app.hooks.pick_open_file = Some(Box::new(|filter: &str, exts: &[&str]| { - rfd::FileDialog::new().add_filter(filter, exts).pick_file().map(|p| p.to_string_lossy().to_string()) + rfd::FileDialog::new().add_filter(filter, &file_filters::extensions(exts)).pick_file().map(|p| p.to_string_lossy().to_string()) + })); + app.hooks.pick_open_project = Some(Box::new(|| { + rfd::FileDialog::new() + .add_filter("FilmCraft Project", &file_filters::extensions(&["fcproj"])) + .pick_file() + .map(|p| p.to_string_lossy().to_string()) })); - app.hooks.pick_open_project = - Some(Box::new(|| rfd::FileDialog::new().add_filter("FilmCraft Project", &["fcproj"]).pick_file().map(|p| p.to_string_lossy().to_string()))); #[cfg(target_os = "macos")] { let (rx, update) = native_menu::install(&app, cc.egui_ctx.clone()); From 33f3656071a787e7705fef09d2128620651abe06 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Fri, 9 Oct 2026 09:33:38 -0400 Subject: [PATCH 06/35] #262: drop new apps/filmcraft/README.md; explain file filters in module docs Co-Authored-By: Claude Opus 5.5 --- apps/filmcraft/README.md | 18 ------------------ apps/filmcraft/src/file_filters.rs | 8 ++++++++ 2 files changed, 8 insertions(+), 18 deletions(-) delete mode 100644 apps/filmcraft/README.md diff --git a/apps/filmcraft/README.md b/apps/filmcraft/README.md deleted file mode 100644 index 02813d0c..00000000 --- a/apps/filmcraft/README.md +++ /dev/null @@ -1,18 +0,0 @@ -# FilmCraft desktop host - -The native host connects the shared egui UI to audio devices, file dialogs, the control -server and platform media APIs. See [contributing](../../docs/contributing.md) for running -the app and [the control protocol](../../docs/control-protocol.md) for automation. - -## File dialogs - -All filtered file dialogs use `file_filters::extensions`. On Linux and the BSDs, rfd's -XDG portal and Zenity backends use case-sensitive globs. The helper adds bracket patterns -so camera files such as `shot.MP4` and `shot.Mp4` appear alongside `shot.mp4`. This also -applies to relinking, opening projects and presets, and save dialogs. Literal extensions -remain first for the default save suffix; macOS and Windows receive ordinary extensions. - -`cargo test --release -p filmcraft --bin filmcraft` checks matching and rejection of -filenames, including mixed case and misleading suffixes, and preserves default save -extensions. The filters follow the -[XDG FileChooser contract](https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.FileChooser.html). diff --git a/apps/filmcraft/src/file_filters.rs b/apps/filmcraft/src/file_filters.rs index 516eb598..bc7dab59 100644 --- a/apps/filmcraft/src/file_filters.rs +++ b/apps/filmcraft/src/file_filters.rs @@ -1,4 +1,12 @@ //! Extension filters for native file dialogs. +//! +//! Every filtered file dialog (import, relink, open file/preset, open/save project) builds +//! its filter with [`extensions`]. On Linux and the BSDs, rfd's XDG portal, GTK and Zenity +//! backends turn each extension into a case-sensitive `*.{ext}` glob, so camera files such +//! as `shot.MP4` or `shot.Mp4` would be hidden. The helper appends one bracket pattern per +//! extension (`mp4` -> `[mM][pP]4`) after the literal extensions, which stay first so save +//! dialogs keep their normal default suffix. macOS and Windows dialogs already ignore case +//! and receive the plain list. pub fn extensions(exts: &[&str]) -> Vec { let extensions: Vec<_> = exts.iter().map(|ext| (*ext).to_owned()).collect(); From 7a719fe411faa631c3d8b2cebe10e16b22380dbe Mon Sep 17 00:00:00 2001 From: David Campos <132998405+David-T-Campos@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:25:06 +0400 Subject: [PATCH 07/35] project: share sequences across edit snapshots --- crates/interchange/src/common.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/interchange/src/common.rs b/crates/interchange/src/common.rs index 5527e06e..b8e6b440 100644 --- a/crates/interchange/src/common.rs +++ b/crates/interchange/src/common.rs @@ -378,7 +378,7 @@ impl Builder { /// Add an empty sequence item (filled later with [`Builder::put_sequence`]). pub fn reserve_sequence(&mut self, name: &str, settings: SequenceSettings, bin: Option) -> ItemId { let seq = empty_sequence(settings); - self.p.add_item(name, Label::Forest, ItemKind::Sequence(Box::new(seq)), bin) + self.p.add_item(name, Label::Forest, ItemKind::Sequence(std::sync::Arc::new(seq)), bin) } pub fn put_sequence(&mut self, id: ItemId, mut seq: Sequence) { From 5f6c57984a7d486a04c415e9cf4ebf3192f63d10 Mon Sep 17 00:00:00 2001 From: David Campos <132998405+David-T-Campos@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:25:46 +0400 Subject: [PATCH 08/35] project: share sequences across edit snapshots (lib.rs) --- crates/project/src/lib.rs | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/crates/project/src/lib.rs b/crates/project/src/lib.rs index 40c2eb9b..eb455920 100644 --- a/crates/project/src/lib.rs +++ b/crates/project/src/lib.rs @@ -256,7 +256,9 @@ impl MediaClip { #[allow(clippy::large_enum_variant)] pub enum ItemKind { Media(MediaClip), - Sequence(Box), + /// Shared so project snapshots used by undo and background readers do not deep-copy every + /// timeline. Mutators detach with `Arc::make_mut`. + Sequence(std::sync::Arc), /// A subclip: a media item restricted to a range. Subclip { parent: ItemId, @@ -321,7 +323,7 @@ impl ProjectItem { } pub fn as_sequence_mut(&mut self) -> Option<&mut Sequence> { match &mut self.kind { - ItemKind::Sequence(s) => Some(s), + ItemKind::Sequence(s) => Some(std::sync::Arc::make_mut(s)), _ => None, } } @@ -1440,7 +1442,7 @@ impl Project { let id = TrackId(self.alloc_id()); seq.audio_tracks.push(Track::new(id, TrackKind::Audio, format!("Audio {}", i + 1))); } - self.add_item(name, Label::Forest, ItemKind::Sequence(Box::new(seq)), bin) + self.add_item(name, Label::Forest, ItemKind::Sequence(std::sync::Arc::new(seq)), bin) } /// Re-base an item's media time: whatever was at media time `t` is at `t - delta` afterwards @@ -1462,6 +1464,7 @@ impl Project { } for it in self.items.values_mut() { if let ItemKind::Sequence(seq) = &mut it.kind { + let seq = std::sync::Arc::make_mut(seq); for t in seq.all_tracks_mut() { for ti in t.items.iter_mut().filter(|ti| users.contains(&ti.item)) { ti.source_in -= delta; @@ -1893,6 +1896,30 @@ mod tests { assert_eq!(p.root.parent_of(i), None); } + #[test] + fn project_clone_shares_sequences_until_mutated() { + fn sequence_arc(p: &Project, id: ItemId) -> &std::sync::Arc { + match &p.item(id).unwrap().kind { + ItemKind::Sequence(q) => q, + _ => unreachable!(), + } + } + + let (mut p, _, a) = demo_project(); + let b = p.new_sequence("Sequence 02", SequenceSettings::default(), 1, 1, None); + let before = p.clone(); + + assert!(std::sync::Arc::ptr_eq(sequence_arc(&p, a), sequence_arc(&before, a))); + assert!(std::sync::Arc::ptr_eq(sequence_arc(&p, b), sequence_arc(&before, b))); + + p.sequence_mut(a).unwrap().mark_in = Some(Tick(1)); + + assert!(!std::sync::Arc::ptr_eq(sequence_arc(&p, a), sequence_arc(&before, a))); + assert!(std::sync::Arc::ptr_eq(sequence_arc(&p, b), sequence_arc(&before, b))); + assert_eq!(before.sequence(a).unwrap().mark_in, None); + assert_eq!(Project::from_json(&p.to_json()).unwrap(), p); + } + #[test] fn nest_cycle_finds_sequences_that_contain_themselves() { let (mut p, clip, a) = demo_project(); @@ -1958,4 +1985,4 @@ mod tests { nest.frame_hold = Some(f(100)); assert_eq!(p.nest_overhang(&nest), Some(nest.range())); } -} + } From 5b01f8720f1d4843464e65b6427bebf029a0ccf5 Mon Sep 17 00:00:00 2001 From: David Campos <132998405+David-T-Campos@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:26:06 +0400 Subject: [PATCH 09/35] project: share sequences across edit snapshots (sequence_extras.rs) --- crates/engine/src/sequence_extras.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/src/sequence_extras.rs b/crates/engine/src/sequence_extras.rs index ff6cafd6..389590c8 100644 --- a/crates/engine/src/sequence_extras.rs +++ b/crates/engine/src/sequence_extras.rs @@ -428,7 +428,7 @@ fn simplify(s: &mut Session, p: &Value) -> Result { let report = simplify_sequence(pr, &mut q, &o); q.check().map_err(EngineError::Other)?; let bin = pr.root.parent_of(seq_id).filter(|b| b.0 != 0); - let id = pr.add_item(&name, src.label, ItemKind::Sequence(Box::new(q)), bin); + let id = pr.add_item(&name, src.label, ItemKind::Sequence(std::sync::Arc::new(q)), bin); st.active_sequence = Some(id); if !st.open_sequences.contains(&id) { st.open_sequences.push(id); From e53ac7eff9ad55ee76758b1a062ac5bc28d4779c Mon Sep 17 00:00:00 2001 From: David Campos <132998405+David-T-Campos@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:26:27 +0400 Subject: [PATCH 10/35] project: share sequences across edit snapshots (lib.rs) --- crates/interchange/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/interchange/src/lib.rs b/crates/interchange/src/lib.rs index 61f46275..ad053755 100644 --- a/crates/interchange/src/lib.rs +++ b/crates/interchange/src/lib.rs @@ -438,6 +438,7 @@ pub fn merge_into(target: &mut Project, fragment: Imported, bin: Option) it.created = it.id.0; match &mut it.kind { ItemKind::Sequence(seq) => { + let seq = std::sync::Arc::make_mut(seq); for m in &mut seq.markers { m.id = MarkerId(target.alloc_id()); } @@ -510,6 +511,7 @@ pub fn rebase_source_timecode(project: &mut Project, item: ItemId, start_tc_fram } for it in project.items.values_mut() { if let ItemKind::Sequence(seq) = &mut it.kind { + let seq = std::sync::Arc::make_mut(seq); for t in seq.all_tracks_mut() { for ti in t.items.iter_mut().filter(|ti| ti.item == item) { ti.source_in -= off; From 101d2686edfc0c1c6201591d92c0db7dfe71660a Mon Sep 17 00:00:00 2001 From: David Campos <132998405+David-T-Campos@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:26:54 +0400 Subject: [PATCH 11/35] project: share sequences across edit snapshots (clip_ops.rs) --- crates/engine/src/clip_ops.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/clip_ops.rs b/crates/engine/src/clip_ops.rs index 27e6fff3..f9c0e077 100644 --- a/crates/engine/src/clip_ops.rs +++ b/crates/engine/src/clip_ops.rs @@ -949,6 +949,7 @@ fn consolidate_duplicates(s: &mut Session, _: &Value) -> Result { for it in pr.items.values_mut() { match &mut it.kind { ItemKind::Sequence(q) => { + let q = std::sync::Arc::make_mut(q); for t in q.all_tracks_mut() { for i in &mut t.items { if let Some(k) = map.get(&i.item) { @@ -1768,4 +1769,4 @@ fn short_clips(s: &Session, clips: &[u64]) -> Vec { (frames > 0).then(|| json!({"clip": c, "shortByFrames": frames})) }) .collect() -} + } From 510156ea018e48cc60f2ac22eecb2e5b3977b79e Mon Sep 17 00:00:00 2001 From: David Campos <132998405+David-T-Campos@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:27:15 +0400 Subject: [PATCH 12/35] project: share sequences across edit snapshots (fixtures.rs) --- crates/format/tests/fixtures.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/format/tests/fixtures.rs b/crates/format/tests/fixtures.rs index 7cfe2281..e99a7853 100644 --- a/crates/format/tests/fixtures.rs +++ b/crates/format/tests/fixtures.rs @@ -131,7 +131,7 @@ pub fn big_project(n: usize) -> Project { let seq_id = ItemId(21); let mut next = 1_000_000u64; let seq = match &mut p.items.get_mut(&seq_id).unwrap().kind { - ItemKind::Sequence(s) => s, + ItemKind::Sequence(s) => std::sync::Arc::make_mut(s), _ => unreachable!(), }; let v0 = seq.video_tracks[0].items[0].clone(); From 89474eaeb508c029e9939d9aa2db4a8e55cd753f Mon Sep 17 00:00:00 2001 From: David Campos <132998405+David-T-Campos@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:27:55 +0400 Subject: [PATCH 13/35] project: share sequences across edit snapshots (scripted.rs) Add test for shortcut dispatch in a single input frame. --- crates/ui-egui/tests/scripted.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/crates/ui-egui/tests/scripted.rs b/crates/ui-egui/tests/scripted.rs index 63903a48..700010e5 100644 --- a/crates/ui-egui/tests/scripted.rs +++ b/crates/ui-egui/tests/scripted.rs @@ -506,6 +506,29 @@ fn keyboard_shortcuts_dialog_assigns_live_and_cancel_restores() { assert_eq!(track_clips(&d.sequence(), 0).len(), n0 + 1); } +#[test] +fn shortcuts_in_one_input_frame_are_all_dispatched() { + let mut d = Driver::demo(); + d.exec("playhead.set", json!({"seconds": 2.0})); + d.ok("ui.set", json!({"focused": "Timeline"})); + let before = d.sequence(); + let n0 = track_clips(&before, 0).len(); + let ph = before["playhead"].as_i64().unwrap(); + + // Raw OS-style events, deliberately in the same egui input frame. These bypass the control + // channel's synthetic pointer sequencing and exercise normal shortcut dispatch directly. + let command = egui::Modifiers { command: true, ..Default::default() }; + let key = |key, modifiers| egui::Event::Key { key, physical_key: Some(key), pressed: true, repeat: false, modifiers }; + let events = &mut d.harness.input_mut().events; + events.push(key(egui::Key::K, command)); + events.push(key(egui::Key::ArrowRight, egui::Modifiers::default())); + d.frames(1); + + let after = d.sequence(); + assert_eq!(track_clips(&after, 0).len(), n0 + 1, "Cmd+K was lost when another key arrived in the same input frame"); + assert!(after["playhead"].as_i64().unwrap() > ph, "Right was lost when another key arrived in the same input frame"); +} + /// Waveform peaks are cached per item id; ids repeat across projects, so opening another project /// must not reuse the previous project's peaks (it showed a long mission-audio clip as silent). #[test] From 626df5ac86c0ba8d6991d4cfd408f880c876d792 Mon Sep 17 00:00:00 2001 From: Cathy Shapiro Date: Fri, 9 Oct 2026 10:59:56 -0500 Subject: [PATCH 14/35] WINCI: Repair upstream HEVC timing and CRLF translation validation Delegate HEVC VUI timing to NVENC's configured frame rate instead of the explicit timing pair reversed by the affected driver. Preserve existing stream quality/timing assertions and cover integer and fractional rates. Normalize line endings in the translation test's source scanner so it excludes test modules in Windows checkouts as well as LF checkouts. Assisted-by: Codex --- crates/platform/src/nvenc/session.rs | 8 +++++--- crates/platform/tests/nvenc_hevc.rs | 16 ++++++++++++++++ crates/ui-egui/src/i18n/mod.rs | 18 ++++++++++++++++-- 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/crates/platform/src/nvenc/session.rs b/crates/platform/src/nvenc/session.rs index 1791c017..6f8d1cfa 100644 --- a/crates/platform/src/nvenc/session.rs +++ b/crates/platform/src/nvenc/session.rs @@ -408,9 +408,11 @@ impl Session { vui.colourPrimaries = u32::from(p.signal.primaries); vui.transferCharacteristics = u32::from(p.signal.transfer); vui.colourMatrix = u32::from(p.signal.matrix); - vui.timingInfoPresentFlag = 1; - vui.numUnitInTicks = p.fps.1; - vui.timeScale = p.fps.0; + // Let NVENC derive timing from frameRateNum/frameRateDen. Some drivers + // reverse the explicitly supplied HEVC VUI timing pair. + vui.timingInfoPresentFlag = 0; + vui.numUnitInTicks = 0; + vui.timeScale = 0; } } diff --git a/crates/platform/tests/nvenc_hevc.rs b/crates/platform/tests/nvenc_hevc.rs index d1083529..acd2bee9 100644 --- a/crates/platform/tests/nvenc_hevc.rs +++ b/crates/platform/tests/nvenc_hevc.rs @@ -321,3 +321,19 @@ fn a_keyframe_every_one_or_two_pictures_is_written_without_b_frames() { assert_eq!(n + dec.flush().len(), 12, "keyint {keyint}"); } } + +#[test] +fn vui_timing_matches_integer_and_fractional_frame_rates() { + let _one = SERIAL.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + for fps in [(24, 1), (25, 1), (30_000, 1_001)] { + let mut cfg = config(640, 360); + cfg.fps = fps; + cfg.bframes = false; + let Some((enc, _, _)) = encode_all(&cfg, 2) else { return }; + let sps_nal = enc.parameter_sets().0; + let sps = Sps::parse(&unescape_rbsp(&sps_nal[2..])).unwrap(); + let (units, scale) = sps.vui.and_then(|v| v.timing).expect("HEVC VUI timing"); + assert!(units > 0 && scale > 0); + assert_eq!(u64::from(scale) * u64::from(fps.1), u64::from(units) * u64::from(fps.0), "timing for {fps:?}"); + } +} diff --git a/crates/ui-egui/src/i18n/mod.rs b/crates/ui-egui/src/i18n/mod.rs index c04416c6..3e7e4869 100644 --- a/crates/ui-egui/src/i18n/mod.rs +++ b/crates/ui-egui/src/i18n/mod.rs @@ -369,14 +369,28 @@ mod tests { dirs.push(p); } else if p.extension().is_some_and(|e| e == "rs") { let text = std::fs::read_to_string(&p).unwrap_or_default(); - let cut = text.find("#[cfg(test)]\nmod tests").unwrap_or(text.len()); - out.push((p.display().to_string(), text[..cut].to_string())); + out.push((p.display().to_string(), production_source(&text))); } } } out } + fn production_source(text: &str) -> String { + let text = text.replace("\r\n", "\n"); + let cut = text.find("#[cfg(test)]\nmod tests").unwrap_or(text.len()); + text[..cut].to_string() + } + + #[test] + fn translation_scan_excludes_test_modules_with_either_line_ending() { + let source = "tl!(\"Save\");\n#[cfg(test)]\nmod tests { tl!(\"test-only string\"); }"; + for text in [source.to_string(), source.replace('\n', "\r\n")] { + let production = production_source(&text); + assert_eq!(tl_literals(&production), vec!["Save"]); + } + } + /// The (unescaped) string literals passed to `tl!` and `tlf!` in a source file. fn tl_literals(text: &str) -> Vec { let mut out = Vec::new(); From c25cc5b18b0310e47414fc48045e716240c7ee0a Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Fri, 9 Oct 2026 21:02:47 +0200 Subject: [PATCH 15/35] FCP7 XML: keep the source channel of split mono audio tracks (#463) --- crates/interchange/src/fcp7.rs | 12 +++++++++- crates/interchange/tests/fcp7.rs | 40 ++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/crates/interchange/src/fcp7.rs b/crates/interchange/src/fcp7.rs index 1b94f712..f1af5aaf 100644 --- a/crates/interchange/src/fcp7.rs +++ b/crates/interchange/src/fcp7.rs @@ -482,6 +482,16 @@ impl<'a, 'i> Imp<'a, 'i, '_> { if let Some(l) = path_text(c, &["labels", "label2"]).and_then(label_from) { ti.label = l; } + // `sourcetrack/trackindex` (1-based) picks the source channel of a mono track split off a multichannel file (#463). + // Index 1 is also what a stereo clip on a stereo track carries, so it keeps the default (empty) channel mapping. + if kind == TrackKind::Audio + && let Some(st) = child(c, "sourcetrack") + && let Some(idx) = child_i64(st, "trackindex") + && idx > 1 + && let Ok(ch) = u16::try_from(idx - 1) + { + ti.source_channels = vec![ch]; + } if let Some(mode) = child_text(c, "compositemode") { match BLEND_XML.iter().find(|(n, _)| n.eq_ignore_ascii_case(mode)) { Some((_, v)) if *v != 0 => set_param(&mut ti, "opacity", "blend", Param::new(ParamValue::Choice(*v))), @@ -962,7 +972,7 @@ impl Exp<'_, '_> { if kind == TrackKind::Audio { self.w.open("sourcetrack", &[]); self.w.text("mediatype", "audio"); - self.w.text("trackindex", 1); + self.w.text("trackindex", c.source_channels.first().map_or(1, |ch| u32::from(*ch).saturating_add(1))); self.w.close(); } if let Some(blend) = param(c, "opacity", "blend").and_then(|p| p.value.as_f64()).map(|v| v as u32).filter(|v| *v != 0) { diff --git a/crates/interchange/tests/fcp7.rs b/crates/interchange/tests/fcp7.rs index 4d3677ef..8d36da38 100644 --- a/crates/interchange/tests/fcp7.rs +++ b/crates/interchange/tests/fcp7.rs @@ -527,3 +527,43 @@ fn sub_frame_clips_keep_equal_source_and_record_spans() { assert_eq!(out_f - in_f, end_f - start_f, "start {start}, in {src}, duration {dur} (tenths of a second): {spans:?}"); } } + +/// #463: a stereo file split onto two mono tracks (A1 = channel 1, A2 = channel 2) lost the right channel, +/// because every audio clip item was written with `sourcetrack/trackindex` 1 and the importer ignored it. +#[test] +fn split_mono_tracks_keep_their_source_channel() { + let r = FrameRate::FPS_23_976; + let mut p = Project::new("P"); + let a = media(&mut p, "/m/a.wav", false, true, r); + let s = sequence(&mut p, "t", r, false); + let left = clip(&mut p, s, TrackKind::Audio, 0, a, 0, 24, 0); + let right = clip(&mut p, s, TrackKind::Audio, 1, a, 0, 24, 0); + for (id, ch) in [(left, 0u16), (right, 1u16)] { + let (_, it) = p.sequence_mut(s).unwrap().find_item_mut(id).unwrap(); + it.source_channels = vec![ch]; + } + let (imp, xml, _) = roundtrip(&p, s, Format::Fcp7Xml, &ExportOptions::default()); + assert!(xml.contains("2"), "{xml}"); + let seq = imp.project.sequence(only_seq(&imp)).unwrap(); + let channels = |t: usize| seq.tracks(TrackKind::Audio)[t].items.iter().map(|i| i.source_channels.clone()).collect::>(); + assert_eq!(channels(1), vec![vec![1u16]], "{xml}"); + let (bytes, _) = export(&imp.project, only_seq(&imp), Format::Fcp7Xml, &ExportOptions::default()).expect("export"); + assert!(String::from_utf8(bytes).unwrap().contains("2")); +} + +#[test] +fn audio_sourcetrack_trackindex_2_imports_as_source_channel_1() { + let doc = format!( + r#" + +S48{RATE} + +"# + ); + let (imp, _) = import(doc.as_bytes(), Format::Fcp7Xml, None).unwrap(); + let seq = imp.project.sequence(only_seq(&imp)).unwrap(); + assert_eq!(seq.tracks(TrackKind::Audio)[0].items[0].source_channels, vec![1u16]); +} From 23f79890fd205cbabdbc296e5e120f38234db589 Mon Sep 17 00:00:00 2001 From: sanyamsingh001 <314051937+sanyamsingh001@users.noreply.github.com> Date: Sat, 10 Oct 2026 06:31:59 +1000 Subject: [PATCH 16/35] UI.SCALAR_PRESS_ORIGIN: Start scalar graph drags at the press position Co-authored-by: Codex --- crates/gpu/src/tests.rs | 2 +- crates/platform/tests/setting.rs | 10 +- crates/ui-egui/src/panels/effect_controls.rs | 148 ++++++++++++++++++- 3 files changed, 155 insertions(+), 5 deletions(-) diff --git a/crates/gpu/src/tests.rs b/crates/gpu/src/tests.rs index 44714db5..8953da0a 100644 --- a/crates/gpu/src/tests.rs +++ b/crates/gpu/src/tests.rs @@ -204,7 +204,7 @@ fn gpu_push_matches_cpu_with_alpha_and_surrounding_blends() { c.composite_prepared(&plan, Some(&prepare(&plan))); let (_, _, gpu) = c.read_output().expect("readback"); let mut diffs: Vec = - cpu.chunks_exact(4).zip(gpu.chunks_exact(4)).map(|(a, b)| (0..3).map(|k| a[k].abs_diff(b[k]) as u32).max().unwrap()).collect(); + cpu.as_chunks::<4>().0.iter().zip(gpu.as_chunks::<4>().0).map(|(a, b)| (0..3).map(|k| a[k].abs_diff(b[k]) as u32).max().unwrap()).collect(); diffs.sort_unstable(); let p99 = diffs[diffs.len() * 99 / 100]; let mean = diffs.iter().sum::() as f64 / diffs.len() as f64; diff --git a/crates/platform/tests/setting.rs b/crates/platform/tests/setting.rs index c177e55d..cae45b19 100644 --- a/crates/platform/tests/setting.rs +++ b/crates/platform/tests/setting.rs @@ -36,8 +36,14 @@ fn off_uses_the_software_decoder() { assert_eq!(d.name(), "FilmCraft HEVC"); return; } - let (backend, hw_name) = if cfg!(target_os = "macos") { ("VideoToolbox", "VideoToolbox HEVC") } else { ("Media Foundation", "Media Foundation HEVC") }; - assert_eq!(d.name(), hw_name); + let backend = if cfg!(target_os = "linux") { + assert!(d.name().starts_with("VA-API HEVC ("), "Linux Auto decoder: {}", d.name()); + "VA-API" + } else { + let (backend, hw_name) = if cfg!(target_os = "macos") { ("VideoToolbox", "VideoToolbox HEVC") } else { ("Media Foundation", "Media Foundation HEVC") }; + assert_eq!(d.name(), hw_name); + backend + }; assert_eq!(filmcraft_codecs::hw::hw_backend(), Some(backend), "perf.stats reports the backend"); assert!(filmcraft_platform::registered(), "register() put the factory in the registry"); let h1 = filmcraft_codecs::hw::hw_stats(); diff --git a/crates/ui-egui/src/panels/effect_controls.rs b/crates/ui-egui/src/panels/effect_controls.rs index e3ae049c..ff149794 100644 --- a/crates/ui-egui/src/panels/effect_controls.rs +++ b/crates/ui-egui/src/panels/effect_controls.rs @@ -28,6 +28,19 @@ fn timeline_time_of(it: &TrackItem, m: Tick) -> Tick { it.start + Tick(((m - it.source_in).0 as f64 / it.speed.abs().max(1e-6)) as i64) } +// The press frame can include movement before the button went down. Start from the press +// origin so that only gesture motion enters the preview, including same-frame movement. +fn graph_drag_offset(ui: &egui::Ui, response: &egui::Response, previous: egui::Vec2) -> egui::Vec2 { + if response.drag_started() { + response + .interact_pointer_pos() + .zip(ui.input(|i| i.pointer.press_origin())) + .map_or_else(|| response.drag_delta(), |(position, origin)| position - origin) + } else { + previous + response.drag_delta() + } +} + fn selected_clip(app: &FilmcraftApp) -> Option<(ClipId, TrackItem, filmcraft_project::TrackKind)> { let seq = app.session.active_sequence()?; let mut best = None; @@ -948,7 +961,7 @@ pub(crate) fn graph_rows( p.circle_filled(hp, 3.5, Color32::from_rgb(0xd0, 0xd0, 0xd0)); let hr = ui.interact(Rect::from_center_size(hp, vec2(10.0, 10.0)), hid.with("h"), Sense::drag()); if hr.dragged() { - let nx = hdx + hr.drag_delta().x; + let nx = graph_drag_offset(ui, &hr, vec2(hdx, 0.0)).x; ui.data_mut(|d| d.insert_temp(hid, nx)); } if hr.drag_stopped() { @@ -960,7 +973,7 @@ pub(crate) fn graph_rows( } p.circle_filled(c, 4.5, if resp.dragged() { t.hot_text } else { Color32::from_rgb(0xe0, 0xe0, 0xe0) }); if resp.dragged() { - let ny = dy + resp.drag_delta().y; + let ny = graph_drag_offset(ui, &resp, vec2(0.0, dy)).y; ui.data_mut(|d| d.insert_temp(id, ny)); p.text(c + vec2(8.0, -10.0), Align2::LEFT_BOTTOM, format!("{:.dec$}", v_of(c.y)), Tokens::ui(10.5), t.hot_text); } @@ -1141,3 +1154,134 @@ mod lane_tests { assert_eq!(labels, ["00:01:00:00", "00:01:02:00", "00:01:04:00"]); } } + +#[cfg(test)] +mod graph_drag_tests { + use super::*; + use filmcraft_project::{Interpolation, Keyframe, Param}; + + struct Driver { + app: FilmcraftApp, + ctx: egui::Context, + clip: ClipId, + time: f64, + } + + impl Driver { + fn new() -> Self { + let mut session = filmcraft_engine::Session::default(); + session.execute("file.openDemoProject", json!({})).unwrap(); + let seq = session.state.active_sequence.unwrap(); + let clip = session.active_sequence().unwrap().video_tracks[0].items[0].id; + let it = std::sync::Arc::make_mut(&mut session.project).sequence_mut(seq).unwrap().find_item_mut(clip).unwrap().1; + let mut a = Keyframe::new(it.source_in + Tick(it.duration.0 / 4), ParamValue::Float(40.0)); + let mut b = Keyframe::new(it.source_in + Tick(it.duration.0 * 3 / 4), ParamValue::Float(80.0)); + a.interp = Interpolation::Bezier; + b.interp = Interpolation::Bezier; + it.effects + .iter_mut() + .find(|e| e.effect == "motion") + .unwrap() + .params + .insert("scale".into(), Param { value: ParamValue::Float(40.0), keyframes: vec![a, b] }); + let app = FilmcraftApp::new(session); + let ctx = egui::Context::default(); + crate::theme::install(&ctx, &app.tokens); + Self { app, ctx, clip, time: 0.0 } + } + + fn param(&self) -> Param { + self.app.session.active_sequence().unwrap().find_item(self.clip).unwrap().1.effect("motion").unwrap().params["scale"].clone() + } + + fn undo_len(&mut self) -> usize { + self.app.session.execute("history.list", json!({})).unwrap()["undo"].as_array().unwrap().len() + } + + // Draw the actual scalar graph and execute its emitted command, as the panel does. + // Current paint shapes give a real held-frame position rather than previous telemetry. + fn frame(&mut self, events: Vec) -> [Pos2; 2] { + self.time += 0.1; + self.app.auto.begin_frame(); + let mut out = self.ctx.run_ui(egui::RawInput { time: Some(self.time), events, ..Default::default() }, |ui| { + let it = self.app.session.active_sequence().unwrap().find_item(self.clip).unwrap().1.clone(); + let index = it.effects.iter().position(|e| e.effect == "motion").unwrap(); + let effect = &it.effects[index]; + let pd = effect.def().unwrap().param("scale").unwrap(); + let body = Rect::from_min_size(pos2(8.0, 8.0), vec2(320.0, 200.0)); + let lane = Rect::from_min_max(pos2(360.0, 8.0), pos2(760.0, 220.0)); + let mut actions = Vec::new(); + graph_rows(&mut self.app, ui, body, self.clip, index, None, pd, &effect.params["scale"], &lane, &it, &mut actions); + run(&mut self.app, ui.ctx(), actions); + }); + out.textures_delta.clear(); + fn first_circle(shape: &egui::Shape, radius: f32) -> Option { + match shape { + egui::Shape::Circle(circle) if circle.radius == radius => Some(circle.center), + egui::Shape::Vec(shapes) => shapes.iter().find_map(|shape| first_circle(shape, radius)), + _ => None, + } + } + [4.5, 3.5].map(|radius| { + out.shapes.iter().find_map(|shape| first_circle(&shape.shape, radius)).expect("scalar keyframe and outgoing influence are painted") + }) + } + } + + fn button(pos: Pos2, pressed: bool) -> egui::Event { + egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() } + } + + #[test] + fn scalar_graph_press_starts_at_the_pointer_origin_and_release_is_one_undo_step() { + for influence in [false, true] { + for batched in [true, false] { + let mut d = Driver::new(); + let before = d.param(); + let undo = d.undo_len(); + let target = usize::from(influence); + let start = d.frame(vec![])[target]; + // A different prior cursor makes the pre-press movement observable. + d.frame(vec![egui::Event::PointerMoved(start - vec2(80.0, 40.0))]); + if batched { + d.frame(vec![egui::Event::PointerMoved(start), button(start, true)]); + } else { + d.frame(vec![egui::Event::PointerMoved(start)]); + d.frame(vec![button(start, true)]); + } + let pressed = d.frame(vec![])[target]; + assert!(pressed.distance(start) < 0.05, "press cannot jump: influence={influence}, batched={batched}, start={start:?}, pressed={pressed:?}"); + assert_eq!(d.param(), before); + assert_eq!(d.undo_len(), undo); + let offset = if influence { vec2(20.0, 0.0) } else { vec2(0.0, -20.0) }; + d.frame(vec![egui::Event::PointerMoved(start + offset)]); + let preview = d.frame(vec![])[target]; + assert!( + preview.distance(start + offset) < 0.05, + "preview follows only gesture motion: influence={influence}, batched={batched}, start={start:?}, preview={preview:?}" + ); + assert_eq!(d.param(), before, "held preview cannot commit"); + assert_eq!(d.undo_len(), undo); + d.frame(vec![button(start + offset, false)]); + d.frame(vec![]); + let changed = d.param(); + assert_eq!(d.undo_len(), undo + 1); + assert_eq!(changed.keyframes[0].time, before.keyframes[0].time); + assert_eq!(changed.keyframes[1], before.keyframes[1]); + if influence { + assert_eq!(changed.keyframes[0].value, before.keyframes[0].value); + assert!(changed.keyframes[0].out_influence > before.keyframes[0].out_influence); + } else { + assert!(changed.keyframes[0].value.as_f64().unwrap() > before.keyframes[0].value.as_f64().unwrap()); + assert_eq!(changed.keyframes[0].out_influence, before.keyframes[0].out_influence); + } + d.app.session.undo().expect("one graph gesture to undo"); + assert_eq!(d.param(), before); + assert_eq!(d.undo_len(), undo); + d.app.session.redo().expect("the graph gesture to redo"); + assert_eq!(d.param(), changed); + assert_eq!(d.undo_len(), undo + 1); + } + } + } +} From 4955515bcd425dda132896bdccb33017e83e0f50 Mon Sep 17 00:00:00 2001 From: "user.email" Date: Sat, 10 Oct 2026 00:41:44 +0300 Subject: [PATCH 17/35] linux: dev desktop entry only for source-tree runs; both Exec layers; exact Exec match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #246: - Gate the install to dev runs: a debug build, or a release run with FILMCRAFT_DEV_DESKTOP_ENTRY set (the documented cargo run --release flow); never when APPIMAGE or FLATPAK_ID is set — an AppImage mounts at a fresh /tmp/.mount_* each launch, so its Exec path changes every time, and packaged installs ship their own entry and must not write into the profile. - Escape Exec for both desktop-entry layers exactly like packaging/linux/install.sh: shell layer first (` " $ \ get a backslash), then the file-value layer (every backslash doubled), always double-quoted. Binary paths with =, % or control characters are rejected instead of written (install.sh rejects them for the same reason). - Compare the Exec path exactly: parse the first Exec argument through both layers instead of a substring contains, so /a/filmcraft-old no longer counts as ours and a stale entry is rewritten. - eprintln -> log::warn for the skip message (the logging module landed meanwhile). 🤖 Generated with [Kimi Code](https://github.com/storyteller/kimi-code) Co-Authored-By: Kimi --- apps/filmcraft/src/dev_icon.rs | 228 +++++++++++++++++++++++---------- docs/contributing.md | 1 + 2 files changed, 163 insertions(+), 66 deletions(-) diff --git a/apps/filmcraft/src/dev_icon.rs b/apps/filmcraft/src/dev_icon.rs index a00986d2..7eb572ad 100644 --- a/apps/filmcraft/src/dev_icon.rs +++ b/apps/filmcraft/src/dev_icon.rs @@ -3,22 +3,40 @@ //! The Linux shell (Wayland *and* X11) resolves the window to a `.desktop` file by app id (set in //! `main.rs` via `eframe`'s `with_app_id`) and takes the taskbar / window-list icon from its //! `Icon=` entry in the hicolor theme. Packaging installs both -//! (`packaging/linux/ai.storyteller.filmcraft.desktop` and the hicolor PNGs) — but a dev machine -//! running `cargo run` has neither, so KDE/GNOME fall back to the generic Wayland icon. The fix is -//! to ensure a user-level entry at startup: when no entry for the app id exists in any XDG data -//! dir, install one (plus the hicolor icon PNGs, embedded so it works whatever the working -//! directory is) into `$XDG_DATA_HOME`/`~/.local/share`. An existing packaged entry is never -//! overridden; the user-owned dev copy is rewritten when it points at another binary (a dev build -//! moves between checkouts). The shell picks the icon up at the *next* app start; +//! (`packaging/linux/ai.storyteller.filmcraft.desktop` and the hicolor PNGs) — but a source-tree +//! run has neither, so KDE/GNOME fall back to the generic Wayland icon. The fix is to ensure a +//! user-level entry for those runs: when no entry for the app id exists in any XDG data dir, +//! install one (plus the hicolor icon PNGs, embedded so it works whatever the working directory +//! is) into `$XDG_DATA_HOME`/`~/.local/share`. An existing packaged entry is never overridden; +//! the user-owned dev copy is rewritten when it points at another binary (a dev build moves +//! between checkouts). The shell picks the icon up at the *next* app start; //! `update-desktop-database`/`kbuildsycoca` is not needed for icon lookup. //! -//! Every failure is reported and skipped — a missing dev icon must never stop the app +//! Only source-tree runs install anything: a debug build, or a release build with +//! `FILMCRAFT_DEV_DESKTOP_ENTRY` set (the docs' `cargo run --release` flow). Packaged runs are +//! excluded — AppImage (`APPIMAGE`) mounts at a fresh `/tmp/.mount_*` each launch, so its +//! `Exec` path changes every time and it ships its own entry, and a Flatpak (`FLATPAK_ID`) +//! installs its entry through the manifest. `packaging/linux/install.sh` writes the packaged +//! `Exec=` the same two-layer way as here; keep the two in sync. +//! +//! Every failure is logged and skipped — a missing dev icon must never stop the app //! ([`AGENTS.md`](/AGENTS.md) §0 "never crash"). -/// Install the user-level desktop entry and icons when the app id has no entry anywhere (Linux). +/// Install the user-level desktop entry and icons for a source-tree run (Linux). pub fn ensure_dev_desktop_entry() { + // Packaged and sandboxed runs never write into the user's profile: an AppImage mounts at a + // fresh /tmp/.mount_* every launch (the Exec path changes each time) and a Flatpak installs + // its own entry. + if std::env::var_os("APPIMAGE").is_some() || std::env::var_os("FLATPAK_ID").is_some() { + return; + } + // Dev builds only: a debug build, or a release run opted in — a packaged release must not + // touch the profile (the packages and install.sh install the real entry). + if !cfg!(debug_assertions) && std::env::var_os("FILMCRAFT_DEV_DESKTOP_ENTRY").is_none() { + return; + } if let Err(e) = ensure() { - eprintln!("filmcraft: desktop entry (taskbar icon) not installed: {e}"); + log::warn!("desktop entry (taskbar icon) not installed: {e}"); } } @@ -27,7 +45,16 @@ fn ensure() -> Result<(), DevIconError> { let file_name = format!("{}.desktop", crate::APP_ID); let user_dir = user_data_dir()?; let exe = std::env::current_exe().map_err(DevIconError::Io)?; - let exe = exe.to_string_lossy(); + // Not `to_string_lossy`: a substituted U+FFFD would install an `Exec` that can never start + // the app, and the same lossy string would match on every later run, so the broken entry + // would never be rewritten or warned about. + let exe = exe.to_str().ok_or(DevIconError::BadExecPath)?; + if exe.contains('=') || exe.contains('%') || exe.chars().any(|c| c.is_control()) { + // A desktop entry cannot carry these (install.sh rejects them for the same reason): + // '=' separates key and value, '%' introduces a field code, control characters are + // not valid in the string layer. + return Err(DevIconError::BadExecPath); + } for dir in search_dirs(&user_dir) { let candidate = dir.join("applications").join(&file_name); if !candidate.is_file() { @@ -39,7 +66,7 @@ fn ensure() -> Result<(), DevIconError> { return Ok(()); } let body = std::fs::read_to_string(&candidate).map_err(DevIconError::Io)?; - if desktop_file_exec_is_ours(&body, &exe) { + if desktop_file_exec_is_ours(&body, exe) { return Ok(()); } break; // a dev entry from another checkout: fall through and rewrite it @@ -55,7 +82,7 @@ fn ensure() -> Result<(), DevIconError> { } let applications = user_dir.join("applications"); std::fs::create_dir_all(&applications).map_err(DevIconError::Io)?; - let body = desktop_file_content(crate::APP_ID, "FilmCraft", &exe); + let body = desktop_file_content(crate::APP_ID, "FilmCraft", exe); std::fs::write(applications.join(&file_name), body).map_err(DevIconError::Io)?; Ok(()) } @@ -68,8 +95,9 @@ const ICONS: &[(u16, &[u8])] = &[ ]; /// The user data dir we install into: `$XDG_DATA_HOME`, else `$HOME/.local/share` (XDG base -/// directory spec). Untrusted environment: an empty value means the default, and a relative or -/// missing one is an error — never a guess that would write into the working directory. +/// directory spec). Untrusted environment: an empty or relative `XDG_DATA_HOME` means the +/// default, and a missing or relative `HOME` is an error — never a guess that would write +/// into the working directory. fn user_data_dir() -> Result { if let Some(v) = std::env::var_os("XDG_DATA_HOME") && !v.is_empty() @@ -77,7 +105,7 @@ fn user_data_dir() -> Result { { return Ok(std::path::PathBuf::from(v)); } - match std::env::var_os("HOME").filter(|h| !h.is_empty()) { + match std::env::var_os("HOME").filter(|h| !h.is_empty() && std::path::Path::new(h).is_absolute()) { Some(home) => Ok(std::path::PathBuf::from(home).join(".local").join("share")), None => Err(DevIconError::NoHome), } @@ -100,15 +128,71 @@ fn search_dirs(user: &std::path::Path) -> Vec { dirs } -/// Whether the `.desktop` body's `Exec` line points at our binary (a stale dev entry from another -/// checkout does not, and is rewritten; a packaged `Exec=filmcraft` never matches a dev path). +/// Whether the `.desktop` body's `Exec` points at our binary exactly (a stale dev entry from +/// another checkout — `/a/filmcraft-old` is not `/a/filmcraft` — does not, and is rewritten; a +/// packaged `Exec=filmcraft` never matches a source-tree path). fn desktop_file_exec_is_ours(body: &str, exe: &str) -> bool { - body.lines().any(|l| l.strip_prefix("Exec=").is_some_and(|rest| rest.contains(exe))) + exec_line(body).is_some_and(|arg| arg == exe) +} + +/// The first `Exec=` argument (the executable), unescaped through both Desktop Entry layers: +/// first the string layer of the file value (`\\` `\s` `\n` `\t` `\r`, per +/// ), then the shell layer of the +/// argument (double quotes, and `\` before `` ` `"` `\` `$` inside them). `None` when there is no +/// `Exec=` line or its first argument cannot be parsed. +fn exec_line(body: &str) -> Option { + let value = body.lines().find(|l| l.starts_with("Exec=")).and_then(|l| l.strip_prefix("Exec="))?; + let value = unescape_string_value(value); + if let Some(rest) = value.strip_prefix('"') { + let mut out = String::new(); + let mut chars = rest.chars(); + while let Some(c) = chars.next() { + match c { + '"' => return Some(out), + '\\' => match chars.next() { + Some(e @ ('\\' | '"' | '`' | '$')) => out.push(e), + Some(e) => { + out.push('\\'); + out.push(e); + } + None => return None, // a dangling escape ends the argument + }, + c => out.push(c), + } + } + None // no closing quote + } else { + Some(value.split(' ').next().unwrap_or_default().to_string()) + } +} + +/// Undo the desktop entry's string layer: `\\` `\s` `\n` `\t` `\r` are the defined sequences; a +/// backslash before anything else is kept as the character itself (lenient, like desktop file +/// parsers in the wild). +fn unescape_string_value(value: &str) -> String { + let mut out = String::with_capacity(value.len()); + let mut chars = value.chars(); + while let Some(c) = chars.next() { + if c != '\\' { + out.push(c); + continue; + } + match chars.next() { + Some('\\') => out.push('\\'), + Some('s') => out.push(' '), + Some('n') => out.push('\n'), + Some('t') => out.push('\t'), + Some('r') => out.push('\r'), + Some(e) => out.push(e), + None => out.push('\\'), + } + } + out } /// The `.desktop` body: same fields as `packaging/linux/ai.storyteller.filmcraft.desktop`, with /// `Exec` pointing at this exact binary (a dev build isn't on `$PATH`, so `TryExec` is left out) -/// and the icon by theme name. `exe` is shell-quoted to survive spaces in the path. +/// and the icon by theme name. fn desktop_file_content(app_id: &str, name: &str, exe: &str) -> String { let mut out = String::new(); out.push_str("[Desktop Entry]\n"); @@ -116,7 +200,7 @@ fn desktop_file_content(app_id: &str, name: &str, exe: &str) -> String { out.push_str(&format!("Name={name}\n")); out.push_str("GenericName=Video Editor\n"); out.push_str("Comment=Edit video, color and sound\n"); - out.push_str(&format!("Exec={} %F\n", quote(exe))); + out.push_str(&format!("Exec=\"{}\" %F\n", escape_exec_argument(exe))); out.push_str(&format!("Icon={app_id}\n")); out.push_str("Terminal=false\n"); out.push_str("StartupNotify=true\n"); @@ -126,30 +210,26 @@ fn desktop_file_content(app_id: &str, name: &str, exe: &str) -> String { out } -/// Quote `s` for a shell `Exec=` line (Desktop Entry spec §"Exec string": an argument with -/// reserved characters is wrapped in double quotes, escaping `\`, `"`, `` ` `` and `$`). -fn quote(s: &str) -> String { - let needs = s.is_empty() || s.bytes().any(|b| b" \"'\\`$<>&~|;()*?[]#".contains(&b)); - if !needs { - return s.into(); - } - let mut out = String::with_capacity(s.len() + 2); - out.push('"'); +/// Escape `s` for the `Exec=` value's two layers, like `packaging/linux/install.sh`: first the +/// shell layer (`` ` `` `"` `$` `\` get a backslash, and the argument is double-quoted), then +/// the string layer of the file value (every backslash doubled). +fn escape_exec_argument(s: &str) -> String { + let mut shell = String::with_capacity(s.len() + 2); for c in s.chars() { - if matches!(c, '\\' | '"' | '`' | '$') { - out.push('\\'); + if matches!(c, '`' | '"' | '$' | '\\') { + shell.push('\\'); } - out.push(c); + shell.push(c); } - out.push('"'); - out + shell.replace('\\', "\\\\") } -/// Everything that can keep the dev icon out of the taskbar; printed, never fatal. +/// Everything that can keep the dev icon out of the taskbar; logged, never fatal. #[derive(Debug)] enum DevIconError { Io(std::io::Error), NoHome, + BadExecPath, } impl std::fmt::Display for DevIconError { @@ -157,57 +237,73 @@ impl std::fmt::Display for DevIconError { match self { DevIconError::Io(e) => write!(f, "{e}"), DevIconError::NoHome => write!(f, "no home directory for the user data dir"), + DevIconError::BadExecPath => write!(f, "the binary path has a character a desktop entry cannot carry (=, % or a control character), or is not valid UTF-8"), } } } -impl From for DevIconError { - fn from(e: std::io::Error) -> Self { - DevIconError::Io(e) - } -} - #[cfg(test)] mod tests { use super::*; - /// The entry mirrors the packaged one (minus `TryExec`, plus the dev `Exec` path). + /// The entry mirrors the packaged one, derived from the real file so the two cannot drift: + /// minus `TryExec` (a dev build isn't on `$PATH`), plus the quoted dev `Exec` path. #[test] fn desktop_file_matches_the_packaged_fields() { + let packaged = include_str!("../../../packaging/linux/ai.storyteller.filmcraft.desktop"); + let expected = packaged + .lines() + .filter(|l| !l.starts_with("TryExec=")) + .map(|l| if l.starts_with("Exec=") { "Exec=\"/target/debug/filmcraft\" %F" } else { l }) + .collect::>() + .join("\n") + + "\n"; let body = desktop_file_content("ai.storyteller.filmcraft", "FilmCraft", "/target/debug/filmcraft"); - assert_eq!( - body, - "[Desktop Entry]\n\ - Type=Application\n\ - Name=FilmCraft\n\ - GenericName=Video Editor\n\ - Comment=Edit video, color and sound\n\ - Exec=/target/debug/filmcraft %F\n\ - Icon=ai.storyteller.filmcraft\n\ - Terminal=false\n\ - StartupNotify=true\n\ - StartupWMClass=ai.storyteller.filmcraft\n\ - Categories=AudioVideo;Video;AudioVideoEditing;\n\ - Keywords=video;editor;film;timeline;color;grading;nle;\n" - ); + assert_eq!(body, expected); } - /// A path with shell-reserved characters stays one `Exec` argument. + /// Both layers are escaped, like install.sh: shell first (`` ` `` `"` `$` `\` get a + /// backslash), then *every* backslash doubled for the file value — including the escape + /// backslashes just added. #[test] - fn exec_quotes_shell_reserved_paths() { - assert_eq!(quote("/opt/film craft/filmcraft"), "\"/opt/film craft/filmcraft\""); - assert_eq!(quote("/bin/filmcraft"), "/bin/filmcraft"); - assert_eq!(quote("/a$b\"c\\d"), "\"/a\\$b\\\"c\\\\d\""); + fn exec_escapes_both_desktop_entry_layers() { + assert_eq!(escape_exec_argument("/opt/film craft/filmcraft"), "/opt/film craft/filmcraft"); + // a\b → shell a\\b → string a\\\\b + assert_eq!(escape_exec_argument("a\\b"), "a\\\\\\\\b"); + // a$b → shell a\$b → string a\\$b + assert_eq!(escape_exec_argument("a$b"), "a\\\\$b"); } - /// Only an `Exec` line that contains our binary marks the entry as ours — a packaged - /// `Exec=filmcraft` or another checkout's path does not. + /// A written entry reads back as the exact binary it was written for, through both layers. + #[test] + fn exec_round_trips_through_both_layers() { + for exe in ["/home/dev/Projects/filmcraft/target/debug/filmcraft", "/opt/film craft/filmcraft", "/a$b\"c`d\\e"] { + let body = desktop_file_content("ai.storyteller.filmcraft", "FilmCraft", exe); + assert_eq!(exec_line(&body).as_deref(), Some(exe)); + assert!(desktop_file_exec_is_ours(&body, exe)); + } + } + + /// Only the exact executable counts: another checkout's path, a path with our path as a + /// prefix, and a packaged `Exec=filmcraft` are all someone else's entry. #[test] fn desktop_file_exec_is_ours_matches_only_our_binary() { let body = desktop_file_content("ai.storyteller.filmcraft", "FilmCraft", "/a/filmcraft"); - assert!(desktop_file_exec_is_ours(&body, "/a/filmcraft")); + assert!(!desktop_file_exec_is_ours(&body, "/a/filmcraft-old")); assert!(!desktop_file_exec_is_ours(&body, "/b/filmcraft")); assert!(!desktop_file_exec_is_ours("Exec=filmcraft %F\n", "/a/filmcraft")); assert!(!desktop_file_exec_is_ours("Name=filmcraft\n", "/a/filmcraft")); + assert!(!desktop_file_exec_is_ours("Exec=\"dangling\n", "/a/filmcraft")); + } + + /// The string layer is undone before the argument is split: `\s` becomes a space, which + /// ends a bare argument (the shell layer's quoting, not `\s`, is how a space travels), and + /// `\\` is one backslash. install.sh always quotes, so a space in our own `Exec` never + /// splits. + #[test] + fn exec_reads_the_string_layer() { + assert_eq!(exec_line("Exec=/a\\sb %F\n").as_deref(), Some("/a")); + assert_eq!(exec_line("Exec=\"/a\\sb\" %F\n").as_deref(), Some("/a b")); + assert_eq!(exec_line("Exec=/a\\\\b %F\n").as_deref(), Some("/a\\b")); } } diff --git a/docs/contributing.md b/docs/contributing.md index 570259e1..c52729bb 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -40,6 +40,7 @@ cargo xtask web --serve 8765 # the web app on http://12 |---|---| | `FILMCRAFT_CONTROL_PORT` | Same as `--control ` | | `FILMCRAFT_CPU_COMPOSITE=1` | Disables the GPU compositor | +| `FILMCRAFT_DEV_DESKTOP_ENTRY=1` | A release run from a source tree installs the user-level desktop entry for the taskbar icon (a debug build always does; packaged runs never) | | `FILMCRAFT_DATA_DIR` | Same as `--data-dir `: auto-save, crash recovery, settings and logs | | `RUST_LOG` | Log levels for standard error and the log file (see [Logs](#logs)) | From 5b3d2c2a0baa6426b6dd48854f838129f9da648a Mon Sep 17 00:00:00 2001 From: "user.email" Date: Sat, 10 Oct 2026 01:00:30 +0300 Subject: [PATCH 18/35] linux: cargo fmt the dev-icon BadExecPath arm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The to_str() refinement landed unformatted: rustfmt wants the long error arm braced. 🤖 Generated with [Kimi Code](https://github.com/storyteller/kimi-code) Co-Authored-By: Kimi --- apps/filmcraft/src/dev_icon.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/filmcraft/src/dev_icon.rs b/apps/filmcraft/src/dev_icon.rs index 7eb572ad..6e2b7c7f 100644 --- a/apps/filmcraft/src/dev_icon.rs +++ b/apps/filmcraft/src/dev_icon.rs @@ -237,7 +237,9 @@ impl std::fmt::Display for DevIconError { match self { DevIconError::Io(e) => write!(f, "{e}"), DevIconError::NoHome => write!(f, "no home directory for the user data dir"), - DevIconError::BadExecPath => write!(f, "the binary path has a character a desktop entry cannot carry (=, % or a control character), or is not valid UTF-8"), + DevIconError::BadExecPath => { + write!(f, "the binary path has a character a desktop entry cannot carry (=, % or a control character), or is not valid UTF-8") + } } } } From d4d1830d994b58266c4aca379f37ebc578fc8447 Mon Sep 17 00:00:00 2001 From: swillunderscore <201272413+swillunderscore@users.noreply.github.com> Date: Fri, 9 Oct 2026 17:59:58 -0500 Subject: [PATCH 19/35] #430: transitions can be selected, removed, resized and moved (engine) - filmcraft_edit::transitions: where an applied transition may sit (over its cut, within the clips it joins; a fade to or from nothing keeps its clip edge), its alignment as Effect Controls shows it (Center at Cut / Start at Cut / End at Cut / Custom Start), set_span, remove, and drag_span for the Timeline (an end changes the duration, the middle slides it). - EditorState::transition_selection; timeline.select {"transitions": [...]} replaces the clip, edit point and caption selections, and a clip selection clears it. - edit.clear (Delete) removes the selected transitions when no clips are selected; sequence.removeTransition removes given or selected ones; Ripple Delete stays clip-only. - sequence.setTransition also takes duration / frames (following the alignment: Center at Cut and Custom Start move both ends, Start at Cut the end, End at Cut the beginning), align, start, and merge / begin so a drag is one undo step. Out-of-range times are refused before any frame arithmetic, atomically. - sequence inspection reports each transition's alignment and the transition selection. Co-Authored-By: Claude Opus 5.5 --- crates/edit/src/lib.rs | 1 + crates/edit/src/transitions.rs | 406 +++++++++++++++++++++++++++++++++ crates/engine/src/commands.rs | 164 ++++++++++++- crates/engine/src/lib.rs | 4 + crates/engine/src/tests.rs | 108 +++++++++ 5 files changed, 672 insertions(+), 11 deletions(-) create mode 100644 crates/edit/src/transitions.rs diff --git a/crates/edit/src/lib.rs b/crates/edit/src/lib.rs index ed66f951..7cfb7006 100644 --- a/crates/edit/src/lib.rs +++ b/crates/edit/src/lib.rs @@ -22,6 +22,7 @@ pub mod frame_size; pub mod multicam; pub mod through; pub mod transcript; +pub mod transitions; use std::collections::HashMap; diff --git a/crates/edit/src/transitions.rs b/crates/edit/src/transitions.rs new file mode 100644 index 00000000..d28c7f7f --- /dev/null +++ b/crates/edit/src/transitions.rs @@ -0,0 +1,406 @@ +//! Applied transitions: where one may sit over its cut, its alignment, and moving / resizing / +//! removing one (Effect Controls' Duration and Alignment, dragging its edges or its middle in the +//! Timeline, Delete). +//! +//! Premiere (Align transitions; Change transition duration): a transition between two clips stays +//! over their cut, "Center at Cut", "Start at Cut", "End at Cut" or, once dragged off-centre, +//! "Custom Start". Changing its duration moves both ends equally for Center at Cut / Custom Start, +//! only the end for Start at Cut and only the beginning for End at Cut. A one-sided transition (a +//! fade from or to nothing) starts or ends at its clip's edge. +//! +//! Premiere limits a transition by the media its clips have beyond the cut (handles). FilmCraft +//! does not check handles when applying one, so here a transition may cover at most the clips it +//! joins: from the outgoing clip's start to the incoming clip's end. + +use filmcraft_project::{Sequence, Track, Transition, TransitionAlign, TransitionId}; +use filmcraft_time::Tick; + +use crate::{Edge, EditError, Result}; + +/// How a transition sits on its cut, as Effect Controls' Alignment menu shows it. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Alignment { + Center, + Start, + End, + /// Premiere's "Custom Start": dragged to a place over the cut that is none of the others. + Custom, +} + +impl Alignment { + pub const MENU: [Alignment; 3] = [Alignment::Center, Alignment::Start, Alignment::End]; + + pub fn id(self) -> &'static str { + match self { + Alignment::Center => "center", + Alignment::Start => "start", + Alignment::End => "end", + Alignment::Custom => "custom", + } + } + + pub fn label(self) -> &'static str { + match self { + Alignment::Center => "Center at Cut", + Alignment::Start => "Start at Cut", + Alignment::End => "End at Cut", + Alignment::Custom => "Custom Start", + } + } + + /// `center`, `start` or `end` (Custom Start is reached by moving a transition, not chosen). + pub fn parse(s: &str) -> Option { + Alignment::MENU.into_iter().find(|a| a.id().eq_ignore_ascii_case(s) || a.label().eq_ignore_ascii_case(s)) + } +} + +/// Where a transition may be: `(earliest start, cut, latest end)`. Between two clips that is the +/// outgoing clip's start, the cut and the incoming clip's end; for a fade to or from nothing the cut +/// is its clip's edge. `None` when its clips are gone. +pub fn bounds(track: &Track, tr: &Transition) -> Option<(Tick, Tick, Tick)> { + let from = tr.from.and_then(|c| track.item(c)); + let to = tr.to.and_then(|c| track.item(c)); + match (from, to) { + (Some(f), Some(t)) => Some((f.start, t.start, t.end())), + (None, Some(t)) => Some((t.start, t.start, t.end())), + (Some(f), None) => Some((f.start, f.end(), f.end())), + (None, None) => None, + } +} + +/// A transition's alignment on `cut`. Centred means within half a `frame` of the cut, as +/// centring an odd number of frames lands on a frame boundary. +pub fn alignment(tr: &Transition, cut: Tick, frame: Tick) -> Alignment { + match (tr.from, tr.to) { + (None, Some(_)) => return Alignment::Start, + (Some(_), None) => return Alignment::End, + _ => {} + } + if tr.start == cut { + Alignment::Start + } else if tr.end() == cut { + Alignment::End + } else if (tr.start.0.saturating_mul(2).saturating_add(tr.duration.0) - cut.0.saturating_mul(2)).abs() <= frame.0.max(0) { + Alignment::Center + } else { + Alignment::Custom + } +} + +fn find(seq: &Sequence, id: TransitionId) -> Option<(&Track, &Transition)> { + seq.all_tracks().find_map(|t| t.transitions.iter().find(|x| x.id == id).map(|x| (t, x))) +} + +/// Where a transition would start for `duration` and `align`, keeping its cut (and, for Center +/// at Cut / Custom Start, its centre; `snap` puts a centred start on a frame). +pub fn start_for(seq: &Sequence, id: TransitionId, duration: Tick, align: Alignment, frame: Tick, snap: impl Fn(Tick) -> Tick) -> Option { + let (track, tr) = find(seq, id)?; + let (_, cut, _) = bounds(track, tr)?; + let one_sided = alignment(tr, cut, frame); + let align = match one_sided { + Alignment::Start | Alignment::End if tr.from.is_none() || tr.to.is_none() => one_sided, + _ => align, + }; + Some(match align { + Alignment::Start => cut, + Alignment::End => cut - duration, + Alignment::Center => snap(cut - duration.mul_ratio(1, 2)), + // Custom Start: the ends move equally, around where it is now + Alignment::Custom => snap(tr.start + tr.duration.mul_ratio(1, 2) - duration.mul_ratio(1, 2)), + }) +} + +/// Move / resize a transition to cover `start .. start + duration`. Refused (leaving the sequence +/// as it was) when its track is locked, it would be shorter than `min`, it would leave its clips, +/// or it would no longer touch its cut (a one-sided one must keep its edge). +pub fn set_span(seq: &mut Sequence, id: TransitionId, start: Tick, duration: Tick, min: Tick, frame: Tick) -> Result<()> { + let (track, tr) = find(seq, id).ok_or_else(|| EditError::Other(format!("no transition {}", id.0)))?; + if track.locked { + return Err(EditError::Locked); + } + let (lo, cut, hi) = bounds(track, tr).ok_or_else(|| EditError::Other("the transition's clips are gone".into()))?; + if duration < min.max(Tick(1)) { + return Err(EditError::Other("a transition lasts at least one frame".into())); + } + let end = start.0.checked_add(duration.0).map(Tick).ok_or_else(|| EditError::Other("transition out of range".into()))?; + if start < lo || end > hi { + return Err(EditError::Other("a transition can't extend past the clips it joins".into())); + } + let on_cut = match (tr.from, tr.to) { + (None, Some(_)) => start == cut, + (Some(_), None) => end == cut, + _ => start <= cut && cut <= end, + }; + if !on_cut { + return Err(EditError::Other(if tr.from.is_some() && tr.to.is_some() { + "a transition between two clips must stay over their cut".into() + } else { + "a one-sided transition stays on its clip's edge".into() + })); + } + let tid = track.id; + let Some(x) = seq.track_mut(tid).and_then(|t| t.transitions.iter_mut().find(|x| x.id == id)) else { + return Err(EditError::Other(format!("no transition {}", id.0))); + }; + x.start = start; + x.duration = duration; + // the stored alignment (interchange): Custom Start has no variant, its span says it all + x.align = match alignment(x, cut, frame) { + Alignment::Start => TransitionAlign::StartAtCut, + Alignment::End => TransitionAlign::EndAtCut, + Alignment::Center | Alignment::Custom => TransitionAlign::CenterAtCut, + }; + Ok(()) +} + +/// Where dragging a transition by `delta` in the Timeline puts it, as `(start, duration)`. Its +/// middle (`edge` None) slides it over the cut. An end changes its duration: a Center at Cut +/// transition grows or shrinks on both sides and stays centred on the cut (its alignment says how +/// it changes length), any other keeps its other end where it is. A fade to or from nothing keeps +/// its clip-edge end, and does not slide. Always clamped to where it may be ([`bounds`], over the +/// cut, at least one `frame`); `snap` puts a centred start on a frame. `None` when its clips are gone. +pub fn drag_span(track: &Track, tr: &Transition, edge: Option, delta: Tick, frame: Tick, snap: impl Fn(Tick) -> Tick) -> Option<(Tick, Tick)> { + let (lo, cut, hi) = bounds(track, tr)?; + let frame = frame.max(Tick(1)); + let (s0, e0, d0) = (tr.start, tr.end(), tr.duration); + let one_sided = tr.from.is_none() || tr.to.is_none(); + let unchanged = Some((s0, d0)); + match edge { + None if one_sided => unchanged, + None => { + let (min_s, max_s) = (lo.max(cut - d0), (hi - d0).min(cut)); + if min_s > max_s { unchanged } else { Some(((s0 + delta).clamp(min_s, max_s), d0)) } + } + Some(e) if !one_sided && alignment(tr, cut, frame) == Alignment::Center => { + // dragging an end by n frames moves both ends by n: the duration changes by 2n + let grow = if e == Edge::In { Tick(0) - delta } else { delta }; + let max_d = Tick((cut - lo).min(hi - cut).0.saturating_mul(2)); + let d = Tick(d0.0.saturating_add(grow.0.saturating_mul(2))).clamp(frame, max_d.max(frame)); + Some((snap(cut - d.mul_ratio(1, 2)), d)) + } + Some(Edge::In) if tr.from.is_none() => unchanged, + Some(Edge::In) => { + let max_s = cut.min(e0 - frame).max(lo); + let st = (s0 + delta).clamp(lo, max_s); + Some((st, e0 - st)) + } + Some(Edge::Out) if tr.to.is_none() => unchanged, + Some(Edge::Out) => { + let min_e = cut.max(s0 + frame).min(hi); + let en = (e0 + delta).clamp(min_e, hi); + Some((s0, en - s0)) + } + } +} + +/// Remove transitions (on unlocked tracks). Returns how many went. +pub fn remove(seq: &mut Sequence, ids: &[TransitionId]) -> Result { + if seq.all_tracks().any(|t| t.locked && t.transitions.iter().any(|x| ids.contains(&x.id))) { + return Err(EditError::Locked); + } + let mut n = 0; + for t in seq.all_tracks_mut() { + let before = t.transitions.len(); + t.transitions.retain(|x| !ids.contains(&x.id)); + n += before - t.transitions.len(); + } + if n == 0 { Err(EditError::Nothing) } else { Ok(n) } +} + +#[cfg(test)] +mod tests { + use super::*; + use filmcraft_project::{ClipId, ItemId, Label, Project, SequenceSettings, TrackItem, find_effect}; + use filmcraft_time::FrameRate; + + const R: FrameRate = FrameRate::FPS_24; + + fn f(n: i64) -> Tick { + R.tick_of(n) + } + + fn clip(id: u64, start: i64, dur: i64) -> TrackItem { + TrackItem { + id: ClipId(id), + item: ItemId(1), + name: format!("c{id}"), + label: Label::Iris, + start: f(start), + duration: f(dur), + source_in: Tick::ZERO, + speed: 1.0, + reverse: false, + enabled: true, + link: None, + group: None, + effects: vec![], + markers: vec![], + gain_db: 0.0, + frame_hold: None, + scale_to_frame: false, + essential: None, + multicam: None, + time_interpolation: Default::default(), + hold_filters: false, + field_options: None, + source_channels: Vec::new(), + audio_stream: 0, + graphic: None, + } + } + + fn transition(id: u64, start: i64, dur: i64, from: Option, to: Option) -> Transition { + // any effect: these tests are about where the transition sits + let effect = find_effect("opacity").unwrap().instance(); + Transition { + id: TransitionId(id), + effect, + start: f(start), + duration: f(dur), + from: from.map(ClipId), + to: to.map(ClipId), + align: TransitionAlign::CenterAtCut, + reverse: false, + } + } + + /// V1: c1 0..48, c2 48..96, c3 96..144; a centred 24-frame transition on 48 (id 1), a fade in + /// at 0 (id 2) and a fade out at 144 (id 3). + fn seq() -> Sequence { + let mut p = Project::new("t"); + let s = p.new_sequence("s", SequenceSettings { frame_rate: R, ..Default::default() }, 1, 0, None); + let mut q = p.sequence(s).unwrap().clone(); + let t = &mut q.video_tracks[0]; + t.items = vec![clip(1, 0, 48), clip(2, 48, 48), clip(3, 96, 48)]; + t.transitions = vec![transition(1, 36, 24, Some(1), Some(2)), transition(2, 0, 12, None, Some(1)), transition(3, 132, 12, Some(3), None)]; + q + } + + fn span(q: &Sequence, id: u64) -> (Tick, Tick) { + let x = q.video_tracks[0].transitions.iter().find(|x| x.id.0 == id).unwrap(); + (x.start, x.duration) + } + + fn align(q: &Sequence, id: u64) -> Alignment { + let (t, x) = find(q, TransitionId(id)).unwrap(); + let (_, cut, _) = bounds(t, x).unwrap(); + alignment(x, cut, f(1)) + } + + #[test] + fn alignment_reads_the_span() { + let mut q = seq(); + assert_eq!((align(&q, 1), align(&q, 2), align(&q, 3)), (Alignment::Center, Alignment::Start, Alignment::End)); + set_span(&mut q, TransitionId(1), f(48), f(24), f(1), f(1)).unwrap(); + assert_eq!(align(&q, 1), Alignment::Start); + set_span(&mut q, TransitionId(1), f(24), f(24), f(1), f(1)).unwrap(); + assert_eq!(align(&q, 1), Alignment::End); + set_span(&mut q, TransitionId(1), f(40), f(24), f(1), f(1)).unwrap(); + assert_eq!(align(&q, 1), Alignment::Custom); + // an odd centred duration sits half a frame off the cut and is still Center at Cut + set_span(&mut q, TransitionId(1), f(42), f(13), f(1), f(1)).unwrap(); + assert_eq!(align(&q, 1), Alignment::Center); + } + + #[test] + fn duration_changes_follow_the_alignment() { + let q = seq(); + let snap = |t: Tick| R.snap(t); + // Center at Cut: both ends move + assert_eq!(start_for(&q, TransitionId(1), f(36), Alignment::Center, f(1), snap), Some(f(30))); + // Start at Cut: only the end moves; End at Cut: only the beginning + assert_eq!(start_for(&q, TransitionId(1), f(36), Alignment::Start, f(1), snap), Some(f(48))); + assert_eq!(start_for(&q, TransitionId(1), f(36), Alignment::End, f(1), snap), Some(f(12))); + // a one-sided transition keeps its edge whatever is asked + assert_eq!(start_for(&q, TransitionId(2), f(20), Alignment::Center, f(1), snap), Some(f(0))); + assert_eq!(start_for(&q, TransitionId(3), f(20), Alignment::Center, f(1), snap), Some(f(124))); + } + + #[test] + fn custom_start_resizes_around_its_middle() { + let mut q = seq(); + set_span(&mut q, TransitionId(1), f(40), f(20), f(1), f(1)).unwrap(); + assert_eq!(start_for(&q, TransitionId(1), f(10), Alignment::Custom, f(1), |t| R.snap(t)), Some(f(45))); + } + + #[test] + fn spans_stay_on_the_cut_and_inside_the_clips() { + let mut q = seq(); + let before = q.clone(); + for (id, start, dur) in [ + (1, -1, 24), // before the outgoing clip + (1, 40, 60), // past the incoming clip's end + (1, 10, 20), // entirely before the cut + (1, 50, 10), // entirely after it + (1, 40, 0), // empty + (2, 1, 12), // a fade in must start at its clip's start + (3, 120, 12), // a fade out must end at its clip's end + (9, 0, 12), // no such transition + ] { + assert!(set_span(&mut q, TransitionId(id), f(start), f(dur), f(1), f(1)).is_err(), "{id} {start} {dur}"); + assert_eq!(q, before, "refused changes leave the sequence as it was"); + } + // the whole of both clips is allowed + set_span(&mut q, TransitionId(1), f(0), f(96), f(1), f(1)).unwrap(); + assert_eq!(span(&q, 1), (f(0), f(96))); + // Tick::MAX never overflows + assert!(set_span(&mut q, TransitionId(1), Tick(i64::MAX - 1), Tick(i64::MAX), f(1), f(1)).is_err()); + } + + fn drag(q: &Sequence, id: u64, edge: Option, frames: i64) -> (i64, i64) { + let (t, x) = find(q, TransitionId(id)).unwrap(); + let (st, d) = drag_span(t, x, edge, f(frames), f(1), |t| R.snap(t)).unwrap(); + (R.frame_at(st), R.frame_at(d)) + } + + #[test] + fn dragging_a_transition() { + let mut q = seq(); + // centred (36..60 over the cut at 48): an end grows / shrinks it on both sides + assert_eq!(drag(&q, 1, Some(Edge::In), -4), (32, 32)); + assert_eq!(drag(&q, 1, Some(Edge::Out), -10), (46, 4)); + assert_eq!(drag(&q, 1, Some(Edge::Out), -100), (47, 1), "at least a frame (centred: it starts on the frame before the cut)"); + assert_eq!(drag(&q, 1, Some(Edge::Out), 100), (0, 96), "at most both clips"); + // the middle slides it, staying over the cut and inside the clips + assert_eq!(drag(&q, 1, None, 5), (41, 24)); + assert_eq!(drag(&q, 1, None, 100), (48, 24)); + assert_eq!(drag(&q, 1, None, -100), (24, 24)); + // not centred: the other end stays + set_span(&mut q, TransitionId(1), f(40), f(20), f(1), f(1)).unwrap(); + assert_eq!(drag(&q, 1, Some(Edge::In), -4), (36, 24)); + assert_eq!(drag(&q, 1, Some(Edge::In), 100), (48, 12), "the start stops at the cut"); + assert_eq!(drag(&q, 1, Some(Edge::Out), 6), (40, 26)); + assert_eq!(drag(&q, 1, Some(Edge::Out), -100), (40, 8), "the end stops at the cut"); + // a fade in keeps its start, a fade out its end; neither slides + assert_eq!(drag(&q, 2, Some(Edge::In), 3), (0, 12)); + assert_eq!(drag(&q, 2, Some(Edge::Out), 3), (0, 15)); + assert_eq!(drag(&q, 2, None, 3), (0, 12)); + assert_eq!(drag(&q, 3, Some(Edge::Out), 3), (132, 12)); + assert_eq!(drag(&q, 3, Some(Edge::In), -3), (129, 15)); + // whatever comes out is a span set_span accepts + for (id, edge, n) in [(1, Some(Edge::In), -70), (1, None, 33), (2, Some(Edge::Out), 99), (3, Some(Edge::In), -99)] { + let (t, x) = find(&q, TransitionId(id)).unwrap(); + let (st, d) = drag_span(t, x, edge, f(n), f(1), |t| R.snap(t)).unwrap(); + let mut c = q.clone(); + set_span(&mut c, TransitionId(id), st, d, f(1), f(1)).unwrap(); + } + } + + #[test] + fn locked_tracks_refuse() { + let mut q = seq(); + q.video_tracks[0].locked = true; + assert_eq!(set_span(&mut q, TransitionId(1), f(40), f(20), f(1), f(1)), Err(EditError::Locked)); + assert_eq!(remove(&mut q, &[TransitionId(1)]), Err(EditError::Locked)); + } + + #[test] + fn remove_takes_only_the_named_transitions() { + let mut q = seq(); + assert_eq!(remove(&mut q, &[TransitionId(1), TransitionId(3)]), Ok(2)); + let left: Vec = q.video_tracks[0].transitions.iter().map(|x| x.id.0).collect(); + assert_eq!(left, vec![2]); + assert_eq!(q.video_tracks[0].items.len(), 3, "the clips stay"); + assert_eq!(remove(&mut q, &[TransitionId(1)]), Err(EditError::Nothing)); + } +} diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index b5e4c847..cfb20310 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -81,6 +81,15 @@ pub(crate) fn has_selection(s: &Session) -> std::result::Result<(), String> { } /// Clips or captions selected (Clear / Ripple Delete work on either). fn has_any_selection(s: &Session) -> std::result::Result<(), String> { + has_seq(s)?; + if s.state.selection.is_empty() && s.state.caption_selection.is_empty() && s.state.transition_selection.is_empty() { + Err("nothing selected".into()) + } else { + Ok(()) + } +} +/// Clips or captions selected (Ripple Delete: a transition has no length of its own to close). +fn has_clip_or_caption_selection(s: &Session) -> std::result::Result<(), String> { has_seq(s)?; if s.state.selection.is_empty() && s.state.caption_selection.is_empty() { Err("nothing selected".into()) } else { Ok(()) } } @@ -1187,6 +1196,11 @@ fn build() -> Vec { cmd!("edit.pasteInsert", "Paste Insert", ["Edit"], Some("Cmd+Shift+V"), "{}", has_clipboard, |s, _| paste(s, true)), cmd!("edit.clear", "Clear", ["Edit"], Some("Backspace"), r#"{"clips":[id]?}"#, has_any_selection, |s, p| { if p.get("clips").is_none() && p.get("clip").is_none() && s.state.selection.is_empty() { + // a selected transition (clicked in the Timeline) goes, as a clip would + if !s.state.transition_selection.is_empty() { + let ids = s.state.transition_selection.clone(); + return remove_transitions(s, &ids, "Clear"); + } let caps = s.state.caption_selection.clone(); return crate::captions::delete(s, &caps, false); } @@ -1198,7 +1212,7 @@ fn build() -> Vec { })?; Ok(Value::Null) }), - cmd!("edit.rippleDelete", "Ripple Delete", ["Edit"], Some("Shift+Delete"), r#"{"clips":[id]?}"#, has_any_selection, |s, p| { + cmd!("edit.rippleDelete", "Ripple Delete", ["Edit"], Some("Shift+Delete"), r#"{"clips":[id]?}"#, has_clip_or_caption_selection, |s, p| { if p.get("clips").is_none() && p.get("clip").is_none() && s.state.selection.is_empty() { let caps = s.state.caption_selection.clone(); return crate::captions::delete(s, &caps, true); @@ -1536,10 +1550,20 @@ fn build() -> Vec { "Edit Transition Settings", [], None, - r#"{"transition":id,"params":{param:value}?,"reverse":bool?,"reset":bool?}"#, + r#"{"transition":id,"params":{param:value}?,"reverse":bool?,"reset":bool?,"duration":ticks?|"frames":n?,"align":"center"|"start"|"end"?|"start":ticks?,"merge":bool?,"begin":bool?}"#, has_seq, set_transition ), + cmd!("sequence.removeTransition", "Clear Transition", [], None, r#"{"transitions":[id]?}"#, has_seq, |s, p| { + let ids: Vec = match p.get("transitions").or_else(|| p.get("transition")) { + Some(v) => transition_ids_p(v).ok_or_else(|| bad("sequence.removeTransition", "`transitions` is a list of transition ids"))?, + None => s.state.transition_selection.clone(), + }; + if ids.is_empty() { + return Err(bad("sequence.removeTransition", "no transition given or selected")); + } + remove_transitions(s, &ids, "Clear Transition") + }), cmd!("sequence.closeGap", "Close Gap", ["Sequence"], None, r#"{"track":"V1"|id,"time":ticks}"#, has_seq, |s, p| { let tr = track_p(s, p, "track", "sequence.closeGap")?.ok_or_else(|| bad("sequence.closeGap", "need `track`"))?; let t = time_p(s, p, "").unwrap_or(s.playhead()); @@ -2019,13 +2043,18 @@ fn build() -> Vec { Ok(json!({"clips": ids.iter().map(|c| c.0).collect::>()})) } ), - cmd!("timeline.select", "Select Clips", [], None, r#"{"clips":[id],"add":bool,"toggle":bool}"#, has_seq, |s, p| { + cmd!("timeline.select", "Select Clips", [], None, r#"{"clips":[id],"transitions":[id]?,"add":bool,"toggle":bool}"#, has_seq, |s, p| { + if let Some(ids) = p.get("transitions") { + return select_transitions(s, ids, bool_p(p, "add").unwrap_or(false), bool_p(p, "toggle").unwrap_or(false)); + } let clips: Vec = p.get("clips").and_then(Value::as_array).map(|a| a.iter().filter_map(|v| v.as_u64().map(ClipId)).collect()).unwrap_or_default(); let clips = with_links(s, &clips); // selecting clips leaves trim mode (Premiere: clip and edit point selections are exclusive) s.state.edit_points.clear(); s.state.trim_shift = Default::default(); + // and a clip selection replaces a transition one + s.state.transition_selection.clear(); if bool_p(p, "toggle").unwrap_or(false) { for c in clips { if let Some(i) = s.state.selection.iter().position(|x| *x == c) { @@ -3151,13 +3180,109 @@ fn set_transition_params(e: &mut filmcraft_project::EffectInstance, params: &Val Ok(()) } -/// `sequence.setTransition`: edit an applied transition's settings (Effect Controls). +/// A JSON list of transition ids (or one id). +fn transition_ids_p(v: &Value) -> Option> { + match v { + Value::Array(a) => a.iter().map(|x| x.as_u64().map(TransitionId)).collect(), + x => x.as_u64().map(|i| vec![TransitionId(i)]), + } +} + +/// `timeline.select {"transitions": […]}`: select transitions (clicking one in the Timeline). It +/// replaces the clip, edit point and caption selections, as a clip selection replaces it. +fn select_transitions(s: &mut Session, ids: &Value, add: bool, toggle: bool) -> Result { + const CMD: &str = "timeline.select"; + let ids = transition_ids_p(ids).ok_or_else(|| bad(CMD, "`transitions` is a list of transition ids"))?; + let q = s.active_sequence().ok_or(EngineError::NoSequence)?; + if let Some(missing) = ids.iter().find(|id| !q.all_tracks().any(|t| t.transitions.iter().any(|x| x.id == **id))) { + return Err(bad(CMD, format!("no transition {}", missing.0))); + } + if toggle { + for id in ids { + match s.state.transition_selection.iter().position(|x| *x == id) { + Some(i) => { + s.state.transition_selection.remove(i); + } + None => s.state.transition_selection.push(id), + } + } + } else if add { + for id in ids { + if !s.state.transition_selection.contains(&id) { + s.state.transition_selection.push(id); + } + } + } else { + s.state.transition_selection = ids; + } + s.state.selection.clear(); + s.state.caption_selection.clear(); + s.state.edit_points.clear(); + s.state.trim_shift = Default::default(); + Ok(json!({"selection": [], "transitionSelection": s.state.transition_selection.iter().map(|t| t.0).collect::>()})) +} + +/// Remove transitions in one undo step and drop them from the selection. +fn remove_transitions(s: &mut Session, ids: &[TransitionId], label: &str) -> Result { + let ids = ids.to_vec(); + let n = s.edit_sequence(label, |q, _, st| { + let n = edit::transitions::remove(q, &ids)?; + st.transition_selection.retain(|x| !ids.contains(x)); + Ok(n) + })?; + Ok(json!({"removed": n})) +} + +/// `sequence.setTransition`: edit an applied transition (Effect Controls, the Timeline): its +/// settings, Reverse, and where it sits. `duration` / `frames` resize it the way its alignment +/// says (Center at Cut and Custom Start: both ends; Start at Cut: the end; End at Cut: the +/// beginning), `align` re-aligns it on its cut, `start` moves it (dragging it over the cut; with +/// `duration`, dragging one edge). One undo step; with `merge`, a drag's steps share one. fn set_transition(s: &mut Session, p: &Value) -> Result { const CMD: &str = "sequence.setTransition"; let id = p.get("transition").and_then(Value::as_u64).ok_or_else(|| bad(CMD, "need `transition`"))?; + let tid = TransitionId(id); let q = s.active_sequence().ok_or(EngineError::NoSequence)?; - let found = q.video_tracks.iter().chain(q.audio_tracks.iter()).find_map(|tr| tr.transitions.iter().find(|x| x.id.0 == id)).cloned(); - let mut cur = found.ok_or_else(|| bad(CMD, format!("no transition {id}")))?; + let rate = q.settings.frame_rate.sane(); + let frame = rate.frame_duration(); + let found = q.all_tracks().find_map(|tr| tr.transitions.iter().find(|x| x.id == tid).map(|x| (tr, x))); + let (track, cur) = found.ok_or_else(|| bad(CMD, format!("no transition {id}")))?; + let mut cur = cur.clone(); + let (lo, cut, hi) = edit::transitions::bounds(track, &cur).ok_or_else(|| bad(CMD, "the transition's clips are gone"))?; + // times outside the clips it joins are refused before any frame arithmetic (hostile input) + let room = Tick(hi.0.saturating_sub(lo.0)); + let duration = match (p.get("frames").and_then(Value::as_i64), p.get("duration").and_then(Value::as_i64)) { + (Some(_), Some(_)) => return Err(bad(CMD, "give `duration` or `frames`, not both")), + (Some(f), None) if (1..=10_000_000).contains(&f) && rate.tick_of(f) <= room => Some(rate.tick_of(f)), + (None, Some(d)) if d > 0 && d <= room.0 => Some(rate.snap_nearest(Tick(d)).max(frame)), + (Some(_), None) | (None, Some(_)) => return Err(bad(CMD, "a transition lasts at least one frame and can't extend past the clips it joins")), + (None, None) => None, + }; + let align = match str_p(p, "align") { + Some(a) => Some(edit::transitions::Alignment::parse(a).ok_or_else(|| bad(CMD, format!("`align` is center, start or end, not `{a}`")))?), + None => None, + }; + let start = match p.get("start").and_then(Value::as_i64) { + Some(t) if (lo.0..=hi.0).contains(&t) => Some(rate.snap_nearest(Tick(t))), + Some(_) => return Err(bad(CMD, "a transition can't extend past the clips it joins")), + None => None, + }; + if start.is_some() && align.is_some() { + return Err(bad(CMD, "give `start` or `align`, not both")); + } + let span = if duration.is_some() || align.is_some() || start.is_some() { + let d = duration.unwrap_or(cur.duration); + let st = match start { + Some(st) => st, + None => { + let al = align.unwrap_or_else(|| edit::transitions::alignment(&cur, cut, frame)); + edit::transitions::start_for(q, tid, d, al, frame, |t| rate.snap(t)).ok_or_else(|| bad(CMD, format!("no transition {id}")))? + } + }; + Some((st, d)) + } else { + None + }; if p.get("reset").and_then(Value::as_bool).unwrap_or(false) && let Some(def) = cur.effect.def() { @@ -3170,16 +3295,32 @@ fn set_transition(s: &mut Session, p: &Value) -> Result { cur.reverse = r; } let name = cur.effect.def().map_or("Transition", |d| d.name).to_string(); - s.edit_sequence(&format!("Edit {name}"), |q, _, _| { + if bool_p(p, "begin").unwrap_or(false) { + s.history.merge_key = None; + } + let merge = bool_p(p, "merge").unwrap_or(false).then(|| format!("setTransition:{id}")); + let after = s.edit_sequence_as(&format!("Edit {name}"), merge.as_deref(), |q, _, _| { + if let Some((st, d)) = span { + edit::transitions::set_span(q, tid, st, d, frame, frame)?; + } + let mut after = None; for tr in q.video_tracks.iter_mut().chain(q.audio_tracks.iter_mut()) { - if let Some(x) = tr.transitions.iter_mut().find(|x| x.id.0 == id) { + if let Some(x) = tr.transitions.iter_mut().find(|x| x.id == tid) { x.effect = cur.effect.clone(); x.reverse = cur.reverse; + after = Some((x.start, x.duration)); } } - Ok(()) + Ok(after) })?; - Ok(json!({"transition": id, "effect": cur.effect.effect, "reverse": cur.reverse})) + let (st, d) = after.unwrap_or((cur.start, cur.duration)); + let al = s + .active_sequence() + .and_then(|q| { + q.all_tracks().find_map(|tr| tr.transitions.iter().find(|x| x.id == tid).and_then(|x| edit::transitions::bounds(tr, x).map(|b| (x.clone(), b.1)))) + }) + .map(|(x, cut)| edit::transitions::alignment(&x, cut, frame).id()); + Ok(json!({"transition": id, "effect": cur.effect.effect, "reverse": cur.reverse, "start": st.0, "duration": d.0, "align": al})) } fn apply_transition(s: &mut Session, p: &Value, kind: TrackKind) -> Result { @@ -3285,7 +3426,7 @@ pub fn inspect_sequence(s: &Session, id: ItemId, q: &filmcraft_project::Sequence "enabled": i.enabled, "link": i.link, "label": i.label.name(), "effects": i.effects.iter().map(|e| json!({"effect": e.effect, "enabled": e.enabled, "masks": e.masks.len(), "params": e.params.iter().map(|(k, p)| (k.clone(), json!({"value": format!("{:?}", p.value), "keyframes": p.keyframes.len()}))).collect::>()})).collect::>(), })).collect::>(), - "transitions": t.transitions.iter().map(|x| json!({"id": x.id.0, "effect": x.effect.effect, "start": x.start.0, "duration": x.duration.0, "from": x.from.map(|c| c.0), "to": x.to.map(|c| c.0), "reverse": x.reverse, "params": x.effect.params.iter().map(|(k, v)| (k.clone(), serde_json::to_value(&v.value).unwrap_or_default())).collect::>()})).collect::>(), + "transitions": t.transitions.iter().map(|x| json!({"id": x.id.0, "effect": x.effect.effect, "start": x.start.0, "duration": x.duration.0, "from": x.from.map(|c| c.0), "to": x.to.map(|c| c.0), "reverse": x.reverse, "align": filmcraft_edit::transitions::bounds(t, x).map(|b| filmcraft_edit::transitions::alignment(x, b.1, rate.frame_duration()).id()), "params": x.effect.params.iter().map(|(k, v)| (k.clone(), serde_json::to_value(&v.value).unwrap_or_default())).collect::>()})).collect::>(), }) }; json!({ @@ -3302,6 +3443,7 @@ pub fn inspect_sequence(s: &Session, id: ItemId, q: &filmcraft_project::Sequence "video": q.video_tracks.iter().map(tr).collect::>(), "audio": q.audio_tracks.iter().map(tr).collect::>(), "selection": s.state.selection.iter().map(|c| c.0).collect::>(), + "transitionSelection": s.state.transition_selection.iter().map(|t| t.0).collect::>(), }) } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 2578420b..26ee2bac 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -260,6 +260,10 @@ pub struct EditorState { /// Selected captions (caption tracks / Captions panel). #[serde(default)] pub caption_selection: Vec, + /// Selected transitions (clicked in the Timeline): Effect Controls shows one, Delete removes + /// them. Selecting clips clears it and selecting transitions clears the clip selection. + #[serde(default)] + pub transition_selection: Vec, /// Selected layers (indices among the graphic layers, 0 = back) of the selected graphic clip. #[serde(default)] pub graphic_layers: Vec, diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 01d5b813..380519cf 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -641,3 +641,111 @@ fn dragging_an_effect_parameter_is_one_undo_step() { s.undo(); assert_eq!(opacity(&s), start, "and then the whole first one"); } + +/// The demo's first video transition between two clips: (id, start, duration, cut). +fn demo_crossing(s: &Session) -> (u64, Tick, Tick, Tick) { + let q = s.active_sequence().unwrap(); + q.video_tracks + .iter() + .find_map(|t| { + t.transitions.iter().find(|x| x.from.is_some() && x.to.is_some()).map(|x| (x.id.0, x.start, x.duration, t.item(x.to.unwrap()).unwrap().start)) + }) + .expect("the demo has a transition between two clips") +} + +fn transition_ids(s: &Session) -> Vec { + s.active_sequence().unwrap().all_tracks().flat_map(|t| t.transitions.iter().map(|x| x.id.0)).collect() +} + +/// #430: a transition can be selected (it replaces the clip selection and back), Delete removes +/// it and leaves its clips, undo brings it back; Ripple Delete is not for a transition alone. +#[test] +fn transitions_select_and_delete() { + let mut s = demo(); + let (tid, ..) = demo_crossing(&s); + let clips = s.active_sequence().unwrap().all_tracks().map(|t| t.items.len()).sum::(); + let some_clip = s.active_sequence().unwrap().video_tracks[0].items[0].id.0; + s.execute("timeline.select", json!({"clips": [some_clip]})).unwrap(); + let r = s.execute("timeline.select", json!({"transitions": [tid]})).unwrap(); + assert_eq!(r["transitionSelection"], json!([tid])); + assert!(s.state.selection.is_empty(), "selecting a transition replaces the clip selection"); + assert!(s.execute("timeline.select", json!({"transitions": [987_654]})).is_err(), "unknown ids are refused"); + assert!(s.execute("edit.rippleDelete", json!({})).is_err(), "Ripple Delete needs clips"); + let before = (*s.project).clone(); + s.execute("edit.clear", json!({})).unwrap(); + assert!(!transition_ids(&s).contains(&tid), "Delete removes the selected transition"); + assert_eq!(s.active_sequence().unwrap().all_tracks().map(|t| t.items.len()).sum::(), clips, "and leaves the clips"); + assert!(s.state.transition_selection.is_empty()); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(*s.project, before, "one undo step"); + // selecting clips clears a transition selection + s.execute("timeline.select", json!({"transitions": [tid]})).unwrap(); + s.execute("timeline.select", json!({"clips": [some_clip]})).unwrap(); + assert!(s.state.transition_selection.is_empty()); + // by id, without a selection + s.execute("sequence.removeTransition", json!({"transitions": [tid]})).unwrap(); + assert!(!transition_ids(&s).contains(&tid)); + assert!(s.execute("sequence.removeTransition", json!({"transitions": [tid]})).is_err(), "nothing left to remove"); + assert!(s.execute("sequence.removeTransition", json!({})).is_err(), "nothing given or selected"); +} + +/// #224 / Effect Controls: Duration follows the alignment, Alignment re-aligns on the cut, Start +/// moves it (Custom Start), all within the clips it joins; a drag is one undo step. +#[test] +fn transition_duration_alignment_and_position() { + let mut s = demo(); + let rate = s.sequence_rate(); + let f = |n: i64| rate.tick_of(n); + let (tid, _, _, cut) = demo_crossing(&s); + let set = |s: &mut Session, p: Value| { + let mut p = p; + p["transition"] = json!(tid); + s.execute("sequence.setTransition", p) + }; + let r = set(&mut s, json!({"align": "start"})).unwrap(); + assert_eq!((r["start"].as_i64(), r["align"].as_str()), (Some(cut.0), Some("start"))); + // Start at Cut: only the end moves + let r = set(&mut s, json!({"frames": 10})).unwrap(); + assert_eq!((r["start"].as_i64(), r["duration"].as_i64()), (Some(cut.0), Some(f(10).0))); + // End at Cut: only the beginning moves + let r = set(&mut s, json!({"align": "end", "frames": 8})).unwrap(); + assert_eq!((r["start"].as_i64(), r["duration"].as_i64(), r["align"].as_str()), (Some((cut - f(8)).0), Some(f(8).0), Some("end"))); + // Center at Cut: both ends + let r = set(&mut s, json!({"align": "center", "frames": 12})).unwrap(); + assert_eq!((r["start"].as_i64(), r["align"].as_str()), (Some((cut - f(6)).0), Some("center"))); + // dragging it over the cut: Custom Start; then a new duration keeps its middle + let r = set(&mut s, json!({"start": (cut - f(3)).0})).unwrap(); + assert_eq!(r["align"], json!("custom")); + let r = set(&mut s, json!({"frames": 6})).unwrap(); + assert_eq!((r["start"].as_i64(), r["duration"].as_i64()), (Some(cut.0), Some(f(6).0)), "centre at cut + 3 frames, 6 frames long"); + // dragging one edge: start and duration together + let r = set(&mut s, json!({"start": (cut - f(4)).0, "duration": f(10).0})).unwrap(); + assert_eq!((r["start"].as_i64(), r["duration"].as_i64()), (Some((cut - f(4)).0), Some(f(10).0))); + // refused, and nothing changes + let before = (*s.project).clone(); + let history = s.history.undo.len(); + for p in [ + json!({"frames": 0}), + json!({"frames": 100_000_000}), + json!({"duration": -5}), + json!({"duration": i64::MAX}), + json!({"frames": 10, "duration": f(10).0}), + json!({"align": "middle"}), + json!({"align": "start", "start": cut.0}), + json!({"start": i64::MIN}), + json!({"start": (cut + f(1)).0, "duration": f(1).0}), + ] { + assert!(set(&mut s, p.clone()).is_err(), "{p}"); + assert_eq!(*s.project, before, "{p}"); + assert_eq!(s.history.undo.len(), history, "{p}"); + } + assert!(s.execute("sequence.setTransition", json!({"transition": 987_654, "frames": 5})).is_err()); + // a drag (merge) is one undo step + let history = s.history.undo.len(); + for (i, n) in [11, 12, 13, 14].into_iter().enumerate() { + set(&mut s, json!({"frames": n, "merge": true, "begin": i == 0})).unwrap(); + } + assert_eq!(s.history.undo.len(), history + 1); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(*s.project, before); +} From efd62f7bdafc77feda7e161f892bb4ce66c2e724 Mon Sep 17 00:00:00 2001 From: swillunderscore <201272413+swillunderscore@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:04:21 -0500 Subject: [PATCH 20/35] #430, #224: select a transition, edit it in Effect Controls, Delete it, drag its ends or middle Timeline: clicking a transition selects it (outlined; Shift adds / removes) and opens it in Effect Controls; Delete removes it. Its ends are edge zones: dragging one changes the duration (a Center at Cut transition grows on both sides and stays centred, any other keeps its other end), dragging its middle slides it over the cut (Custom Start); each drag is previewed and committed as one sequence.setTransition. Double-clicking it opens Set Transition Duration. Effect Controls for a selected transition: Duration (timecode), Alignment (Center at Cut, Start at Cut, End at Cut; Custom Start shown when dragged off-centre), Reverse, its own settings (numbers, angles, choices, colours, points, checkboxes), Reset. Dragging a value is one undo step. Tests: transitions_ui.rs drives it all over the control channel (select, Effect Controls, the Delete key, undo, edge and middle drags, the Alignment menu, the dialog, Push's settings), with FILMCRAFT_UI_SNAPSHOT_DIR snapshots. Spanish for the two new strings. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/i18n/es.tsv | 2 + crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/panels/dialogs.rs | 6 + crates/ui-egui/src/panels/effect_controls.rs | 5 + crates/ui-egui/src/panels/mod.rs | 1 + crates/ui-egui/src/panels/timeline.rs | 77 ++++- crates/ui-egui/src/panels/timeline_hit.rs | 29 +- .../ui-egui/src/panels/transition_controls.rs | 305 ++++++++++++++++++ crates/ui-egui/src/state.rs | 13 + crates/ui-egui/tests/transitions_ui.rs | 275 ++++++++++++++++ 10 files changed, 705 insertions(+), 10 deletions(-) create mode 100644 crates/ui-egui/src/panels/transition_controls.rs create mode 100644 crates/ui-egui/tests/transitions_ui.rs diff --git a/crates/ui-egui/src/i18n/es.tsv b/crates/ui-egui/src/i18n/es.tsv index a39dc82e..a5c296e6 100644 --- a/crates/ui-egui/src/i18n/es.tsv +++ b/crates/ui-egui/src/i18n/es.tsv @@ -3480,3 +3480,5 @@ New Sequence Nueva secuencia Tracks Pistas tracks pistas + Clear Transition Borrar transición + Set Transition Duration Establecer duración de la transición diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 83d18261..e88c57c2 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -162,6 +162,8 @@ pub enum Dialog { AddTracks, /// Sequence ▸ Sequence Settings…. SequenceSettings, + /// Set Transition Duration (double-click a transition in the Timeline). + TransitionDuration, } #[derive(Default)] diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index e82e2c3c..eed8fd28 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -67,6 +67,12 @@ pub fn show(app: &mut FilmcraftApp, ctx: &egui::Context) { } return; } + Dialog::TransitionDuration => { + if !crate::panels::transition_controls::duration_dialog(app, ctx) && app.dialog == Some(d) { + app.dialog = None; + } + return; + } Dialog::SequenceSettings => { if !crate::panels::sequence_settings::show(app, ctx) && app.dialog == Some(d) { app.dialog = None; diff --git a/crates/ui-egui/src/panels/effect_controls.rs b/crates/ui-egui/src/panels/effect_controls.rs index e3ae049c..905089f1 100644 --- a/crates/ui-egui/src/panels/effect_controls.rs +++ b/crates/ui-egui/src/panels/effect_controls.rs @@ -46,6 +46,11 @@ fn selected_clip(app: &FilmcraftApp) -> Option<(ClipId, TrackItem, filmcraft_pro pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { let t = app.tokens; let Some((clip, it, kind)) = selected_clip(app) else { + // a transition clicked in the Timeline (#430) + if let Some(id) = crate::panels::transition_controls::selected(app) { + crate::panels::transition_controls::effect_controls(app, ui, rect, id); + return; + } crate::dock::placeholder(ui, rect, &t, tl!("(no clip selected)")); return; }; diff --git a/crates/ui-egui/src/panels/mod.rs b/crates/ui-egui/src/panels/mod.rs index e19bbce3..5591f357 100644 --- a/crates/ui-egui/src/panels/mod.rs +++ b/crates/ui-egui/src/panels/mod.rs @@ -47,6 +47,7 @@ pub mod timeline_automation; pub mod timeline_captions; pub mod timeline_hit; pub mod tools; +pub mod transition_controls; pub mod trim_monitor; pub mod voiceover; pub mod workspaces; diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 811a75ee..a826e9a4 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -128,6 +128,16 @@ pub enum Drag { /// As in [`Drag::Trim`]. from: Tick, }, + /// A transition dragged by an end (`edge`: its duration) or its middle (slides it over the + /// cut); `start` / `duration` are where it is shown now, committed on release (#224). + Transition { + id: filmcraft_project::TransitionId, + edge: Option, + /// As in [`Drag::Trim`]. + from: Tick, + start: Tick, + duration: Tick, + }, Pan { last: Pos2, }, @@ -411,13 +421,22 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { } // selected edit points (trim mode): red brackets for ripple, yellow for roll / regular trim super::trim_monitor::paint_edit_points(app, &p, &seq, tr, r, &layout); - // transitions + // transitions (one being dragged where the drag puts it; the selected ones outlined) for trn in &tr.transitions { - let x0 = layout.x_of(trn.start); - let x1 = layout.x_of(trn.end()); + let (start, end) = match &app.tl.drag { + Some(Drag::Transition { id, start, duration, .. }) if *id == trn.id => (*start, *start + *duration), + _ => (trn.start, trn.end()), + }; + let x0 = layout.x_of(start); + let x1 = layout.x_of(end); let tr_rect = Rect::from_min_max(pos2(x0, r.rect.min.y + 17.0), pos2(x1, r.rect.max.y - 1.0)); - draw_transition(&p, tr_rect, trn, &t); + let selected = app.session.state.transition_selection.contains(&trn.id); + draw_transition(&p, tr_rect, trn, selected, &t); app.auto.add(&format!("timeline.transition.{}", trn.id.0), tr_rect, &trn.effect.effect); + // its ends, for dragging the duration (the same zones as the hit test) + let zone = super::timeline_hit::EDGE_PX.min(tr_rect.width() / 3.0); + app.auto.add(&format!("timeline.transition.{}.in", trn.id.0), Rect::from_min_max(tr_rect.min, pos2(x0 + zone, tr_rect.max.y)), "start"); + app.auto.add(&format!("timeline.transition.{}.out", trn.id.0), Rect::from_min_max(pos2(x1 - zone, tr_rect.min.y), tr_rect.max), "end"); } } @@ -896,7 +915,7 @@ fn paint_hatch(p: &egui::Painter, r: Rect) { } } -fn draw_transition(p: &egui::Painter, r: Rect, trn: &filmcraft_project::Transition, _t: &Tokens) { +fn draw_transition(p: &egui::Painter, r: Rect, trn: &filmcraft_project::Transition, selected: bool, t: &Tokens) { let cp = p.with_clip_rect(r.intersect(p.clip_rect())); cp.rect_filled(r, 0.0, Color32::from_black_alpha(90)); paint_hatch(p, r); @@ -918,6 +937,9 @@ fn draw_transition(p: &egui::Painter, r: Rect, trn: &filmcraft_project::Transiti cp.add(egui::Shape::line(b, Stroke::new(1.0, Color32::WHITE))); } cp.rect_stroke(r, 0.0, Stroke::new(1.0, Color32::from_rgb(0xeb, 0xeb, 0xeb)), StrokeKind::Inside); + if selected { + cp.rect_stroke(r, 0.0, Stroke::new(2.0, t.accent), StrokeKind::Inside); + } if r.width() > 34.0 && !audio { let name = trn.effect.def().map(|d| d.name).unwrap_or(&trn.effect.effect); let tr = Rect::from_min_size(r.min + vec2(2.0, 2.0), vec2((r.width() - 4.0).min(80.0), 13.0)); @@ -1460,7 +1482,7 @@ pub fn hit_json(app: &FilmcraftApp, pos: Pos2, mods: egui::Modifiers) -> Value { Hit::Clip { track, clip, edge } => { json!({"hit": "clip", "track": track.0, "clip": clip.0, "edge": edge.map(|e| format!("{e:?}")), "kind": kind, "time": t.0}) } - Hit::Transition { track, id } => json!({"hit": "transition", "track": track.0, "transition": id.0}), + Hit::Transition { track, id, edge } => json!({"hit": "transition", "track": track.0, "transition": id.0, "edge": edge.map(|e| format!("{e:?}"))}), Hit::Empty { track } => json!({"hit": "empty", "track": track.0, "time": t.0}), Hit::None => json!({"hit": "none"}), } @@ -1730,6 +1752,8 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & let g = grab_at(seq, layout, p, tool, mods, roll_ripple); let cur = match (tool, g) { (_, Grab::Edge { .. }) => CursorIcon::ResizeColumn, + (Tool::Selection, Grab::Other(Hit::Transition { edge: Some(_), .. })) => CursorIcon::ResizeColumn, + (Tool::Selection, Grab::Other(Hit::Transition { edge: None, .. })) => CursorIcon::Grab, (Tool::Razor, Grab::Other(Hit::Clip { .. })) => CursorIcon::Crosshair, (Tool::Slip | Tool::Slide, Grab::Other(Hit::Clip { .. })) => CursorIcon::ResizeHorizontal, (Tool::Hand, _) => CursorIcon::Grab, @@ -1834,6 +1858,7 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & } else if !sel.contains(&clip) { if mods.alt { app.session.state.selection = vec![clip]; + app.session.state.transition_selection.clear(); } else { let _ = app.session.execute("timeline.select", json!({"clips": [clip.0]})); } @@ -1845,6 +1870,21 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & None } } + // click a transition to select it (Shift adds / removes); drag an end to change its + // duration or its middle to slide it over the cut (#430, #224) + (Tool::Selection, Grab::Other(Hit::Transition { id, edge, .. })) => { + if !(app.session.state.transition_selection.contains(&id) && !mods.shift && resp.drag_started()) { + let r = app.session.execute("timeline.select", json!({"transitions": [id.0], "toggle": mods.shift})); + if let Err(e) = r { + app.ui.status = e.to_string(); + } + } + let span = seq.all_tracks().find_map(|tr| tr.transitions.iter().find(|x| x.id == id)).map(|x| (x.start, x.duration)); + match span { + Some((start, duration)) if resp.drag_started() => Some(Drag::Transition { id, edge, from: t, start, duration }), + _ => None, + } + } (_, Grab::Other(Hit::Transition { .. })) => None, (_, Grab::Other(Hit::Empty { .. })) => { if resp.drag_started() { @@ -1852,6 +1892,7 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & } else { app.session.state.selection.clear(); app.session.state.edit_points.clear(); + app.session.state.transition_selection.clear(); None } } @@ -1939,6 +1980,17 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & let origin = ctx.input(|i| i.pointer.press_origin()).map(|o| layout.tick_at(o.x)).unwrap_or(t_here); Some(Drag::Slide { clip, delta: rate.snap_nearest(t_here - origin) }) } + Drag::Transition { id, edge, from, start, duration } => { + let delta = rate.snap_nearest(t_here - from); + let span = seq.all_tracks().find_map(|tr| { + tr.transitions + .iter() + .find(|x| x.id == id) + .and_then(|x| filmcraft_edit::transitions::drag_span(tr, x, edge, delta, rate.frame_duration(), |t| rate.snap(t))) + }); + let (start, duration) = span.unwrap_or((start, duration)); + Some(Drag::Transition { id, edge, from, start, duration }) + } Drag::Marquee { start } => { let r = Rect::from_two_pos(start, p); ui.painter().rect_filled(r, 0.0, Color32::from_white_alpha(18)); @@ -1985,6 +2037,10 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & Drag::Roll { left, right, delta, .. } if delta != Tick::ZERO => Some(app.session.execute("timeline.roll", json!({"left": left.0, "right": right.0, "delta": delta.0}))), Drag::Slip { clip, delta } if delta != Tick::ZERO => Some(app.session.execute("timeline.slip", json!({"clip": clip.0, "delta": delta.0}))), Drag::Slide { clip, delta } if delta != Tick::ZERO => Some(app.session.execute("timeline.slide", json!({"clip": clip.0, "delta": delta.0}))), + Drag::Transition { id, start, duration, .. } => { + let moved = seq.all_tracks().find_map(|tr| tr.transitions.iter().find(|x| x.id == id)).is_some_and(|x| (x.start, x.duration) != (start, duration)); + moved.then(|| app.session.execute("sequence.setTransition", json!({"transition": id.0, "start": start.0, "duration": duration.0}))) + } Drag::Marquee { start } => { if let Some(end) = resp.interact_pointer_pos() { let r = Rect::from_two_pos(start, end); @@ -2008,6 +2064,15 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & } } + // ---- double-click a transition: Set Transition Duration (Premiere) + if resp.double_clicked() + && tool == Tool::Selection + && let Some(p) = resp.interact_pointer_pos() + && let Hit::Transition { id, .. } = hit(seq, layout, p) + { + crate::panels::transition_controls::open_duration(app, id); + } + // ---- double-click a nested sequence clip: open it in its own Timeline tab if resp.double_clicked() && tool == Tool::Selection diff --git a/crates/ui-egui/src/panels/timeline_hit.rs b/crates/ui-egui/src/panels/timeline_hit.rs index fa536c77..efc1dad0 100644 --- a/crates/ui-egui/src/panels/timeline_hit.rs +++ b/crates/ui-egui/src/panels/timeline_hit.rs @@ -50,9 +50,21 @@ pub(crate) fn edge_geometry(seq: &Sequence, layout: &Layout, track: TrackId, cli #[derive(Clone, Copy, Debug, PartialEq)] pub enum Hit { Ruler, - Clip { track: TrackId, clip: ClipId, edge: Option }, - Transition { track: TrackId, id: TransitionId }, - Empty { track: TrackId }, + Clip { + track: TrackId, + clip: ClipId, + edge: Option, + }, + /// A transition; `edge` is its start (`In`) or end (`Out`) when the press is on one of its + /// ends (dragging it changes the duration), `None` on its middle (dragging slides it). + Transition { + track: TrackId, + id: TransitionId, + edge: Option, + }, + Empty { + track: TrackId, + }, None, } @@ -69,7 +81,16 @@ pub fn hit(seq: &Sequence, layout: &Layout, pos: Pos2) -> Hit { let x0 = layout.x_of(trn.start); let x1 = layout.x_of(trn.end()); if pos.x >= x0 && pos.x <= x1 && pos.y > row.rect.min.y + 17.0 { - return Hit::Transition { track: row.track, id: trn.id }; + // the ends are edge zones, narrower on a short transition so its middle stays grabbable + let zone = EDGE_PX.min((x1 - x0) / 3.0); + let edge = if pos.x - x0 <= zone { + Some(Edge::In) + } else if x1 - pos.x <= zone { + Some(Edge::Out) + } else { + None + }; + return Hit::Transition { track: row.track, id: trn.id, edge }; } } if let Some((clip, edge)) = edge_at(tr, layout, pos.x) { diff --git a/crates/ui-egui/src/panels/transition_controls.rs b/crates/ui-egui/src/panels/transition_controls.rs new file mode 100644 index 00000000..80a8a0d4 --- /dev/null +++ b/crates/ui-egui/src/panels/transition_controls.rs @@ -0,0 +1,305 @@ +//! A selected transition (#430): its Effect Controls (Duration, Alignment, Reverse and its own +//! settings, as Premiere's Change transition settings / Align transitions / Change transition +//! duration pages describe them) and the Set Transition Duration dialog a double-click on it in +//! the Timeline opens. Every change is one `sequence.setTransition`; dragging a value is one undo +//! step. +//! +//! Not here yet: the A/B preview thumbnails with their Start / End sliders, Show Actual Sources, +//! and dragging the transition in Effect Controls' own time ruler (the Timeline does that). +//! +//! Automation ids: `effectControls.transition.duration`, `.alignment` (+ `.alignment.option.` while open), `.reverse`, `.reset`, `.param.` (a point's are `.param..x` / +//! `.y`; a list's entries `.param..option.` while open); the dialog's +//! `transitionDuration.value`, `.ok`, `.cancel`. + +use egui::{Rect, RichText, vec2}; +use filmcraft_edit::transitions::{self as trn, Alignment}; +use filmcraft_project::{ParamKind, ParamValue, Transition, TransitionId}; +use filmcraft_time::FrameRate; +use serde_json::{Value, json}; + +use crate::FilmcraftApp; +use crate::state::TransitionDurationDraft; + +type Elems = Vec<(String, Rect, String)>; + +/// The selected transition, when it still exists, with its alignment and how many frames it may +/// last at most (the clips it joins). +fn lookup(app: &FilmcraftApp, id: TransitionId) -> Option<(Transition, Alignment, i64)> { + let seq = app.session.active_sequence()?; + let rate = seq.settings.frame_rate.sane(); + seq.all_tracks().find_map(|tr| { + let x = tr.transitions.iter().find(|x| x.id == id)?; + let (lo, cut, hi) = trn::bounds(tr, x)?; + let max = rate.frame_at(hi - lo).max(1); + Some((x.clone(), trn::alignment(x, cut, rate.frame_duration()), max)) + }) +} + +/// The first selected transition that still exists. +pub fn selected(app: &FilmcraftApp) -> Option { + app.session.state.transition_selection.iter().copied().find(|id| lookup(app, *id).is_some()) +} + +fn timecode(frames: f64, rate: FrameRate, drop_frame: bool) -> String { + filmcraft_time::format_timecode_frames(frames.round() as i64, rate, drop_frame) +} + +/// A duration field in frames that reads and takes timecode (or a plain frame count). +fn duration_field(ui: &mut egui::Ui, frames: &mut i64, max: i64, rate: FrameRate, drop_frame: bool) -> egui::Response { + ui.add(egui::DragValue::new(frames).range(1..=max.max(1)).speed(0.25).custom_formatter(move |v, _| timecode(v, rate, drop_frame)).custom_parser(move |s| { + match s.trim().parse::() { + Ok(n) => Some(n as f64), + Err(_) => filmcraft_time::parse_timecode(s, rate, drop_frame, 0).ok().map(|n| n as f64), + } + })) +} + +/// Effect Controls for a selected transition. `rect` is the panel. +pub fn effect_controls(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect, id: TransitionId) { + let t = app.tokens; + let Some((x, align, max_frames)) = lookup(app, id) else { + crate::dock::placeholder(ui, rect, &t, tl!("(no clip selected)")); + return; + }; + let Some(seq) = app.session.active_sequence() else { return }; + let (rate, drop_frame) = (seq.settings.frame_rate.sane(), seq.settings.drop_frame); + let (fw, fh) = (f64::from(seq.settings.width), f64::from(seq.settings.height)); + let seq_name = app.session.state.active_sequence.and_then(|i| app.session.project.item(i)).map(|i| i.name.clone()).unwrap_or_default(); + let def = x.effect.def(); + let name = def.map_or(x.effect.effect.as_str(), |d| d.name).to_string(); + let one_sided = x.from.is_none() || x.to.is_none(); + let mut changes: Vec = Vec::new(); + let mut elems: Elems = Vec::new(); + let mut child = ui.new_child(egui::UiBuilder::new().max_rect(rect.shrink2(vec2(10.0, 6.0))).id_salt(("tr-ec", id.0))); + child.label(RichText::new(format!("{seq_name} · {name}")).color(t.text_dim)); + child.add_space(4.0); + egui::ScrollArea::vertical().id_salt(("tr-ec-scroll", id.0)).auto_shrink([false, false]).show(&mut child, |ui| { + egui::Grid::new(("tr-ec-grid", id.0)).num_columns(2).spacing([14.0, 8.0]).show(ui, |ui| { + ui.label(tl!("Duration")); + let mut frames = rate.frame_at(x.duration).max(1); + let r = duration_field(ui, &mut frames, max_frames, rate, drop_frame); + elems.push(("effectControls.transition.duration".into(), r.rect, timecode(frames as f64, rate, drop_frame))); + if r.changed() { + changes.push(json!({"frames": frames, "merge": true, "begin": r.drag_started() || !r.dragged()})); + } + ui.end_row(); + + ui.label(tl!("Alignment")); + let shown = align.label(); + let r = ui + .add_enabled_ui(!one_sided, |ui| { + egui::ComboBox::from_id_salt(("tr-align", id.0)) + .selected_text(shown) + .width(170.0) + .show_ui(ui, |ui| { + for a in Alignment::MENU { + let o = ui.selectable_label(a == align, a.label()); + if o.clicked() && a != align { + changes.push(json!({"align": a.id()})); + } + elems.push((format!("effectControls.transition.alignment.option.{}", a.id()), o.rect, a.label().to_string())); + } + }) + .response + }) + .inner; + elems.push(("effectControls.transition.alignment".into(), r.rect, shown.to_string())); + ui.end_row(); + + ui.label(tl!("Reverse")); + let mut rev = x.reverse; + let r = ui.checkbox(&mut rev, ""); + elems.push(("effectControls.transition.reverse".into(), r.rect, rev.to_string())); + if r.changed() { + changes.push(json!({"reverse": rev})); + } + ui.end_row(); + + for pd in def.map(|d| d.params.as_slice()).unwrap_or(&[]) { + let value = x.effect.params.get(pd.id).map_or(&pd.default, |p| &p.value); + ui.label(crate::i18n::t(pd.label)); + if let Some(v) = param_widget(ui, &mut elems, pd.id, &pd.kind, value, (fw, fh)) { + changes.push(v); + } + ui.end_row(); + } + }); + ui.add_space(8.0); + let r = ui.button(tl!("Reset")); + elems.push(("effectControls.transition.reset".into(), r.rect, "Reset".into())); + if r.clicked() { + changes.push(json!({"reset": true})); + } + }); + for (eid, r, l) in elems { + app.auto.add(&eid, r, &l); + } + for mut c in changes { + c["transition"] = json!(id.0); + if let Err(e) = app.session.execute("sequence.setTransition", c) { + app.ui.status = e.to_string(); + } + } +} + +/// One of a transition's own settings. Returns the change, as `sequence.setTransition` params. +fn param_widget(ui: &mut egui::Ui, elems: &mut Elems, pid: &str, kind: &ParamKind, value: &ParamValue, frame: (f64, f64)) -> Option { + let eid = format!("effectControls.transition.param.{pid}"); + let drag = |r: &egui::Response| json!({"merge": true, "begin": r.drag_started() || !r.dragged()}); + match (kind, value) { + (ParamKind::Float { min, max, soft_min, soft_max, unit, decimals }, ParamValue::Float(v)) => { + let mut v = *v; + let speed = ((soft_max - soft_min) / 200.0).max(0.001); + let r = ui.add(egui::DragValue::new(&mut v).range(*min..=*max).speed(speed).max_decimals(usize::from(*decimals)).suffix(*unit)); + elems.push((eid, r.rect, format!("{v}"))); + r.changed().then(|| { + let mut c = drag(&r); + c["params"] = json!({ pid: v }); + c + }) + } + (ParamKind::Angle, ParamValue::Float(v)) => { + let mut v = *v; + let r = ui.add(egui::DragValue::new(&mut v).speed(0.5).suffix("°")); + elems.push((eid, r.rect, format!("{v}"))); + r.changed().then(|| { + let mut c = drag(&r); + c["params"] = json!({ pid: v }); + c + }) + } + (ParamKind::Bool, ParamValue::Bool(b)) => { + let mut b = *b; + let r = ui.checkbox(&mut b, ""); + elems.push((eid, r.rect, b.to_string())); + r.changed().then(|| json!({"params": { pid: b }})) + } + (ParamKind::Choice(opts), ParamValue::Choice(c)) => { + let shown = opts.get(*c as usize).copied().unwrap_or(""); + let mut picked = None; + let mut options = Vec::new(); + let r = egui::ComboBox::from_id_salt(("tr-param", pid.to_string())).selected_text(crate::i18n::t(shown)).show_ui(ui, |ui| { + for (i, o) in opts.iter().enumerate() { + let r = ui.selectable_label(i == *c as usize, crate::i18n::t(o)); + if r.clicked() && i != *c as usize { + picked = Some(i); + } + options.push((format!("{eid}.option.{i}"), r.rect, o.to_string())); + } + }); + elems.extend(options); + elems.push((eid, r.response.rect, shown.to_string())); + picked.map(|i| json!({"params": { pid: i }})) + } + (ParamKind::Color, ParamValue::Color(c)) => { + let mut c = *c; + let r = ui.color_edit_button_rgba_unmultiplied(&mut c); + elems.push((eid, r.rect, format!("{c:?}"))); + r.changed().then(|| { + let mut v = drag(&r); + v["params"] = json!({ pid: [c[0], c[1], c[2], c[3]] }); + v + }) + } + (ParamKind::Point, ParamValue::Vec2(p)) => { + // an automatic point (NaN) is the frame's centre until it is moved + let (mut px, mut py) = (if p.x.is_nan() { frame.0 / 2.0 } else { p.x }, if p.y.is_nan() { frame.1 / 2.0 } else { p.y }); + let mut out = None; + ui.horizontal(|ui| { + let rx = ui.add(egui::DragValue::new(&mut px).speed(1.0).prefix("x ")); + let ry = ui.add(egui::DragValue::new(&mut py).speed(1.0).prefix("y ")); + elems.push((format!("{eid}.x"), rx.rect, format!("{px}"))); + elems.push((format!("{eid}.y"), ry.rect, format!("{py}"))); + for r in [&rx, &ry] { + if r.changed() { + let mut v = drag(r); + v["params"] = json!({ pid: [px, py] }); + out = Some(v); + } + } + }); + out + } + (_, v) => { + let r = ui.label(RichText::new(format!("{v:?}")).weak()); + elems.push((eid, r.rect, String::new())); + None + } + } +} + +/// Double-click a transition in the Timeline: Set Transition Duration. +pub fn open_duration(app: &mut FilmcraftApp, id: TransitionId) { + let Some((x, ..)) = lookup(app, id) else { return }; + let rate = app.session.active_sequence().map(|q| q.settings.frame_rate.sane()).unwrap_or_default(); + app.ui.transition_duration = TransitionDurationDraft { transition: id.0, frames: rate.frame_at(x.duration).max(1) }; + let _ = app.session.execute("timeline.select", json!({"transitions": [id.0]})); + app.dialog = Some(crate::Dialog::TransitionDuration); +} + +/// The Set Transition Duration dialog. Returns whether it stays open. +pub fn duration_dialog(app: &mut FilmcraftApp, ctx: &egui::Context) -> bool { + let mut d = app.ui.transition_duration.clone(); + let id = TransitionId(d.transition); + let Some((_, _, max_frames)) = lookup(app, id) else { return false }; + let Some(seq) = app.session.active_sequence() else { return false }; + let (rate, drop_frame) = (seq.settings.frame_rate.sane(), seq.settings.drop_frame); + let accent = app.tokens.accent; + let (mut keep, mut apply) = (true, false); + let mut elems: Elems = Vec::new(); + egui::Window::new(tl!("Set Transition Duration")) + .collapsible(false) + .resizable(false) + .default_width(260.0) + .anchor(egui::Align2::CENTER_CENTER, [0.0, 0.0]) + .show(ctx, |ui| { + ui.horizontal(|ui| { + ui.label(tl!("Duration:")); + let r = duration_field(ui, &mut d.frames, max_frames, rate, drop_frame); + elems.push(("transitionDuration.value".into(), r.rect, timecode(d.frames as f64, rate, drop_frame))); + }); + ui.add_space(8.0); + // one row high: a right-to-left layout would otherwise take the window's whole height + let row = egui::vec2(ui.available_width().max(220.0), ui.spacing().interact_size.y); + ui.allocate_ui_with_layout(row, egui::Layout::right_to_left(egui::Align::Center), |ui| { + let o = ui.add(egui::Button::new(RichText::new(tl!("OK")).color(egui::Color32::WHITE)).fill(accent)); + elems.push(("transitionDuration.ok".into(), o.rect, "OK".into())); + apply |= o.clicked(); + let c = ui.button(tl!("Cancel")); + elems.push(("transitionDuration.cancel".into(), c.rect, "Cancel".into())); + keep &= !c.clicked(); + }); + }); + for (eid, r, l) in elems { + app.auto.add(&eid, r, &l); + } + if ctx.input(|i| i.key_pressed(egui::Key::Escape)) { + keep = false; + } + if ctx.input(|i| i.key_pressed(egui::Key::Enter)) && !ctx.egui_wants_keyboard_input() { + apply = true; + } + if apply { + let cur = lookup(app, id).map(|(x, ..)| rate.frame_at(x.duration)); + if cur != Some(d.frames) + && let Err(e) = app.session.execute("sequence.setTransition", json!({"transition": id.0, "frames": d.frames})) + { + app.ui.status = e.to_string(); + } + keep = false; + } + app.ui.transition_duration = d; + keep +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn durations_read_as_timecode() { + assert_eq!(timecode(24.0, FrameRate::FPS_24, false), "00:00:01:00"); + assert_eq!(timecode(30.0, FrameRate::FPS_29_97, true), "00;00;01;00"); + } +} diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index 140d60cc..b8baf5a3 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -441,6 +441,9 @@ pub struct UiState { /// Sequence Settings dialog draft. #[serde(default)] pub sequence_settings: SequenceSettingsDraft, + /// Set Transition Duration dialog draft (double-click a transition). + #[serde(default)] + pub transition_duration: TransitionDurationDraft, /// On-monitor text editing (Type tool / double-click on a text layer). #[serde(default)] pub gfx_edit: Option, @@ -744,6 +747,15 @@ impl Default for AddTracksDraft { } } +/// The Set Transition Duration dialog (double-click a transition in the Timeline): which +/// transition, and its duration in frames. +#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] +#[serde(default)] +pub struct TransitionDurationDraft { + pub transition: u64, + pub frames: i64, +} + /// The Sequence Settings dialog (Sequence ▸ Sequence Settings…), filled from the active sequence /// when it opens (`panels::sequence_settings::open`). `tab`: `general`, `color` or `vr`; the /// timebase is `fps_num`/`fps_den`; `mix`: `Stereo`, `Mono`, `5.1` or `Adaptive`; `working_space`: @@ -890,6 +902,7 @@ impl Default for UiState { add_tracks: AddTracksDraft::default(), delete_tracks: DeleteTracksDraft::default(), sequence_settings: SequenceSettingsDraft::default(), + transition_duration: TransitionDurationDraft::default(), gfx_edit: None, pen_points: vec![], link_media: None, diff --git a/crates/ui-egui/tests/transitions_ui.rs b/crates/ui-egui/tests/transitions_ui.rs new file mode 100644 index 00000000..0cd4db43 --- /dev/null +++ b/crates/ui-egui/tests/transitions_ui.rs @@ -0,0 +1,275 @@ +//! Headless UI tests of applied transitions in the Timeline (#430, #224): click one to select it +//! and see it in Effect Controls, Delete it, drag its end or its middle, double-click it for Set +//! Transition Duration. The real `FilmcraftApp` under `egui_kittest`, driven over the control +//! channel by automation id. +//! +//! Set `FILMCRAFT_UI_SNAPSHOT_DIR=` to also render the window offscreen with wgpu and write +//! `transition-*.png` there; without it no GPU is needed. + +use std::sync::mpsc::{Sender, channel}; + +use egui_kittest::Harness; +use filmcraft_engine::Session; +use filmcraft_ui_egui::FilmcraftApp; +use filmcraft_ui_egui::control::ControlRequest; +use serde_json::{Value, json}; + +struct Driver { + harness: Harness<'static, FilmcraftApp>, + tx: Sender, + snapshots: Option, +} + +/// The demo's first video transition between two clips. +#[derive(Clone, Copy, Debug)] +struct Crossing { + id: u64, + start: i64, + duration: i64, + cut: i64, +} + +impl Driver { + fn demo() -> Self { + let mut session = Session::default(); + session.execute("file.openDemoProject", json!({})).expect("demo project"); + let (tx, rx) = channel(); + let app = FilmcraftApp::new(session).with_control(rx); + let snapshots = std::env::var_os("FILMCRAFT_UI_SNAPSHOT_DIR").map(std::path::PathBuf::from); + // 60 fps steps, so a double-click's two clicks fall within egui's double-click time + let mut b = Harness::builder().with_size(egui::vec2(1600.0, 980.0)).with_step_dt(1.0 / 60.0).with_max_steps(10_000); + if snapshots.is_some() { + b = b.wgpu(); + } + let harness = b.build_eframe(move |_cc| app); + let mut d = Driver { harness, tx, snapshots }; + d.frames(4); + d.ok("ui.panel.show", json!({"panel": "EffectControls"})); + d.frames(3); + d + } + + fn frames(&mut self, n: usize) { + for _ in 0..n { + let ctx = self.harness.ctx.clone(); + let mut raw = std::mem::take(self.harness.input_mut()); + eframe::App::raw_input_hook(self.harness.state_mut(), &ctx, &mut raw); + *self.harness.input_mut() = raw; + self.harness.step(); + } + } + + fn call(&mut self, method: &str, params: Value) -> Value { + let (req, reply) = ControlRequest::new(method, params.clone()); + self.tx.send(req).unwrap(); + for _ in 0..600 { + self.frames(1); + if let Ok(v) = reply.try_recv() { + return v; + } + } + panic!("no reply to {method} {params}"); + } + + fn ok(&mut self, method: &str, params: Value) -> Value { + let v = self.call(method, params.clone()); + assert_eq!(v["ok"], json!(true), "{method} {params} failed: {v}"); + v["result"].clone() + } + + fn exec(&mut self, command: &str, params: Value) -> Value { + self.ok("engine.execute", json!({"command": command, "params": params})) + } + + fn find(&mut self, id: &str) -> Option<(f64, f64, f64, f64, String)> { + self.frames(2); + let v = self.ok("ui.elements", json!({"prefix": id})); + let e = v.as_array().unwrap().iter().find(|e| e["id"] == id)?.clone(); + let r: Vec = e["rect"].as_array().unwrap().iter().map(|x| x.as_f64().unwrap()).collect(); + Some((r[0], r[1], r[2], r[3], e["label"].as_str().unwrap_or_default().to_string())) + } + + /// With `FILMCRAFT_UI_SNAPSHOT_DIR` set, render the whole window offscreen to `.png`. + fn snapshot(&mut self, name: &str) { + let Some(dir) = self.snapshots.clone() else { return }; + self.frames(2); + match self.harness.render() { + Ok(img) => { + std::fs::create_dir_all(&dir).unwrap(); + img.save(dir.join(format!("{name}.png"))).unwrap(); + } + Err(e) => eprintln!("snapshot {name} skipped: {e}"), + } + } + + fn label(&mut self, id: &str) -> String { + self.find(id).unwrap_or_else(|| panic!("no element {id}")).4 + } + + fn click(&mut self, id: &str) { + self.find(id).unwrap_or_else(|| panic!("no element {id}")); + self.ok("ui.click", json!({"id": id})); + self.frames(3); + } + + fn crossing(&mut self) -> Crossing { + let q = self.exec("sequence.inspect", json!({})); + for tr in q["video"].as_array().unwrap() { + for x in tr["transitions"].as_array().unwrap() { + if let (Some(_), Some(to)) = (x["from"].as_u64(), x["to"].as_u64()) { + let cut = tr["items"].as_array().unwrap().iter().find(|i| i["clip"] == to).unwrap()["start"].as_i64().unwrap(); + return Crossing { id: x["id"].as_u64().unwrap(), start: x["start"].as_i64().unwrap(), duration: x["duration"].as_i64().unwrap(), cut }; + } + } + } + panic!("the demo has a transition between two clips"); + } + + fn transition(&mut self, id: u64) -> Option { + let q = self.exec("sequence.inspect", json!({})); + q["video"].as_array().unwrap().iter().flat_map(|t| t["transitions"].as_array().unwrap().clone()).find(|x| x["id"] == id) + } + + fn rate(&mut self) -> (i64, i64) { + let q = self.exec("sequence.inspect", json!({})); + (q["settings"]["frame_rate"]["num"].as_i64().unwrap(), q["settings"]["frame_rate"]["den"].as_i64().unwrap()) + } +} + +/// Ticks in one frame of `rate`. +fn frame_ticks((num, den): (i64, i64)) -> i64 { + 254_016_000_000 * den / num +} + +/// Clicking a transition selects it (and only it) and opens it in Effect Controls; Delete removes +/// it, leaving its clips; undo brings it back. +#[test] +fn click_selects_a_transition_and_delete_removes_it() { + let mut d = Driver::demo(); + let c = d.crossing(); + let tid = format!("timeline.transition.{}", c.id); + d.click(&tid); + let q = d.exec("sequence.inspect", json!({})); + assert_eq!(q["transitionSelection"], json!([c.id]), "{q}"); + assert_eq!(q["selection"], json!([]), "a transition replaces the clip selection"); + // Effect Controls shows it: duration as timecode, its alignment, Reverse + let (num, den) = d.rate(); + let rate = filmcraft_engine::time::FrameRate { num, den }; + let frames = c.duration / frame_ticks((num, den)); + assert_eq!(d.label("effectControls.transition.duration"), filmcraft_engine::time::format_timecode_frames(frames, rate, false)); + assert_eq!(d.label("effectControls.transition.alignment"), "Center at Cut"); + assert_eq!(d.label("effectControls.transition.reverse"), "false"); + d.snapshot("transition-selected"); + let clips_before = q["video"].as_array().unwrap().iter().map(|t| t["items"].as_array().unwrap().len()).sum::(); + // Delete + d.ok("ui.key", json!({"key": "Delete"})); + d.frames(3); + assert!(d.transition(c.id).is_none(), "Delete removes the selected transition"); + let q = d.exec("sequence.inspect", json!({})); + assert_eq!(q["video"].as_array().unwrap().iter().map(|t| t["items"].as_array().unwrap().len()).sum::(), clips_before, "its clips stay"); + assert!(d.find("effectControls.transition.duration").is_none(), "Effect Controls lets go of it"); + d.exec("edit.undo", json!({})); + assert!(d.transition(c.id).is_some(), "undo brings it back"); + // clicking a clip selects the clip instead + let clip = q["video"][0]["items"][0]["clip"].as_u64().unwrap(); + d.exec("timeline.select", json!({"transitions": [c.id]})); + d.exec("timeline.select", json!({"clips": [clip]})); + assert_eq!(d.exec("sequence.inspect", json!({}))["transitionSelection"], json!([])); +} + +/// Dragging a centred transition's end changes its duration on both sides of the cut (#224); +/// dragging its middle slides it over the cut (Custom Start). Each drag is one undo step. +#[test] +fn dragging_a_transition_end_or_middle() { + let mut d = Driver::demo(); + let c = d.crossing(); + let frame = frame_ticks(d.rate()); + let undo_before = d.exec("sequence.inspect", json!({})); + let (x, y, w, h, _) = d.find(&format!("timeline.transition.{}.out", c.id)).expect("the transition's end"); + let (fx, fy) = (x + w / 2.0, y + h / 2.0); + d.ok("ui.drag", json!({"from": {"x": fx, "y": fy}, "to": {"x": fx + 40.0, "y": fy}, "steps": 8})); + d.frames(3); + let x1 = d.transition(c.id).unwrap(); + let (s1, d1) = (x1["start"].as_i64().unwrap(), x1["duration"].as_i64().unwrap()); + assert!(d1 > c.duration, "dragging the end out lengthens it: {} -> {d1}", c.duration); + assert_eq!(x1["align"], json!("center"), "a centred transition stays centred"); + assert!((s1 + d1 / 2 - c.cut).abs() <= frame, "still centred on the cut"); + assert_eq!((d1 - c.duration) % (2 * frame), 0, "both ends moved by whole frames"); + // the middle: slide it right of centre + let (x, y, w, h, _) = d.find(&format!("timeline.transition.{}", c.id)).unwrap(); + let (mx, my) = (x + w / 2.0, y + h / 2.0); + // more than egui's 6 px click tolerance, or the press is a click + d.ok("ui.drag", json!({"from": {"x": mx, "y": my}, "to": {"x": mx + 14.0, "y": my}, "steps": 6})); + d.frames(3); + let x2 = d.transition(c.id).unwrap(); + assert_eq!(x2["duration"].as_i64(), Some(d1), "sliding keeps the duration"); + assert!(x2["start"].as_i64().unwrap() > s1, "and moves it right"); + assert!(x2["start"].as_i64().unwrap() <= c.cut, "still over the cut"); + assert_eq!(x2["align"], json!("custom")); + // two drags, two undo steps + d.exec("edit.undo", json!({})); + d.exec("edit.undo", json!({})); + let back = d.transition(c.id).unwrap(); + assert_eq!((back["start"].as_i64(), back["duration"].as_i64()), (Some(c.start), Some(c.duration)), "{undo_before}"); +} + +/// Double-clicking a transition opens Set Transition Duration, whose OK sets the duration +/// (following the alignment); Effect Controls' Alignment menu re-aligns it on the cut. +#[test] +fn set_transition_duration_and_alignment_menu() { + let mut d = Driver::demo(); + let c = d.crossing(); + let frame = frame_ticks(d.rate()); + // double-click it + let (x0, y0, w, h, _) = d.find(&format!("timeline.transition.{}", c.id)).unwrap(); + d.ok("ui.click", json!({"x": x0 + w / 2.0, "y": y0 + h / 2.0, "count": 2})); + d.frames(3); + let frames = c.duration / frame; + assert!(d.find("transitionDuration.value").is_some(), "Set Transition Duration opened"); + assert_eq!(d.harness.state().ui.transition_duration.frames, frames, "with its duration"); + assert_eq!(d.exec("sequence.inspect", json!({}))["transitionSelection"], json!([c.id]), "and selected it"); + d.snapshot("transition-duration-dialog"); + // Center at Cut: a new duration moves both ends + d.harness.state_mut().ui.transition_duration.frames = 10; + d.frames(2); + d.click("transitionDuration.ok"); + assert!(d.find("transitionDuration.value").is_none(), "closed"); + let x = d.transition(c.id).unwrap(); + assert_eq!((x["start"].as_i64(), x["duration"].as_i64(), x["align"].as_str()), (Some(c.cut - 5 * frame), Some(10 * frame), Some("center"))); + // the Alignment menu: Start at Cut + d.click("effectControls.transition.alignment"); + d.click("effectControls.transition.alignment.option.start"); + let x = d.transition(c.id).unwrap(); + assert_eq!((x["start"].as_i64(), x["duration"].as_i64(), x["align"].as_str()), (Some(c.cut), Some(10 * frame), Some("start"))); + assert_eq!(d.label("effectControls.transition.alignment"), "Start at Cut"); + // Reverse + d.click("effectControls.transition.reverse"); + assert_eq!(d.transition(c.id).unwrap()["reverse"], json!(true)); +} + +/// A transition's own settings are in Effect Controls and change it (Push: Direction, Motion Blur). +#[test] +fn transition_settings_in_effect_controls() { + let mut d = Driver::demo(); + let q = d.exec("sequence.inspect", json!({})); + let push = q["video"] + .as_array() + .unwrap() + .iter() + .flat_map(|t| t["transitions"].as_array().unwrap().clone()) + .find(|x| x["effect"] == "push") + .expect("the demo has a Push"); + let id = push["id"].as_u64().unwrap(); + d.click(&format!("timeline.transition.{id}")); + for p in ["direction", "motion_blur"] { + assert!(d.find(&format!("effectControls.transition.param.{p}")).is_some(), "{p} shown"); + } + d.snapshot("transition-push-settings"); + d.click("effectControls.transition.param.direction"); + d.click("effectControls.transition.param.direction.option.0"); + assert_eq!(d.transition(id).unwrap()["params"]["direction"], json!({"Choice": 0})); + d.click("effectControls.transition.reset"); + let def = filmcraft_engine::project::vtransition::find_transition("push", filmcraft_engine::project::EffectKind::VideoTransition).unwrap(); + let want = serde_json::to_value(&def.instance().params.get("direction").unwrap().value).unwrap(); + assert_eq!(d.transition(id).unwrap()["params"]["direction"], want, "Reset puts Push's own defaults back"); +} From f61669d2088437bf2a14d734d645c2a86f8afb2d Mon Sep 17 00:00:00 2001 From: swillunderscore <201272413+swillunderscore@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:20:25 -0500 Subject: [PATCH 21/35] #224: dragging a transition's end trims that end only Adobe's Change transition duration page shows the Trim-In / Trim-Out icon on a transition's end in the Timeline: the drag trims that end, and the other stays. A centred transition dragged by one end therefore becomes Custom Start, rather than growing on both sides. The alignment rule ("how the transition changes length depends on the alignment") stays for typed durations: Effect Controls' Duration and Set Transition Duration. Co-Authored-By: Claude Opus 5.5 --- crates/edit/src/transitions.rs | 33 +++++++++++--------------- crates/ui-egui/src/panels/timeline.rs | 5 +--- crates/ui-egui/tests/transitions_ui.rs | 13 +++++----- 3 files changed, 22 insertions(+), 29 deletions(-) diff --git a/crates/edit/src/transitions.rs b/crates/edit/src/transitions.rs index d28c7f7f..c487c7d2 100644 --- a/crates/edit/src/transitions.rs +++ b/crates/edit/src/transitions.rs @@ -154,12 +154,13 @@ pub fn set_span(seq: &mut Sequence, id: TransitionId, start: Tick, duration: Tic } /// Where dragging a transition by `delta` in the Timeline puts it, as `(start, duration)`. Its -/// middle (`edge` None) slides it over the cut. An end changes its duration: a Center at Cut -/// transition grows or shrinks on both sides and stays centred on the cut (its alignment says how -/// it changes length), any other keeps its other end where it is. A fade to or from nothing keeps +/// middle (`edge` None) slides it over the cut. An end trims that end only (Premiere shows the +/// Trim-In / Trim-Out icon there): the other end stays, whatever the alignment, so a Center at +/// Cut transition becomes Custom Start. (A typed duration, in Effect Controls or Set Transition +/// Duration, follows the alignment instead: see [`start_for`].) A fade to or from nothing keeps /// its clip-edge end, and does not slide. Always clamped to where it may be ([`bounds`], over the -/// cut, at least one `frame`); `snap` puts a centred start on a frame. `None` when its clips are gone. -pub fn drag_span(track: &Track, tr: &Transition, edge: Option, delta: Tick, frame: Tick, snap: impl Fn(Tick) -> Tick) -> Option<(Tick, Tick)> { +/// cut, at least one `frame`). `None` when its clips are gone. +pub fn drag_span(track: &Track, tr: &Transition, edge: Option, delta: Tick, frame: Tick) -> Option<(Tick, Tick)> { let (lo, cut, hi) = bounds(track, tr)?; let frame = frame.max(Tick(1)); let (s0, e0, d0) = (tr.start, tr.end(), tr.duration); @@ -171,13 +172,6 @@ pub fn drag_span(track: &Track, tr: &Transition, edge: Option, delta: Tick let (min_s, max_s) = (lo.max(cut - d0), (hi - d0).min(cut)); if min_s > max_s { unchanged } else { Some(((s0 + delta).clamp(min_s, max_s), d0)) } } - Some(e) if !one_sided && alignment(tr, cut, frame) == Alignment::Center => { - // dragging an end by n frames moves both ends by n: the duration changes by 2n - let grow = if e == Edge::In { Tick(0) - delta } else { delta }; - let max_d = Tick((cut - lo).min(hi - cut).0.saturating_mul(2)); - let d = Tick(d0.0.saturating_add(grow.0.saturating_mul(2))).clamp(frame, max_d.max(frame)); - Some((snap(cut - d.mul_ratio(1, 2)), d)) - } Some(Edge::In) if tr.from.is_none() => unchanged, Some(Edge::In) => { let max_s = cut.min(e0 - frame).max(lo); @@ -349,18 +343,19 @@ mod tests { fn drag(q: &Sequence, id: u64, edge: Option, frames: i64) -> (i64, i64) { let (t, x) = find(q, TransitionId(id)).unwrap(); - let (st, d) = drag_span(t, x, edge, f(frames), f(1), |t| R.snap(t)).unwrap(); + let (st, d) = drag_span(t, x, edge, f(frames), f(1)).unwrap(); (R.frame_at(st), R.frame_at(d)) } #[test] fn dragging_a_transition() { let mut q = seq(); - // centred (36..60 over the cut at 48): an end grows / shrinks it on both sides - assert_eq!(drag(&q, 1, Some(Edge::In), -4), (32, 32)); - assert_eq!(drag(&q, 1, Some(Edge::Out), -10), (46, 4)); - assert_eq!(drag(&q, 1, Some(Edge::Out), -100), (47, 1), "at least a frame (centred: it starts on the frame before the cut)"); - assert_eq!(drag(&q, 1, Some(Edge::Out), 100), (0, 96), "at most both clips"); + // centred (36..60 over the cut at 48): an end trims that end only, the other stays + assert_eq!(drag(&q, 1, Some(Edge::In), -4), (32, 28)); + assert_eq!(drag(&q, 1, Some(Edge::Out), -10), (36, 14)); + assert_eq!(drag(&q, 1, Some(Edge::Out), -100), (36, 12), "the end stops at the cut"); + assert_eq!(drag(&q, 1, Some(Edge::Out), 100), (36, 60), "and at the incoming clip's end"); + assert_eq!(drag(&q, 1, Some(Edge::In), -100), (0, 60), "the start at the outgoing clip's start"); // the middle slides it, staying over the cut and inside the clips assert_eq!(drag(&q, 1, None, 5), (41, 24)); assert_eq!(drag(&q, 1, None, 100), (48, 24)); @@ -380,7 +375,7 @@ mod tests { // whatever comes out is a span set_span accepts for (id, edge, n) in [(1, Some(Edge::In), -70), (1, None, 33), (2, Some(Edge::Out), 99), (3, Some(Edge::In), -99)] { let (t, x) = find(&q, TransitionId(id)).unwrap(); - let (st, d) = drag_span(t, x, edge, f(n), f(1), |t| R.snap(t)).unwrap(); + let (st, d) = drag_span(t, x, edge, f(n), f(1)).unwrap(); let mut c = q.clone(); set_span(&mut c, TransitionId(id), st, d, f(1), f(1)).unwrap(); } diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index a826e9a4..a1b1d926 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -1983,10 +1983,7 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & Drag::Transition { id, edge, from, start, duration } => { let delta = rate.snap_nearest(t_here - from); let span = seq.all_tracks().find_map(|tr| { - tr.transitions - .iter() - .find(|x| x.id == id) - .and_then(|x| filmcraft_edit::transitions::drag_span(tr, x, edge, delta, rate.frame_duration(), |t| rate.snap(t))) + tr.transitions.iter().find(|x| x.id == id).and_then(|x| filmcraft_edit::transitions::drag_span(tr, x, edge, delta, rate.frame_duration())) }); let (start, duration) = span.unwrap_or((start, duration)); Some(Drag::Transition { id, edge, from, start, duration }) diff --git a/crates/ui-egui/tests/transitions_ui.rs b/crates/ui-egui/tests/transitions_ui.rs index 0cd4db43..fbcbabb5 100644 --- a/crates/ui-egui/tests/transitions_ui.rs +++ b/crates/ui-egui/tests/transitions_ui.rs @@ -177,8 +177,8 @@ fn click_selects_a_transition_and_delete_removes_it() { assert_eq!(d.exec("sequence.inspect", json!({}))["transitionSelection"], json!([])); } -/// Dragging a centred transition's end changes its duration on both sides of the cut (#224); -/// dragging its middle slides it over the cut (Custom Start). Each drag is one undo step. +/// Dragging a transition's end trims that end only (#224); dragging its middle slides it over the +/// cut (Custom Start). Each drag is one undo step. #[test] fn dragging_a_transition_end_or_middle() { let mut d = Driver::demo(); @@ -192,9 +192,9 @@ fn dragging_a_transition_end_or_middle() { let x1 = d.transition(c.id).unwrap(); let (s1, d1) = (x1["start"].as_i64().unwrap(), x1["duration"].as_i64().unwrap()); assert!(d1 > c.duration, "dragging the end out lengthens it: {} -> {d1}", c.duration); - assert_eq!(x1["align"], json!("center"), "a centred transition stays centred"); - assert!((s1 + d1 / 2 - c.cut).abs() <= frame, "still centred on the cut"); - assert_eq!((d1 - c.duration) % (2 * frame), 0, "both ends moved by whole frames"); + assert_eq!(s1, c.start, "the start stays where it was"); + assert_eq!((d1 - c.duration) % frame, 0, "by whole frames"); + assert_eq!(x1["align"], json!("custom"), "no longer centred: Custom Start"); // the middle: slide it right of centre let (x, y, w, h, _) = d.find(&format!("timeline.transition.{}", c.id)).unwrap(); let (mx, my) = (x + w / 2.0, y + h / 2.0); @@ -205,7 +205,8 @@ fn dragging_a_transition_end_or_middle() { assert_eq!(x2["duration"].as_i64(), Some(d1), "sliding keeps the duration"); assert!(x2["start"].as_i64().unwrap() > s1, "and moves it right"); assert!(x2["start"].as_i64().unwrap() <= c.cut, "still over the cut"); - assert_eq!(x2["align"], json!("custom")); + // off-centre (Custom Start), or Start at Cut if the slide reached the cut, where it stops + assert!(matches!(x2["align"].as_str(), Some("custom" | "start")), "{}", x2["align"]); // two drags, two undo steps d.exec("edit.undo", json!({})); d.exec("edit.undo", json!({})); From 71dd56ff8f17404017f9f70a5e9578a78a01e892 Mon Sep 17 00:00:00 2001 From: Mara Johnson Date: Fri, 9 Oct 2026 20:47:33 -0400 Subject: [PATCH 22/35] fix(mcp): media_import schema mirrors file.import (#499) --- crates/automation/src/server.rs | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/crates/automation/src/server.rs b/crates/automation/src/server.rs index 4a43cd2c..fcce5cfd 100644 --- a/crates/automation/src/server.rs +++ b/crates/automation/src/server.rs @@ -139,6 +139,18 @@ pub struct TypeParams { pub text: String, } +#[derive(Debug, Deserialize, JsonSchema)] +pub struct MediaImportParams { + /// Absolute paths to media files (MP4/MOV, WAV/MP3/FLAC/AIFF/Ogg, PNG/JPEG/…). + pub paths: Vec, + /// Bin ID to import into (optional). + #[serde(default)] + pub bin: Option, + /// Import numbered stills as an image sequence (optional, default false). + #[serde(default)] + pub image_sequence: Option, +} + #[derive(Debug, Default, Deserialize, JsonSchema)] pub struct ElementsParams { /// Only element ids starting with this prefix (e.g. `timeline.clip.`). @@ -381,9 +393,15 @@ impl FilmcraftMcp { description = "Import media files by absolute path (MP4/MOV, WAV/MP3/FLAC/AIFF/Ogg, PNG/JPEG/…).", annotations(read_only_hint = false, destructive_hint = false, idempotent_hint = false, open_world_hint = false) )] - async fn media_import(&self, Parameters(p): Parameters) -> Result { - let paths: Vec<&str> = p.text.split('\n').map(str::trim).filter(|s| !s.is_empty()).collect(); - wrap(self.run("file.import", json!({"paths": paths})).await) + async fn media_import(&self, Parameters(p): Parameters) -> Result { + let mut params = json!({"paths": p.paths}); + if let Some(bin) = p.bin { + params["bin"] = json!(bin); + } + if let Some(seq) = p.image_sequence { + params["imageSequence"] = json!(seq); + } + wrap(self.run("file.import", params).await) } #[tool( From b6c604c104c00234c7c48ff543095cc58e226f70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Cie=C5=9Blik?= <132496025+Hathor875@users.noreply.github.com> Date: Sat, 10 Oct 2026 04:21:06 +0200 Subject: [PATCH 23/35] clip.link: pair video clips with overlapping audio clips (#507) When linking a selection containing multiple video clips, create one link per video clip with its overlapping audio clips rather than a single link for all clips, matching Premiere Pro behavior. --- contributors/people.toml | 4 ++ crates/engine/src/commands.rs | 71 +++++++++++++++++++-- crates/engine/src/explicit_targets_tests.rs | 38 +++++++++++ 3 files changed, 109 insertions(+), 4 deletions(-) diff --git a/contributors/people.toml b/contributors/people.toml index 25cf020e..c7762744 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -40,3 +40,7 @@ sync_github_name = true [people.ItzBloxy] display_name = "Bloxy" + +[people.Hathor875] +real_name = "Krzysztof Cieślik" +sync_github_name = true diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index b5e4c847..d6065824 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -1342,10 +1342,73 @@ fn build() -> Vec { let sel = clips_p(s, p); let linked = sel.iter().all(|c| s.active_sequence().and_then(|q| q.find_item(*c)).is_some_and(|(_, i)| i.link.is_some())); s.edit_sequence(if linked { "Unlink" } else { "Link" }, |q, ctx, _| { - let l = if linked { None } else { Some(ctx.alloc()) }; - for c in &sel { - if let Some((_, i)) = q.find_item_mut(*c) { - i.link = l; + if linked { + for c in &sel { + if let Some((_, i)) = q.find_item_mut(*c) { + i.link = None; + } + } + } else { + let has_video = sel.iter().any(|c| q.find_item(*c).is_some_and(|(tid, _)| q.video_tracks.iter().any(|t| t.id == tid))); + let has_audio = sel.iter().any(|c| q.find_item(*c).is_some_and(|(tid, _)| q.audio_tracks.iter().any(|t| t.id == tid))); + let v_clips: Vec<(ClipId, filmcraft_project::ItemId, filmcraft_time::TimeRange)> = sel + .iter() + .filter_map(|c| { + let (tid, i) = q.find_item(*c)?; + q.video_tracks.iter().any(|t| t.id == tid).then_some((*c, i.item, i.range())) + }) + .collect(); + if has_video && has_audio && v_clips.len() > 1 { + let audio_specs: Vec<(ClipId, filmcraft_project::ItemId, filmcraft_time::TimeRange)> = sel + .iter() + .filter_map(|c| { + let (tid, i) = q.find_item(*c)?; + q.audio_tracks.iter().any(|t| t.id == tid).then_some((*c, i.item, i.range())) + }) + .collect(); + let mut linked_audios = std::collections::HashSet::new(); + for (vid, item_id, vrange) in &v_clips { + let l = ctx.alloc(); + if let Some((_, vi)) = q.find_item_mut(*vid) { + vi.link = Some(l); + } + // Pair with overlapping audio clips from the same item, or overlapping audio clips if no same-item audio + let matching: Vec = audio_specs + .iter() + .filter(|(aid, aitem, arange)| !linked_audios.contains(aid) && aitem == item_id && arange.overlaps(vrange)) + .map(|(aid, _, _)| *aid) + .collect(); + let to_link = if !matching.is_empty() { + matching + } else { + audio_specs + .iter() + .filter(|(aid, _, arange)| !linked_audios.contains(aid) && arange.overlaps(vrange)) + .map(|(aid, _, _)| *aid) + .collect() + }; + for aid in to_link { + linked_audios.insert(aid); + if let Some((_, ai)) = q.find_item_mut(aid) { + ai.link = Some(l); + } + } + } + // Any remaining selected audio clips get their own links + for (aid, _, _) in audio_specs { + if !linked_audios.contains(&aid) + && let Some((_, ai)) = q.find_item_mut(aid) + { + ai.link = Some(ctx.alloc()); + } + } + } else { + let l = ctx.alloc(); + for c in &sel { + if let Some((_, i)) = q.find_item_mut(*c) { + i.link = Some(l); + } + } } } Ok(()) diff --git a/crates/engine/src/explicit_targets_tests.rs b/crates/engine/src/explicit_targets_tests.rs index 1f2eca77..d581a495 100644 --- a/crates/engine/src/explicit_targets_tests.rs +++ b/crates/engine/src/explicit_targets_tests.rs @@ -211,3 +211,41 @@ fn project_delete_takes_explicit_bins_and_items_without_a_selection() { s.execute("edit.undo", json!({})).unwrap(); assert_eq!(s.project.items.len(), before); } + +#[test] +fn link_multiple_video_and_audio_pairs_creates_one_link_per_pair() { + let mut s = demo(); + let v_clips = v1(&s); + let a_clips = a1(&s); + assert!(v_clips.len() >= 3 && a_clips.len() >= 3); + let (v0, v1, v2) = (v_clips[0].id, v_clips[1].id, v_clips[2].id); + let (a0, a1, a2) = (a_clips[0].id, a_clips[1].id, a_clips[2].id); + + // First, unlink all three pairs + let r = s.execute("clip.link", json!({"clips": [v0.0, a0.0, v1.0, a1.0, v2.0, a2.0]})).unwrap(); + assert_eq!(r["linked"], false); + for cid in [v0, a0, v1, a1, v2, a2] { + assert_eq!(clip(&s, cid).unwrap().link, None); + } + + // Now, run clip.link on all six clips. In Premiere parity (#507), + // it should create 3 separate links (one per pair), not one big link for all six. + let r = s.execute("clip.link", json!({"clips": [v0.0, a0.0, v1.0, a1.0, v2.0, a2.0]})).unwrap(); + assert_eq!(r["linked"], true); + + let l0 = clip(&s, v0).unwrap().link; + let la0 = clip(&s, a0).unwrap().link; + let l1 = clip(&s, v1).unwrap().link; + let la1 = clip(&s, a1).unwrap().link; + let l2 = clip(&s, v2).unwrap().link; + let la2 = clip(&s, a2).unwrap().link; + + assert!(l0.is_some() && l1.is_some() && l2.is_some()); + assert_eq!(l0, la0, "v0 linked to a0"); + assert_eq!(l1, la1, "v1 linked to a1"); + assert_eq!(l2, la2, "v2 linked to a2"); + + assert_ne!(l0, l1, "different pairs must not share the same link id"); + assert_ne!(l1, l2, "different pairs must not share the same link id"); + assert_ne!(l0, l2, "different pairs must not share the same link id"); +} From a0708ff24322e863fba72804f289bc9755d385ba Mon Sep 17 00:00:00 2001 From: Mitch Date: Sat, 10 Oct 2026 14:14:19 +1100 Subject: [PATCH 24/35] #171: files dropped onto the Timeline are placed there, not just imported Dropping OS files on the Timeline panel now imports them and places them (overwrite) at the pointer, snapped, chained end to end. Controlled by Settings > Timeline > "Place files dropped onto the Timeline directly on the Timeline" (timeline.dropImportToTimeline, default on). Drops elsewhere stay a plain bin import. Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/settings.rs | 4 + crates/engine/src/settings_tests.rs | 41 ++++++ crates/ui-egui/src/i18n/es.tsv | 1 + crates/ui-egui/src/lib.rs | 21 +++- crates/ui-egui/src/panels/timeline.rs | 79 ++++++++++++ crates/ui-egui/tests/timeline_drop_ui.rs | 154 +++++++++++++++++++++++ 6 files changed, 298 insertions(+), 2 deletions(-) create mode 100644 crates/ui-egui/tests/timeline_drop_ui.rs diff --git a/crates/engine/src/settings.rs b/crates/engine/src/settings.rs index 1e035035..d417298f 100644 --- a/crates/engine/src/settings.rs +++ b/crates/engine/src/settings.rs @@ -428,6 +428,8 @@ pub struct TimelinePrefs { pub match_frame_sets_in: bool, pub restore_open_sequences: bool, pub add_tracks_automatically: bool, + /// Files dropped onto the Timeline are placed directly on it. + pub drop_import_to_timeline: bool, } impl Default for TimelinePrefs { @@ -454,6 +456,7 @@ impl Default for TimelinePrefs { match_frame_sets_in: false, restore_open_sequences: true, add_tracks_automatically: true, + drop_import_to_timeline: true, } } } @@ -1005,6 +1008,7 @@ static CATEGORIES: &[Category] = &[ b("timeline.matchFrameSetsIn", "Match frame sets in point", false), b("timeline.restoreOpenSequences", "Restore open sequences when opening projects", false), b("timeline.addTracksAutomatically", "Add tracks automatically when editing source clips onto the Timeline", false), + b("timeline.dropImportToTimeline", "Place files dropped onto the Timeline directly on the Timeline", true), ], }, Category { diff --git a/crates/engine/src/settings_tests.rs b/crates/engine/src/settings_tests.rs index d706e4a1..1686bf57 100644 --- a/crates/engine/src/settings_tests.rs +++ b/crates/engine/src/settings_tests.rs @@ -551,3 +551,44 @@ fn hardware_decoding_setting_drives_the_decoder_switch() { assert!(filmcraft_codecs::hw::hardware_decoding()); assert!(settings::field("playback.hardwareDecoding").is_some_and(|f| f.wired && matches!(f.kind, Kind::Choice(_)))); } + +/// Settings ▸ Timeline ▸ "Place files dropped onto the Timeline directly on the Timeline": on by default. +#[test] +fn drop_import_to_timeline_defaults_to_on() { + assert!(settings::TimelinePrefs::default().drop_import_to_timeline); + assert!(Preferences::default().timeline.drop_import_to_timeline); +} + +/// The key is saved under `timeline.dropImportToTimeline`, read back by `prefs.get`, and persisted. +#[test] +fn drop_import_to_timeline_round_trips_through_prefs() { + let dir = tmp_dir("drop-import-to-timeline"); + let path = dir.join("preferences.json"); + let mut s = Session { prefs_path: Some(path.clone()), ..Session::default() }; + set(&mut s, "timeline.dropImportToTimeline", json!(false)); + assert!(!s.prefs.timeline.drop_import_to_timeline); + assert_eq!(s.execute("prefs.get", json!({"key": "timeline.dropImportToTimeline"})).unwrap(), json!(false)); + assert!(!Preferences::load(&path).timeline.drop_import_to_timeline); + // a non-boolean is rejected and the stored value is kept + assert!(s.execute("prefs.set", json!({"key": "timeline.dropImportToTimeline", "value": "no"})).is_err()); + assert!(!s.prefs.timeline.drop_import_to_timeline); + std::fs::remove_dir_all(dir).unwrap(); +} + +/// Preferences files written before the key existed load it as on. +#[test] +fn drop_import_to_timeline_defaults_to_on_for_older_prefs() { + assert!(serde_json::from_value::(json!({})).unwrap().drop_import_to_timeline); + let old = Preferences::from_value(json!({"version": 2, "timeline": {"stillImageDuration": 3}})); + assert!(old.timeline.drop_import_to_timeline); + assert_eq!(old.timeline.still_image_duration, 3.0); +} + +/// The key is a Boolean field of the Timeline category in the settings registry. +#[test] +fn drop_import_to_timeline_is_in_the_settings_registry() { + let (cat, f) = settings::fields().into_iter().find(|(_, f)| f.key == "timeline.dropImportToTimeline").expect("registry field"); + assert_eq!(cat, "timeline"); + assert!(matches!(f.kind, Kind::Bool)); + assert_eq!(settings::field("timeline.dropImportToTimeline").map(|f| f.key), Some("timeline.dropImportToTimeline")); +} diff --git a/crates/ui-egui/src/i18n/es.tsv b/crates/ui-egui/src/i18n/es.tsv index a39dc82e..d51f4cf7 100644 --- a/crates/ui-egui/src/i18n/es.tsv +++ b/crates/ui-egui/src/i18n/es.tsv @@ -1251,6 +1251,7 @@ Active Monitor Monitor activo Add Añadir Add tracks automatically when editing source clips onto the Timeline Añadir pistas automáticamente al editar clips de origen en la línea de tiempo + Place files dropped onto the Timeline directly on the Timeline Colocar los archivos soltados en la línea de tiempo directamente en ella Additional Scale Escala adicional Additive Dissolve Disolución aditiva Additive Dissolve (Legacy) Disolución aditiva (heredada) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 83d18261..b846cc99 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -266,6 +266,8 @@ pub struct FilmcraftApp { /// The sequence whose view `ui.timeline` holds, and that view as it was last exchanged with /// `session.state.timeline_views` (see `sync_timeline_view`). timeline_view_of: Option, + /// Items imported by an OS file drop, to place on the Timeline at the drop point this frame. + pending_timeline_drop: Option<(Vec, egui::Pos2)>, timeline_view_last: Option, pub fps: f32, last_time: f64, @@ -453,6 +455,7 @@ impl FilmcraftApp { integrated_titlebar: false, last_timeline_width: 1000.0, timeline_view_of: None, + pending_timeline_drop: None, timeline_view_last: None, fps: 60.0, last_time: 0.0, @@ -1126,7 +1129,8 @@ impl FilmcraftApp { r } - /// Import dropped files. + /// Import dropped files. With Timeline ▸ drop imports to the timeline, the new items are also + /// queued to be placed at the pointer (see `timeline::interact`). fn handle_drops(&mut self, ctx: &egui::Context) { let dropped = ctx.input(|i| i.raw.dropped_files.clone()); let mut paths = Vec::new(); @@ -1137,7 +1141,18 @@ impl FilmcraftApp { } } if !paths.is_empty() { - let _ = self.session.execute("file.import", json!({"paths": paths, "bin": self.import_bin().0})); + let pointer = ctx.input(|i| i.pointer.hover_pos().or(i.pointer.latest_pos())); + let imported = self.session.execute("file.import", json!({"paths": paths, "bin": self.import_bin().0})); + if let Ok(v) = imported + && let Some(pos) = pointer + && self.session.prefs.timeline.drop_import_to_timeline + { + let items: Vec = + v["items"].as_array().map(|a| a.iter().filter_map(|i| i.as_u64()).map(filmcraft_engine::project::ItemId).collect()).unwrap_or_default(); + if !items.is_empty() { + self.pending_timeline_drop = Some((items, pos)); + } + } } } @@ -1366,6 +1381,8 @@ impl FilmcraftApp { state::Mode::Import => panels::import_mode::show(self, ui, body), state::Mode::Export => panels::export_mode::show(self, ui, body), } + // the Timeline has had its chance to place a file drop this frame; anything left stays a bin import + self.pending_timeline_drop = None; panels::dialogs::show(self, &ctx); if !self.ui.show_status_bar { // no bar to draw the job in, but a finished preview render still plays diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 811a75ee..6c649471 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -2186,9 +2186,50 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & } } } + // ---- OS file drops imported with "drop imports to the timeline": placed at the drop point, one after another + if let Some((items, p)) = app.pending_timeline_drop.clone() + && layout.content.contains(p) + { + app.pending_timeline_drop = None; + let row = layout.row_at(p.y).cloned().or_else(|| layout.rows.iter().find(|r| r.kind == TrackKind::Video).cloned()); + if let Some(row) = row { + let (vt, at) = match row.kind { + TrackKind::Video => (Some(row.track.0), seq.audio_tracks.get(row.index).or(seq.audio_tracks.first()).map(|t| t.id.0)), + TrackKind::Audio => (seq.video_tracks.get(row.index).or(seq.video_tracks.first()).map(|t| t.id.0), Some(row.track.0)), + }; + let still = app.session.prefs.timeline.still_duration(rate); + let durations: Vec = items + .iter() + .map(|&item| { + let is_still = app.session.project.item(item).and_then(|i| i.as_media()).is_some_and(|m| m.info.kind == filmcraft_media::MediaKind::Still); + app.session.project.item(item).map(|i| i.duration()).filter(|d| d.0 > 0 && !is_still).unwrap_or(still) + }) + .collect(); + let start = snap(app, seq, layout, rate.snap_nearest(layout.tick_at(p.x).max(Tick::ZERO)), &[]); + for (item, t) in items.iter().zip(chain_starts(start, &durations)) { + let params = json!({"item": item.0, "track": vt, "audioTrack": at, "time": t.0, "insert": false}); + if let Err(e) = app.session.execute("timeline.place", params) { + app.ui.status = e.to_string(); + } + } + } + } app.auto.add("timeline.tracks", layout.content, "tracks"); } +/// Start tick of each clip when the clips are laid end to end from `start`. Saturates rather than overflows. +pub(crate) fn chain_starts(start: Tick, durations: &[Tick]) -> Vec { + let mut t = start; + durations + .iter() + .map(|d| { + let at = t; + t = Tick(t.0.saturating_add(d.0.max(0))); + at + }) + .collect() +} + #[cfg(test)] mod waveform_tests { use super::waveform_display_gain; @@ -2211,3 +2252,41 @@ mod waveform_tests { assert!((db(waveform_display_gain(1.0, -6.0)) - (-6.0)).abs() < 0.1); } } + +#[cfg(test)] +mod chain_starts_tests { + use super::chain_starts; + use filmcraft_time::Tick; + + #[test] + fn empty_list_has_no_starts() { + assert!(chain_starts(Tick(100), &[]).is_empty()); + } + + #[test] + fn one_clip_starts_at_the_drop_point() { + assert_eq!(chain_starts(Tick(7), &[Tick(30)]), vec![Tick(7)]); + } + + #[test] + fn several_clips_are_laid_end_to_end() { + let durations = [Tick(10), Tick(25), Tick(5)]; + assert_eq!(chain_starts(Tick(100), &durations), vec![Tick(100), Tick(110), Tick(135)]); + } + + #[test] + fn start_near_i64_max_saturates_without_panicking() { + let starts = chain_starts(Tick(i64::MAX - 5), &[Tick(10), Tick(10), Tick(10)]); + assert_eq!(starts, vec![Tick(i64::MAX - 5), Tick(i64::MAX), Tick(i64::MAX)]); + } + + #[test] + fn zero_length_clips_share_one_start() { + assert_eq!(chain_starts(Tick(42), &[Tick(0), Tick(0), Tick(9)]), vec![Tick(42), Tick(42), Tick(42)]); + } + + #[test] + fn negative_durations_count_as_zero() { + assert_eq!(chain_starts(Tick(5), &[Tick(-3), Tick(4)]), vec![Tick(5), Tick(5)]); + } +} diff --git a/crates/ui-egui/tests/timeline_drop_ui.rs b/crates/ui-egui/tests/timeline_drop_ui.rs new file mode 100644 index 00000000..76bdde9f --- /dev/null +++ b/crates/ui-egui/tests/timeline_drop_ui.rs @@ -0,0 +1,154 @@ +//! Files dropped from the OS onto the Timeline. With Preferences ▸ Timeline ▸ "Place files dropped +//! onto the Timeline directly on the Timeline" (`drop_import_to_timeline`) they are also placed at +//! the pointer; with it off they are only imported to the bin. + +use std::path::{Path, PathBuf}; + +use egui_kittest::Harness; +use filmcraft_engine::Session; +use filmcraft_ui_egui::FilmcraftApp; +use serde_json::json; + +fn step(h: &mut Harness<'static, FilmcraftApp>) { + let ctx = h.ctx.clone(); + let mut raw = std::mem::take(h.input_mut()); + eframe::App::raw_input_hook(h.state_mut(), &ctx, &mut raw); + *h.input_mut() = raw; + ctx.request_repaint(); + h.step(); +} + +fn harness() -> Harness<'static, FilmcraftApp> { + let mut s = Session::default(); + s.execute("file.openDemoProject", json!({})).unwrap(); + s.execute("file.newSequence", json!({"name":"Drop test","width":1280,"height":720,"fps":24})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 980.0)).build_eframe(move |_cc| FilmcraftApp::new(s)); + for _ in 0..4 { + step(&mut h); + } + h +} + +fn pointer_to(h: &mut Harness<'static, FilmcraftApp>, pos: egui::Pos2) { + h.input_mut().events.push(egui::Event::PointerMoved(pos)); + step(h); +} + +fn control_rect(h: &Harness<'static, FilmcraftApp>, id: &str) -> egui::Rect { + let [x, y, w, height] = h.state().auto.elements.iter().find(|e| e.id == id).or_else(|| h.state().auto.find(id)).unwrap().rect; + egui::Rect::from_min_size(egui::pos2(x, y), egui::vec2(w, height)) +} + +/// A point 60 px into the Timeline's clip area on the named track row. +fn point_on_row(h: &Harness<'static, FilmcraftApp>, row_control: &str) -> egui::Pos2 { + let row = control_rect(h, row_control); + let panel = control_rect(h, "panel.Timeline"); + egui::pos2(panel.min.x + h.state().ui.timeline.header_w + 60.0, row.center().y) +} + +/// A file the OS dropped on the window, as the native integration hands it to egui. +#[derive(Debug)] +struct OsFile(PathBuf); + +impl egui::DroppedFile for OsFile { + fn path(&self) -> &Path { + &self.0 + } + + fn bytes(&self) -> Result, String> { + std::fs::read(&self.0).map_err(|e| e.to_string()) + } +} + +/// Drops `path` with the pointer over `row_control`, as the OS does: the pointer is hovering first, +/// then one frame carries the dropped file. The file is cleared afterwards, as egui only reports it once. +fn drop_on_row(h: &mut Harness<'static, FilmcraftApp>, row_control: &str, path: &Path) { + let at = point_on_row(h, row_control); + pointer_to(h, at); + h.input_mut().dropped_files.push(std::sync::Arc::new(OsFile(path.to_path_buf()))); + step(h); + h.input_mut().dropped_files.clear(); + step(h); +} + +/// (video clips, audio clips) on the active sequence. +fn clip_counts(h: &Harness<'static, FilmcraftApp>) -> (usize, usize) { + let q = h.state().session.active_sequence().unwrap(); + (q.video_tracks.iter().map(|t| t.items.len()).sum(), q.audio_tracks.iter().map(|t| t.items.len()).sum()) +} + +fn fixture(name: &str) -> PathBuf { + let dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../target/fixtures/ui-egui"); + std::fs::create_dir_all(&dir).unwrap(); + dir.join(name) +} + +/// A small generated PNG still. +fn still(name: &str) -> PathBuf { + let p = fixture(name); + image::RgbaImage::from_pixel(64, 36, image::Rgba([40, 140, 220, 255])).save(&p).unwrap(); + p +} + +/// A 0.5 s mono 48 kHz 16-bit WAV tone (audio only). +fn tone(name: &str) -> PathBuf { + let p = fixture(name); + let rate = 48_000u32; + let n = rate / 2; + let mut data = Vec::with_capacity(n as usize * 2); + for i in 0..n { + let s = (f32::sin(2.0 * std::f32::consts::PI * 440.0 * i as f32 / rate as f32) * 0.5 * i16::MAX as f32) as i16; + data.extend_from_slice(&s.to_le_bytes()); + } + let mut wav = Vec::with_capacity(44 + data.len()); + wav.extend_from_slice(b"RIFF"); + wav.extend_from_slice(&(36 + data.len() as u32).to_le_bytes()); + wav.extend_from_slice(b"WAVEfmt "); + wav.extend_from_slice(&16u32.to_le_bytes()); + wav.extend_from_slice(&1u16.to_le_bytes()); // PCM + wav.extend_from_slice(&1u16.to_le_bytes()); // mono + wav.extend_from_slice(&rate.to_le_bytes()); + wav.extend_from_slice(&(rate * 2).to_le_bytes()); + wav.extend_from_slice(&2u16.to_le_bytes()); + wav.extend_from_slice(&16u16.to_le_bytes()); + wav.extend_from_slice(b"data"); + wav.extend_from_slice(&(data.len() as u32).to_le_bytes()); + wav.extend_from_slice(&data); + std::fs::write(&p, wav).unwrap(); + p +} + +#[test] +fn still_dropped_on_the_video_row_is_placed_at_the_pointer() { + let mut h = harness(); + let items_before = h.state().session.project.items.len(); + drop_on_row(&mut h, "timeline.track.V1.target", &still("drop_still_video_row.png")); + assert_eq!(h.state().session.project.items.len(), items_before + 1, "the still is imported to the bin"); + assert_eq!(clip_counts(&h), (1, 0), "status: {}", h.state().ui.status); +} + +#[test] +fn still_dropped_on_an_audio_row_goes_to_the_video_track_without_error() { + // a video-only item over an audio row: the picture takes the matching video track, no error + let mut h = harness(); + drop_on_row(&mut h, "timeline.track.A1.target", &still("drop_still_audio_row.png")); + assert_eq!(clip_counts(&h), (1, 0), "status: {}", h.state().ui.status); +} + +#[test] +fn tone_dropped_on_a_video_row_goes_to_the_audio_track_without_error() { + // an audio-only item over a video row: the sound takes the matching audio track, no error + let mut h = harness(); + drop_on_row(&mut h, "timeline.track.V1.target", &tone("drop_tone_video_row.wav")); + assert_eq!(clip_counts(&h), (0, 1), "status: {}", h.state().ui.status); +} + +#[test] +fn with_drop_import_off_the_file_is_only_imported() { + let mut h = harness(); + h.state_mut().session.prefs.timeline.drop_import_to_timeline = false; + let items_before = h.state().session.project.items.len(); + drop_on_row(&mut h, "timeline.track.V1.target", &still("drop_still_pref_off.png")); + assert_eq!(h.state().session.project.items.len(), items_before + 1, "the still is imported to the bin"); + assert_eq!(clip_counts(&h), (0, 0), "status: {}", h.state().ui.status); +} From b630643ec945a8c6f4ee616217172132534eebb2 Mon Sep 17 00:00:00 2001 From: Mitch Date: Sat, 10 Oct 2026 14:21:49 +1100 Subject: [PATCH 25/35] #171: read the OS cursor for drops from the file manager winit on Windows sends no cursor events while files are dragged in, so the pointer was stale and the drop was never placed on the timeline. The desktop app now passes a host hook that reads the cursor from the OS (GetCursorPos), which the UI maps into window points. Co-Authored-By: Claude Sonnet 5.5 --- apps/filmcraft/src/main.rs | 1 + crates/platform/Cargo.toml | 1 + crates/platform/src/cursor.rs | 22 +++++++++++++++++++++ crates/platform/src/lib.rs | 1 + crates/ui-egui/src/lib.rs | 36 ++++++++++++++++++++++++++++++++++- 5 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 crates/platform/src/cursor.rs diff --git a/apps/filmcraft/src/main.rs b/apps/filmcraft/src/main.rs index 18fb4d7b..c14cb1c8 100644 --- a/apps/filmcraft/src/main.rs +++ b/apps/filmcraft/src/main.rs @@ -228,6 +228,7 @@ fn main() -> eframe::Result { Some(Box::new(|dir: &str| rfd::FileDialog::new().set_directory(dir).pick_folder().map(|p| p.to_string_lossy().into_owned()))); app.hooks.open_path = Some(Box::new(open_path)); // Settings ▸ General ▸ Interface Language ▸ System Language (#218). + app.hooks.cursor_screen_position = Some(Box::new(filmcraft_platform::cursor::cursor_screen_position)); app.hooks.system_languages = Some(Box::new(|| sys_locale::get_locales().collect())); app.hooks.raise_without_focus = Some(Box::new(|| { window_raise::raise_without_focus(); diff --git a/crates/platform/Cargo.toml b/crates/platform/Cargo.toml index 55e3be06..9ec69b93 100644 --- a/crates/platform/Cargo.toml +++ b/crates/platform/Cargo.toml @@ -50,6 +50,7 @@ windows = { version = "0.62", features = [ "Win32_Media_MediaFoundation", "Win32_System_Com", "Win32_System_LibraryLoader", + "Win32_UI_WindowsAndMessaging", ] } # Linux: libva (VA-API) is loaded at run time with libloading (ISC; already in the lockfile through diff --git a/crates/platform/src/cursor.rs b/crates/platform/src/cursor.rs new file mode 100644 index 00000000..aa238b84 --- /dev/null +++ b/crates/platform/src/cursor.rs @@ -0,0 +1,22 @@ +//! The mouse position in screen pixels, for when the windowing layer has none. +//! +//! While files are dragged in from the OS file manager, winit on Windows delivers no cursor events +//! (it only reports hovered and dropped files), so the UI cannot learn where a drop landed. + +/// The cursor's position on the virtual screen in physical pixels, or `None` where the OS cannot +/// say or this platform's windowing layer already reports it during a drag. +#[cfg(target_os = "windows")] +#[allow(unsafe_code)] +pub fn cursor_screen_position() -> Option<(i32, i32)> { + use windows::Win32::Foundation::POINT; + use windows::Win32::UI::WindowsAndMessaging::GetCursorPos; + let mut p = POINT { x: 0, y: 0 }; + // SAFETY: `p` is a valid, writable POINT for the duration of the call. + unsafe { GetCursorPos(&mut p) }.ok()?; + Some((p.x, p.y)) +} + +#[cfg(not(target_os = "windows"))] +pub fn cursor_screen_position() -> Option<(i32, i32)> { + None +} diff --git a/crates/platform/src/lib.rs b/crates/platform/src/lib.rs index da0164e2..93e89458 100644 --- a/crates/platform/src/lib.rs +++ b/crates/platform/src/lib.rs @@ -34,6 +34,7 @@ mod annexb; #[cfg_attr(not(any(target_os = "windows", target_os = "linux")), allow(dead_code))] mod biplanar; +pub mod cursor; #[cfg(target_os = "macos")] pub mod hardware_encode; pub mod hybrid; diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index b846cc99..1a297a96 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -104,6 +104,8 @@ pub struct AudioDevices { #[derive(Default)] pub struct HostHooks { pub pick_files: Option Vec>>, + /// The cursor in physical screen pixels. Where the windowing layer reports no pointer while files are dragged in from the OS (Windows), a drop is placed where this says it landed. + pub cursor_screen_position: Option Option<(i32, i32)>>>, pub pick_save: Option Option>>, pub pick_open_project: Option Option>>, /// Save dialog with a filter: (filter name, extensions, suggested file name) → path. @@ -1141,7 +1143,14 @@ impl FilmcraftApp { } } if !paths.is_empty() { - let pointer = ctx.input(|i| i.pointer.hover_pos().or(i.pointer.latest_pos())); + // The OS reports no cursor motion while files are dragged in (winit, Windows): ask it where the drop landed. + let os_pointer = self + .hooks + .cursor_screen_position + .as_ref() + .and_then(|f| f()) + .and_then(|px| ctx.input(|i| i.viewport().inner_rect.map(|r| screen_px_to_ui(px, i.pixels_per_point, r.min)))); + let pointer = os_pointer.or_else(|| ctx.input(|i| i.pointer.hover_pos().or(i.pointer.latest_pos()))); let imported = self.session.execute("file.import", json!({"paths": paths, "bin": self.import_bin().0})); if let Ok(v) = imported && let Some(pos) = pointer @@ -1929,3 +1938,28 @@ mod audio_recovery_tests { app.stop(); } } + +/// A physical-pixel screen position as a point in the window's UI space, given the window's content origin in points. +fn screen_px_to_ui(px: (i32, i32), pixels_per_point: f32, content_min: egui::Pos2) -> egui::Pos2 { + let ppp = if pixels_per_point.is_finite() && pixels_per_point > 0.0 { pixels_per_point } else { 1.0 }; + egui::pos2(px.0 as f32 / ppp - content_min.x, px.1 as f32 / ppp - content_min.y) +} + +#[cfg(test)] +mod screen_px_tests { + use super::screen_px_to_ui; + + #[test] + fn maps_screen_pixels_into_the_window() { + let p = screen_px_to_ui((1300, 900), 2.0, egui::pos2(100.0, 50.0)); + assert_eq!((p.x, p.y), (550.0, 400.0)); + } + + #[test] + fn a_hostile_scale_does_not_divide_by_zero() { + for ppp in [0.0, -1.0, f32::NAN, f32::INFINITY] { + let p = screen_px_to_ui((10, 20), ppp, egui::pos2(0.0, 0.0)); + assert!(p.x.is_finite() && p.y.is_finite()); + } + } +} From 88c9ec1701bb8d523ca3b2388411d7c3ed4590a8 Mon Sep 17 00:00:00 2001 From: Mitch Date: Sat, 10 Oct 2026 14:25:58 +1100 Subject: [PATCH 26/35] #171: preview where dragged files will land on the Timeline While files are dragged over the window from the OS, the Timeline shows a placeholder at the snapped drop point on the row under the cursor, labelled with the file count. The cursor is polled each frame because the OS sends no motion events during such a drag. Co-Authored-By: Claude Sonnet 5.5 --- crates/ui-egui/src/i18n/es.tsv | 2 ++ crates/ui-egui/src/lib.rs | 32 ++++++++++++++++++++------- crates/ui-egui/src/panels/timeline.rs | 15 +++++++++++++ 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/i18n/es.tsv b/crates/ui-egui/src/i18n/es.tsv index d51f4cf7..498f7966 100644 --- a/crates/ui-egui/src/i18n/es.tsv +++ b/crates/ui-egui/src/i18n/es.tsv @@ -1252,6 +1252,8 @@ Add Añadir Add tracks automatically when editing source clips onto the Timeline Añadir pistas automáticamente al editar clips de origen en la línea de tiempo Place files dropped onto the Timeline directly on the Timeline Colocar los archivos soltados en la línea de tiempo directamente en ella + 1 file 1 archivo + {n} files {n} archivos Additional Scale Escala adicional Additive Dissolve Disolución aditiva Additive Dissolve (Legacy) Disolución aditiva (heredada) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 1a297a96..861723fa 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -270,6 +270,8 @@ pub struct FilmcraftApp { timeline_view_of: Option, /// Items imported by an OS file drop, to place on the Timeline at the drop point this frame. pending_timeline_drop: Option<(Vec, egui::Pos2)>, + /// Files being dragged over the window from the OS (count, pointer): the Timeline previews where they would land. + file_drag_hover: Option<(usize, egui::Pos2)>, timeline_view_last: Option, pub fps: f32, last_time: f64, @@ -458,6 +460,7 @@ impl FilmcraftApp { last_timeline_width: 1000.0, timeline_view_of: None, pending_timeline_drop: None, + file_drag_hover: None, timeline_view_last: None, fps: 60.0, last_time: 0.0, @@ -1134,6 +1137,14 @@ impl FilmcraftApp { /// Import dropped files. With Timeline ▸ drop imports to the timeline, the new items are also /// queued to be placed at the pointer (see `timeline::interact`). fn handle_drops(&mut self, ctx: &egui::Context) { + // While files hover, the OS sends no cursor events: poll the cursor each frame to draw the Timeline preview. + let hovering = ctx.input(|i| i.raw.hovered_files.len()); + self.file_drag_hover = if hovering > 0 && self.session.prefs.timeline.drop_import_to_timeline { + ctx.request_repaint(); + self.drop_pointer(ctx).map(|p| (hovering, p)) + } else { + None + }; let dropped = ctx.input(|i| i.raw.dropped_files.clone()); let mut paths = Vec::new(); for f in dropped { @@ -1143,14 +1154,7 @@ impl FilmcraftApp { } } if !paths.is_empty() { - // The OS reports no cursor motion while files are dragged in (winit, Windows): ask it where the drop landed. - let os_pointer = self - .hooks - .cursor_screen_position - .as_ref() - .and_then(|f| f()) - .and_then(|px| ctx.input(|i| i.viewport().inner_rect.map(|r| screen_px_to_ui(px, i.pixels_per_point, r.min)))); - let pointer = os_pointer.or_else(|| ctx.input(|i| i.pointer.hover_pos().or(i.pointer.latest_pos()))); + let pointer = self.drop_pointer(ctx); let imported = self.session.execute("file.import", json!({"paths": paths, "bin": self.import_bin().0})); if let Ok(v) = imported && let Some(pos) = pointer @@ -1165,6 +1169,18 @@ impl FilmcraftApp { } } + /// Where files dragged in from the OS are over the window. The OS reports no cursor motion during such a drag + /// (winit, Windows), so ask it through the host hook, else use the last pointer position egui saw. + fn drop_pointer(&self, ctx: &egui::Context) -> Option { + let os_pointer = self + .hooks + .cursor_screen_position + .as_ref() + .and_then(|f| f()) + .and_then(|px| ctx.input(|i| i.viewport().inner_rect.map(|r| screen_px_to_ui(px, i.pixels_per_point, r.min)))); + os_pointer.or_else(|| ctx.input(|i| i.pointer.hover_pos().or(i.pointer.latest_pos()))) + } + // ---------------------------------------------------------------- input fn handle_shortcuts(&mut self, ctx: &egui::Context) { diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 6c649471..13f4f503 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -2214,6 +2214,21 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & } } } + // preview of OS files being dragged over the Timeline: a placeholder where the drop would start + if let Some((count, p)) = app.file_drag_hover + && layout.content.contains(p) + { + let row = layout.row_at(p.y).cloned().or_else(|| layout.rows.iter().find(|r| r.kind == TrackKind::Video).cloned()); + if let Some(row) = row { + let t = snap(app, seq, layout, rate.snap_nearest(layout.tick_at(p.x).max(Tick::ZERO)), &[]); + let x = layout.x_of(t); + let r = Rect::from_min_max(pos2(x, row.rect.min.y + 1.0), pos2((x + 120.0).min(layout.content.max.x), row.rect.max.y - 1.0)); + ui.painter().rect_filled(r, 3.0, Color32::from_white_alpha(40)); + ui.painter().rect_stroke(r, 3.0, Stroke::new(1.5, Color32::WHITE), StrokeKind::Inside); + let label = if count == 1 { tl!("1 file").to_string() } else { tlf!("{n} files", n = count) }; + ui.painter().text(r.left_top() + vec2(4.0, 2.0), Align2::LEFT_TOP, label, Tokens::ui(10.0), Color32::WHITE); + } + } app.auto.add("timeline.tracks", layout.content, "tracks"); } From ecfa560ed39dab5eb0c01c719c193de9619060ab Mon Sep 17 00:00:00 2001 From: Tommaso Maccagni Date: Sat, 10 Oct 2026 01:00:05 +0200 Subject: [PATCH 27/35] engine: effects.setParam takes a choice by name and can write a keyframe A choice parameter takes its option name as well as its index, matched without case, as transition parameters and graphics.set already do. "keyframe": true writes a keyframe at `time` and animates the parameter (the stopwatch and the value in one step, like clipMixer.set). Without it nothing changes: a parameter that is not animated gets a static value whatever the time. A parameter that cannot be animated refuses the keyframe. The result is {"keyframes": n} instead of null, so a caller can tell that a value sent with a time became a static value (0) and not a keyframe. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/commands.rs | 33 ++++++++++++-- crates/engine/src/tests.rs | 86 +++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index b5e4c847..d57b96c6 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -2391,13 +2391,14 @@ fn build() -> Vec { "Set Effect Parameter", [], None, - r##"{"clip":id,"effect":"motion"|index,"param":str,"mask":n?,"value":num|[x,y]|"#rrggbb"|bool|path,"time":ticks?,"merge":bool?,"begin":bool?}"##, + r##"{"clip":id,"effect":"motion"|index,"param":str,"mask":n?,"value":num|[x,y]|"#rrggbb"|bool|path|"option name" (a choice takes its index or its name),"time":ticks? (where the keyframe goes when the parameter is animated; a parameter that is not animated gets a new static value whatever the time),"keyframe":bool? (true: write a keyframe at `time` and animate the parameter),"merge":bool?,"begin":bool?}"##, has_seq, |s, p| { let c = clip_p(p, "clip").ok_or_else(|| bad("effects.setParam", "need `clip`"))?; let pid = str_p(p, "param").ok_or_else(|| bad("effects.setParam", "need `param`"))?.to_string(); let val = p.get("value").cloned().ok_or_else(|| bad("effects.setParam", "need `value`"))?; let eff = p.get("effect").cloned().unwrap_or(json!("motion")); + let keyframe = bool_p(p, "keyframe").unwrap_or(false); let ph = s.playhead(); let tl = time_p(s, p, "").unwrap_or(ph); let pq = p.clone(); @@ -2406,7 +2407,7 @@ fn build() -> Vec { if bool_p(p, "begin").unwrap_or(false) { s.history.merge_key = None; } - s.edit_sequence_as("Change Effect Parameter", merge.as_deref(), |q, _, _| { + let keyframes = s.edit_sequence_as("Change Effect Parameter", merge.as_deref(), |q, _, _| { let (_, it) = q.find_item_mut(c).ok_or(filmcraft_edit::EditError::NoItem(c))?; let mt = it.source_time_at(tl.clamp(it.start, (it.end() - Tick(1)).max(it.start))); let e = match &eff { @@ -2415,6 +2416,22 @@ fn build() -> Vec { _ => None, } .ok_or_else(|| bad("effects.setParam", "no such effect on clip"))?; + // the parameter's definition (a mask's own parameters have none) + let def = if pq.get("mask").is_none_or(Value::is_null) { e.def().and_then(|d| d.param(&pid)) } else { None }; + // a choice takes its option name as well as its index, as transition parameters do + let val = match (def.map(|d| &d.kind), &val) { + (Some(filmcraft_project::ParamKind::Choice(opts)), Value::String(name)) => { + let i = opts + .iter() + .position(|o| o.eq_ignore_ascii_case(name)) + .ok_or_else(|| bad("effects.setParam", format!("`{pid}` must be one of {opts:?}")))?; + json!(i) + } + _ => val.clone(), + }; + if keyframe && def.is_some_and(|d| !d.animatable) { + return Err(bad("effects.setParam", format!("`{pid}` cannot be animated"))); + } if eff == json!("opacity") || eff == json!("motion") { e.enabled = true; } @@ -2426,10 +2443,16 @@ fn build() -> Vec { } let prm = crate::masks::target_param(e, &pq, &pid).ok_or_else(|| bad("effects.setParam", format!("no param `{pid}`")))?; let v = json_to_param(&prm.value, &val).ok_or_else(|| bad("effects.setParam", "value has the wrong type"))?; - prm.set_at(mt, v); - Ok(()) + if keyframe { + // the stopwatch and the value in one step + prm.put_keyframe(mt, v); + } else { + prm.set_at(mt, v); + } + Ok(prm.keyframes.len()) })?; - Ok(Value::Null) + // 0 keyframes: the value is static (`time` was not used) + Ok(json!({"keyframes": keyframes})) } ), cmd!("effects.toggleAnimation", "Toggle Animation", [], None, r#"{"clip":id,"effect":str|index,"param":str,"mask":n?}"#, has_seq, |s, p| { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 01d5b813..5143fdf1 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -282,6 +282,92 @@ fn add_keyframe_toggles_the_keyframe_at_the_playhead() { assert_eq!(scale(&s), (0, 100.0)); } +/// `effects.setParam` writes a keyframe only on an animated parameter. An agent that sends a value +/// with a `time` to a parameter that is not animated gets a static value (the result says so: +/// 0 keyframes); `"keyframe": true` is the stopwatch and the value in one step. +#[test] +fn set_param_writes_a_keyframe_when_asked_or_animated() { + let mut s = demo(); + let c = s.active_sequence().unwrap().video_tracks[0].items[0].id.0; + // (keyframes, static value, value at 0.5 s, value at 2.5 s) + let scale = |s: &Session| { + let it = s.active_sequence().unwrap().find_item(filmcraft_project::ClipId(c)).unwrap().1; + let p = &it.effect("motion").unwrap().params["scale"]; + let at = |seconds: f64| p.f64_at(it.source_time_at(Tick::from_seconds_f64(seconds))); + (p.keyframes.len(), p.value.as_f64().unwrap(), at(0.5), at(2.5)) + }; + let set = |value: f64, seconds: f64| json!({"clip": c, "effect": "motion", "param": "scale", "value": value, "seconds": seconds}); + // not animated: the time is not used and the value is static + let r = s.execute("effects.setParam", set(80.0, 0.5)).unwrap(); + assert_eq!(r["keyframes"], 0, "{r}"); + assert_eq!(scale(&s), (0, 80.0, 80.0, 80.0)); + // `keyframe`: the parameter becomes animated at that time + let mut first = set(60.0, 0.5); + first["keyframe"] = json!(true); + let r = s.execute("effects.setParam", first).unwrap(); + assert_eq!(r["keyframes"], 1, "{r}"); + assert_eq!(scale(&s).0, 1); + // animated now, so a plain set at another time is a second keyframe + let r = s.execute("effects.setParam", set(120.0, 2.5)).unwrap(); + assert_eq!(r["keyframes"], 2, "{r}"); + let (n, _, a, b) = scale(&s); + assert_eq!((n, a, b), (2, 60.0, 120.0)); + // a keyframe that is already there is replaced, not doubled + let mut again = set(70.0, 0.5); + again["keyframe"] = json!(true); + assert_eq!(s.execute("effects.setParam", again).unwrap()["keyframes"], 2); + assert_eq!(scale(&s).2, 70.0); + s.undo(); + assert_eq!(scale(&s).2, 60.0); + s.undo(); + assert_eq!(scale(&s).0, 1); + s.redo(); + s.redo(); + let (n, _, a, b) = scale(&s); + assert_eq!((n, a, b), (2, 70.0, 120.0)); + // a parameter that cannot be animated refuses the keyframe and keeps its value + let blend = json!({"clip": c, "effect": "opacity", "param": "blend", "value": 8, "keyframe": true}); + let e = s.execute("effects.setParam", blend).unwrap_err().to_string(); + assert!(e.contains("cannot be animated"), "{e}"); + let it = s.active_sequence().unwrap().find_item(filmcraft_project::ClipId(c)).unwrap().1; + assert_eq!(it.effect("opacity").unwrap().params["blend"].value, filmcraft_project::ParamValue::Choice(0)); + // `keyframe` that is not a bool means no keyframe request + let mut odd = set(70.0, 0.5); + odd["keyframe"] = json!("yes"); + assert_eq!(s.execute("effects.setParam", odd).unwrap()["keyframes"], 2); +} + +/// A choice parameter takes its option name as well as its index, as transition parameters do +/// (`effects.list {"detail": true}` lists the names). +#[test] +fn set_param_takes_a_choice_by_name() { + use filmcraft_project::ParamValue; + let mut s = demo(); + let c = s.active_sequence().unwrap().video_tracks[0].items[0].id.0; + let blend = + |s: &Session| s.active_sequence().unwrap().find_item(filmcraft_project::ClipId(c)).unwrap().1.effect("opacity").unwrap().params["blend"].value.clone(); + let set = |value: serde_json::Value| json!({"clip": c, "effect": "opacity", "param": "blend", "value": value}); + s.execute("effects.setParam", set(json!("Screen"))).unwrap(); + assert_eq!(blend(&s), ParamValue::Choice(8)); + s.execute("effects.setParam", set(json!("linear dodge (add)"))).unwrap(); + assert_eq!(blend(&s), ParamValue::Choice(10), "names are matched without case"); + s.execute("effects.setParam", set(json!(12))).unwrap(); + assert_eq!(blend(&s), ParamValue::Choice(12), "an index works as before"); + s.undo(); + assert_eq!(blend(&s), ParamValue::Choice(10)); + s.redo(); + assert_eq!(blend(&s), ParamValue::Choice(12)); + // a name that is not an option is an error that lists the options and changes nothing + let e = s.execute("effects.setParam", set(json!("Glow"))).unwrap_err().to_string(); + assert!(e.contains("Screen") && e.contains("Overlay"), "{e}"); + for bad in [json!(""), json!(null), json!([8]), json!({"name": "Screen"}), json!(-1), json!(1.5)] { + assert!(s.execute("effects.setParam", set(bad.clone())).is_err(), "{bad}"); + } + assert_eq!(blend(&s), ParamValue::Choice(12)); + // a name is still the wrong type for a parameter that is not a choice + assert!(s.execute("effects.setParam", json!({"clip": c, "effect": "motion", "param": "scale", "value": "Screen"})).is_err()); +} + #[test] fn transitions_and_markers() { let mut s = demo(); From 24f8279327cdc0a47a20092b075b23b4ff5118c8 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 10 Oct 2026 05:42:52 -0400 Subject: [PATCH 28/35] FCP7: trackindex 1 means channel 0 when the file is split across mono tracks (#463) A split-stereo left clip exported trackindex 1 and re-imported as full stereo, losing the left/right split. Treat trackindex 1 as channel 0 when another audio clip item of the same file in the sequence uses a higher index. Co-Authored-By: Claude Opus 5.5 --- crates/interchange/src/fcp7.rs | 33 ++++++++++++++++++++++++++++---- crates/interchange/tests/fcp7.rs | 33 ++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/crates/interchange/src/fcp7.rs b/crates/interchange/src/fcp7.rs index f1af5aaf..d5de669d 100644 --- a/crates/interchange/src/fcp7.rs +++ b/crates/interchange/src/fcp7.rs @@ -110,6 +110,24 @@ struct Imp<'a, 'i, 'r> { in_progress: HashSet, links: Vec, xml_clip_ids: HashMap<(ItemId, String), ClipId>, + /// `(sequence key, file key)` pairs where some audio clip item of that file uses `sourcetrack/trackindex` > 1, + /// i.e. the file's channels are split onto mono tracks in that sequence (#463). + split_files: HashSet<(String, String)>, +} + +/// The `(sequence key, file key)` an audio clip item belongs to, if it references a file. +fn clip_seq_file(c: Node) -> Option<(String, String)> { + let seq = c.ancestors().skip(1).find(|a| a.has_tag_name("sequence"))?; + Some((node_key(seq), node_key(child(c, "file")?))) +} + +/// The 1-based `sourcetrack/trackindex` of an audio clip item. +fn audio_track_index(c: Node) -> Option { + let st = child(c, "sourcetrack")?; + if child_text(st, "mediatype").is_some_and(|m| m != "audio") { + return None; + } + child_i64(st, "trackindex") } fn node_key(n: Node) -> String { @@ -136,9 +154,15 @@ pub(crate) fn import(text_in: &str, opts: &ImportOptions, report: &mut Report) - in_progress: HashSet::new(), links: Vec::new(), xml_clip_ids: HashMap::new(), + split_files: HashSet::new(), }; for n in root.descendants().filter(|n| n.is_element()) { match n.tag_name().name() { + "clipitem" if audio_track_index(n).is_some_and(|i| i > 1) => { + if let Some(key) = clip_seq_file(n) { + imp.split_files.insert(key); + } + } "file" if elements(n).next().is_some() => { imp.files.entry(node_key(n)).or_insert(n); } @@ -483,11 +507,12 @@ impl<'a, 'i> Imp<'a, 'i, '_> { ti.label = l; } // `sourcetrack/trackindex` (1-based) picks the source channel of a mono track split off a multichannel file (#463). - // Index 1 is also what a stereo clip on a stereo track carries, so it keeps the default (empty) channel mapping. + // Index 1 is also what a stereo clip on a stereo track carries, so it means channel 0 only when another clip + // item of the same file in this sequence uses a higher index (the file is split); otherwise it keeps the + // default (empty) channel mapping. if kind == TrackKind::Audio - && let Some(st) = child(c, "sourcetrack") - && let Some(idx) = child_i64(st, "trackindex") - && idx > 1 + && let Some(idx) = audio_track_index(c) + && (idx > 1 || (idx == 1 && clip_seq_file(c).is_some_and(|k| self.split_files.contains(&k)))) && let Ok(ch) = u16::try_from(idx - 1) { ti.source_channels = vec![ch]; diff --git a/crates/interchange/tests/fcp7.rs b/crates/interchange/tests/fcp7.rs index 8d36da38..6b2a9414 100644 --- a/crates/interchange/tests/fcp7.rs +++ b/crates/interchange/tests/fcp7.rs @@ -546,6 +546,7 @@ fn split_mono_tracks_keep_their_source_channel() { assert!(xml.contains("2"), "{xml}"); let seq = imp.project.sequence(only_seq(&imp)).unwrap(); let channels = |t: usize| seq.tracks(TrackKind::Audio)[t].items.iter().map(|i| i.source_channels.clone()).collect::>(); + assert_eq!(channels(0), vec![vec![0u16]], "{xml}"); assert_eq!(channels(1), vec![vec![1u16]], "{xml}"); let (bytes, _) = export(&imp.project, only_seq(&imp), Format::Fcp7Xml, &ExportOptions::default()).expect("export"); assert!(String::from_utf8(bytes).unwrap().contains("2")); @@ -567,3 +568,35 @@ fn audio_sourcetrack_trackindex_2_imports_as_source_channel_1() { let seq = imp.project.sequence(only_seq(&imp)).unwrap(); assert_eq!(seq.tracks(TrackKind::Audio)[0].items[0].source_channels, vec![1u16]); } + +/// #463: Premiere writes a stereo file split onto two mono tracks as clip items with `trackindex` 1 and 2; +/// a file used only with `trackindex` 1 is a stereo clip and keeps the default (all channels) mapping. +#[test] +fn audio_trackindex_1_is_channel_0_only_when_the_file_is_split() { + let doc = format!( + r#" + +S48{RATE} + +"# + ); + let (imp, _) = import(doc.as_bytes(), Format::Fcp7Xml, None).unwrap(); + let seq = imp.project.sequence(only_seq(&imp)).unwrap(); + let a1 = &seq.tracks(TrackKind::Audio)[0].items; + assert_eq!(a1[0].source_channels, vec![0u16]); + assert!(a1[1].source_channels.is_empty(), "{:?}", a1[1].source_channels); + assert_eq!(seq.tracks(TrackKind::Audio)[1].items[0].source_channels, vec![1u16]); +} From e0d2c9c73b857af24891af90ee21d03d322cb862 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 10 Oct 2026 05:49:28 -0400 Subject: [PATCH 29/35] MCP media_import: test the paths schema and param mapping; update docs/agents.md (#499) Co-Authored-By: Claude Opus 5.5 --- crates/automation/src/server.rs | 36 +++++++++++++++++++++++++-------- docs/agents.md | 2 +- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/crates/automation/src/server.rs b/crates/automation/src/server.rs index 2d9a1ad7..0aebbc31 100644 --- a/crates/automation/src/server.rs +++ b/crates/automation/src/server.rs @@ -151,6 +151,18 @@ pub struct MediaImportParams { pub image_sequence: Option, } +/// `file.import` params for a `media_import` call. +fn media_import_params(p: MediaImportParams) -> Value { + let mut params = json!({"paths": p.paths}); + if let Some(bin) = p.bin { + params["bin"] = json!(bin); + } + if let Some(seq) = p.image_sequence { + params["imageSequence"] = json!(seq); + } + params +} + #[derive(Debug, Default, Deserialize, JsonSchema)] pub struct ElementsParams { /// Only element ids starting with this prefix (e.g. `timeline.clip.`). @@ -394,14 +406,7 @@ impl FilmcraftMcp { annotations(read_only_hint = false, destructive_hint = false, idempotent_hint = false, open_world_hint = false) )] async fn media_import(&self, Parameters(p): Parameters) -> Result { - let mut params = json!({"paths": p.paths}); - if let Some(bin) = p.bin { - params["bin"] = json!(bin); - } - if let Some(seq) = p.image_sequence { - params["imageSequence"] = json!(seq); - } - wrap(self.run("file.import", params).await) + wrap(self.run("file.import", media_import_params(p)).await) } #[tool( @@ -753,6 +758,21 @@ mod tests { } } + /// #499: `media_import` takes `paths` (a required array), and forwards `bin` / `image_sequence` to `file.import`. + #[test] + fn media_import_schema_and_params() { + let m = FilmcraftMcp::headless(Session::default()); + let def = m.tool_router.get("media_import").unwrap(); + let schema = Value::Object((*def.input_schema).clone()); + assert_eq!(schema["properties"]["paths"]["type"], "array", "{schema}"); + assert!(schema["required"].as_array().unwrap().iter().any(|r| r == "paths"), "{schema}"); + let p: MediaImportParams = serde_json::from_value(json!({"paths": ["/a.png"], "bin": 7, "image_sequence": true})).unwrap(); + assert_eq!(media_import_params(p), json!({"paths": ["/a.png"], "bin": 7, "imageSequence": true})); + let p: MediaImportParams = serde_json::from_value(json!({"paths": ["/a.wav"]})).unwrap(); + assert_eq!(media_import_params(p), json!({"paths": ["/a.wav"]})); + assert!(serde_json::from_value::(json!({"text": "/a.wav"})).is_err()); + } + fn demo() -> Session { let mut s = Session::default(); s.execute("file.openDemoProject", json!({})).unwrap(); diff --git a/docs/agents.md b/docs/agents.md index 39d53f77..753bfdd5 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -61,7 +61,7 @@ claude mcp add filmcraft -- filmcraft-cli mcp | `render_preview` | both | same as `render_frame` | | `project_inspect` | both | bins and items with ids, types, durations; active sequence | | `sequence_inspect` | both | the active sequence: tracks, clips (`start` / `end` and `sourceIn` / `sourceOut` in ticks, frames, `speed`, `reverse`: the clip plays its `sourceIn` to `sourceOut` stretch backward, `gainDb`), effects, transitions, markers (name and `comment`), playhead, selection | -| `media_import` | both | import files by absolute path (`text`, one path per line) | +| `media_import` | both | import files by absolute path (`paths`, an array; optional `bin`, `image_sequence`) | | `render_frame` | both | PNG of the program frame at `seconds` (headless renders; bridge screenshots the Program monitor). Read-only: the playhead and selection are left as they were | | `ui_inspect` | bridge | UI state: tool, workspace, panels, zoom, playback, fps | | `ui_elements` | bridge | on-screen interactive elements with id, label and rect (`prefix` filter) | From 81268330e90a06f3e97ce74ed2b5495dbfb5fafc Mon Sep 17 00:00:00 2001 From: gyro Date: Fri, 9 Oct 2026 21:32:42 -0700 Subject: [PATCH 30/35] Essential Graphics: Make a property drag one undo step --- crates/engine/src/graphic_templates.rs | 2 +- crates/engine/src/graphics.rs | 22 ++++--- crates/engine/src/graphics_tests.rs | 32 ++++++++++ crates/ui-egui/src/panels/graphics.rs | 12 +++- crates/ui-egui/tests/graphics_ui.rs | 84 ++++++++++++++++++++++++++ docs/graphics.md | 2 +- 6 files changed, 143 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/graphic_templates.rs b/crates/engine/src/graphic_templates.rs index db00d6ee..8da84862 100644 --- a/crates/engine/src/graphic_templates.rs +++ b/crates/engine/src/graphic_templates.rs @@ -291,7 +291,7 @@ fn set_controls(s: &mut Session, clip: ClipId, values: &serde_json::Map Result; @@ -255,10 +255,11 @@ fn apply_props(e: &mut EffectInstance, props: &serde_json::Map, m } /// Set properties `props` on a layer (keyframe-aware at time `tl`). Changing the text keeps -/// per-character styles on their characters. -pub(crate) fn set_props(s: &mut Session, clip: ClipId, eidx: usize, props: &serde_json::Map, tl: Tick, label: &str) -> Result<()> { +/// per-character styles on their characters. `merge` folds consecutive calls with the same key +/// into one undo step (a drag is one undoable change, like `effects.setParam`). +pub(crate) fn set_props(s: &mut Session, clip: ClipId, eidx: usize, props: &serde_json::Map, tl: Tick, label: &str, merge: Option<&str>) -> Result<()> { let props = props.clone(); - s.edit_sequence(label, |q, _, _| { + s.edit_sequence_as(label, merge, |q, _, _| { let (_, it) = q.find_item_mut(clip).ok_or(filmcraft_edit::EditError::NoItem(clip))?; let mt = it.source_time_at(tl.clamp(it.start, it.end() - Tick(1))); let e = it.effects.get_mut(eidx).ok_or_else(|| bad("graphics.set", "no such layer"))?; @@ -737,7 +738,7 @@ fn menu_commands() -> Vec { props.insert("sides".into(), json!(n)); let ph = s.playhead(); let before = s.history.undo.len(); - set_props(s, clip, ei, &props, ph, "Change Graphic Property")?; + set_props(s, clip, ei, &props, ph, "Change Graphic Property", None)?; if s.history.undo.len() > before { s.history.undo.pop(); } @@ -956,7 +957,7 @@ pub fn commands() -> Vec { let mut props = serde_json::Map::new(); props.insert("text".into(), Value::String(text)); let ph = s.playhead(); - set_props(s, clip, ei, &props, ph, "Edit Text")?; + set_props(s, clip, ei, &props, ph, "Edit Text", None)?; if merge && s.history.undo.len() >= 2 { // keep the snapshot from before the typing session s.history.undo.pop(); @@ -969,14 +970,19 @@ pub fn commands() -> Vec { "Set Graphic Properties", &[], None, - r##"{"clip":id?,"layer":n|name?,"props":{"font":"Inter","font_style":"Bold","size":120,"align":"center","tracking":50,"leading":0,"fill_color":"#ffcc00","stroke":true,"stroke_width":6,"background":true,"shadow":true,"position":[x,y],"scale":100,"rotation":0,"opacity":100,…},"time":ticks?}"##, + r##"{"clip":id?,"layer":n|name?,"props":{"font":"Inter","font_style":"Bold","size":120,"align":"center","tracking":50,"leading":0,"fill_color":"#ffcc00","stroke":true,"stroke_width":6,"background":true,"shadow":true,"position":[x,y],"scale":100,"rotation":0,"opacity":100,…},"time":ticks?,"merge":bool?,"begin":bool?}"##, has_graphic, |s, p| { let clip = target_clip(s, p).ok_or_else(|| bad("graphics.set", "no graphic clip"))?; let (_, ei) = layer_effect_index(s, clip, p)?; let props = p.get("props").and_then(Value::as_object).ok_or_else(|| bad("graphics.set", "need `props`"))?.clone(); let tl = time_p(s, p, "").unwrap_or_else(|| s.playhead()); - set_props(s, clip, ei, &props, tl, "Change Graphic Property")?; + // `merge`: a drag of a property is one undo step; `begin` starts a new one (#201) + let merge = bool_p(p, "merge").unwrap_or(false).then(|| format!("graphics.set:{}:{ei}", clip.0)); + if bool_p(p, "begin").unwrap_or(false) { + s.history.merge_key = None; + } + set_props(s, clip, ei, &props, tl, "Change Graphic Property", merge.as_deref())?; Ok(Value::Null) }, ), diff --git a/crates/engine/src/graphics_tests.rs b/crates/engine/src/graphics_tests.rs index 0c7cc4cb..ac219f46 100644 --- a/crates/engine/src/graphics_tests.rs +++ b/crates/engine/src/graphics_tests.rs @@ -123,6 +123,38 @@ fn typing_coalesces_into_one_undo_step_and_props_set() { assert_eq!(text_of(&layers(&s, clip)[0]), ""); } +/// A drag of a property is one undo step: `graphics.set` with `merge` folds consecutive calls +/// into the step the first change of the press began (`begin`), so one undo restores the +/// original value (the color picker in Essential Graphics was committing one step per frame). +#[test] +fn a_property_drag_is_one_undo_step() { + let mut s = demo(); + let r = s.execute("graphics.newText", json!({"text": "Title"})).unwrap(); + let clip = ClipId(r["clip"].as_u64().unwrap()); + s.execute("graphics.set", json!({"clip": clip.0, "layer": 0, "props": {"shadow_color": [0.0, 0.0, 0.0, 1.0]}})).unwrap(); + let color = |s: &Session| -> [f32; 4] { + let q = s.active_sequence().unwrap(); + let (_, it) = q.find_item(clip).unwrap(); + let e = it.effects.iter().find(|e| e.effect == "graphic_text").unwrap(); + e.param("shadow_color").unwrap().value_at(Tick::ZERO).as_color().unwrap() + }; + let before = s.history.undo.len(); + // the drag: the first change of the press begins the step, the rest fold into it + for (i, c) in [[0.1, 0.0, 0.0, 1.0], [0.2, 0.0, 0.0, 1.0], [0.3, 0.0, 0.0, 1.0], [0.4, 0.0, 0.0, 1.0]].iter().enumerate() { + s.execute("graphics.set", json!({"clip": clip.0, "layer": 0, "props": {"shadow_color": c}, "merge": true, "begin": i == 0})).unwrap(); + } + assert_eq!(s.history.undo.len(), before + 1, "one undo step for the whole drag"); + assert_eq!(color(&s)[0], 0.4); + // a second drag (a new press) begins its own step + s.execute("graphics.set", json!({"clip": clip.0, "layer": 0, "props": {"shadow_color": [1.0, 0.0, 0.0, 1.0]}, "merge": true, "begin": true})).unwrap(); + s.execute("graphics.set", json!({"clip": clip.0, "layer": 0, "props": {"shadow_color": [0.0, 1.0, 0.0, 1.0]}, "merge": true})).unwrap(); + assert_eq!(s.history.undo.len(), before + 2, "a new press begins a new step"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(color(&s)[0], 0.4, "undo takes back only the second drag"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(color(&s)[0], 0.0, "and then the first"); +} + #[test] fn align_and_distribute_layers() { let mut s = demo(); diff --git a/crates/ui-egui/src/panels/graphics.rs b/crates/ui-egui/src/panels/graphics.rs index 46feeb4c..f3594015 100644 --- a/crates/ui-egui/src/panels/graphics.rs +++ b/crates/ui-egui/src/panels/graphics.rs @@ -1575,7 +1575,17 @@ pub fn properties(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { for (id, r, label) in autos { app.auto.add(&id, r, &label); } - for (cmd, p) in actions { + for (cmd, mut p) in actions { + // a drag of a property is one undo step: the first change of each press begins a new one, + // the rest fold into it (like Effect Controls, #201) + if cmd == "graphics.set" && ui.ctx().input(|i| i.pointer.any_down()) { + let key = egui::Id::new("gfx-props-drag-step"); + let press = ui.ctx().input(|i| i.pointer.press_start_time()); + let begun = ui.ctx().data(|d| d.get_temp::>(key)).flatten(); + p["merge"] = json!(true); + p["begin"] = json!(begun != press); + ui.ctx().data_mut(|d| d.insert_temp(key, press)); + } if let Err(e) = app.session.execute(&cmd, p) { app.ui.status = e.to_string(); } diff --git a/crates/ui-egui/tests/graphics_ui.rs b/crates/ui-egui/tests/graphics_ui.rs index b866b9bf..8da4e406 100644 --- a/crates/ui-egui/tests/graphics_ui.rs +++ b/crates/ui-egui/tests/graphics_ui.rs @@ -152,3 +152,87 @@ fn type_tool_edit_style_and_move() { d.ok("ui.set", json!({"tool": "Selection"})); d.shot("shape-layer"); } + + +/// A drag of a property in the Properties panel is one undo step, so one Cmd+Z restores the +/// original value (a drag was committing one undo step per frame, so undo only stepped back one +/// frame's worth; the shadow color picker showed it as "sometimes undo works, sometimes not"). +#[test] +fn a_property_drag_is_one_undo_step() { + let mut d = Driver::demo(); + d.ok("ui.set", json!({"workspace": "Captions and Graphics"})); + let r = d.exec("graphics.newText", json!({"text": "Title"})); + let clip = r["clip"].as_u64().unwrap(); + d.exec("graphics.selectLayer", json!({"clip": clip, "layers": [0]})); + d.frames(4); + // the layer's Opacity number (visible in the Align and Transform section) + d.exec("graphics.set", json!({"clip": clip, "layer": 0, "props": {"opacity": 50}})); + d.frames(2); + let opacity = |d: &mut Driver| -> f64 { + let seq = d.exec("sequence.inspect", json!({})); + let item = seq["video"].as_array().unwrap().iter().flat_map(|t| t["items"].as_array().unwrap().iter()).find(|i| i["clip"].as_u64() == Some(clip)).unwrap(); + let e = item["effects"].as_array().unwrap().iter().find(|e| e["effect"] == "graphic_text").unwrap(); + let v = e["params"]["opacity"]["value"].as_str().unwrap(); + v.trim_start_matches("Float(").trim_end_matches(')').parse::().unwrap() + }; + let before = opacity(&mut d); + let undo0 = d.exec("history.list", json!({}))["undo"].as_array().unwrap().len(); + // drag the Opacity number to the left: several move frames, one undo step + let n = d.rect("graphics.prop.opacity"); + let (cx, cy) = (n[0] + n[2] / 2.0, n[1] + n[3] / 2.0); + d.ok("ui.drag", json!({"from": {"x": cx, "y": cy}, "to": {"x": cx - 60.0, "y": cy}, "steps": 6})); + d.frames(4); + let after = opacity(&mut d); + assert!(after < before - 5.0, "the drag changed the opacity: {before} -> {after}"); + let undo1 = d.exec("history.list", json!({}))["undo"].as_array().unwrap().len(); + assert_eq!(undo1, undo0 + 1, "a property drag is one undo step"); + d.exec("edit.undo", json!({})); + assert_eq!(opacity(&mut d), before, "one undo restores the original value"); +} + + + +/// The shadow color picker popup: a drag in the 2D picker is one undo step, so one Cmd+Z +/// restores the original color (a drag was committing one undo step per frame, so undo only +/// stepped back one frame's worth — the reported "sometimes undo works, sometimes not"). +#[test] +fn shadow_color_drag_is_one_undo_step() { + let mut d = Driver::demo(); + d.ok("ui.set", json!({"workspace": "Captions and Graphics"})); + let r = d.exec("graphics.newText", json!({"text": "Title"})); + let clip = r["clip"].as_u64().unwrap(); + d.exec("graphics.selectLayer", json!({"clip": clip, "layers": [0]})); + d.frames(4); + let color = |d: &mut Driver| -> String { + let seq = d.exec("sequence.inspect", json!({})); + let item = seq["video"].as_array().unwrap().iter().flat_map(|t| t["items"].as_array().unwrap().iter()).find(|i| i["clip"].as_u64() == Some(clip)).unwrap(); + let e = item["effects"].as_array().unwrap().iter().find(|e| e["effect"] == "graphic_text").unwrap(); + e["params"].get("shadow_color").and_then(|p| p["value"].as_str()).unwrap_or("Color([0.0, 0.0, 0.0, 1.0])").to_string() + }; + // scroll the properties panel so the shadow row is visible, then open the picker + d.ok("ui.scroll", json!({"x": 1400.0, "y": 500.0, "dx": 0.0, "dy": -400.0})); + d.frames(3); + let sw = d.rect("graphics.prop.shadow_color"); + d.ok("ui.click", json!({"x": sw[0] + sw[2] / 2.0, "y": sw[1] + sw[3] / 2.0})); + d.frames(3); + // the popup flips above the swatch (it does not fit below); find a point in its 2D picker + let undo0 = d.exec("history.list", json!({}))["undo"].as_array().unwrap().len(); + let mut hit = None; + for (dx, dy) in [(30.0, 30.0), (150.0, 30.0), (30.0, 150.0), (150.0, 150.0)] { + let (px, py) = (sw[0] + dx, sw[1] - dy); + let before = color(&mut d); + d.ok("ui.drag", json!({"from": {"x": px, "y": py}, "to": {"x": px + 60.0, "y": py + 40.0}, "steps": 4})); + d.frames(3); + let after = color(&mut d); + if after != before { + hit = Some((before, after)); + break; + } + } + let (before, after) = hit.expect("no drag point hit the color picker"); + assert_ne!(after, before, "the drag changed the shadow color"); + let undo1 = d.exec("history.list", json!({}))["undo"].as_array().unwrap().len(); + assert_eq!(undo1, undo0 + 1, "a color drag is one undo step"); + d.exec("edit.undo", json!({})); + assert_eq!(color(&mut d), before, "one undo restores the original color"); +} diff --git a/docs/graphics.md b/docs/graphics.md index 6d935e82..c7dce451 100644 --- a/docs/graphics.md +++ b/docs/graphics.md @@ -89,7 +89,7 @@ Timecode and Clip Name effects also draw with the text engine. | `graphics.newShape` | (agents) | `shape` (rectangle/ellipse/polygon/path), `position`, `size`, `points`, `clip` | | `graphics.setTextType` | Text Properties ▸ Text Layer Type | `clip`, `layer`, `type` (`point` / `paragraph`); the text stays where it is | | `graphics.setText` | typing on the monitor | `clip`, `layer`, `text`, `merge` (coalesce one typing session into one undo step) | -| `graphics.set` | Properties panel | `clip`, `layer`, `props` {parameter id or camelCase alias: value; choices by index or name}, `time` | +| `graphics.set` | Properties panel | `clip`, `layer`, `props` {parameter id or camelCase alias: value; choices by index or name}, `time`, `merge` + `begin` (a drag is one undo step: send `merge: true` on every change and `begin: true` on the first change of each press) | | `graphics.selectLayer` | layer list / monitor click | `clip`, `layers` | | `graphics.deleteLayer`, `graphics.arrangeLayer` | layer list | `clip`, `layer`, `to` (front/back/forward/backward/index) | | `graphics.align` | Align and Transform | `align` (left/hcenter/right/top/vcenter/bottom), `to` (frame/group/selection), `layers` | From 19c756c252df50e8e4b9db7af79caa7281240db4 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 10 Oct 2026 06:18:12 -0400 Subject: [PATCH 31/35] Merge origin/main into #254; prefer pipewire/pulse, cache the fallback, translate notes - When the default output cannot be opened, probe pipewire/pulse first and raw hw:/plughw: last (those take the card exclusively). - The fallback list is probed only after the default fails, and the pick is cached per host, so devices()/configure() no longer open every PCM. - Status-bar notes go through i18n::t/fmt. Co-Authored-By: Claude Opus 5.5 --- apps/filmcraft/src/audio.rs | 97 +++++++++++++++++++++++++++++++------ 1 file changed, 82 insertions(+), 15 deletions(-) diff --git a/apps/filmcraft/src/audio.rs b/apps/filmcraft/src/audio.rs index b8d164bb..f0e9bc48 100644 --- a/apps/filmcraft/src/audio.rs +++ b/apps/filmcraft/src/audio.rs @@ -1,9 +1,10 @@ //! cpal audio output: the playback master clock. Settings ▸ Audio Hardware picks the host //! ("Device Class"), the output device, the I/O buffer size and the sample rate. -use std::sync::Arc; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::sync::{Arc, Mutex, PoisonError}; + use cpal::traits::{DeviceTrait, HostTrait, StreamTrait}; use filmcraft_engine::settings::AudioHardwarePrefs; use filmcraft_ui_egui::{AudioDevices, AudioOut}; @@ -19,7 +20,29 @@ pub struct CpalOut { document_rate: Option, /// Status-bar note: the host's default device could not be opened and a fallback device is /// in use, or no output device was found (playing without sound). - note: Option, + note: Option, + /// The fallback picked when the default device could not be opened, as (host, device) names, + /// so later lookups (`devices`, `configure`, `start`) reuse it instead of probing every PCM. + fallback: Mutex>, +} + +/// A hardware note for the status bar, translated when it is shown. +#[derive(Clone, Debug, PartialEq)] +enum Note { + /// The default output device cannot be opened; this named device is used instead. + Fallback(String), + /// No output device at all: playback runs without sound. + NoDevice, +} + +impl Note { + fn text(&self) -> String { + use filmcraft_ui_egui::i18n::{fmt, t}; + match self { + Note::Fallback(device) => fmt(t("Audio hardware: the default output device cannot be opened; using '{device}'"), &[("device", device)]), + Note::NoDevice => t("No audio output device found: playing without sound (check Settings ▸ Audio Hardware)").to_string(), + } + } } /// The host named in the settings (empty or unknown = the default host). @@ -37,13 +60,31 @@ fn host(name: &str) -> cpal::Host { struct DeviceChoice { device: cpal::Device, /// The host's default device could not be opened: this device is the fallback. - fallback: Option, + fallback: Option, +} + +/// Fallback order when the default device cannot be opened: the sound-server bridges first +/// (`pipewire`, `pulse`: they share the card with every other app), then other named PCMs, and +/// raw `hw:`/`plughw:` devices last (opening one takes the card exclusively and silences the +/// rest of the desktop). +fn fallback_rank(name: &str) -> u8 { + let n = name.to_ascii_lowercase(); + if n == "pipewire" || n.starts_with("pipewire:") { + 0 + } else if n == "pulse" || n.starts_with("pulse:") { + 1 + } else if n.starts_with("hw:") || n.starts_with("plughw:") { + 3 + } else { + 2 + } } /// The output device named in the settings (empty or missing = the host's default). When the /// default cannot be opened — a broken ALSA `default` (missing `99-pipewire-default.conf`, -/// #23/#106) — the first device that can, so the app is not silently mute. -fn output_device(h: &cpal::Host, name: &str) -> Option { +/// #23/#106) — a fallback that can, so the app is not silently mute. The fallback list is probed +/// only after the default fails, in [`fallback_rank`] order, and the pick is cached in `cache`. +fn output_device(h: &cpal::Host, name: &str, cache: &Mutex>) -> Option { if !name.is_empty() && let Ok(mut devs) = h.output_devices() && let Some(d) = devs.find(|d| d.name().is_ok_and(|n| n == name)) @@ -55,23 +96,34 @@ fn output_device(h: &cpal::Host, name: &str) -> Option { { return Some(DeviceChoice { device: d, fallback: None }); } - let mut devs = h.output_devices().ok()?; - let d = devs.find(|d| d.default_output_config().is_ok())?; - let found = d.name().unwrap_or_default(); + let host_name = h.id().name().to_string(); + let mut cache = cache.lock().unwrap_or_else(PoisonError::into_inner); + let mut devs: Vec<(String, cpal::Device)> = h.output_devices().ok()?.filter_map(|d| d.name().ok().map(|n| (n, d))).collect(); + if let Some((cached_host, cached)) = cache.as_ref() + && *cached_host == host_name + && let Some(i) = devs.iter().position(|(n, _)| n == cached) + { + let (found, d) = devs.swap_remove(i); + return Some(DeviceChoice { device: d, fallback: Some(Note::Fallback(found)) }); + } + devs.sort_by_key(|(n, _)| fallback_rank(n)); + let (found, d) = devs.into_iter().find(|(_, d)| d.default_output_config().is_ok())?; log::warn!("the default audio output device cannot be opened; falling back to '{found}'"); - Some(DeviceChoice { device: d, fallback: Some(format!("Audio hardware: the default output device cannot be opened; using '{found}'")) }) + *cache = Some((host_name, found.clone())); + Some(DeviceChoice { device: d, fallback: Some(Note::Fallback(found)) }) } impl CpalOut { pub fn new() -> Self { let host = cpal::default_host(); - let choice = output_device(&host, ""); + let fallback = Mutex::new(None); + let choice = output_device(&host, "", &fallback); let cfg = choice.as_ref().and_then(|c| c.device.default_output_config().ok()); let note = match choice { Some(c) => c.fallback, None => { log::warn!("no audio output device found"); - Some("No audio output device found: playing without sound (check Settings ▸ Audio Hardware)".into()) + Some(Note::NoDevice) } }; Self { @@ -83,6 +135,7 @@ impl CpalOut { hw: AudioHardwarePrefs::default(), document_rate: None, note, + fallback, } } @@ -154,7 +207,7 @@ impl AudioOut for CpalOut { fn start(&mut self, mut fill: Box) -> Result { self.stop(); let host = host(&self.hw.device_class); - let DeviceChoice { device: dev, fallback } = output_device(&host, &self.hw.default_output).ok_or("no output device")?; + let DeviceChoice { device: dev, fallback } = output_device(&host, &self.hw.default_output, &self.fallback).ok_or("no output device")?; self.note = fallback; let cfg = self.config(&dev)?; let channels = cfg.channels() as usize; @@ -216,7 +269,8 @@ impl AudioOut for CpalOut { let h = host(&self.hw.device_class); let outputs = h.output_devices().map(|d| d.filter_map(|x| x.name().ok()).collect()).unwrap_or_default(); let inputs = h.input_devices().map(|d| d.filter_map(|x| x.name().ok()).collect()).unwrap_or_default(); - let output_channels = output_device(&h, &self.hw.default_output).and_then(|c| self.config(&c.device).ok()).map(|cfg| cfg.channels()).unwrap_or(0); + let output_channels = + output_device(&h, &self.hw.default_output, &self.fallback).and_then(|c| self.config(&c.device).ok()).map(|cfg| cfg.channels()).unwrap_or(0); AudioDevices { hosts: cpal::available_hosts().iter().map(|h| h.name().to_string()).collect(), inputs, outputs, output_channels } } fn configure(&mut self, hw: &AudioHardwarePrefs, document_rate: Option) { @@ -224,7 +278,7 @@ impl AudioOut for CpalOut { self.document_rate = document_rate; // the rate playback mixes at must be known before `start` let host = host(&self.hw.device_class); - if let Some(choice) = output_device(&host, &self.hw.default_output) + if let Some(choice) = output_device(&host, &self.hw.default_output, &self.fallback) && let Ok(cfg) = self.config(&choice.device) { self.rate = cfg.sample_rate().0; @@ -233,7 +287,7 @@ impl AudioOut for CpalOut { } } fn note(&self) -> Option { - self.note.clone() + self.note.as_ref().map(Note::text) } } @@ -242,6 +296,19 @@ mod tests { use super::*; use cpal::Sample; + #[test] + fn fallback_prefers_sound_servers_over_raw_hardware() { + let mut names = vec!["plughw:CARD=PCH,DEV=0", "hw:CARD=PCH,DEV=0", "sysdefault:CARD=PCH", "pulse", "pipewire"]; + names.sort_by_key(|n| fallback_rank(n)); + assert_eq!(names, ["pipewire", "pulse", "sysdefault:CARD=PCH", "plughw:CARD=PCH,DEV=0", "hw:CARD=PCH,DEV=0"]); + } + + #[test] + fn notes_name_the_fallback_device() { + assert!(Note::Fallback("pipewire".into()).text().contains("'pipewire'")); + assert!(!Note::NoDevice.text().is_empty()); + } + #[test] fn integer_conversion_clips_and_preserves_interleaved_channels() { let mut signed = [0_i16; 8]; From e19eaa760b2dec0b3350cde80f6692334636b2be Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 10 Oct 2026 06:22:16 -0400 Subject: [PATCH 32/35] Merge origin/main into #246; dev desktop entry is opt-in, icons not embedded - Nothing is written unless FILMCRAFT_DEV_DESKTOP_ENTRY=1 (debug builds no longer write into ~/.local/share on every cargo run). - The 256/512 PNGs are copied from this checkout's assets/app-icon at runtime instead of include_bytes! into every Linux binary; a missing icon is logged and skipped. Co-Authored-By: Claude Opus 5.5 --- apps/filmcraft/src/dev_icon.rs | 56 ++++++++++++++++++++++------------ docs/contributing.md | 2 +- 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/apps/filmcraft/src/dev_icon.rs b/apps/filmcraft/src/dev_icon.rs index 6e2b7c7f..8c987679 100644 --- a/apps/filmcraft/src/dev_icon.rs +++ b/apps/filmcraft/src/dev_icon.rs @@ -6,17 +6,17 @@ //! (`packaging/linux/ai.storyteller.filmcraft.desktop` and the hicolor PNGs) — but a source-tree //! run has neither, so KDE/GNOME fall back to the generic Wayland icon. The fix is to ensure a //! user-level entry for those runs: when no entry for the app id exists in any XDG data dir, -//! install one (plus the hicolor icon PNGs, embedded so it works whatever the working directory -//! is) into `$XDG_DATA_HOME`/`~/.local/share`. An existing packaged entry is never overridden; -//! the user-owned dev copy is rewritten when it points at another binary (a dev build moves -//! between checkouts). The shell picks the icon up at the *next* app start; +//! install one (plus the hicolor icon PNGs, read from this checkout's `assets/app-icon`, so +//! nothing is embedded in the binary) into `$XDG_DATA_HOME`/`~/.local/share`. An existing +//! packaged entry is never overridden; the user-owned dev copy is rewritten when it points at +//! another binary (a dev build moves between checkouts). The shell picks the icon up at the *next* app start; //! `update-desktop-database`/`kbuildsycoca` is not needed for icon lookup. //! -//! Only source-tree runs install anything: a debug build, or a release build with -//! `FILMCRAFT_DEV_DESKTOP_ENTRY` set (the docs' `cargo run --release` flow). Packaged runs are -//! excluded — AppImage (`APPIMAGE`) mounts at a fresh `/tmp/.mount_*` each launch, so its -//! `Exec` path changes every time and it ships its own entry, and a Flatpak (`FLATPAK_ID`) -//! installs its entry through the manifest. `packaging/linux/install.sh` writes the packaged +//! Opt-in only: nothing is written unless `FILMCRAFT_DEV_DESKTOP_ENTRY=1` is set, so ordinary +//! `cargo run`s (agents, worktrees, CI) never touch the user's profile. Packaged runs are +//! excluded even then — AppImage (`APPIMAGE`) mounts at a fresh `/tmp/.mount_*` each launch, +//! so its `Exec` path changes every time and it ships its own entry, and a Flatpak +//! (`FLATPAK_ID`) installs its entry through the manifest. `packaging/linux/install.sh` writes the packaged //! `Exec=` the same two-layer way as here; keep the two in sync. //! //! Every failure is logged and skipped — a missing dev icon must never stop the app @@ -30,9 +30,10 @@ pub fn ensure_dev_desktop_entry() { if std::env::var_os("APPIMAGE").is_some() || std::env::var_os("FLATPAK_ID").is_some() { return; } - // Dev builds only: a debug build, or a release run opted in — a packaged release must not - // touch the profile (the packages and install.sh install the real entry). - if !cfg!(debug_assertions) && std::env::var_os("FILMCRAFT_DEV_DESKTOP_ENTRY").is_none() { + // Opt-in only: a plain `cargo run` (agents, worktrees, other checkouts) must not write into + // the user's profile, and a packaged release never does (packages and install.sh install + // the real entry). + if std::env::var_os("FILMCRAFT_DEV_DESKTOP_ENTRY").is_none_or(|v| v != "1") { return; } if let Err(e) = ensure() { @@ -71,13 +72,20 @@ fn ensure() -> Result<(), DevIconError> { } break; // a dev entry from another checkout: fall through and rewrite it } - for (size, png) in ICONS { - let dest = user_dir.join("icons").join("hicolor").join(format!("{size}x{size}")).join("apps").join(format!("{}.png", crate::APP_ID)); + for size in ICON_SIZES { + let rel = std::path::Path::new("hicolor").join(format!("{size}x{size}")).join("apps").join(format!("{}.png", crate::APP_ID)); + let dest = user_dir.join("icons").join(&rel); if !dest.is_file() && let Some(parent) = dest.parent() { + // The checkout this binary was built from; a missing icon only costs the icon. + let src = std::path::Path::new(ICON_DIR).join(&rel); + if !src.is_file() { + log::warn!("dev desktop entry: icon {} not found; skipping it", src.display()); + continue; + } std::fs::create_dir_all(parent).map_err(DevIconError::Io)?; - std::fs::write(&dest, png).map_err(DevIconError::Io)?; + std::fs::copy(&src, &dest).map_err(DevIconError::Io)?; } } let applications = user_dir.join("applications"); @@ -89,10 +97,11 @@ fn ensure() -> Result<(), DevIconError> { /// The hicolor icon sizes installed for the taskbar (a subset large enough for panel and topbar; /// the shell picks the nearest size, so 256 and 512 cover both). -const ICONS: &[(u16, &[u8])] = &[ - (256, include_bytes!("../../../assets/app-icon/hicolor/256x256/apps/ai.storyteller.filmcraft.png")), - (512, include_bytes!("../../../assets/app-icon/hicolor/512x512/apps/ai.storyteller.filmcraft.png")), -]; +const ICON_SIZES: [u16; 2] = [256, 512]; + +/// The app icons in the source tree this binary was built from (a path, not embedded bytes: +/// the entry is a dev-checkout convenience and release binaries must not carry the PNGs). +const ICON_DIR: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/../../assets/app-icon"); /// The user data dir we install into: `$XDG_DATA_HOME`, else `$HOME/.local/share` (XDG base /// directory spec). Untrusted environment: an empty or relative `XDG_DATA_HOME` means the @@ -250,6 +259,15 @@ mod tests { /// The entry mirrors the packaged one, derived from the real file so the two cannot drift: /// minus `TryExec` (a dev build isn't on `$PATH`), plus the quoted dev `Exec` path. + /// The icons the entry installs exist in the checkout. + #[test] + fn icon_sources_exist_in_the_checkout() { + for size in ICON_SIZES { + let p = std::path::Path::new(ICON_DIR).join(format!("hicolor/{size}x{size}/apps/ai.storyteller.filmcraft.png")); + assert!(p.is_file(), "{}", p.display()); + } + } + #[test] fn desktop_file_matches_the_packaged_fields() { let packaged = include_str!("../../../packaging/linux/ai.storyteller.filmcraft.desktop"); diff --git a/docs/contributing.md b/docs/contributing.md index c52729bb..c38791fc 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -40,7 +40,7 @@ cargo xtask web --serve 8765 # the web app on http://12 |---|---| | `FILMCRAFT_CONTROL_PORT` | Same as `--control ` | | `FILMCRAFT_CPU_COMPOSITE=1` | Disables the GPU compositor | -| `FILMCRAFT_DEV_DESKTOP_ENTRY=1` | A release run from a source tree installs the user-level desktop entry for the taskbar icon (a debug build always does; packaged runs never) | +| `FILMCRAFT_DEV_DESKTOP_ENTRY=1` | Linux, opt-in: a run from a source tree installs a user-level desktop entry (and the icons from `assets/app-icon`) so the taskbar shows the FilmCraft icon. Nothing is written without it; packaged runs never write one | | `FILMCRAFT_DATA_DIR` | Same as `--data-dir `: auto-save, crash recovery, settings and logs | | `RUST_LOG` | Log levels for standard error and the log file (see [Logs](#logs)) | From d0addce05f8f75d728f196e878a93e85fbaa4de3 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 10 Oct 2026 06:28:34 -0400 Subject: [PATCH 33/35] Graphics drag undo: cargo fmt; UI tests read params through typed accessors Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/graphics.rs | 10 +++++++++- crates/ui-egui/tests/graphics_ui.rs | 24 +++++++++--------------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/crates/engine/src/graphics.rs b/crates/engine/src/graphics.rs index 3e39b1e8..83e98712 100644 --- a/crates/engine/src/graphics.rs +++ b/crates/engine/src/graphics.rs @@ -264,7 +264,15 @@ fn apply_props(e: &mut EffectInstance, props: &serde_json::Map, m /// Set properties `props` on a layer (keyframe-aware at time `tl`). Changing the text keeps /// per-character styles on their characters. `merge` folds consecutive calls with the same key /// into one undo step (a drag is one undoable change, like `effects.setParam`). -pub(crate) fn set_props(s: &mut Session, clip: ClipId, eidx: usize, props: &serde_json::Map, tl: Tick, label: &str, merge: Option<&str>) -> Result<()> { +pub(crate) fn set_props( + s: &mut Session, + clip: ClipId, + eidx: usize, + props: &serde_json::Map, + tl: Tick, + label: &str, + merge: Option<&str>, +) -> Result<()> { let props = props.clone(); s.edit_sequence_as(label, merge, |q, _, _| { let (_, it) = q.find_item_mut(clip).ok_or(filmcraft_edit::EditError::NoItem(clip))?; diff --git a/crates/ui-egui/tests/graphics_ui.rs b/crates/ui-egui/tests/graphics_ui.rs index 8da4e406..c7aa5138 100644 --- a/crates/ui-egui/tests/graphics_ui.rs +++ b/crates/ui-egui/tests/graphics_ui.rs @@ -153,6 +153,13 @@ fn type_tool_edit_style_and_move() { d.shot("shape-layer"); } +/// The value of `param` on the text layer of graphic clip `clip`, at its first frame. +fn text_param(d: &mut Driver, clip: u64, param: &str) -> Option { + let s = &d.harness.state().session; + let (_, it) = s.active_sequence()?.find_item(filmcraft_project::ClipId(clip))?; + let e = it.effects.iter().find(|e| e.effect == "graphic_text")?; + Some(e.param(param)?.value_at(filmcraft_time::Tick::ZERO)) +} /// A drag of a property in the Properties panel is one undo step, so one Cmd+Z restores the /// original value (a drag was committing one undo step per frame, so undo only stepped back one @@ -168,13 +175,7 @@ fn a_property_drag_is_one_undo_step() { // the layer's Opacity number (visible in the Align and Transform section) d.exec("graphics.set", json!({"clip": clip, "layer": 0, "props": {"opacity": 50}})); d.frames(2); - let opacity = |d: &mut Driver| -> f64 { - let seq = d.exec("sequence.inspect", json!({})); - let item = seq["video"].as_array().unwrap().iter().flat_map(|t| t["items"].as_array().unwrap().iter()).find(|i| i["clip"].as_u64() == Some(clip)).unwrap(); - let e = item["effects"].as_array().unwrap().iter().find(|e| e["effect"] == "graphic_text").unwrap(); - let v = e["params"]["opacity"]["value"].as_str().unwrap(); - v.trim_start_matches("Float(").trim_end_matches(')').parse::().unwrap() - }; + let opacity = |d: &mut Driver| -> f64 { text_param(d, clip, "opacity").and_then(|v| v.as_f64()).unwrap() }; let before = opacity(&mut d); let undo0 = d.exec("history.list", json!({}))["undo"].as_array().unwrap().len(); // drag the Opacity number to the left: several move frames, one undo step @@ -190,8 +191,6 @@ fn a_property_drag_is_one_undo_step() { assert_eq!(opacity(&mut d), before, "one undo restores the original value"); } - - /// The shadow color picker popup: a drag in the 2D picker is one undo step, so one Cmd+Z /// restores the original color (a drag was committing one undo step per frame, so undo only /// stepped back one frame's worth — the reported "sometimes undo works, sometimes not"). @@ -203,12 +202,7 @@ fn shadow_color_drag_is_one_undo_step() { let clip = r["clip"].as_u64().unwrap(); d.exec("graphics.selectLayer", json!({"clip": clip, "layers": [0]})); d.frames(4); - let color = |d: &mut Driver| -> String { - let seq = d.exec("sequence.inspect", json!({})); - let item = seq["video"].as_array().unwrap().iter().flat_map(|t| t["items"].as_array().unwrap().iter()).find(|i| i["clip"].as_u64() == Some(clip)).unwrap(); - let e = item["effects"].as_array().unwrap().iter().find(|e| e["effect"] == "graphic_text").unwrap(); - e["params"].get("shadow_color").and_then(|p| p["value"].as_str()).unwrap_or("Color([0.0, 0.0, 0.0, 1.0])").to_string() - }; + let color = |d: &mut Driver| text_param(d, clip, "shadow_color").and_then(|v| v.as_color()); // scroll the properties panel so the shadow row is visible, then open the picker d.ok("ui.scroll", json!({"x": 1400.0, "y": 500.0, "dx": 0.0, "dy": -400.0})); d.frames(3); From 68c297b9876caeec1fda33341fcbc3227365be3d Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 10 Oct 2026 07:08:56 -0400 Subject: [PATCH 34/35] Arc: fix the missed &mut sites and closing braces; drop the unrelated key-dispatch test source_monitor markers and format's audio-stream clamp now detach with Arc::make_mut; restore the final braces in clip_ops.rs and project/src/lib.rs (cargo fmt); scripted.rs is back to main (the test covered key dispatch, which this PR doesn't change). Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/clip_ops.rs | 2 +- crates/engine/src/source_monitor.rs | 2 +- crates/format/src/lib.rs | 1 + crates/project/src/lib.rs | 2 +- crates/ui-egui/tests/scripted.rs | 23 ----------------------- 5 files changed, 4 insertions(+), 26 deletions(-) diff --git a/crates/engine/src/clip_ops.rs b/crates/engine/src/clip_ops.rs index f9c0e077..1ef93e95 100644 --- a/crates/engine/src/clip_ops.rs +++ b/crates/engine/src/clip_ops.rs @@ -1769,4 +1769,4 @@ fn short_clips(s: &Session, clips: &[u64]) -> Vec { (frames > 0).then(|| json!({"clip": c, "shortByFrames": frames})) }) .collect() - } +} diff --git a/crates/engine/src/source_monitor.rs b/crates/engine/src/source_monitor.rs index 836fff49..cdbcf35f 100644 --- a/crates/engine/src/source_monitor.rs +++ b/crates/engine/src/source_monitor.rs @@ -94,7 +94,7 @@ fn marker_list(p: &mut Project, item: ItemId) -> Result<&mut Vec> { let it = p.item_mut(item).ok_or_else(|| EngineError::Other("Source item is unavailable".into()))?; match &mut it.kind { filmcraft_project::ItemKind::Media(m) => Ok(&mut m.markers), - filmcraft_project::ItemKind::Sequence(q) => Ok(&mut q.markers), + filmcraft_project::ItemKind::Sequence(q) => Ok(&mut std::sync::Arc::make_mut(q).markers), _ => Err(EngineError::Other("Source marker editing requires media or a sequence".into())), } } diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 1abdc666..e1b628be 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -224,6 +224,7 @@ fn validate_loaded(mut loaded: Loaded) -> Result { m.info.audio_streams.truncate(max); } filmcraft_project::ItemKind::Sequence(sequence) => { + let sequence = std::sync::Arc::make_mut(sequence); for clip in sequence.audio_tracks.iter_mut().flat_map(|t| t.items.iter_mut()) { if clip.audio_stream >= max { log::warn!("sequence `{name}`: clip `{}` uses audio stream {}, reset to 0", clip.name, clip.audio_stream); diff --git a/crates/project/src/lib.rs b/crates/project/src/lib.rs index 6261889d..d5e508b6 100644 --- a/crates/project/src/lib.rs +++ b/crates/project/src/lib.rs @@ -1996,4 +1996,4 @@ mod tests { nest.frame_hold = Some(f(100)); assert_eq!(p.nest_overhang(&nest), Some(nest.range())); } - } +} diff --git a/crates/ui-egui/tests/scripted.rs b/crates/ui-egui/tests/scripted.rs index 700010e5..63903a48 100644 --- a/crates/ui-egui/tests/scripted.rs +++ b/crates/ui-egui/tests/scripted.rs @@ -506,29 +506,6 @@ fn keyboard_shortcuts_dialog_assigns_live_and_cancel_restores() { assert_eq!(track_clips(&d.sequence(), 0).len(), n0 + 1); } -#[test] -fn shortcuts_in_one_input_frame_are_all_dispatched() { - let mut d = Driver::demo(); - d.exec("playhead.set", json!({"seconds": 2.0})); - d.ok("ui.set", json!({"focused": "Timeline"})); - let before = d.sequence(); - let n0 = track_clips(&before, 0).len(); - let ph = before["playhead"].as_i64().unwrap(); - - // Raw OS-style events, deliberately in the same egui input frame. These bypass the control - // channel's synthetic pointer sequencing and exercise normal shortcut dispatch directly. - let command = egui::Modifiers { command: true, ..Default::default() }; - let key = |key, modifiers| egui::Event::Key { key, physical_key: Some(key), pressed: true, repeat: false, modifiers }; - let events = &mut d.harness.input_mut().events; - events.push(key(egui::Key::K, command)); - events.push(key(egui::Key::ArrowRight, egui::Modifiers::default())); - d.frames(1); - - let after = d.sequence(); - assert_eq!(track_clips(&after, 0).len(), n0 + 1, "Cmd+K was lost when another key arrived in the same input frame"); - assert!(after["playhead"].as_i64().unwrap() > ph, "Right was lost when another key arrived in the same input frame"); -} - /// Waveform peaks are cached per item id; ids repeat across projects, so opening another project /// must not reuse the previous project's peaks (it showed a long mission-audio clip as silent). #[test] From ab227a1853f6a659090ef447f78a96667e2c8ea5 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 10 Oct 2026 07:19:37 -0400 Subject: [PATCH 35/35] Timeline file drops: Chinese and Japanese strings Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/i18n/ja.tsv | 3 +++ crates/ui-egui/src/i18n/zh-cn.tsv | 3 +++ 2 files changed, 6 insertions(+) diff --git a/crates/ui-egui/src/i18n/ja.tsv b/crates/ui-egui/src/i18n/ja.tsv index 016d6bc1..aa74f7b8 100644 --- a/crates/ui-egui/src/i18n/ja.tsv +++ b/crates/ui-egui/src/i18n/ja.tsv @@ -1253,6 +1253,9 @@ Active Monitor アクティブなモニター Add 加算 Add tracks automatically when editing source clips onto the Timeline ソースクリップをタイムラインに配置するとき、トラックを自動的に追加する + Place files dropped onto the Timeline directly on the Timeline タイムラインにドロップしたファイルをタイムラインに直接配置 + 1 file 1 個のファイル + {n} files {n} 個のファイル Additional Scale 追加スケール Additive Dissolve 加算ディゾルブ Additive Dissolve (Legacy) 加算ディゾルブ (従来版) diff --git a/crates/ui-egui/src/i18n/zh-cn.tsv b/crates/ui-egui/src/i18n/zh-cn.tsv index 5da60959..8eac80b1 100644 --- a/crates/ui-egui/src/i18n/zh-cn.tsv +++ b/crates/ui-egui/src/i18n/zh-cn.tsv @@ -1246,6 +1246,9 @@ Active Monitor 活动监视器 Add 添加 Add tracks automatically when editing source clips onto the Timeline 将源剪辑编辑到时间轴时自动添加轨道 + Place files dropped onto the Timeline directly on the Timeline 将拖放到时间轴上的文件直接放置在时间轴上 + 1 file 1 个文件 + {n} files {n} 个文件 Additional Scale 附加缩放 Additive Dissolve 添加式溶解 Additive Dissolve (Legacy) 添加式溶解(旧版)