From 83841fc74aa01a3abfbbfba0af05e11e2dfbad69 Mon Sep 17 00:00:00 2001 From: keigoly <61711237+keigoly@users.noreply.github.com> Date: Tue, 20 Oct 2026 22:30:27 +0900 Subject: [PATCH 1/3] Clip Mismatch Warning: a first clip that doesn't match an empty sequence asks Settings > Timeline > Show Clip Mismatch Warning dialog was there but did nothing: putting a clip with another frame size or frame rate in an empty sequence (a 120 fps phone or game capture in a 23.976 fps sequence) kept the sequence's settings without a word, so the clip was edited and exported at 23.976. Now the first clip put in an empty sequence that doesn't match it asks. Change Sequence Settings gives the sequence the clip's frame size, frame rate and pixel aspect ratio (its other settings stay), Keep Existing Settings puts the clip in as before, and Escape puts nothing in. It covers a drag from the Project panel or a file dropped onto the Timeline, and Insert / Overwrite (their buttons, the , and . keys and the Clip menu). With the setting off nothing asks, as before. The engine side is a `matchSequence` parameter on timeline.place, source.insert and source.overwrite, off by default so scripts and the CLI work as before: with it, the empty sequence takes the clip's settings in the same undo step as the edit. commands::clip_mismatch says what would change. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/clip_mismatch_tests.rs | 140 +++++++++++++ crates/engine/src/commands.rs | 60 +++++- crates/engine/src/lib.rs | 2 + crates/ui-egui/src/i18n/es.tsv | 7 + crates/ui-egui/src/i18n/ja.tsv | 7 + crates/ui-egui/src/i18n/zh-cn.tsv | 7 + crates/ui-egui/src/lib.rs | 7 + crates/ui-egui/src/menus.rs | 4 + crates/ui-egui/src/panels/clip_mismatch.rs | 108 ++++++++++ crates/ui-egui/src/panels/dialogs.rs | 6 + crates/ui-egui/src/panels/file_dialogs.rs | 4 +- crates/ui-egui/src/panels/mod.rs | 1 + crates/ui-egui/src/panels/timeline.rs | 8 +- crates/ui-egui/tests/clip_mismatch_ui.rs | 227 +++++++++++++++++++++ 14 files changed, 576 insertions(+), 12 deletions(-) create mode 100644 crates/engine/src/clip_mismatch_tests.rs create mode 100644 crates/ui-egui/src/panels/clip_mismatch.rs create mode 100644 crates/ui-egui/tests/clip_mismatch_ui.rs diff --git a/crates/engine/src/clip_mismatch_tests.rs b/crates/engine/src/clip_mismatch_tests.rs new file mode 100644 index 000000000..735b90e8d --- /dev/null +++ b/crates/engine/src/clip_mismatch_tests.rs @@ -0,0 +1,140 @@ +//! Settings ▸ Timeline ▸ Show Clip Mismatch Warning dialog: [`crate::commands::clip_mismatch`] and +//! `matchSequence` on `timeline.place`, `source.insert` and `source.overwrite`. + +use filmcraft_media::MediaKind; +use filmcraft_project::{ItemId, ItemKind}; +use filmcraft_time::FrameRate; +use serde_json::json; + +use crate::Session; +use crate::commands::clip_mismatch; + +/// A movie of the demo project (23.976 fps, 1920 x 1080). +fn demo_movie(s: &Session, not: Option) -> ItemId { + s.project + .items + .values() + .find(|i| Some(i.id) != not && i.has_video() && i.has_audio() && i.as_media().is_some_and(|m| m.info.kind == MediaKind::Movie)) + .map(|i| i.id) + .unwrap() +} + +/// The demo project with a new, empty sequence open (1920 x 1080, 23.976 fps), and one of the +/// demo's movies made to play at 120 fps, like a phone or game capture. +fn empty_sequence_and_120fps_movie() -> (Session, ItemId) { + let mut s = Session::default(); + s.execute("file.openDemoProject", json!({})).unwrap(); + let movie = demo_movie(&s, None); + s.edit("120 fps", |pr, _| { + if let Some(ItemKind::Media(m)) = pr.item_mut(movie).map(|i| &mut i.kind) + && let Some(v) = m.info.video.as_mut() + { + v.frame_rate = FrameRate::FPS_120; + } + Ok(()) + }) + .unwrap(); + s.execute("file.newSequence", json!({"name": "Empty"})).unwrap(); + (s, movie) +} + +fn settings(s: &Session) -> (u32, u32, FrameRate) { + let q = s.active_sequence().unwrap(); + (q.settings.width, q.settings.height, q.settings.frame_rate) +} + +fn clips(s: &Session) -> usize { + s.active_sequence().unwrap().all_tracks().map(|t| t.items.len()).sum() +} + +fn last_undo(s: &Session) -> Option<&str> { + s.history.undo.last().map(|u| u.0.as_str()) +} + +#[test] +fn a_first_clip_that_differs_from_an_empty_sequence_mismatches() { + let (mut s, movie) = empty_sequence_and_120fps_movie(); + assert_eq!(settings(&s), (1920, 1080, FrameRate::FPS_23_976)); + let st = clip_mismatch(&s, movie).expect("a 120 fps movie does not match a 23.976 fps sequence"); + assert_eq!((st.width, st.height, st.frame_rate), (1920, 1080, FrameRate::FPS_120)); + assert_eq!(st.preset, "1920x1080 120"); + assert_eq!(st.sample_rate, s.active_sequence().unwrap().settings.sample_rate, "the sequence's other settings stay"); + // once the sequence holds a clip, the next one is not asked about + s.execute("timeline.place", json!({"item": movie.0, "time": 0})).unwrap(); + assert!(clip_mismatch(&s, movie).is_none()); +} + +#[test] +fn a_matching_movie_sound_a_still_and_a_sequence_do_not_mismatch() { + let (mut s, movie) = empty_sequence_and_120fps_movie(); + assert!(clip_mismatch(&s, demo_movie(&s, Some(movie))).is_none(), "the demo's other movies match the sequence"); + let sound = s.project.items.values().find(|i| !i.has_video() && i.has_audio()).map(|i| i.id).unwrap(); + assert!(clip_mismatch(&s, sound).is_none()); + let sequence = s.project.items.values().find(|i| i.as_sequence().is_some()).map(|i| i.id).unwrap(); + assert!(clip_mismatch(&s, sequence).is_none()); + // a still of another rate: stills have no frame rate of their own to match + s.edit("still", |pr, _| { + if let Some(ItemKind::Media(m)) = pr.item_mut(movie).map(|i| &mut i.kind) { + m.info.kind = MediaKind::Still; + } + Ok(()) + }) + .unwrap(); + assert!(clip_mismatch(&s, movie).is_none()); +} + +#[test] +fn match_sequence_takes_the_clips_settings_in_the_same_undo_step() { + let (mut s, movie) = empty_sequence_and_120fps_movie(); + let steps = s.history.undo.len(); + let r = s.execute("timeline.place", json!({"item": movie.0, "time": 0, "matchSequence": true})).unwrap(); + assert_eq!(r["clips"].as_array().map(Vec::len), Some(2), "picture and sound: {r}"); + assert_eq!(settings(&s), (1920, 1080, FrameRate::FPS_120)); + assert_eq!(s.active_sequence().unwrap().settings.preset, "1920x1080 120"); + assert_eq!(s.history.undo.len(), steps + 1, "one undo step"); + assert_eq!(last_undo(&s), Some("Place Clip")); + s.undo(); + assert_eq!(clips(&s), 0); + assert_eq!(settings(&s), (1920, 1080, FrameRate::FPS_23_976), "the one undo puts the settings back too"); +} + +#[test] +fn without_match_sequence_the_settings_stay() { + for params in [json!({"time": 0}), json!({"time": 0, "matchSequence": false})] { + let (mut s, movie) = empty_sequence_and_120fps_movie(); + let mut p = params.clone(); + p["item"] = json!(movie.0); + s.execute("timeline.place", p).unwrap(); + assert_eq!(clips(&s), 2, "{params}"); + assert_eq!(settings(&s), (1920, 1080, FrameRate::FPS_23_976), "{params}"); + } +} + +#[test] +fn match_sequence_leaves_a_sequence_that_holds_clips_alone() { + let (mut s, movie) = empty_sequence_and_120fps_movie(); + s.execute("timeline.place", json!({"item": demo_movie(&s, Some(movie)).0, "time": 0})).unwrap(); + s.execute("timeline.place", json!({"item": movie.0, "seconds": 60.0, "matchSequence": true})).unwrap(); + assert_eq!(settings(&s), (1920, 1080, FrameRate::FPS_23_976)); + assert_eq!(clips(&s), 4); +} + +#[test] +fn match_sequence_on_items_insert_and_overwrite() { + // `items`: the first item decides + let (mut s, movie) = empty_sequence_and_120fps_movie(); + s.execute("timeline.place", json!({"items": [movie.0], "time": 0, "matchSequence": true})).unwrap(); + assert_eq!(settings(&s).2, FrameRate::FPS_120); + assert_eq!(last_undo(&s), Some("Place Clips")); + // Insert and Overwrite from the Source monitor + for (command, label) in [("source.insert", "Insert"), ("source.overwrite", "Overwrite")] { + let (mut s, movie) = empty_sequence_and_120fps_movie(); + s.execute("source.open", json!({"item": movie.0})).unwrap(); + s.execute(command, json!({"matchSequence": true})).unwrap(); + assert_eq!(settings(&s).2, FrameRate::FPS_120, "{command}"); + assert_eq!(clips(&s), 2, "{command}"); + assert_eq!(last_undo(&s), Some(label), "{command}"); + s.undo(); + assert_eq!((settings(&s).2, clips(&s)), (FrameRate::FPS_23_976, 0), "{command}"); + } +} diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index 162fb18ca..495c2cb52 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -646,6 +646,44 @@ pub(crate) fn default_seq_settings_for(media: &MediaClip) -> SequenceSettings { st } +/// Settings ▸ Timeline ▸ Show Clip Mismatch Warning dialog: the settings the active sequence takes +/// to match `item` when that is the first clip put in it, its frame size, frame rate and pixel +/// aspect ratio (Interpret Footage's, else the file's), the sequence's other settings kept. `None` +/// when the sequence already holds clips, the item has no moving picture (sound alone, a still, a +/// sequence) or nothing differs. `matchSequence` on `timeline.place`, `source.insert` and +/// `source.overwrite` applies it. +pub fn clip_mismatch(s: &Session, item: ItemId) -> Option { + let q = s.active_sequence()?; + if q.all_tracks().any(|t| !t.items.is_empty()) { + return None; + } + let (_, m, _) = s.project.resolve_media(item)?; + let v = m.info.video.as_ref().filter(|_| m.info.kind != filmcraft_media::MediaKind::Still)?; + let mut st = q.settings.clone(); + (st.width, st.height, st.frame_rate, st.par) = (v.width, v.height, m.frame_rate(), m.pixel_aspect()); + if (st.width, st.height, st.frame_rate, st.par) == (q.settings.width, q.settings.height, q.settings.frame_rate, q.settings.par) { + return None; + } + st.drop_frame &= st.frame_rate.supports_drop_frame(); + st.preset = format!("{}x{} {}", st.width, st.height, st.frame_rate.label()); + // a damaged file's size or rate must not reach the sequence + st.validate().ok().map(|()| st) +} + +/// `matchSequence`: before the edit, the active (still empty) sequence takes the clip's settings +/// ([`clip_mismatch`]), in the same undo step as the edit. +fn edit_matching_sequence(s: &mut Session, p: &Value, item: Option, label: &str, f: impl FnOnce(&mut Session) -> Result) -> Result { + let Some(st) = item.filter(|_| bool_p(p, "matchSequence") == Some(true)).and_then(|i| clip_mismatch(s, i)) else { return f(s) }; + let id = s.state.active_sequence.ok_or(EngineError::NoSequence)?; + s.edit_group(label, |s| { + s.edit("Sequence Settings", |pr, _| { + pr.sequence_mut(id).ok_or(EngineError::NoSequence)?.settings = st; + Ok(()) + })?; + f(s) + }) +} + /// Import bytes as a media item (engine-level; the UI reads files through services). pub fn import_bytes(s: &mut Session, path: &str, bytes: std::sync::Arc<[u8]>, bin: Option) -> Result { let name = std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| path.to_string()); @@ -2573,19 +2611,29 @@ fn build() -> Vec { let v = crate::clip_ops::source_view(s, item).ok_or_else(|| bad("source.inspect", "the source item is gone"))?; Ok(v.to_json(s.state.source_playhead)) }), - cmd!("source.insert", "Insert", ["Clip"], Some(","), "{}", has_source, |s, _| edit_at_source(s, true)), - cmd!("source.overwrite", "Overwrite", ["Clip"], Some("."), "{}", has_source, |s, _| edit_at_source(s, false)), + cmd!("source.insert", "Insert", ["Clip"], Some(","), r#"{"matchSequence":bool=false}"#, has_source, |s, p| { + let item = s.state.source_item; + edit_matching_sequence(s, p, item, "Insert", |s| edit_at_source(s, true)) + }), + cmd!("source.overwrite", "Overwrite", ["Clip"], Some("."), r#"{"matchSequence":bool=false}"#, has_source, |s, p| { + let item = s.state.source_item; + edit_matching_sequence(s, p, item, "Overwrite", |s| edit_at_source(s, false)) + }), // ================= Timeline gestures ================= cmd!( "timeline.place", "Place Clip", [], None, - r#"{"item":id|"items":[id] (back to back, one undo step),"track":"V1"|id|"A1" (sound only)|"new" (a video track after the last)?,"audioTrack":"A1"|id|"new"?,"time":ticks|"frame":i64|"seconds":f64,"insert":bool,"sourceIn":ticks?,"duration":ticks?,"video":bool=true,"audio":bool=true}"#, + r#"{"item":id|"items":[id] (back to back, one undo step),"track":"V1"|id|"A1" (sound only)|"new" (a video track after the last)?,"audioTrack":"A1"|id|"new"?,"time":ticks|"frame":i64|"seconds":f64,"insert":bool,"sourceIn":ticks?,"duration":ticks?,"video":bool=true,"audio":bool=true,"matchSequence":bool=false (an empty sequence first takes the clip's frame size and rate)}"#, has_seq, - |s, p| match p.get("items") { - Some(items) => place_items(s, p, items), - None => place_one(s, p), + |s, p| { + let first = item_p(p, "item").or_else(|| p.get("items").and_then(Value::as_array).and_then(|a| a.first()).and_then(Value::as_u64).map(ItemId)); + let label = if p.get("items").is_some() { "Place Clips" } else { "Place Clip" }; + edit_matching_sequence(s, p, first, label, |s| match p.get("items") { + Some(items) => place_items(s, p, items), + None => place_one(s, p), + }) } ), cmd!("timeline.select", "Select Clips", [], None, r#"{"clips":[id],"transitions":[id]?,"add":bool,"toggle":bool,"linked":bool?}"#, has_seq, |s, p| { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 5d0a044d0..0918da525 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1221,6 +1221,8 @@ mod autosave_tests; #[cfg(test)] mod avi_import_tests; #[cfg(test)] +mod clip_mismatch_tests; +#[cfg(test)] mod clip_ops_tests; #[cfg(test)] mod color_tests; diff --git a/crates/ui-egui/src/i18n/es.tsv b/crates/ui-egui/src/i18n/es.tsv index 2ac92decc..5606d0419 100644 --- a/crates/ui-egui/src/i18n/es.tsv +++ b/crates/ui-egui/src/i18n/es.tsv @@ -3617,3 +3617,10 @@ Open a sequence to search its graphics Abre una secuencia para buscar en sus gráficos No text graphics in this sequence. Add text with the Type tool. Esta secuencia no tiene gráficos con texto. Añade texto con la herramienta Texto. No matching text. Ningún texto coincide. + Clip Mismatch Warning Aviso de discrepancia de clip + This clip does not match the sequence settings. Este clip no coincide con los ajustes de la secuencia. + Clip: {settings} Clip: {settings} + Sequence: {settings} Secuencia: {settings} + Change the sequence to match the clip's settings? ¿Cambiar la secuencia para que coincida con los ajustes del clip? + Change Sequence Settings Cambiar ajustes de secuencia + Keep Existing Settings Mantener los ajustes actuales diff --git a/crates/ui-egui/src/i18n/ja.tsv b/crates/ui-egui/src/i18n/ja.tsv index d0fb3d4e7..7d5c540a3 100644 --- a/crates/ui-egui/src/i18n/ja.tsv +++ b/crates/ui-egui/src/i18n/ja.tsv @@ -3616,3 +3616,10 @@ Open a sequence to search its graphics シーケンスを開くとグラフィックスを検索できます No text graphics in this sequence. Add text with the Type tool. このシーケンスにはテキストのグラフィックスがありません。文字ツールでテキストを追加してください。 No matching text. 一致するテキストはありません。 + Clip Mismatch Warning クリップの不一致警告 + This clip does not match the sequence settings. このクリップはシーケンス設定と一致しません。 + Clip: {settings} クリップ: {settings} + Sequence: {settings} シーケンス: {settings} + Change the sequence to match the clip's settings? シーケンスをクリップの設定に合わせますか？ + Change Sequence Settings シーケンス設定を変更 + Keep Existing Settings 現在の設定を維持 diff --git a/crates/ui-egui/src/i18n/zh-cn.tsv b/crates/ui-egui/src/i18n/zh-cn.tsv index 244646f47..68682c0af 100644 --- a/crates/ui-egui/src/i18n/zh-cn.tsv +++ b/crates/ui-egui/src/i18n/zh-cn.tsv @@ -3592,3 +3592,10 @@ Open a sequence to search its graphics 打开序列以搜索其中的图形 No text graphics in this sequence. Add text with the Type tool. 此序列中没有文本图形。请用文字工具添加文本。 No matching text. 没有匹配的文本。 + Clip Mismatch Warning 剪辑不匹配警告 + This clip does not match the sequence settings. 此剪辑与序列设置不匹配。 + Clip: {settings} 剪辑：{settings} + Sequence: {settings} 序列：{settings} + Change the sequence to match the clip's settings? 是否更改序列以匹配剪辑的设置？ + Change Sequence Settings 更改序列设置 + Keep Existing Settings 保留现有设置 diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index c838e6fe9..04300fa3b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -217,6 +217,9 @@ pub enum Dialog { SequenceSettings, /// Set Transition Duration (double-click a transition in the Timeline). TransitionDuration, + /// Clip Mismatch Warning: the first clip put in an empty sequence doesn't match its settings + /// (Settings ▸ Timeline ▸ Show Clip Mismatch Warning dialog; `panels::clip_mismatch`). + ClipMismatch, } #[derive(Default)] @@ -357,6 +360,9 @@ 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)>, + /// The edit the Clip Mismatch Warning asks about (command, parameters); it runs with + /// `matchSequence` once answered. + clip_mismatch: Option<(String, serde_json::Value)>, /// 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, @@ -589,6 +595,7 @@ impl FilmcraftApp { last_timeline_width: 1000.0, timeline_view_of: None, pending_timeline_drop: None, + clip_mismatch: None, file_drag_hover: None, timeline_view_last: None, fps: 60.0, diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 1d760d0c0..f8d763acf 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -182,6 +182,10 @@ pub fn invoke(app: &mut FilmcraftApp, ctx: &egui::Context, id: &str, mut params: if id == "edit.selectAll" && (crate::panels::graphics::select_all_edited_text(app) || crate::panels::effect_controls::select_all_keyframes(app)) { return Ok(Value::Null); } + // Insert / Overwrite of a first clip that doesn't match the empty sequence: the Clip Mismatch Warning asks first + if matches!(id, "source.insert" | "source.overwrite") && crate::panels::clip_mismatch::ask(app, id, ¶ms) { + return Ok(json!({"dialog": "clipMismatch"})); + } if filmcraft_engine::source_monitor::source_command(id) && targets_source(app, ¶ms) && params.get("target").is_none() { let object = params.as_object_mut().ok_or("command parameters must be an object")?; object.insert("target".into(), json!("source")); diff --git a/crates/ui-egui/src/panels/clip_mismatch.rs b/crates/ui-egui/src/panels/clip_mismatch.rs new file mode 100644 index 000000000..499b81075 --- /dev/null +++ b/crates/ui-egui/src/panels/clip_mismatch.rs @@ -0,0 +1,108 @@ +//! Settings ▸ Timeline ▸ Show Clip Mismatch Warning dialog: the first clip put in an empty sequence +//! whose frame size, frame rate or pixel aspect ratio differ (a 120 fps phone or game capture in a +//! 23.976 fps sequence) asks whether the sequence takes the clip's settings, for a drag or a file +//! dropped onto the Timeline and for Insert / Overwrite. The answer runs the edit with +//! `matchSequence` ([`filmcraft_engine::commands::clip_mismatch`]); Escape puts nothing in. +//! +//! Automation ids: `clipMismatch.change`, `clipMismatch.keep`. + +use egui::{Frame, Margin, RichText}; +use filmcraft_engine::project::{ItemId, SequenceSettings}; +use serde_json::{Value, json}; + +use crate::{Dialog, FilmcraftApp}; + +/// The clip an edit puts in the sequence: `timeline.place`'s `item` (or the first of its `items`), +/// the Source monitor's clip for Insert and Overwrite. +fn clip_of(app: &FilmcraftApp, command: &str, params: &Value) -> Option { + match command { + "timeline.place" => params.get("item").and_then(Value::as_u64).or_else(|| params.get("items")?.as_array()?.first()?.as_u64()).map(ItemId), + "source.insert" | "source.overwrite" => app.session.state.source_item, + _ => None, + } +} + +/// Opens the warning instead of running `command` when it would put a first clip in an empty +/// sequence that doesn't match it; true when it did. Parameters that already answer it +/// (`matchSequence`) and the setting turned off run the edit as it is. +pub(crate) fn ask(app: &mut FilmcraftApp, command: &str, params: &Value) -> bool { + if !app.session.prefs.timeline.clip_mismatch_warning || params.get("matchSequence").is_some() { + return false; + } + let Some(item) = clip_of(app, command, params) else { return false }; + if filmcraft_engine::commands::clip_mismatch(&app.session, item).is_none() { + return false; + } + app.clip_mismatch = Some((command.to_string(), params.clone())); + app.dialog = Some(Dialog::ClipMismatch); + true +} + +/// Runs an edit dropped onto the Timeline, asking first when [`ask`] does. +pub(crate) fn execute(app: &mut FilmcraftApp, command: &str, params: Value) -> Result { + if ask(app, command, ¶ms) { + return Ok(json!({"dialog": "clipMismatch"})); + } + app.session.execute(command, params).map_err(|e| e.to_string()) +} + +/// `1920 x 1080 · 120 fps`, with the pixel aspect ratio when it isn't square. +fn describe(st: &SequenceSettings) -> String { + let (n, d) = st.par; + let par = if n == d { String::new() } else { format!(" ({:.4})", f64::from(n) / f64::from(d.max(1))) }; + format!("{} x {}{par} · {} fps", st.width, st.height, st.frame_rate.label()) +} + +/// The warning; returns whether it stays open. +pub fn show(app: &mut FilmcraftApp, ctx: &egui::Context) -> bool { + let Some((command, params)) = app.clip_mismatch.clone() else { return false }; + // the sequence or the clip may have changed under it (an undo, another sequence opened) + let clip = clip_of(app, &command, ¶ms).and_then(|i| filmcraft_engine::commands::clip_mismatch(&app.session, i)); + let (Some(clip), Some(seq)) = (clip, app.session.active_sequence().map(|q| q.settings.clone())) else { + app.clip_mismatch = None; + return false; + }; + let t = app.tokens; + let mut answer = None; + let r = egui::Modal::new(egui::Id::new("clip-mismatch-modal")).frame(super::file_dialogs::modal_frame(&t)).show(ctx, |ui| { + ui.set_width(500.0); + Frame::new().inner_margin(Margin { left: 28, right: 28, top: 24, bottom: 20 }).show(ui, |ui| { + ui.label(RichText::new(tl!("Clip Mismatch Warning")).size(18.0).strong().color(t.text)); + ui.add_space(6.0); + ui.separator(); + ui.add_space(6.0); + ui.label(RichText::new(tl!("This clip does not match the sequence settings.")).size(13.5).color(t.text)); + ui.add_space(8.0); + ui.label(RichText::new(tlf!("Clip: {settings}", settings = describe(&clip))).size(12.5).color(t.text_dim)); + ui.label(RichText::new(tlf!("Sequence: {settings}", settings = describe(&seq))).size(12.5).color(t.text_dim)); + ui.add_space(8.0); + ui.label(RichText::new(tl!("Change the sequence to match the clip's settings?")).size(13.5).color(t.text)); + ui.add_space(18.0); + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + if super::file_dialogs::button(app, ui, "clipMismatch.change", tl!("Change Sequence Settings"), true) { + answer = Some(true); + } + if super::file_dialogs::button(app, ui, "clipMismatch.keep", tl!("Keep Existing Settings"), false) { + answer = Some(false); + } + }); + }); + }); + if let Some(change) = answer { + app.clip_mismatch = None; + let mut params = params; + if let Some(p) = params.as_object_mut() { + p.insert("matchSequence".into(), json!(change)); + } + if let Err(e) = app.session.execute(&command, params) { + app.ui.status = e.to_string(); + } + return false; + } + // (the modal's own close also fires on Escape: not while a drag is being cancelled, #580) + if r.should_close() && !ctx.input(|i| i.pointer.any_down()) { + app.clip_mismatch = None; + return false; + } + true +} diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 8bc23dbc3..2c426acc5 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -81,6 +81,12 @@ pub fn show(app: &mut FilmcraftApp, ctx: &egui::Context) { } return; } + Dialog::ClipMismatch => { + if !crate::panels::clip_mismatch::show(app, ctx) && app.dialog == Some(d) { + app.dialog = None; + } + return; + } Dialog::NewSequence | Dialog::Preferences | Dialog::Recovery | Dialog::RevertConfirm => {} } if !open { diff --git a/crates/ui-egui/src/panels/file_dialogs.rs b/crates/ui-egui/src/panels/file_dialogs.rs index 23905b4ba..be0d82e27 100644 --- a/crates/ui-egui/src/panels/file_dialogs.rs +++ b/crates/ui-egui/src/panels/file_dialogs.rs @@ -16,11 +16,11 @@ pub struct FileDialogState { pub recovery_choice: usize, } -fn modal_frame(t: &Tokens) -> Frame { +pub(crate) fn modal_frame(t: &Tokens) -> Frame { Frame::new().fill(t.panel_bg).stroke(Stroke::new(1.0, t.separator)).corner_radius(CornerRadius::same(10)).inner_margin(Margin::same(0)) } -fn button(app: &mut FilmcraftApp, ui: &mut Ui, id: &str, label: &str, primary: bool) -> bool { +pub(crate) fn button(app: &mut FilmcraftApp, ui: &mut Ui, id: &str, label: &str, primary: bool) -> bool { let t = app.tokens; let text = RichText::new(label).size(13.0).color(if primary { Color32::WHITE } else { t.text }); let b = egui::Button::new(text) diff --git a/crates/ui-egui/src/panels/mod.rs b/crates/ui-egui/src/panels/mod.rs index ff18ab0f2..74648766f 100644 --- a/crates/ui-egui/src/panels/mod.rs +++ b/crates/ui-egui/src/panels/mod.rs @@ -3,6 +3,7 @@ pub mod audio_fx_editor; pub mod clip_dialogs; +pub mod clip_mismatch; pub mod color_dialogs; pub mod dialogs; pub mod effect_controls; diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index a73611886..71f0f7464 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -2996,8 +2996,8 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & params["video"] = json!(source.video); params["audio"] = json!(source.audio); } - if let Err(e) = app.session.execute("timeline.place", params) { - app.ui.status = e.to_string(); + if let Err(e) = crate::panels::clip_mismatch::execute(app, "timeline.place", params) { + app.ui.status = e; } crate::panels::clear_drag(ui); } @@ -3098,8 +3098,8 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & // one command, one undo step: the files go end to end from the drop point let ids: Vec = items.iter().map(|i| i.0).collect(); let params = json!({"items": ids, "track": vt, "audioTrack": at, "time": start.0, "insert": false}); - if let Err(e) = app.session.execute("timeline.place", params) { - app.ui.status = e.to_string(); + if let Err(e) = crate::panels::clip_mismatch::execute(app, "timeline.place", params) { + app.ui.status = e; } } } diff --git a/crates/ui-egui/tests/clip_mismatch_ui.rs b/crates/ui-egui/tests/clip_mismatch_ui.rs new file mode 100644 index 000000000..9db5c5a59 --- /dev/null +++ b/crates/ui-egui/tests/clip_mismatch_ui.rs @@ -0,0 +1,227 @@ +//! Headless UI tests of the Clip Mismatch Warning (Settings ▸ Timeline ▸ Show Clip Mismatch Warning +//! dialog, `panels::clip_mismatch`): the first clip dragged onto an empty sequence it doesn't match, +//! or put in with Overwrite, asks whether the sequence takes the clip's settings. +//! +//! Set `FILMCRAFT_UI_SNAPSHOT_DIR=` to also render offscreen with wgpu and write +//! `clip-mismatch.png`. + +use std::sync::mpsc::{Sender, channel}; + +use egui_kittest::Harness; +use filmcraft_engine::Session; +use filmcraft_project::BinEntry; +use filmcraft_time::FrameRate; +use filmcraft_ui_egui::FilmcraftApp; +use filmcraft_ui_egui::control::ControlRequest; +use serde_json::{Value, json}; + +/// The empty sequence the tests open, and the demo's movies, which don't match it. +const SEQUENCE: (u32, u32, FrameRate) = (1280, 720, FrameRate::FPS_25); +const CLIP: (u32, u32, FrameRate) = (1920, 1080, FrameRate::FPS_23_976); + +struct Driver { + harness: Harness<'static, FilmcraftApp>, + tx: Sender, + snapshots: Option, +} + +impl Driver { + /// The demo project with a new, empty 1280 x 720, 25 fps sequence open. + fn new() -> Self { + let mut s = Session::default(); + s.execute("file.openDemoProject", json!({})).unwrap(); + s.execute("file.newSequence", json!({"name": "Empty", "width": 1280, "height": 720, "fps": 25})).unwrap(); + let (tx, rx) = channel(); + let app = FilmcraftApp::new(s).with_control(rx); + let snapshots = std::env::var_os("FILMCRAFT_UI_SNAPSHOT_DIR").map(std::path::PathBuf::from); + let mut b = Harness::builder().with_size(egui::vec2(1600.0, 980.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 + } + + 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 click(&mut self, id: &str) { + self.ok("ui.click", json!({"id": id})); + self.frames(3); + } + + fn has(&mut self, id: &str) -> bool { + let v = self.ok("ui.elements", json!({"prefix": id})); + v.as_array().unwrap().iter().any(|e| e["id"] == json!(id)) + } + + fn wait_for(&mut self, id: &str) { + for _ in 0..400 { + if self.has(id) { + return; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + self.frames(1); + } + panic!("{id} never appeared"); + } + + fn app(&mut self) -> &mut FilmcraftApp { + self.harness.state_mut() + } + + fn dialog(&mut self) -> Value { + self.ok("ui.inspect", json!({}))["dialog"].clone() + } + + fn settings(&mut self) -> (u32, u32, FrameRate) { + let q = self.app().session.active_sequence().unwrap(); + (q.settings.width, q.settings.height, q.settings.frame_rate) + } + + fn clips(&mut self) -> usize { + self.app().session.active_sequence().unwrap().all_tracks().map(|t| t.items.len()).sum() + } + + /// The first movie of the demo's Footage bin, shown in the Project panel (the bin opened in place). + fn movie(&mut self) -> u64 { + let p = &self.app().session.project; + let bin = p + .root + .children + .iter() + .find_map(|e| match e { + BinEntry::Bin(b) if b.name == "Footage" => Some(b.clone()), + _ => None, + }) + .expect("Footage bin"); + let movie = bin + .children + .iter() + .find_map(|e| match e { + BinEntry::Item(i) if p.item(*i).is_some_and(|it| it.has_video() && it.has_audio()) => Some(i.0), + _ => None, + }) + .expect("a movie in Footage"); + self.ok("ui.menu.invoke", json!({"id": "projectPanel.openBin", "params": {"bin": bin.id.0, "how": "inPlace"}})); + self.wait_for(&format!("project.item.{movie}")); + movie + } + + fn drag_to_timeline(&mut self, item: u64) { + self.ok("ui.drag", json!({"from": {"id": format!("project.item.{item}")}, "to": {"id": "timeline.tracks", "fx": 0.3, "fy": 0.1}, "steps": 16})); + self.frames(4); + } + + 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}"), + } + } +} + +#[test] +fn a_first_clip_that_does_not_match_asks_and_the_sequence_can_take_its_settings() { + let mut d = Driver::new(); + let movie = d.movie(); + d.drag_to_timeline(movie); + let status = d.app().ui.status.clone(); + assert_eq!(d.dialog(), json!("ClipMismatch"), "status: {status}"); + assert_eq!(d.clips(), 0, "nothing goes in before the answer"); + assert!(d.has("clipMismatch.change") && d.has("clipMismatch.keep")); + d.snapshot("clip-mismatch"); + d.click("clipMismatch.change"); + assert_eq!(d.dialog(), Value::Null); + assert_eq!((d.settings(), d.clips()), (CLIP, 2), "picture and sound, in a sequence that took the clip's settings"); + // one undo takes the clip out and puts the sequence's settings back + d.exec("edit.undo", json!({})); + assert_eq!((d.settings(), d.clips()), (SEQUENCE, 0)); +} + +#[test] +fn keep_existing_settings_puts_the_clip_in_as_it_is() { + let mut d = Driver::new(); + let movie = d.movie(); + d.drag_to_timeline(movie); + assert_eq!(d.dialog(), json!("ClipMismatch")); + d.click("clipMismatch.keep"); + assert_eq!(d.dialog(), Value::Null); + assert_eq!((d.settings(), d.clips()), (SEQUENCE, 2)); + // the sequence holds clips now: the next one goes in without asking + d.drag_to_timeline(movie); + assert_eq!(d.dialog(), Value::Null); +} + +#[test] +fn escape_puts_nothing_in() { + let mut d = Driver::new(); + let movie = d.movie(); + d.drag_to_timeline(movie); + assert_eq!(d.dialog(), json!("ClipMismatch")); + d.ok("ui.key", json!({"key": "Escape"})); + d.frames(3); + assert_eq!(d.dialog(), Value::Null); + assert_eq!((d.settings(), d.clips()), (SEQUENCE, 0)); +} + +#[test] +fn with_the_warning_turned_off_the_clip_goes_in_as_it_is() { + let mut d = Driver::new(); + d.exec("prefs.set", json!({"key": "timeline.clipMismatchWarning", "value": false})); + let movie = d.movie(); + d.drag_to_timeline(movie); + assert_eq!(d.dialog(), Value::Null); + assert_eq!((d.settings(), d.clips()), (SEQUENCE, 2)); +} + +#[test] +fn overwrite_from_the_source_monitor_asks_too() { + let mut d = Driver::new(); + let movie = d.movie(); + d.exec("source.open", json!({"item": movie})); + // the menu path, as the Overwrite button and the . key take + let r = d.ok("ui.menu.invoke", json!({"id": "source.overwrite", "params": {}})); + assert_eq!(r, json!({"dialog": "clipMismatch"})); + d.frames(2); + assert_eq!(d.dialog(), json!("ClipMismatch")); + assert_eq!(d.clips(), 0); + d.click("clipMismatch.change"); + assert_eq!((d.settings(), d.clips()), (CLIP, 2)); +} From e317859dd23da981c0e85ce57dd12107e4161ca5 Mon Sep 17 00:00:00 2001 From: keigoly <61711237+keigoly@users.noreply.github.com> Date: Tue, 20 Oct 2026 22:38:45 +0900 Subject: [PATCH 2/3] source_drag_ui: the Clip Mismatch Warning stays out of the drag tests The tests drag the demo's 1080p 23.976 clip into an empty 720p 24 fps sequence, which now asks first. They are about the Source monitor's drag controls, so they turn the warning off; clip_mismatch_ui.rs covers the warning itself. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/tests/source_drag_ui.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/ui-egui/tests/source_drag_ui.rs b/crates/ui-egui/tests/source_drag_ui.rs index 92f7fc9de..6e4f07f9b 100644 --- a/crates/ui-egui/tests/source_drag_ui.rs +++ b/crates/ui-egui/tests/source_drag_ui.rs @@ -14,6 +14,9 @@ fn step(h: &mut Harness<'static, FilmcraftApp>) { } fn harness(item: u64) -> Harness<'static, FilmcraftApp> { let mut s = Session::default(); + // the demo's 1080p 23.976 clip doesn't match the empty 720p 24 fps sequence: these tests are + // about the drag, so the Clip Mismatch Warning (clip_mismatch_ui.rs) stays out of the way + s.prefs.timeline.clip_mismatch_warning = false; s.execute("file.openDemoProject", json!({})).unwrap(); s.execute("file.newSequence", json!({"name":"Drop test","width":1280,"height":720,"fps":24})).unwrap(); s.execute("source.open", json!({"item":item})).unwrap(); From 9338b4636cd0ebe786b01e7bc20434bc18bbcb14 Mon Sep 17 00:00:00 2001 From: keigoly <61711237+keigoly@users.noreply.github.com> Date: Wed, 21 Oct 2026 03:03:42 +0900 Subject: [PATCH 3/3] Clip Mismatch Warning: the setting is wired Settings > Timeline > Show Clip Mismatch Warning dialog now has an effect, so its row says so (`wired: true`) and docs/project-files.md lists it with the Timeline settings that take effect. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/clip_mismatch_tests.rs | 6 ++++++ crates/engine/src/settings.rs | 2 +- docs/project-files.md | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/clip_mismatch_tests.rs b/crates/engine/src/clip_mismatch_tests.rs index 735b90e8d..77004430e 100644 --- a/crates/engine/src/clip_mismatch_tests.rs +++ b/crates/engine/src/clip_mismatch_tests.rs @@ -138,3 +138,9 @@ fn match_sequence_on_items_insert_and_overwrite() { assert_eq!((settings(&s).2, clips(&s)), (FrameRate::FPS_23_976, 0), "{command}"); } } + +#[test] +fn the_setting_is_wired() { + // the UI's Clip Mismatch Warning reads it, so Settings no longer lists it as kept only for parity + assert!(crate::settings::field("timeline.clipMismatchWarning").is_some_and(|f| f.wired)); +} diff --git a/crates/engine/src/settings.rs b/crates/engine/src/settings.rs index f1f39c484..33e60deae 100644 --- a/crates/engine/src/settings.rs +++ b/crates/engine/src/settings.rs @@ -1343,7 +1343,7 @@ static CATEGORIES: &[Category] = &[ b("timeline.returnToBeginning", "At playback end, return to beginning when restarting playback", true), b("timeline.outOfSyncUnlinked", "Display out of sync indicators for unlinked clips", false), b("timeline.playAfterRendering", "Play after rendering previews", true), - b("timeline.clipMismatchWarning", "Show Clip Mismatch Warning dialog", false), + b("timeline.clipMismatchWarning", "Show Clip Mismatch Warning dialog", true), b("timeline.matchFrameSetsIn", "Match frame sets in point", false), b("timeline.restoreOpenSequences", "Restore open sequences when opening projects", true), b("timeline.addTracksAutomatically", "Add tracks automatically when editing source clips onto the Timeline", false), diff --git a/docs/project-files.md b/docs/project-files.md index 3ca0bc0be..b755f5441 100644 --- a/docs/project-files.md +++ b/docs/project-files.md @@ -237,7 +237,7 @@ opens the dialog on a page. | Memory | frame cache budget of the monitors and thumbnails | | Playback | preroll / postroll (Play Around, trim loop), Step forward/back many | | Plugins | stub (no plugins) | -| Timeline | video / audio transition and still image default durations (frames or seconds), playback auto-scrolling (none / page / smooth), snap playhead, return to beginning at playback end, play after rendering previews | +| Timeline | video / audio transition and still image default durations (frames or seconds), playback auto-scrolling (none / page / smooth), snap playhead, return to beginning at playback end, play after rendering previews, Show Clip Mismatch Warning dialog (the first clip put into an empty sequence it doesn't match asks whether the sequence takes the clip's settings) | | Trim | Large Trim Offset, Selection tool picks roll (on the cut) / ripple (on the edge) without a modifier, playhead determines trim loop | The remaining fields are kept for parity and shown, but have no effect yet (`wired: false`).