From 5ed4952d43f6328d9c2bf7990f1c2e15dfa25011 Mon Sep 17 00:00:00 2001 From: Whatever Industries <273006022+whatever-industries@users.noreply.github.com> Date: Sat, 31 Oct 2026 19:04:14 -0700 Subject: [PATCH] Add shared custom colors and complete marker editing (#1435) * M3: Add shared custom colors and complete marker editing * #1435: refine native colors and range endpoint flags Use the macOS Colors panel for custom label colors and reuse the established egui picker on other hosts. Preserve draft confirmation, exact sRGB values, native text shortcuts and project ownership. Compact the swatches and center the add control, split range glyphs across their endpoints, localize the new controls, protect newer marker edits from stale dialogs, and cover the behavior with rendered UI and hit-test regressions. Co-authored-by: Codex * #1435: preserve imported palettes and marker double-click ownership Union saved OTIO palettes across timelines with deduplication, bounded capacity, first-name preservation and explicit overflow warnings. Keep null authors consistent with omitted fields, and consume marker double-clicks before the work-area gesture so opening the editor does not alter project history. Correct the CSV URL column and schema documentation. Add six regressions for palette retention/capacity, author handling, and physical overlapping gestures. Validation: full cargo xtask ci (3666 passed, 45 skipped), 42 rendered UI tests, and strict workspace/all-target Clippy on Rust 1.99.1. Co-authored-by: Codex * #1435: retain OTIO swatch names and generator labels Ignore orphan name entries when retaining saved palette names so input ordering does not hide valid names after the first 64 entries. Keep names bounded by the retained palette and preserve existing valid names. Include generator preset labels alongside custom label colors in OTIO metadata. Add regressions for orphan-prefixed name maps and three generator kinds with preset/custom colors through actual export/import round trips. Validation: both regressions failed before fixes; all 72 interchange tests and full cargo xtask ci pass (3668 tests, 45 skipped), plus strict workspace/all-target Clippy on Rust 1.99.1. Co-authored-by: Codex * #1435: preserve distinct OTIO generator item identity Use exported FilmCraft item IDs for generator media identity so identical content can retain distinct source names, labels and custom colors. Repeated references to one source item still share a single imported media entry. Preserve content-based fallback for missing or invalid item IDs. Cover black video, bars and tone, and color mattes with distinct items/shared references, plus legacy and malformed identity metadata. Validation: distinct-generator regression failed before fix; legacy compatibility passed. All 74 interchange tests and final full cargo xtask ci pass (3670 tests, 45 skipped); strict workspace/all-target Clippy also passes on Rust 1.99.1. Co-authored-by: Codex * #1435: align Edit Marker layout and add safe marker navigation Give the editor a wide aligned form, larger Notes area, side action column, and grouped palette with vertical marker types. Expand the Web Link field with the form and keep actions accessible in small windows. Previous/Next saves valid edits as one Undo step and stays with the original sequence/source owner, including coincident markers. Invalid drafts stay open; unchanged navigation and Done add no history. Keep native/shared color pickers, Author, Notes, URL validation and immediate marker creation. Validation: all 45 rendered sequence/marker UI tests pass, including five new navigation/owner/validation/layout regressions. Native macOS layout, picker Cancel and exact single-step Undo restoration verified. Full standard cargo xtask ci passes 3675 tests (45 skipped), doctests and all remaining gates; strict Rust 1.99.1 workspace/all-target Clippy also passes. Co-authored-by: Codex * #1435: refine Edit Marker proportions and action alignment Narrow the dialog to 680 logical pixels and keep the core order Name, Time/Duration, Notes, Color and marker type. Place Author after the type-specific options as secondary metadata. Preserve our palette, custom picker and range controls. Use consistently sized, centered rounded action buttons with a primary Done action. Shorten Delete Marker to Delete and remove the navigation helper and unused translations. Keep the existing navigation save/validation/Undo semantics and automation IDs. Validation: all 45 rendered marker/sequence UI tests pass, including small-window action accessibility. Actual macOS layout and no-op Previous/Next/Cancel verified. Full standard cargo xtask ci passes 3675 tests (45 skipped), doctests, formatting, Clippy, translations, layers, assets and 49 WASM crates. Strict Rust 1.99.1 workspace/all-target Clippy passes separately. Co-authored-by: Codex * #1435: tighten marker layout and validate text editing Use a narrower aligned form with Author below Name, bounded scrolling Notes, left-aligned colors, centered actions and an inline Web Link URL/open icon. Remove the legacy Flash Cue choice while preserving imported marker types. Constrain timecode typing/paste to complete numeric colon-separated fields, validate drop-frame labels and frame bounds, and disable Done for invalid timing. Preserve high-frame-rate timecodes and point-marker semantics when Duration is unchecked. Route native/menu clipboard and text Undo commands to focused fields; explicit engine commands retain project Undo and selection semantics. Validation: standard cargo xtask ci passes 3679 tests (45 standard skips), doctests, fmt, strict workspace/all-target Clippy, i18n, layers, 69 assets and 49 WASM crates. Also passed 48 rendered marker/sequence tests and strict Rust 1.99.1 Clippy. Actual macOS clipboard, bounded Notes, point save and exact Undo were checked. Windows/Linux desktop interaction remains pending. Co-authored-by: Codex * M0: Wait for frame-worker recovery before asserting Do not treat a nonempty request queue as proof of worker death in the panic-recovery regression. Keep waiting for all injected faults and enough post-fault completions, then drain the queue without adding more seeks. Retain the exact fault-count, successful-recovery and empty-queue assertions and bound the wait with a two-minute deadline. The inherited test failed twice in independent fork CI. It did not reproduce locally: baseline passed 20 normal and 10 delayed-report runs; the repair passed 50 normal and 10 delayed-report runs. No product or marker UI changes. Validation: standard cargo xtask ci passed all 4485 tests (52 standard skips), doctests, formatting, strict workspace/all-target Clippy, i18n, layers, 69 assets and 49 WASM crates. Assisted-by: OpenAI Codex --------- Co-authored-by: Codex Co-authored-by: Codex --- Cargo.lock | 45 +- Cargo.toml | 1 + ROADMAP.md | 2 + apps/filmcraft/Cargo.toml | 2 +- apps/filmcraft/src/main.rs | 5 +- apps/filmcraft/src/native_color.rs | 103 ++ apps/filmcraft/src/native_menu.rs | 53 +- contributors/people.toml | 4 + crates/edit/src/frame_size.rs | 1 + crates/edit/src/tests.rs | 1 + crates/edit/src/through.rs | 1 + crates/edit/src/transcript/tests.rs | 1 + crates/edit/src/transitions.rs | 1 + crates/engine/src/clip_ops/audio_options.rs | 3 + crates/engine/src/clip_ops/common.rs | 2 +- crates/engine/src/clip_ops/edit_menu.rs | 12 +- crates/engine/src/clip_ops/file.rs | 1 + crates/engine/src/clip_ops/replace.rs | 1 + crates/engine/src/clip_ops/subclip.rs | 9 + crates/engine/src/clip_ops_tests.rs | 3 + crates/engine/src/commands.rs | 236 ++-- crates/engine/src/demo.rs | 3 + crates/engine/src/lib.rs | 7 + crates/engine/src/marker_edit.rs | 173 +++ crates/engine/src/marker_export.rs | 46 +- crates/engine/src/multicam_tests.rs | 3 + crates/engine/src/palette.rs | 85 ++ crates/engine/src/panels.rs | 21 +- crates/engine/src/project_panel.rs | 4 +- crates/engine/src/project_tools.rs | 1 + crates/engine/src/scene_detect.rs | 3 + crates/engine/src/sequence_extras.rs | 7 +- crates/engine/src/sequence_tools.rs | 83 +- crates/engine/src/sequence_tools_tests.rs | 7 +- crates/engine/src/source_monitor.rs | 58 +- crates/engine/tests/marker_properties.rs | 520 ++++++++ crates/format/src/lib.rs | 20 +- crates/format/tests/fixtures.rs | 3 + crates/interchange/src/aaf.rs | 1 + crates/interchange/src/common.rs | 1 + crates/interchange/src/comp.rs | 3 + crates/interchange/src/edl.rs | 4 + crates/interchange/src/fcp7.rs | 4 + crates/interchange/src/fcpxml.rs | 4 + crates/interchange/src/lib.rs | 52 + crates/interchange/src/omf.rs | 1 + crates/interchange/src/otio.rs | 63 +- crates/interchange/tests/it/aaf_omf.rs | 6 + crates/interchange/tests/it/api.rs | 60 + crates/interchange/tests/it/fcp7.rs | 6 + crates/interchange/tests/it/fcpxml.rs | 6 + .../tests/it/media_marker_export.rs | 3 + .../tests/it/nested_sequence_markers.rs | 6 + crates/interchange/tests/it/otio.rs | 273 +++- crates/interchange/tests/it/point_markers.rs | 6 + crates/project/Cargo.toml | 1 + crates/project/src/essential.rs | 1 + crates/project/src/find.rs | 2 +- crates/project/src/item.rs | 3 + crates/project/src/label.rs | 17 +- crates/project/src/lib.rs | 17 +- crates/project/src/marker_color.rs | 82 ++ crates/project/src/marker_url.rs | 53 + crates/project/src/track.rs | 13 + crates/ui-egui/README.md | 73 ++ crates/ui-egui/src/control.rs | 5 +- crates/ui-egui/src/hooks.rs | 2 + crates/ui-egui/src/i18n/de.tsv | 64 + crates/ui-egui/src/i18n/es.tsv | 59 + crates/ui-egui/src/i18n/fr.tsv | 74 +- crates/ui-egui/src/i18n/id.tsv | 58 + crates/ui-egui/src/i18n/it.tsv | 59 + crates/ui-egui/src/i18n/ja.tsv | 59 + crates/ui-egui/src/i18n/ko.tsv | 59 + crates/ui-egui/src/i18n/pt-br.tsv | 58 + crates/ui-egui/src/i18n/ru.tsv | 62 + crates/ui-egui/src/i18n/uk.tsv | 79 ++ crates/ui-egui/src/i18n/zh-cn.tsv | 59 + crates/ui-egui/src/icons.rs | 8 + crates/ui-egui/src/menus.rs | 58 +- crates/ui-egui/src/panels.rs | 6 + crates/ui-egui/src/panels/custom_colors.rs | 132 ++ crates/ui-egui/src/panels/dialogs.rs | 4 +- .../ui-egui/src/panels/marker_properties.rs | 628 ++++++++++ crates/ui-egui/src/panels/markers_panel.rs | 225 ++++ crates/ui-egui/src/panels/media_browser.rs | 1 + crates/ui-egui/src/panels/misc.rs | 319 ++--- crates/ui-egui/src/panels/monitor.rs | 77 +- crates/ui-egui/src/panels/palette_manager.rs | 124 ++ crates/ui-egui/src/panels/project_views.rs | 19 +- crates/ui-egui/src/panels/timeline.rs | 2 - crates/ui-egui/src/panels/timeline/clips.rs | 16 +- .../ui-egui/src/panels/timeline/interact.rs | 17 +- .../src/panels/timeline/marker_dialog.rs | 118 -- crates/ui-egui/src/panels/timeline/menus.rs | 2 +- crates/ui-egui/src/panels/timeline/ruler.rs | 24 +- crates/ui-egui/src/panels/timeline_hit.rs | 22 +- crates/ui-egui/src/panels/timeline_volume.rs | 1 + crates/ui-egui/src/state.rs | 35 +- crates/ui-egui/src/widgets.rs | 24 + crates/ui-egui/src/widgets/color_picker.rs | 191 +++ crates/ui-egui/tests/it/marker_edit_ui.rs | 24 +- crates/ui-egui/tests/it/sequence_ui.rs | 1102 ++++++++++++++++- crates/ui-egui/tests/it/source_playback_ui.rs | 10 +- crates/ui-egui/tests/robustness_ui.rs | 18 +- docs/agents.md | 86 +- docs/project-files.md | 15 +- 107 files changed, 5602 insertions(+), 695 deletions(-) create mode 100644 apps/filmcraft/src/native_color.rs create mode 100644 crates/engine/src/marker_edit.rs create mode 100644 crates/engine/src/palette.rs create mode 100644 crates/engine/tests/marker_properties.rs create mode 100644 crates/project/src/marker_color.rs create mode 100644 crates/project/src/marker_url.rs create mode 100644 crates/ui-egui/src/panels/custom_colors.rs create mode 100644 crates/ui-egui/src/panels/marker_properties.rs create mode 100644 crates/ui-egui/src/panels/markers_panel.rs create mode 100644 crates/ui-egui/src/panels/palette_manager.rs delete mode 100644 crates/ui-egui/src/panels/timeline/marker_dialog.rs create mode 100644 crates/ui-egui/src/widgets/color_picker.rs diff --git a/Cargo.lock b/Cargo.lock index 2c7dd0631..f70d2fe4f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2371,6 +2371,7 @@ dependencies = [ "serde", "serde_json", "thiserror 2.0.21", + "url", ] [[package]] @@ -2668,6 +2669,15 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "aa9a19cbb55df58761df49b23516a86d432839add4af60fc256da840f66ed35b" +[[package]] +name = "form_urlencoded" +version = "1.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb4cb245038516f5f85277875cdaa4f7d2c9a0fa0468de06ed190163b1581fcf" +dependencies = [ + "percent-encoding", +] + [[package]] name = "futures" version = "0.3.34" @@ -3634,6 +3644,27 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" +[[package]] +name = "idna" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b0875f23caa03898994f6ddc501886a45c7d3d62d04d2d90788d47be1b1e4de" +dependencies = [ + "idna_adapter", + "smallvec", + "utf8_iter", +] + +[[package]] +name = "idna_adapter" +version = "1.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb68373c0d6620ef8105e855e7745e18b0d00d3bdb07fb532e434244cdb9a714" +dependencies = [ + "icu_normalizer", + "icu_properties", +] + [[package]] name = "image" version = "0.25.10" @@ -4715,7 +4746,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7d8fae84b431384b68627d0f9b3b1245fcf9f46f6c0e3dc902e9dce64edd1967" dependencies = [ "libc", - "windows-sys 0.45.0", + "windows-sys 0.61.2", ] [[package]] @@ -6933,6 +6964,18 @@ dependencies = [ "log", ] +[[package]] +name = "url" +version = "2.5.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ff67a8a4397373c3ef660812acab3268222035010ab8680ec4215f38ba3d0eed" +dependencies = [ + "form_urlencoded", + "idna", + "percent-encoding", + "serde", +] + [[package]] name = "utf16_iter" version = "1.0.5" diff --git a/Cargo.toml b/Cargo.toml index cdc02167d..e8050869f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -66,6 +66,7 @@ filmcraft-testkit = { path = "crates/testkit" } thiserror = "2" serde = { version = "1", features = ["derive", "rc"] } serde_json = "1" +url = "2.5.8" log = "0.4" # All date/time conversion goes through chrono (no hand-rolled calendar code beyond the 400-year range extension; AGENTS.md). chrono = { version = "0.4.45", default-features = false, features = ["std"] } diff --git a/ROADMAP.md b/ROADMAP.md index d06963a1b..d3fcd3f34 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1,5 +1,7 @@ # FilmCraft Roadmap +- M3: Markers and clips share the existing sixteen clip presets plus a project-local custom hex palette. A reusable wheel/hex editor defines colors separately from marker editing; timeline markers expose Edit Marker with Name, Author and Notes. Marker/clip colors, palette edits and authors persist with undo and project saves. AVB marker import remains separate. + - UI.6: Compact Source and Program monitors separate timecodes from view settings and retain Source drag choices. Transport controls use the shared toolbar sizing helper, with overflow actions in an operable More menu and a scrolling fallback at tiny widths. - UI.3: Panel tabs, edge splits, tab reordering and floating groups use the shared docking model and drop resolver. Floating geometry is saved with named workspaces while legacy layout JSON remains readable. Dock dividers use bounded shared geometry and keyboard-accessible resize handles. Tools and dock tabs expose actual widget focus, paint focus outlines, and support keyboard traversal/activation. Control input reaches focused widgets in background windows; optional input isolation supports unattended validation. Existing workspace trees and saved layout formats are retained. diff --git a/apps/filmcraft/Cargo.toml b/apps/filmcraft/Cargo.toml index 9e3f1cba8..af3e15efd 100644 --- a/apps/filmcraft/Cargo.toml +++ b/apps/filmcraft/Cargo.toml @@ -49,7 +49,7 @@ pollster = "1" [target.'cfg(target_os = "macos")'.dependencies] muda = "0.21" objc2-foundation = { version = "0.3", default-features = false, features = ["std", "NSProcessInfo", "NSString", "NSArray"] } -objc2-app-kit = { version = "0.3", default-features = false, features = ["std", "NSApplication", "NSResponder", "NSWindow"] } +objc2-app-kit = { version = "0.3", default-features = false, features = ["std", "NSApplication", "NSResponder", "NSWindow", "NSPanel", "NSColorPanel", "NSColor", "NSColorSpace"] } objc2 = "0.6" # Same winit as eframe: macOS event-loop options (no app activation when driven by an agent). winit = { version = "0.30", default-features = false } diff --git a/apps/filmcraft/src/main.rs b/apps/filmcraft/src/main.rs index f5597cdcf..34eb7e249 100644 --- a/apps/filmcraft/src/main.rs +++ b/apps/filmcraft/src/main.rs @@ -35,6 +35,8 @@ mod file_filters; mod graphics; mod logging; #[cfg(target_os = "macos")] +mod native_color; +#[cfg(target_os = "macos")] mod native_menu; mod window_raise; @@ -360,7 +362,8 @@ fn app_creator(launch: Launch, graphics: graphics::GraphicsStart, created: Arc, + active: Rc>, + id: u64, + text_menu: crate::native_menu::NativeTextMenu, +} + +impl std::fmt::Debug for ColorSession { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ColorSession").field("id", &self.id).finish_non_exhaustive() + } +} + +impl ColorSession { + fn owns_panel(&self) -> bool { + self.active.get() == self.id + } +} + +impl Drop for ColorSession { + fn drop(&mut self) { + // NSColorPanel is shared. An older draft must never close a newer draft's panel. + if self.owns_panel() { + if let Err(error) = (self.text_menu)(false) { + log::warn!("Cannot restore FilmCraft Edit menu: {error}"); + } + self.panel.orderOut(None); + } + } +} + +fn color(rgb: [u8; 3]) -> Retained { + let [r, g, b] = rgb.map(|c| f64::from(c) / 255.0); + NSColor::colorWithSRGBRed_green_blue_alpha(r, g, b, 1.0) +} + +fn channel(value: f64) -> Result { + if !value.is_finite() { + return Err("The selected color has an invalid component".into()); + } + Ok((value.clamp(0.0, 1.0) * 255.0).round() as u8) +} + +impl NativeColorSession for ColorSession { + fn poll(&self) -> Result { + if !self.owns_panel() { + return Ok(NativeColorState { color: None, open: false }); + } + (self.text_menu)(self.panel.isKeyWindow())?; + let rgb = self.panel.color().colorUsingColorSpace(&NSColorSpace::sRGBColorSpace()).ok_or("Cannot convert the selected color to sRGB")?; + let rgb = [channel(rgb.redComponent())?, channel(rgb.greenComponent())?, channel(rgb.blueComponent())?]; + Ok(NativeColorState { color: Some(rgb), open: self.panel.isVisible() }) + } + + fn set_color(&self, rgb: [u8; 3]) -> Result<(), String> { + if !self.owns_panel() { + return Err("This color picker belongs to an earlier draft".into()); + } + self.panel.setColor(&color(rgb)); + Ok(()) + } +} + +pub fn picker(text_menu: crate::native_menu::NativeTextMenu) -> NativeColorPicker { + let active = Rc::new(Cell::new(0_u64)); + Box::new(move |rgb| { + let mtm = MainThreadMarker::new().ok_or("The macOS color picker must open on the main thread")?; + let id = active.get().checked_add(1).ok_or("Too many color picker requests")?; + // Complete fallible setup before changing AppKit's shared panel or its owner. + text_menu(true)?; + let panel = NSColorPanel::sharedColorPanel(mtm); + panel.setShowsAlpha(false); + panel.setContinuous(true); + panel.setColor(&color(rgb)); + active.set(id); + panel.makeKeyAndOrderFront(None); + Ok(Rc::new(ColorSession { panel, active: active.clone(), id, text_menu: text_menu.clone() })) + }) +} + +#[cfg(test)] +mod tests { + use super::channel; + + #[test] + fn srgb_channels_are_rounded_bounded_and_finite() { + assert_eq!(channel(0.5).unwrap(), 128); + assert_eq!(channel(-1.0).unwrap(), 0); + assert_eq!(channel(2.0).unwrap(), 255); + assert!(channel(f64::NAN).is_err()); + assert!(channel(f64::INFINITY).is_err()); + for c in 0..=255_u8 { + assert_eq!(channel(f64::from(c) / 255.0).unwrap(), c); + } + } +} diff --git a/apps/filmcraft/src/native_menu.rs b/apps/filmcraft/src/native_menu.rs index ec2817bce..14895e260 100644 --- a/apps/filmcraft/src/native_menu.rs +++ b/apps/filmcraft/src/native_menu.rs @@ -1,7 +1,11 @@ //! Native macOS menu bar (muda), generated from the command registry — like Premiere, menus live in //! the system menu bar. Items send command ids to the app's command inbox. -use std::sync::mpsc::{Receiver, channel}; +use std::{ + cell::Cell, + rc::Rc, + sync::mpsc::{Receiver, channel}, +}; use filmcraft_ui_egui::FilmcraftApp; use filmcraft_ui_egui::menus::{MENUS, MenuItem as Item, NATIVE_APP_MENU, menu_items}; @@ -31,7 +35,11 @@ fn quit_item(app: &FilmcraftApp) -> (&'static str, Option) { /// Updates native item labels, key equivalents and enablement when the menu tree changes. pub type MenuUpdater = Box; -pub fn install(app: &FilmcraftApp, ctx: egui::Context) -> (Receiver, MenuUpdater) { +/// Swap only Edit while a native color window owns keyboard focus. Native predefined +/// actions go to AppKit's first responder, rather than FilmCraft's timeline inbox. +pub type NativeTextMenu = Rc Result<(), String>>; + +pub fn install(app: &FilmcraftApp, ctx: egui::Context) -> (Receiver, MenuUpdater, NativeTextMenu) { let items = menu_items(app); // each native item with the menu entry it currently shows, so updates touch only what changed let mut native: Vec<(Item, MenuItem)> = Vec::new(); @@ -80,6 +88,7 @@ pub fn install(app: &FilmcraftApp, ctx: egui::Context) -> (Receiver, Men &quit, ]); let _ = bar.append(&app_menu); + let mut edit_menu = None; for top in MENUS { let sub = Submenu::new(app.ui.language.tr(top), true); titles.push((top.into(), sub.clone())); @@ -106,8 +115,42 @@ pub fn install(app: &FilmcraftApp, ctx: egui::Context) -> (Receiver, Men } let _ = parent.append(&mi); } + if top == "Edit" { + edit_menu = Some(sub.clone()); + } let _ = bar.append(&sub); } + let native_edit = Submenu::new(app.ui.language.tr("Edit"), true); + let text_items = [ + ("Undo", PredefinedMenuItem::undo(Some(app.ui.language.tr("Undo")))), + ("Redo", PredefinedMenuItem::redo(Some(app.ui.language.tr("Redo")))), + ("Cut", PredefinedMenuItem::cut(Some(app.ui.language.tr("Cut")))), + ("Copy", PredefinedMenuItem::copy(Some(app.ui.language.tr("Copy")))), + ("Paste", PredefinedMenuItem::paste(Some(app.ui.language.tr("Paste")))), + ("Delete", PredefinedMenuItem::delete(Some(app.ui.language.tr("Delete")))), + ("Select All", PredefinedMenuItem::select_all(Some(app.ui.language.tr("Select All")))), + ]; + for (_, item) in &text_items { + let _ = native_edit.append(item); + } + let menu_bar = bar.clone(); + let color_edit = native_edit.clone(); + let native_shown = Cell::new(false); + let text_menu: NativeTextMenu = Rc::new(move |use_native| { + if use_native == native_shown.get() { + return Ok(()); + } + let regular = edit_menu.as_ref().ok_or("FilmCraft Edit menu is unavailable")?; + let index = MENUS.iter().position(|top| *top == "Edit").ok_or("FilmCraft Edit menu is unavailable")? + 1; + let (old, new) = if use_native { (regular, &color_edit) } else { (&color_edit, regular) }; + menu_bar.insert(new, index).map_err(|e| e.to_string())?; + if let Err(error) = menu_bar.remove(old) { + let _ = menu_bar.remove(new); + return Err(error.to_string()); + } + native_shown.set(use_native); + Ok(()) + }); bar.init_for_nsapp(); if let Some(help) = bar.items().last().and_then(|i| i.as_submenu().cloned()) { help.set_as_help_menu_for_nsapp(); @@ -166,6 +209,10 @@ pub fn install(app: &FilmcraftApp, ctx: egui::Context) -> (Receiver, Men submenu.set_text(language.tr(title)); } } + native_edit.set_text(language.tr("Edit")); + for (label, item) in &text_items { + item.set_text(language.tr(label)); + } for (label, item) in &predefined { item.set_text(language.tr(label)); } @@ -185,7 +232,7 @@ pub fn install(app: &FilmcraftApp, ctx: egui::Context) -> (Receiver, Men *shown = it.clone(); } }); - (rx, update) + (rx, update, text_menu) } #[cfg(test)] diff --git a/contributors/people.toml b/contributors/people.toml index 3f8743f7f..6c0200d13 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -14,6 +14,10 @@ # To remove your names, delete your entry (or set fields empty) in a PR. The next # contributors.py run, and the next build, drop them. +[people.whatever-industries] +display_name = "Benji Stinson" +sync_github_name = false + [people.echelon] real_name = "Brandon Thomas" display_name = "Brandon Thomas" diff --git a/crates/edit/src/frame_size.rs b/crates/edit/src/frame_size.rs index 90585c9cd..45a09e217 100644 --- a/crates/edit/src/frame_size.rs +++ b/crates/edit/src/frame_size.rs @@ -89,6 +89,7 @@ mod tests { motion.param_mut("position").unwrap().value = ParamValue::Vec2(filmcraft_geom::Vec2::new(pos.0, pos.1)); motion.param_mut("scale").unwrap().value = ParamValue::Float(scale); TrackItem { + custom_label_color: None, id: ClipId(id), item: ItemId(1), name: format!("c{id}"), diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 10942c8b0..69ccd7058 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -37,6 +37,7 @@ impl Fx { fn item(&mut self, start: i64, dur: i64, src_in: i64) -> TrackItem { self.next += 1; TrackItem { + custom_label_color: None, id: ClipId(self.next), item: ItemId(1), name: format!("c{}", self.next), diff --git a/crates/edit/src/through.rs b/crates/edit/src/through.rs index 263bbcce0..8756e836c 100644 --- a/crates/edit/src/through.rs +++ b/crates/edit/src/through.rs @@ -173,6 +173,7 @@ mod tests { fn item(id: u64, start: i64, dur: i64, src: i64) -> TrackItem { TrackItem { + custom_label_color: None, id: ClipId(id), item: ItemId(1), name: "c".into(), diff --git a/crates/edit/src/transcript/tests.rs b/crates/edit/src/transcript/tests.rs index afd8c52bf..cadd29f85 100644 --- a/crates/edit/src/transcript/tests.rs +++ b/crates/edit/src/transcript/tests.rs @@ -34,6 +34,7 @@ fn seq() -> Sequence { fn clip(id: u64, start: f64, dur: f64, src_in: f64) -> TrackItem { TrackItem { + custom_label_color: None, id: ClipId(id), item: MEDIA, name: "interview".into(), diff --git a/crates/edit/src/transitions.rs b/crates/edit/src/transitions.rs index c487c7d24..1d1faed91 100644 --- a/crates/edit/src/transitions.rs +++ b/crates/edit/src/transitions.rs @@ -215,6 +215,7 @@ mod tests { fn clip(id: u64, start: i64, dur: i64) -> TrackItem { TrackItem { + custom_label_color: None, id: ClipId(id), item: ItemId(1), name: format!("c{id}"), diff --git a/crates/engine/src/clip_ops/audio_options.rs b/crates/engine/src/clip_ops/audio_options.rs index 2d944bd94..1348d2eff 100644 --- a/crates/engine/src/clip_ops/audio_options.rs +++ b/crates/engine/src/clip_ops/audio_options.rs @@ -32,6 +32,9 @@ pub(super) fn breakout_to_mono(s: &mut Session, p: &Value) -> Result { mm.markers.clear(); mm.proxy = None; let id = pr.add_item(&format!("{} {suffix}", it.name), it.label, ItemKind::Media(Box::new(mm)), bin); + if let Some(copy) = pr.item_mut(id) { + copy.custom_label_color = it.custom_label_color; + } made.push(id); } } diff --git a/crates/engine/src/clip_ops/common.rs b/crates/engine/src/clip_ops/common.rs index 334719976..ea5763186 100644 --- a/crates/engine/src/clip_ops/common.rs +++ b/crates/engine/src/clip_ops/common.rs @@ -46,7 +46,7 @@ impl SourceView { json!({ "item": self.item.0, "media": self.media.0, "start": self.start.0, "end": self.end.0, "fps": self.rate.as_f64(), "playhead": playhead.0, "markIn": self.mark_in.map(|t| t.0), "markOut": self.mark_out.map(|t| t.0), - "markers": self.markers.iter().map(|m| json!({"id": m.id.0, "start": m.start.0, "name": m.name, "duration": m.duration.0})).collect::>(), + "markers": self.markers.iter().map(|m| json!({"id": m.id.0, "start": m.start.0, "name": m.name, "duration": m.duration.0, "color":m.color_value(), "author":m.author, "comment":m.comment, "url":m.url, "kind":m.kind})).collect::>(), "subclip": self.subclip.map(|(r, restrict)| json!({"start": r.start.0, "end": r.end().0, "restrictTrims": restrict})), }) } diff --git a/crates/engine/src/clip_ops/edit_menu.rs b/crates/engine/src/clip_ops/edit_menu.rs index 74a231f28..fa650018a 100644 --- a/crates/engine/src/clip_ops/edit_menu.rs +++ b/crates/engine/src/clip_ops/edit_menu.rs @@ -25,6 +25,7 @@ pub(super) fn apply_label(s: &mut Session, p: &Value, l: Label) -> Result for i in &items { if let Some(it) = pr.item_mut(*i) { it.label = l; + it.custom_label_color = None; n += 1; } } @@ -32,6 +33,7 @@ pub(super) fn apply_label(s: &mut Session, p: &Value, l: Label) -> Result for c in &clips { if let Some((_, it)) = q.find_item_mut(*c) { it.label = l; + it.custom_label_color = None; n += 1; } } @@ -42,17 +44,19 @@ pub(super) fn apply_label(s: &mut Session, p: &Value, l: Label) -> Result } pub(super) fn select_label_group(s: &mut Session, _: &Value) -> Result { + let key = |label: Label, custom: Option| (if custom.is_some() { "" } else { label.name() }, custom); if !s.state.selection.is_empty() { let q = s.active_sequence().ok_or(EngineError::NoSequence)?; - let labels: BTreeSet<&str> = s.state.selection.iter().filter_map(|c| q.find_item(*c).map(|(_, i)| i.label.name())).collect(); - let sel: Vec = q.all_tracks().flat_map(|t| t.items.iter()).filter(|i| labels.contains(i.label.name())).map(|i| i.id).collect(); + let labels: BTreeSet<_> = s.state.selection.iter().filter_map(|c| q.find_item(*c).map(|(_, i)| key(i.label, i.custom_label_color))).collect(); + let sel: Vec = + q.all_tracks().flat_map(|t| t.items.iter()).filter(|i| labels.contains(&key(i.label, i.custom_label_color))).map(|i| i.id).collect(); s.state.selection = sel; return Ok(json!({"clips": s.state.selection.iter().map(|c| c.0).collect::>()})); } - let labels: BTreeSet<&str> = s.state.project_selection.iter().filter_map(|i| s.project.item(*i).map(|it| it.label.name())).collect(); + let labels: BTreeSet<_> = s.state.project_selection.iter().filter_map(|i| s.project.item(*i).map(|it| key(it.label, it.custom_label_color))).collect(); let mut all = Vec::new(); s.project.root.all_items(&mut all); - let sel: Vec = all.into_iter().filter(|i| s.project.item(*i).is_some_and(|it| labels.contains(it.label.name()))).collect(); + let sel: Vec = all.into_iter().filter(|i| s.project.item(*i).is_some_and(|it| labels.contains(&key(it.label, it.custom_label_color)))).collect(); s.state.project_selection = sel; Ok(json!({"items": s.state.project_selection.iter().map(|i| i.0).collect::>()})) } diff --git a/crates/engine/src/clip_ops/file.rs b/crates/engine/src/clip_ops/file.rs index 43e545327..1b4966a00 100644 --- a/crates/engine/src/clip_ops/file.rs +++ b/crates/engine/src/clip_ops/file.rs @@ -219,6 +219,7 @@ pub(super) fn close_project(s: &mut Session, p: &Value) -> Result { return Err(EngineError::Other("the project has unsaved changes: save it first, or pass {\"force\": true} to discard them".into())); } s.project = std::sync::Arc::new(Project::new("Untitled")); + s.project_instance = std::sync::Arc::new(()); s.document_epoch = s.document_epoch.wrapping_add(1); s.history = Default::default(); s.history.limit = 200; diff --git a/crates/engine/src/clip_ops/replace.rs b/crates/engine/src/clip_ops/replace.rs index a46de29ef..99c0f3440 100644 --- a/crates/engine/src/clip_ops/replace.rs +++ b/crates/engine/src/clip_ops/replace.rs @@ -73,6 +73,7 @@ pub(super) fn replace_with(s: &mut Session, p: &Value, how: Replace) -> Result Option { pub(super) fn make_subclip(s: &mut Session, p: &Value) -> Result { let (parent, range, base, label) = subclip_plan(s, p)?; + let custom = if let Some(clip) = clip_p_or_selection(s, p) { + s.active_sequence().and_then(|q| q.find_item(clip)).and_then(|(_, it)| it.custom_label_color) + } else { + item_p(p, "item").or_else(|| subclip_source(s)).and_then(|id| s.project.item(id)).and_then(|it| it.custom_label_color) + }; + let rate = item_rate(&s.project, parent); let a = p .get("start") @@ -61,6 +67,9 @@ pub(super) fn make_subclip(s: &mut Session, p: &Value) -> Result { let bin = s.project.root.parent_of(parent).filter(|b| *b != s.project.root.id); let id = s.edit("Make Subclip", |pr, st| { let id = pr.add_item(&name, label, ItemKind::Subclip { parent, range: TimeRange::from_bounds(a, b), restrict_trims: restrict }, bin); + if let Some(item) = pr.item_mut(id) { + item.custom_label_color = custom; + } st.project_selection = vec![id]; Ok(id) })?; diff --git a/crates/engine/src/clip_ops_tests.rs b/crates/engine/src/clip_ops_tests.rs index 193679870..b9eaa0963 100644 --- a/crates/engine/src/clip_ops_tests.rs +++ b/crates/engine/src/clip_ops_tests.rs @@ -880,6 +880,9 @@ fn subclip_source_monitor_trims_and_markers() { if let Some(ItemKind::Media(m)) = p.item_mut(ocean).map(|i| &mut i.kind) { for (k, n) in [10, 30, 60, 90].into_iter().enumerate() { m.markers.push(filmcraft_project::Marker { + url: String::new(), + author: String::new(), + custom_color: None, id: filmcraft_project::MarkerId(9000 + k as u64), start: f(n), duration: Tick::ZERO, diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index 72c820393..33ac46d50 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -190,44 +190,6 @@ pub(crate) fn named_items(s: &Session, spec: &CommandSpec, p: &Value) -> Option< // ---------- param helpers ---------- -/// The longest marker `markers.edit` takes, in frames (over 46 hours at 60 fps). -const MAX_MARKER_FRAMES: i64 = 10_000_000; - -/// `markers.edit`'s `durationFrames`: absent or null is `None`; anything but a whole number from 0 -/// to [`MAX_MARKER_FRAMES`] is an error. -fn marker_frames_p(p: &Value) -> Result> { - match p.get("durationFrames").filter(|v| !v.is_null()) { - None => Ok(None), - Some(v) => match v.as_i64() { - Some(f @ 0..=MAX_MARKER_FRAMES) => Ok(Some(f)), - _ => Err(bad("markers.edit", "`durationFrames` must be a whole number from 0 to 10000000")), - }, - } -} - -/// `markers.edit`'s `kind`: absent or null is `None`; anything but one of the kind names is an -/// error. -pub(crate) fn marker_kind_p(p: &Value) -> Result> { - match p.get("kind").filter(|v| !v.is_null()) { - None => Ok(None), - Some(v) => { - v.as_str().and_then(marker_kind).map(Some).ok_or_else(|| bad("markers.edit", "`kind` must be comment, chapter, segmentation, webLink or flashCue")) - } - } -} - -/// A marker type by its `markers.edit` name. -fn marker_kind(name: &str) -> Option { - Some(match name { - "comment" => MarkerKind::Comment, - "chapter" => MarkerKind::Chapter, - "segmentation" => MarkerKind::Segmentation, - "webLink" => MarkerKind::WebLink, - "flashCue" => MarkerKind::FlashCue, - _ => return None, - }) -} - pub(crate) fn bad(cmd: &str, msg: impl Into) -> EngineError { EngineError::BadParams { cmd: cmd.into(), msg: msg.into() } } @@ -236,17 +198,33 @@ pub(crate) fn bad(cmd: &str, msg: impl Into) -> EngineError { pub(crate) fn reports_only(id: &str, p: &Value) -> bool { id == "sequence.settings" && (p.is_null() || p.as_object().is_some_and(|m| m.is_empty())) } -/// A marker's `color` param: absent (or null) is `None`; anything but a label name is an error -/// that lists the accepted names, instead of silently becoming Green (#857). -pub(crate) fn marker_color_p(p: &Value, cmd: &str) -> Result> { +/// A marker accepts either an unchanged preset label or an exact six-digit sRGB hex color. +pub(crate) fn marker_color_p(p: &Value, cmd: &str) -> Result)>> { let v = match p.get("color") { None | Some(Value::Null) => return Ok(None), Some(v) => v, }; - v.as_str().and_then(Label::from_name).map(Some).ok_or_else(|| { - let names: Vec<&str> = Label::ALL.iter().map(|l| l.name()).collect(); - bad(cmd, format!("unknown marker color {v}; use one of: {}", names.join(", "))) - }) + if let Some(text) = v.as_str() { + if let Some(label) = Label::from_color_name(text) { + return Ok(Some((label, None))); + } + if let Some(hex) = filmcraft_project::HexColor::parse(text) { + return Ok(Some((Label::Green, Some(hex)))); + } + } + let names: Vec<&str> = Label::ALL.iter().map(|l| l.name()).chain(Label::COLOR_ALIASES.iter().map(|(name, _)| *name)).collect(); + Err(bad(cmd, format!("unknown marker color {v}; use #RRGGBB or one of: {}", names.join(", ")))) +} + +/// Missing/null authors preserve the existing value; an empty string explicitly clears it. +/// Reject non-text values, control characters and more than 1024 UTF-8 bytes before editing. +pub(crate) fn marker_author_p(p: &Value, cmd: &str) -> Result> { + let Some(value) = p.get("author").filter(|v| !v.is_null()) else { return Ok(None) }; + let text = value.as_str().ok_or_else(|| bad(cmd, "author must be text"))?; + if text.len() > 1024 || text.chars().any(char::is_control) { + return Err(bad(cmd, "author must be at most 1024 UTF-8 bytes without control characters")); + } + Ok(Some(text.to_string())) } pub(crate) fn str_p<'a>(p: &'a Value, k: &str) -> Option<&'a str> { p.get(k).and_then(Value::as_str) @@ -1489,6 +1467,7 @@ fn build() -> Vec { cmd!("file.newProject", "Project…", ["File", "New"], Some("Cmd+Alt+N"), r#"{"name":str}"#, always, |s, p| { let name = str_p(p, "name").unwrap_or("Untitled").to_string(); s.project = std::sync::Arc::new(filmcraft_project::Project::new(&name)); + s.project_instance = std::sync::Arc::new(()); s.document_epoch = s.document_epoch.wrapping_add(1); s.history = Default::default(); s.history.limit = 200; @@ -1505,6 +1484,7 @@ fn build() -> Vec { cmd!("file.openDemoProject", "Demo Project", ["File", "New"], None, "{}", always, |s, _| { let (p, seq) = crate::demo::demo_project(&s.media); s.project = std::sync::Arc::new(p); + s.project_instance = std::sync::Arc::new(()); s.document_epoch = s.document_epoch.wrapping_add(1); s.history = Default::default(); s.history.limit = 200; @@ -2161,6 +2141,9 @@ fn build() -> Vec { if let Some(it) = p.item(id).cloned() { let bin = p.root.parent_of(id); let nid = p.add_item(&format!("{} Copy", it.name), it.label, it.kind.clone(), bin); + if let Some(copy) = p.item_mut(nid) { + copy.custom_label_color = it.custom_label_color; + } out.push(nid); } } @@ -2169,28 +2152,83 @@ fn build() -> Vec { })?; Ok(json!({"items": ids.iter().map(|i| i.0).collect::>()})) }), - cmd!("edit.label", "Label", [], None, r#"{"label":"Violet|Iris|…"}"#, always, |s, p| { - let l = str_p(p, "label").and_then(Label::from_name).ok_or_else(|| bad("edit.label", "unknown label"))?; - let clips = s.state.selection.clone(); - let items = s.state.project_selection.clone(); - let seq = s.state.active_sequence; - s.edit("Label", |pr, _| { - for i in &items { - if let Some(it) = pr.item_mut(*i) { - it.label = l; - } + cmd!( + "edit.label", + "Label", + [], + None, + r##"{"label":"preset|#RRGGBB","clips":[id]?,"items":[id]?,"sequence":id?,"saveColor":bool?}"##, + always, + |s, p| { + let text = str_p(p, "label").ok_or_else(|| bad("edit.label", "need a preset name or hex color"))?; + let preset = Label::from_color_name(text); + let custom = filmcraft_project::HexColor::parse(text); + if preset.is_none() && custom.is_none() { + return Err(bad("edit.label", "unknown label; use a preset or #RRGGBB")); } - if let Some(q) = seq.and_then(|q| pr.sequence_mut(q)) { - for c in &clips { - if let Some((_, it)) = q.find_item_mut(*c) { - it.label = l; + let explicit = p.get("clips").is_some() || p.get("items").is_some(); + let clips: Vec = match p.get("clips") { + Some(v) => serde_json::from_value(v.clone()).map_err(|_| bad("edit.label", "clips must be an array of ids"))?, + None => { + if explicit { + Vec::new() + } else { + s.state.selection.clone() } } + }; + let items: Vec = match p.get("items") { + Some(v) => serde_json::from_value(v.clone()).map_err(|_| bad("edit.label", "items must be an array of ids"))?, + None => { + if explicit { + Vec::new() + } else { + s.state.project_selection.clone() + } + } + }; + let seq = match p.get("sequence") { + Some(v) => Some(ItemId(v.as_u64().ok_or_else(|| bad("edit.label", "sequence must be an id"))?)), + None => s.state.active_sequence, + }; + let save = match p.get("saveColor") { + Some(v) => v.as_bool().ok_or_else(|| bad("edit.label", "saveColor must be boolean"))?, + None => false, + }; + if save && custom.is_none() { + return Err(bad("edit.label", "saveColor requires a hex color")); } - Ok(()) - })?; - Ok(Value::Null) - }), + s.edit("Label", |pr, _| { + if let Some(color) = custom.filter(|_| save) + && !pr.custom_colors.contains(&color) + { + if pr.custom_colors.len() >= 64 { + return Err(bad("edit.label", "the project already has 64 custom colors")); + } + pr.custom_colors.push(color); + } + for i in &items { + let it = pr.item_mut(*i).ok_or_else(|| bad("edit.label", "project item no longer exists"))?; + if let Some(preset) = preset { + it.label = preset; + } + it.custom_label_color = custom; + } + if !clips.is_empty() { + let q = seq.and_then(|id| pr.sequence_mut(id)).ok_or(EngineError::NoSequence)?; + for c in &clips { + let (_, it) = q.find_item_mut(*c).ok_or_else(|| bad("edit.label", "clip no longer exists"))?; + if let Some(preset) = preset { + it.label = preset; + } + it.custom_label_color = custom; + } + } + Ok(()) + })?; + Ok(Value::Null) + } + ), // ================= Clip ================= cmd!("clip.rename", "Rename…", ["Clip"], None, r#"{"clip":id?,"clips":[id]?,"item":id?,"name":str?}"#, always, |s, p| { // a blank name is refused, as the Project panel's inline rename keeps the old one @@ -2896,12 +2934,13 @@ fn build() -> Vec { })?; Ok(Value::Null) }), + cmd!("colors.addCustom", "Add Custom Color", [], None, r#"{"color":hex,"name":str?}"#, always, crate::palette::add), cmd!( "markers.add", "Add Marker", ["Markers"], Some("M"), - r#"{"target":"program|source","time":ticks?,"name":str?,"comment":str?,"color":label?,"durationFrames":i64?}"#, + r#"{"target":"program|source","time":ticks?,"name":str?,"comment":str?,"color":label_or_hex?,"author":str?,"durationFrames":i64?}"#, always, |s, p| { if let Some(v) = crate::source_monitor::route(s, "markers.add", p)? { @@ -2911,11 +2950,23 @@ fn build() -> Vec { let t = sequence_time_p(s, p, "", "markers.add")?.unwrap_or(ph); let name = str_p(p, "name").unwrap_or("").to_string(); let comment = str_p(p, "comment").unwrap_or("").to_string(); - let color = marker_color_p(p, "markers.add")?.unwrap_or(Label::Green); + let (color, custom_color) = marker_color_p(p, "markers.add")?.unwrap_or((Label::Green, None)); + let author = marker_author_p(p, "markers.add")?.unwrap_or_default(); let dur = p.get("durationFrames").and_then(Value::as_i64).map(|f| s.sequence_rate().tick_of(f)).unwrap_or_default(); let id = s.edit_sequence("Add Marker", |q, ctx, _| { let id = MarkerId(ctx.alloc()); - q.markers.push(Marker { id, start: t, duration: dur, name, comment, kind: MarkerKind::Comment, color }); + q.markers.push(Marker { + url: String::new(), + author, + custom_color, + id, + start: t, + duration: dur, + name, + comment, + kind: MarkerKind::Comment, + color, + }); q.markers.sort_by_key(|m| m.start); Ok(id) })?; @@ -2979,41 +3030,21 @@ fn build() -> Vec { "Edit Marker…", [], None, - r#"{"target":"program|source","marker":id,"name":str?,"comment":str?,"color":label?,"durationFrames":i64?,"kind":"comment|chapter|segmentation|webLink|flashCue"?}"#, + r#"{"target":"program|source","sequence":id?,"item":id?,"marker":id|"markers":[id],"time":ticks?,"frame":i64?,"durationFrames":i64?,"name":str?,"comment":str?,"author":str?,"url":str?,"color":label_or_hex?,"kind":"comment|chapter|segmentation|webLink|flashCue"?}"#, always, - |s, p| { - // checked before routing, so Source markers are held to the same limits - let frames = marker_frames_p(p)?; - let kind = marker_kind_p(p)?; - if let Some(v) = crate::source_monitor::route(s, "markers.edit", p)? { - return Ok(v); - } - let id = u64_p(p, "marker").map(MarkerId).ok_or_else(|| bad("markers.edit", "need `marker`"))?; - let rate = s.sequence_rate(); - let color = marker_color_p(p, "markers.edit")?; - let p = p.clone(); - s.edit_sequence("Edit Marker", |q, _, _| { - let m = q.markers.iter_mut().find(|m| m.id == id).ok_or_else(|| bad("markers.edit", "no such marker"))?; - if let Some(n) = str_p(&p, "name") { - m.name = n.into(); - } - if let Some(c) = str_p(&p, "comment") { - m.comment = c.into(); - } - if let Some(c) = color { - m.color = c; - } - if let Some(f) = frames { - m.duration = rate.tick_of(f); - } - if let Some(k) = kind { - m.kind = k; - } - Ok(()) - })?; - Ok(Value::Null) - } + |s, p| crate::marker_edit::run(s, p, false) ), + cmd!( + "markers.delete", + "Delete Markers", + [], + None, + r#"{"target":"program|source","sequence":id?,"item":id?,"marker":id|"markers":[id]}"#, + always, + |s, p| crate::marker_edit::run(s, p, true) + ), + cmd!("colors.updateCustom", "Edit Custom Color", [], None, r#"{"color":hex,"replacement":hex?,"name":str?}"#, always, crate::palette::update), + cmd!("colors.removeCustom", "Remove Custom Color", [], None, r#"{"color":hex}"#, always, crate::palette::remove), // ================= Playhead / navigation ================= cmd!("playhead.set", "Set Playhead", [], None, r#"{"time":ticks|"frame":i64|"seconds":f64|"timecode":str}"#, has_seq, |s, p| { let t = time_p(s, p, "").ok_or_else(|| bad("playhead.set", "need time/frame/seconds/timecode"))?; @@ -5025,7 +5056,7 @@ pub fn inspect_project(s: &Session) -> Value { "children": b.children.iter().map(|c| match c { filmcraft_project::BinEntry::Item(i) => { let it = p.item(*i); - json!({"item": i.0, "name": it.map(|x| x.name.clone()), "type": it.map(|x| x.type_label()), "duration": it.map(|x| x.duration().0), "label": it.map(|x| x.label.name())}) + json!({"item": i.0, "name": it.map(|x| x.name.clone()), "type": it.map(|x| x.type_label()), "duration": it.map(|x| x.duration().0), "label": it.map(|x| x.label.name()), "customLabelColor": it.and_then(|x| x.custom_label_color)}) } filmcraft_project::BinEntry::Bin(b) => bin(b, p), }).collect::>() @@ -5052,7 +5083,7 @@ pub fn inspect_sequence(s: &Session, id: ItemId, q: &filmcraft_project::Sequence "clip": i.id.0, "item": i.item.0, "name": i.name, "start": i.start.0, "duration": i.duration.0, "startFrame": rate.frame_at(i.start), "durationFrames": rate.frame_at(i.duration), "sourceIn": i.source_in.0, "speed": i.speed, "reverse": i.reverse, "end": i.end().0, "endFrame": rate.frame_at(i.end()), "sourceOut": i.source_out().0, "gainDb": i.gain_db, - "enabled": i.enabled, "link": i.link, "label": i.label.name(), + "enabled": i.enabled, "link": i.link, "label": i.label.name(), "customLabelColor": i.custom_label_color, "effects": i.effects.iter().map(|e| json!({"effect": e.effect, "enabled": e.enabled, "masks": e.masks.len(), "params": e.params.iter().map(|(k, p)| (k.clone(), json!({"value": format!("{:?}", p.value), "keyframes": p.keyframes.len()}))).collect::>()})).collect::>(), })).collect::>(), "transitions": t.transitions.iter().map(|x| json!({"id": x.id.0, "effect": x.effect.effect, "start": x.start.0, "duration": x.duration.0, "from": x.from.map(|c| c.0), "to": x.to.map(|c| c.0), "reverse": x.reverse, "align": filmcraft_edit::transitions::bounds(t, x).map(|b| filmcraft_edit::transitions::alignment(x, b.1, rate.frame_duration()).id()), "params": x.effect.params.iter().map(|(k, v)| (k.clone(), serde_json::to_value(&v.value).unwrap_or_default())).collect::>()})).collect::>(), @@ -5070,7 +5101,7 @@ pub fn inspect_sequence(s: &Session, id: ItemId, q: &filmcraft_project::Sequence "in": q.mark_in.map(|t| t.0), "out": q.mark_out.map(|t| t.0), "split": q.split, - "markers": q.markers.iter().map(|m| json!({"id": m.id.0, "start": m.start.0, "duration": m.duration.0, "kind": format!("{:?}", m.kind), "name": m.name, "comment": m.comment, "color": m.color.name()})).collect::>(), + "markers": q.markers.iter().map(|m| json!({"id": m.id.0, "start": m.start.0, "duration": m.duration.0, "kind": format!("{:?}", m.kind), "name": m.name, "comment": m.comment, "color": m.color_value(), "author": m.author, "url": m.url})).collect::>(), "video": q.video_tracks.iter().map(tr).collect::>(), "audio": q.audio_tracks.iter().map(tr).collect::>(), "selection": s.state.selection.iter().map(|c| c.0).collect::>(), @@ -5148,6 +5179,7 @@ fn install_project(s: &mut Session, proj: filmcraft_project::Project, path: Opti let first = proj.sequences().next().map(|i| i.id); s.media.add_stored(&proj, path.as_deref()); s.project = std::sync::Arc::new(proj); + s.project_instance = std::sync::Arc::new(()); s.document_epoch = s.document_epoch.wrapping_add(1); s.history = Default::default(); s.history.limit = 200; diff --git a/crates/engine/src/demo.rs b/crates/engine/src/demo.rs index 6b641070e..8f00ae12f 100644 --- a/crates/engine/src/demo.rs +++ b/crates/engine/src/demo.rs @@ -208,6 +208,9 @@ fn build_demo(pool: &MediaPool) -> Option<(Project, ItemId)> { .enumerate() { s.markers.push(Marker { + url: String::new(), + author: String::new(), + custom_color: None, id: MarkerId(800_000 + i as u64), start: r.snap(Tick::from_seconds_f64(*sec)), duration: Tick::ZERO, diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 98f8e198c..5ea12df9a 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -25,6 +25,7 @@ pub mod graphics; pub mod interchange; pub mod keyboard; mod keyframe_move; +mod marker_edit; mod marker_export; pub mod masks; pub mod media_browser; @@ -35,6 +36,7 @@ pub mod media_watch; pub mod mixer; pub mod multicam; pub mod narration; +mod palette; pub mod panels; pub mod paste_media; pub mod perf; @@ -400,6 +402,8 @@ pub struct EditorState { /// Marker colours hidden in the Markers panel (Show All Marker Colors clears this). #[serde(default)] pub hidden_marker_colors: Vec, + #[serde(default)] + pub hidden_custom_marker_colors: Vec, /// Sequence markers copied with the clipboard clips (start relative to the copied range). #[serde(skip)] pub clipboard_markers: Vec, @@ -436,6 +440,8 @@ pub enum Event { pub struct Session { pub project: Arc, + /// Runtime identity of the opened project, stable across edits/undo and renewed on replacement. + pub project_instance: Arc<()>, pub history: History, pub revision: u64, /// Bumped whenever the session installs a different project document (`file.newProject`, @@ -558,6 +564,7 @@ impl Session { pub fn new(services: Arc) -> Self { Self { project: Arc::new(Project::new("Untitled")), + project_instance: Arc::new(()), history: History { limit: 200, ..Default::default() }, revision: 1, document_epoch: 0, diff --git a/crates/engine/src/marker_edit.rs b/crates/engine/src/marker_edit.rs new file mode 100644 index 000000000..5aa518243 --- /dev/null +++ b/crates/engine/src/marker_edit.rs @@ -0,0 +1,173 @@ +//! Explicit marker ownership and atomic single/bulk edits. +use crate::{ + EngineError, Result, Session, + commands::{bad, marker_author_p, marker_color_p}, +}; +use filmcraft_project::{ItemId, Marker, MarkerId, MarkerKind, Project}; +use filmcraft_time::{FrameRate, Tick}; +use serde_json::{Value, json}; +use std::collections::BTreeSet; + +fn list(project: &mut Project, owner: ItemId) -> Result<&mut Vec> { + match &mut project.item_mut(owner).ok_or_else(|| EngineError::Other("Marker owner no longer exists".into()))?.kind { + filmcraft_project::ItemKind::Media(m) => Ok(&mut m.markers), + filmcraft_project::ItemKind::Sequence(q) => Ok(&mut std::sync::Arc::make_mut(q).markers), + _ => Err(EngineError::Other("This item cannot own markers".into())), + } +} + +fn item(p: &Value, key: &str, fallback: Option, id: &str) -> Result { + match p.get(key) { + Some(v) => v.as_u64().map(ItemId).ok_or_else(|| bad(id, "owner must be an item id")), + None => fallback.ok_or_else(|| bad(id, "no marker owner is open")), + } +} +fn frames(p: &Value, key: &str, rate: FrameRate, id: &str) -> Result> { + p.get(key) + .map(|v| { + let f = v.as_i64().ok_or_else(|| bad(id, "frame values must be integers"))?; + if f < 0 || f > rate.frame_at(Tick::MAX) { + return Err(bad(id, "frame value is outside the supported time range")); + } + Ok(rate.tick_of(f)) + }) + .transpose() +} + +pub(crate) fn run(s: &mut Session, p: &Value, delete: bool) -> Result { + let id = if delete { "markers.delete" } else { "markers.edit" }; + let (owner, rate, lower, upper, visible) = match p.get("target") { + None | Some(Value::String(_)) if p.get("target").is_none_or(|v| v == "program") => { + let owner = item(p, "sequence", s.state.active_sequence, id)?; + let q = s.project.sequence(owner).ok_or_else(|| bad(id, "no such sequence"))?; + (owner, q.settings.frame_rate, Tick::ZERO, Tick::MAX, None) + } + Some(Value::String(t)) if t == "source" => { + let source = item(p, "item", s.state.source_item, id)?; + let v = crate::clip_ops::source_view(s, source).ok_or_else(|| bad(id, "no such source"))?; + (v.media, v.rate, v.start, v.end, Some(v.markers.iter().map(|m| m.id).collect::>())) + } + _ => return Err(bad(id, "target must be program or source")), + }; + let ids: BTreeSet = if let Some(v) = p.get("markers") { + if p.get("marker").is_some() { + return Err(bad(id, "use marker or markers, not both")); + } + let a = v.as_array().filter(|a| !a.is_empty() && a.len() <= 100_000).ok_or_else(|| bad(id, "markers must contain 1 to 100000 ids"))?; + a.iter().map(|v| v.as_u64().map(MarkerId).ok_or_else(|| bad(id, "invalid marker id"))).collect::>()? + } else { + BTreeSet::from([MarkerId(p.get("marker").and_then(Value::as_u64).ok_or_else(|| bad(id, "need marker id"))?)]) + }; + let time = match p.get("time") { + Some(v) => { + if p.get("frame").is_some() { + return Err(bad(id, "use time or frame, not both")); + } + Some(Tick(v.as_i64().ok_or_else(|| bad(id, "time must be integer ticks"))?)) + } + None => frames(p, "frame", rate, id)?, + }; + // Match the established command contract: null leaves duration unchanged, and + // edited durations are bounded independently of a marker's imported original. + let duration = match p.get("durationFrames").filter(|v| !v.is_null()) { + None => None, + Some(value) => { + let frames = + value.as_i64().filter(|f| (0..=10_000_000).contains(f)).ok_or_else(|| bad(id, "durationFrames must be a whole number from 0 to 10000000"))?; + Some(rate.tick_of(frames)) + } + }; + if ids.len() > 1 && (time.is_some() || duration.is_some()) { + return Err(bad(id, "time edits require a single marker")); + } + let color = marker_color_p(p, id)?; + let author = marker_author_p(p, id)?; + let kind = match p.get("kind") { + None | Some(Value::Null) => None, + Some(value) => Some(match value.as_str() { + Some("comment") => MarkerKind::Comment, + Some("chapter") => MarkerKind::Chapter, + Some("segmentation") => MarkerKind::Segmentation, + Some("webLink") => MarkerKind::WebLink, + Some("flashCue") => MarkerKind::FlashCue, + _ => return Err(bad(id, "kind must be comment, chapter, segmentation, webLink or flashCue")), + }), + }; + let text = |key| -> Result> { + p.get(key) + .map(|v| v.as_str().filter(|v| v.len() <= 65536).map(str::to_owned).ok_or_else(|| bad(id, "marker text must be a string of at most 65536 bytes"))) + .transpose() + }; + let name = text("name")?; + let comment = text("comment")?; + let url = match p.get("url") { + None | Some(Value::Null) => None, + Some(value) => { + let value = value + .as_str() + .filter(|v| v.len() <= 8192 && !v.chars().any(char::is_control)) + .ok_or_else(|| bad(id, "url must be text of at most 8192 bytes without control characters"))? + .trim(); + if !value.is_empty() && filmcraft_project::marker_web_url(value).is_none() { + return Err(bad(id, "url must be an absolute HTTP or HTTPS address without credentials")); + } + Some(value.to_string()) + } + }; + s.edit(if delete { "Delete Markers" } else { "Edit Markers" }, |project, _| { + let markers = list(project, owner)?; + let present: BTreeSet<_> = markers.iter().map(|m| m.id).collect(); + if !ids.is_subset(&present) { + return Err(bad(id, "no such marker in this owner")); + } + if visible.as_ref().is_some_and(|v| !ids.is_subset(v)) { + return Err(bad(id, "marker is outside the source view")); + } + for m in markers.iter().filter(|m| ids.contains(&m.id)) { + if !delete && (time.is_some() || duration.is_some()) { + let start = time.unwrap_or(m.start); + let dur = duration.unwrap_or(m.duration); + let end = start.0.checked_add(dur.0).ok_or_else(|| bad(id, "marker end overflows"))?; + if start < lower || start >= upper || dur < Tick::ZERO || end > upper.0 { + return Err(bad(id, "marker range is outside the source or supported time range")); + } + } + } + if delete { + markers.retain(|m| !ids.contains(&m.id)); + } else { + for m in markers.iter_mut().filter(|m| ids.contains(&m.id)) { + if let Some(v) = &name { + m.name.clone_from(v); + } + if let Some(v) = &comment { + m.comment.clone_from(v); + } + if let Some(v) = &url { + m.url.clone_from(v); + } + if let Some(v) = &author { + m.author.clone_from(v); + } + if let Some(kind) = kind { + m.kind = kind; + } + if let Some((preset, custom)) = color { + if custom.is_none() { + m.color = preset; + } + m.custom_color = custom; + } + if let Some(v) = time { + m.start = v; + } + if let Some(v) = duration { + m.duration = v; + } + } + markers.sort_by_key(|m| m.start); + } + Ok(()) + })?; + Ok(json!({"count":ids.len()})) +} diff --git a/crates/engine/src/marker_export.rs b/crates/engine/src/marker_export.rs index 86a6da6b5..15696f502 100644 --- a/crates/engine/src/marker_export.rs +++ b/crates/engine/src/marker_export.rs @@ -33,9 +33,11 @@ pub(crate) fn export(s: &mut Session, p: &Value) -> Result { for m in &seq.markers { let row = name .len() - .checked_add(m.name.len()) + .checked_add(m.author.len()) + .and_then(|n| n.checked_add(m.name.len())) .and_then(|n| n.checked_add(m.comment.len())) - .and_then(|n| n.checked_add(label_name(s, m.color).len())) + .and_then(|n| n.checked_add(m.url.len())) + .and_then(|n| n.checked_add(marker_color_name(s, m).len())) .and_then(|n| n.checked_mul(2)) .and_then(|n| n.checked_add(1024)); bound = row.and_then(|n| bound.checked_add(n)).filter(|n| *n <= MAX_BYTES).ok_or_else(|| bad(CMD, "marker report exceeds the 16 MiB limit"))?; @@ -43,8 +45,9 @@ pub(crate) fn export(s: &mut Session, p: &Value) -> Result { let rate = seq.settings.frame_rate; let mut markers: Vec<_> = seq.markers.iter().collect(); markers.sort_by_key(|m| (m.start, m.id)); - let mut csv = - String::from("Sequence,Marker ID,Name,Comment,Kind,Color,Start Timecode,End Timecode,Duration Frames,Start Ticks,Duration Ticks,Frame Rate\r\n"); + let mut csv = String::from( + "Sequence,Marker ID,Name,Comment,Kind,Color,Start Timecode,End Timecode,Duration Frames,Start Ticks,Duration Ticks,Frame Rate,Author,URL\r\n", + ); for m in markers { if m.start.0 < 0 || m.duration.0 < 0 { return Err(bad(CMD, format!("marker {} has a negative start or duration", m.id.0))); @@ -60,13 +63,15 @@ pub(crate) fn export(s: &mut Session, p: &Value) -> Result { m.name.clone(), m.comment.clone(), format!("{:?}", m.kind), - label_name(s, m.color).to_string(), + marker_color_name(s, m), tc(m.start)?, tc(Tick(end))?, rate.frame_at(m.duration).to_string(), m.start.0.to_string(), m.duration.0.to_string(), format!("{}/{}", rate.num, rate.den), + m.author.clone(), + m.url.clone(), ]; for (i, field) in fields.iter().enumerate() { if i > 0 { @@ -75,7 +80,7 @@ pub(crate) fn export(s: &mut Session, p: &Value) -> Result { csv.push('"'); // Quoting alone does not stop spreadsheet formulas. Escape only arbitrary text, // including formulas preceded by whitespace; numeric/timecode fields stay numeric. - if matches!(i, 0 | 2 | 3 | 5) && field.trim_start().starts_with(['=', '+', '-', '@']) { + if matches!(i, 0 | 2 | 3 | 5 | 12 | 13) && field.trim_start().starts_with(['=', '+', '-', '@']) { csv.push('\''); } for ch in field.chars() { @@ -92,6 +97,10 @@ pub(crate) fn export(s: &mut Session, p: &Value) -> Result { Ok(json!({"path": path, "markers": seq.markers.len(), "bytes": csv.len()})) } +fn marker_color_name(s: &Session, marker: &filmcraft_project::Marker) -> String { + marker.custom_color.map_or_else(|| label_name(s, marker.color).to_string(), filmcraft_project::HexColor::hex) +} + #[cfg(test)] mod tests { use super::*; @@ -124,6 +133,9 @@ mod tests { } fn marker(id: u64, start: Tick, duration: Tick, name: &str) -> Marker { Marker { + url: String::new(), + author: String::new(), + custom_color: None, id: MarkerId(id), start, duration, @@ -144,6 +156,22 @@ mod tests { String::from_utf8(files.writes.lock().unwrap().last().unwrap().1.clone()).unwrap() } + #[test] + fn csv_preserves_exact_custom_color_and_author() { + let files = Arc::new(Files::default()); + let mut s = session(files.clone()); + let mut m = marker(100, Tick::ZERO, Tick::ZERO, "Note"); + m.custom_color = filmcraft_project::HexColor::parse("#ffffff"); + m.author = "Editor 日本語".into(); + m.url = "https://example.com/review?q=1,2&take=\"B\"".into(); + set_markers(&mut s, vec![m]); + s.execute(CMD, json!({"path":"custom.csv"})).unwrap(); + let csv = written(&files); + assert!(csv.contains("Frame Rate,Author,URL\r\n")); + assert!(csv.contains("\"Chapter\",\"#FFFFFF\"")); + assert!(csv.contains("\"Editor 日本語\",\"https://example.com/review?q=1,2&take=\"\"B\"\"\"\r\n")); + } + #[test] fn command_exports_sorted_unicode_review_notes_without_changing_the_edit() { let files = Arc::new(Files::default()); @@ -161,7 +189,7 @@ mod tests { assert_eq!(result["bytes"], csv.len()); assert!(csv.contains("\"Review, 日本語\",\"101\",\"早い\",\"First line\r\nSecond, \"\"quoted\"\"\"")); assert!(csv.find("\"101\"").unwrap() < csv.find("\"100\"").unwrap()); - assert!(csv.contains("\"00:00:02:00\",\"00:00:03:00\",\"24\",\"508032000000\",\"254016000000\",\"24/1\"\r\n")); + assert!(csv.contains("\"00:00:02:00\",\"00:00:03:00\",\"24\",\"508032000000\",\"254016000000\",\"24/1\",\"\",\"\"\r\n")); assert_eq!(*s.project, *before); assert_eq!(serde_json::to_value(&s.state).unwrap(), state); assert_eq!(s.history.undo.len(), undo); @@ -195,6 +223,8 @@ mod tests { for text in ["=1+1", "+1+1", "-1+1", "@SUM(1)", "\t =1+1"] { let mut m = marker(100, Tick::ZERO, Tick::ZERO, text); m.comment = text.into(); + m.author = text.into(); + m.url = text.into(); set_markers(&mut s, vec![m]); let id = s.state.active_sequence.unwrap(); s.edit("Name", |p, _| { @@ -203,7 +233,7 @@ mod tests { }) .unwrap(); s.execute(CMD, json!({"path":"safe.csv"})).unwrap(); - assert_eq!(written(&files).matches(&format!("\"'{text}\"")).count(), 3); + assert_eq!(written(&files).matches(&format!("\"'{text}\"")).count(), 5); s.prefs.labels.colors.get_mut("iris").unwrap().name = text.into(); s.execute(CMD, json!({"path":"safe-label.csv"})).unwrap(); assert!(written(&files).contains(&format!("\"Chapter\",\"'{}\"", text.trim()))); diff --git a/crates/engine/src/multicam_tests.rs b/crates/engine/src/multicam_tests.rs index 7d5a1f27b..ffdf3412d 100644 --- a/crates/engine/src/multicam_tests.rs +++ b/crates/engine/src/multicam_tests.rs @@ -219,6 +219,9 @@ fn timecode_marker_and_in_point_sync_within_one_frame() { for (it, t) in [(items[0], rate.tick_of(event_frame_a)), (items[1], rate.snap(at_b))] { let m = p.item_mut(it).unwrap().as_media_mut().unwrap(); m.markers.push(Marker { + url: String::new(), + author: String::new(), + custom_color: None, id: MarkerId(900 + it.0), start: t, duration: Tick::ZERO, diff --git a/crates/engine/src/palette.rs b/crates/engine/src/palette.rs new file mode 100644 index 000000000..c203b4cc0 --- /dev/null +++ b/crates/engine/src/palette.rs @@ -0,0 +1,85 @@ +//! Saved swatch operations do not change colors already assigned to media or markers. +use crate::{Result, Session, commands::bad}; +use filmcraft_project::HexColor; +use serde_json::{Value, json}; +fn color(p: &Value, key: &str, id: &str) -> Result { + p.get(key).and_then(Value::as_str).and_then(HexColor::parse).ok_or_else(|| bad(id, "need a six-digit hex color")) +} +pub(crate) fn update(s: &mut Session, p: &Value) -> Result { + let id = "colors.updateCustom"; + let old = color(p, "color", id)?; + let new = if p.get("replacement").is_some() { color(p, "replacement", id)? } else { old }; + let name = p + .get("name") + .map(|v| { + v.as_str() + .filter(|s| s.len() <= 128 && !s.chars().any(char::is_control)) + .map(str::to_owned) + .ok_or_else(|| bad(id, "name must be at most 128 bytes without control characters")) + }) + .transpose()?; + if !s.project.custom_colors.contains(&old) { + return Err(bad(id, "no such saved color")); + } + if old != new && s.project.custom_colors.contains(&new) { + return Err(bad(id, "that color is already saved")); + } + s.edit("Edit Custom Color", |project, _| { + for c in &mut project.custom_colors { + if *c == old { + *c = new; + } + } + let previous = project.custom_color_names.remove(&old).unwrap_or_default(); + let name = name.unwrap_or(previous); + if !name.trim().is_empty() { + project.custom_color_names.insert(new, name); + } + Ok(()) + })?; + Ok(json!({"color":new})) +} +pub(crate) fn remove(s: &mut Session, p: &Value) -> Result { + let id = "colors.removeCustom"; + let old = color(p, "color", id)?; + if !s.project.custom_colors.contains(&old) { + return Err(bad(id, "no such saved color")); + } + s.edit("Remove Custom Color", |project, _| { + project.custom_colors.retain(|c| *c != old); + project.custom_color_names.remove(&old); + Ok(()) + })?; + Ok(Value::Null) +} + +pub(crate) fn add(s: &mut Session, p: &Value) -> Result { + let id = "colors.addCustom"; + let color = color(p, "color", id)?; + let name = p + .get("name") + .map(|v| { + v.as_str() + .filter(|s| s.len() <= 128 && !s.chars().any(char::is_control)) + .map(str::to_owned) + .ok_or_else(|| bad(id, "name must be at most 128 bytes without control characters")) + }) + .transpose()?; + if s.project.custom_colors.contains(&color) { + if let Some(name) = name { + return update(s, &json!({"color":color,"name":name})); + } + return Ok(json!({"color":color})); + } + if s.project.custom_colors.len() >= 64 { + return Err(bad(id, "the project already has 64 custom colors")); + } + s.edit("Add Custom Color", |project, _| { + project.custom_colors.push(color); + if let Some(name) = name.filter(|s| !s.trim().is_empty()) { + project.custom_color_names.insert(color, name); + } + Ok(()) + })?; + Ok(json!({"color":color})) +} diff --git a/crates/engine/src/panels.rs b/crates/engine/src/panels.rs index 8d54e0f31..5e1804016 100644 --- a/crates/engine/src/panels.rs +++ b/crates/engine/src/panels.rs @@ -194,7 +194,7 @@ pub fn metadata_fields(p: &Project, id: ItemId) -> Vec { let mut out = Vec::new(); let mut add = |section, name: &str, value: String, editable| out.push(Field { section, name: name.to_string(), value, editable }); add("Clip", "Name", it.name.clone(), true); - add("Clip", "Label", it.label.name().to_string(), true); + add("Clip", "Label", it.label_value(), true); add("Clip", "Media Type", it.type_label().to_string(), false); let col = |c: &str| filmcraft_project::find::column_text(p, it, c); add("Clip", "Frame Rate", col("Frame Rate"), false); @@ -300,7 +300,7 @@ fn metadata_set(s: &mut Session, p: &Value) -> Result { if k.trim().is_empty() { return Err(bad("metadata.set", "empty field name")); } - if k.eq_ignore_ascii_case("label") && Label::from_name(v).is_none() { + if k.eq_ignore_ascii_case("label") && Label::from_color_name(v).is_none() && filmcraft_project::HexColor::parse(v).is_none() { return Err(bad("metadata.set", format!("unknown label `{v}`"))); } if k.eq_ignore_ascii_case("name") && v.trim().is_empty() { @@ -314,7 +314,13 @@ fn metadata_set(s: &mut Session, p: &Value) -> Result { let it = s.project.item(id).ok_or_else(|| bad("metadata.set", "no such item"))?; changes.iter().all(|(k, v)| match k.to_ascii_lowercase().as_str() { "name" => it.name == *v, - "label" => Label::from_name(v) == Some(it.label), + "label" => { + if let Some(color) = filmcraft_project::HexColor::parse(v) { + it.custom_label_color == Some(color) + } else { + it.custom_label_color.is_none() && Label::from_color_name(v) == Some(it.label) + } + } _ => it.metadata.get(k).map(String::as_str).unwrap_or("") == v.as_str(), }) }; @@ -324,7 +330,12 @@ fn metadata_set(s: &mut Session, p: &Value) -> Result { for (k, v) in &changes { match k.to_ascii_lowercase().as_str() { "name" => it.name = v.trim().to_string(), - "label" => it.label = Label::from_name(v).unwrap_or(it.label), + "label" => { + if let Some(label) = Label::from_color_name(v) { + it.label = label; + } + it.custom_label_color = filmcraft_project::HexColor::parse(v); + } _ if v.is_empty() => { it.metadata.remove(k); } @@ -337,7 +348,7 @@ fn metadata_set(s: &mut Session, p: &Value) -> Result { })?; } let it = s.project.item(id).ok_or_else(|| bad("metadata.set", "no such item"))?; - Ok(json!({"item": id.0, "name": it.name, "label": it.label.name(), "metadata": it.metadata, "changed": !unchanged})) + Ok(json!({"item": id.0, "name": it.name, "label": it.label_value(), "metadata": it.metadata, "changed": !unchanged})) } // ------------------------------------------------------------------------------------ project notes diff --git a/crates/engine/src/project_panel.rs b/crates/engine/src/project_panel.rs index 735f5b281..628a7e198 100644 --- a/crates/engine/src/project_panel.rs +++ b/crates/engine/src/project_panel.rs @@ -358,7 +358,7 @@ pub fn cell(p: &Project, it: &ProjectItem, column: &str) -> (String, SortKey) { }; match column { "Name" => text(it.name.clone()), - "Label" => text(it.label.name().to_string()), + "Label" => text(it.label_value()), "Media Type" => text(it.type_label().to_string()), "Frame Rate" if has_v => (format!("{} fps", rate.label()), SortKey::Num(rate.num as f64 / rate.den.max(1) as f64)), "Frame Rate" => (String::new(), SortKey::Num(0.0)), @@ -1047,7 +1047,7 @@ fn items(s: &mut Session, p: &Value) -> Result { for id in items { let Some(it) = pr.item(id) else { continue }; let cells: serde_json::Map = view.columns.iter().map(|c| (c.name.clone(), json!(cell(pr, it, &c.name).0))).collect(); - rows.push(json!({"item": id.0, "name": it.name, "label": it.label.name(), "depth": depth, "cells": cells})); + rows.push(json!({"item": id.0, "name": it.name, "label": it.label_value(), "depth": depth, "cells": cells})); } } walk(pr, bin, view, 0, recursive, &mut rows); diff --git a/crates/engine/src/project_tools.rs b/crates/engine/src/project_tools.rs index 7db708397..12fa0e8de 100644 --- a/crates/engine/src/project_tools.rs +++ b/crates/engine/src/project_tools.rs @@ -678,6 +678,7 @@ fn new_from_template(s: &mut Session, p: &Value) -> Result { proj.name = name.clone(); proj.root.name = name; s.project = std::sync::Arc::new(proj); + s.project_instance = std::sync::Arc::new(()); s.document_epoch = s.document_epoch.wrapping_add(1); s.history = Default::default(); s.history.limit = 200; diff --git a/crates/engine/src/scene_detect.rs b/crates/engine/src/scene_detect.rs index 0ec63b3ff..7b9eec162 100644 --- a/crates/engine/src/scene_detect.rs +++ b/crates/engine/src/scene_detect.rs @@ -286,6 +286,9 @@ fn apply_results(s: &mut Session, seq_id: ItemId, apply: SceneApply, results: Ve continue; } m.markers.push(Marker { + url: String::new(), + author: String::new(), + custom_color: None, id: MarkerId(id), start: *mt, duration: Tick::ZERO, diff --git a/crates/engine/src/sequence_extras.rs b/crates/engine/src/sequence_extras.rs index 28fef8be9..839048016 100644 --- a/crates/engine/src/sequence_extras.rs +++ b/crates/engine/src/sequence_extras.rs @@ -83,7 +83,7 @@ pub(crate) fn commands() -> Vec { "Add Flash Cue Marker…", &["Markers"], None, - r#"{"time":ticks?,"name":str?,"comment":str?,"color":label?}"#, + r#"{"time":ticks?,"name":str?,"comment":str?,"color":label_or_hex?,"author":str?}"#, has_seq, add_flash_cue, ), @@ -543,11 +543,12 @@ fn add_flash_cue(s: &mut Session, p: &Value) -> Result { let n = s.active_sequence().map(|q| q.markers.iter().filter(|m| m.kind == MarkerKind::FlashCue).count()).unwrap_or(0) + 1; let name = str_p(p, "name").map(str::to_string).unwrap_or_else(|| format!("Cue Point {n}")); let comment = str_p(p, "comment").unwrap_or("").to_string(); - let color = marker_color_p(p, "markers.addFlashCue")?.unwrap_or(Label::Green); + let (color, custom_color) = marker_color_p(p, "markers.addFlashCue")?.unwrap_or((Label::Green, None)); + let author = crate::commands::marker_author_p(p, "markers.addFlashCue")?.unwrap_or_default(); let dur = u64_p(p, "durationFrames").map(|f| s.sequence_rate().tick_of(f as i64)).unwrap_or(Tick::ZERO); let id = s.edit_sequence("Add Flash Cue Marker", |q, ctx, _| { let id = MarkerId(ctx.alloc()); - q.markers.push(Marker { id, start: t, duration: dur, name, comment, kind: MarkerKind::FlashCue, color }); + q.markers.push(Marker { url: String::new(), author, custom_color, id, start: t, duration: dur, name, comment, kind: MarkerKind::FlashCue, color }); q.markers.sort_by_key(|m| m.start); Ok(id) })?; diff --git a/crates/engine/src/sequence_tools.rs b/crates/engine/src/sequence_tools.rs index 1e9e88302..9996dcc7d 100644 --- a/crates/engine/src/sequence_tools.rs +++ b/crates/engine/src/sequence_tools.rs @@ -198,7 +198,7 @@ fn groups() -> Vec<(&'static str, Vec)> { "Add Range Marker", &["Markers"], Some("Ctrl+Shift+M"), - r#"{"time":ticks?,"durationFrames":i64=1s,"duration":ticks?,"name":str?,"comment":str?,"color":label?}"#, + r#"{"time":ticks?,"durationFrames":i64=1s,"duration":ticks?,"name":str?,"comment":str?,"color":label_or_hex?,"author":str?}"#, has_seq, add_range_marker, ), @@ -207,7 +207,7 @@ fn groups() -> Vec<(&'static str, Vec)> { "Add Range Marker to In and Out", &["Markers"], Some("Ctrl+M"), - r#"{"name":str?,"comment":str?,"color":label?}"#, + r#"{"name":str?,"comment":str?,"color":label_or_hex?,"author":str?}"#, has_in_and_out, add_range_marker_in_out, ), @@ -218,9 +218,18 @@ fn groups() -> Vec<(&'static str, Vec)> { vec![ spec("markers.showAllMarkerColors", "Show All Marker Colors", &["Markers"], None, "{}", has_hidden_colors, |s, _| { s.state.hidden_marker_colors.clear(); + s.state.hidden_custom_marker_colors.clear(); Ok(json!({"hidden": []})) }), - spec("markers.filterColors", "Marker Colour Filter", &[], None, r#"{"hidden":[label]}|{"color":label,"visible":bool?}"#, always, filter_colors), + spec( + "markers.filterColors", + "Marker Colour Filter", + &[], + None, + r#"{"hidden":[label_or_hex]}|{"color":label_or_hex,"visible":bool?}"#, + always, + filter_colors, + ), ], ), ( @@ -231,7 +240,7 @@ fn groups() -> Vec<(&'static str, Vec)> { "Add Chapter Marker…", &["Markers"], None, - r#"{"time":ticks?,"name":str?,"comment":str?,"color":label?}"#, + r#"{"time":ticks?,"name":str?,"comment":str?,"color":label_or_hex?,"author":str?}"#, has_seq, |s, p| { let t = sequence_time_p(s, p, "", "markers.addChapter")?.unwrap_or(s.playhead()); @@ -289,7 +298,7 @@ fn has_split(s: &Session) -> std::result::Result<(), String> { if program || source { Ok(()) } else { Err("there are no split points".into()) } } fn has_hidden_colors(s: &Session) -> std::result::Result<(), String> { - if s.state.hidden_marker_colors.is_empty() { Err("all marker colours are shown".into()) } else { Ok(()) } + if s.state.hidden_marker_colors.is_empty() && s.state.hidden_custom_marker_colors.is_empty() { Err("all marker colours are shown".into()) } else { Ok(()) } } // ---------- selection follows playhead ---------- @@ -1117,10 +1126,11 @@ struct MarkerInput<'a> { fn add_marker(s: &mut Session, input: MarkerInput<'_>) -> Result { let MarkerInput { cmd, label, start, duration, kind, name, params } = input; let comment = str_p(params, "comment").unwrap_or("").to_string(); - let color = marker_color_p(params, cmd)?.unwrap_or(Label::Green); + let (color, custom_color) = marker_color_p(params, cmd)?.unwrap_or((Label::Green, None)); + let author = crate::commands::marker_author_p(params, cmd)?.unwrap_or_default(); s.edit_sequence(label, |q, ctx, _| { let id = MarkerId(ctx.alloc()); - q.markers.push(Marker { id, start, duration, name, comment, kind, color }); + q.markers.push(Marker { url: String::new(), author, custom_color, id, start, duration, name, comment, kind, color }); q.markers.sort_by_key(|m| m.start); Ok(id) }) @@ -1160,23 +1170,52 @@ fn add_range_marker_in_out(s: &mut Session, p: &Value) -> Result { } fn filter_colors(s: &mut Session, p: &Value) -> Result { - if let Some(a) = p.get("hidden").and_then(Value::as_array) { - let mut v = Vec::new(); - for x in a { - let n = x.as_str().unwrap_or_default(); - v.push(Label::from_name(n).ok_or_else(|| bad("markers.filterColors", format!("unknown colour `{n}`")))?); + let id = "markers.filterColors"; + let parse = |v: &Value| -> Result<(Label, Option)> { + crate::commands::marker_color_p(&json!({"color":v}), id)?.ok_or_else(|| bad(id, "need a color")) + }; + if let Some(value) = p.get("hidden") { + let a = value.as_array().filter(|a| a.len() <= 80).ok_or_else(|| bad(id, "hidden must contain at most 80 colors"))?; + let mut presets = Vec::new(); + let mut custom = Vec::new(); + for v in a { + let (preset, hex) = parse(v)?; + if let Some(hex) = hex { + if !custom.contains(&hex) { + custom.push(hex); + } + } else if !presets.contains(&preset) { + presets.push(preset); + } } - s.state.hidden_marker_colors = v; - } else if let Some(n) = str_p(p, "color") { - let c = Label::from_name(n).ok_or_else(|| bad("markers.filterColors", format!("unknown colour `{n}`")))?; - let hidden = s.state.hidden_marker_colors.contains(&c); - let visible = bool_p(p, "visible").unwrap_or(hidden); - s.state.hidden_marker_colors.retain(|x| *x != c); - if !visible { - s.state.hidden_marker_colors.push(c); + if custom.len() > 64 { + return Err(bad(id, "too many custom color filters")); + } + s.state.hidden_marker_colors = presets; + s.state.hidden_custom_marker_colors = custom; + } else if let Some(value) = p.get("color") { + let (preset, hex) = parse(value)?; + let visible = p.get("visible").map(|v| v.as_bool().ok_or_else(|| bad(id, "visible must be a boolean"))).transpose()?; + if let Some(hex) = hex { + let visible = visible.unwrap_or(s.state.hidden_custom_marker_colors.contains(&hex)); + s.state.hidden_custom_marker_colors.retain(|c| *c != hex); + if !visible { + if s.state.hidden_custom_marker_colors.len() >= 64 { + return Err(bad(id, "too many custom color filters")); + } + s.state.hidden_custom_marker_colors.push(hex); + } + } else { + let visible = visible.unwrap_or(s.state.hidden_marker_colors.contains(&preset)); + s.state.hidden_marker_colors.retain(|c| *c != preset); + if !visible { + s.state.hidden_marker_colors.push(preset); + } } } else { - return Err(bad("markers.filterColors", "need `hidden` or `color`")); + return Err(bad(id, "need hidden or color")); } - Ok(json!({"hidden": s.state.hidden_marker_colors.iter().map(|c| c.name()).collect::>()})) + Ok( + json!({"hidden":s.state.hidden_marker_colors.iter().map(|c|c.name().to_owned()).chain(s.state.hidden_custom_marker_colors.iter().map(|c|c.hex())).collect::>()}), + ) } diff --git a/crates/engine/src/sequence_tools_tests.rs b/crates/engine/src/sequence_tools_tests.rs index 07c3a2972..f63fdbdcd 100644 --- a/crates/engine/src/sequence_tools_tests.rs +++ b/crates/engine/src/sequence_tools_tests.rs @@ -663,11 +663,11 @@ fn range_and_chapter_markers() { #[test] fn unknown_marker_colours_are_rejected_not_stored_as_green() { - // #857: `markers.add {color:"Red"}` used to succeed and store Green + // Unknown names and non-string values must not silently become Green. let mut s = demo(); let before = s.project.to_json(); for cmd in ["markers.add", "markers.addRange", "markers.addChapter", "markers.addFlashCue"] { - for color in [json!("Red"), json!("NotAColor"), json!(3)] { + for color in [json!("White"), json!("NotAColor"), json!(3)] { let err = s.execute(cmd, json!({"frame": 10, "color": color})).unwrap_err().to_string(); assert!(err.contains("Rose"), "{cmd}: the error lists the accepted names: {err}"); assert_eq!(s.project.to_json(), before, "{cmd} {color} must not add a marker"); @@ -676,7 +676,7 @@ fn unknown_marker_colours_are_rejected_not_stored_as_green() { let id = s.execute("markers.add", json!({"frame": 10, "color": "rose"})).unwrap()["marker"].as_u64().unwrap(); let colour = |s: &Session| s.active_sequence().unwrap().markers.iter().find(|m| m.id.0 == id).unwrap().color; assert_eq!(colour(&s), Label::Rose); - assert!(s.execute("markers.edit", json!({"marker": id, "color": "Red"})).is_err()); + assert!(s.execute("markers.edit", json!({"marker": id, "color": "NotAColor"})).is_err()); assert_eq!(colour(&s), Label::Rose, "a bad colour leaves the marker unchanged"); s.execute("markers.edit", json!({"marker": id, "color": "Teal"})).unwrap(); assert_eq!(colour(&s), Label::Teal); @@ -691,6 +691,7 @@ fn edit_marker_sets_every_field_in_one_undo_step_and_rejects_hostile_values() { let before = s.project.to_json(); for p in [ json!({"marker": id, "durationFrames": -1}), + json!({"marker": id, "durationFrames": 10_000_001}), json!({"marker": id, "durationFrames": i64::MAX}), json!({"marker": id, "durationFrames": "12"}), json!({"marker": id, "durationFrames": 1.5}), diff --git a/crates/engine/src/source_monitor.rs b/crates/engine/src/source_monitor.rs index 4aa7c4e19..0e633085c 100644 --- a/crates/engine/src/source_monitor.rs +++ b/crates/engine/src/source_monitor.rs @@ -87,6 +87,7 @@ pub fn source_command(id: &str) -> bool { | "markers.clearCurrent" | "markers.clearAll" | "markers.edit" + | "markers.delete" ) } @@ -196,7 +197,8 @@ pub(crate) fn route(s: &mut Session, id: &str, p: &Value) -> Result Result Result { - let marker = MarkerId(p.get("marker").and_then(Value::as_u64).ok_or_else(|| bad(id, "need marker id"))?); - let name = text(p, "name", id)?.map(str::to_string); - let comment = text(p, "comment", id)?.map(str::to_string); - let color = color(p, id)?; - let duration = if p.get("durationFrames").is_some_and(|v| !v.is_null()) { Some(duration(p, &view, id)?) } else { None }; - let kind = crate::commands::marker_kind_p(p)?; - s.edit("Edit Source Marker", |pr, _| { - let m = marker_list(pr, view.media)?.iter_mut().find(|m| m.id == marker).ok_or_else(|| bad(id, "no such Source marker"))?; - if let Some(name) = name { - m.name = name; - } - if let Some(kind) = kind { - m.kind = kind; - } - if let Some(comment) = comment { - m.comment = comment; - } - if let Some(color) = color { - m.color = color; - } - if let Some(d) = duration { - m.duration = d.min(Tick(view.end.0.saturating_sub(m.start.0))); - } - Ok(()) - })?; - } _ => return Err(bad(id, "unsupported Source command")), } Ok(Some(Value::Null)) @@ -277,7 +263,7 @@ pub(crate) fn open_clip(s: &mut Session, clip: ClipId) -> Result { fn text<'a>(p: &'a Value, key: &str, id: &str) -> Result> { p.get(key).map(|v| v.as_str().ok_or_else(|| bad(id, format!("{key} must be text")))).transpose() } -fn color(p: &Value, id: &str) -> Result> { +fn color(p: &Value, id: &str) -> Result)>> { crate::commands::marker_color_p(p, id) } fn duration(p: &Value, view: &crate::clip_ops::SourceView, id: &str) -> Result { @@ -346,16 +332,24 @@ mod tests { s.execute("source.setPlayhead", json!({"time":cursor.0})).unwrap(); let before = s.project.to_json(); let qbefore = s.active_sequence().unwrap().clone(); - let marker = s.execute("markers.add", json!({"target":"source","name":"Cue"})).unwrap()["marker"].as_u64().unwrap(); + let marker = + s.execute("markers.add", json!({"target":"source","name":"Cue","color":"#FFFFFF","author":"Source editor"})).unwrap()["marker"].as_u64().unwrap(); let v = source_view(&s, ItemId(5)).unwrap(); assert_eq!(v.markers.len(), 1); + assert_eq!(v.markers[0].rgb(), [255; 3]); + assert_eq!(v.markers[0].author, "Source editor"); assert_eq!(v.markers[0].start, cursor); assert_eq!(s.active_sequence().unwrap(), &qbefore); s.execute("edit.undo", json!({})).unwrap(); assert_eq!(s.project.to_json(), before); s.execute("edit.redo", json!({})).unwrap(); - s.execute("markers.edit", json!({"target":"source","marker":marker,"name":"Changed"})).unwrap(); + s.execute("markers.edit", json!({"target":"source","marker":marker,"name":"Changed","color":"#000000","author":"Other editor"})).unwrap(); assert_eq!(source_view(&s, ItemId(5)).unwrap().markers[0].name, "Changed"); + assert_eq!(source_view(&s, ItemId(5)).unwrap().markers[0].rgb(), [0; 3]); + assert_eq!(source_view(&s, ItemId(5)).unwrap().markers[0].author, "Other editor"); + s.execute("markers.edit", json!({"target":"source","marker":marker,"color":"CYAN"})).unwrap(); + assert_eq!(source_view(&s, ItemId(5)).unwrap().markers[0].color, Label::Teal); + assert_eq!(source_view(&s, ItemId(5)).unwrap().markers[0].custom_color, None); s.execute("markers.clearCurrent", json!({"target":"source"})).unwrap(); assert!(source_view(&s, ItemId(5)).unwrap().markers.is_empty()); } diff --git a/crates/engine/tests/marker_properties.rs b/crates/engine/tests/marker_properties.rs new file mode 100644 index 000000000..702022251 --- /dev/null +++ b/crates/engine/tests/marker_properties.rs @@ -0,0 +1,520 @@ +use filmcraft_engine::Session; +use filmcraft_project::{HexColor, Label}; +use serde_json::json; + +#[test] +fn review_null_author_is_omitted_for_creation_and_preserves_existing_author() { + let mut s = Session::default(); + s.execute("file.newSequence", json!({})).unwrap(); + for cmd in ["markers.add", "markers.addRange"] { + let id = s.execute(cmd, json!({"frame":0,"durationFrames":10,"author":null})).unwrap()["marker"].as_u64().unwrap(); + s.execute("markers.edit", json!({"marker":id,"author":"Editor"})).unwrap(); + let before = s.project.to_json(); + let history = s.history.undo.len(); + s.execute("markers.edit", json!({"marker":id,"author":null,"name":"Updated"})).unwrap(); + let marker = s.active_sequence().unwrap().markers.iter().find(|m| m.id.0 == id).unwrap(); + assert_eq!(marker.author, "Editor"); + assert_eq!(marker.name, "Updated"); + assert_eq!(s.history.undo.len(), history + 1); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); + } +} + +#[test] +fn web_link_url_is_separate_from_notes_atomic_persistent_and_optional_in_old_projects() { + use filmcraft_project::MarkerKind; + let mut s = Session::default(); + s.execute("file.newSequence", json!({})).unwrap(); + let id = s.execute("markers.add", json!({"frame":0,"name":"Review","comment":"Use take two"})).unwrap()["marker"].as_u64().unwrap(); + let sequence = s.state.active_sequence.unwrap(); + let before = s.project.to_json(); + let history = s.history.undo.len(); + let url = "https://example.com/review?take=2#notes"; + s.execute("markers.edit", json!({"marker":id,"kind":"webLink","url":format!(" {url} "),"comment":"Awaiting approval"})).unwrap(); + let marker = s.active_sequence().unwrap().markers[0].clone(); + assert_eq!((&*marker.url, &*marker.comment, marker.kind), (url, "Awaiting approval", MarkerKind::WebLink)); + assert_eq!(s.history.undo.len(), history + 1); + let loaded = filmcraft_format::decode(&filmcraft_format::encode(&s.project, false)).unwrap(); + assert_eq!(loaded.project.sequence(sequence).unwrap().markers[0], marker); + assert_eq!(s.execute("sequence.inspect", json!({})).unwrap()["markers"][0]["url"], url); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); + s.execute("edit.redo", json!({})).unwrap(); + s.execute("markers.edit", json!({"marker":id,"kind":"comment","url":null})).unwrap(); + assert_eq!(s.active_sequence().unwrap().markers[0].url, url); + s.execute("markers.edit", json!({"marker":id,"kind":"webLink"})).unwrap(); + assert_eq!(s.active_sequence().unwrap().markers[0].url, url); + let mut legacy: serde_json::Value = serde_json::from_slice(&filmcraft_format::encode(&s.project, false)).unwrap(); + legacy["schema_version"] = json!(16); + fn remove_urls(value: &mut serde_json::Value) { + match value { + serde_json::Value::Object(map) => { + map.remove("url"); + for value in map.values_mut() { + remove_urls(value); + } + } + serde_json::Value::Array(values) => { + for value in values { + remove_urls(value); + } + } + _ => {} + } + } + remove_urls(&mut legacy); + let loaded = filmcraft_format::decode(&serde_json::to_vec(&legacy).unwrap()).unwrap(); + assert!(loaded.migrated()); + assert!(loaded.project.sequence(sequence).unwrap().markers[0].url.is_empty()); + for invalid in [ + json!("file:///tmp/a"), + json!("javascript:alert(1)"), + json!("https://user:pass@example.com"), + json!("https://a\nb"), + json!("https://"), + json!("https:example.com"), + json!("relative/path"), + json!(42), + json!({}), + json!(["https://example.com"]), + json!("x".repeat(8193)), + ] { + let before = s.project.to_json(); + let history = s.history.undo.len(); + assert!(s.execute("markers.edit", json!({"marker":id,"url":invalid,"name":"Must not change"})).is_err()); + assert_eq!(s.project.to_json(), before); + assert_eq!(s.history.undo.len(), history); + } + s.execute("markers.edit", json!({"marker":id,"url":""})).unwrap(); + assert!(s.active_sequence().unwrap().markers[0].url.is_empty()); + assert_eq!(s.active_sequence().unwrap().markers[0].comment, "Awaiting approval"); +} + +#[test] +fn review_close_project_renews_runtime_identity() { + let mut session = Session::default(); + session.execute("file.newSequence", json!({})).unwrap(); + let original = session.project_instance.clone(); + assert!(session.execute("file.closeProject", json!({})).is_err()); + assert!(std::sync::Arc::ptr_eq(&original, &session.project_instance), "a rejected close must retain the live project's identity"); + session.execute("file.closeProject", json!({"force":true})).unwrap(); + assert!(!std::sync::Arc::ptr_eq(&original, &session.project_instance)); + assert!(session.project.items.is_empty()); + assert!(!session.history.can_undo()); +} + +#[test] +fn marker_kinds_edit_atomically_and_reject_invalid_values() { + use filmcraft_project::MarkerKind; + let mut s = Session::default(); + s.execute("file.newSequence", json!({})).unwrap(); + let marker = s.execute("markers.add", json!({"frame":0,"name":"Original"})).unwrap()["marker"].as_u64().unwrap(); + for (value, expected) in [ + ("chapter", MarkerKind::Chapter), + ("segmentation", MarkerKind::Segmentation), + ("webLink", MarkerKind::WebLink), + ("flashCue", MarkerKind::FlashCue), + ("comment", MarkerKind::Comment), + ] { + let before = s.project.to_json(); + let history = s.history.undo.len(); + s.execute("markers.edit", json!({"marker":marker,"kind":value,"name":"Changed","comment":"Together"})).unwrap(); + assert_eq!(s.active_sequence().unwrap().markers[0].kind, expected); + assert_eq!(s.history.undo.len(), history + 1); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(s.active_sequence().unwrap().markers[0].kind, expected); + } + for kind in [json!("unknown"), json!(true), json!(42), json!([]), json!({})] { + let before = s.project.to_json(); + let history = s.history.undo.len(); + assert!(s.execute("markers.edit", json!({"marker":marker,"kind":kind,"name":"Must not change"})).is_err()); + assert_eq!(s.project.to_json(), before); + assert_eq!(s.history.undo.len(), history); + } +} + +#[test] +fn custom_marker_properties_survive_undo_and_project_reopen() { + let mut s = Session::default(); + s.execute("file.newSequence", json!({"name":"Review"})).unwrap(); + let id = s.execute("markers.add", json!({"frame":0,"name":"Note","author":"BENJI","color":"#ff80c0"})).unwrap()["marker"].as_u64().unwrap(); + let seq = s.state.active_sequence.unwrap(); + let original = s.active_sequence().unwrap().markers[0].clone(); + assert_eq!(original.rgb(), [255, 128, 192]); + assert_eq!(original.color_value(), "#FF80C0"); + assert_eq!(original.author, "BENJI"); + let loaded = filmcraft_format::decode(&filmcraft_format::encode(&s.project, false)).unwrap(); + assert_eq!(loaded.project.sequence(seq).unwrap().markers[0], original); + s.execute("markers.edit", json!({"marker":id,"color":"White"})).unwrap_err(); + s.execute("markers.edit", json!({"marker":id,"comment":"Keep"})).unwrap(); + assert_eq!(s.active_sequence().unwrap().markers[0].custom_color, original.custom_color); + s.execute("markers.edit", json!({"marker":id,"color":"Blue","author":""})).unwrap(); + let marker = &s.active_sequence().unwrap().markers[0]; + assert_eq!(marker.color, Label::Blue); + assert_eq!(marker.custom_color, None); + assert_eq!(marker.author, ""); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.active_sequence().unwrap().markers[0].custom_color, HexColor::parse("#FF80C0")); + assert_eq!(s.active_sequence().unwrap().markers[0].author, "BENJI"); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(s.active_sequence().unwrap().markers[0].custom_color, None); + let inspected = s.execute("sequence.inspect", json!({})).unwrap(); + assert_eq!(inspected["markers"][0]["author"], ""); + assert_eq!(inspected["markers"][0]["color"], "Blue"); +} + +#[test] +fn invalid_marker_properties_leave_project_and_undo_unchanged() { + let mut s = Session::default(); + s.execute("file.newSequence", json!({})).unwrap(); + let id = s.execute("markers.add", json!({})).unwrap()["marker"].as_u64().unwrap(); + for change in [ + json!({"color":"#123"}), + json!({"color":"#00000000"}), + json!({"color":"#€aaaa"}), + json!({"color":[0,0,0]}), + json!({"author":true}), + json!({"author":"a\nb"}), + json!({"author":"a".repeat(1025)}), + ] { + let before = s.project.to_json(); + let history = s.history.undo.len(); + let mut params = change; + params["marker"] = json!(id); + assert!(s.execute("markers.edit", params).is_err()); + assert_eq!(s.project.to_json(), before); + assert_eq!(s.history.undo.len(), history); + } +} + +#[test] +fn all_marker_creation_commands_accept_hex_and_author() { + let mut s = Session::default(); + s.execute("file.newSequence", json!({})).unwrap(); + for cmd in ["markers.add", "markers.addRange", "markers.addChapter", "markers.addFlashCue"] { + s.execute(cmd, json!({"name":"Cue", "durationFrames":10,"color":"#FFFFFF","author":"Editor"})).unwrap(); + let marker = s.active_sequence().unwrap().markers.last().unwrap(); + assert_eq!(marker.rgb(), [255; 3], "{cmd}"); + assert_eq!(marker.author, "Editor", "{cmd}"); + } +} + +#[test] +fn custom_palette_is_project_local_bounded_and_undoable() { + let mut s = Session::default(); + let before = s.project.to_json(); + for invalid in [json!("Rose"), json!("#123"), json!(true), json!(42)] { + assert!(s.execute("colors.addCustom", json!({"color":invalid})).is_err()); + assert_eq!(s.project.to_json(), before); + } + s.execute("colors.addCustom", json!({"color":"#ff80c0"})).unwrap(); + let history = s.history.undo.len(); + s.execute("colors.addCustom", json!({"color":"#FF80C0"})).unwrap(); + assert_eq!(s.history.undo.len(), history, "duplicates do not create edits"); + let loaded = filmcraft_format::decode(&filmcraft_format::encode(&s.project, false)).unwrap(); + assert_eq!(loaded.project.custom_colors, s.project.custom_colors); + assert!(Session::default().project.custom_colors.is_empty()); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); + s.execute("edit.redo", json!({})).unwrap(); + for value in 0..63 { + s.execute("colors.addCustom", json!({"color":format!("#{value:06X}")})).unwrap(); + } + assert_eq!(s.project.custom_colors.len(), 64); + let full = s.project.to_json(); + assert!(s.execute("colors.addCustom", json!({"color":"#FFFFFF"})).is_err()); + assert_eq!(s.project.to_json(), full); +} + +#[test] +fn custom_clip_colors_share_palette_and_survive_save_preset_switch_and_undo() { + let mut s = Session::default(); + s.execute("file.openDemoProject", json!({})).unwrap(); + let seq = s.state.active_sequence.unwrap(); + let clip = s.active_sequence().unwrap().video_tracks[0].items[0].clone(); + let before = s.project.to_json(); + let history = s.history.undo.len(); + s.execute("edit.label", json!({"clips":[clip.id.0],"label":"#12abcd","saveColor":true})).unwrap(); + let color = HexColor::parse("#12ABCD"); + assert_eq!(s.active_sequence().unwrap().find_item(clip.id).unwrap().1.custom_label_color, color); + assert_eq!(s.project.custom_colors, vec![color.unwrap()]); + s.execute("sequence.linkedSelection", json!({"on":false})).unwrap(); + s.execute("timeline.select", json!({"clips":[clip.id.0]})).unwrap(); + s.execute("edit.selectLabelGroup", json!({})).unwrap(); + assert_eq!(s.state.selection, vec![clip.id]); + assert_eq!(s.history.undo.len(), history + 1); + let loaded = filmcraft_format::decode(&filmcraft_format::encode(&s.project, false)).unwrap(); + assert_eq!(loaded.project.sequence(seq).unwrap().find_item(clip.id).unwrap().1.custom_label_color, color); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); + s.execute("edit.redo", json!({})).unwrap(); + s.execute("edit.label.violet", json!({"clips":[clip.id.0]})).unwrap(); + assert_eq!(s.active_sequence().unwrap().find_item(clip.id).unwrap().1.custom_label_color, None); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.active_sequence().unwrap().find_item(clip.id).unwrap().1.custom_label_color, color); + s.execute("edit.label", json!({"items":[clip.item.0],"label":"#12ABCD"})).unwrap(); + let mut project = (*s.project).clone(); + let created = project + .make_track_item( + clip.item, + filmcraft_project::TrackKind::Video, + clip.start, + filmcraft_time::TimeRange::new(clip.source_in, clip.duration), + s.sequence_rate(), + ) + .unwrap(); + assert_eq!(created.custom_label_color, color); + s.execute("project.select", json!({"items":[clip.item.0]})).unwrap(); + let copy = s.execute("edit.duplicate", json!({})).unwrap()["items"][0].as_u64().unwrap(); + assert_eq!(s.project.item(filmcraft_project::ItemId(copy)).unwrap().custom_label_color, color); + let before = s.project.to_json(); + for params in [json!({"label":"#XYZXYZ"}), json!({"label":"#FFFFFF","clips":[-1]}), json!({"label":"#FFFFFF","clips":[u64::MAX],"saveColor":true})] { + assert!(s.execute("edit.label", params).is_err()); + assert_eq!(s.project.to_json(), before); + } +} + +#[test] +fn metadata_label_edits_use_the_effective_color_and_clear_custom_overrides() { + let mut s = Session::default(); + s.execute("file.openDemoProject", json!({})).unwrap(); + let item = s.active_sequence().unwrap().video_tracks[0].items[0].item; + let preset = s.project.item(item).unwrap().label; + s.execute("edit.label", json!({"items":[item.0],"label":"#123456"})).unwrap(); + let metadata = s.execute("metadata.get", json!({"item":item.0})).unwrap(); + assert_eq!(metadata["fields"].as_array().unwrap().iter().find(|f| f["name"] == "Label").unwrap()["value"], "#123456"); + let result = s.execute("metadata.set", json!({"item":item.0,"field":"Label","value":preset.name()})).unwrap(); + assert_eq!(result["changed"], true); + assert_eq!(s.project.item(item).unwrap().custom_label_color, None); + s.execute("edit.undo", json!({})).unwrap(); + let result = s.execute("metadata.set", json!({"item":item.0,"field":"Label","value":"#123456"})).unwrap(); + assert_eq!(result["changed"], false); + assert_eq!(result["label"], "#123456"); + s.execute("metadata.set", json!({"item":item.0,"field":"Label","value":"#abcdef"})).unwrap(); + assert_eq!(s.project.item(item).unwrap().label_value(), "#ABCDEF"); + s.execute("metadata.set", json!({"item":item.0,"field":"Label","value":"Red"})).unwrap(); + assert_eq!(s.project.item(item).unwrap().label, Label::Rose); + assert_eq!(s.project.item(item).unwrap().custom_label_color, None); +} + +#[test] +fn marker_and_clip_aliases_resolve_to_the_same_unchanged_presets() { + let mut s = Session::default(); + s.execute("file.openDemoProject", json!({})).unwrap(); + let clip = s.active_sequence().unwrap().video_tracks[0].items[0].id; + for (alias, preset) in Label::COLOR_ALIASES { + let id = s.execute("markers.add", json!({"color":alias.to_lowercase()})).unwrap()["marker"].as_u64().unwrap(); + let marker = s.active_sequence().unwrap().markers.iter().find(|m| m.id.0 == id).unwrap(); + assert_eq!(marker.color, preset); + assert_eq!(marker.rgb(), preset.rgb()); + s.execute("edit.label", json!({"clips":[clip.0],"label":alias})).unwrap(); + assert_eq!(s.active_sequence().unwrap().find_item(clip).unwrap().1.label, preset); + s.execute("markers.filterColors", json!({"color":alias,"visible":false})).unwrap(); + assert!(s.state.hidden_marker_colors.contains(&preset)); + } + assert_eq!(Label::Rose.rgb(), [0x80, 0x18, 0x36]); + assert_eq!(Label::Mango.rgb(), [0x80, 0x3f, 0x17]); + assert_eq!(Label::Teal.rgb(), [0x1f, 0x4d, 0x45]); +} + +#[test] +fn marker_time_edits_are_sorted_exact_and_reject_hostile_ranges_atomically() { + let mut s = Session::default(); + s.execute("file.newSequence", json!({})).unwrap(); + let first = s.execute("markers.add", json!({"frame":10})).unwrap()["marker"].as_u64().unwrap(); + let second = s.execute("markers.add", json!({"frame":20})).unwrap()["marker"].as_u64().unwrap(); + let rate = s.sequence_rate(); + s.execute("markers.edit", json!({"marker":first,"frame":30,"durationFrames":5})).unwrap(); + let markers = &s.active_sequence().unwrap().markers; + assert_eq!(markers[0].id.0, second); + assert_eq!(markers[1].start, rate.tick_of(30)); + assert_eq!(markers[1].duration, rate.tick_of(5)); + for change in [ + json!({"frame":-1}), + json!({"frame":i64::MAX}), + json!({"durationFrames":-1}), + json!({"durationFrames":"2"}), + json!({"durationFrames":1.5}), + json!({"durationFrames":i64::MAX}), + json!({"time":i64::MAX}), + json!({"time":true}), + json!({"frame":1,"time":0}), + json!({"name":false}), + ] { + let before = s.project.to_json(); + let history = s.history.undo.len(); + let mut p = change; + p["marker"] = json!(first); + assert!(s.execute("markers.edit", p).is_err()); + assert_eq!(s.project.to_json(), before); + assert_eq!(s.history.undo.len(), history); + } + s.execute("markers.edit", json!({"marker":first,"durationFrames":0})).unwrap(); + assert_eq!(s.active_sequence().unwrap().markers[1].duration, filmcraft_time::Tick::ZERO); + s.execute("markers.edit", json!({"marker":first,"time":123})).unwrap(); + s.execute("markers.edit", json!({"marker":first,"author":"Preserve subframes"})).unwrap(); + assert_eq!(s.active_sequence().unwrap().markers[0].start.0, 123); +} + +#[test] +fn bulk_marker_changes_are_owner_scoped_atomic_and_one_undo() { + let mut s = Session::default(); + s.execute("file.newSequence", json!({})).unwrap(); + let owner = s.state.active_sequence.unwrap(); + let a = s.execute("markers.add", json!({"frame":10})).unwrap()["marker"].as_u64().unwrap(); + let b = s.execute("markers.add", json!({"frame":10})).unwrap()["marker"].as_u64().unwrap(); + s.execute("file.newSequence", json!({})).unwrap(); + let before = s.project.to_json(); + let history = s.history.undo.len(); + s.execute("markers.edit", json!({"sequence":owner.0,"markers":[a,b],"color":"#123456"})).unwrap(); + assert_eq!(s.history.undo.len(), history + 1); + assert!(s.project.sequence(owner).unwrap().markers.iter().all(|m| m.rgb() == [0x12, 0x34, 0x56])); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); + for params in [ + json!({"markers":[a,u64::MAX]}), + json!({"markers":[]}), + json!({"marker":a,"markers":[b]}), + json!({"markers":[-1]}), + json!({"marker":a,"target":"nonsense"}), + ] { + let mut p = params; + p["sequence"] = json!(owner.0); + for cmd in ["markers.edit", "markers.delete"] { + assert!(s.execute(cmd, p.clone()).is_err()); + assert_eq!(s.project.to_json(), before); + assert_eq!(s.history.undo.len(), history); + } + } + s.execute("markers.delete", json!({"sequence":owner.0,"marker":a})).unwrap(); + assert_eq!(s.project.sequence(owner).unwrap().markers.iter().map(|m| m.id.0).collect::>(), vec![b]); + s.execute("edit.undo", json!({})).unwrap(); + s.execute("markers.delete", json!({"sequence":owner.0,"markers":[a,b]})).unwrap(); + assert_eq!(s.history.undo.len(), history + 1); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); +} + +#[test] +fn source_marker_edits_keep_their_owner_and_enforce_bounds() { + let mut s = Session::default(); + s.execute("file.openDemoProject", json!({})).unwrap(); + let source = filmcraft_project::ItemId(5); + s.execute("source.open", json!({"item":source.0})).unwrap(); + let marker = s.execute("markers.add", json!({"target":"source","time":0})).unwrap()["marker"].as_u64().unwrap(); + let view = filmcraft_engine::clip_ops::source_view(&s, source).unwrap(); + let other = s.project.items.values().find(|i| i.as_media().is_some() && i.id != source).unwrap().id; + s.execute("source.open", json!({"item":other.0})).unwrap(); + s.execute( + "markers.edit", + json!({"target":"source","item":source.0,"marker":marker,"frame":1,"durationFrames":2,"kind":"webLink","url":"https://example.com/source"}), + ) + .unwrap(); + assert_eq!(filmcraft_engine::clip_ops::source_view(&s, source).unwrap().markers[0].start, view.rate.tick_of(1)); + assert_eq!(filmcraft_engine::clip_ops::source_view(&s, source).unwrap().markers[0].kind, filmcraft_project::MarkerKind::WebLink); + assert_eq!(filmcraft_engine::clip_ops::source_view(&s, source).unwrap().markers[0].url, "https://example.com/source"); + let before = s.project.to_json(); + let history = s.history.undo.len(); + for change in [json!({"time":view.end.0}), json!({"time":view.end.0-1,"durationFrames":2}), json!({"durationFrames":i64::MAX}), json!({"time":-1})] { + let mut p = change; + p["target"] = json!("source"); + p["item"] = json!(source.0); + p["marker"] = json!(marker); + assert!(s.execute("markers.edit", p).is_err()); + assert_eq!(s.project.to_json(), before); + assert_eq!(s.history.undo.len(), history); + } + assert!(s.execute("markers.delete", json!({"target":"source","marker":marker})).is_err()); + s.execute("markers.delete", json!({"target":"source","item":source.0,"marker":marker})).unwrap(); + assert!(filmcraft_engine::clip_ops::source_view(&s, source).unwrap().markers.is_empty()); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); +} + +#[test] +fn named_palette_management_preserves_assignments_and_roundtrips() { + let mut s = Session::default(); + s.execute("file.newSequence", json!({})).unwrap(); + s.execute("markers.add", json!({"color":"#123456"})).unwrap(); + s.execute("colors.addCustom", json!({"color":"#123456","name":"Review"})).unwrap(); + let before = s.project.to_json(); + s.execute("colors.updateCustom", json!({"color":"#123456","replacement":"#654321","name":"Approved"})).unwrap(); + assert_eq!(s.active_sequence().unwrap().markers[0].color_value(), "#123456"); + let new = HexColor::parse("654321").unwrap(); + assert_eq!(s.project.custom_color_names[&new], "Approved"); + let loaded = filmcraft_format::decode(&filmcraft_format::encode(&s.project, false)).unwrap(); + assert_eq!(loaded.project.custom_color_names, s.project.custom_color_names); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); + for (cmd, p) in [ + ("colors.updateCustom", json!({"color":"#123456","replacement":"oops"})), + ("colors.updateCustom", json!({"color":"#FFFFFF"})), + ("colors.addCustom", json!({"color":"#FFFFFF","name":"bad\nname"})), + ("colors.removeCustom", json!({"color":true})), + ] { + let history = s.history.undo.len(); + assert!(s.execute(cmd, p).is_err()); + assert_eq!(s.project.to_json(), before); + assert_eq!(s.history.undo.len(), history); + } + s.execute("colors.removeCustom", json!({"color":"#123456"})).unwrap(); + assert!(s.project.custom_colors.is_empty()); + assert!(s.project.custom_color_names.is_empty()); + assert_eq!(s.active_sequence().unwrap().markers[0].color_value(), "#123456"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); +} + +#[test] +fn subclip_marker_edits_share_parent_storage_but_obey_view_bounds() { + let mut s = Session::default(); + s.execute("file.openDemoProject", json!({})).unwrap(); + s.execute("source.open", json!({"item":5})).unwrap(); + let rate = filmcraft_engine::clip_ops::source_view(&s, filmcraft_project::ItemId(5)).unwrap().rate; + let inside = s.execute("markers.add", json!({"target":"source","time":rate.tick_of(15).0})).unwrap()["marker"].as_u64().unwrap(); + let outside = s.execute("markers.add", json!({"target":"source","time":rate.tick_of(5).0})).unwrap()["marker"].as_u64().unwrap(); + let sub = s.execute("clip.makeSubclip", json!({"item":5,"startFrame":10,"endFrame":30})).unwrap()["item"].as_u64().unwrap(); + s.execute("markers.edit", json!({"target":"source","item":sub,"marker":inside,"frame":20,"durationFrames":10,"author":"Shared"})).unwrap(); + let parent = filmcraft_engine::clip_ops::source_view(&s, filmcraft_project::ItemId(5)).unwrap(); + assert_eq!(parent.markers.iter().find(|m| m.id.0 == inside).unwrap().author, "Shared"); + let before = s.project.to_json(); + for p in [ + json!({"marker":outside,"name":"Hidden"}), + json!({"marker":inside,"frame":5}), + json!({"marker":inside,"durationFrames":11}), + json!({"markers":[inside,outside],"color":"Blue"}), + ] { + let mut p = p; + p["target"] = json!("source"); + p["item"] = json!(sub); + assert!(s.execute("markers.edit", p).is_err()); + assert_eq!(s.project.to_json(), before); + } + s.execute("markers.delete", json!({"target":"source","item":sub,"marker":inside})).unwrap(); + assert_eq!(filmcraft_engine::clip_ops::source_view(&s, filmcraft_project::ItemId(5)).unwrap().markers.len(), 1); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); +} + +#[test] +fn custom_marker_filters_share_show_all_and_reject_invalid_updates() { + let mut s = Session::default(); + s.execute("markers.filterColors", json!({"hidden":["Red","#123456"]})).unwrap(); + assert!(s.state.hidden_marker_colors.contains(&Label::Rose)); + assert_eq!(s.state.hidden_custom_marker_colors, vec![HexColor::parse("123456").unwrap()]); + let before = serde_json::to_value(&s.state).unwrap(); + for p in [json!({"hidden":["Blue","bad"]}), json!({"hidden":true}), json!({"color":"#123456","visible":"false"})] { + assert!(s.execute("markers.filterColors", p).is_err()); + assert_eq!(serde_json::to_value(&s.state).unwrap(), before); + } + s.execute("markers.showAllMarkerColors", json!({})).unwrap(); + assert!(s.state.hidden_marker_colors.is_empty()); + assert!(s.state.hidden_custom_marker_colors.is_empty()); + s.execute("markers.filterColors", json!({"color":"#123456","visible":false})).unwrap(); + assert!(s.is_enabled("markers.showAllMarkerColors")); + s.execute("markers.showAllMarkerColors", json!({})).unwrap(); + assert!(s.state.hidden_custom_marker_colors.is_empty()); +} diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index b77968dd1..d616d5e19 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -39,6 +39,8 @@ //! would drop them when saving. //! - **v15**: project notes (`Project::notes`, Window ▸ Project Notes). No-op step; older builds //! would drop them when saving. +//! - **v16**: marker authors, named project swatches, and exact marker/clip hex colors. +//! - **v17**: optional marker URLs, independent of notes and retained across marker-type changes. #![cfg_attr(not(test), deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable))] @@ -59,8 +61,10 @@ 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, v14_to_v15]; +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, v14_to_v15, + v15_to_v16, v16_to_v17, +]; /// The schema version this build writes (and the newest it reads). pub const SCHEMA_VERSION: u32 = MIGRATIONS.len() as u32 + 1; @@ -392,13 +396,23 @@ fn v14_to_v15(doc: Value) -> Result { Ok(doc) } +/// v15 → v16: existing colors remain presets; authors and the named custom palette default empty. +fn v15_to_v16(doc: Value) -> Result { + Ok(doc) +} + +/// v16 → v17: marker URLs default empty. Older builds must not silently drop saved URLs. +fn v16_to_v17(doc: Value) -> Result { + Ok(doc) +} + #[cfg(test)] mod tests { use super::*; #[test] fn schema_version_matches_table() { - assert_eq!(SCHEMA_VERSION, 15); + assert_eq!(SCHEMA_VERSION, 17); } #[test] diff --git a/crates/format/tests/fixtures.rs b/crates/format/tests/fixtures.rs index fa1b7027d..d92a474a0 100644 --- a/crates/format/tests/fixtures.rs +++ b/crates/format/tests/fixtures.rs @@ -291,6 +291,9 @@ fn next_id_is_raised_past_every_id_in_the_file() { let base = decode(&fixture("v1-edit.fcproj")).unwrap().project; let seq = ItemId(21); let marker = |id| Marker { + url: String::new(), + author: String::new(), + custom_color: None, id: MarkerId(id), start: Tick::ZERO, duration: Tick::ZERO, diff --git a/crates/interchange/src/aaf.rs b/crates/interchange/src/aaf.rs index 952ffa80c..068410b12 100644 --- a/crates/interchange/src/aaf.rs +++ b/crates/interchange/src/aaf.rs @@ -58,6 +58,7 @@ pub struct AafOptions { /// Export `sequence` as an AAF file. pub fn export(project: &Project, sequence: ItemId, opts: &AafOptions) -> Result<(Vec, Report)> { let mut report = Report::default(); + crate::warn_marker_metadata_loss(project, sequence, &mut report); let name = opts.name.clone().or_else(|| project.item(sequence).map(|i| i.name.clone())).unwrap_or_else(|| "Sequence".into()); let doc = comp::from_project(project, sequence, &name, &opts.media, comp::Nests::Compositions, &mut report)?; let version = if opts.small_sectors { filmcraft_cfb::Version::V3 } else { filmcraft_cfb::Version::V4 }; diff --git a/crates/interchange/src/common.rs b/crates/interchange/src/common.rs index 915c5a8a5..5e351473c 100644 --- a/crates/interchange/src/common.rs +++ b/crates/interchange/src/common.rs @@ -463,6 +463,7 @@ impl Builder { pub fn clip(&mut self, item: ItemId, kind: TrackKind, name: &str, start: Tick, duration: Tick, source_in: Tick) -> TrackItem { let label = self.p.item(item).map(|i| i.label).unwrap_or(Label::Iris); TrackItem { + custom_label_color: self.p.item(item).and_then(|i| i.custom_label_color), id: ClipId(self.p.alloc_id()), item, name: name.to_string(), diff --git a/crates/interchange/src/comp.rs b/crates/interchange/src/comp.rs index 32c8698f2..5595b0e10 100644 --- a/crates/interchange/src/comp.rs +++ b/crates/interchange/src/comp.rs @@ -653,6 +653,9 @@ fn marker_out(m: &Marker) -> CMarker { fn marker_in(b: &mut Builder, m: &CMarker) -> Marker { Marker { + url: String::new(), + author: String::new(), + custom_color: None, id: MarkerId(b.alloc()), start: m.start, duration: m.duration, diff --git a/crates/interchange/src/edl.rs b/crates/interchange/src/edl.rs index 194e0c65c..f1d520c7d 100644 --- a/crates/interchange/src/edl.rs +++ b/crates/interchange/src/edl.rs @@ -440,6 +440,9 @@ pub(crate) fn import(text: &str, opts: &ImportOptions, report: &mut Report) -> R let t = ctx.rec(*tc, ev.df); let id = MarkerId(ctx.b.alloc()); ctx.seq.markers.push(Marker { + url: String::new(), + author: String::new(), + custom_color: None, id, start: t, duration: Tick::ZERO, @@ -985,6 +988,7 @@ fn track_order(context: &TrackExport<'_>) -> u8 { } pub(crate) fn export(p: &Project, seq_id: ItemId, opts: &ExportOptions, report: &mut Report) -> Result { + crate::warn_marker_metadata_loss(p, seq_id, report); let seq = p.sequence(seq_id).ok_or(crate::Error::NoSequence(seq_id))?; let rate = seq.settings.frame_rate; let df = seq.settings.drop_frame && rate.supports_drop_frame(); diff --git a/crates/interchange/src/fcp7.rs b/crates/interchange/src/fcp7.rs index c896c6234..d5e618c0d 100644 --- a/crates/interchange/src/fcp7.rs +++ b/crates/interchange/src/fcp7.rs @@ -796,6 +796,9 @@ fn read_markers(n: Node, rate: FrameRate, b: &mut Builder) -> Vec { let duration = if out_f > in_f { rate.tick_of(out_f) - start } else { Tick::ZERO }; let color = child_text(m, "color").and_then(label_from).unwrap_or(Label::Green); out.push(Marker { + url: String::new(), + author: String::new(), + custom_color: None, id: MarkerId(b.alloc()), start, duration, @@ -827,6 +830,7 @@ struct Exp<'a, 'r> { } pub(crate) fn export(p: &Project, seq_id: ItemId, opts: &ExportOptions, report: &mut Report) -> Result { + crate::warn_marker_metadata_loss(p, seq_id, report); p.sequence(seq_id).ok_or(Error::NoSequence(seq_id))?; let mut x = Exp { p, diff --git a/crates/interchange/src/fcpxml.rs b/crates/interchange/src/fcpxml.rs index 614a2fcbf..559c07fe8 100644 --- a/crates/interchange/src/fcpxml.rs +++ b/crates/interchange/src/fcpxml.rs @@ -536,6 +536,9 @@ impl<'a, 'i> Imp<'a, 'i, '_> { let start = if src_in == Tick::ZERO && speed == 1.0 { rel } else { src_in + scaled(rel, speed) }; let d = attr_time(m, "duration").unwrap_or(Tick::ZERO); out.push(Marker { + url: String::new(), + author: String::new(), + custom_color: None, id: MarkerId(self.b.alloc()), start, duration: if d <= frame { Tick::ZERO } else { d }, @@ -704,6 +707,7 @@ struct Exp<'a, 'r> { } pub(crate) fn export(p: &Project, seq_id: ItemId, opts: &ExportOptions, report: &mut Report) -> Result { + crate::warn_marker_metadata_loss(p, seq_id, report); let seq = p.sequence(seq_id).ok_or(Error::NoSequence(seq_id))?; let mut x = Exp { p, diff --git a/crates/interchange/src/lib.rs b/crates/interchange/src/lib.rs index 04773c484..8daeca640 100644 --- a/crates/interchange/src/lib.rs +++ b/crates/interchange/src/lib.rs @@ -48,6 +48,47 @@ use serde::{Deserialize, Serialize}; pub use common::{file_url_to_path, path_to_file_url, resolve_path}; pub use comp::ExtractedMedia; +/// Warn once when an export cannot carry exact marker metadata, including nested/source markers. +fn warn_marker_metadata_loss(project: &Project, sequence: ItemId, report: &mut Report) { + if !project.custom_colors.is_empty() { + report.warn("This format does not preserve the saved custom-color palette or its swatch names. Use a FilmCraft project or OTIO to retain the palette."); + } + let mut pending = std::collections::BTreeSet::from([sequence]); + let mut seen = std::collections::BTreeSet::new(); + let extra = |m: &filmcraft_project::Marker| m.custom_color.is_some() || !m.author.is_empty() || !m.url.is_empty(); + while let Some(id) = pending.pop_first() { + if !seen.insert(id) { + continue; + } + let Some(item) = project.item(id) else { continue }; + let has_extra = item.custom_label_color.is_some() + || match &item.kind { + ItemKind::Sequence(seq) => { + let mut found = seq.markers.iter().any(extra); + for clip in seq.all_tracks().flat_map(|track| &track.items) { + found |= clip.markers.iter().any(extra) || clip.custom_label_color.is_some(); + if !seen.contains(&clip.item) { + pending.insert(clip.item); + } + } + found + } + ItemKind::Media(media) => media.markers.iter().any(extra), + ItemKind::Subclip { parent, .. } => { + if !seen.contains(parent) { + pending.insert(*parent); + } + false + } + _ => false, + }; + if has_extra { + report.warn("This format does not preserve marker authors or exact custom colors (including clip labels), or marker URLs. Use a FilmCraft project or OTIO for those properties."); + return; + } + } +} + /// An interchange format. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] pub enum Format { @@ -462,6 +503,17 @@ pub fn export_edl_per_track(project: &Project, sequence: ItemId, opts: &ExportOp /// top-level sequences. pub fn merge_into(target: &mut Project, fragment: Imported, bin: Option) -> Vec { let Imported { project: frag, sequences } = fragment; + for color in &frag.custom_colors { + if !target.custom_colors.contains(color) { + if target.custom_colors.len() >= 64 { + continue; + } + target.custom_colors.push(*color); + } + if let Some(name) = frag.custom_color_names.get(color) { + target.custom_color_names.entry(*color).or_insert_with(|| name.clone()); + } + } let mut items: HashMap = HashMap::new(); for id in frag.items.keys() { items.insert(*id, ItemId(target.alloc_id())); diff --git a/crates/interchange/src/omf.rs b/crates/interchange/src/omf.rs index 1d8ba8744..4fef8b8a7 100644 --- a/crates/interchange/src/omf.rs +++ b/crates/interchange/src/omf.rs @@ -40,6 +40,7 @@ pub struct OmfOptions { /// Export the audio of `sequence` as an OMF 2.0 file. pub fn export(project: &Project, sequence: ItemId, opts: &OmfOptions) -> Result<(Vec, Report)> { let mut report = Report::default(); + crate::warn_marker_metadata_loss(project, sequence, &mut report); let name = opts.name.clone().or_else(|| project.item(sequence).map(|i| i.name.clone())).unwrap_or_else(|| "Sequence".into()); let mut media = opts.media.clone(); media.audio_only = true; diff --git a/crates/interchange/src/otio.rs b/crates/interchange/src/otio.rs index b1c447125..13aa084ad 100644 --- a/crates/interchange/src/otio.rs +++ b/crates/interchange/src/otio.rs @@ -159,8 +159,8 @@ impl Exp<'_, '_> { fn seq_meta(&self, id: ItemId) -> Value { let Some((it, s)) = self.p.item(id).and_then(|it| Some((it, it.as_sequence()?))) else { return json!({}) }; json!({ - "item": id.0, - "label": it.label, + "item": id.0, "custom_colors": self.p.custom_colors, "custom_color_names": self.p.custom_color_names, + "label": it.label, "custom_label_color": it.custom_label_color, "settings": s.settings, "start_timecode": s.start_timecode, "master_volume_db": s.master_volume_db, @@ -245,7 +245,7 @@ impl Exp<'_, '_> { return json!({"OTIO_SCHEMA": "MissingReference.1", "name": name, "available_range": null, "metadata": {}}); }; let avail = tr(m.info.start_timecode.map(|f| m.frame_rate().tick_of(f)).unwrap_or(Tick::ZERO), m.info.duration, rate); - let meta = json!({"filmcraft": {"item": item.0, "label": self.p.item(item).map(|i| i.label), "info": m.info, "offline": m.offline}}); + let meta = json!({"filmcraft": {"item": item.0, "label": self.p.item(item).map(|i| i.label), "custom_label_color": self.p.item(item).and_then(|i| i.custom_label_color), "info": m.info, "offline": m.offline}}); match &m.media { MediaRef::Generator(g) => json!({ "OTIO_SCHEMA": "GeneratorReference.1", @@ -253,7 +253,7 @@ impl Exp<'_, '_> { "generator_kind": match g { Generator::ColorMatte { .. } => "SolidColor", Generator::BlackVideo => "black", Generator::BarsAndTone => "SMPTEBars", _ => "FilmCraftGenerator" }, "parameters": match g { Generator::ColorMatte { color } => json!({"color": color}), _ => json!({}) }, "available_range": avail, - "metadata": {"filmcraft": {"item": item.0, "generator": g, "info": m.info}}, + "metadata": {"filmcraft": {"item": item.0, "generator": g, "info": m.info, "label": self.p.item(item).map(|i| i.label), "custom_label_color": self.p.item(item).and_then(|i| i.custom_label_color)}}, }), MediaRef::File { path } => { let url = match &self.opts.relative_to { @@ -280,7 +280,7 @@ impl Exp<'_, '_> { effects.push(json!({"OTIO_SCHEMA": "LinearTimeWarp.1", "name": "", "effect_name": "LinearTimeWarp", "time_scalar": s, "metadata": {}})); } let meta = json!({"filmcraft": { - "label": it.label, "link": it.link, "group": it.group, "effects": it.effects, + "label": it.label, "custom_label_color": it.custom_label_color, "link": it.link, "group": it.group, "effects": it.effects, "gain_db": it.gain_db, "scale_to_frame": it.scale_to_frame, "source_in_ticks": it.source_in.0, "duration_ticks": it.duration.0, }}); @@ -346,7 +346,7 @@ fn marker_json(m: &Marker, rate: FrameRate) -> Value { "comment": m.comment, "color": color_name(m.color), "marked_range": tr(m.start, m.duration, rate), - "metadata": {"filmcraft": {"kind": m.kind, "label": m.color, "start_ticks": m.start.0, "duration_ticks": m.duration.0}}, + "metadata": {"filmcraft": {"kind": m.kind, "url": m.url, "label": m.color, "custom_color": m.custom_color, "author": m.author, "start_ticks": m.start.0, "duration_ticks": m.duration.0}}, }) } @@ -448,6 +448,39 @@ fn first_rate(v: &Value) -> Option { } impl Imp<'_, '_> { + /// Union each timeline's saved palette into the project's 64 retained colors. + /// Keep the first valid name, fill missing names even at capacity, and never keep orphan names. + fn merge_palette(&mut self, meta: Option<&Value>) { + if let Some(colors) = meta.and_then(|m| m.get("custom_colors")).and_then(Value::as_array) { + let mut overflow = false; + for color in colors.iter().filter_map(Value::as_str).filter_map(filmcraft_project::HexColor::parse) { + if self.b.p.custom_colors.contains(&color) { + continue; + } + if self.b.p.custom_colors.len() >= 64 { + overflow = true; + break; + } + self.b.p.custom_colors.push(color); + } + if overflow { + self.report.warn("OTIO saved palettes exceed the 64-color project limit; additional colors were not imported."); + } + } + if let Some(names) = meta.and_then(|m| m.get("custom_color_names")).and_then(Value::as_object) { + // Palette membership bounds retained names to 64; orphan input entries consume no slots. + for (key, value) in names { + if let (Some(color), Some(name)) = (filmcraft_project::HexColor::parse(key), value.as_str()) + && self.b.p.custom_colors.contains(&color) + && name.len() <= 128 + && !name.chars().any(char::is_control) + { + self.b.p.custom_color_names.entry(color).or_insert_with(|| name.to_owned()); + } + } + } + } + fn timeline(&mut self, tl: &Value) -> ItemId { let name = name_of(tl); let name = if name.is_empty() { self.opts.name.clone().unwrap_or_else(|| "Timeline".into()) } else { name }; @@ -461,6 +494,8 @@ impl Imp<'_, '_> { _ => tl, }; let meta = fc(tl).or_else(|| fc(stack)); + self.merge_palette(meta); + let rate = tl.get("global_start_time").and_then(read_rate).or_else(|| first_rate(stack)).unwrap_or_default(); let id = self.sequence(stack, &name, meta, rate); if let Some(st) = tl.get("global_start_time").and_then(read_rt) @@ -480,6 +515,9 @@ impl Imp<'_, '_> { { it.label = l; } + if let Some(it) = self.b.p.item_mut(id) { + it.custom_label_color = meta.and_then(|m| m.get("custom_label_color")).and_then(Value::as_str).and_then(filmcraft_project::HexColor::parse); + } let mut seq = empty_sequence(settings); if let Some(m) = meta { seq.start_timecode = m.get("start_timecode").and_then(Value::as_i64).unwrap_or(0); @@ -622,6 +660,7 @@ impl Imp<'_, '_> { } } let Some(m) = meta else { return }; + ti.custom_label_color = m.get("custom_label_color").and_then(Value::as_str).and_then(filmcraft_project::HexColor::parse); if let Some(l) = m.get("label").and_then(|l| serde_json::from_value::