diff --git a/Cargo.lock b/Cargo.lock index 68dc628b1..5529201a2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1686,6 +1686,7 @@ dependencies = [ "objc2 0.6.4", "objc2-app-kit 0.3.2", "objc2-foundation 0.3.2", + "open", "rfd", "serde_json", "sys-locale", @@ -1829,7 +1830,6 @@ dependencies = [ "filmcraft-vp9", "image", "log", - "rayon", "serde_json", "symphonia", "thiserror 2.0.21", @@ -3233,6 +3233,25 @@ dependencies = [ "generic-array", ] +[[package]] +name = "is-docker" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "928bae27f42bc99b60d9ac7334e3a21d10ad8f1835a4e12ec3ec0464765ed1b3" +dependencies = [ + "once_cell", +] + +[[package]] +name = "is-wsl" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "173609498df190136aa7dea1a91db051746d339e18476eed5ca40521f02d7aa5" +dependencies = [ + "is-docker", + "once_cell", +] + [[package]] name = "itertools" version = "0.15.0" @@ -4158,6 +4177,16 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c08d65885ee38876c4f86fa503fb49d7b507c2b62552df7c70b2fce627e06381" +[[package]] +name = "open" +version = "5.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aa576c76302b7b808eecc68061e67336c47833ef9d22caa74dda10fa9675eebc" +dependencies = [ + "is-wsl", + "libc", +] + [[package]] name = "openssl-probe" version = "0.2.1" diff --git a/apps/filmcraft-cli/src/args.rs b/apps/filmcraft-cli/src/args.rs index 49cb2acd5..ef4187665 100644 --- a/apps/filmcraft-cli/src/args.rs +++ b/apps/filmcraft-cli/src/args.rs @@ -61,6 +61,12 @@ impl Args { self.opts.iter().any(|(n, _)| n == k) } + /// A version query: `--version` anywhere, or `-V` in the subcommand position, so global + /// options may come first (`--demo --version`, `--compact -V`). + pub fn wants_version(&self) -> bool { + self.flag("--version") || self.pos(0) == Some("-V") + } + /// The first option that is not in `allowed`, if any. pub fn unknown_opt(&self, allowed: &[&str]) -> Option<&str> { self.opts.iter().map(|(n, _)| n.as_str()).find(|n| !allowed.contains(n)) @@ -141,6 +147,16 @@ mod tests { assert_eq!(a.params_from(2).unwrap(), json!({"a": {"b": [1]}})); } + #[test] + fn version_after_global_options() { + for s in ["--version", "-V", "--demo --version", "--compact -V", "--data-dir d --version", "export --version"] { + assert!(args(s).wants_version(), "{s}"); + } + for s in ["version", "--data-dir -V", "exec x -V", "-- --version"] { + assert!(!args(s).wants_version(), "{s}"); + } + } + #[test] fn unknown_opt_reports_first_unlisted_option() { let allowed = ["--bridge", "--project"]; diff --git a/apps/filmcraft-cli/src/main.rs b/apps/filmcraft-cli/src/main.rs index 666457778..9ead306d5 100644 --- a/apps/filmcraft-cli/src/main.rs +++ b/apps/filmcraft-cli/src/main.rs @@ -252,12 +252,6 @@ async fn main() { } async fn cli() { - if matches!(std::env::args().nth(1).as_deref(), Some("--version" | "-V")) { - outln!("filmcraft-cli {}", env!("CARGO_PKG_VERSION")); - return; - } - register_hardware_decoders(); - register_gpu_frame_renderer(); let a = Args::parse(std::env::args().skip(1)); // `--help` anywhere (`filmcraft-cli --help`, `filmcraft-cli export --help`) prints the reference: // the parser takes it as a flag option, so it never reaches the subcommand match. @@ -265,6 +259,13 @@ async fn cli() { out!("{HELP}"); return; } + // `--version` anywhere and `-V` as the subcommand, also after global options (`--demo --version`, #710). + if a.wants_version() { + outln!("filmcraft-cli {}", env!("CARGO_PKG_VERSION")); + return; + } + register_hardware_decoders(); + register_gpu_frame_renderer(); let Some(cmd) = a.pos(0) else { usage("missing subcommand") }; match cmd { "help" | "-h" => out!("{HELP}"), diff --git a/apps/filmcraft-cli/tests/cli.rs b/apps/filmcraft-cli/tests/cli.rs index 2736806e5..8552cc640 100644 --- a/apps/filmcraft-cli/tests/cli.rs +++ b/apps/filmcraft-cli/tests/cli.rs @@ -24,6 +24,16 @@ fn help_and_usage_errors() { assert_eq!(cli(&["exec", "file.newBin", "notkv"]).status.code(), Some(2)); } +#[test] +fn version_after_global_options() { + let want = format!("filmcraft-cli {}\n", env!("CARGO_PKG_VERSION")); + for args in [&["--version"][..], &["-V"], &["version"], &["--demo", "--version"], &["--compact", "-V"], &["--demo", "version"]] { + let o = cli(args); + assert!(o.status.success(), "{args:?}: {}", String::from_utf8_lossy(&o.stderr)); + assert_eq!(String::from_utf8_lossy(&o.stdout), want, "{args:?}"); + } +} + #[test] fn long_help_matches_help_command() { let reference = cli(&["help"]); diff --git a/apps/filmcraft/Cargo.toml b/apps/filmcraft/Cargo.toml index dc9b1c472..85175489d 100644 --- a/apps/filmcraft/Cargo.toml +++ b/apps/filmcraft/Cargo.toml @@ -36,6 +36,8 @@ cpal = "0.16" arboard = "3" chrono = { version = "0.4", default-features = false, features = ["clock"] } sys-locale = "0.3" +# Help menu / About / Discord links in the system browser (MIT; as PhotoCraft). +open = "5" [dev-dependencies] glob = "0.3" diff --git a/apps/filmcraft/src/main.rs b/apps/filmcraft/src/main.rs index 646c22bac..591bc00fb 100644 --- a/apps/filmcraft/src/main.rs +++ b/apps/filmcraft/src/main.rs @@ -237,6 +237,10 @@ fn main() -> eframe::Result { app.hooks.pick_folder_at = 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)); + // Help menu, About and Discord links (#642): eframe is built without its `links` + // feature, so `ctx.open_url` does nothing on the desktop. Detached, so a launcher + // that waits for the browser never freezes the UI. + app.hooks.open_url = Some(Box::new(|url: &str| open::that_detached(url).map_err(|e| e.to_string()))); app.hooks.clipboard_media = Some(Box::new(clipboard_media)); // Settings ▸ General ▸ Interface Language ▸ System Language (#218). app.hooks.cursor_screen_position = Some(Box::new(filmcraft_platform::cursor::cursor_screen_position)); diff --git a/contributors/people.toml b/contributors/people.toml index 92d8f4b70..e33a37927 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -64,3 +64,8 @@ sync_github_name = false [people.ossini] sync_github_name = true + +[people.vangel76] +real_name = "Valentin Klein" +display_name = "Valentin Klein" +sync_github_name = false diff --git a/crates/audio-dsp/src/effects/dynamics.rs b/crates/audio-dsp/src/effects/dynamics.rs index 40dec64cb..5807d1ba8 100644 --- a/crates/audio-dsp/src/effects/dynamics.rs +++ b/crates/audio-dsp/src/effects/dynamics.rs @@ -263,8 +263,6 @@ pub struct Limiter { true_peak: bool, /// Look-ahead (samples) and detector alignment delay. look: usize, - /// Delay-line capacity (samples of look-ahead) allocated at construction for 0…30 ms. - max_look: usize, det_delay: usize, bank: PolyphaseBank, interp: Vec, @@ -287,21 +285,17 @@ impl Limiter { ParamSpec::new("input_gain", "Input Boost", 0.0, 24.0, 0.0, Unit::Decibels), ParamSpec::new("ceiling", "Ceiling", -24.0, 0.0, -1.0, Unit::Decibels), ParamSpec::log("release", "Release", 1.0, 1000.0, 60.0, Unit::Milliseconds), - ParamSpec::new("lookahead", "Look-Ahead", 0.0, 30.0, 5.0, Unit::Milliseconds), ParamSpec::toggle("true_peak", "True Peak", true), ]; - /// Default look-ahead when the parameter is left at its DSP default. + /// Fixed look-ahead time. pub const LOOKAHEAD_MS: f32 = 5.0; - /// Longest look-ahead the delay line is sized for (Hard Limiter's 0…30 ms range). - pub const MAX_LOOKAHEAD_MS: f32 = 30.0; pub fn new(sample_rate: f32, channels: usize) -> Self { let channels = channels.max(1); let bank = PolyphaseBank::new(true_peak_factor(sample_rate as f64).max(2)); - let max_look = ((Self::MAX_LOOKAHEAD_MS * 0.001 * sample_rate).round() as usize).max(1); - let look = ((Self::LOOKAHEAD_MS * 0.001 * sample_rate).round() as usize).max(1).min(max_look); + let look = ((Self::LOOKAHEAD_MS * 0.001 * sample_rate).round() as usize).max(1); let det_delay = bank.delay(); - let dlen = max_look + det_delay + 1; + let dlen = look + det_delay + 1; let mut s = Limiter { pv: ParamValues::new(Self::PARAMS), sr: sample_rate, @@ -310,7 +304,6 @@ impl Limiter { rel: 0.0, true_peak: true, look, - max_look, det_delay, bank, interp: vec![Interpolator::default(); channels], @@ -333,15 +326,6 @@ impl Limiter { self.ceiling = db_to_gain(self.pv.v("ceiling")); self.rel = coef(self.pv.v("release"), self.sr); self.true_peak = self.pv.on("true_peak"); - let look = ((self.pv.v("lookahead") * 0.001 * self.sr).round() as usize).max(1).min(self.max_look); - if look != self.look { - self.look = look; - self.minq.clear(); - self.box_ring = vec![1.0; look + 1]; - self.box_pos = 0; - self.box_sum = (look + 1) as f64; - self.rel_state = 1.0; - } if snap { self.in_gain.snap(); } @@ -379,7 +363,7 @@ impl AudioEffect for Limiter { for i in 0..n { let g_in = self.in_gain.tick(); let det_pos = (self.wpos + dlen - self.det_delay) % dlen; - let out_pos = (self.wpos + dlen - (self.look + self.det_delay)) % dlen; + let out_pos = (self.wpos + 1) % dlen; // oldest = look + det_delay samples ago let mut det = 0.0f32; for ch in 0..nch { let x = channels[ch][i] * g_in; @@ -571,24 +555,4 @@ mod tests { let peak = tail.iter().fold(0.0f32, |a, v| a.max(v.abs())); assert!((lin_to_db(peak) + 6.0).abs() < 0.3, "{}", lin_to_db(peak)); } - - #[test] - fn limiter_lookahead_changes_latency_and_transient_shape() { - let mut a = Limiter::new(SR, 1); - let mut b = Limiter::new(SR, 1); - a.set_param("lookahead", 0.0); - b.set_param("lookahead", 30.0); - a.set_param("ceiling", -6.0); - b.set_param("ceiling", -6.0); - assert!(b.latency() > a.latency(), "{} vs {}", b.latency(), a.latency()); - let mut xa: Vec = (0..4800).map(|i| 0.08 * (2.0 * std::f32::consts::PI * 997.0 * i as f32 / SR).sin()).collect(); - for s in &mut xa[1200..1248] { - *s = 0.9; - } - let mut xb = xa.clone(); - a.process(&mut [&mut xa]); - b.process(&mut [&mut xb]); - let max = xa.iter().zip(&xb).map(|(x, y)| (x - y).abs()).fold(0.0f32, f32::max); - assert!(max > 1e-4, "look-ahead of 0 ms and 30 ms produced the same waveform (max Δ {max})"); - } } diff --git a/crates/captions/src/ttml.rs b/crates/captions/src/ttml.rs index 1987ba328..6c98a7ff7 100644 --- a/crates/captions/src/ttml.rs +++ b/crates/captions/src/ttml.rs @@ -287,8 +287,12 @@ fn parse_time(s: &str, tm: Timing) -> Option { "f" => { let n: i128 = vi.parse().ok()?; let fd = tm.rate.tick_of(1).0 as i128; - let whole = tm.rate.tick_of(n as i64).0 as i128; - whole + decimal_ticks("0", vf, fd)? + let whole = tm.rate.tick_of(i64::try_from(n).ok()?); + // tick_of saturates; a saturated count overflowed rather than named a real time + if whole == Tick(i64::MAX) { + return None; + } + whole.0 as i128 + decimal_ticks("0", vf, fd)? } "t" => { let n: i128 = vi.parse().ok()?; diff --git a/crates/codecs/Cargo.toml b/crates/codecs/Cargo.toml index 7caf5eb98..3a490e2c4 100644 --- a/crates/codecs/Cargo.toml +++ b/crates/codecs/Cargo.toml @@ -31,7 +31,6 @@ filmcraft-av1 = { workspace = true } filmcraft-apv = { workspace = true } filmcraft-aac = { workspace = true } filmcraft-opus = { workspace = true } -rayon = { workspace = true } thiserror = { workspace = true } log = { workspace = true } # Decode timing counters (`gop_stats`); std's Instant panics on wasm32. diff --git a/crates/codecs/src/gop.rs b/crates/codecs/src/gop.rs index 4f4428875..b23b130e5 100644 --- a/crates/codecs/src/gop.rs +++ b/crates/codecs/src/gop.rs @@ -109,9 +109,8 @@ impl std::ops::Sub for GopStats { } } -/// A container's video sample table, in decode (file) order. `Sync`: a cache decodes on its own -/// threads ([`isolated`]). -pub trait VideoSamples: Sync { +/// A container's video sample table, in decode (file) order. +pub trait VideoSamples { fn count(&self) -> usize; /// Presentation timestamp of sample `i` (track units). fn pts(&self, i: usize) -> i64; @@ -426,57 +425,6 @@ const MIN_FRAMES: usize = 64; /// rather than restarting at the sync sample. const CONTINUE_THROUGH: usize = 48; -/// Threads that run the caches' decoding (see [`isolated`]); `None` where none can be made. -#[cfg(not(target_arch = "wasm32"))] -fn decode_pool() -> Option<&'static rayon::ThreadPool> { - static POOL: std::sync::OnceLock> = std::sync::OnceLock::new(); - POOL.get_or_init(|| { - let n = std::thread::available_parallelism().map(|n| n.get()).unwrap_or(1); - rayon::ThreadPoolBuilder::new().num_threads(n).thread_name(|i| format!("decode-{i}")).build().ok() - }) - .as_ref() -} - -/// Run a cache's decoding, its lock held by the caller, on [`decode_pool`], the caller waiting -/// without doing other rayon work meanwhile (#480). -/// -/// That decoding starts rayon work (a decoder's slices, cropping and rotating the frames). On the -/// pool of the render jobs that ask the cache for frames, a worker that took part of it could, -/// waiting inside it, pick up another of those jobs and block on the lock: the lock holder then -/// waits for its work on the blocked worker's stack, and that worker for the lock, for good (an -/// export of rotated iPhone clips stopped making frames). On the decode pool that work only ever -/// runs beside other caches' decoding, which never waits for a cache's lock. A caller that is a -/// rayon worker would also run other jobs of its pool while it waits for the decode pool, some -/// of them asking for a cache's lock while it holds this one: it waits on a plain thread instead. -fn isolated(f: impl FnOnce() -> crate::Result + Send) -> crate::Result { - let ctx = filmcraft_media::cancel::Context::current(); - let job = move || ctx.run(f); - #[cfg(not(target_arch = "wasm32"))] - if let Some(pool) = decode_pool() { - if rayon::current_thread_index().is_none() { - return pool.install(job); - } - let slot = Mutex::new(Some(job)); - let waited = std::thread::scope(|scope| { - let spawned = std::thread::Builder::new().name("decode-wait".into()).spawn_scoped(scope, || { - let job = slot.lock().unwrap_or_else(PoisonError::into_inner).take(); - job.map(|job| pool.install(job)) - }); - spawned.map(|h| h.join()) - }); - return match waited { - Ok(Ok(Some(r))) => r, - Ok(Err(_)) => Err(CodecError::Decode("internal error while decoding".into())), - // no thread to wait on: decode here, as before - Ok(Ok(None)) | Err(_) => match slot.into_inner().unwrap_or_else(PoisonError::into_inner) { - Some(job) => job(), - None => Err(CodecError::Decode("internal error while decoding".into())), - }, - }; - } - job() -} - impl GopCache { pub fn new(explicit_color: Option) -> Self { // unit tests count decoder restarts: each of their caches gets a pool of its own @@ -534,11 +482,7 @@ impl GopCache { } fn store(&self, st: &mut State, pts: i64, f: VideoFrame, draft: bool) { - self.store_finished(st, pts, self.finish(f), draft); - } - - /// Cache a frame [`Self::finish`] has been applied to. - fn store_finished(&self, st: &mut State, pts: i64, f: VideoFrame, draft: bool) { + let f = self.finish(f); st.last_used = Instant::now(); if draft { st.drafts.insert(pts); @@ -660,22 +604,6 @@ impl GopCache { if st.intra { return self.intra_frame(st, s, i, want_pts); } - // with the lock held: see `isolated` - let st = &mut *st; - isolated(|| self.decode_until(st, s, i, want_pts, late_before, draft)) - } - - /// Decode (the lock held) until the frame with `want_pts` (sample `i`) is cached; return it. - fn decode_until( - &self, - st: &mut State, - s: &dyn VideoSamples, - i: usize, - want_pts: i64, - late_before: Option, - draft: bool, - ) -> crate::Result> { - let n = s.count(); let mut key = s.sync_before(i); // Continue the running decoder when it has passed the wanted sample's sync sample and // either has not reached the sample yet or has been fed it without outputting it yet @@ -728,14 +656,14 @@ impl GopCache { let out = timed(|| decoder.decode(&data, s.pts(k)))?; st.next += 1; DECODED.fetch_add(1, Ordering::Relaxed); - self.store_output(st, out); + self.store_output(&mut st, out); if st.cached(want_pts, draft).is_some() { break; } } if st.cached(want_pts, draft).is_none() { let out = st.decoder.as_mut().map(|d| timed(|| d.flush())).unwrap_or_default(); - self.store_output(st, out); + self.store_output(&mut st, out); st.next = usize::MAX; } // nearest decoded frame at or before the wanted pts (robust to decoder pts quirks) @@ -820,17 +748,14 @@ impl GopCache { DECODED.fetch_add(1, Ordering::Relaxed); timed(|| dec.decode(&data, want_pts)) }); - // crop and rotate before taking the lock: that work runs on rayon (see `isolated`) - let res = res.map(|out| { - count_frames(&out); - out.into_iter().map(|d| (d.pts, self.finish(d.frame), d.draft)).collect::>() - }); let mut st = self.shared.state.lock().unwrap_or_else(|e| e.into_inner()); if st.spare.len() < 16 { st.spare.push(dec); } - for (pts, frame, draft) in res? { - self.store_finished(&mut st, pts, frame, draft); + let res = res?; + count_frames(&res); + for d in res { + self.store(&mut st, d.pts, d.frame, d.draft); } st.at_or_before(want_pts, true).ok_or_else(|| CodecError::Decode("frame not produced".into())) } @@ -962,74 +887,6 @@ mod tests { assert!(s.decodes.load(Ordering::Relaxed) <= 60 + 24 + 1); } - /// A decoder that, like a slice-threaded one, splits each picture's work over rayon. - struct ParDec(Box); - - impl VideoDecoder for ParDec { - fn decode(&mut self, sample: &[u8], pts: i64) -> crate::Result> { - use rayon::prelude::*; - let rows: u64 = - (0..64u32).into_par_iter().with_max_len(1).map(|r| (0..20_000u64).fold(u64::from(r), |a, x| a.wrapping_mul(31).wrapping_add(x))).sum(); - std::hint::black_box(rows); - self.0.decode(sample, pts) - } - fn flush(&mut self) -> Vec { - self.0.flush() - } - fn reset(&mut self) { - self.0.reset() - } - fn name(&self) -> &str { - "par" - } - } - - struct ParSamples(Samples); - - impl VideoSamples for ParSamples { - fn count(&self) -> usize { - self.0.count() - } - fn pts(&self, i: usize) -> i64 { - self.0.pts(i) - } - fn sync_before(&self, i: usize) -> usize { - self.0.sync_before(i) - } - fn sample_at(&self, t: i64) -> Option { - self.0.sample_at(t) - } - fn read(&self, i: usize) -> crate::Result> { - self.0.read(i) - } - fn make_decoder(&self) -> crate::Result> { - Ok(Box::new(ParDec(self.0.make_decoder()?))) - } - } - - /// #480: render jobs on rayon asking one source for frames while its decoding (a decoder's - /// slices, rotating the frames) runs on rayon too. A worker that took part of that work and, - /// waiting inside it, picked up another frame request blocked on the cache's lock for good: - /// an export of rotated iPhone clips stopped making frames. - #[test] - fn rayon_frame_requests_while_decoding_on_rayon_never_deadlock() { - use rayon::prelude::*; - let s = Arc::new(ParSamples(samples(240, 30, 4, false))); - let (tx, rx) = std::sync::mpsc::channel(); - std::thread::spawn(move || { - for round in 0..40usize { - // export-like: many frame workers, frames of several GOPs of a fresh source at once - let c = GopCache::new(None).with_rotation(1); - (0..96usize).into_par_iter().with_max_len(1).for_each(|k| { - let i = (k * 37 + round * 53) % 240; - assert_eq!(index_of(&c.frame(&*s, i as i64 * 1000).expect("frame")), i); - }); - } - let _ = tx.send(()); - }); - assert!(rx.recv_timeout(Duration::from_secs(120)).is_ok(), "frame requests deadlocked"); - } - #[test] fn running_decoder_continues_through_a_nearby_sync_sample() { let s = samples(200, 30, 0, false); diff --git a/crates/codecs/src/hw_frame.rs b/crates/codecs/src/hw_frame.rs index b37fc4d01..dfa64771c 100644 --- a/crates/codecs/src/hw_frame.rs +++ b/crates/codecs/src/hw_frame.rs @@ -82,6 +82,13 @@ pub(crate) fn obus(data: &[u8]) -> Vec<(u8, &[u8])> { out } +/// What an AV1 sequence header fixes about the pictures: profile, maximum size and the colour +/// configuration (bit depth, chroma format, colour description). `None` when it does not parse. +fn av1_format(seq: &[u8]) -> Option<(u8, u32, u32, filmcraft_av1::ColorConfig)> { + let h = filmcraft_av1::SequenceHeader::parse(seq).ok()?; + Some((h.profile, h.max_frame_width, h.max_frame_height, h.color)) +} + /// VP9 colour as our decoder reports it: `color_space` and range from the bitstream, transfer and /// primaries from the container when it says. (Shared with `Vp9Decoder`.) pub(crate) fn vp9_color(width: u32, height: u32, color_space: u8, full_range: bool, transfer: Option, primaries: Option) -> ColorInfo { @@ -243,14 +250,58 @@ impl FrameStreamInfo { } /// Whether `sample` declares parameters the hardware session was not set up for: an AV1 - /// sequence header that differs from `av1C`'s, a VP9 key frame of another size, bit depth or - /// chroma format. The decoder then hands the stream to our software decoder. + /// sequence header whose picture format (profile, size, colour configuration) differs from + /// `av1C`'s, a VP9 key frame of another size, bit depth or chroma format. The decoder then + /// hands the stream to our software decoder. pub fn parameters_changed(&self, sample: &[u8]) -> bool { match self.codec { - FrameCodec::Av1 => obus(sample).into_iter().any(|(t, p)| t == 1 && self.sequence_header.as_deref() != Some(p)), + // Only what the picture format depends on counts: encoders such as SVT-AV1 put a + // provisional sequence header in `av1C` whose coding tool flags (CDEF, restoration, + // warped motion…) differ from the in-band one, and decoders follow the in-band one. + FrameCodec::Av1 => obus(sample).into_iter().any(|(t, p)| t == 1 && self.sequence_header.as_deref().is_none_or(|s| av1_format(s) != av1_format(p))), FrameCodec::Vp9 => { self.picture_params(sample).is_some_and(|p| p.size != self.size || p.bit_depth != self.bit_depth || p.subsampling != self.subsampling) } } } } + +#[cfg(test)] +mod tests { + use super::*; + + /// An `av1C` sequence header from SVT-AV1 (1280x720 10-bit) and the in-band one of the same + /// stream: they differ only in coding tool flags. + const AV1C: [u8; 14] = [0x0a, 0x0c, 0x02, 0x00, 0x00, 0x2d, 0x6a, 0x67, 0xfd, 0x9e, 0x01, 0x7c, 0x20, 0x20]; + const IN_BAND: [u8; 14] = [0x0a, 0x0c, 0x02, 0x00, 0x00, 0x2d, 0x6a, 0x67, 0xfd, 0x9e, 0x35, 0x7c, 0xe0, 0x20]; + + fn info() -> FrameStreamInfo { + let c = filmcraft_isobmff::Av1Config { + seq_profile: 0, + seq_level_idx_0: 0, + seq_tier_0: false, + high_bitdepth: true, + twelve_bit: false, + monochrome: false, + chroma_subsampling_x: true, + chroma_subsampling_y: true, + chroma_sample_position: 0, + initial_presentation_delay_minus_one: None, + config_obus: AV1C.to_vec(), + }; + FrameStreamInfo::from_av1(&c, (0, 0)).unwrap() + } + + #[test] + fn av1_sequence_headers_compare_by_picture_format() { + let i = info(); + assert_eq!((i.size, i.bit_depth), ((1280, 720), 10)); + assert!(!i.parameters_changed(&AV1C), "the same header"); + // used to send every SVT-AV1 MP4 to the software decoder from its first frame + assert!(!i.parameters_changed(&IN_BAND), "only coding tool flags differ"); + // a header that does not parse is a change + assert!(i.parameters_changed(&[0x0a, 0x02, 0xe0, 0x00])); + // no sequence header in the sample: nothing changed + assert!(!i.parameters_changed(&[0x12, 0x00])); + } +} diff --git a/crates/codecs/src/mkv.rs b/crates/codecs/src/mkv.rs index 2efb5b537..97de67431 100644 --- a/crates/codecs/src/mkv.rs +++ b/crates/codecs/src/mkv.rs @@ -104,10 +104,9 @@ fn audio_starts(file: &MkvFile, bytes: &crate::Src, ti: usize, rate: i64) -> Vec fn opus_starts(file: &MkvFile, bytes: &crate::Src, ti: usize, head: &filmcraft_opus::OpusHead, rate: i64) -> Vec { let Some(t) = file.tracks.get(ti) else { return Vec::new() }; let (n, d) = tb(t); - let scale_ns = (n as i128 * 1_000_000_000 / d as i128).max(1); let delay_ns = t.codec_delay_ns as i128; - // The demuxer's rounding of CodecDelay to ticks (half away from zero), undone here. - let delay_ticks = ((delay_ns + scale_ns / 2) / scale_ns) as i64; + // The demuxer's rounding of CodecDelay to ticks, undone here. + let delay_ticks = codec_delay_ticks(t); let skip = if t.codec_delay_ns > 0 { (delay_ns * rate as i128 / 1_000_000_000) as i64 } else { head.pre_skip as i64 }; let at = |pts: i64| ((pts.saturating_add(delay_ticks) as i128 * n as i128 * rate as i128 / d as i128) as i64).saturating_sub(skip); let packets = t.samples.iter().enumerate().map(|(i, s)| crate::audio::PacketTime { @@ -118,6 +117,46 @@ fn opus_starts(file: &MkvFile, bytes: &crate::Src, ti: usize, head: &filmcraft_o crate::audio::contiguous_starts(packets, stamp_tolerance(t, rate)) } +/// `CodecDelay` in timestamp ticks, rounded as the demuxer rounds it (half away from zero) before +/// subtracting it from every timestamp of the track. +fn codec_delay_ticks(t: &filmcraft_matroska::Track) -> i64 { + let (n, d) = tb(t); + let scale_ns = (n as i128 * 1_000_000_000 / d as i128).max(1); + i64::try_from((t.codec_delay_ns as i128 + scale_ns / 2) / scale_ns).unwrap_or(i64::MAX) +} + +/// The track timestamp that media time zero stands for (#715): the earliest start among the +/// playable tracks. Block timestamps count from the start of the Segment, and a remux that keeps +/// its source's timestamps (`ffmpeg -copyts`, a cut from a recording) starts its first picture +/// seconds in; taken as media time, that repeated the opening picture until then. An audio track +/// starts where its first packet does before `CodecDelay` was taken off (the delay only hides +/// priming), and never before zero. +fn media_origin(file: &MkvFile, vtrack: Option, atracks: &[usize]) -> i64 { + let start = |i: usize| { + let t = file.tracks.get(i)?; + let first = t.samples.iter().map(|s| s.pts).min()?; + Some(if t.kind == TrackKind::Audio { first.saturating_add(codec_delay_ticks(t)).max(0) } else { first }) + }; + vtrack.into_iter().chain(atracks.iter().copied()).filter_map(start).min().unwrap_or(0).max(0) +} + +/// Move every track back by `origin` timestamp ticks, so the streams keep their relative timing +/// (a delay between picture and sound survives). +fn rebase(file: &mut MkvFile, origin: i64) { + if origin <= 0 { + return; + } + for s in file.tracks.iter_mut().flat_map(|t| t.samples.iter_mut()) { + s.pts = s.pts.saturating_sub(origin); + } +} + +/// End of the last sample of track `i`, in its timebase. +fn track_end(file: &MkvFile, i: usize) -> Option<(i64, &filmcraft_matroska::Track)> { + let t = file.tracks.get(i)?; + Some((t.samples.iter().map(|s| s.pts.saturating_add(i64::try_from(s.duration).unwrap_or(i64::MAX))).max().unwrap_or(0), t)) +} + /// Two timestamp ticks in sample frames at `rate`, plus one: how far a packet's Matroska timestamp /// can be from its exact start. fn stamp_tolerance(t: &filmcraft_matroska::Track, rate: i64) -> i64 { @@ -146,7 +185,7 @@ fn tb(t: &filmcraft_matroska::Track) -> (i64, i64) { fn to_tick(t: &filmcraft_matroska::Track, pts: i64) -> Tick { let (n, d) = tb(t); - Tick::from_rational(pts * n, 1, d) + Tick::from_rational(pts, n, d) } /// A time as the track timestamp to look a frame up by: rounded to the nearest tick, as muxers @@ -275,7 +314,7 @@ impl MkvSource { /// Open from a random-access reader: only the index is read now, samples on demand. pub fn open_reader(name: &str, reader: filmcraft_media::SharedReader) -> crate::Result { let bytes = crate::Src(reader); - let file = filmcraft_matroska::open(&bytes).map_err(|e| CodecError::Container(e.to_string()))?; + let mut file = filmcraft_matroska::open(&bytes).map_err(|e| CodecError::Container(e.to_string()))?; let vtrack = file.tracks.iter().position(|t| t.kind == TrackKind::Video && !t.samples.is_empty()); // every audio track is a stream (the track count comes from the file: capped) let atracks: Vec = file @@ -289,6 +328,24 @@ impl MkvSource { if vtrack.is_none() && atracks.is_empty() { return Err(CodecError::Unsupported("no playable tracks".into())); } + let main = vtrack.or(atracks.first().copied()); + // The Segment's end (`Info/Duration` or the last sample), in Segment time. + let segment_end = match file.duration_ns() { + Some(ns) => Tick::from_rational(i64::try_from(ns).unwrap_or(i64::MAX), 1, 1_000_000_000), + None => main.and_then(|i| track_end(&file, i)).map(|(end, t)| to_tick(t, end)).unwrap_or_default(), + }; + // Media time zero is the first picture (or sound): the streams start at their common origin. + let origin = media_origin(&file, vtrack, &atracks); + rebase(&mut file, origin); + // `Info/Duration` is the Segment's end: the media lasts from the origin to there (never less + // than its main track's samples). + let duration = match main.and_then(|i| track_end(&file, i)) { + Some((end, t)) if origin > 0 => { + let (n, d) = tb(t); + (segment_end - Tick::from_rational(origin, n, d)).max(to_tick(t, end)) + } + _ => segment_end, + }; let mut explicit_color = None; let mut ventry = None; // display rotation from the track's Projection (portrait phone video is stored landscape) @@ -312,7 +369,7 @@ impl MkvSource { if explicit { explicit_color = Some(color); } - let secs = file.duration_ns().unwrap_or(0) as f64 / 1e9; + let secs = duration.seconds(); let bitrate = (secs > 0.0).then(|| (t.samples.iter().map(|s| s.size as u64).sum::() as f64 * 8.0 / secs) as u64); let ((w, h), par) = if rotation % 2 == 1 { ((h, w), (v.pixel_aspect().1, v.pixel_aspect().0)) } else { ((w, h), v.pixel_aspect()) }; VideoStreamInfo { @@ -342,17 +399,6 @@ impl MkvSource { AudioStreamInfo { sample_rate: rate, channels, codec: codec_label(&t.codec), bits_per_sample: a.bit_depth.map(|b| b as u32) } }) .collect(); - let duration = match file.duration_ns() { - Some(ns) => Tick::from_rational(ns as i64, 1, 1_000_000_000), - None => vtrack - .or(atracks.first().copied()) - .map(|i| { - let t = &file.tracks[i]; - let end = t.samples.iter().map(|s| s.pts + s.duration as i64).max().unwrap_or(0); - to_tick(t, end) - }) - .unwrap_or_default(), - }; let info = MediaInfo { name: name.to_string(), kind: if video.is_some() { MediaKind::Movie } else { MediaKind::AudioOnly }, @@ -614,3 +660,52 @@ mod multi_audio_tests { assert!(source.audio_stream(2, 0, 50, 48_000).is_err()); } } + +#[cfg(test)] +mod origin_tests { + use filmcraft_frame::PixelData; + use filmcraft_matroska::{MkvWriter, MuxOptions, TrackKind, TrackSpec}; + use filmcraft_media::{FrameRequest, MediaSource}; + use filmcraft_time::Tick; + + /// A flat grey Motion-JPEG picture: its level says which frame it is. + fn picture(k: u8) -> Vec { + let img = image::GrayImage::from_pixel(16, 16, image::Luma([20 + 20 * k])); + let mut out = Vec::new(); + image::codecs::jpeg::JpegEncoder::new_with_quality(&mut out, 95).encode_image(&img).unwrap(); + out + } + + /// #715: a remux that keeps its source's timestamps (first picture at 1.48 s, sound 0.2 s after + /// it) starts at its first picture, lasts as long as its pictures, and keeps the sound's delay. + /// It repeated picture 0 until 1.48 s, and the clip ran to the Segment's absolute end. + #[test] + fn timestamp_preserving_remux_starts_at_the_first_picture() { + let mut v = TrackSpec::new(TrackKind::Video, "V_MJPEG"); + v.video_size = Some((16, 16)); + v.default_duration_ns = Some(40_000_000); + let mut a = TrackSpec::new(TrackKind::Audio, "A_PCM/INT/LIT"); + a.audio = Some((48_000.0, 1, Some(16))); + let mut w = MkvWriter::new(std::io::Cursor::new(Vec::new()), vec![v, a], MuxOptions::default()).unwrap(); + for k in 0..10u8 { + let pts = 1_480_000_000 + i64::from(k) * 40_000_000; + w.write_frame(0, pts, true, &picture(k), None).unwrap(); + if k == 5 { + let pcm: Vec = (0..960).flat_map(|_| 16_384i16.to_le_bytes()).collect(); + w.write_frame(1, 1_680_000_000, true, &pcm, Some(20_000_000)).unwrap(); + } + } + let src = super::MkvSource::open("shifted.mkv", w.finish().unwrap().into_inner().into()).unwrap(); + assert_eq!(src.info().duration, Tick::from_rational(10, 1, 25), "ten pictures at 25 fps"); + let level = |k: i64| { + let f = src.video_frame(FrameRequest::full(Tick::from_rational(k, 1, 25))).unwrap(); + let PixelData::Rgba8(px) = &f.data else { panic!("MJPEG decodes to RGBA8") }; + px[0] + }; + let seen: Vec = (0..10).map(level).collect(); + assert!(seen.windows(2).all(|p| p[0] < p[1]), "one picture per frame, in order: {seen:?}"); + let sound = src.audio(0, 48_000 * 2 / 5, 48_000).unwrap(); + assert!(sound.channels[0][9_590].abs() < 0.001, "silent before the delay"); + assert!((sound.channels[0][9_610] - 0.5).abs() < 0.001, "sound 0.2 s after the first picture"); + } +} diff --git a/crates/edit/src/lib.rs b/crates/edit/src/lib.rs index 3bd1b370e..66c524228 100644 --- a/crates/edit/src/lib.rs +++ b/crates/edit/src/lib.rs @@ -491,17 +491,45 @@ pub fn move_items(seq: &mut Sequence, moves: &[(ClipId, TrackId, Tick)], insert_ it.start = (*start).max(Tick::ZERO); placed.push((*dest, it)); } - // carry transitions with moved clips? Premiere drops transitions whose partner is not moved. + let carried = carried_transitions(&work, &placed); + // a transition whose partner stays behind is dropped, as in Premiere delete_items(&mut work, &moves.iter().map(|m| m.0).collect::>()); if insert_mode { insert(&mut work, placed, ctx)?; } else { overwrite(&mut work, placed, ctx)?; } + for (dest, tr) in carried { + let t = track_mut(&mut work, dest)?; + if [tr.from, tr.to].into_iter().flatten().all(|c| t.item(c).is_some()) { + t.transitions.push(tr); + t.sort(); + } + } *seq = work; Ok(()) } +/// The transitions that travel with a move (#227, #374): those whose clips all move, onto one +/// track and by one offset (a fade at a moved clip's edge, or the cut between two clips that move +/// together), shifted to where those clips land. +fn carried_transitions(seq: &Sequence, placed: &[(TrackId, TrackItem)]) -> Vec<(TrackId, Transition)> { + let landing = |c: ClipId| { + let (dest, it) = placed.iter().find(|(_, it)| it.id == c)?; + let (_, old) = seq.find_item(c)?; + Some((*dest, it.start - old.start)) + }; + let mut carried = Vec::new(); + for tr in seq.all_tracks().filter(|t| !t.locked).flat_map(|t| t.transitions.iter()) { + let lands: Option> = [tr.from, tr.to].into_iter().flatten().map(landing).collect(); + let Some((&(dest, delta), rest)) = lands.as_deref().and_then(<[_]>::split_first) else { continue }; + if rest.iter().all(|l| *l == (dest, delta)) { + carried.push((dest, Transition { start: tr.start + delta, ..tr.clone() })); + } + } + carried +} + fn media_len(ctx: &EditCtx, item: &TrackItem) -> Option { if item.frame_hold.is_some() { return None; @@ -573,6 +601,31 @@ pub fn clamp_trim(seq: &Sequence, clip: ClipId, edge: Edge, mode: TrimMode, delt Ok(d) } +/// What moving one edge by `d` (already clamped) does to the item itself: its length, the media +/// it starts at (a reversed clip consumes media from its other end; a frame hold has none to +/// move), and, for the In edge with `move_start`, where it starts. The trims and the timeline's +/// live trim preview share it, so the preview shows the media the edit will keep (#374). +pub fn move_edge(it: &mut TrackItem, edge: Edge, d: Tick, move_start: bool) { + let speed = it.speed.abs(); + match edge { + Edge::In => { + if !it.reverse && it.frame_hold.is_none() { + it.source_in += src_of(d, speed); + } + it.duration -= d; + if move_start { + it.start += d; + } + } + Edge::Out => { + if it.reverse { + it.source_in -= src_of(d, speed); + } + it.duration += d; + } + } +} + /// Trim one edge of an item (Selection-tool edge drag = Regular; Ripple tool = Ripple). /// Linked partners should be trimmed by the caller with the same delta. pub fn trim(seq: &mut Sequence, clip: ClipId, edge: Edge, mode: TrimMode, delta: Tick, ctx: &mut EditCtx) -> Result { @@ -584,27 +637,10 @@ pub fn trim(seq: &mut Sequence, clip: ClipId, edge: Edge, mode: TrimMode, delta: let old_end = it.end(); let old_start = it.start; let own_link = it.link; - let speed = it.speed.abs(); let mut work = seq.clone(); { let (_, it) = work.find_item_mut(clip).ok_or(EditError::NoItem(clip))?; - match edge { - Edge::In => { - if !it.reverse && it.frame_hold.is_none() { - it.source_in += src_of(d, speed); - } - it.duration -= d; - if mode == TrimMode::Regular { - it.start += d; - } - } - Edge::Out => { - if it.reverse { - it.source_in -= src_of(d, speed); - } - it.duration += d; - } - } + move_edge(it, edge, d, mode == TrimMode::Regular); } if mode == TrimMode::Ripple { let (at, shift) = match edge { @@ -662,25 +698,11 @@ pub fn ripple_trim_group(seq: &mut Sequence, clips: &[ClipId], edge: Edge, delta let mut origins: Vec<(TrackId, Tick)> = Vec::new(); for c in clips { let (tid, it) = work.find_item_mut(*c).ok_or(EditError::NoItem(*c))?; - let speed = it.speed.abs(); let from = match edge { Edge::In => it.start + Tick(1), Edge::Out => it.end(), }; - match edge { - Edge::In => { - if !it.reverse && it.frame_hold.is_none() { - it.source_in += src_of(d, speed); - } - it.duration -= d; - } - Edge::Out => { - if it.reverse { - it.source_in -= src_of(d, speed); - } - it.duration += d; - } - } + move_edge(it, edge, d, false); if !origins.iter().any(|(t, _)| *t == tid) { origins.push((tid, from)); } @@ -880,6 +902,27 @@ pub fn slide(seq: &mut Sequence, clip: ClipId, delta: Tick, ctx: &mut EditCtx) - Ok(d) } +/// Slide several items (a linked picture/sound pair, a selection) by one common delta: the most +/// restrictive item's neighbour limit clamps them all, so their relative timing is kept. +/// Returns the delta applied. +pub fn slide_items(seq: &mut Sequence, clips: &[ClipId], delta: Tick, ctx: &mut EditCtx) -> Result { + let mut d = delta; + for c in clips { + let x = slide(&mut seq.clone(), *c, d, ctx)?; + if x.0.signum() != d.0.signum() { + d = Tick::ZERO; + } else if x.abs() < d.abs() { + d = x; + } + } + if d != Tick::ZERO { + for c in clips { + slide(seq, *c, d, ctx)?; + } + } + Ok(d) +} + /// Rate stretch: change an item's duration by dragging an edge, adjusting speed to keep the same media. pub fn rate_stretch(seq: &mut Sequence, clip: ClipId, edge: Edge, delta: Tick, ctx: &mut EditCtx) -> Result { let (tid, it) = seq.find_item(clip).ok_or(EditError::NoItem(clip))?; diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index ae258cb0f..93064ff72 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -464,6 +464,39 @@ fn transitions_stay_on_their_cuts() { fx.seq.check().unwrap(); } +#[test] +fn transitions_travel_with_moved_clips() { + // V1: a [0,10) fading in, b [10,20), c [20,30); b→c crossfade, c fading out + let mut fx = Fx::new(); + let (v1, v2) = (fx.v(0), fx.v(1)); + let a = fx.put(v1, 0, 10, 10); + let b = fx.put(v1, 10, 10, 10); + let c = fx.put(v1, 20, 10, 10); + let tr = |id, start, from, to| Transition { + id: TransitionId(id), + effect: filmcraft_project::find_effect("cross_dissolve").unwrap().instance(), + start: f(start), + duration: f(4), + from, + to, + align: Default::default(), + reverse: false, + }; + fx.seq.track_mut(v1).unwrap().transitions = vec![tr(1, 0, None, Some(a)), tr(2, 18, Some(b), Some(c)), tr(3, 26, Some(c), None)]; + let at = |fx: &Fx, track: TrackId| -> Vec<(u64, i64)> { fx.seq.track(track).unwrap().transitions.iter().map(|t| (t.id.0, R.frame_at(t.start))).collect() }; + let mut n = fx.next; + // a fade at a clip's edge goes with it, to another track too + move_items(&mut fx.seq, &[(a, v2, f(40))], false, &mut Fx::ctx(&mut n)).unwrap(); + assert_eq!((at(&fx, v1), at(&fx, v2)), (vec![(2, 18), (3, 26)], vec![(1, 40)])); + // a cut whose two clips move together keeps its transition, in an insert move as well + move_items(&mut fx.seq, &[(b, v1, f(60)), (c, v1, f(70))], true, &mut Fx::ctx(&mut n)).unwrap(); + assert_eq!(at(&fx, v1), vec![(2, 68), (3, 76)]); + // a crossfade whose partner stays behind is dropped; the other clip's fade stays with it + move_items(&mut fx.seq, &[(c, v1, f(90))], false, &mut Fx::ctx(&mut n)).unwrap(); + assert_eq!(at(&fx, v1), vec![(3, 96)]); + fx.seq.check().unwrap(); +} + #[test] fn rate_stretch_and_speed() { let mut fx = Fx::new(); @@ -749,3 +782,40 @@ fn roll_reversed_clips_keeps_media_mapping() { let (_, r) = fx.seq.find_item(ib).unwrap(); assert_eq!((r.start, r.duration, r.source_in), (f(40), f(20), f(50))); } + +/// #374: moving an edge must not change which media sits under any timeline time the clip still +/// covers (the timeline's live trim preview draws its waveform from this item), and must leave the +/// item as the trim itself does. +#[test] +fn moving_an_edge_keeps_the_media_under_the_timeline() { + let fx = Fx::new(); + let v1 = fx.v(0); + for (speed, reverse, hold) in [(1.0, false, None), (2.0, false, None), (0.5, false, None), (1.0, true, None), (2.0, true, None), (1.0, false, Some(f(50)))] + { + for (edge, d) in [(Edge::In, f(7)), (Edge::In, -f(4)), (Edge::Out, -f(9)), (Edge::Out, f(3))] { + let mut fresh = Fx::new(); + let c = fresh.put(v1, 100, 40, 200); + { + let it = fresh.seq.find_item_mut(c).unwrap().1; + (it.speed, it.reverse, it.frame_hold) = (speed, reverse, hold); + } + let before = fresh.seq.find_item(c).unwrap().1.clone(); + let mut moved = before.clone(); + move_edge(&mut moved, edge, d, true); + let label = format!("speed {speed} reverse {reverse} hold {hold:?} {edge:?} {d:?}"); + // every frame still covered shows the same media as before the move + let mut t = moved.start.max(before.start); + while t < moved.end().min(before.end()) { + let (a, b) = (before.source_time_at(t), moved.source_time_at(t)); + assert!((a - b).0.abs() <= 1, "{label}: at {t:?} {a:?} became {b:?}"); + t += f(1); + } + // and the item is what the real (unclamped) trim makes of it + let mut n = fresh.next; + let got = trim(&mut fresh.seq, c, edge, TrimMode::Regular, d, &mut Fx::ctx(&mut n)).unwrap(); + assert_eq!(got, d, "{label}: not clamped"); + let trimmed = fresh.seq.find_item(c).unwrap().1; + assert_eq!((trimmed.start, trimmed.duration, trimmed.source_in), (moved.start, moved.duration, moved.source_in), "{label}"); + } + } +} diff --git a/crates/engine/src/clip_ops_tests.rs b/crates/engine/src/clip_ops_tests.rs index ae73e578f..11707e251 100644 --- a/crates/engine/src/clip_ops_tests.rs +++ b/crates/engine/src/clip_ops_tests.rs @@ -924,6 +924,40 @@ fn linked_slip_keeps_picture_and_sound_aligned_at_unequal_media_limits() { assert_eq!((clip(&s, v).source_in, clip(&s, a).source_in), (Tick(fr.0 * 10), Tick(fr.0 * 10))); } +#[test] +fn reset_keeps_keyframes_and_resets_one_parameter() { + // #284: Reset Effect used to replace the effect with a fresh one, deleting every keyframe + let mut s = demo(); + let a = v1(&s)[0].clone(); + let sec = Tick(TICKS_PER_SECOND); + let (t0, t1, mid) = (a.start, a.start + sec, a.start + Tick(TICKS_PER_SECOND / 2)); + for (t, v) in [(t0, 150.0), (t1, 50.0)] { + s.execute("effects.setParam", json!({"clip": a.id.0, "effect": "motion", "param": "scale", "value": v, "keyframe": true, "time": t.0})).unwrap(); + } + s.execute("effects.setParam", json!({"clip": a.id.0, "effect": "motion", "param": "rotation", "value": 30.0})).unwrap(); + let idx = clip(&s, a.id.0).effects.iter().position(|e| e.effect == "motion").unwrap(); + s.execute("playhead.set", json!({"time": mid.0})).unwrap(); + s.execute("effects.reset", json!({"clip": a.id.0, "index": idx})).unwrap(); + let it = clip(&s, a.id.0); + let scale = it.effect("motion").unwrap().param("scale").unwrap(); + // (the playhead snaps to a frame) + let mid_media = it.source_time_at(s.playhead()); + let kf: Vec<(Tick, f64)> = scale.keyframes.iter().map(|k| (k.time, k.value.as_f64().unwrap())).collect(); + assert_eq!(kf, vec![(it.source_time_at(t0), 150.0), (mid_media, 100.0), (it.source_time_at(t1), 50.0)], "keyframes kept, default at the playhead"); + assert_eq!(f64_param(&it, "motion", "rotation"), 0.0); + // one parameter: only it goes back to its default + s.execute("effects.setParam", json!({"clip": a.id.0, "effect": "motion", "param": "rotation", "value": 45.0})).unwrap(); + s.execute("effects.setParam", json!({"clip": a.id.0, "effect": "opacity", "param": "opacity", "value": 40.0})).unwrap(); + s.execute("effects.resetParam", json!({"clip": a.id.0, "effect": "motion", "param": "rotation"})).unwrap(); + let it = clip(&s, a.id.0); + assert_eq!(f64_param(&it, "motion", "rotation"), 0.0); + assert_eq!(f64_param(&it, "opacity", "opacity"), 40.0); + assert_eq!(it.effect("motion").unwrap().param("scale").unwrap().keyframes.len(), 3); + assert!(s.execute("effects.resetParam", json!({"clip": a.id.0, "effect": "motion", "param": "nope"})).is_err()); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(f64_param(&clip(&s, a.id.0), "motion", "rotation"), 45.0); +} + #[test] fn keyboard_slip_keeps_linked_clips_in_sync_past_the_shorter_handle() { let mut s = Session::default(); diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index b34d03bbf..d554ffe16 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -144,6 +144,29 @@ pub(crate) fn named_clips(s: &Session, spec: &CommandSpec, p: &Value) -> Option< let clips: Vec = named_ids(spec, p, "clips", "clip")?.into_iter().map(ClipId).filter(|c| seq.find_item(*c).is_some()).collect(); (!clips.is_empty()).then_some(clips) } +/// Why the clips a caller named in `clips` / `clip` stand in for no selection (#591): a clip of +/// another sequence, an id of no clip (a track, a project item…) or a value that is not an id. +/// `None` when the caller named nothing or no sequence is open, so the selection's reason stands. +pub(crate) fn named_clips_missing(s: &Session, spec: &CommandSpec, p: &Value) -> Option { + s.active_sequence()?; + let first = if spec.params.contains("\"clips\":[id]") + && let Some(a) = p.get("clips").and_then(Value::as_array) + { + a.first()? + } else if spec.params.contains("\"clip\":id") { + p.get("clip").filter(|v| !v.is_null())? + } else { + return None; + }; + let Some(id) = first.as_u64() else { + return Some(format!("{first} is not a clip id (clip ids are the `clip` numbers of the items in sequence.inspect)")); + }; + let elsewhere = s.project.sequences().find(|i| i.as_sequence().is_some_and(|q| q.find_item(ClipId(id)).is_some())); + Some(match elsewhere { + Some(i) => format!("clip {id} is in sequence \"{}\", not the active one: open it first with sequence.open {{\"item\":{}}}", i.name, i.id.0), + None => format!("no clip {id} in the active sequence (clip ids are the `clip` numbers of the items in sequence.inspect)"), + }) +} /// Project items named explicitly in `items` / `item`, likewise. The Project panel's selection /// holds bins beside items (one id space, see `project.select`), so a bin named here counts too; /// the project's own top bin does not, since no command acts on it (#244). @@ -388,6 +411,31 @@ pub fn with_links(s: &Session, clips: &[ClipId]) -> Vec { out } +/// The delta a trim of `clips` (a clip and its linked partners) can apply together: `delta` +/// clamped by each one's media and neighbours. +fn clamp_trim_linked(q: &filmcraft_project::Sequence, clips: &[ClipId], edge: Edge, mode: TrimMode, delta: Tick, ctx: &edit::EditCtx) -> Result { + let mut d = delta; + for c in clips { + let x = edit::clamp_trim(q, *c, edge, mode, d, ctx)?; + if x.abs() < d.abs() { + d = x; + } + } + Ok(d) +} + +/// The delta `timeline.trim` would apply for `delta`, without editing anything: the Timeline's +/// trim drag shows this, so the edge stops at the end of the media or the neighbouring clip +/// instead of following the pointer past it (#653). +pub fn trim_delta(s: &Session, clip: ClipId, edge: Edge, mode: TrimMode, delta: Tick) -> Result { + let q = s.active_sequence().ok_or(EngineError::NoSequence)?; + let durations = |id: ItemId| crate::media_duration(&s.project, &s.media, id); + let starts = |id: ItemId| crate::media_start(&s.project, id); + let mut next = s.project.next_id; + let ctx = edit::EditCtx { next_id: &mut next, media_duration: &durations, media_start: &starts, min_duration: q.settings.frame_rate.frame_duration() }; + clamp_trim_linked(q, &with_links(s, &[clip]), edge, mode, delta, &ctx) +} + /// Sequence settings matching a media clip (New Sequence From Clip, New Sequence from an item): /// its frame size, rate and pixel aspect ratio (Interpret Footage's, else the file's), so a /// 1440 x 1080 clip with 4:3 pixels makes a 1440 x 1080 sequence that displays 16:9. @@ -1220,7 +1268,7 @@ fn build() -> Vec { "Media…", ["File", "Export"], None, - r#"{"path":str,"preset":str?,"settings":ExportSettings?,"format":"h264|hevc|prores|dnxhr|apv|mjpeg|mxf-op1a|mxf-opatom|png|tiff|bmp|gif|wav|aiff"?,"width":u32?,"height":u32?,"fps":f64?,"bitrateKbps":u32?,"bitrateMode":"cbr|vbr1Pass|vbr2Pass|crf"?,"crf":0..51=23,"hardwareEncoding":"off|auto"?,"scale":f32=1,"audio":bool=true,"quality":0..100,"burnCaptions":bool=false,"alpha":bool=false,"captionSidecar":"srt|vtt"?,"loudnessLufs":f64?,"proresProfile":"proxy|lt|standard|hq"?,"dnxProfile":"lb|sq|hq|hqx"?,"apvProfile":"422-10|422-12|444-10|444-12"?,"mxfVideoCodec":"dnxhr|proRes|h264"?,"sequence":id?,"range":"entire|inOut|workArea|custom"?,"startSeconds":f64?,"endSeconds":f64?,"wait":bool=false}"#, + r#"{"path":str,"preset":str?,"settings":ExportSettings?,"format":"h264|hevc|prores|dnxhr|apv|mjpeg|mxf-op1a|mxf-opatom|png|tiff|bmp|gif|wav|aiff"?,"width":u32?,"height":u32?,"fps":f64?,"bitrateKbps":u32?,"bitrateMode":"cbr|vbr1Pass|vbr2Pass|crf"?,"crf":0..51=23,"hardwareEncoding":"off|auto"?,"scale":f32=1,"audio":bool=true,"quality":0..100,"burnCaptions":bool=false,"alpha":bool=false,"captionSidecar":"srt|vtt"?,"loudnessLufs":f64?,"proresProfile":"proxy|lt|standard|hq|4444|4444xq"?,"dnxProfile":"lb|sq|hq|hqx"?,"apvProfile":"422-10|422-12|444-10|444-12"?,"mxfVideoCodec":"dnxhr|proRes|h264"?,"sequence":id?,"range":"entire|inOut|workArea|custom"?,"startSeconds":f64?,"endSeconds":f64?,"wait":bool=false}"#, has_seq, crate::export_tools::export_media ), @@ -1267,7 +1315,7 @@ fn build() -> Vec { let caps = s.state.caption_selection.clone(); return crate::captions::delete(s, &caps, false); } - let sel = if p.get("clips").is_some() || p.get("clip").is_some() { with_links(s, &clips_p(s, p)) } else { s.state.selection.clone() }; + let sel = with_links(s, &clips_p(s, p)); s.edit_sequence("Clear", |q, _, st| { edit::delete_items(q, &sel); st.selection.clear(); @@ -1285,7 +1333,7 @@ fn build() -> Vec { let caps = s.state.caption_selection.clone(); return crate::captions::delete(s, &caps, true); } - let sel = if p.get("clips").is_some() || p.get("clip").is_some() { with_links(s, &clips_p(s, p)) } else { s.state.selection.clone() }; + let sel = with_links(s, &clips_p(s, p)); s.edit_sequence("Ripple Delete", |q, _, st| { let spans = edit::ripple_delete_items(q, &sel)?; if st.ripple_sequence_markers { @@ -2113,9 +2161,12 @@ fn build() -> Vec { "Place Clip", [], None, - r#"{"item":id,"track":"V1"|id|"A1" (sound only)?,"audioTrack":"A1"|id?,"time":ticks|"frame":i64|"seconds":f64,"insert":bool,"sourceIn":ticks?,"duration":ticks?,"video":bool=true,"audio":bool=true}"#, + r#"{"item":id,"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}"#, has_seq, |s, p| { + if crate::sequence_tools::wants_new_track(p) { + return crate::sequence_tools::place_on_new_tracks(s, p); + } let stream_flag = |key: &str| match p.get(key) { None => Ok(true), Some(Value::Bool(v)) => Ok(*v), @@ -2166,13 +2217,13 @@ fn build() -> Vec { Ok(json!({"clips": ids.iter().map(|c| c.0).collect::>()})) } ), - cmd!("timeline.select", "Select Clips", [], None, r#"{"clips":[id],"transitions":[id]?,"add":bool,"toggle":bool,"linked":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 = if bool_p(p, "linked").unwrap_or(true) { with_links(s, &clips) } else { clips }; + 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(); @@ -2230,13 +2281,7 @@ fn build() -> Vec { let applied = s.edit_sequence(if mode == TrimMode::Ripple { "Ripple Trim" } else { "Trim" }, |q, ctx, st| { let before = q.find_item(c).map(|(_, i)| i.range()); // clamp across all linked partners, then apply the common delta - let mut dd = d; - for c in &clips { - let x = edit::clamp_trim(q, *c, edge, mode, dd, ctx)?; - if x.abs() < dd.abs() { - dd = x; - } - } + let mut dd = clamp_trim_linked(q, &clips, edge, mode, d, ctx)?; if mode == TrimMode::Ripple { // the clip and its linked partners ripple as one edit let mut group = vec![c]; @@ -2372,20 +2417,8 @@ fn build() -> Vec { .or_else(|| p.get("deltaFrames").and_then(Value::as_i64).map(|f| s.sequence_rate().tick_of(f))) .unwrap_or_default(); let clips = with_links(s, &[c]); - s.edit_sequence("Slide", |q, ctx, _| { - // clamp across all linked partners, then slide them by the common delta - let mut dd = d; - for c in &clips { - let x = edit::slide(&mut q.clone(), *c, dd, ctx)?; - if x.abs() < dd.abs() { - dd = x; - } - } - for c in &clips { - edit::slide(q, *c, dd, ctx)?; - } - Ok(()) - })?; + // clamp across all linked partners, then slide them by the common delta + s.edit_sequence("Slide", |q, ctx, _| Ok(edit::slide_items(q, &clips, d, ctx)?))?; Ok(Value::Null) }), cmd!("timeline.rateStretch", "Rate Stretch", [], None, r#"{"clip":id,"edge":"in|out","delta":ticks}"#, has_seq, |s, p| { @@ -2661,15 +2694,35 @@ fn build() -> Vec { cmd!("effects.reset", "Reset Effect", [], None, r#"{"clip":id,"index":n}"#, has_seq, |s, p| { let c = clip_p(p, "clip").ok_or_else(|| bad("effects.reset", "need `clip`"))?; let idx = u64_p(p, "index").unwrap_or(0) as usize; - let (fw, fh) = s.active_sequence().map(|q| (q.settings.width, q.settings.height)).unwrap_or((1920, 1080)); + let frame = s.active_sequence().map(|q| (q.settings.width, q.settings.height)).unwrap_or((1920, 1080)); + let ph = s.playhead(); s.edit_sequence("Reset Effect", |q, _, _| { let (_, it) = q.find_item_mut(c).ok_or(filmcraft_edit::EditError::NoItem(c))?; - if let Some(e) = it.effects.get_mut(idx) - && let Some(d) = e.def() - { - let mut fresh = d.instance(); - resolve_auto_points(&mut fresh, (fw, fh), (fw, fh)); - *e = fresh; + let mt = it.source_time_at(ph.clamp(it.start, (it.end() - Tick(1)).max(it.start))); + if let Some(e) = it.effects.get_mut(idx) { + reset_params(e, None, frame, mt); + } + Ok(()) + })?; + Ok(Value::Null) + }), + cmd!("effects.resetParam", "Reset Parameter", [], None, r#"{"clip":id,"effect":str|index,"param":str}"#, has_seq, |s, p| { + let c = clip_p(p, "clip").ok_or_else(|| bad("effects.resetParam", "need `clip`"))?; + let pid = str_p(p, "param").ok_or_else(|| bad("effects.resetParam", "need `param`"))?.to_string(); + let eff = p.get("effect").cloned().unwrap_or(json!("motion")); + let frame = s.active_sequence().map(|q| (q.settings.width, q.settings.height)).unwrap_or((1920, 1080)); + let ph = s.playhead(); + s.edit_sequence("Reset Parameter", |q, _, _| { + let (_, it) = q.find_item_mut(c).ok_or(filmcraft_edit::EditError::NoItem(c))?; + let mt = it.source_time_at(ph.clamp(it.start, (it.end() - Tick(1)).max(it.start))); + let e = match &eff { + Value::Number(n) => it.effects.get_mut(n.as_u64().unwrap_or(0) as usize), + Value::String(sid) => it.effects.iter_mut().find(|e| &e.effect == sid), + _ => None, + } + .ok_or_else(|| bad("effects.resetParam", "no such effect on clip"))?; + if !reset_params(e, Some(&pid), frame, mt) { + return Err(bad("effects.resetParam", format!("no param `{pid}`"))); } Ok(()) })?; @@ -2771,7 +2824,7 @@ fn build() -> Vec { s.fix_state(); Ok(json!({"items": items.len(), "bins": bins.len()})) }), - cmd!("project.moveToBin", "Move to Bin", [], None, r#"{"items":[id]?,"bin":binId|null}"#, always, |s, p| { + cmd!("project.moveToBin", "Move to Bin", [], None, r#"{"items":[id|binId]?,"bin":binId|null}"#, always, |s, p| { let items: Vec = match p.get("items").and_then(Value::as_array) { Some(a) => a.iter().filter_map(|v| v.as_u64().map(ItemId)).collect(), None => s.state.project_selection.clone(), @@ -2886,6 +2939,30 @@ fn build() -> Vec { v } +/// Put `e`'s parameters (all of them, or only `only`) back to their defaults, as Premiere's reset +/// buttons do: a parameter that is not animated gets its default as its static value, and an +/// animated one keeps its keyframes and gets the default as a keyframe at media time `mt`. +/// Masks are left alone. Returns whether any parameter was reset. +fn reset_params(e: &mut filmcraft_project::EffectInstance, only: Option<&str>, frame: (u32, u32), mt: Tick) -> bool { + let Some(d) = e.def() else { return false }; + let mut fresh = d.instance(); + resolve_auto_points(&mut fresh, frame, frame); + let mut any = false; + for (id, prm) in fresh.params { + if only.is_some_and(|o| o != id) { + continue; + } + any = true; + match e.params.get_mut(&id) { + Some(old) if old.is_animated() => old.set_at(mt, prm.value), + _ => { + e.params.insert(id, prm); + } + } + } + any +} + fn keyframe_op(s: &mut Session, p: &Value, op: &str) -> Result { let c = clip_p(p, "clip").ok_or_else(|| bad("keyframe", "need `clip`"))?; let pid = str_p(p, "param").ok_or_else(|| bad("keyframe", "need `param`"))?.to_string(); diff --git a/crates/engine/src/explicit_targets_tests.rs b/crates/engine/src/explicit_targets_tests.rs index ce377f64d..c3df07e18 100644 --- a/crates/engine/src/explicit_targets_tests.rs +++ b/crates/engine/src/explicit_targets_tests.rs @@ -119,10 +119,15 @@ fn clip_commands_take_explicit_clips_without_a_selection() { fn naming_nothing_usable_does_not_enable_a_command() { let mut s = demo(); let before = s.project.clone(); - // no ids, ids of no clip, the wrong type, and a key the command does not take - for p in [json!({"clips": []}), json!({"clips": [987_654_321u64]}), json!({"clips": "all"}), json!({"clips": [-1, 1.5, null]}), json!({"items": [1]})] { + // no ids, the wrong type, and a key the command does not take + for p in [json!({"clips": []}), json!({"clips": "all"}), json!({"items": [1]})] { assert_eq!(disabled(s.execute("edit.rippleDelete", p.clone())), "nothing selected", "{p}"); } + // ids of no clip and values that are not ids: still disabled, and the reason names them (#591) + let why = disabled(s.execute("edit.rippleDelete", json!({"clips": [987_654_321u64]}))); + assert!(why.starts_with("no clip 987654321 in the active sequence"), "{why}"); + let why = disabled(s.execute("edit.rippleDelete", json!({"clips": [-1, 1.5, null]}))); + assert!(why.starts_with("-1 is not a clip id"), "{why}"); assert_eq!(disabled(s.execute("clip.replaceFromBin", json!({"clips": [v1(&s)[0].id.0], "item": 987_654_321u64}))), "select a clip in the Project panel"); // `edit.cut` takes no `clips`: it works on the selection only assert_eq!(disabled(s.execute("edit.cut", json!({"clips": [v1(&s)[0].id.0]}))), "no clips selected"); @@ -159,6 +164,44 @@ fn link_and_speed_take_explicit_clips_without_a_selection() { assert!(s.state.selection.is_empty() && !s.is_enabled("clip.speedDuration"), "menu enablement still follows the selection"); } +/// #591: a caller that names clips which cannot stand in for the selection hears why, instead of +/// "no clips selected": a clip of another sequence, an id of something else, a string id. +#[test] +fn named_clips_that_are_not_usable_say_why() { + let mut s = demo(); + let demo_seq = s.state.active_sequence.unwrap(); + let demo_name = s.project.item(demo_seq).unwrap().name.clone(); + let last = v1(&s).last().unwrap().clone(); + // a track id and a project item id are not clip ids + let track = s.active_sequence().unwrap().video_tracks[0].id.0; + let why = disabled(s.execute("clip.speedDuration", json!({"clips": [track], "speed": 50}))); + assert_eq!(why, format!("no clip {track} in the active sequence (clip ids are the `clip` numbers of the items in sequence.inspect)")); + let why = disabled(s.execute("clip.speedDuration", json!({"clips": [last.item.0], "speed": 50}))); + assert!(why.starts_with(&format!("no clip {} in the active sequence", last.item.0)), "{why}"); + // a string is not an id + let why = disabled(s.execute("clip.speedDuration", json!({"clips": [last.id.0.to_string()], "speed": 50}))); + assert!(why.starts_with(&format!("\"{}\" is not a clip id", last.id.0)), "{why}"); + // `clip`, where the command documents it + let why = disabled(s.execute("graphics.setText", json!({"clip": "x", "text": "x"}))); + assert!(why.starts_with("\"x\" is not a clip id"), "{why}"); + // a clip of a sequence that is not the active one + s.execute("file.newSequence", json!({"name": "Other"})).unwrap(); + assert_ne!(s.state.active_sequence, Some(demo_seq)); + let why = disabled(s.execute("clip.speedDuration", json!({"clips": [last.id.0], "speed": 200}))); + assert_eq!( + why, + format!("clip {} is in sequence \"{demo_name}\", not the active one: open it first with sequence.open {{\"item\":{}}}", last.id.0, demo_seq.0) + ); + // and doing what it says works + s.execute("sequence.open", json!({"item": demo_seq.0})).unwrap(); + s.execute("clip.speedDuration", json!({"clips": [last.id.0], "speed": 200})).unwrap(); + assert!(clip(&s, last.id).unwrap().duration < last.duration); + // naming nothing keeps the selection's reason, for menus and callers alike + s.state.selection.clear(); + assert_eq!(disabled(s.execute("clip.speedDuration", json!({"speed": 50}))), "no clips selected"); + assert_eq!(disabled(s.execute("clip.speedDuration", json!({"clips": [], "speed": 50}))), "no clips selected"); +} + #[test] fn naming_targets_lifts_only_the_selection_condition() { // no sequence: named clips cannot stand in for one diff --git a/crates/engine/src/export_tests.rs b/crates/engine/src/export_tests.rs index dec8a8fa7..e0d58794c 100644 --- a/crates/engine/src/export_tests.rs +++ b/crates/engine/src/export_tests.rs @@ -217,9 +217,16 @@ fn every_builtin_preset_exports_and_ffprobe_confirms() { "proxy" => ("Proxy", 45.0), "lt" => ("LT", 102.0), "standard" => ("Standard", 147.0), + "4444" => ("4444", 330.0), + "4444xq" => ("XQ", 500.0), _ => ("HQ", 220.0), }; assert_eq!(v["profile"], profile, "{name}"); + if matches!(profile, "4444" | "XQ") { + let pix = v["pix_fmt"].as_str().unwrap_or_default(); + let alpha = settings["alpha"].as_bool().unwrap_or(false); + assert!(pix.contains("444") && pix.starts_with("yuva") == alpha, "{name}: {pix}"); + } assert!(vbr <= mbps * px * 1.5, "{name}: {vbr:.1} Mb/s vs nominal {:.1}", mbps * px); assert_eq!(a["codec_name"], "pcm_s24le", "{name}"); } @@ -318,7 +325,7 @@ fn user_presets_persist_with_favourites_import_and_export() { let favs = s.execute("export.presets.list", json!({"favorites": true})).unwrap()["presets"].as_array().unwrap().len(); assert_eq!(favs, 2); let hits = s.execute("export.presets.list", json!({"query": "prores"})).unwrap()["presets"].as_array().unwrap().len(); - assert_eq!(hits, 5, "four QuickTime ProRes presets and MXF OP1a ProRes 422 HQ"); + assert_eq!(hits, 8, "seven QuickTime ProRes presets and MXF OP1a ProRes 422 HQ"); // persisted: a fresh session with the same data directory sees both let mut s2 = demo(); s2.export_presets.set_dir(&dir.0); diff --git a/crates/engine/src/image_sequence_tests.rs b/crates/engine/src/image_sequence_tests.rs index 8d028fe7a..9989be581 100644 --- a/crates/engine/src/image_sequence_tests.rs +++ b/crates/engine/src/image_sequence_tests.rs @@ -11,6 +11,17 @@ use serde_json::{Value, json}; use crate::Session; use crate::media_test_util::tmp_dir; +#[test] +fn import_accepts_uppercase_extensions() { + let dir = tmp_dir("import-uppercase"); + let wav = dir.join("TONE.WAV"); + std::fs::write(&wav, filmcraft_media::wav::write_wav16(&[0.1; 4800], 2, 48_000)).unwrap(); + let mut s = Session::default(); + let r = s.execute("file.import", json!({"paths": [wav.to_string_lossy()]})).unwrap(); + assert_eq!(r["items"].as_array().unwrap().len(), 1); + assert!(s.project.items.values().any(|it| it.name == "TONE.WAV")); +} + #[test] fn import_replace_starts_from_an_empty_project() { let dir = tmp_dir("import-replace"); diff --git a/crates/engine/src/keyboard.rs b/crates/engine/src/keyboard.rs index ad5f5d2aa..46d063b2a 100644 --- a/crates/engine/src/keyboard.rs +++ b/crates/engine/src/keyboard.rs @@ -272,12 +272,8 @@ fn slip_selection(s: &mut Session, frames: i64) -> Result { fn slide_selection(s: &mut Session, frames: i64) -> Result { let d = s.sequence_rate().tick_of(frames); let clips = with_links(s, &s.state.selection.clone()); - s.edit_sequence("Slide", |q, ctx, _| { - for c in &clips { - edit::slide(q, *c, d, ctx)?; - } - Ok(()) - })?; + // one common delta, clamped by the most restrictive member, keeps linked partners in sync + s.edit_sequence("Slide", |q, ctx, _| Ok(edit::slide_items(q, &clips, d, ctx)?))?; Ok(Value::Null) } diff --git a/crates/engine/src/keyboard_tests.rs b/crates/engine/src/keyboard_tests.rs index b23a9809d..60e9b7b43 100644 --- a/crates/engine/src/keyboard_tests.rs +++ b/crates/engine/src/keyboard_tests.rs @@ -376,3 +376,29 @@ fn reveal_nested_sequence_opens_it_at_the_matching_frame() { assert_ne!(s.state.active_sequence, Some(outer)); assert_eq!(s.playhead(), nest_clip.source_in + rate.tick_of(7), "{r}"); } + +#[test] +fn keyboard_slide_keeps_linked_partners_aligned_at_a_neighbour_limit() { + // #712: a three-frame sound-only follower limits only the sound partner + let mut s = Session::default(); + s.execute("file.newProject", json!({"name": "Slide"})).unwrap(); + s.execute("file.newSequence", json!({"name": "Sequence", "fps": 25, "video": 1, "audio": 1, "width": 16, "height": 16})).unwrap(); + let r = s.execute("file.newOfflineFile", json!({"name": "Media", "seconds": 4, "fps": 25, "video": true, "audio": true})).unwrap(); + let item = r["item"].as_u64().unwrap(); + let fr = s.sequence_rate().tick_of(1); + let r = + s.execute("timeline.place", json!({"item": item, "track": "V1", "audioTrack": "A1", "time": 0, "sourceIn": fr.0 * 20, "duration": fr.0 * 60})).unwrap(); + let (v, a) = (r["clips"][0].as_u64().unwrap(), r["clips"][1].as_u64().unwrap()); + let r = s.execute("timeline.place", json!({"item": item, "track": "A1", "time": fr.0 * 60, "sourceIn": 0, "duration": fr.0 * 3})).unwrap(); + let f = r["clips"][0].as_u64().unwrap(); + s.execute("sequence.linkedSelection", json!({"on": true})).unwrap(); + s.execute("timeline.select", json!({"clips": [v]})).unwrap(); + s.execute("timeline.slideRight5", json!({})).unwrap(); + assert_eq!(clip(&s, v).start, Tick(fr.0 * 2), "the pair moves only as far as the sound may"); + assert_eq!(clip(&s, a).start, clip(&s, v).start, "picture and sound stay aligned"); + let fl = clip(&s, f); + assert_eq!((fl.start, fl.duration, fl.source_in), (Tick(fr.0 * 62), fr, Tick(fr.0 * 2))); + // nothing more to give: a further slide right moves neither partner + s.execute("timeline.slideRight", json!({})).unwrap(); + assert_eq!((clip(&s, v).start, clip(&s, a).start), (Tick(fr.0 * 2), Tick(fr.0 * 2))); +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 2d0f2b14e..2e161bbdc 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -708,8 +708,11 @@ impl Session { } let clips = commands::named_clips(self, spec, params); let items = commands::named_items(self, spec, params); + // clips were named but none is one of the active sequence: say which and why, rather + // than "no clips selected" to a caller that did not mean the selection (#591) + let missing = if clips.is_none() { commands::named_clips_missing(self, spec, params) } else { None }; if clips.is_none() && items.is_none() { - return by_selection; + return by_selection.map_err(|why| missing.unwrap_or(why)); } let mut clips = clips.unwrap_or_else(|| self.state.selection.clone()); let mut items = items.unwrap_or_else(|| self.state.project_selection.clone()); @@ -718,7 +721,7 @@ impl Session { let by_params = (spec.enabled)(self); self.state.selection = clips; self.state.project_selection = items; - by_params + by_params.map_err(|why| missing.unwrap_or(why)) } /// Whether the command can run on the current selection (what the menus show). diff --git a/crates/engine/src/media_browser_tests.rs b/crates/engine/src/media_browser_tests.rs index aa1b59389..3c026a340 100644 --- a/crates/engine/src/media_browser_tests.rs +++ b/crates/engine/src/media_browser_tests.rs @@ -29,6 +29,7 @@ impl FakeFs { "/home/me/Movies/edit.edl", "/home/me/Movies/subs.srt", "/home/me/Movies/clip.mov", + "/home/me/Movies/CLIP.MOV", ] { files.insert(f.to_string(), vec![0; 16]); } @@ -100,7 +101,7 @@ fn listing_puts_folders_first_and_hides_unsupported_files() { assert_eq!(l["dir"], "/home/me"); assert_eq!(names(&l), ["Movies"]); let l = s.execute("mediaBrowser.list", json!({"path": "/home/me/Movies"})).unwrap(); - assert_eq!(names(&l), ["Selects", "A-interview.wav", "b-roll.wav", "clip.mov", "edit.edl", "shot_0001.png", "shot_0002.png", "subs.srt"]); + assert_eq!(names(&l), ["Selects", "A-interview.wav", "b-roll.wav", "CLIP.MOV", "clip.mov", "edit.edl", "shot_0001.png", "shot_0002.png", "subs.srt"]); let e = &l["entries"][0]; assert_eq!((e["isDir"].as_bool(), e["kind"].as_str()), (Some(true), Some("folder"))); let shot = l["entries"].as_array().unwrap().iter().find(|e| e["name"] == "shot_0001.png").unwrap(); @@ -117,7 +118,7 @@ fn file_type_filter() { let l = s.execute("mediaBrowser.list", json!({"path": "/home/me/Movies", "fileTypes": "image"})).unwrap(); assert_eq!(names(&l), ["Selects", "shot_0001.png", "shot_0002.png"]); let l = s.execute("mediaBrowser.list", json!({"path": "/home/me/Movies", "fileTypes": "mov"})).unwrap(); - assert_eq!(names(&l), ["Selects", "clip.mov"]); + assert_eq!(names(&l), ["Selects", "CLIP.MOV", "clip.mov"]); // the setting persists and applies by default s.execute("mediaBrowser.settings", json!({"fileTypes": "project"})).unwrap(); let l = s.execute("mediaBrowser.list", json!({"path": "/home/me/Movies"})).unwrap(); diff --git a/crates/engine/src/panels.rs b/crates/engine/src/panels.rs index 45da2e219..6246e7a31 100644 --- a/crates/engine/src/panels.rs +++ b/crates/engine/src/panels.rs @@ -4,6 +4,7 @@ //! |---|---| //! | `events.list` / `events.clear` | the Events panel: warnings and errors of commands, background jobs and the auto-save worker ([`EventLog`]) | //! | `metadata.get` / `metadata.set` | the Metadata panel: an item's clip and file properties; editable log fields (undoable) | +//! | `project.notes` / `project.setNotes` | the Project Notes panel: free plain text saved in the project (undoable) | //! //! The Progress panel uses `jobs.list` / `jobs.cancel`; the Timecode panel and the Reference //! Monitor are frontend state only. @@ -339,6 +340,38 @@ fn metadata_set(s: &mut Session, p: &Value) -> Result { Ok(json!({"item": id.0, "name": it.name, "label": it.label.name(), "metadata": it.metadata, "changed": !unchanged})) } +// ------------------------------------------------------------------------------------ project notes + +/// Longest project notes accepted (bytes of UTF-8), so a script or a pasted file cannot grow the +/// project (and every undo step) without bound. +pub const MAX_NOTES_BYTES: usize = 1 << 20; + +/// `project.notes`: the project notes. +fn notes_get(s: &mut Session, _: &Value) -> Result { + Ok(json!({"text": s.project.notes})) +} + +/// `project.setNotes {text, merge?}`: replace the project notes as one undo step. Calls with the +/// same `merge` key and no other edit in between share that step (typing in the panel). +fn notes_set(s: &mut Session, p: &Value) -> Result { + let text = p.get("text").and_then(Value::as_str).ok_or_else(|| bad("project.setNotes", "need `text` (a string)"))?; + if text.len() > MAX_NOTES_BYTES { + return Err(bad("project.setNotes", format!("the notes are too long ({} bytes; at most {MAX_NOTES_BYTES})", text.len()))); + } + let changed = s.project.notes != text; + if changed { + let body = |pr: &mut Project, _: &mut crate::EditorState| { + pr.notes = text.to_string(); + Ok(()) + }; + match str_p(p, "merge") { + Some(key) => s.edit_merged("Edit Project Notes", &format!("project.notes:{key}"), body)?, + None => s.edit("Edit Project Notes", body)?, + } + } + Ok(json!({"text": s.project.notes, "changed": changed})) +} + // ------------------------------------------------------------------------------------ registry pub(crate) fn commands() -> Vec { @@ -370,5 +403,16 @@ pub(crate) fn commands() -> Vec { run: metadata_set, journal: true, }, + q("project.notes", "Get Project Notes", "{}", notes_get), + CommandSpec { + id: "project.setNotes", + label: "Edit Project Notes", + menu: &[], + shortcut: None, + params: r#"{"text":str,"merge":str?}"#, + enabled: always, + run: notes_set, + journal: true, + }, ] } diff --git a/crates/engine/src/panels_tests.rs b/crates/engine/src/panels_tests.rs index 357e995a4..8be530abe 100644 --- a/crates/engine/src/panels_tests.rs +++ b/crates/engine/src/panels_tests.rs @@ -156,3 +156,40 @@ fn events_report_background_jobs() { s.poll_persistence(); assert_eq!(s.log.entries.len(), 6); } + +/// #620: project notes are saved in the project file; typing (one `merge` key) is one undo step. +#[test] +fn project_notes_are_saved_and_typing_is_one_undo_step() { + let mut s = demo(); + assert_eq!(s.execute("project.notes", json!({})).unwrap()["text"], ""); + let undo0 = s.history.undo.len(); + for text in ["T", "To", "To do: fix the logo at 00:01:02:03"] { + s.execute("project.setNotes", json!({"text": text, "merge": "1"})).unwrap(); + } + assert_eq!(s.history.undo.len(), undo0 + 1, "one typing session, one undo step"); + assert_eq!(s.history.undo.last().unwrap().0, "Edit Project Notes"); + // the same text again changes nothing and adds no step + let r = s.execute("project.setNotes", json!({"text": "To do: fix the logo at 00:01:02:03"})).unwrap(); + assert_eq!(r["changed"], json!(false)); + assert_eq!(s.history.undo.len(), undo0 + 1); + + let loaded = filmcraft_format::decode(&filmcraft_format::encode(&s.project, false)).unwrap(); + assert_eq!(loaded.project.notes, "To do: fix the logo at 00:01:02:03"); + // an older file without notes still loads, with none + let mut doc: Value = serde_json::from_slice(&filmcraft_format::encode(&filmcraft_project::Project::new("old"), false)).unwrap(); + doc["schema_version"] = json!(14); + assert_eq!(filmcraft_format::decode(&serde_json::to_vec(&doc).unwrap()).unwrap().project.notes, ""); + + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.notes, ""); +} + +#[test] +fn project_notes_refuse_bad_input() { + let mut s = demo(); + assert!(s.execute("project.setNotes", json!({})).is_err()); + assert!(s.execute("project.setNotes", json!({"text": 5})).is_err()); + let huge = "x".repeat(crate::panels::MAX_NOTES_BYTES + 1); + assert!(s.execute("project.setNotes", json!({"text": huge})).is_err()); + assert_eq!(s.project.notes, ""); +} diff --git a/crates/engine/src/sequence_tools.rs b/crates/engine/src/sequence_tools.rs index a0027913d..dc6c57953 100644 --- a/crates/engine/src/sequence_tools.rs +++ b/crates/engine/src/sequence_tools.rs @@ -723,6 +723,54 @@ pub(crate) fn add_tracks(s: &mut Session, p: &Value) -> Result { Ok(json!({"video": video, "audio": audio, "submix": submix})) } +/// `timeline.place` names `"new"` as its `track` or `audioTrack`. +pub(crate) fn wants_new_track(p: &Value) -> bool { + ["track", "audioTrack"].iter().any(|k| p.get(*k).and_then(Value::as_str).is_some_and(|v| v.eq_ignore_ascii_case("new"))) +} + +/// `timeline.place` with `"track":"new"` and/or `"audioTrack":"new"` (#483: media dropped in the +/// empty space above the video tracks or below the audio tracks): a track is added after the +/// last one of its kind for each stream the item carries (and the call enables), then the clip +/// goes on it. One undo step; when the placement fails, the new tracks go away again. +pub(crate) fn place_on_new_tracks(s: &mut Session, p: &Value) -> Result { + let cmd = "timeline.place"; + let item = crate::commands::item_p(p, "item").ok_or_else(|| bad(cmd, "need `item`"))?; + let pi = s.project.item(item).ok_or_else(|| bad(cmd, "no such item"))?; + let is_new = |k: &str| p.get(k).and_then(Value::as_str).is_some_and(|v| v.eq_ignore_ascii_case("new")); + let enabled = |k: &str| p.get(k).and_then(Value::as_bool).unwrap_or(true); + let new_v = is_new("track") && pi.has_video() && enabled("video"); + let new_a = is_new("audioTrack") && pi.has_audio() && enabled("audio"); + if !new_v && !new_a { + return Err(bad(cmd, "no new track to add: the item has no picture or sound for it")); + } + let added = add_tracks(s, &json!({"video": u64::from(new_v), "audio": u64::from(new_a)}))?; + let mark = s.history.undo.len(); + let mut q = p.clone(); + for (key, kind) in [("track", "video"), ("audioTrack", "audio")] { + if is_new(key) { + q[key] = added.get(kind).and_then(|ids| ids.get(0)).cloned().unwrap_or(Value::Null); + } + } + let place = crate::commands::find(cmd).ok_or_else(|| EngineError::UnknownCommand(cmd.into()))?.run; + match place(s, &q) { + Ok(v) => { + // the new tracks and the clip are one undo step + if mark >= 1 + && s.history.undo.len() == mark + 1 + && let (Some((label, _)), Some((_, before))) = (s.history.undo.pop(), s.history.undo.pop()) + { + s.history.undo.push((label, before)); + } + Ok(v) + } + Err(e) => { + s.undo(); + s.history.redo.pop(); + Err(e) + } + } +} + fn delete_tracks(s: &mut Session, p: &Value) -> Result { let seq_id = s.state.active_sequence.ok_or(EngineError::NoSequence)?; let q = s.active_sequence().ok_or(EngineError::NoSequence)?; diff --git a/crates/engine/src/sequence_tools_tests.rs b/crates/engine/src/sequence_tools_tests.rs index 9e164ce2c..d275f9b3e 100644 --- a/crates/engine/src/sequence_tools_tests.rs +++ b/crates/engine/src/sequence_tools_tests.rs @@ -575,3 +575,35 @@ fn go_to_in_out_without_marks_go_to_the_sequence_ends() { s.execute("markers.goToIn", json!({})).unwrap(); assert_eq!(s.playhead(), s.active_sequence().unwrap().mark_in.unwrap()); } + +/// #483: media dropped in the empty space past the last track goes on a new track (with +/// `"new"`), one undo step; a placement that fails leaves no new track behind. +#[test] +fn place_on_new_tracks_adds_them_in_one_undo_step() { + let (mut s, item) = fresh(); + let before = s.active_sequence().unwrap().clone(); + let (nv, na) = (before.video_tracks.len(), before.audio_tracks.len()); + let undo = s.history.undo.len(); + let r = s.execute("timeline.place", json!({"item": item, "track": "new", "audioTrack": "new", "time": 0})).unwrap(); + let q = s.active_sequence().unwrap(); + assert_eq!((q.video_tracks.len(), q.audio_tracks.len()), (nv + 1, na + 1)); + let clips: Vec = r["clips"].as_array().unwrap().iter().map(|c| ClipId(c.as_u64().unwrap())).collect(); + assert_eq!(clips.len(), 2); + assert_eq!(q.video_tracks[nv].items.iter().map(|i| i.id).collect::>(), [clips[0]], "the picture is on the new top video track"); + assert_eq!(q.audio_tracks[na].items.iter().map(|i| i.id).collect::>(), [clips[1]], "the sound is on the new last audio track"); + assert_eq!(s.history.undo.len(), undo + 1, "one undo step"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(*s.active_sequence().unwrap(), before); + + // only the picture gets a new track; the sound follows the given track + s.execute("timeline.place", json!({"item": item, "track": "new", "audioTrack": "A1", "time": 0})).unwrap(); + let q = s.active_sequence().unwrap(); + assert_eq!((q.video_tracks.len(), q.audio_tracks.len()), (nv + 1, na)); + assert_eq!((q.video_tracks[nv].items.len(), q.audio_tracks[0].items.len()), (1, 1)); + s.execute("edit.undo", json!({})).unwrap(); + + // a placement that fails after the tracks were added: nothing changes + assert!(s.execute("timeline.place", json!({"item": item, "track": "new", "audioTrack": "A99", "time": 0})).is_err()); + assert_eq!(*s.active_sequence().unwrap(), before); + assert_eq!(s.history.undo.len(), undo); +} diff --git a/crates/engine/src/settings.rs b/crates/engine/src/settings.rs index 7b0c11dde..845828aa0 100644 --- a/crates/engine/src/settings.rs +++ b/crates/engine/src/settings.rs @@ -1097,7 +1097,7 @@ static CATEGORIES: &[Category] = &[ f("timeline.multichannelMonoMediaTracks", "Multichannel Mono Media", Kind::Choice(TRACKS), false), ], ), - b("timeline.focusTimelineOnEdit", "Set focus on the Timeline when performing Insert/Overwrite edits", true), + b("timeline.focusTimelineOnEdit", "Set focus on the Timeline when performing Insert/Overwrite edits", false), b("timeline.snapPlayhead", "Snap playhead in Timeline when Snap is enabled", true), 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), diff --git a/crates/engine/src/settings_tests.rs b/crates/engine/src/settings_tests.rs index 241f36f6d..50a3cf3af 100644 --- a/crates/engine/src/settings_tests.rs +++ b/crates/engine/src/settings_tests.rs @@ -251,6 +251,18 @@ fn schema_command_describes_every_category() { assert_eq!(auto["value"], "pageScroll"); assert_eq!(auto["wired"], true); assert!(auto["kind"]["choices"].as_array().unwrap().iter().any(|c| c["value"] == "smoothScroll")); + + // This option is stored, but Insert/Overwrite does not yet implement focus transfer. + let focus = tl["fields"].as_array().unwrap().iter().find(|f| f["key"] == "timeline.focusTimelineOnEdit").unwrap(); + assert_eq!(focus["kind"], json!({"type": "bool"})); + assert_eq!(focus["wired"], false); + assert_eq!(focus["value"], false); + set(&mut s, "timeline.focusTimelineOnEdit", json!(true)); + let updated = s.execute("prefs.schema", json!({"category": "timeline"})).unwrap(); + let focus = updated["categories"][0]["fields"].as_array().unwrap().iter().find(|f| f["key"] == "timeline.focusTimelineOnEdit").unwrap(); + assert_eq!(focus["value"], true); + assert_eq!(focus["wired"], false); + let one = s.execute("prefs.schema", json!({"category": "trim"})).unwrap(); assert_eq!(one["categories"].as_array().unwrap().len(), 1); assert!(s.execute("prefs.schema", json!({"category": "nope"})).is_err()); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f2b50dd8a..9852734d7 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -139,45 +139,6 @@ fn trim_linked_and_ripple_delete() { assert_eq!(q.video_tracks[0].items[0].start, s.sequence_rate().tick_of(12), "the trim gap remains"); } -#[test] -fn clear_respects_individually_selected_linked_clips_and_undo() { - for audio in [false, true] { - let mut s = demo(); - let video = s.active_sequence().unwrap().video_tracks[0].items[0].clone(); - let sound = s.active_sequence().unwrap().audio_tracks[0].items.iter().find(|i| i.link == video.link).unwrap().clone(); - let (selected, partner) = if audio { (sound.id, video.id) } else { (video.id, sound.id) }; - s.execute("timeline.select", json!({"clips": [selected.0], "linked": false})).unwrap(); - assert_eq!(s.state.selection, vec![selected]); - s.execute("edit.clear", json!({})).unwrap(); - assert!(s.active_sequence().unwrap().find_item(selected).is_none()); - assert!(s.active_sequence().unwrap().find_item(partner).is_some()); - s.execute("edit.undo", json!({})).unwrap(); - assert!(s.active_sequence().unwrap().find_item(selected).is_some()); - s.execute("timeline.select", json!({"clips": [selected.0]})).unwrap(); - assert_eq!(s.state.selection.len(), 2); - s.execute("edit.clear", json!({})).unwrap(); - assert!(s.active_sequence().unwrap().find_item(partner).is_none()); - } -} - -#[test] -fn ripple_delete_respects_individual_selection_and_explicit_clear_keeps_link_behavior() { - let mut s = demo(); - let video = s.active_sequence().unwrap().video_tracks[0].items[0].clone(); - let sound = s.active_sequence().unwrap().audio_tracks[0].items.iter().find(|i| i.link == video.link).unwrap().id; - let tracks: Vec<_> = s.active_sequence().unwrap().all_tracks().map(|t| t.id.0).collect(); - for track in tracks { - s.execute("timeline.setTrack", json!({"track": track, "syncLock": false})).unwrap(); - } - s.execute("timeline.select", json!({"clips": [video.id.0], "linked": false})).unwrap(); - s.execute("edit.rippleDelete", json!({})).unwrap(); - assert!(s.active_sequence().unwrap().find_item(video.id).is_none()); - assert!(s.active_sequence().unwrap().find_item(sound).is_some()); - s.execute("edit.undo", json!({})).unwrap(); - s.execute("edit.clear", json!({"clips": [video.id.0]})).unwrap(); - assert!(s.active_sequence().unwrap().find_item(sound).is_none()); -} - #[test] fn slide_moves_linked_audio_with_the_video() { let mut s = demo(); diff --git a/crates/engine/src/trim_tests.rs b/crates/engine/src/trim_tests.rs index 257357120..b003e48dd 100644 --- a/crates/engine/src/trim_tests.rs +++ b/crates/engine/src/trim_tests.rs @@ -236,3 +236,21 @@ fn extend_to_playhead_moves_each_unequal_out_point_to_the_playhead() { assert_eq!(q.find_item(a).unwrap().1.end(), ph); assert_eq!(q.find_item(b).unwrap().1.end(), ph); } + +/// #653: the delta the Timeline previews while an edge is dragged is the one `timeline.trim` +/// applies on release: dragging the last clip's Out edge far past the end of its media stops at +/// the media's end instead of following the pointer. +#[test] +fn trim_delta_stops_at_the_end_of_the_media() { + let mut s = demo(); + let last = *v1(&s).last().unwrap(); + let clip = ClipId(last.0); + let far = Tick(3600 * filmcraft_time::TICKS_PER_SECOND); + let shown = commands::trim_delta(&s, clip, filmcraft_edit::Edge::Out, filmcraft_edit::TrimMode::Regular, far).unwrap(); + assert!(Tick::ZERO < shown && shown < far, "clamped to the media left after the clip: {shown:?}"); + let r = s.execute("timeline.trim", json!({"clip": last.0, "edge": "out", "mode": "regular", "delta": far.0})).unwrap(); + assert_eq!(r["delta"], json!(shown.0), "the preview and the trim agree"); + let small = s.sequence_rate().frame_duration(); + let back = commands::trim_delta(&s, clip, filmcraft_edit::Edge::Out, filmcraft_edit::TrimMode::Regular, -small).unwrap(); + assert_eq!(back, -small, "a trim within the media is not clamped"); +} diff --git a/crates/export/src/lib.rs b/crates/export/src/lib.rs index 43966c055..4ea991d8f 100644 --- a/crates/export/src/lib.rs +++ b/crates/export/src/lib.rs @@ -239,7 +239,7 @@ pub struct ExportSettings { /// `progress.total`, `finished` and the final status; this call only adds to `done`. #[serde(default)] pub part_of_batch: bool, - /// ProRes flavour: `proxy`, `lt`, `standard` or `hq` (empty = HQ). + /// ProRes flavour: `proxy`, `lt`, `standard`, `hq`, `4444` or `4444xq` (empty = HQ). #[serde(default)] pub prores_profile: String, /// DNxHR profile: `lb`, `sq`, `hq` or `hqx` (empty = HQ). @@ -256,8 +256,9 @@ pub struct ExportSettings { /// sequence's HDR space and signal it (VUI / `colr` / `mdcv` / `clli` / SEI). #[serde(default)] pub sdr: bool, - /// Keep the alpha channel in PNG and TIFF sequences (straight alpha, Premiere's "Include - /// Alpha Channel"). Off, every frame is flattened over black (#160). Other formats ignore it. + /// Keep the alpha channel in PNG and TIFF sequences and ProRes 4444 / 4444 XQ QuickTime movies + /// (straight alpha, Premiere's "Include Alpha Channel"). Off, every frame is flattened over + /// black (#160). Other formats ignore it. #[serde(default)] pub alpha: bool, /// Output frame size (None = Match Source: the sequence size times `scale`). @@ -1006,6 +1007,8 @@ fn aac_factory(_format: Format, sample_rate: u32, channels: u32, s: &ExportSetti struct ProResEncoder { enc: filmcraft_prores::Encoder, profile: filmcraft_prores::Profile, + /// Code the frames' alpha channel (4444 profiles with Include Alpha Channel). + alpha: bool, w: u32, h: u32, rate: FrameRate, @@ -1022,13 +1025,19 @@ impl VideoEncoder for ProResEncoder { self.rate.num as u32 } fn encode(&mut self, f: &EncoderFrame) -> Result> { - let mut fr = filmcraft_prores::Frame::new(f.width, f.height, filmcraft_prores::ChromaFormat::Yuv422, 10, false); - timed(Stage::Convert, || match f.hdr { - Some(rgb) => { - let (kr, kb) = self.signal.kr_kb(); - rgbf_to_yuv422_10(rgb, f.width as usize, f.height as usize, kr, kb, &mut fr.y, &mut fr.cb, &mut fr.cr) + let chroma = self.profile.chroma(); + let mut fr = filmcraft_prores::Frame::new(f.width, f.height, chroma, 10, self.alpha); + let (w, h) = (f.width as usize, f.height as usize); + timed(Stage::Convert, || match (chroma, f.hdr) { + (filmcraft_prores::ChromaFormat::Yuv444, hdr) => { + let (kr, kb) = if hdr.is_some() { self.signal.kr_kb() } else { (0.2126, 0.0722) }; + to_yuv444_10(f.rgba, hdr, w, h, kr, kb, &mut fr.y, &mut fr.cb, &mut fr.cr, fr.alpha.as_deref_mut()) } - None => rgba_to_yuv422_10(f.rgba, f.width as usize, f.height as usize, &mut fr.y, &mut fr.cb, &mut fr.cr), + (_, Some(rgb)) => { + let (kr, kb) = self.signal.kr_kb(); + rgbf_to_yuv422_10(rgb, w, h, kr, kb, &mut fr.y, &mut fr.cb, &mut fr.cr) + } + (_, None) => rgba_to_yuv422_10(f.rgba, w, h, &mut fr.y, &mut fr.cb, &mut fr.cr), }); let data = self.enc.encode(&fr).map_err(|e| ExportError::Encode(e.to_string()))?; Ok(vec![EncodedPacket { data, key: true, duration: self.rate.den as u32, composition_offset: 0 }]) @@ -1038,6 +1047,47 @@ impl VideoEncoder for ProResEncoder { } } +/// Limited-range 10-bit 4:4:4 with matrix (Kr, Kb) from straight RGBA8, or from encoded R'G'B' +/// floats (3 per pixel) when `hdr` is given. `alpha` (full range, 0 = transparent) takes the +/// RGBA8 alpha; it stays opaque for float input, which carries none. +#[allow(clippy::too_many_arguments)] +pub fn to_yuv444_10( + rgba: &[u8], + hdr: Option<&[f32]>, + w: usize, + h: usize, + kr: f32, + kb: f32, + y: &mut [u16], + cb: &mut [u16], + cr: &mut [u16], + alpha: Option<&mut [u16]>, +) { + let (w, n) = (w.max(1), w.saturating_mul(h)); + let kg = 1.0 - kr - kb; + let (sb, sr) = (2.0 * (1.0 - kb), 2.0 * (1.0 - kr)); + let rgb_at = |i: usize| -> [f32; 3] { + match hdr { + Some(f) => f.get(i * 3..i * 3 + 3).map_or([0.0; 3], |p| [p[0], p[1], p[2]]), + None => rgba.get(i * 4..i * 4 + 3).map_or([0.0; 3], |p| [p[0] as f32 / 255.0, p[1] as f32 / 255.0, p[2] as f32 / 255.0]), + } + }; + y.par_chunks_mut(w).zip(cb.par_chunks_mut(w).zip(cr.par_chunks_mut(w))).enumerate().for_each(|(row, (yr, (cbr, crr)))| { + for (x, (yv, (cbv, crv))) in yr.iter_mut().zip(cbr.iter_mut().zip(crr.iter_mut())).enumerate() { + let [r, g, b] = rgb_at(row * w + x); + let yy = kr * r + kg * g + kb * b; + *yv = (64.0 + 876.0 * yy).round().clamp(4.0, 1019.0) as u16; + *cbv = (512.0 + 896.0 * (b - yy) / sb).round().clamp(4.0, 1019.0) as u16; + *crv = (512.0 + 896.0 * (r - yy) / sr).round().clamp(4.0, 1019.0) as u16; + } + }); + if let (Some(a), None) = (alpha, hdr) { + for (i, dst) in a.iter_mut().enumerate().take(n) { + *dst = rgba.get(i * 4 + 3).map_or(1023, |&v| ((v as u32 * 1023 + 127) / 255) as u16); + } + } +} + /// BT.709 limited-range 10-bit 4:2:2 from straight RGBA8 (chroma averaged horizontally). pub fn rgba_to_yuv422_10(rgba: &[u8], w: usize, h: usize, y: &mut [u16], cb: &mut [u16], cr: &mut [u16]) { let cw = w.div_ceil(2); @@ -1071,6 +1121,8 @@ pub fn prores_profile(name: &str) -> filmcraft_prores::Profile { "proxy" => Profile::Proxy, "lt" => Profile::Lt, "standard" | "422" => Profile::Standard, + "4444" => Profile::P4444, + "4444xq" | "4444 xq" | "4444-xq" | "xq" => Profile::P4444Xq, _ => Profile::Hq, } } @@ -1109,7 +1161,10 @@ fn prores_factory(format: Format, w: u32, h: u32, rate: FrameRate, s: &ExportSet if s.signal.is_hdr() { cfg.color = filmcraft_prores::ColorInfo { primaries: s.signal.primaries, transfer: s.signal.transfer, matrix: s.signal.matrix }; } - Ok(Box::new(ProResEncoder { enc: filmcraft_prores::Encoder::with_config(cfg), profile, w, h, rate, signal: s.signal }) as Box) + let alpha = s.keeps_alpha(); + cfg.encode_alpha = alpha; + let enc = filmcraft_prores::Encoder::with_config(cfg); + Ok(Box::new(ProResEncoder { enc, profile, alpha, w, h, rate, signal: s.signal }) as Box) }) } diff --git a/crates/export/src/mxf_out.rs b/crates/export/src/mxf_out.rs index 24233e7a3..ecd8ede23 100644 --- a/crates/export/src/mxf_out.rs +++ b/crates/export/src/mxf_out.rs @@ -38,7 +38,9 @@ fn picture(settings: &ExportSettings, w: u32, h: u32) -> PictureDesc { Profile::Proxy => 1, Profile::Lt => 2, Profile::Standard => 3, - _ => 4, + Profile::Hq => 4, + Profile::P4444 => 5, + Profile::P4444Xq => 6, }, } } @@ -53,9 +55,6 @@ fn picture(settings: &ExportSettings, w: u32, h: u32) -> PictureDesc { _ => PictureCoding::Vc3 { cid: crate::dnx_profile(&settings.dnx_profile).cid() }, }; let mut d = PictureDesc::new(coding, w, h); - if matches!(coding, PictureCoding::ProRes { .. }) { - d.depth = 10; - } d.color = match settings.signal { s if s == ColorSignal::PQ => ColorSpace::Rec2020Pq, s if s == ColorSignal::HLG => ColorSpace::Rec2020Hlg, diff --git a/crates/export/src/pipeline.rs b/crates/export/src/pipeline.rs index 306aef2fd..78503d002 100644 --- a/crates/export/src/pipeline.rs +++ b/crates/export/src/pipeline.rs @@ -12,7 +12,7 @@ use filmcraft_time::FrameRate; use rayon::prelude::*; use crate::settings::{ExportEffects, Placement, Scaling, TextOverlay}; -use crate::{ExportError, ExportSettings, Format, FrameRenderer, GpuRendering, Result, build_frame_renderer, note_gpu_fallback, note_gpu_frame}; +use crate::{ExportError, ExportSettings, FrameRenderer, GpuRendering, Result, build_frame_renderer, note_gpu_fallback, note_gpu_frame}; /// Where the rendered picture lands in the output frame. #[derive(Clone, Copy, Debug, PartialEq)] @@ -201,7 +201,7 @@ impl Pipeline { geom, opts, hdr_out, - alpha: settings.alpha && matches!(settings.format, Format::PngSequence | Format::TiffSequence), + alpha: settings.keeps_alpha(), out_tf, effects, overlay, diff --git a/crates/export/src/presets.rs b/crates/export/src/presets.rs index 3aa37d056..4323de772 100644 --- a/crates/export/src/presets.rs +++ b/crates/export/src/presets.rs @@ -122,6 +122,14 @@ pub fn builtin_presets() -> Vec { p("Apple ProRes 422", bc, "QuickTime, ProRes 422, 24-bit 48 kHz PCM", mov(Format::ProRes, "standard")), p("Apple ProRes 422 LT", bc, "QuickTime, ProRes 422 LT, 24-bit 48 kHz PCM", mov(Format::ProRes, "lt")), p("Apple ProRes 422 Proxy", bc, "QuickTime, ProRes 422 Proxy, 24-bit 48 kHz PCM", mov(Format::ProRes, "proxy")), + p("Apple ProRes 4444", bc, "QuickTime, ProRes 4444 (4:4:4), 24-bit 48 kHz PCM", mov(Format::ProRes, "4444")), + p( + "Apple ProRes 4444 with Alpha", + bc, + "QuickTime, ProRes 4444 (4:4:4) with alpha channel, 24-bit 48 kHz PCM", + ExportSettings { alpha: true, ..mov(Format::ProRes, "4444") }, + ), + p("Apple ProRes 4444 XQ", bc, "QuickTime, ProRes 4444 XQ (4:4:4), 24-bit 48 kHz PCM", mov(Format::ProRes, "4444xq")), p("Avid DNxHR HQ", bc, "QuickTime, DNxHR HQ (8-bit 4:2:2), 24-bit 48 kHz PCM", mov(Format::DnxHr, "hq")), p("Avid DNxHR SQ", bc, "QuickTime, DNxHR SQ (8-bit 4:2:2), 24-bit 48 kHz PCM", mov(Format::DnxHr, "sq")), p("Avid DNxHR LB", bc, "QuickTime, DNxHR LB (8-bit 4:2:2), 24-bit 48 kHz PCM", mov(Format::DnxHr, "lb")), diff --git a/crates/export/src/settings.rs b/crates/export/src/settings.rs index e56b09216..fc220e8b3 100644 --- a/crates/export/src/settings.rs +++ b/crates/export/src/settings.rs @@ -378,6 +378,21 @@ impl ExportSettings { if self.format.is_mxf() { self.mxf_video_codec.encoder_format() } else { self.format } } + /// Whether the output can carry an alpha channel ([`ExportSettings::alpha`]): PNG and TIFF + /// sequences and ProRes 4444 / 4444 XQ QuickTime movies. + pub fn supports_alpha(&self) -> bool { + match self.format { + Format::PngSequence | Format::TiffSequence => true, + Format::ProRes => crate::prores_profile(&self.prores_profile).chroma() == filmcraft_prores::ChromaFormat::Yuv444, + _ => false, + } + } + + /// Whether the exported frames keep their alpha channel instead of being flattened over black. + pub fn keeps_alpha(&self) -> bool { + self.alpha && self.supports_alpha() + } + /// Whether the output is a numbered image sequence. pub fn is_image_sequence(&self) -> bool { matches!(self.format, Format::PngSequence | Format::TiffSequence | Format::BmpSequence) @@ -536,7 +551,9 @@ impl ExportSettings { Profile::Proxy => ", ProRes 422 Proxy", Profile::Lt => ", ProRes 422 LT", Profile::Standard => ", ProRes 422", - _ => ", ProRes 422 HQ", + Profile::Hq => ", ProRes 422 HQ", + Profile::P4444 => ", ProRes 4444", + Profile::P4444Xq => ", ProRes 4444 XQ", } } Format::DnxHr => v += &format!(", DNxHR {}", if self.dnx_profile.is_empty() { "HQ".into() } else { self.dnx_profile.to_ascii_uppercase() }), diff --git a/crates/export/src/tests.rs b/crates/export/src/tests.rs index 656ddd9f5..ae8e9fd6c 100644 --- a/crates/export/src/tests.rs +++ b/crates/export/src/tests.rs @@ -101,6 +101,51 @@ fn prores_export_roundtrip() { assert!(f[0] > 240 && f[1] < 15 && f[2] < 15, "{:?}", &f[..4]); } +/// ProRes 4444 / 4444 XQ export (#342): 4:4:4 frames under the `ap4h` / `ap4x` codes, and with +/// Include Alpha Channel the picture's alpha survives instead of being flattened over black. +#[test] +fn prores_4444_export_roundtrip() { + let (p, seq, m) = project(); + for (profile, label) in [("4444", "ProRes 4444"), ("4444xq", "ProRes 4444 XQ")] { + let path = tmp(&format!("pr_{profile}.mov")); + let s = ExportSettings { format: Format::ProRes, path: path.clone(), prores_profile: profile.into(), ..Default::default() }; + export(&p, seq, &s, &m, &Progress::default()).unwrap(); + let bytes: Arc<[u8]> = std::fs::read(&path).unwrap().into(); + let src = filmcraft_codecs::open_bytes("pr.mov", bytes).unwrap(); + let codec = src.info().video.as_ref().unwrap().codec.clone(); + assert!(codec.contains(label), "{profile}: {codec}"); + let f = src.video_frame(FrameRequest::full(Tick(TICKS_PER_SECOND / 3))).unwrap(); + assert!(f.format_label().contains("4:4:4"), "{profile}: {}", f.format_label()); + let px = f.to_rgba8(); + assert!(px[0] > 240 && px[1] < 15 && px[2] < 15 && px[3] == 255, "{profile}: {:?}", &px[..4]); + } + + // a half-transparent matte keeps its alpha with Include Alpha Channel, and only then + let mut p = (*p).clone(); + let half = GeneratorSource::new(Generator::ColorMatte { color: [1.0, 0.0, 0.0, 0.5] }, 320, 180, FrameRate::FPS_24, Tick(2 * TICKS_PER_SECOND)); + let item = p.sequence(seq).unwrap().video_tracks[0].items[0].item; + if let ItemKind::Media(mc) = &mut p.item_mut(item).unwrap().kind { + mc.media = MediaRef::Generator(half.generator.clone()); + } + let mut m = m; + m.0.insert(item, Arc::new(half)); + let p = Arc::new(p); + for alpha in [true, false] { + let path = tmp(&format!("pr_4444_alpha_{alpha}.mov")); + let s = ExportSettings { format: Format::ProRes, path: path.clone(), prores_profile: "4444".into(), alpha, ..Default::default() }; + assert!(s.keeps_alpha() == alpha); + export(&p, seq, &s, &m, &Progress::default()).unwrap(); + let bytes: Arc<[u8]> = std::fs::read(&path).unwrap().into(); + let src = filmcraft_codecs::open_bytes("pr.mov", bytes).unwrap(); + let px = src.video_frame(FrameRequest::full(Tick(TICKS_PER_SECOND / 3))).unwrap().to_rgba8(); + if alpha { + assert!((px[3] as i32 - 128).abs() <= 3, "alpha kept: {:?}", &px[..4]); + } else { + assert_eq!(px[3], 255, "flattened: {:?}", &px[..4]); + } + } +} + #[test] fn dnxhr_export_roundtrip() { let (p, seq, m) = project(); diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 238649afd..70af3c747 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -37,6 +37,8 @@ //! `audio` objects read as stream zero; older builds must refuse new multi-stream projects. //! - **v14** (M7.9): narrations (`Project::narrations`, Text to Speech). No-op step; older builds //! would drop them when saving. +//! - **v15**: project notes (`Project::notes`, Window ▸ Project Notes). No-op step; older builds +//! would drop them when saving. #![cfg_attr(not(test), deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable))] @@ -58,7 +60,7 @@ pub type Migration = fn(Value) -> Result; /// `MIGRATIONS[i]` upgrades schema `i + 1` to `i + 2`. Append one function per schema bump; never /// edit a shipped one. pub const MIGRATIONS: &[Migration] = - &[v1_to_v2, v2_to_v3, v3_to_v4, v4_to_v5, v5_to_v6, v6_to_v7, v7_to_v8, v8_to_v9, v9_to_v10, v10_to_v11, v11_to_v12, v12_to_v13, v13_to_v14]; + &[v1_to_v2, v2_to_v3, v3_to_v4, v4_to_v5, v5_to_v6, v6_to_v7, v7_to_v8, v8_to_v9, v9_to_v10, v10_to_v11, v11_to_v12, v12_to_v13, v13_to_v14, v14_to_v15]; /// The schema version this build writes (and the newest it reads). pub const SCHEMA_VERSION: u32 = MIGRATIONS.len() as u32 + 1; @@ -354,13 +356,18 @@ fn v13_to_v14(doc: Value) -> Result { Ok(doc) } +/// v14 → v15: project notes. Existing data needs no change. +fn v14_to_v15(doc: Value) -> Result { + Ok(doc) +} + #[cfg(test)] mod tests { use super::*; #[test] fn schema_version_matches_table() { - assert_eq!(SCHEMA_VERSION, 14); + assert_eq!(SCHEMA_VERSION, 15); } #[test] diff --git a/crates/gpu/src/fx.rs b/crates/gpu/src/fx.rs index 0247105fa..959a0fa0d 100644 --- a/crates/gpu/src/fx.rs +++ b/crates/gpu/src/fx.rs @@ -213,7 +213,7 @@ fn steps(op: &FxOp) -> Vec { let w0 = 1.0 - wh * 0.15; let sh_k = sh * 0.35; let hl_k = hl * 0.35; - let vround_aspect = if *vround < 0.0 { aspect.powf(-vround) } else { 1.0 }; + let vround_aspect = filmcraft_render::gpufx::lumetri_vignette_aspect(*aspect, *vround); let st_k = [(st[0] - 0.5) * 0.3, (st[1] - 0.5) * 0.3, (st[2] - 0.5) * 0.3]; let ht_k = [(ht[0] - 0.5) * 0.3, (ht[1] - 0.5) * 0.3, (ht[2] - 0.5) * 0.3]; diff --git a/crates/media/src/cancel.rs b/crates/media/src/cancel.rs index 5c2b5f28b..bfb391dd2 100644 --- a/crates/media/src/cancel.rs +++ b/crates/media/src/cancel.rs @@ -96,64 +96,10 @@ pub fn cancelled() -> bool { CURRENT.with(|c| c.borrow().as_ref().is_some_and(|f| f.load(Ordering::Relaxed))) } -/// The hints of the work running on this thread ([`with_cancel`], [`with_catch_up`], -/// [`with_draft`], [`with_background`]), to carry them to another thread doing part of that work. -#[derive(Clone, Default)] -pub struct Context { - cancel: Option>, - catch_up: Option, - draft: bool, - background: bool, -} - -impl Context { - /// This thread's hints. - pub fn current() -> Self { - Context { cancel: CURRENT.with(|c| c.borrow().clone()), catch_up: catch_up(), draft: draft(), background: background() } - } - - /// Run `f` with these hints, restoring this thread's however `f` ends (a pool thread must - /// not keep another job's hints after a panic). - pub fn run(self, f: impl FnOnce() -> R) -> R { - struct Restore(Context); - impl Drop for Restore { - fn drop(&mut self) { - let prev = std::mem::take(&mut self.0); - CURRENT.with(|c| *c.borrow_mut() = prev.cancel); - CATCH_UP.with(|c| c.set(prev.catch_up)); - DRAFT.with(|c| c.set(prev.draft)); - BACKGROUND.with(|c| c.set(prev.background)); - } - } - let _restore = Restore(Context::current()); - CURRENT.with(|c| *c.borrow_mut() = self.cancel); - CATCH_UP.with(|c| c.set(self.catch_up)); - DRAFT.with(|c| c.set(self.draft)); - BACKGROUND.with(|c| c.set(self.background)); - f() - } -} - #[cfg(test)] mod tests { use super::*; - #[test] - fn context_carries_the_hints_to_another_thread_and_restores_its_own() { - let flag = Arc::new(AtomicBool::new(true)); - let ctx = with_cancel(&flag, || with_draft(true, || with_background(true, || with_catch_up(Some(Tick(5)), Context::current)))); - std::thread::scope(|s| { - s.spawn(|| { - let seen = with_draft(false, || ctx.clone().run(|| (cancelled(), draft(), background(), catch_up()))); - assert_eq!(seen, (true, true, true, Some(Tick(5)))); - // a panic inside leaves this thread's own hints in place - let r = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| ctx.clone().run(|| panic!("job")))); - assert!(r.is_err()); - assert_eq!((cancelled(), draft(), background(), catch_up()), (false, false, false, None)); - }); - }); - } - #[test] fn flag_is_scoped_to_the_closure() { let flag = Arc::new(AtomicBool::new(false)); diff --git a/crates/media/src/lib.rs b/crates/media/src/lib.rs index 6b5ff1b1e..bfc94de79 100644 --- a/crates/media/src/lib.rs +++ b/crates/media/src/lib.rs @@ -230,3 +230,17 @@ impl MediaSource for OfflineSource { Ok(AudioBuffer::silence(sample_rate, self.info.audio().map_or(2, |a| a.channels as usize), frames)) } } + +#[cfg(test)] +mod importable_tests { + use super::*; + use std::path::Path; + + #[test] + fn camera_extensions_match_regardless_of_case() { + for name in ["clip.mov", "clip.MOV", "clip.Mov", "shot.MP4", "IMG_0001.JPG", "sound.WAV", "take.M2TS"] { + assert!(is_importable(Path::new(name)), "{name}"); + } + assert!(!is_importable(Path::new("notes.txt"))); + } +} diff --git a/crates/media/src/sequence.rs b/crates/media/src/sequence.rs index 9521cba44..8c60e0158 100644 --- a/crates/media/src/sequence.rs +++ b/crates/media/src/sequence.rs @@ -75,7 +75,15 @@ impl Numbered { /// The frame number of a sibling file name of this sequence (same prefix, suffix and padding). pub fn number_of(&self, file_name: &str) -> Option { - let mid = file_name.strip_prefix(self.prefix.as_str())?.strip_suffix(self.suffix.as_str())?; + let rest = file_name.strip_prefix(self.prefix.as_str())?; + let mid = { + let n = rest.as_bytes(); + let s = self.suffix.as_bytes(); + if n.len() < s.len() || !n[n.len() - s.len()..].eq_ignore_ascii_case(s) { + return None; + } + &rest[..n.len() - s.len()] + }; if mid.is_empty() || !mid.bytes().all(|b| b.is_ascii_digit()) || mid.len() > 18 { return None; } @@ -231,6 +239,7 @@ mod tests { assert!(!u.padded()); assert_eq!(u.file_name(10), "frame10.TIF"); assert_eq!(u.number_of("frame10.TIF"), Some(10)); + assert_eq!(u.number_of("frame10.tif"), Some(10), "Linux stills often mix .TIF and .tif"); assert_eq!(Numbered::parse("clip.mov"), None, "not a still"); assert_eq!(Numbered::parse("nodigits.png"), None); let w = Numbered::parse("C:\\renders\\out.0010.exr.png").unwrap(); diff --git a/crates/project/src/lib.rs b/crates/project/src/lib.rs index e8a988568..1291ca053 100644 --- a/crates/project/src/lib.rs +++ b/crates/project/src/lib.rs @@ -1253,6 +1253,10 @@ pub struct Project { /// made from, keyed by that item. Schema v14. #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub narrations: BTreeMap, + /// Project notes (Window ▸ Project Notes): free plain text kept with the project, for to-dos, + /// instructions for whoever opens it next, frame numbers and the like. Schema v15. + #[serde(default, skip_serializing_if = "String::is_empty")] + pub notes: String, } /// How a sequence is shown in the Timeline panel: its zoom, scroll position and track heights. @@ -1348,6 +1352,7 @@ impl Project { search_bins: Vec::new(), source_graphics: BTreeMap::new(), narrations: BTreeMap::new(), + notes: String::new(), } } @@ -1380,20 +1385,35 @@ impl Project { id } - /// Move items into a bin (the root when `bin` is None). Returns how many moved; unknown items - /// are skipped. Fails (returns None) when the bin does not exist. + /// Move items, and bins named by id, into a bin (the root when `bin` is None). A bin goes with + /// everything in it. Returns how many moved; unknown ids are skipped, and so is a bin that + /// would land in itself or in one of its own sub-bins. Fails (returns None) when the target + /// bin does not exist. pub fn move_to_bin(&mut self, items: &[ItemId], bin: Option) -> Option { let target = bin.unwrap_or(self.root.id); self.root.find_bin(target)?; let mut moved = 0; for &id in items { - if !self.items.contains_key(&id) || !self.root.remove_item(id) { - continue; - } - if let Some(b) = self.root.find_bin_mut(target) { - b.children.push(BinEntry::Item(id)); - moved += 1; + let entry = if self.items.contains_key(&id) { + if !self.root.remove_item(id) { + continue; + } + BinEntry::Item(id) + } else { + let b = BinId(id.0); + // never the root, nor a bin that holds the target (or is it) + if b == self.root.id || self.root.find_bin(b).is_none_or(|x| x.find_bin(target).is_some()) { + continue; + } + let Some(sub) = self.root.remove_bin(b) else { continue }; + BinEntry::Bin(sub) + }; + match self.root.find_bin_mut(target) { + Some(t) => t.children.push(entry), + // cannot happen (the target is outside what was taken out), but never lose it + None => self.root.children.push(entry), } + moved += 1; } Some(moved) } @@ -1994,6 +2014,29 @@ mod tests { assert_eq!(p.root.parent_of(i), None); } + #[test] + fn a_bin_moves_into_another_bin_with_its_contents() { + let mut p = Project::new("x"); + let outer = p.add_bin("Footage", None); + let inner = p.add_bin("B-Roll", None); + let deep = p.add_bin("Drone", Some(inner)); + let i = p.add_item("a", Label::Iris, ItemKind::AdjustmentLayer { width: 10, height: 10, rate: FrameRate::FPS_24, duration: Tick(1) }, Some(deep)); + assert_eq!(p.move_to_bin(&[ItemId(inner.0)], Some(outer)), Some(1)); + let o = p.root.find_bin(outer).unwrap(); + assert!(o.children.iter().any(|c| matches!(c, BinEntry::Bin(b) if b.id == inner))); + assert!(!p.root.children.iter().any(|c| matches!(c, BinEntry::Bin(b) if b.id == inner))); + assert_eq!(p.root.parent_of(i), Some(deep)); + assert!(o.find_bin(deep).is_some()); + // never into itself or its own sub-bin, and the root never moves + assert_eq!(p.move_to_bin(&[ItemId(outer.0)], Some(outer)), Some(0)); + assert_eq!(p.move_to_bin(&[ItemId(outer.0)], Some(deep)), Some(0)); + assert_eq!(p.move_to_bin(&[ItemId(p.root.id.0)], Some(outer)), Some(0)); + assert!(p.root.find_bin(outer).unwrap().find_bin(deep).is_some()); + // and back out to the root + assert_eq!(p.move_to_bin(&[ItemId(inner.0)], None), Some(1)); + assert!(p.root.children.iter().any(|c| matches!(c, BinEntry::Bin(b) if b.id == inner))); + } + #[test] fn project_clone_shares_sequences_until_mutated() { fn sequence_arc(p: &Project, id: ItemId) -> &std::sync::Arc { diff --git a/crates/render/src/audio_fx.rs b/crates/render/src/audio_fx.rs index adf9d35ed..db84a1f6c 100644 --- a/crates/render/src/audio_fx.rs +++ b/crates/render/src/audio_fx.rs @@ -44,12 +44,11 @@ pub(crate) fn mapping(id: &str) -> Option { }, "hard_limiter" => Mapping { dsp: "limiter", - preroll: |e, t| (f(e, "lookahead", t) as f64 / 1000.0 + f(e, "release", t) as f64 / 1000.0 * 5.0).clamp(0.05, 2.0), + preroll: |e, t| (f(e, "release", t) as f64 / 1000.0 * 5.0).clamp(0.05, 2.0), apply: |d, e, t| { d.set_param("ceiling", f(e, "max", t)); d.set_param("input_gain", f(e, "boost", t).max(0.0)); d.set_param("release", f(e, "release", t)); - d.set_param("lookahead", f(e, "lookahead", t)); }, }, "delay" => Mapping { diff --git a/crates/render/src/effects.rs b/crates/render/src/effects.rs index a20e94756..f4eeb3ba0 100644 --- a/crates/render/src/effects.rs +++ b/crates/render/src/effects.rs @@ -679,7 +679,7 @@ fn lumetri(img: &mut Image, e: &EffectInstance, cx: &FxCtx) { v = v.map(|q| l3 + (q - l3) * s); // vignette if va.abs() > 1e-4 { - let nx = (x as f32 / w - 0.5) * 2.0 * (1.0 + vround * 0.0) * if vround < 0.0 { aspect.powf(-vround) } else { 1.0 }; + let nx = (x as f32 / w - 0.5) * 2.0 * crate::gpufx::lumetri_vignette_aspect(aspect, vround); let ny = (y as f32 / h - 0.5) * 2.0; let d = (nx * nx + ny * ny).sqrt() / std::f32::consts::SQRT_2; let edge = ((d - vmid * 0.9) / (vfeather.max(0.01) * 0.9)).clamp(0.0, 1.0); @@ -1515,4 +1515,57 @@ mod lumetri_cpu_parity_tests { eprintln!("{name}: max sample diff vs fn lumetri: {max_diff:.2e}"); } } + + /// #719: positive Roundness rounds the Lumetri vignette (it used to be ignored), +100 + /// is a circle and negative values widen the oval, on the CPU path and the GPU op alike. + #[test] + fn lumetri_vignette_roundness_shapes_the_vignette() { + let (w, h) = (80usize, 40usize); + let cx = FxCtx { + t: Tick::ZERO, + px_scale: 1.0, + seconds: 0.0, + timecode: "", + clip_name: "", + project: None, + env: None, + working: filmcraft_color::WorkingSpace::Rec709, + }; + let render = |roundness: f64| { + let e = effect(&[ + ("basic_on", ParamValue::Bool(false)), + ("creative_on", ParamValue::Bool(false)), + ("vignette_amount", fl(-3.0)), + ("vignette_roundness", fl(roundness)), + ]); + let grey = Image { w, h, px: [0.5, 0.5, 0.5, 1.0].repeat(w * h) }; + let mut cpu = grey.clone(); + lumetri(&mut cpu, &e, &cx); + let mut op_img = grey; + let op = FxOp::eval(&e, &cx, w, h).expect("lumetri op"); + op.apply(&mut op_img); + for (a, b) in cpu.px.iter().zip(&op_img.px) { + assert!((a - b).abs() <= 1e-6, "roundness {roundness}: CPU and GPU op disagree"); + } + cpu + }; + let red = |img: &Image, x: usize, y: usize| img.px[(y * w + x) * 4]; + // 16 px right of and 16 px below the centre (40, 20) + let (right, below) = ((56, 20), (40, 36)); + + let ellipse = render(0.0); + assert!(red(&ellipse, right.0, right.1) > red(&ellipse, below.0, below.1) + 0.01, "0 follows the frame"); + + let circle = render(100.0); + assert_ne!(circle.px, ellipse.px, "positive roundness changes the vignette"); + let (r, b) = (red(&circle, right.0, right.1), red(&circle, below.0, below.1)); + assert!((r - b).abs() < 1e-4, "+100 is a circle: {r} vs {b}"); + assert!(r < red(&ellipse, right.0, right.1), "+100 darkens the sides more than 0"); + let half = render(50.0); + assert!(red(&half, 70, 20) < red(&ellipse, 70, 20), "+50 darkens the sides more than 0"); + assert!(red(&half, 70, 20) > red(&circle, 70, 20), "+50 darkens the sides less than +100"); + + let oval = render(-100.0); + assert!(red(&oval, 79, 20) > red(&ellipse, 79, 20), "negative values widen the oval"); + } } diff --git a/crates/render/src/gpufx.rs b/crates/render/src/gpufx.rs index 0b432ea0c..572d9627d 100644 --- a/crates/render/src/gpufx.rs +++ b/crates/render/src/gpufx.rs @@ -981,7 +981,7 @@ impl FxOp { v = v.map(|q| l3 + (q - l3) * s); // vignette if vignette_on && va.abs() > 1e-4 { - let nx = (x as f32 / w - 0.5) * 2.0 * (1.0 + vround * 0.0) * if vround < 0.0 { aspect.powf(-vround) } else { 1.0 }; + let nx = (x as f32 / w - 0.5) * 2.0 * lumetri_vignette_aspect(aspect, vround); let ny = (y as f32 / h - 0.5) * 2.0; let d = (nx * nx + ny * ny).sqrt() / std::f32::consts::SQRT_2; let edge = ((d - vmid * 0.9) / (vfeather.max(0.01) * 0.9)).clamp(0.0, 1.0); @@ -996,6 +996,17 @@ impl FxOp { } } +/// Horizontal scale of the Lumetri vignette's coordinates for a Roundness of `vround` +/// (−1 … 1) on a frame `aspect` wide. At 0 the vignette is an ellipse that follows the +/// frame; positive values move it towards a circle (1 = a true circle), negative values +/// stretch it into a wider oval. Non-finite or out-of-range input never yields NaN. +pub fn lumetri_vignette_aspect(aspect: f32, vround: f32) -> f32 { + if !aspect.is_finite() || aspect <= 0.0 || !vround.is_finite() { + return 1.0; + } + aspect.powf(vround.clamp(-1.0, 1.0)) +} + /// sRGB encoding that keeps values above 1 (super-whites) instead of clamping them. #[inline] fn linear_to_enc(v: f32) -> f32 { diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index ecb02d9d0..31f5edfc0 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -301,16 +301,6 @@ fn audio_fx_limiter_holds_ceiling() { assert!(peak <= ceiling * 1.01 && peak > ceiling * 0.7, "peak {peak} vs ceiling {ceiling}"); } -#[test] -fn audio_fx_limiter_lookahead_changes_the_mix() { - let (p0, seq0, map0) = tone_with(&[("amplify", &[("gain", 30.0)]), ("hard_limiter", &[("max", -6.0), ("lookahead", 0.0)])]); - let (p1, seq1, map1) = tone_with(&[("amplify", &[("gain", 30.0)]), ("hard_limiter", &[("max", -6.0), ("lookahead", 30.0)])]); - let a = mix(&p0, seq0, &map0, 0, 48_000); - let b = mix(&p1, seq1, &map1, 0, 48_000); - let max = a.iter().zip(&b).map(|(x, y)| (x - y).abs()).fold(0f32, f32::max); - assert!(max > 1e-4, "look-ahead of 0 ms and 30 ms mixed the same clip"); -} - #[test] fn audio_fx_chain_is_continuous_and_random_access_matches() { let (p, seq, map) = tone_with(&[("delay", &[("delay", 0.05), ("feedback", 50.0), ("mix", 50.0)])]); diff --git a/crates/ui-egui/src/dock.rs b/crates/ui-egui/src/dock.rs index 05acf45b7..d2242b260 100644 --- a/crates/ui-egui/src/dock.rs +++ b/crates/ui-egui/src/dock.rs @@ -37,10 +37,11 @@ pub enum PanelKind { Progress, ReferenceMonitor, Timecode, + ProjectNotes, } impl PanelKind { - pub const ALL: [PanelKind; 27] = [ + pub const ALL: [PanelKind; 28] = [ PanelKind::Project, PanelKind::MediaBrowser, PanelKind::Libraries, @@ -68,6 +69,7 @@ impl PanelKind { PanelKind::Progress, PanelKind::ReferenceMonitor, PanelKind::Timecode, + PanelKind::ProjectNotes, ]; pub fn title(self) -> &'static str { match self { @@ -98,6 +100,7 @@ impl PanelKind { PanelKind::Progress => "Progress", PanelKind::ReferenceMonitor => "Reference Monitor", PanelKind::Timecode => "Timecode", + PanelKind::ProjectNotes => "Project Notes", } } pub fn id(self) -> String { diff --git a/crates/ui-egui/src/header.rs b/crates/ui-egui/src/header.rs index e3ccfda26..721e35cfa 100644 --- a/crates/ui-egui/src/header.rs +++ b/crates/ui-egui/src/header.rs @@ -147,7 +147,7 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { } if resp.clicked() { - crate::links::open(ui.ctx(), crate::links::DISCORD); + crate::links::open(app, ui.ctx(), crate::links::DISCORD); } } let popup_id = egui::Id::new("workspaces-popup"); diff --git a/crates/ui-egui/src/i18n/es.tsv b/crates/ui-egui/src/i18n/es.tsv index 3f45ce9da..ca7301880 100644 --- a/crates/ui-egui/src/i18n/es.tsv +++ b/crates/ui-egui/src/i18n/es.tsv @@ -972,6 +972,7 @@ Reset… Restablecer… Source · {name} Origen · {name} Reset Effect Restablecer efecto + Reset Parameter Restablecer parámetro Save Preset… Guardar ajuste preestablecido… Custom Setup Configuración personalizada Edit… Editar… @@ -2984,6 +2985,9 @@ Apple ProRes 422 HQ Apple ProRes 422 HQ Apple ProRes 422 LT Apple ProRes 422 LT Apple ProRes 422 Proxy Apple ProRes 422 Proxy + Apple ProRes 4444 Apple ProRes 4444 + Apple ProRes 4444 with Alpha Apple ProRes 4444 con alfa + Apple ProRes 4444 XQ Apple ProRes 4444 XQ Application Aplicación Apply Guide Template Aplicar plantilla de guías Auto Save Now Guardar automáticamente ahora @@ -3193,6 +3197,9 @@ QuickTime, ProRes 422 HQ, 24-bit 48 kHz PCM QuickTime, ProRes 422 HQ, PCM de 24 bits y 48 kHz QuickTime, ProRes 422 LT, 24-bit 48 kHz PCM QuickTime, ProRes 422 LT, PCM de 24 bits y 48 kHz QuickTime, ProRes 422 Proxy, 24-bit 48 kHz PCM QuickTime, ProRes 422 Proxy, PCM de 24 bits y 48 kHz + QuickTime, ProRes 4444 (4:4:4), 24-bit 48 kHz PCM QuickTime, ProRes 4444 (4:4:4), PCM de 24 bits y 48 kHz + QuickTime, ProRes 4444 (4:4:4) with alpha channel, 24-bit 48 kHz PCM QuickTime, ProRes 4444 (4:4:4) con canal alfa, PCM de 24 bits y 48 kHz + QuickTime, ProRes 4444 XQ (4:4:4), 24-bit 48 kHz PCM QuickTime, ProRes 4444 XQ (4:4:4), PCM de 24 bits y 48 kHz QuickTime, ProRes 422, 24-bit 48 kHz PCM QuickTime, ProRes 422, PCM de 24 bits y 48 kHz Quit FilmCraft Salir de FilmCraft Rate Stretch Tool Herramienta Ajuste de velocidad @@ -3524,4 +3531,8 @@ Composite on the GPU when available Componer en la GPU cuando esté disponible Text to Speech Texto a voz Edit Narration Editar narración + Project Notes Notas del proyecto + Get Project Notes Obtener notas del proyecto + Edit Project Notes Editar notas del proyecto + Notes for this project: to-dos, instructions, frame numbers… Saved with the project. Notas de este proyecto: tareas pendientes, instrucciones, números de fotograma… Se guardan con el proyecto. Use Effect Shutter Angle Usar el ángulo de obturación del efecto diff --git a/crates/ui-egui/src/i18n/ja.tsv b/crates/ui-egui/src/i18n/ja.tsv index 3d54f8737..7f02f91e4 100644 --- a/crates/ui-egui/src/i18n/ja.tsv +++ b/crates/ui-egui/src/i18n/ja.tsv @@ -970,6 +970,7 @@ Reset… リセット… Source · {name} ソース · {name} Reset Effect エフェクトをリセット + Reset Parameter パラメーターをリセット Save Preset… プリセットを保存… Custom Setup カスタム設定 Edit… 編集… @@ -2974,6 +2975,9 @@ Apple ProRes 422 HQ Apple ProRes 422 HQ Apple ProRes 422 LT Apple ProRes 422 LT Apple ProRes 422 Proxy Apple ProRes 422 Proxy + Apple ProRes 4444 Apple ProRes 4444 + Apple ProRes 4444 with Alpha Apple ProRes 4444（アルファ付き） + Apple ProRes 4444 XQ Apple ProRes 4444 XQ Application アプリケーション Apply Guide Template ガイドテンプレートを適用 Auto Save Now 今すぐ自動保存 @@ -3183,6 +3187,9 @@ QuickTime, ProRes 422 HQ, 24-bit 48 kHz PCM QuickTime、ProRes 422 HQ、24-bit 48 kHz PCM QuickTime, ProRes 422 LT, 24-bit 48 kHz PCM QuickTime、ProRes 422 LT、24-bit 48 kHz PCM QuickTime, ProRes 422 Proxy, 24-bit 48 kHz PCM QuickTime、ProRes 422 Proxy、24-bit 48 kHz PCM + QuickTime, ProRes 4444 (4:4:4), 24-bit 48 kHz PCM QuickTime、ProRes 4444 (4:4:4)、24-bit 48 kHz PCM + QuickTime, ProRes 4444 (4:4:4) with alpha channel, 24-bit 48 kHz PCM QuickTime、ProRes 4444 (4:4:4) アルファチャンネル付き、24-bit 48 kHz PCM + QuickTime, ProRes 4444 XQ (4:4:4), 24-bit 48 kHz PCM QuickTime、ProRes 4444 XQ (4:4:4)、24-bit 48 kHz PCM QuickTime, ProRes 422, 24-bit 48 kHz PCM QuickTime、ProRes 422、24-bit 48 kHz PCM Quit FilmCraft FilmCraft を終了 Rate Stretch Tool レートストレッチツール @@ -3512,4 +3519,8 @@ Edit Narration ナレーションを編集 Clear Transition トランジションを消去 Set Transition Duration トランジションのデュレーションを設定 + Project Notes プロジェクトメモ + Get Project Notes プロジェクトメモを取得 + Edit Project Notes プロジェクトメモを編集 + Notes for this project: to-dos, instructions, frame numbers… Saved with the project. このプロジェクトのメモ: やること、指示、フレーム番号など… プロジェクトと一緒に保存されます。 GPU acceleration is unavailable; preview renders on the CPU GPU アクセラレーションを利用できません。プレビューは CPU でレンダリングされます diff --git a/crates/ui-egui/src/i18n/pt-br.tsv b/crates/ui-egui/src/i18n/pt-br.tsv index 1e9c995ea..fc7b9fd50 100644 --- a/crates/ui-egui/src/i18n/pt-br.tsv +++ b/crates/ui-egui/src/i18n/pt-br.tsv @@ -355,3 +355,4 @@ # System appearance mode menu command. Next Appearance Mode Próximo modo de aparência + Project Notes Notas do projeto diff --git a/crates/ui-egui/src/i18n/uk.tsv b/crates/ui-egui/src/i18n/uk.tsv index 6fa303f4d..13312b311 100644 --- a/crates/ui-egui/src/i18n/uk.tsv +++ b/crates/ui-egui/src/i18n/uk.tsv @@ -355,3 +355,4 @@ Zoom In Збільшити Zoom Out Зменшити Zoom to Sequence Умістити секвенцію + Project Notes Нотатки проєкту diff --git a/crates/ui-egui/src/i18n/zh-cn.tsv b/crates/ui-egui/src/i18n/zh-cn.tsv index e504e3416..53fc1521d 100644 --- a/crates/ui-egui/src/i18n/zh-cn.tsv +++ b/crates/ui-egui/src/i18n/zh-cn.tsv @@ -963,6 +963,7 @@ Reset… 重置… Source · {name} 源 · {name} Reset Effect 重置效果 + Reset Parameter 重置参数 Save Preset… 保存预设… Custom Setup 自定义设置 Edit… 编辑… @@ -2967,6 +2968,9 @@ Apple ProRes 422 HQ Apple ProRes 422 HQ Apple ProRes 422 LT Apple ProRes 422 LT Apple ProRes 422 Proxy Apple ProRes 422 Proxy + Apple ProRes 4444 Apple ProRes 4444 + Apple ProRes 4444 with Alpha Apple ProRes 4444（带 Alpha） + Apple ProRes 4444 XQ Apple ProRes 4444 XQ Application 应用程序 Apply Guide Template 应用参考线模板 Auto Save Now 立即自动保存 @@ -3176,6 +3180,9 @@ QuickTime, ProRes 422 HQ, 24-bit 48 kHz PCM QuickTime，ProRes 422 HQ，24 位 48 kHz PCM QuickTime, ProRes 422 LT, 24-bit 48 kHz PCM QuickTime，ProRes 422 LT，24 位 48 kHz PCM QuickTime, ProRes 422 Proxy, 24-bit 48 kHz PCM QuickTime，ProRes 422 Proxy，24 位 48 kHz PCM + QuickTime, ProRes 4444 (4:4:4), 24-bit 48 kHz PCM QuickTime，ProRes 4444 (4:4:4)，24 位 48 kHz PCM + QuickTime, ProRes 4444 (4:4:4) with alpha channel, 24-bit 48 kHz PCM QuickTime，ProRes 4444 (4:4:4)，带 Alpha 通道，24 位 48 kHz PCM + QuickTime, ProRes 4444 XQ (4:4:4), 24-bit 48 kHz PCM QuickTime，ProRes 4444 XQ (4:4:4)，24 位 48 kHz PCM QuickTime, ProRes 422, 24-bit 48 kHz PCM QuickTime，ProRes 422，24 位 48 kHz PCM Quit FilmCraft 退出 FilmCraft Rate Stretch Tool 速率伸缩工具 @@ -3501,6 +3508,10 @@ Edit Narration 编辑旁白 Clear Transition 清除过渡 Set Transition Duration 设置过渡持续时间 + Project Notes 项目备注 + Get Project Notes 获取项目备注 + Edit Project Notes 编辑项目备注 + Notes for this project: to-dos, instructions, frame numbers… Saved with the project. 此项目的备注：待办事项、说明、帧号等… 随项目一起保存。 GPU acceleration is unavailable; preview renders on the CPU GPU 加速不可用；预览在 CPU 上渲染 Caption Tracks 字幕轨道 Delete Caption Track 删除字幕轨道 diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 0faa82e1b..2bfa8a561 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -139,6 +139,10 @@ pub struct HostHooks { /// Open a file in its default application, or (`true`) reveal it in the file manager (Edit ▸ /// Edit Original, Help ▸ Reveal Log Files). pub open_path: Option Result<(), String>>>, + /// Open a URL in the system browser (Help menu links, About dialog, Discord button, Import + /// screen). Without it, or when it fails, links go through `egui::Context::open_url`, which + /// opens a new tab on the web (the native egui backend is built without its `links` feature). + pub open_url: Option Result<(), String>>>, /// The operating system's light or dark appearance when egui cannot report it (Linux desktops /// whose Wayland compositor sends no theme to winit). Without it, or without an answer, Auto /// uses `egui::Context::system_theme`. diff --git a/crates/ui-egui/src/links.rs b/crates/ui-egui/src/links.rs index 641463d52..6d980e6c8 100644 --- a/crates/ui-egui/src/links.rs +++ b/crates/ui-egui/src/links.rs @@ -31,8 +31,17 @@ pub fn url_for(command: &str) -> Option<&'static str> { ALL.iter().find(|(id, _, _)| *id == command).map(|(_, _, u)| *u) } -/// Open `url` in the system browser (a new tab on the web). -pub fn open(ctx: &egui::Context, url: &str) { +/// Open `url` in the system browser (a new tab on the web). The desktop host's +/// [`HostHooks::open_url`](crate::HostHooks::open_url) opens it; without the hook, or when it +/// fails, it goes through `ctx.open_url`, which only does something on the web: the native egui +/// backend is built without its `links` feature, so there it would only log (#642). +pub fn open(app: &mut crate::FilmcraftApp, ctx: &egui::Context, url: &str) { + if let Some(f) = app.hooks.open_url.as_mut() { + match f(url) { + Ok(()) => return, + Err(e) => log::warn!("can't open {url} in the browser: {e}"), + } + } ctx.open_url(egui::OpenUrl::new_tab(url)); } @@ -48,4 +57,35 @@ mod tests { assert_eq!(url_for("help.discord"), Some(DISCORD)); assert_eq!(url_for("help.nope"), None); } + + #[test] + fn links_open_through_the_host_hook() { + // #642: on the desktop every link (Help menu, About, Discord button, Import screen) must + // reach the host's browser opener; `ctx.open_url` alone is a no-op natively. + use std::cell::RefCell; + use std::rc::Rc; + let opened: Rc>> = Rc::default(); + let rec = opened.clone(); + let mut app = crate::FilmcraftApp::new(filmcraft_engine::Session::default()); + app.hooks.open_url = Some(Box::new(move |u: &str| { + rec.borrow_mut().push(u.to_string()); + Ok(()) + })); + let ctx = egui::Context::default(); + for (id, _, url) in ALL { + let r = crate::menus::invoke(&mut app, &ctx, id, serde_json::json!({})).unwrap(); + assert_eq!(r["url"], url, "{id}"); + assert_eq!(opened.borrow().last().map(String::as_str), Some(url), "{id}"); + } + open(&mut app, &ctx, DISCORD); + assert_eq!(opened.borrow().last().map(String::as_str), Some(DISCORD)); + assert_eq!(opened.borrow().len(), ALL.len() + 1); + + // A failing opener falls back to `ctx.open_url` instead of losing the click. + app.hooks.open_url = Some(Box::new(|_: &str| Err("no browser".into()))); + open(&mut app, &ctx, ISSUES); + let fallback: Vec = + ctx.output(|o| o.commands.iter().filter_map(|c| if let egui::OutputCommand::OpenUrl(u) = c { Some(u.url.clone()) } else { None }).collect()); + assert_eq!(fallback, [ISSUES]); + } } diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 86314ed27..e9807536a 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -405,7 +405,7 @@ pub fn invoke(app: &mut FilmcraftApp, ctx: &egui::Context, id: &str, mut params: } id if crate::links::url_for(id).is_some() => { let url = crate::links::url_for(id).unwrap_or_default(); - crate::links::open(ctx, url); + crate::links::open(app, ctx, url); app.ui.status = tlf!("Opened {url}", url); return Ok(json!({"url": url})); } @@ -483,6 +483,15 @@ pub fn invoke(app: &mut FilmcraftApp, ctx: &egui::Context, id: &str, mut params: crate::panels::color_dialogs::open_sequence(app); return Ok(json!({"dialog": "sequenceColor"})); } + // New Bin from menus, shortcuts and the Project panel asks for the name (inline rename); + // agents pass `name` to name the bin directly + "file.newBin" if params.get("name").is_none() => { + let r = crate::panels::project::new_bin(app, ¶ms); + if let Err(e) = &r { + app.ui.status = e.clone(); + } + return r; + } // From menus/shortcuts (no params) these ask first; agents pass params to act directly. "file.revert" if params.as_object().is_none_or(|m| m.is_empty()) && app.session.is_dirty() => { if app.session.path.is_none() { diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 8ee127502..40ffadc64 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -479,7 +479,7 @@ fn about_tab(app: &mut FilmcraftApp, ui: &mut egui::Ui) { fg, ); if resp.clicked() { - links::open(ui.ctx(), url); + links::open(app, ui.ctx(), url); } }; link(ui, "discord", Icon::Chat, tl!("Join the ArtCraft Discord"), links::DISCORD, true); diff --git a/crates/ui-egui/src/panels/effect_controls.rs b/crates/ui-egui/src/panels/effect_controls.rs index e6161e368..6b071c4ca 100644 --- a/crates/ui-egui/src/panels/effect_controls.rs +++ b/crates/ui-egui/src/panels/effect_controls.rs @@ -13,6 +13,10 @@ use crate::state::KeyframeRef; use crate::theme::Tokens; const ROW_H: f32 = 22.0; +/// The narrowest the effect list and the keyframe area can be dragged (#643): the list's rows are +/// laid out for at least 260 points (values from 150 points in, the reset button at the end). +const MIN_LIST_W: f32 = 260.0; +const MIN_LANE_W: f32 = 60.0; /// Keyframe lane header: the time ruler (beside the Source / Sequence pills) and the clip's bar. const RULER_H: f32 = 24.0; const CLIP_BAR_H: f32 = 18.0; @@ -99,7 +103,13 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { crate::dock::placeholder(ui, rect, &t, tl!("(no sequences)")); return; }; - let split = rect.min.x + (rect.width() * 0.58).max(260.0).min(rect.width() - 60.0); + // the divider between the effect list and the keyframe area; dragging it sets the list's width + // (#643), each side keeping a minimum width + // (the keyframe area has 4 points of margin left of it and 6 right) + let max_list = (rect.width() - MIN_LANE_W - 10.0).max(MIN_LIST_W); + let list_w = if app.ui.effect_controls_split > 0.0 { app.ui.effect_controls_split } else { (rect.width() * 0.58).max(260.0) }; + let list_w = list_w.clamp(MIN_LIST_W, max_list); + let split = rect.min.x + list_w; let head = Rect::from_min_size(rect.min + vec2(8.0, 4.0), vec2(split - rect.min.x - 12.0, 24.0)); // Premiere: two pill tabs — "Source · clip" and "Sequence · clip" (active) let pill = |ui: &mut egui::Ui, r: Rect, text: &str, active: bool| { @@ -122,26 +132,15 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { let lx = |tk: Tick| -> f32 { lane.min.x + (((tk - it.start).0 as f64 / dur) as f32).clamp(0.0, 1.0) * lane.width() }; let rate = seq.settings.frame_rate; let ruler = Rect::from_min_max(lane.min, pos2(lane.max.x, lane.min.y + RULER_H)); - paint_ruler(ui.painter(), ruler, &it, rate, seq.settings.drop_frame, &t); let bar = Rect::from_min_max(pos2(lane.min.x, ruler.max.y + 4.0), pos2(lane.max.x, ruler.max.y + 4.0 + CLIP_BAR_H)); - ui.painter().rect_filled(bar.shrink2(vec2(0.0, 2.0)), 2.0, t.clip_bar_bg); - ui.painter().with_clip_rect(bar).text(pos2(bar.min.x + 4.0, bar.center().y), Align2::LEFT_CENTER, &it.name, Tokens::ui(10.0), t.text); - // click or drag anywhere on the ruler (or the bar) to move the playhead let scrub = Rect::from_min_max(ruler.min, bar.max); - let sresp = ui.interact(scrub, egui::Id::new(("ec-scrub", clip.0)), Sense::click_and_drag()); app.auto.add("effectControls.lane", lane, "keyframe lane"); app.auto.add("effectControls.ruler", scrub, "time ruler"); - if (sresp.dragged() || sresp.clicked()) - && let Some(pos) = sresp.interact_pointer_pos() - { - let f = ((pos.x - lane.min.x) / lane.width()).clamp(0.0, 1.0) as f64; - let tk = rate.snap_nearest(it.start + Tick((f * it.duration.0 as f64) as i64)); - app.stop(); - app.session.set_playhead(tk); - } let body = Rect::from_min_max(pos2(rect.min.x, head.max.y + 4.0), pos2(split, rect.max.y - 26.0)); let mut actions: Vec<(String, Value)> = Vec::new(); - let mut bui = ui.new_child(egui::UiBuilder::new().max_rect(body).id_salt("ec-body")); + // the rows span the list and the keyframe area: the wheel scrolls anywhere over them and the + // scrollbar sits at the panel's right edge (#643) + let mut bui = ui.new_child(egui::UiBuilder::new().max_rect(Rect::from_min_max(body.min, pos2(lane.max.x, body.max.y))).id_salt("ec-body")); bui.set_clip_rect(Rect::from_min_max(body.min, pos2(rect.max.x, body.max.y))); let scroll_out = egui::ScrollArea::vertical().id_salt("ec-scroll").auto_shrink([false, false]).show(&mut bui, |bui| { for (clip, it, kind) in &clips { @@ -162,6 +161,7 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { if resp.hovered() { bui.painter().rect_filled(r, 0.0, t.hover); } + row_line(bui, r, &lane, &t); icons::paint( bui.painter(), Rect::from_center_size(pos2(r.min.x + 10.0, r.center().y), vec2(10.0, 10.0)), @@ -178,7 +178,8 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { if fxresp.clicked() { actions.push(("effects.toggleEnabled".into(), json!({"clip": clip.0, "index": idx}))); } - bui.painter().text(pos2(r.min.x + 42.0, r.center().y), Align2::LEFT_CENTER, crate::i18n::t(def.name), Tokens::ui(12.0), t.text); + // (the reset button sits at the row's right end) + row_label(bui, pos2(r.min.x + 42.0, r.center().y), crate::i18n::t(def.name), r.max.x - 26.0 - (r.min.x + 42.0), Tokens::ui(12.0), t.text); // reset button let rr = Rect::from_center_size(pos2(r.max.x - 14.0, r.center().y), vec2(16.0, 16.0)); let rresp = bui.interact(rr, egui::Id::new(("fxreset", clip.0, idx)), Sense::click()).on_hover_text(tl!("Reset Effect")); @@ -215,7 +216,7 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { continue; } if crate::panels::audio_fx_editor::has_editor(&e.effect) { - custom_setup_row(app, bui, body, clip, idx, &e.effect); + custom_setup_row(app, bui, body, &lane, clip, idx, &e.effect); } for pd in &def.params { param_row(app, bui, body, clip, idx, e, None, pd, mt_now, &mut actions, &lane, &lx, it); @@ -234,6 +235,32 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { } }); let _ = scroll_out; + // the ruler and the clip's bar, painted after the rows so rows scrolled up under them (their + // lines and keyframes) stay hidden + ui.painter().rect_filled(Rect::from_min_max(lane.min, pos2(lane.max.x, bar.max.y)), 0.0, t.tl_bg); + paint_ruler(ui.painter(), ruler, &it, rate, seq.settings.drop_frame, &t); + ui.painter().rect_filled(bar.shrink2(vec2(0.0, 2.0)), 2.0, t.clip_bar_bg); + ui.painter().with_clip_rect(bar).text(pos2(bar.min.x + 4.0, bar.center().y), Align2::LEFT_CENTER, &it.name, Tokens::ui(10.0), t.text); + // click or drag anywhere on the ruler (or the bar) to move the playhead (after the rows, which + // reach under it, so it gets the pointer first) + let sresp = ui.interact(scrub, egui::Id::new(("ec-scrub", clip.0)), Sense::click_and_drag()); + if (sresp.dragged() || sresp.clicked()) + && let Some(pos) = sresp.interact_pointer_pos() + { + let f = ((pos.x - lane.min.x) / lane.width()).clamp(0.0, 1.0) as f64; + let tk = rate.snap_nearest(it.start + Tick((f * it.duration.0 as f64) as i64)); + app.stop(); + app.session.set_playhead(tk); + } + // the divider: a line the height of the panel, a resize cursor and a wider grab area + let divider = Rect::from_min_max(pos2(split - 2.0, rect.min.y), pos2(split + 4.0, rect.max.y - 26.0)); + let dresp = ui.interact(divider, egui::Id::new("ec-divider"), Sense::drag()).on_hover_cursor(egui::CursorIcon::ResizeHorizontal); + if dresp.dragged() { + app.ui.effect_controls_split = (list_w + dresp.drag_delta().x).clamp(MIN_LIST_W, max_list); + } + let dcol = if dresp.hovered() || dresp.dragged() { t.accent } else { t.separator }; + ui.painter().line_segment([pos2(split + 1.5, rect.min.y), pos2(split + 1.5, rect.max.y - 26.0)], Stroke::new(1.0, dcol)); + app.auto.add("effectControls.divider", divider, "divider"); // playhead: a handle on the ruler and a line down the lane, while it is on the clip if ph >= it.start && ph <= it.end() { let px = lx(ph); @@ -249,10 +276,37 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { run(app, ui.ctx(), actions); } +/// The thin line under a row, across the names, the values and the keyframe lane, so a row can be +/// followed across the panel, as in Premiere (#640). It runs through the middle of the spacing +/// between rows, so each row sits centred between its two lines. +pub(crate) fn row_line(ui: &egui::Ui, r: Rect, lane: &Rect, t: &Tokens) { + let y = r.max.y + ui.spacing().item_spacing.y / 2.0; + ui.painter().line_segment([pos2(r.min.x, y), pos2(lane.max.x, y)], Stroke::new(1.0, t.separator)); +} + +/// A row's label, cut off with "…" where it would run into what follows it, as Premiere does when +/// the effect list is narrow (#643). +pub(crate) fn row_label(ui: &egui::Ui, left_center: Pos2, text: &str, max_w: f32, font: egui::FontId, color: Color32) { + let mut job = egui::text::LayoutJob::single_section(text.to_owned(), egui::TextFormat::simple(font, color)); + job.wrap.max_width = max_w.max(0.0); + job.wrap.max_rows = 1; + job.wrap.break_anywhere = true; + job.wrap.overflow_character = Some('…'); + let g = ui.fonts_mut(|f| f.layout_job(job)); + ui.painter().galley(pos2(left_center.x, left_center.y - g.size().y / 2.0), g, color); +} + +/// Make a dropdown or a checkbox in a row fit inside the row, centred between its row lines. +pub(crate) fn fit_to_row(ui: &mut egui::Ui) { + ui.spacing_mut().interact_size.y = ROW_H - 4.0; + ui.spacing_mut().button_padding.y = 1.0; +} + /// Premiere's "Custom Setup ▸ Edit…" row: opens the effect's Clip Fx Editor window. -fn custom_setup_row(app: &mut FilmcraftApp, ui: &mut egui::Ui, body: Rect, clip: ClipId, idx: usize, effect: &str) { +fn custom_setup_row(app: &mut FilmcraftApp, ui: &mut egui::Ui, body: Rect, lane: &Rect, clip: ClipId, idx: usize, effect: &str) { let t = app.tokens; let (r, _) = ui.allocate_exact_size(vec2(body.width(), ROW_H), Sense::hover()); + row_line(ui, r, lane, &t); ui.painter().text(pos2(r.min.x + 42.0, r.center().y), Align2::LEFT_CENTER, tl!("Custom Setup"), Tokens::ui(12.0), t.text_dim); let br = Rect::from_min_size(pos2(r.min.x + 160.0, r.min.y + 2.0), vec2(60.0, ROW_H - 4.0)); let resp = ui.interact(br, egui::Id::new(("fx-custom-setup", clip.0, idx)), Sense::click()); @@ -342,10 +396,11 @@ fn keyframe_nav( for (d, target, name, tip) in [(-1.0, param.prev_keyframe(mt), "prevKeyframe", "Go to previous keyframe"), (1.0, param.next_keyframe(mt), "nextKeyframe", "Go to next keyframe")] { - let r = Rect::from_center_size(at + vec2(d * NAV_STEP, 0.0), vec2(12.0, 14.0)); + // the arrows almost the row's height to click, as Premiere's + let r = Rect::from_center_size(at + vec2(d * NAV_STEP, 0.0), vec2(14.0, 20.0)); let resp = ui.interact(r, id.with(name), Sense::click()).on_hover_text(crate::i18n::t(tip)); let c = r.center(); - let pts = vec![c + vec2(-3.0 * d, -4.0), c + vec2(-3.0 * d, 4.0), c + vec2(3.0 * d, 0.0)]; + let pts = vec![c + vec2(-3.5 * d, -5.5), c + vec2(-3.5 * d, 5.5), c + vec2(3.5 * d, 0.0)]; ui.painter().add(egui::Shape::convex_polygon(pts, if target.is_some() { t.text } else { t.text_faint }, Stroke::NONE)); app.auto.add(&format!("{auto}.{name}"), r, tip); if resp.clicked() @@ -355,12 +410,12 @@ fn keyframe_nav( } } } - let r = Rect::from_center_size(at, vec2(13.0, 13.0)); + let r = Rect::from_center_size(at, vec2(15.0, 15.0)); let resp = ui.interact(r.expand(1.5), id.with("key"), Sense::click()).on_hover_text(crate::i18n::t(tip)); if param.is_some_and(|p| p.keyframes.iter().any(|k| k.time == mt)) { icons::paint(ui.painter(), r, Icon::Keyframe, t.hot_text); } else { - let s = 4.0; + let s = 4.7; let outline = vec![at + vec2(0.0, -s), at + vec2(s, 0.0), at + vec2(0.0, s), at + vec2(-s, 0.0)]; ui.painter().add(egui::Shape::closed_line(outline, Stroke::new(1.2, if resp.hovered() { t.text } else { t.text_dim }))); } @@ -436,6 +491,7 @@ pub(crate) fn param_row( v }; let (r, _) = ui.allocate_exact_size(vec2(body.width(), ROW_H), Sense::hover()); + row_line(ui, r, lane, &t); let mut x = r.min.x + 26.0; // twirl-down for the value/velocity graphs (animated scalar and point params) if param.is_animated() && graphable(¶m.value) { @@ -463,8 +519,8 @@ pub(crate) fn param_row( app.auto.add(&format!("effectControls.{}.{}.stopwatch", e.effect, pkey), sw, "Toggle animation"); } x += 14.0; - ui.painter().text(pos2(x, r.center().y), Align2::LEFT_CENTER, crate::i18n::t(pd.label), Tokens::ui(12.0), t.text); let vx = r.min.x + (r.width() * 0.5).max(150.0); + row_label(ui, pos2(x, r.center().y), crate::i18n::t(pd.label), vx - 6.0 - x, Tokens::ui(12.0), t.text); let value = param.value_at(mt); let mut vui = ui.new_child( egui::UiBuilder::new() @@ -496,13 +552,17 @@ pub(crate) fn param_row( } (ParamKind::Bool, ParamValue::Bool(b)) => { let mut v = *b; + fit_to_row(&mut vui); if vui.checkbox(&mut v, "").changed() { set = Some(json!(v)); } } (ParamKind::Choice(opts), ParamValue::Choice(c)) => { let mut sel = *c as usize; - egui::ComboBox::from_id_salt(id).selected_text(opts.get(sel).map_or("", |o| crate::i18n::t(o))).width(130.0).show_ui(&mut vui, |ui| { + fit_to_row(&mut vui); + // (narrower when the effect list is narrow, so it stays left of the divider) + let w = vui.available_width().min(130.0); + egui::ComboBox::from_id_salt(id).selected_text(opts.get(sel).map_or("", |o| crate::i18n::t(o))).width(w).show_ui(&mut vui, |ui| { for (i, o) in opts.iter().enumerate() { if ui.selectable_value(&mut sel, i, crate::i18n::t(o)).changed() { set = Some(json!(i)); @@ -559,6 +619,17 @@ pub(crate) fn param_row( actions.push(("effects.addKeyframe".into(), with_mask(json!({"clip": clip.0, "effect": eff_json, "param": pd.id})))); } } + // reset to the default, under the effect's own reset button (an animated parameter keeps its + // keyframes and gets one at the playhead) + if mask.is_none() { + let rr = Rect::from_center_size(pos2(r.max.x - 14.0, r.center().y), vec2(12.0, 12.0)); + let rresp = ui.interact(rr, egui::Id::new(("param-reset", clip.0, idx, pkey)), Sense::click()).on_hover_text(tl!("Reset Parameter")); + icons::paint(ui.painter(), rr, Icon::Reset, if rresp.hovered() { t.text } else { t.text_dim }); + app.auto.add(&format!("effectControls.{}.{}.reset", e.effect, pkey), rr, "Reset Parameter"); + if rresp.clicked() { + actions.push(("effects.resetParam".into(), json!({"clip": clip.0, "effect": eff_json, "param": pd.id}))); + } + } // keyframes in the lane: draggable diamonds; right-click for interpolation if param.is_animated() { let y = r.center().y; @@ -573,7 +644,8 @@ pub(crate) fn param_row( let id = egui::Id::new(("kf", clip.0, idx, pkey, k.time.0)); let drag_off: Option = ui.data(|d| d.get_temp(id)); let kx = lane.min.x + f * lane.width() + drag_off.unwrap_or(0.0); - let kr = Rect::from_center_size(pos2(kx, y), vec2(11.0, 11.0)); + // about Premiere's size: a diamond ~9 points wide + let kr = Rect::from_center_size(pos2(kx, y), vec2(15.0, 15.0)); let resp = ui.interact(kr.expand(2.0), id, Sense::click_and_drag()); app.auto.add(&format!("effectControls.{}.{}.keyframe.{}", e.effect, pkey, k.time.0), kr, "keyframe"); // highlighted when selected (not merely under the playhead: that lit up every @@ -586,11 +658,11 @@ pub(crate) fn param_row( let col = if sel { t.hot_text } else { Color32::from_rgb(0xb0, 0xb0, 0xb0) }; match k.interp { filmcraft_project::Interpolation::Hold => { - ui.painter().rect_filled(Rect::from_center_size(kr.center(), vec2(8.0, 8.0)), 0.0, col); + ui.painter().rect_filled(Rect::from_center_size(kr.center(), vec2(9.0, 9.0)), 0.0, col); } filmcraft_project::Interpolation::Linear => icons::paint(ui.painter(), kr, Icon::Keyframe, col), _ => { - ui.painter().circle_filled(kr.center(), 4.5, col); + ui.painter().circle_filled(kr.center(), 5.5, col); } } if resp.dragged() { @@ -952,6 +1024,7 @@ pub(crate) fn graph_rows( let t = app.tokens; let (vr, _) = ui.allocate_exact_size(vec2(body.width(), 110.0), Sense::hover()); let (velr, _) = ui.allocate_exact_size(vec2(body.width(), 64.0), Sense::hover()); + row_line(ui, velr, lane, &t); let speed = it.speed.abs().max(1e-6); let dur = it.duration.0.max(1) as f64; let to_media = |f: f64| it.source_in + Tick((f * dur * speed) as i64); diff --git a/crates/ui-egui/src/panels/effects.rs b/crates/ui-egui/src/panels/effects.rs index 76f4f8c28..df3e7d939 100644 --- a/crates/ui-egui/src/panels/effects.rs +++ b/crates/ui-egui/src/panels/effects.rs @@ -3,7 +3,8 @@ //! //! Lumetri Presets: each preset row has a thumbnail (our Lumetri on the procedural preview //! picture, `filmcraft_render::lumetri_presets`); double-click (or right-click ▸ Apply to Selected -//! Clips, `effects.presetMenu.apply`) applies it (`lumetri.applyPreset`). +//! Clips, `effects.presetMenu.apply`) applies it (`lumetri.applyPreset`), and dragging it onto a +//! Timeline clip applies it to that clip (rows and grid cells alike). //! When the panel is wide (maximized), clicking a Lumetri Presets folder shows its presets as a //! thumbnail grid to the right of the tree. Automation ids: `effects.lumetriPreset.`, //! `effects.presetGrid` (the grid area), `effects.presetGrid.`. @@ -232,7 +233,7 @@ fn lumetri_rows(app: &mut FilmcraftApp, ui: &mut egui::Ui, filter: &str) -> Opti continue; } for p in items { - let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), 24.0), Sense::click()); + let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), 24.0), Sense::click_and_drag()); if resp.hovered() { ui.painter().rect_filled(r, 0.0, t.hover); } @@ -242,6 +243,9 @@ fn lumetri_rows(app: &mut FilmcraftApp, ui: &mut egui::Ui, filter: &str) -> Opti ui.painter().text(pos2(tr.max.x + 8.0, r.center().y), Align2::LEFT_CENTER, crate::i18n::t(p.name), Tokens::ui(12.0), t.text); app.auto.add(&format!("effects.lumetriPreset.{}", p.name), r, p.name); let resp = resp.on_hover_text(crate::i18n::t(p.description)); + if resp.drag_started() { + crate::panels::start_drag_lumetri_preset(ui, p.name); + } if apply_menu(app, &resp) { apply = Some(p.name.to_string()); } @@ -273,12 +277,15 @@ fn preset_grid(app: &mut FilmcraftApp, ui: &mut egui::Ui, area: Rect) -> Option< } let pic = Rect::from_min_size(cell.min, vec2(cw, ch)); let tex = preset_texture(app, ui.ctx(), p, 160); - let resp = ui.interact(cell, egui::Id::new(("lumetri-grid", p.name)), Sense::click()).on_hover_text(crate::i18n::t(p.description)); + let resp = ui.interact(cell, egui::Id::new(("lumetri-grid", p.name)), Sense::click_and_drag()).on_hover_text(crate::i18n::t(p.description)); ui.painter().rect_filled(cell, 3.0, if resp.hovered() { t.hover } else { t.field_bg }); ui.painter().image(tex, pic.shrink(2.0), Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), Color32::WHITE); let label = ui.painter().layout(crate::i18n::t(p.name).to_string(), Tokens::ui(11.0), t.text, cw - 6.0); ui.painter().galley(pos2(cell.min.x + 4.0, pic.max.y + 2.0), label, t.text); app.auto.add(&format!("effects.presetGrid.{}", p.name), cell, p.name); + if resp.drag_started() { + crate::panels::start_drag_lumetri_preset(ui, p.name); + } if apply_menu(app, &resp) { apply = Some(p.name.to_string()); } diff --git a/crates/ui-egui/src/panels/export_mode.rs b/crates/ui-egui/src/panels/export_mode.rs index e2298ec97..2e6c8e360 100644 --- a/crates/ui-egui/src/panels/export_mode.rs +++ b/crates/ui-egui/src/panels/export_mode.rs @@ -606,7 +606,7 @@ fn video_section(ui: &mut egui::Ui, reg: &mut Reg, s: &mut ExportSettings, t: &T } }); } - if matches!(s.format, Format::PngSequence | Format::TiffSequence) { + if s.supports_alpha() { row(ui, t, tl!("Alpha"), |ui| { check(ui, reg, "export.video.alpha", &mut s.alpha, tl!("Include Alpha Channel")); }); @@ -708,8 +708,9 @@ fn video_section(ui: &mut egui::Ui, reg: &mut Reg, s: &mut ExportSettings, t: &T } Format::ProRes => { row(ui, t, tl!("Profile"), |ui| { - let keys = ["proxy", "lt", "standard", "hq"]; - let labels = ["Apple ProRes 422 Proxy", "Apple ProRes 422 LT", "Apple ProRes 422", "Apple ProRes 422 HQ"]; + let keys = ["proxy", "lt", "standard", "hq", "4444", "4444xq"]; + let labels = + ["Apple ProRes 422 Proxy", "Apple ProRes 422 LT", "Apple ProRes 422", "Apple ProRes 422 HQ", "Apple ProRes 4444", "Apple ProRes 4444 XQ"]; let cur = keys.iter().position(|k| *k == s.prores_profile).unwrap_or(3); if let Some(i) = combo(ui, reg, "export.video.proresProfile", labels[cur], &opts(&labels), 220.0) { s.prores_profile = keys[i].into(); diff --git a/crates/ui-egui/src/panels/import_mode.rs b/crates/ui-egui/src/panels/import_mode.rs index 5d78e93e6..ef68c0aab 100644 --- a/crates/ui-egui/src/panels/import_mode.rs +++ b/crates/ui-egui/src/panels/import_mode.rs @@ -67,7 +67,7 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { crate::icons::paint(ui.painter(), Rect::from_center_size(pos2(r.min.x + 20.0, r.center().y), vec2(16.0, 16.0)), icon, fg); ui.painter().galley(pos2(r.min.x + 34.0, r.center().y - g.size().y / 2.0), g, fg); if resp.clicked() { - crate::links::open(&ctx, url); + crate::links::open(app, &ctx, url); } x = r.max.x + 12.0; } diff --git a/crates/ui-egui/src/panels/keyboard.rs b/crates/ui-egui/src/panels/keyboard.rs index 674769d6f..dfc960aad 100644 --- a/crates/ui-egui/src/panels/keyboard.rs +++ b/crates/ui-egui/src/panels/keyboard.rs @@ -210,7 +210,7 @@ pub fn route(app: &mut FilmcraftApp, ctx: &egui::Context, id: &str, params: &Val } "graphics.beginTextEditing" => begin_text_editing(app), "help.filmcraftHelp" => { - crate::links::open(ctx, HELP_URL); + crate::links::open(app, ctx, HELP_URL); Ok(json!({"url": HELP_URL})) } "app.quit" => { diff --git a/crates/ui-egui/src/panels/masks.rs b/crates/ui-egui/src/panels/masks.rs index 4ffba906a..5062918ea 100644 --- a/crates/ui-egui/src/panels/masks.rs +++ b/crates/ui-egui/src/panels/masks.rs @@ -45,6 +45,7 @@ pub fn effect_rows( let t = app.tokens; // creation tools: ellipse, 4-point polygon, free-draw Bézier let (r, _) = ui.allocate_exact_size(vec2(body.width(), ROW_H), Sense::hover()); + crate::panels::effect_controls::row_line(ui, r, lane, &t); let pen_on = app.ui.mask_pen.as_ref().is_some_and(|p| p.clip == clip.0 && p.effect == idx); for (i, (icon, kind, tip)) in [ (Icon::Ellipse, "ellipse", tl!("Create ellipse mask")), @@ -85,6 +86,7 @@ pub fn effect_rows( } else if resp.hovered() { ui.painter().rect_filled(r, 0.0, t.hover); } + crate::panels::effect_controls::row_line(ui, r, lane, &t); let tw = Rect::from_center_size(pos2(r.min.x + 26.0, r.center().y), vec2(10.0, 10.0)); icons::paint(ui.painter(), tw, if open { Icon::ChevronDown } else { Icon::ChevronRight }, t.text_dim); let twresp = ui.interact(tw.expand(3.0), egui::Id::new(("mask-twirl", clip.0, idx, k)), Sense::click()); @@ -99,13 +101,22 @@ pub fn effect_rows( Icon::Pen }; icons::paint(ui.painter(), glyph, gi, if selected { t.accent } else { t.text_dim }); - ui.painter().text(pos2(r.min.x + 54.0, r.center().y), Align2::LEFT_CENTER, &m.name, Tokens::ui(12.0), t.text); + // (the mode dropdown starts 118 points from the row's right end) + crate::panels::effect_controls::row_label( + ui, + pos2(r.min.x + 54.0, r.center().y), + &m.name, + r.max.x - 122.0 - (r.min.x + 54.0), + Tokens::ui(12.0), + t.text, + ); let base = format!("effectControls.{}.mask{k}", e.effect); app.auto.add(&base, r, &m.name); // mode let mr = Rect::from_min_size(pos2(r.max.x - 118.0, r.min.y + 2.0), vec2(96.0, ROW_H - 4.0)); let mut mui = ui.new_child(egui::UiBuilder::new().max_rect(mr).layout(egui::Layout::left_to_right(egui::Align::Center))); let mut mode = m.mode; + crate::panels::effect_controls::fit_to_row(&mut mui); egui::ComboBox::from_id_salt(("mask-mode", clip.0, idx, k)).selected_text(crate::i18n::t(mode.label())).width(88.0).show_ui(&mut mui, |ui| { for md in MaskMode::ALL { if ui.selectable_value(&mut mode, md, crate::i18n::t(md.label())).changed() { @@ -148,9 +159,11 @@ pub fn effect_rows( } // Inverted let (r, _) = ui.allocate_exact_size(vec2(body.width(), ROW_H), Sense::hover()); + crate::panels::effect_controls::row_line(ui, r, lane, &t); ui.painter().text(pos2(r.min.x + 40.0, r.center().y), Align2::LEFT_CENTER, tl!("Inverted"), Tokens::ui(12.0), t.text); let cr = Rect::from_min_size(pos2(r.min.x + (r.width() * 0.5).max(150.0), r.min.y + 2.0), vec2(20.0, ROW_H - 4.0)); - let mut cui = ui.new_child(egui::UiBuilder::new().max_rect(cr)); + let mut cui = ui.new_child(egui::UiBuilder::new().max_rect(cr).layout(egui::Layout::left_to_right(egui::Align::Center))); + crate::panels::effect_controls::fit_to_row(&mut cui); let mut inv = m.inverted; if cui.checkbox(&mut inv, "").changed() { actions.push(("masks.set".into(), json!({"clip": clip.0, "effect": idx, "mask": k, "inverted": inv}))); diff --git a/crates/ui-egui/src/panels/mod.rs b/crates/ui-egui/src/panels/mod.rs index bcb386fbb..3f6f9a79f 100644 --- a/crates/ui-egui/src/panels/mod.rs +++ b/crates/ui-egui/src/panels/mod.rs @@ -29,6 +29,7 @@ pub mod mixer; pub mod monitor; pub mod monitor_view; pub mod multicam; +pub mod notes; pub mod panel_state; pub mod presets; pub mod project; @@ -101,6 +102,7 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, p: PanelKind, rect: Rect) PanelKind::Text => text::show(app, ui, rect), PanelKind::EssentialSound => essential_sound::show(app, ui, rect), PanelKind::TextToSpeech => tts::show(app, ui, rect), + PanelKind::ProjectNotes => notes::show(app, ui, rect), other => crate::dock::placeholder(ui, rect, &app.tokens, &tlf!("{panel} — coming in a later milestone", panel = crate::i18n::t(other.title()))), } } @@ -111,6 +113,8 @@ pub mod source_range; #[derive(Clone, Debug)] enum DragPayload { Item(ItemId), + /// A bin (by id) dragged in the Project panel. + Bin(u64), Source(source_drag::SourceDrag), Effect(String), /// A graphics template (id, name) from Essential Graphics ▸ Browse. @@ -133,6 +137,15 @@ pub fn dragged_source(ui: &egui::Ui) -> Option { pub fn start_drag_item(ui: &egui::Ui, item: ItemId) { ui.ctx().data_mut(|d| d.insert_temp(payload_id(), Some(DragPayloadBox(DragPayload::Item(item))))); } +pub fn start_drag_bin(ui: &egui::Ui, bin: u64) { + ui.ctx().data_mut(|d| d.insert_temp(payload_id(), Some(DragPayloadBox(DragPayload::Bin(bin))))); +} +pub fn dragged_bin(ui: &egui::Ui) -> Option { + match payload(ui) { + Some(DragPayload::Bin(b)) => Some(b), + _ => None, + } +} pub fn start_drag_template(ui: &egui::Ui, id: &str, name: &str) { ui.ctx().data_mut(|d| d.insert_temp(payload_id(), Some(DragPayloadBox(DragPayload::Template(id.to_string(), name.to_string()))))); } @@ -145,6 +158,13 @@ pub fn dragged_template(ui: &egui::Ui) -> Option { pub fn start_drag_effect(ui: &egui::Ui, id: &str) { ui.ctx().data_mut(|d| d.insert_temp(payload_id(), Some(DragPayloadBox(DragPayload::Effect(id.to_string()))))); } +/// Prefix of a dragged effect payload that names a Lumetri preset rather than an effect id. +pub const LUMETRI_PRESET_PREFIX: &str = "lumetri:"; +/// A Lumetri preset (Effects ▸ Lumetri Presets) dragged onto a clip: an effect payload +/// `lumetri:` the Timeline applies with `lumetri.applyPreset`. +pub fn start_drag_lumetri_preset(ui: &egui::Ui, name: &str) { + start_drag_effect(ui, &format!("{LUMETRI_PRESET_PREFIX}{name}")); +} #[derive(Clone, Debug)] struct DragPayloadBox(DragPayload); @@ -175,6 +195,7 @@ pub fn drag_ghost(app: &FilmcraftApp, ui: &egui::Ui) { if let Some(p) = ctx.pointer_hover_pos() { let label = match &pl { DragPayload::Item(i) => app.session.project.item(*i).map(|x| x.name.clone()).unwrap_or_default(), + DragPayload::Bin(b) => app.session.project.root.find_bin(filmcraft_project::BinId(*b)).map(|x| x.name.clone()).unwrap_or_default(), DragPayload::Source(s) => { let name = app.session.project.item(s.item).map(|i| i.name.as_str()).unwrap_or("Source"); let mode = match (s.video, s.audio) { @@ -185,9 +206,10 @@ pub fn drag_ghost(app: &FilmcraftApp, ui: &egui::Ui) { format!("{name} · {mode} · {:.2} s", s.range.duration.seconds()) } DragPayload::Template(_, name) => name.clone(), - DragPayload::Effect(e) => match e.strip_prefix("preset:") { - Some(name) => name.to_string(), - None => filmcraft_project::find_effect(e).map(|d| d.name.to_string()).unwrap_or_default(), + DragPayload::Effect(e) => match (e.strip_prefix("preset:"), e.strip_prefix(LUMETRI_PRESET_PREFIX)) { + (Some(name), _) => name.to_string(), + (_, Some(name)) => crate::i18n::t(name).to_string(), + _ => filmcraft_project::find_effect(e).map(|d| d.name.to_string()).unwrap_or_default(), }, }; let painter = ctx.layer_painter(egui::LayerId::new(egui::Order::Tooltip, egui::Id::new("drag-ghost"))); diff --git a/crates/ui-egui/src/panels/notes.rs b/crates/ui-egui/src/panels/notes.rs new file mode 100644 index 000000000..740db1f46 --- /dev/null +++ b/crates/ui-egui/src/panels/notes.rs @@ -0,0 +1,53 @@ +//! Project Notes panel (#620): free plain text kept in the project file, for to-dos, +//! instructions for whoever opens the project next, frame numbers and the like. Every change is +//! a `project.setNotes`; one typing session (from focusing the editor until it loses focus) is +//! one undo step. +//! +//! Automation ids: `notes.text` (the editor; its label is the start of the notes). Agents read +//! and write the notes with `project.notes` / `project.setNotes`. + +use egui::{Rect, vec2}; +use serde_json::json; + +use crate::FilmcraftApp; + +/// Longest automation label (characters): the full notes are `project.notes`. +const LABEL_CHARS: usize = 200; + +pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { + let t = app.tokens; + let mut child = ui.new_child(egui::UiBuilder::new().max_rect(rect.shrink2(vec2(8.0, 6.0)))); + let ui = &mut child; + let mut text = app.session.project.notes.clone(); + let height = ui.available_height(); + let r = egui::ScrollArea::vertical() + .id_salt("project-notes-scroll") + .auto_shrink([false, false]) + .show(ui, |ui| { + ui.add( + egui::TextEdit::multiline(&mut text) + .id_salt("project-notes") + .hint_text(tl!("Notes for this project: to-dos, instructions, frame numbers… Saved with the project.")) + .text_color(t.text) + .desired_width(f32::INFINITY) + .min_size(vec2(0.0, height)), + ) + }) + .inner; + // each focus of the editor starts a new undo step; the edits while it keeps focus join it + let typing = egui::Id::new("project-notes-typing"); + if r.gained_focus() { + ui.data_mut(|d| { + let n = d.get_temp::(typing).unwrap_or(0).wrapping_add(1); + d.insert_temp(typing, n); + }); + } + if r.changed() { + let key = ui.data(|d| d.get_temp::(typing)).unwrap_or(0); + if let Err(e) = app.session.execute("project.setNotes", json!({"text": text, "merge": key.to_string()})) { + app.ui.status = e.to_string(); + } + } + let label: String = app.session.project.notes.chars().take(LABEL_CHARS).collect(); + app.auto.add("notes.text", r.rect, &label); +} diff --git a/crates/ui-egui/src/panels/project.rs b/crates/ui-egui/src/panels/project.rs index 551f5cce3..9aef769ed 100644 --- a/crates/ui-egui/src/panels/project.rs +++ b/crates/ui-egui/src/panels/project.rs @@ -622,7 +622,7 @@ fn footer(app: &mut FilmcraftApp, ui: &mut egui::Ui, bar: Rect, v: &View, action "find" => actions.push(("edit.find".into(), json!({}))), "file.newBin" => { let parent = (v.bin != app.session.project.root.id).then_some(v.bin.0); - actions.push(("file.newBin".into(), json!({"name": "New Bin", "parent": parent}))); + actions.push(("file.newBin".into(), json!({"parent": parent, "panel": pre}))); } "project.delete" => match crate::panels::project_views::clear_params(&app.session.state.project_selection, app.ui.project_panel.selected_bin) { Some(p) => actions.push((id.into(), p)), @@ -719,7 +719,7 @@ pub fn panel_menu(app: &mut FilmcraftApp, ui: &mut egui::Ui) -> bool { ui.separator(); if item(ui, "newBin", tl!("New Bin"), true, s_bin.as_deref()) { let parent = (v.bin != app.session.project.root.id).then_some(v.bin.0); - actions.push(("file.newBin".into(), json!({"name": "New Bin", "parent": parent}))); + actions.push(("file.newBin".into(), json!({"parent": parent}))); } if item(ui, "newBinFromSelection", tl!("New Bin From Selection"), !app.session.state.project_selection.is_empty(), Some("Shift+B")) { actions.push(("file.newBinFromSelection".into(), json!({}))); @@ -958,6 +958,32 @@ fn start_rename(app: &mut FilmcraftApp, params: &Value) -> Result Ok(json!({"item": item, "bin": bin})) } +/// New Bin from a menu, a shortcut or the panel's buttons (no `name`): make a "New Bin" in `parent` +/// (left out: the bin the Project panel shows; null: the project's root) and start renaming it, so +/// its name can be typed right away (#456). `panel` names the panel instance that edits the name. +/// Agents pass `name` to `file.newBin` to name the bin directly. +pub fn new_bin(app: &mut FilmcraftApp, params: &Value) -> Result { + let shown = view_of(app, shown_inst(app)); + let parent = match params.get("parent") { + None => (shown.bin != app.session.project.root.id).then_some(shown.bin.0), + Some(Value::Null) => None, + Some(p) => Some(p.as_u64().ok_or("`parent` must be a bin id")?), + }; + let r = app.session.execute("file.newBin", json!({"name": tl!("New Bin"), "parent": parent})).map_err(|e| e.to_string())?; + let bin = r.get("bin").and_then(Value::as_u64).ok_or("the new bin has no id")?; + let text = app.session.project.root.find_bin(BinId(bin)).map(|b| b.name.clone()).ok_or("no such bin")?; + // a bin made inside a collapsed bin of the list opens it, so the name field is visible + if let Some(p) = parent + && !app.ui.expanded_bins.contains(&p) + { + app.ui.expanded_bins.push(p); + } + let panel = params.get("panel").and_then(Value::as_str).map_or(shown.prefix, str::to_string); + app.ui.project_panel.selected_bin = Some(bin); + app.ui.project_panel.rename = Some(Rename { item: None, bin: Some(bin), text, panel }); + Ok(r) +} + /// Commit the inline rename (Enter / focus lost). pub fn commit_rename(app: &mut FilmcraftApp) { let Some(r) = app.ui.project_panel.rename.take() else { return }; diff --git a/crates/ui-egui/src/panels/project_views.rs b/crates/ui-egui/src/panels/project_views.rs index 46d800234..e48d98dd4 100644 --- a/crates/ui-egui/src/panels/project_views.rs +++ b/crates/ui-egui/src/panels/project_views.rs @@ -5,7 +5,7 @@ use egui::{Align2, Color32, Rect, Sense, Stroke, StrokeKind, pos2, vec2}; use filmcraft_engine::project_panel::{self as pp, Card, SortSpec, ViewMode}; -use filmcraft_project::{Bin, BinEntry, ItemId, ItemKind}; +use filmcraft_project::{Bin, BinEntry, BinId, ItemId, ItemKind}; use filmcraft_time::Tick; use serde_json::json; @@ -311,7 +311,7 @@ fn bin_menu(app: &mut FilmcraftApp, ui: &mut egui::Ui, v: &View, bin: u64, actio ui.close(); } if ui.button(tl!("New Bin")).clicked() { - actions.push(("file.newBin".into(), json!({"name": "New Bin", "parent": bin}))); + actions.push(("file.newBin".into(), json!({"parent": bin, "panel": v.prefix}))); ui.close(); } ui.separator(); @@ -331,21 +331,46 @@ pub fn clear_params(selection: &[filmcraft_project::ItemId], selected_bin: Optio } /// Drop project items dragged onto a bin: move them (the selection when the dragged item is in it). +/// A dragged bin nests in it, unless that would put the bin inside itself. fn accept_bin_drop(app: &FilmcraftApp, ui: &egui::Ui, r: Rect, bin: u64, actions: &mut Actions) -> bool { - let Some(item) = crate::panels::dragged_project_item(ui) else { return false }; + let items: Vec = if let Some(item) = crate::panels::dragged_project_item(ui) { + let sel = &app.session.state.project_selection; + if sel.contains(&item) { sel.iter().map(|i| i.0).collect() } else { vec![item.0] } + } else if let Some(dragged) = crate::panels::dragged_bin(ui) { + let root = &app.session.project.root; + if root.find_bin(BinId(dragged)).is_none_or(|d| d.find_bin(BinId(bin)).is_some()) { + return false; + } + vec![dragged] + } else { + return false; + }; if !ui.rect_contains_pointer(r) { return false; } ui.painter().rect_stroke(r, 3.0, Stroke::new(1.5, app.tokens.accent), StrokeKind::Inside); if ui.input(|i| i.pointer.any_released()) { - let sel = &app.session.state.project_selection; - let items: Vec = if sel.contains(&item) { sel.iter().map(|i| i.0).collect() } else { vec![item.0] }; actions.push(("project.moveToBin".into(), json!({"items": items, "bin": bin}))); crate::panels::clear_drag(ui); } true } +/// A bin dropped on empty space moves into the bin the view shows (out of the bin it was nested in). +fn accept_view_drop(app: &FilmcraftApp, ui: &egui::Ui, r: Rect, v: &View, actions: &mut Actions) { + let Some(dragged) = crate::panels::dragged_bin(ui) else { return }; + let root = &app.session.project.root; + // already shown here, or the view is inside the dragged bin + let here = root.find_bin(v.bin).is_some_and(|b| b.children.iter().any(|c| matches!(c, BinEntry::Bin(x) if x.id.0 == dragged))); + let inside = root.find_bin(BinId(dragged)).is_none_or(|d| d.find_bin(v.bin).is_some()); + if here || inside || !ui.rect_contains_pointer(r) || !ui.input(|i| i.pointer.any_released()) { + return; + } + let bin = (v.bin != root.id).then_some(v.bin.0); + actions.push(("project.moveToBin".into(), json!({"items": [dragged], "bin": bin}))); + crate::panels::clear_drag(ui); +} + /// The inline rename field at `r`; true while it is shown. fn rename_field(app: &mut FilmcraftApp, ui: &mut egui::Ui, r: Rect, item: Option, bin: Option, pre: &str) -> bool { let Some(rn) = app.ui.project_panel.rename.clone() else { return false }; @@ -521,6 +546,7 @@ fn empty_space(app: &mut FilmcraftApp, ui: &mut egui::Ui, v: &View, bg: &egui::R ui.allocate_rect(r, Sense::hover()); app.auto.add(&format!("{}.empty", v.prefix), r.intersect(ui.clip_rect()), "Empty area"); marquee(app, ui, v, bg, ui.max_rect().min, actions); + accept_view_drop(app, ui, r, v, actions); if bg.clicked() { app.ui.project_panel.selected_bin = None; actions.push(("project.select".into(), json!({"items": []}))); @@ -599,7 +625,7 @@ fn background_menu(app: &mut FilmcraftApp, ui: &mut egui::Ui, v: &View, bin: Opt (tl!("Import…"), "file.import"), ] { if ui.button(label).clicked() { - let p = if cmd == "file.newBin" { json!({"name": tl!("New Bin"), "parent": bin}) } else { json!({}) }; + let p = if cmd == "file.newBin" { json!({"parent": bin, "panel": v.prefix}) } else { json!({}) }; actions.push((cmd.into(), p)); ui.close(); } @@ -614,7 +640,7 @@ fn list_bin(app: &mut FilmcraftApp, ui: &mut egui::Ui, bin: &Bin, depth: usize, for bid in bins { let Some(b) = app.session.project.root.find_bin(bid).cloned() else { continue }; let open = app.ui.expanded_bins.contains(&b.id.0) || !filter.is_empty(); - let (r, resp) = ui.allocate_exact_size(vec2(lc.width, lc.row_h), Sense::click()); + let (r, resp) = ui.allocate_exact_size(vec2(lc.width, lc.row_h), Sense::click_and_drag()); let selected = app.ui.project_panel.selected_bin == Some(b.id.0); if selected { ui.painter().rect_filled(r, 0.0, t.row_selected); @@ -633,6 +659,9 @@ fn list_bin(app: &mut FilmcraftApp, ui: &mut egui::Ui, bin: &Bin, depth: usize, app.auto.add(&format!("{}.bin.{}", v.prefix, b.id.0), r.intersect(ui.clip_rect()), &b.name); app.auto.add(&format!("{}.bin.{}.toggle", v.prefix, b.id.0), tri, "Expand"); accept_bin_drop(app, ui, r, b.id.0, actions); + if resp.drag_started() { + crate::panels::start_drag_bin(ui, b.id.0); + } if resp.clicked() { let on_tri = resp.interact_pointer_pos().is_some_and(|p| p.x < x + 12.0); if on_tri || app.ui.project_panel.selected_bin == Some(b.id.0) && !resp.double_clicked() { @@ -844,7 +873,7 @@ fn card(app: &mut FilmcraftApp, ui: &mut egui::Ui, r: Rect, id: ItemId, pre: &st fn bin_card(app: &mut FilmcraftApp, ui: &mut egui::Ui, r: Rect, b: &Bin, v: &View, actions: &mut Actions) { let t = app.tokens; let th = Rect::from_min_size(r.min, vec2(r.width(), r.width() * 9.0 / 16.0)); - let resp = ui.interact(r.expand(2.0), egui::Id::new((&v.prefix, "bincard", b.id.0)), Sense::click()); + let resp = ui.interact(r.expand(2.0), egui::Id::new((&v.prefix, "bincard", b.id.0)), Sense::click_and_drag()); let selected = app.ui.project_panel.selected_bin == Some(b.id.0); let bg = Rect::from_min_max(th.min - vec2(6.0, 6.0), pos2(th.max.x + 6.0, r.max.y + 2.0)); ui.painter().rect_filled(bg, 4.0, if selected { t.tl_header_bg } else { t.panel_bg }); @@ -863,6 +892,9 @@ fn bin_card(app: &mut FilmcraftApp, ui: &mut egui::Ui, r: Rect, b: &Bin, v: &Vie } app.auto.add(&format!("{}.bin.{}", v.prefix, b.id.0), th, &b.name); accept_bin_drop(app, ui, th, b.id.0, actions); + if resp.drag_started() { + crate::panels::start_drag_bin(ui, b.id.0); + } if resp.clicked() { app.ui.project_panel.selected_bin = Some(b.id.0); actions.push(("project.select".into(), json!({"items": []}))); diff --git a/crates/ui-egui/src/panels/settings.rs b/crates/ui-egui/src/panels/settings.rs index 021b8473a..336ba0970 100644 --- a/crates/ui-egui/src/panels/settings.rs +++ b/crates/ui-egui/src/panels/settings.rs @@ -749,7 +749,7 @@ pub fn show(app: &mut FilmcraftApp, ctx: &egui::Context) -> bool { ui.horizontal(|ui| { ui.add_space(20.0); if button(app, ui, "settings.help", tl!("Help"), false) { - crate::links::open(ctx, &format!("{}/blob/main/docs/project-files.md#settings", crate::links::GITHUB)); + crate::links::open(app, ctx, &format!("{}/blob/main/docs/project-files.md#settings", crate::links::GITHUB)); } if button(app, ui, "settings.reset", tl!("Reset…"), false) { // this category back to its defaults (OK applies it) diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index ca77bbe7d..f5bfa7736 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -22,7 +22,7 @@ use crate::icons::{self, Icon}; use crate::state::{ThumbnailMode, TimelineView, Tool}; use crate::theme::Tokens; -use super::timeline_hit::{EdgeKind, Grab, edge_geometry, grab_at}; +use super::timeline_hit::{EdgeKind, Grab, edge_geometry, grab_at, marker_at, marker_rect}; pub use super::timeline_hit::{Hit, ROLL_PX, hit, selection_trim_kind}; const TOP_H: f32 = 58.0; // timecode + toolbar (left) / ruler (right) @@ -191,6 +191,26 @@ impl Layout { } } +/// #483: where a drop at `y` adds a track: the empty space above the top video track (a video +/// track) or below the last audio track (an audio track), never over a track, the caption tracks +/// or the Mix row. Gives the kind and the band the new track would take (`vh` / `ah` high). +fn new_track_at(seq: &Sequence, layout: &Layout, y: f32, vh: f32, ah: f32) -> Option<(TrackKind, Rect)> { + if layout.row_at(y).is_some() { + return None; + } + let cap_n = seq.caption_tracks.len(); + let tracks_top = layout.content.min.y + if cap_n > 0 { cap_n as f32 * super::timeline_captions::ROW_H + DIVIDER_H } else { 0.0 }; + let band = |y0: f32, y1: f32| Rect::from_min_max(pos2(layout.content.min.x, y0), pos2(layout.content.max.x, y1)); + if y < layout.split_y { + let top = layout.rows.iter().filter(|r| r.kind == TrackKind::Video).map(|r| r.rect.min.y).fold(layout.split_y, f32::min); + (y >= tracks_top && y < top).then(|| (TrackKind::Video, band((top - vh).max(tracks_top), top))) + } else { + let bottom = layout.rows.iter().filter(|r| r.kind == TrackKind::Audio).map(|r| r.rect.max.y).fold(layout.split_y + DIVIDER_H, f32::max); + let mix = layout.content.max.y - MASTER_H; + (y >= bottom && y < mix).then(|| (TrackKind::Audio, band(bottom, (bottom + ah).min(mix)))) + } +} + /// How far the Timeline reaches, in seconds: ten minutes past the end of the sequence. Premiere /// Pro's Timeline does the same (on a 6 s sequence its view stops with 00:10:06 at the right /// edge). The view cannot be scrolled past it and the scroll bar spans it. @@ -376,7 +396,10 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { let visible = (layout.tick_at(content.min.x - 2.0), layout.tick_at(content.max.x + 2.0)); let selection: Vec = app.session.state.selection.clone(); let mut previews: HashMap)> = HashMap::new(); // live drag preview: (start, dur, track) - preview_drag(app, &seq, &layout, &mut previews); + // clips whose media changes in the preview (a trimmed or rolled edge): drawn from the edited + // item, so the waveform and thumbnail stay where the media is instead of squeezing (#374) + let mut edited: HashMap = HashMap::new(); + preview_drag(app, &seq, &mut previews, &mut edited); for r in &rows { let Some(tr) = seq.track(r.track) else { continue }; let clip_rect = if r.kind == TrackKind::Video { vclip } else { aclip }; @@ -397,6 +420,7 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { } } for it in &tr.items { + let it = edited.get(&it.id).unwrap_or(it); let (start, dur, moved_track) = previews.get(&it.id).copied().unwrap_or((it.start, it.duration, None)); if moved_track.is_some_and(|m| m != r.track) { continue; @@ -1001,12 +1025,12 @@ fn draw_transition(p: &egui::Painter, r: Rect, trn: &filmcraft_project::Transiti } } -/// A Premiere track-header button (patch / target): blue fill when on, full track height. +/// A Premiere track-header button (patch / target): highlight-blue fill when on, full track height. fn patch_button(ui: &mut egui::Ui, r: Rect, clip: Rect, label: &str, on: bool, show_off: bool, id: egui::Id, t: &Tokens) -> egui::Response { let resp = ui.interact(r.intersect(clip), id, Sense::click()); let p = ui.painter().with_clip_rect(clip); if on { - p.rect_filled(r, 2.0, Color32::from_rgb(0x26, 0x5b, 0xc1)); + p.rect_filled(r, 2.0, t.track_target); p.text(r.center(), Align2::CENTER_CENTER, label, Tokens::semibold(10.0), Color32::from_rgb(0xeb, 0xeb, 0xeb)); } else if show_off { if resp.hovered() { @@ -1159,38 +1183,6 @@ fn draw_headers(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, rows: egui::Id::new(("hdr", r.track.0)), Sense::click(), ); - app.auto.add(&format!("timeline.track.{label}.name"), resp.rect, "Track name (right-click to rename)"); - let rename_id = egui::Id::new(("timeline-track-rename", r.track.0)); - if resp.secondary_clicked() { - // Start each menu opening from the committed name, not a cancelled draft. - ui.data_mut(|d| d.insert_temp(rename_id, tr.name.clone())); - } - egui::Popup::context_menu(&resp).close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside).show(|ui| { - ui.set_min_width(220.0); - ui.label(tl!("Rename")); - let mut name: String = ui.data(|d| d.get_temp(rename_id)).unwrap_or_else(|| tr.name.clone()); - let input = ui.add(egui::TextEdit::singleline(&mut name).desired_width(210.0)); - app.auto.add(&format!("timeline.track.{label}.renameField"), input.rect, "New track name"); - if resp.secondary_clicked() { - input.request_focus(); - } - let save_on_enter = input.has_focus() && ui.input(|i| i.key_pressed(egui::Key::Enter)); - let valid = !name.trim().is_empty(); - ui.horizontal(|ui| { - let apply = ui.add_enabled(valid, egui::Button::new(tl!("Apply"))); - app.auto.add(&format!("timeline.track.{label}.renameApply"), apply.rect, "Apply track name"); - if (apply.clicked() || (valid && save_on_enter)) && name.trim() != tr.name { - actions.push(("timeline.setTrack".into(), json!({"track": r.track.0, "name": name.trim()}))); - ui.close(); - } else if apply.clicked() || (valid && save_on_enter) { - ui.close(); - } - if ui.button(tl!("Cancel")).clicked() { - ui.close(); - } - }); - ui.data_mut(|d| d.insert_temp(rename_id, name)); - }); if resp.double_clicked() { let h = if r.kind == TrackKind::Video { &mut app.ui.timeline.video_track_h } else { &mut app.ui.timeline.audio_track_h }; *h = if *h < 50.0 { 64.0 } else { 30.0 }; @@ -1399,7 +1391,7 @@ fn draw_top(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect, seq: &Sequenc clip.rect_filled(Rect::from_min_max(pos2(x, marker_y), pos2(layout.x_of(m.start + m.duration), marker_y + 8.0)), 0.0, c.gamma_multiply(0.6)); } clip.add(egui::Shape::convex_polygon(shape, c, Stroke::NONE)); - let mr = Rect::from_center_size(pos2(x, marker_y + 6.0), vec2(10.0, 13.0)); + let mr = marker_rect(layout, m.start); app.auto.add(&format!("timeline.marker.{}", m.id.0), mr, &m.name); if ui.rect_contains_pointer(mr) && !m.name.is_empty() { egui::Tooltip::always_open(ui.ctx().clone(), ui.layer_id(), egui::Id::new(("mk", m.id.0)), egui::PopupAnchor::Pointer).show(|ui| { @@ -1613,7 +1605,14 @@ pub fn locate(app: &FilmcraftApp, clip: u64, edge: Option<&str>) -> Option<(f32, Some((x, y)) } -fn preview_drag(app: &FilmcraftApp, seq: &Sequence, _layout: &Layout, out: &mut HashMap)>) { +fn preview_drag(app: &FilmcraftApp, seq: &Sequence, out: &mut HashMap)>, edited: &mut HashMap) { + // the item as the edge move leaves it, and its place in the preview + let mut moved = |it: &TrackItem, edge: filmcraft_edit::Edge, delta: Tick| { + let mut e = it.clone(); + filmcraft_edit::move_edge(&mut e, edge, delta, true); + out.insert(e.id, (e.start, e.duration, None)); + edited.insert(e.id, e); + }; let Some(d) = &app.tl.drag else { return }; match d { Drag::Move { clips, offset, track_delta, .. } => { @@ -1629,7 +1628,15 @@ fn preview_drag(app: &FilmcraftApp, seq: &Sequence, _layout: &Layout, out: &mut out.insert(*clip, (it.start, it.duration + *delta, None)); } } - Drag::Trim { clip, edge, delta, .. } | Drag::Stretch { clip, edge, delta, .. } => { + Drag::Trim { clip, edge, delta, .. } => { + for c in filmcraft_engine::commands::with_links(&app.session, &[*clip]) { + if let Some((_, it)) = seq.find_item(c) { + moved(it, *edge, *delta); + } + } + } + // a rate stretch keeps the same media in a new length: that one does squeeze + Drag::Stretch { clip, edge, delta, .. } => { let ids = filmcraft_engine::commands::with_links(&app.session, &[*clip]); for c in ids { if let Some((_, it)) = seq.find_item(c) { @@ -1643,10 +1650,10 @@ fn preview_drag(app: &FilmcraftApp, seq: &Sequence, _layout: &Layout, out: &mut } Drag::Roll { left, right, delta, .. } => { if let Some((_, l)) = seq.find_item(*left) { - out.insert(*left, (l.start, l.duration + *delta, None)); + moved(l, filmcraft_edit::Edge::Out, *delta); } if let Some((_, r)) = seq.find_item(*right) { - out.insert(*right, (r.start + *delta, r.duration - *delta, None)); + moved(r, filmcraft_edit::Edge::In, *delta); } } Drag::Slide { clip, delta } => { @@ -1673,6 +1680,22 @@ fn shift_track(seq: &Sequence, tid: TrackId, delta: i32) -> Option { /// The clip context menu: groups (separated by rules) of (label, command id). Entries marked `…` /// open their dialog through `menus::invoke`, like the same item in the Clip menu. +/// The menu of a right-clicked Timeline marker (automation ids `timeline.markerMenu.`); +/// the marker is under the playhead, where Clear Selected Marker looks for it (#695). +fn marker_menu(app: &mut FilmcraftApp, ctx: &egui::Context, ui: &mut egui::Ui) { + for (label, cmd) in [("Clear Selected Marker", "markers.clearCurrent"), ("Clear Markers", "markers.clearAll")] { + let label = crate::i18n::t(label); + let r = ui.button(label); + app.auto.add(&format!("timeline.markerMenu.{cmd}"), r.rect, label); + if r.clicked() { + if let Err(e) = crate::menus::invoke(app, ctx, cmd, json!({})) { + app.ui.status = e; + } + ui.close(); + } + } +} + pub(crate) const CLIP_MENU: &[&[(&str, &str)]] = &[ &[ ("Cut", "edit.cut"), @@ -2071,9 +2094,14 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & // select (shift toggles; alt selects one side of a link) let sel = &app.session.state.selection; if mods.shift { - let _ = app.session.execute("timeline.select", json!({"clips": [clip.0], "toggle": true, "linked": !mods.alt})); - } else if mods.alt || !sel.contains(&clip) { - let _ = app.session.execute("timeline.select", json!({"clips": [clip.0], "linked": !mods.alt})); + let _ = app.session.execute("timeline.select", json!({"clips": [clip.0], "toggle": true})); + } 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]})); + } } if resp.drag_started() { let clips = app.session.state.selection.clone(); @@ -2128,6 +2156,10 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & if mods.shift || app.session.prefs.timeline.snap_playhead { tt = snap_playhead(app, seq, layout, tt); } + // a press on a marker selects it: the playhead goes exactly onto it (#695) + if let Some(m) = marker_at(seq, layout, p) { + tt = m.max(Tick::ZERO); + } app.session.set_playhead(tt); } if resp.drag_started() { @@ -2179,7 +2211,9 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & Drag::Trim { clip, edge, mode, from, .. } => seq.find_item(clip).map(|(_, it)| { let base = if edge == filmcraft_edit::Edge::In { it.start } else { it.end() }; let target = snap(app, seq, layout, rate.snap_nearest(base + (t_here - from)), &[clip]); - Drag::Trim { clip, edge, mode, delta: target - base, from } + // stop at the end of the media (or the neighbour), as the trim on release will (#653) + let delta = filmcraft_engine::commands::trim_delta(&app.session, clip, edge, mode, target - base).unwrap_or(target - base); + Drag::Trim { clip, edge, mode, delta, from } }), Drag::Remix { clip, from, .. } => seq.find_item(clip).map(|(_, it)| { let target = rate.snap_nearest(it.end() + (t_here - from)).max(it.start + rate.frame_duration()); @@ -2327,6 +2361,17 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & ctx.data_mut(|d| d.insert_temp::>(edit_point_menu_id, None)); } } + // on a marker of the ruler, the marker menu instead (right-clicking a marker selects it first) + let marker_menu_id = egui::Id::new("timeline.markerMenu.open"); + if resp.secondary_clicked() + && let Some(p) = resp.interact_pointer_pos() + { + let marker = marker_at(seq, layout, p); + if let Some(m) = marker { + app.session.set_playhead(m.max(Tick::ZERO)); + } + ctx.data_mut(|d| d.insert_temp(marker_menu_id, marker.is_some())); + } if resp.secondary_clicked() && let Some(p) = resp.interact_pointer_pos() && let Hit::Clip { clip, edge, track } = hit(seq, layout, p) @@ -2354,6 +2399,10 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & } resp.context_menu(|ui| { ui.set_min_width(220.0); + if ctx.data(|d| d.get_temp::(marker_menu_id)).unwrap_or(false) { + marker_menu(app, &ctx, ui); + return; + } if let Some(target) = ctx.data(|d| d.get_temp::>(edit_point_menu_id)).flatten() { edit_point_menu(app, &ctx, ui, seq, target); return; @@ -2432,21 +2481,34 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & let dur = source .map(|s| s.range.duration) .unwrap_or_else(|| app.session.project.item(item).map(|i| i.duration()).filter(|d| d.0 > 0 && !is_still).unwrap_or(still)); - if let Some(row) = &row { - let r = Rect::from_min_max(pos2(layout.x_of(t), row.rect.min.y + 1.0), pos2(layout.x_of(t + dur), row.rect.max.y - 1.0)); + // #483: in the empty space above the top video track or below the last audio track, a new track + let new_track = if row.is_some() { None } else { new_track_at(seq, layout, p.y, app.ui.timeline.video_track_h, app.ui.timeline.audio_track_h) }.filter( + |(kind, _)| { + let carries = app.session.project.item(item).is_some_and(|i| if *kind == TrackKind::Video { i.has_video() } else { i.has_audio() }); + carries && source.is_none_or(|s| if *kind == TrackKind::Video { s.video } else { s.audio }) + }, + ); + if let Some(band) = row.as_ref().map(|r| r.rect).or(new_track.map(|(_, band)| band)) { + let r = Rect::from_min_max(pos2(layout.x_of(t), band.min.y + 1.0), pos2(layout.x_of(t + dur), band.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); if mods.command { ui.painter().text(r.left_top() + vec2(4.0, -2.0), Align2::LEFT_BOTTOM, tl!("Insert"), Tokens::ui(10.0), Color32::WHITE); } } + let target = match (&row, new_track) { + (Some(row), _) => Some(match row.kind { + TrackKind::Video => (json!(row.track.0), json!(seq.audio_tracks.get(row.index).or(seq.audio_tracks.first()).map(|t| t.id.0))), + TrackKind::Audio => (json!(seq.video_tracks.get(row.index).or(seq.video_tracks.first()).map(|t| t.id.0)), json!(row.track.0)), + }), + // the other stream goes on the track of the same number, or on a new one as well + (None, Some((TrackKind::Video, _))) => Some((json!("new"), seq.audio_tracks.get(seq.video_tracks.len()).map_or(json!("new"), |t| json!(t.id.0)))), + (None, Some((TrackKind::Audio, _))) => Some((seq.video_tracks.get(seq.audio_tracks.len()).map_or(json!("new"), |t| json!(t.id.0)), json!("new"))), + (None, None) => None, + }; if ctx.input(|i| i.pointer.any_released()) - && let Some(row) = row + && let Some((vt, at)) = target { - 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 mut params = json!({"item": item.0, "track": vt, "audioTrack": at, "time": t.0, "insert": mods.command}); if let Some(source) = source { params["sourceIn"] = json!(source.range.start.0); @@ -2523,6 +2585,8 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: & if ctx.input(|i| i.pointer.any_released()) { let r = if let Some(name) = effect.strip_prefix("preset:") { app.session.execute("presets.apply", json!({"preset": name, "clips": [clip.0]})) + } else if let Some(name) = effect.strip_prefix(crate::panels::LUMETRI_PRESET_PREFIX) { + app.session.execute("lumetri.applyPreset", json!({"name": name, "clips": [clip.0]})) } else if transition_kind.is_some() { app.session.execute("effects.apply", params) } else { @@ -2596,6 +2660,72 @@ pub(crate) fn chain_starts(start: Tick, durations: &[Tick]) -> Vec { .collect() } +/// #374: trimming an audio clip must cut its waveform off at the moving edge, not squeeze the whole +/// waveform into the shorter clip: the preview draws from the item the trim will leave, which shows +/// the same media at every timeline time it still covers. +#[cfg(test)] +mod trim_preview_tests { + use super::*; + + fn demo() -> (FilmcraftApp, Sequence) { + let mut s = filmcraft_engine::Session::default(); + s.execute("file.openDemoProject", serde_json::json!({})).unwrap(); + let seq = s.active_sequence().unwrap().clone(); + (FilmcraftApp::new(s), seq) + } + + fn assert_same_media(before: &TrackItem, after: &TrackItem) { + let mut t = after.start.max(before.start); + while t < after.end().min(before.end()) { + assert!((before.source_time_at(t) - after.source_time_at(t)).0.abs() <= 1, "{}: media moved at {t:?}", before.name); + t += Tick(filmcraft_time::TICKS_PER_SECOND / 10); + } + } + + #[test] + fn trimming_the_music_keeps_its_waveform_in_place() { + let (mut app, seq) = demo(); + let music = seq.audio_tracks.iter().flat_map(|t| &t.items).find(|i| i.name.contains("Ambient")).unwrap().clone(); + for (edge, delta) in [(filmcraft_edit::Edge::In, 5), (filmcraft_edit::Edge::Out, -5), (filmcraft_edit::Edge::In, -1)] { + let delta = Tick(delta * filmcraft_time::TICKS_PER_SECOND); + app.tl.drag = Some(Drag::Trim { clip: music.id, edge, mode: filmcraft_edit::TrimMode::Regular, delta, from: Tick::ZERO }); + let (mut out, mut edited) = (HashMap::new(), HashMap::new()); + preview_drag(&app, &seq, &mut out, &mut edited); + let shown = edited.get(&music.id).expect("the trimmed clip is drawn from its edited item"); + // where the preview places it is unchanged + let want = match edge { + filmcraft_edit::Edge::In => (music.start + delta, music.duration - delta, None), + filmcraft_edit::Edge::Out => (music.start, music.duration + delta, None), + }; + assert_eq!(out.get(&music.id).copied(), Some(want)); + assert_eq!((shown.start, shown.duration), (want.0, want.1)); + // the music under every remaining timeline time is the same: the waveform stays put + assert_same_media(&music, shown); + if edge == filmcraft_edit::Edge::In { + assert_eq!(shown.source_in, music.source_in + delta, "5 s into the music, not the music squeezed"); + } + } + } + + #[test] + fn rolling_an_edit_keeps_both_sides_in_place_but_a_stretch_still_squeezes() { + let (mut app, seq) = demo(); + let (left, right) = seq.audio_tracks[0].items.windows(2).find(|w| w[0].end() == w[1].start).map(|w| (w[0].clone(), w[1].clone())).unwrap(); + let delta = Tick(filmcraft_time::TICKS_PER_SECOND / 2); + app.tl.drag = Some(Drag::Roll { left: left.id, right: right.id, delta, from: Tick::ZERO }); + let (mut out, mut edited) = (HashMap::new(), HashMap::new()); + preview_drag(&app, &seq, &mut out, &mut edited); + assert_same_media(&left, &edited[&left.id]); + assert_same_media(&right, &edited[&right.id]); + assert_eq!(edited[&right.id].start, right.start + delta); + // Rate Stretch keeps the same media in a new length: drawn from the original item + app.tl.drag = Some(Drag::Stretch { clip: left.id, edge: filmcraft_edit::Edge::Out, delta, from: Tick::ZERO }); + let (mut out, mut edited) = (HashMap::new(), HashMap::new()); + preview_drag(&app, &seq, &mut out, &mut edited); + assert!(edited.is_empty() && out.contains_key(&left.id)); + } +} + #[cfg(test)] mod waveform_tests { use super::waveform_display_gain; @@ -2700,3 +2830,40 @@ mod thumbnail_tiles_tests { assert!(thumbnail_tiles(ThumbnailMode::Continuous, 0.0, 0.0, 80.0, (0.0, 800.0), dur, FRAME).is_empty()); } } + +#[cfg(test)] +mod new_track_tests { + use super::*; + use filmcraft_project::{Project, SequenceSettings}; + + /// #483: a drop above the top video track adds a video track, one below the last audio track an + /// audio track; over a track, between the areas or on the Mix row it adds none. + #[test] + fn empty_space_past_the_tracks_adds_a_track() { + let mut p = Project::new("t"); + let id = p.new_sequence("s", SequenceSettings::default(), 1, 1, None); + let seq = p.sequence(id).cloned().expect("sequence"); + // video area 0..200 (V1 at 150..200), divider, audio area 205..466 (A1 at 205..255), Mix 466..500 + let row = |kind, track: &filmcraft_project::Track, y0: f32| Row { + track: track.id, + kind, + index: 0, + rect: Rect::from_min_max(pos2(0.0, y0), pos2(800.0, y0 + 50.0)), + lane: false, + }; + let layout = Layout { + content: Rect::from_min_max(pos2(0.0, 0.0), pos2(800.0, 500.0)), + ruler: Rect::from_min_max(pos2(0.0, -20.0), pos2(800.0, 0.0)), + rows: vec![row(TrackKind::Video, &seq.video_tracks[0], 150.0), row(TrackKind::Audio, &seq.audio_tracks[0], 205.0)], + pps: 100.0, + scroll: 0.0, + split_y: 200.0, + }; + let at = |y| new_track_at(&seq, &layout, y, 50.0, 50.0); + assert_eq!(at(40.0), Some((TrackKind::Video, Rect::from_min_max(pos2(0.0, 100.0), pos2(800.0, 150.0))))); + assert_eq!(at(300.0), Some((TrackKind::Audio, Rect::from_min_max(pos2(0.0, 255.0), pos2(800.0, 305.0))))); + for y in [160.0, 202.0, 230.0, 480.0] { + assert_eq!(at(y), None, "y = {y}"); + } + } +} diff --git a/crates/ui-egui/src/panels/timeline_hit.rs b/crates/ui-egui/src/panels/timeline_hit.rs index 16dd5f712..9e311f50d 100644 --- a/crates/ui-egui/src/panels/timeline_hit.rs +++ b/crates/ui-egui/src/panels/timeline_hit.rs @@ -5,9 +5,10 @@ use std::cmp::Ordering; -use egui::{Modifiers, Pos2}; +use egui::{Modifiers, Pos2, Rect, pos2, vec2}; use filmcraft_edit::Edge; use filmcraft_project::{ClipId, Sequence, Track, TrackId, TransitionId}; +use filmcraft_time::Tick; use super::timeline::Layout; use crate::state::Tool; @@ -68,6 +69,18 @@ pub enum Hit { None, } +/// Where the Timeline ruler draws (and hit-tests) the pentagon of a marker at `start`. +pub fn marker_rect(layout: &Layout, start: Tick) -> Rect { + Rect::from_center_size(pos2(layout.x_of(start), layout.ruler.min.y + 8.0), vec2(10.0, 13.0)) +} + +/// The start of the marker whose pentagon is under `pos` on the ruler (the topmost, i.e. last +/// drawn, when several overlap). A press there puts the playhead on the marker, so Markers ▸ Clear +/// Selected Marker removes it (#695). +pub fn marker_at(seq: &Sequence, layout: &Layout, pos: Pos2) -> Option { + seq.markers.iter().rev().map(|m| m.start).find(|t| marker_rect(layout, *t).contains(pos)) +} + pub fn hit(seq: &Sequence, layout: &Layout, pos: Pos2) -> Hit { if layout.ruler.contains(pos) { return Hit::Ruler; @@ -404,6 +417,26 @@ mod tests { } } + #[test] + fn a_press_on_a_marker_lands_on_its_start() { + let (mut seq, l, _) = fixture(clips()); + let off = Tick::from_seconds_f64(3.017); + seq.markers.push(filmcraft_project::Marker { + id: filmcraft_project::MarkerId(9), + start: off, + duration: Tick::ZERO, + name: String::new(), + comment: String::new(), + kind: filmcraft_project::MarkerKind::Comment, + color: Label::Green, + }); + // 3 px beside the pentagon's tip, where the nearest frame is not the marker's time + assert_eq!(marker_at(&seq, &l, pos2(304.7, 10.0)), Some(off)); + assert_eq!(marker_at(&seq, &l, pos2(310.0, 10.0)), None); + assert_eq!(marker_at(&seq, &l, pos2(301.7, 30.0)), None); + assert_eq!(marker_at(&seq, &l, pos2(f32::NAN, 10.0)), None); + } + use egui::Modifiers; use crate::state::Tool; diff --git a/crates/ui-egui/src/panels/transition_controls.rs b/crates/ui-egui/src/panels/transition_controls.rs index 80a8a0d4a..47decaf28 100644 --- a/crates/ui-egui/src/panels/transition_controls.rs +++ b/crates/ui-egui/src/panels/transition_controls.rs @@ -74,6 +74,11 @@ pub fn effect_controls(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect, id 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); + // the line under each row, as for a clip's effects (#640): halfway into the row spacing + let row_line = |ui: &egui::Ui| { + let y = ui.cursor().top() - 4.0; + ui.painter().line_segment([egui::pos2(rect.min.x, y), egui::pos2(rect.max.x, y)], egui::Stroke::new(1.0, t.separator)); + }; 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")); @@ -84,6 +89,7 @@ pub fn effect_controls(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect, id changes.push(json!({"frames": frames, "merge": true, "begin": r.drag_started() || !r.dragged()})); } ui.end_row(); + row_line(ui); ui.label(tl!("Alignment")); let shown = align.label(); @@ -106,6 +112,7 @@ pub fn effect_controls(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect, id .inner; elems.push(("effectControls.transition.alignment".into(), r.rect, shown.to_string())); ui.end_row(); + row_line(ui); ui.label(tl!("Reverse")); let mut rev = x.reverse; @@ -115,6 +122,7 @@ pub fn effect_controls(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect, id changes.push(json!({"reverse": rev})); } ui.end_row(); + row_line(ui); 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); @@ -123,6 +131,7 @@ pub fn effect_controls(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect, id changes.push(v); } ui.end_row(); + row_line(ui); } }); ui.add_space(8.0); diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index aa89ca024..115762d45 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -442,6 +442,10 @@ pub struct UiState { pub lumetri_grid_folder: Option, /// Collapsed effect sections in Effect Controls ("clip:index"). pub collapsed_fx: Vec, + /// Width of Effect Controls' effect list, left of the keyframe area (points; 0 = the default + /// share of the panel). Dragging the divider sets it (#643). + #[serde(default)] + pub effect_controls_split: f32, pub show_menu_bar: bool, /// The header bar (Home, Import, Edit, Export, workspaces). An app that embeds FilmCraft can hide it. #[serde(default = "shown")] @@ -952,6 +956,7 @@ impl Default for UiState { expanded_fx: vec!["Video Transitions".into(), "Video Transitions/Dissolve".into()], lumetri_grid_folder: None, collapsed_fx: vec![], + effect_controls_split: 0.0, show_menu_bar: true, show_header: true, show_status_bar: true, diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index b7560f321..a86834950 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -133,6 +133,8 @@ pub struct Tokens { pub tl_ruler_tick: Color32, pub tl_ruler_text: Color32, pub playhead: Color32, + /// Fill of a track header's source-patch / track-targeting button while it is on. + pub track_target: Color32, pub in_out_shade: Color32, pub clip_selected_border: Color32, pub render_red: Color32, @@ -225,6 +227,7 @@ impl Tokens { tl_ruler_tick: Color32::from_rgb(0x8d, 0x8d, 0x8d), tl_ruler_text: Color32::from_rgb(0xb0, 0xb0, 0xb0), playhead: Color32::from_rgb(0x58, 0x95, 0xec), + track_target: Color32::from_rgb(0x26, 0x5b, 0xc1), in_out_shade: Color32::from_rgb(0x3f, 0x3f, 0x3f), clip_selected_border: Color32::from_rgb(0xeb, 0xeb, 0xeb), render_red: Color32::from_rgb(0xe3, 0x48, 0x50), @@ -330,11 +333,28 @@ impl Tokens { } } - /// Settings ▸ Appearance on top of a theme: the highlight colour (selections, focus, primary - /// buttons) and accessible colour contrast (brighter secondary text and borders). + /// Settings ▸ Appearance on top of a theme: the highlight colour (selections, primary buttons, + /// the focus outline, the playhead, timecodes, drag values and track targeting) and accessible + /// colour contrast (brighter secondary text and borders). pub fn with_appearance(mut self, highlight: Option<[u8; 3]>, accessible_contrast: bool) -> Self { if let Some([r, g, b]) = highlight { let c = Color32::from_rgb(r, g, b); + // A highlight of the user's own also colours the theme's other blues: the same hue, as + // much more or less saturated and bright as each of them is than the theme's highlight. + // The theme's own highlight leaves them exactly as designed. + if c != self.accent { + let tint = |s: f32, v: f32| { + let mut h = egui::ecolor::HsvaGamma::from(c); + h.s = (h.s * s).clamp(0.0, 1.0); + h.v = (h.v * v).clamp(0.0, 1.0); + Color32::from(h) + }; + self.focus = tint(0.92, 1.06); + self.playhead = self.focus; + self.timecode = tint(0.98, 1.09); + self.hot_text = self.timecode; + self.track_target = tint(1.0, 0.865); + } let lift = |v: u8| v.saturating_add(16); self.accent = c; self.accent_hover = Color32::from_rgb(lift(r), lift(g), lift(b)); @@ -550,6 +570,32 @@ mod tests { } } + /// The colours the highlight drives besides the accent (#707). + fn highlight_blues(t: &Tokens) -> [Color32; 5] { + [t.focus, t.playhead, t.timecode, t.hot_text, t.track_target] + } + + #[test] + fn the_default_highlight_leaves_the_theme_blues_as_designed() { + let default = filmcraft_engine::settings::parse_hex(filmcraft_engine::settings::DEFAULT_HIGHLIGHT); + for k in [ThemeKind::Dark, ThemeKind::Medium, ThemeKind::Light] { + assert_eq!(highlight_blues(&Tokens::for_kind(k).with_appearance(default, false)), highlight_blues(&Tokens::for_kind(k)), "{k:?}"); + } + } + + #[test] + fn a_highlight_of_ones_own_colours_the_playhead_timecodes_focus_and_targeting() { + let pink = Color32::from_rgb(0xe0, 0x45, 0x7b); + let t = Tokens::for_kind(ThemeKind::Dark).with_appearance(Some([0xe0, 0x45, 0x7b]), false); + let hue = |c: Color32| egui::ecolor::HsvaGamma::from(c).h; + for (c, was) in highlight_blues(&t).into_iter().zip(highlight_blues(&Tokens::for_kind(ThemeKind::Dark))) { + assert_ne!(c, was); + assert!((hue(c) - hue(pink)).abs() < 0.01, "{c:?} has the highlight's hue"); + } + let v = |c: Color32| egui::ecolor::HsvaGamma::from(c).v; + assert!(v(t.playhead) > v(pink) && v(t.timecode) > v(pink) && v(t.track_target) < v(pink), "lighter and darker as the default blues are"); + } + /// Chinese media and track names must render even when the interface stays in English. /// Compare the rasterized glyphs with the missing-glyph box, including custom weight families. #[test] diff --git a/crates/ui-egui/tests/color_ui.rs b/crates/ui-egui/tests/color_ui.rs index 956e7f211..1ea047c84 100644 --- a/crates/ui-egui/tests/color_ui.rs +++ b/crates/ui-egui/tests/color_ui.rs @@ -324,3 +324,26 @@ fn lumetri_presets_in_the_effects_panel() { let e = lumetri(&mut d, clip); assert_eq!(e["params"]["temperature"]["value"], json!("Float(35.0)"), "{e}"); } + +#[test] +fn lumetri_presets_drag_onto_a_timeline_clip() { + let mut d = Driver::demo(); + d.ok("ui.panel.show", json!({"panel": "Effects"})); + d.frames(3); + // the first clip is selected; the preset is dropped on the second, which gets it alone + let selected = select_first_clip(&mut d); + let seq = d.exec("sequence.inspect", json!({})); + let target = seq["video"][0]["items"][1]["clip"].as_u64().unwrap_or_else(|| panic!("no second clip: {seq}")); + d.click("effects.folder.Lumetri Presets"); + d.click("effects.folder.Lumetri Presets/Monochrome"); + d.frames(2); + assert!(d.element(&format!("timeline.clip.{target}")).is_some(), "target clip not on screen"); + d.ok("ui.drag", json!({"from": {"id": "effects.lumetriPreset.Neutral Mono"}, "to": {"id": format!("timeline.clip.{target}")}, "steps": 8})); + d.frames(3); + let e = lumetri(&mut d, target); + assert_eq!(e["params"]["saturation"]["value"], json!("Float(0.0)"), "dropped preset on the target clip: {e}"); + assert_eq!(lumetri(&mut d, selected), Value::Null, "the selected clip it was not dropped on is untouched"); + // one undo step removes it again + d.exec("edit.undo", json!({})); + assert_eq!(lumetri(&mut d, target), Value::Null); +} diff --git a/crates/ui-egui/tests/keyboard_ui.rs b/crates/ui-egui/tests/keyboard_ui.rs index 930ae086e..52ece3995 100644 --- a/crates/ui-egui/tests/keyboard_ui.rs +++ b/crates/ui-egui/tests/keyboard_ui.rs @@ -71,31 +71,6 @@ impl Driver { } } -#[test] -fn alt_click_isolates_already_selected_linked_clip_before_delete() { - for audio in [false, true] { - let mut d = Driver::new(); - let video = d.app().session.active_sequence().unwrap().video_tracks[0].items[0].clone(); - let sound = d.app().session.active_sequence().unwrap().audio_tracks[0].items.iter().find(|i| i.link == video.link).unwrap().clone(); - let (selected, partner) = if audio { (sound.id, video.id) } else { (video.id, sound.id) }; - d.exec("timeline.select", json!({"clips": [selected.0]})); - assert_eq!(d.app().session.state.selection.len(), 2); - let r = d.app().auto.find(&format!("timeline.clip.{}", selected.0)).unwrap().rect; - d.harness.input_mut().events.push(egui::Event::ModifiersChanged(egui::Modifiers::ALT)); - d.frames(1); - d.ok("ui.click", json!({"x": r[0] + r[2] * 0.5, "y": r[1] + r[3] * 0.3, "modifiers": {"alt": true}})); - d.frames(3); - assert_eq!(d.app().session.state.selection, vec![selected]); - d.harness.input_mut().events.push(egui::Event::ModifiersChanged(egui::Modifiers::NONE)); - d.frames(1); - d.key("Delete"); - assert!(d.app().session.active_sequence().unwrap().find_item(selected).is_none()); - assert!(d.app().session.active_sequence().unwrap().find_item(partner).is_some()); - d.exec("edit.undo", json!({})); - assert!(d.app().session.active_sequence().unwrap().find_item(selected).is_some()); - } -} - /// Ids added for keyboard parity (engine + UI). fn new_ids() -> Vec { let mut v: Vec = filmcraft_engine::keyboard::commands().iter().map(|c| c.id.to_string()).collect(); diff --git a/crates/ui-egui/tests/keyframes_ui.rs b/crates/ui-egui/tests/keyframes_ui.rs index c97cd5066..fe12275f6 100644 --- a/crates/ui-egui/tests/keyframes_ui.rs +++ b/crates/ui-egui/tests/keyframes_ui.rs @@ -184,6 +184,22 @@ fn properties_diamond_adds_and_removes_the_keyframe_at_the_playhead() { assert!(d.find(&format!("{SCALE}.prevKeyframe")).is_none()); } +/// #284: every Effect Controls property has a reset button; on an animated property it puts the +/// default in as a keyframe at the playhead and keeps the other keyframes. +#[test] +fn effect_controls_reset_button_keeps_keyframes() { + let (mut d, clip) = Driver::demo(); + let t0 = d.seek(0.5); + d.click(&format!("{SCALE}.addKeyframe")); + d.exec("effects.setParam", json!({"clip": clip.id, "effect": "motion", "param": "scale", "value": 150.0, "time": t0})); + d.seek(2.5); + d.click("effectControls.motion.scale.reset"); + d.shot("effect-controls-reset"); + let (keys, value) = d.scale(&clip); + assert_eq!(keys.len(), 2, "the keyframe stays and the default is keyed at the playhead: {keys:?}"); + assert_eq!(value, 100.0); +} + /// Crop is not on a clip until it is used: its diamond applies the effect, then adds the keyframe. #[test] fn properties_diamond_applies_crop_first() { @@ -258,3 +274,42 @@ fn clicking_a_lane_keyframe_selects_only_that_keyframe() { d.click("effectControls.motion.position.addKeyframe"); assert_eq!(selection(&mut d), json!([]), "a deleted keyframe is not selected"); } + +/// Effect Controls' divider (#643): dragging it widens the effect list or the keyframe area, each +/// keeping a minimum width, and the width stays. The mouse wheel scrolls the rows over the +/// keyframe area too, not only over the effect list. +#[test] +fn effect_controls_divider_and_scrolling_anywhere() { + let (mut d, clip) = Driver::demo(); + let lane = d.rect("effectControls.lane"); + let div = d.rect("effectControls.divider"); + let (x, y) = (div[0] + div[2] / 2.0, div[1] + div[3] / 2.0); + d.ok("ui.drag", json!({"from": {"x": x, "y": y}, "to": {"x": x + 80.0, "y": y}, "steps": 6})); + let narrower = d.rect("effectControls.lane"); + assert!((narrower[0] - (lane[0] + 80.0)).abs() <= 1.0, "the keyframe area starts 80 points further right: {narrower:?} vs {lane:?}"); + assert!((narrower[0] + narrower[2] - (lane[0] + lane[2])).abs() <= 1.0, "and still ends at the panel's edge"); + d.frames(3); + assert_eq!(d.rect("effectControls.lane"), narrower, "the width stays"); + // dragged all the way right, the keyframe area keeps its minimum width + let drag_divider = |d: &mut Driver, by: f64| { + let div = d.rect("effectControls.divider"); + let (x, y) = (div[0] + div[2] / 2.0, div[1] + div[3] / 2.0); + d.ok("ui.drag", json!({"from": {"x": x, "y": y}, "to": {"x": x + by, "y": y}, "steps": 6})); + }; + drag_divider(&mut d, 3000.0); + assert!(d.rect("effectControls.lane")[2] >= 59.0, "the keyframe area can't vanish"); + // and all the way left, the effect list keeps the width its rows are laid out for + drag_divider(&mut d, -3000.0); + assert!(d.rect("effectControls.effect.motion")[2] >= 259.0, "the effect list stays wide enough for its values"); + + // enough effects to scroll; the wheel over the keyframe area scrolls the rows + for fx in ["gaussian_blur", "transform", "crop", "tint"] { + d.exec("effects.apply", json!({"clips": [clip.id], "effect": fx})); + } + let before = d.rect("effectControls.motion.scale.stopwatch"); + let lane = d.rect("effectControls.lane"); + d.ok("ui.scroll", json!({"x": lane[0] + lane[2] / 2.0, "y": lane[1] + lane[3] * 0.7, "dx": 0.0, "dy": -80.0})); + d.frames(4); + let after = d.find("effectControls.motion.scale.stopwatch"); + assert!(after.is_none_or(|r| r[1] < before[1] - 20.0), "the rows scrolled up: {before:?} → {after:?}"); +} diff --git a/crates/ui-egui/tests/panels_ui.rs b/crates/ui-egui/tests/panels_ui.rs index e395f4a8a..d548d6cb9 100644 --- a/crates/ui-egui/tests/panels_ui.rs +++ b/crates/ui-egui/tests/panels_ui.rs @@ -158,7 +158,7 @@ fn every_panel_opens_from_the_window_menu() { let menu = d.ok("ui.menu.list", json!({})); let window: Vec = menu.as_array().unwrap().iter().filter(|m| m["path"] == json!(["Window"])).filter_map(|m| m["label"].as_str().map(str::to_string)).collect(); - for p in ["Events", "Lumetri Scopes", "Metadata", "Progress", "Reference Monitor", "Timecode"] { + for p in ["Events", "Lumetri Scopes", "Metadata", "Progress", "Project Notes", "Reference Monitor", "Timecode"] { assert!(window.iter().any(|w| w == p), "{p} not in Window: {window:?}"); } for (panel, key) in [ @@ -168,6 +168,7 @@ fn every_panel_opens_from_the_window_menu() { ("Events", "events.clearAll"), ("Progress", "progress.showFinished"), ("ReferenceMonitor", "reference.gang"), + ("ProjectNotes", "notes.text"), ] { d.ok("ui.menu.invoke", json!({"id": format!("window.panel.{panel}")})); d.frames(3); @@ -267,6 +268,31 @@ fn metadata_panel_edits_a_field_with_undo() { assert_eq!(d.label("metadata.field.TapeName"), "A001"); } +/// #620: notes typed into Project Notes land in the project; one typing session is one undo step. +#[test] +fn project_notes_panel_types_into_the_project_with_undo() { + let mut d = Driver::demo(); + d.ok("ui.menu.invoke", json!({"id": "window.panel.ProjectNotes"})); + d.frames(3); + let undo0 = d.app().session.history.undo.len(); + d.click("notes.text"); + d.ok("ui.type", json!({"text": "Fix logo"})); + d.frames(2); + d.ok("ui.key", json!({"key": "Enter"})); + d.ok("ui.type", json!({"text": "at 1:02"})); + d.frames(3); + assert_eq!(d.app().session.project.notes, "Fix logo\nat 1:02"); + assert_eq!(d.app().session.history.undo.len(), undo0 + 1); + assert_eq!(d.label("notes.text"), "Fix logo\nat 1:02"); + d.exec("edit.undo", json!({})); + d.frames(2); + assert_eq!(d.app().session.project.notes, ""); + // an agent writes them, the panel shows them + d.exec("project.setNotes", json!({"text": "From the script"})); + d.frames(2); + assert_eq!(d.label("notes.text"), "From the script"); +} + #[test] fn timecode_panel_rows_and_modes() { let mut d = Driver::demo(); diff --git a/crates/ui-egui/tests/project_panel_ui.rs b/crates/ui-egui/tests/project_panel_ui.rs index 3fac337e5..d4ee713e6 100644 --- a/crates/ui-egui/tests/project_panel_ui.rs +++ b/crates/ui-egui/tests/project_panel_ui.rs @@ -485,6 +485,41 @@ fn select_all_takes_only_the_shown_bin() { assert_eq!(sorted(&d.app().session.state.project_selection), footage); } +#[test] +fn a_dragged_bin_nests_in_another_and_back_out() { + // #456: a bin stayed wherever it was created; dragging it onto another bin did nothing + let mut d = project_driver(); + d.exec("project.view.set", json!({"view": "list"})); + let outer = d.exec("file.newBin", json!({"name": "Outer"}))["bin"].as_u64().unwrap(); + let inner = d.exec("file.newBin", json!({"name": "Inner"}))["bin"].as_u64().unwrap(); + d.app().ui.expanded_bins.clear(); + d.frames(3); + let parent = + |d: &mut Driver, b: u64| filmcraft_ui_egui::panels::project::parent_bin(&d.app().session.project.root, filmcraft_project::BinId(b)).map(|p| p.0); + let root = d.app().session.project.root.id.0; + let drag = |d: &mut Driver, from: [f32; 4], to: (f32, f32)| { + let start = (from[0] + 80.0, from[1] + from[3] / 2.0); + d.ok("ui.drag", json!({"from": {"x": start.0, "y": start.1}, "to": {"x": to.0, "y": to.1}, "steps": 12})); + d.frames(3); + }; + // onto the Outer row: Inner nests in it + let (from, to) = (d.rect(&format!("project.bin.{inner}")), d.rect(&format!("project.bin.{outer}"))); + drag(&mut d, from, (to[0] + 80.0, to[1] + to[3] / 2.0)); + assert_eq!(parent(&mut d, inner), Some(outer)); + // a bin never goes into itself + d.app().ui.expanded_bins.push(outer); + d.frames(3); + let (from, to) = (d.rect(&format!("project.bin.{outer}")), d.rect(&format!("project.bin.{inner}"))); + drag(&mut d, from, (to[0] + 80.0, to[1] + to[3] / 2.0)); + assert_eq!(parent(&mut d, outer), Some(root)); + assert_eq!(parent(&mut d, inner), Some(outer)); + // onto empty space: back to the bin the view shows + let from = d.rect(&format!("project.bin.{inner}")); + let to = centre(d.rect("project.empty")); + drag(&mut d, from, to); + assert_eq!(parent(&mut d, inner), Some(root)); +} + #[test] fn footer_buttons_are_wired() { let mut d = project_driver(); @@ -515,6 +550,26 @@ fn footer_buttons_are_wired() { assert_eq!(d.app().session.prefs.project_panel.view.icon_sort.column, "Name"); } +#[test] +fn new_bin_asks_for_its_name() { + // #456: New Bin opens the name field of the new bin instead of leaving it "New Bin" + let mut d = project_driver(); + d.exec("project.view.set", json!({"view": "list"})); + d.click("project.button.file.newBin"); + d.frames(2); + let bin = d.app().ui.project_panel.rename.as_ref().and_then(|r| r.bin).expect("the new bin's name is being edited"); + assert!(d.has("project.rename"), "the name field is shown"); + d.key("Cmd+A"); + d.ok("ui.type", json!({"text": "Interviews"})); + d.key("Enter"); + let name = d.app().session.project.root.find_bin(filmcraft_project::BinId(bin)).unwrap().name.clone(); + assert_eq!(name, "Interviews"); + // with a name (agents) the bin is named directly and nothing is edited + let r = d.ok("ui.menu.invoke", json!({"id": "file.newBin", "params": {"name": "B-roll"}})); + assert!(r["bin"].as_u64().is_some(), "{r}"); + assert!(d.app().ui.project_panel.rename.is_none()); +} + // ------------------------------------------------------------------------------------ Media Browser fn media_dir(tag: &str) -> std::path::PathBuf { @@ -665,6 +720,123 @@ fn media_browser_drag_imports_into_the_project_panel() { let _ = std::fs::remove_dir_all(&dir); } +/// Double-clicking empty space in the Project panel opens Import, also between and beside the +/// Icon view's cards once the project has some. Only the strip under the last row used to answer, +/// so with a project loaded a double-click in the visible blank space did nothing. Real pointer +/// input at 60 frames per second, so the two clicks are a real double-click. +#[test] +fn double_click_empty_area_imports_with_a_project_loaded() { + fn driver(demo: bool) -> (Driver, std::sync::Arc) { + let mut session = Session::default(); + if demo { + session.execute("file.openDemoProject", json!({})).expect("demo project"); + } + let (tx, rx) = channel(); + let mut app = FilmcraftApp::new(session).with_control(rx); + let picks = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let seen = picks.clone(); + app.hooks.pick_files = Some(Box::new(move |_| { + seen.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + Vec::new() + })); + let harness = Harness::builder().with_size(egui::vec2(1600.0, 980.0)).with_step_dt(1.0 / 60.0).with_max_steps(10_000).build_eframe(move |_cc| app); + let mut d = Driver { harness, tx, snapshots: None }; + d.frames(10); + (d, picks) + } + fn double_click(d: &mut Driver, at: egui::Pos2) { + let push = |d: &mut Driver, e: egui::Event| d.harness.input_mut().events.push(e); + // a pause first, as a person makes: clicks closer together continue the last gesture + // (a triple click) instead of starting a double-click + d.frames(60); + push(d, egui::Event::PointerMoved(at)); + d.frames(1); + for _ in 0..2 { + for pressed in [true, false] { + push(d, egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }); + d.frames(1); + } + } + d.frames(3); + } + let count = |p: &std::sync::Arc| p.load(std::sync::atomic::Ordering::SeqCst); + + // an empty project: the whole panel is the empty area + let (mut d, picks) = driver(false); + let r = d.rect("project.empty"); + double_click(&mut d, egui::pos2(r[0] + r[2] / 2.0, r[1] + r[3] / 2.0)); + assert_eq!(count(&picks), 1, "empty project"); + + // the demo project, Icon view: the gap between the first two bins, and the space right of the + // last card in the first row + let (mut d, picks) = driver(true); + d.ok("ui.set", json!({})); + let (a, b) = (d.rect("project.bin.1"), d.rect("project.bin.2")); + assert!((a[1] - b[1]).abs() < 1.0 && b[0] > a[0] + a[2], "bins 1 and 2 share a row: {a:?} {b:?}"); + double_click(&mut d, egui::pos2((a[0] + a[2] + b[0]) / 2.0, a[1] + a[3] / 2.0)); + assert_eq!(count(&picks), 1, "between two bins"); + let row: Vec<[f32; 4]> = d.ids("project.bin.").iter().map(|id| d.rect(id)).filter(|r| (r[1] - a[1]).abs() < 1.0).collect(); + let last = row.iter().fold(a, |m, r| if r[0] > m[0] { *r } else { m }); + let panel = d.rect("project.count"); + if last[0] + last[2] + 30.0 < panel[0] + panel[2] { + double_click(&mut d, egui::pos2(last[0] + last[2] + 20.0, last[1] + last[3] / 2.0)); + assert_eq!(count(&picks), 2, "right of the last card"); + } + // a card still takes its own double-click: the bin opens, Import does not + double_click(&mut d, egui::pos2(a[0] + a[2] / 2.0, a[1] + a[3] / 2.0)); + let picked = count(&picks); + assert!(picked <= 2, "double-clicking a bin must not open Import"); +} + +/// The report: with audio placed in the timeline, double-clicking the Project panel's empty space +/// no longer opened Import. A new sequence with a tone on A1 (so the panel shows the sequence and +/// the audio clip as cards); every empty spot opens Import: the strip under the cards and the space +/// beside them. +#[test] +fn double_click_imports_with_audio_in_the_timeline() { + let mut session = Session::default(); + session.execute("file.newSequence", json!({"name": "Mix", "audio": 2, "video": 1})).unwrap(); + let inter: Vec = (0..48_000).flat_map(|i| [(i as f32 * 0.0576).sin() * 0.5; 2]).collect(); + let bytes: std::sync::Arc<[u8]> = filmcraft_engine::previews::write_wav_f32(&inter, 48_000).into(); + let item = filmcraft_engine::commands::import_bytes(&mut session, "/tone.wav", bytes, None).unwrap(); + session.execute("timeline.place", json!({"item": item.0, "audioTrack": "A1", "seconds": 0.0})).unwrap(); + let (tx, rx) = channel(); + let mut app = FilmcraftApp::new(session).with_control(rx); + let picks = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let seen = picks.clone(); + app.hooks.pick_files = Some(Box::new(move |_| { + seen.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + Vec::new() + })); + let harness = Harness::builder().with_size(egui::vec2(1600.0, 980.0)).with_step_dt(1.0 / 60.0).with_max_steps(10_000).build_eframe(move |_cc| app); + let mut d = Driver { harness, tx, snapshots: None }; + d.frames(10); + let double_click = |d: &mut Driver, at: egui::Pos2| { + let push = |d: &mut Driver, e: egui::Event| d.harness.input_mut().events.push(e); + d.frames(60); + push(d, egui::Event::PointerMoved(at)); + d.frames(1); + for _ in 0..2 { + for pressed in [true, false] { + push(d, egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }); + d.frames(1); + } + } + d.frames(3); + }; + let count = || picks.load(std::sync::atomic::Ordering::SeqCst); + let cards: Vec<[f32; 4]> = d.ids("project.item.").iter().filter(|id| id.matches('.').count() == 2).map(|id| d.rect(id)).collect(); + assert!(!cards.is_empty(), "the sequence and the tone show as cards"); + let empty = d.rect("project.empty"); + eprintln!("cards {cards:?}, empty strip {empty:?}"); + double_click(&mut d, egui::pos2(empty[0] + empty[2] / 2.0, empty[1] + empty[3] / 2.0)); + let strip = count(); + let last = cards.iter().fold(cards[0], |m, r| if r[0] > m[0] { *r } else { m }); + double_click(&mut d, egui::pos2(last[0] + last[2] + 40.0, last[1] + last[3] / 2.0)); + let beside = count() - strip; + assert_eq!((strip, beside), (1, 1), "double-click on (the strip under the cards, the space beside them)"); +} + // ---- marquee selection (#578) /// The Footage bin open in place in `view`, with its items' rects (row by row, left to right). diff --git a/crates/ui-egui/tests/sequence_ui.rs b/crates/ui-egui/tests/sequence_ui.rs index 0a16547f8..460b7929a 100644 --- a/crates/ui-egui/tests/sequence_ui.rs +++ b/crates/ui-egui/tests/sequence_ui.rs @@ -321,6 +321,32 @@ fn markers_panel_colour_filter() { assert_eq!(d.ids("markers.row.").len(), 2); } +/// #695: a marker on the Timeline could not be cleared: with snapping off a click on it left the +/// playhead on the frame under the pointer, not on the marker Clear Selected Marker looks for, and +/// right-clicking it opened the clip menu. +#[test] +fn a_clicked_timeline_marker_can_be_cleared() { + let mut d = Driver::demo(); + d.exec("markers.clearAll", json!({})); + let first = d.exec("markers.add", json!({"frame": 30}))["marker"].as_u64().unwrap(); + let second = d.exec("markers.add", json!({"frame": 90}))["marker"].as_u64().unwrap(); + d.exec("sequence.snap", json!({"on": false})); + d.exec("playhead.set", json!({"frame": 0})); + // zoomed out so that a few px beside a marker is another frame + let v = &mut d.app().ui.timeline; + (v.pps, v.target_pps, v.scroll, v.target_scroll) = (60.0, 60.0, 0.0, 0.0); + d.frames(4); + let markers = |d: &mut Driver| d.app().session.active_sequence().unwrap().markers.iter().map(|m| m.id.0).collect::>(); + d.ok("ui.click", json!({"id": format!("timeline.marker.{first}"), "fx": 0.95})); + d.frames(2); + d.exec("markers.clearCurrent", json!({})); + assert_eq!(markers(&mut d), vec![second], "the clicked marker is cleared"); + d.ok("ui.click", json!({"id": format!("timeline.marker.{second}"), "fx": 0.95, "button": "right"})); + d.frames(3); + d.click("timeline.markerMenu.markers.clearCurrent"); + assert!(markers(&mut d).is_empty(), "cleared from its context menu"); +} + #[test] fn shift_semicolon_goes_to_the_next_gap() { let mut d = Driver::demo(); diff --git a/crates/ui-egui/tests/timeline_rename_ui.rs b/crates/ui-egui/tests/timeline_rename_ui.rs deleted file mode 100644 index aa1ef4c94..000000000 --- a/crates/ui-egui/tests/timeline_rename_ui.rs +++ /dev/null @@ -1,46 +0,0 @@ -//! Right-click renaming a Timeline track uses timeline.setTrack, which is undoable. - -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 click(h: &mut Harness<'static, FilmcraftApp>, id: &str, button: egui::PointerButton) { - let [x, y, w, height] = h.state().auto.find(id).expect("visible control").rect; - let at = egui::pos2(x + w * 0.5, y + height * 0.5); - h.input_mut().events.push(egui::Event::PointerMoved(at)); - step(h); - h.input_mut().events.push(egui::Event::PointerButton { pos: at, button, pressed: true, modifiers: Default::default() }); - step(h); - h.input_mut().events.push(egui::Event::PointerButton { pos: at, button, pressed: false, modifiers: Default::default() }); - step(h); -} - -#[test] -fn rename_from_video_track_header_is_undoable() { - let mut s = Session::default(); - s.execute("file.openDemoProject", json!({})).expect("demo project"); - let mut h = Harness::builder().with_size(egui::vec2(1600.0, 980.0)).with_step_dt(1.0 / 60.0).build_eframe(move |_cc| FilmcraftApp::new(s)); - for _ in 0..5 { - step(&mut h); - } - let original = h.state().session.active_sequence().expect("active sequence").video_tracks[0].name.clone(); - let id = h.state().session.active_sequence().expect("active sequence").video_tracks[0].id.0; - click(&mut h, "timeline.track.V1.name", egui::PointerButton::Secondary); - assert!(h.state().auto.find("timeline.track.V1.renameField").is_some(), "track rename popup should open"); - h.ctx.data_mut(|d| d.insert_temp(egui::Id::new(("timeline-track-rename", id)), "B-roll".to_string())); - step(&mut h); - click(&mut h, "timeline.track.V1.renameApply", egui::PointerButton::Primary); - assert_eq!(h.state().session.active_sequence().expect("active sequence").video_tracks[0].name, "B-roll"); - h.state_mut().session.execute("edit.undo", json!({})).expect("undo rename"); - assert_eq!(h.state().session.active_sequence().expect("active sequence").video_tracks[0].name, original); -} diff --git a/crates/ui-egui/tests/timeline_trim_ui.rs b/crates/ui-egui/tests/timeline_trim_ui.rs index bac77555f..c68d56129 100644 --- a/crates/ui-egui/tests/timeline_trim_ui.rs +++ b/crates/ui-egui/tests/timeline_trim_ui.rs @@ -299,6 +299,72 @@ fn dragging_a_caption_edge_inward_trims_it() { assert!(e1 < e0, "the caption's end moved in: {e0} → {e1}"); } +/// #374: renders the Ambient_Score music clip mid-way through a trim of its In edge, with the +/// mouse button still down, for visual review (needs a GPU): `cargo test -p filmcraft-ui-egui +/// --test timeline_trim_ui -- --ignored trim_waveform_screenshots`. The waveform stays where it was +/// and is cut off at the moving edge; it used to squeeze the whole clip's waveform into the shorter +/// clip, so the sound under the pointer moved while dragging. +#[test] +#[ignore] +fn trim_waveform_screenshots() { + let mut d = Driver::demo(); + let seq = d.app().session.active_sequence().expect("sequence"); + let music = seq.audio_tracks.iter().flat_map(|t| &t.items).find(|i| i.name.contains("Ambient")).expect("music clip").id.0; + let (x0, _, y) = d.edges(music); + // waveform peaks are read on a background thread + for _ in 0..40 { + std::thread::sleep(std::time::Duration::from_millis(50)); + d.frames(1); + } + d.screenshot("trim-waveform-before"); + let push = |d: &mut Driver, e: egui::Event| d.harness.input_mut().events.push(e); + let to = pos2(x0 + 160.0, y); + push(&mut d, egui::Event::PointerMoved(pos2(x0, y))); + d.frames(1); + push(&mut d, egui::Event::PointerButton { pos: pos2(x0, y), button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() }); + d.frames(1); + for i in 1..=20 { + push(&mut d, egui::Event::PointerMoved(pos2(x0 + 160.0 * i as f32 / 20.0, y))); + d.frames(1); + } + d.screenshot("trim-waveform-dragging"); + push(&mut d, egui::Event::PointerButton { pos: to, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }); + d.frames(2); + d.screenshot("trim-waveform-released"); +} + +/// #653: an Out edge dragged far past the end of the clip's media stops there while the drag is +/// still in progress, at the same place the trim lands on release. It used to follow the pointer +/// past the media and then jump back when the button came up. +#[test] +fn an_edge_dragged_past_the_media_stops_at_its_end() { + use filmcraft_ui_egui::panels::timeline::Drag; + let mut d = Driver::demo(); + let last = d.app().session.active_sequence().expect("sequence").video_tracks[0].items.last().expect("a clip on V1").id.0; + let (_, x1, y) = d.edges(last); + let (_, e0) = d.span(last); + let far = filmcraft_time::Tick(3600 * filmcraft_time::TICKS_PER_SECOND); + let limit = filmcraft_engine::commands::trim_delta(&d.app().session, ClipId(last), filmcraft_edit::Edge::Out, filmcraft_edit::TrimMode::Regular, far) + .expect("limit"); + let (from, to) = (pos2(x1 - 3.0, y), pos2(x1 + 300.0, y)); + let push = |d: &mut Driver, e: egui::Event| d.harness.input_mut().events.push(e); + push(&mut d, egui::Event::PointerMoved(from)); + d.frames(1); + push(&mut d, egui::Event::PointerButton { pos: from, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() }); + d.frames(1); + for i in 1..=60 { + push(&mut d, egui::Event::PointerMoved(from + (to - from) * (i as f32 / 60.0))); + d.frames(1); + } + match &d.app().tl.drag { + Some(Drag::Trim { delta, .. }) => assert_eq!(*delta, limit, "the dragged edge stops at the end of the media"), + other => panic!("not trimming: {other:?}"), + } + push(&mut d, egui::Event::PointerButton { pos: to, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }); + d.frames(2); + assert_eq!(d.span(last).1, e0 + limit.0, "released where it was shown"); +} + /// Dragging a picture in the Trim Monitor must not deadlock: the shift bookkeeping used to call /// `drag_delta()` inside `data_mut()`, and both take egui's non-reentrant context write-lock, so /// the drag froze the whole app on its first recognised frame (the delta is read before locking diff --git a/docs/architecture.md b/docs/architecture.md index 0c7908bb5..24a62065a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -731,7 +731,8 @@ All of these dispatch the same command ids. - **Panels with engine data** (M8.9 / M12.6): Lumetri Scopes (`scopes.read` returns the same scopes as numbers), Metadata (`metadata.get` / `metadata.set`, one undo step per edit, stored in `ProjectItem::metadata`), Events (`events.list` / `events.clear`), Progress (`jobs.list` / - `jobs.cancel`). Timecode and Reference Monitor are frontend-only views. Their settings are + `jobs.cancel`), Project Notes (`project.notes` / `project.setNotes`, plain text in + `Project::notes`, schema v15; one typing session is one undo step). Timecode and Reference Monitor are frontend-only views. Their settings are `UiState::panels` (`ui.set {"panels": {...}}`). - **Project panel and Media Browser** (M12.7). View settings (List / Icon / Freeform, thumbnail and font size, Preview Area, Hover Scrub), the List view's columns, widths and sort, and ten view