From e043fc0bb02b18086d92a4d521d17d3a3cff61b3 Mon Sep 17 00:00:00 2001 From: nanox333 Date: Tue, 13 Oct 2026 15:47:05 +0200 Subject: [PATCH 01/29] #712: color pickers get an eyedropper everywhere The Character panel's pipette (#938) only sampled text. Every color picker now has an eyedropper in its hex row: click it, then click in the Composition panel to take the color under the pointer. The picker is one shared popup, so Solid Settings, Fill and Stroke, attached images, effects and the rest get it without caller changes. - color_picker: the eyedropper arms the picker. Escape or a second click disarms it and the picker stays open. While armed, the viewer shows a crosshair, a "Click to sample color" chip and a swatch under the pointer, and the next press on the viewer samples into the picker. A press over the picker or a dialog is not a sample, and a press elsewhere closes the picker as before. - The Character panel's pipette glyph moves to color_picker::draw_pipette, so both show the same icon. - Automation id colorPicker.eyedropper. - Modal dialogs (modal_fit) and the picker's popup register their rects as overlays, so a press on them never samples the viewer beneath. Not compiled, tested or rustfmt-checked: the machine it was written on has no Rust toolchain. Run cargo xtask ci before relying on it. Pushed to the fork for testing only. --- crates/ui-egui/src/color_picker.rs | 233 ++++++++++++++++++- crates/ui-egui/src/lib.rs | 3 + crates/ui-egui/src/panels/dialogs.rs | 2 + crates/ui-egui/src/panels/effect_controls.rs | 26 +++ crates/ui-egui/src/panels/fx_widgets.rs | 7 +- crates/ui-egui/src/panels/text_panels.rs | 3 +- 6 files changed, 261 insertions(+), 13 deletions(-) diff --git a/crates/ui-egui/src/color_picker.rs b/crates/ui-egui/src/color_picker.rs index 35d8e4eb4..a692d78db 100644 --- a/crates/ui-egui/src/color_picker.rs +++ b/crates/ui-egui/src/color_picker.rs @@ -1,6 +1,7 @@ //! The colour picker every colour input opens, laid out like After Effects' Color Picker: a //! square and a slider for the channel chosen with the H / S / B / R / G / B buttons, the new -//! colour over the original, the HSB and RGB values, the hex code, and OK / Cancel. +//! colour over the original, the HSB and RGB values, the hex code with an eyedropper that samples +//! the Composition viewer, and OK / Cancel. //! //! Colours are sRGB components (0–1), as the project stores them. Changes apply as they are made //! (After Effects' Preview); Cancel or Escape goes back to the colour the picker opened with. @@ -30,29 +31,49 @@ pub fn srgb_color_button(ui: &mut Ui, c: &mut [f32; 3]) -> Response { } /// The colour picker as a popup at `pos`, bound to `c`, opened with [`open_popup`]. Returns true -/// when the colour changed (Cancel, Escape and the original swatch change it back). +/// when the colour changed (Cancel, Escape and the original swatch change it back; the eyedropper +/// changes it to the colour it samples). pub fn color_popup(ui: &mut Ui, id: egui::Id, pos: egui::Pos2, c: &mut [f32; 3]) -> bool { if !popup_is_open(ui, id) { return false; } let orig_id = id.with("original"); let original = ui.data_mut(|d| *d.get_temp_mut_or_insert_with(orig_id, || *c)); + // Read before a sample is taken: the press that samples the viewer lands outside the picker, and + // must not close it. + let armed = armed_for(ui.ctx(), id); + let mut sampled = false; + if let Some(rgb) = take_sample(ui.ctx(), id) + && rgb != *c + { + *c = rgb; + sampled = true; + } let mut out = Outcome::default(); let area = egui::Area::new(id.with("area")).order(egui::Order::Foreground).fixed_pos(pos + vec2(0.0, 4.0)).show(ui.ctx(), |ui| { egui::Frame::popup(ui.style()).show(ui, |ui| out = color_picker(ui, id, c, original)); }); - // Escape is the picker's: consume it, so nothing later this pass takes it too. + // Escape is the picker's: consume it, so nothing later this pass takes it too. While the + // eyedropper is armed, Escape cancels it and the picker stays open. if ui.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Escape)) { - out.close = Some(false); + if armed { + disarm(ui.ctx(), id); + } else { + out.close = Some(false); + } } - if pressed_outside(ui.ctx(), &area.response) { + // A press outside closes the picker, unless it is the armed eyedropper's press on the viewer. + let on_viewer = armed && eyedropper_point(ui.ctx()).is_some(); + if pressed_outside(ui.ctx(), &area.response) && !on_viewer { out.close.get_or_insert(true); } + note_overlay(ui.ctx(), area.response.rect); if let Some(keep) = out.close { if !keep && *c != original { *c = original; out.changed = true; } + disarm(ui.ctx(), id); ui.data_mut(|d| { d.insert_temp(id.with("open"), false); d.remove::<[f32; 3]>(orig_id); @@ -60,11 +81,12 @@ pub fn color_popup(ui: &mut Ui, id: egui::Id, pos: egui::Pos2, c: &mut [f32; 3]) d.remove::(open_pass_id()); }); } else { + refresh_armed(ui.ctx(), id); // Still open after this pass: dialogs leave Escape to the picker (see `open_recently`). let pass = ui.ctx().cumulative_pass_nr(); ui.data_mut(|d| d.insert_temp(open_pass_id(), pass)); } - out.changed + out.changed || sampled } fn open_pass_id() -> egui::Id { @@ -80,6 +102,177 @@ pub fn open_recently(ctx: &egui::Context) -> bool { pass.is_some_and(|p| p.saturating_add(1) >= ctx.cumulative_pass_nr()) } +/// The armed eyedropper of a picker: the picker's popup id, the pass it was last drawn in, and the +/// colour the viewer sampled for it, if one is waiting for the picker's next draw. It lives in the +/// egui context, since the viewer that samples has no picker in hand. +#[derive(Clone, Copy)] +struct Armed { + popup: egui::Id, + pass: u64, + sample: Option<[f32; 3]>, +} + +fn armed_key() -> egui::Id { + egui::Id::new("color-picker-eyedropper") +} + +/// The armed eyedropper, if its picker was drawn in this pass or the last: one whose picker stops +/// being drawn (its dialog gone) lapses. +fn armed_record(ctx: &egui::Context) -> Option { + let armed: Option = ctx.data(|d| d.get_temp(armed_key())); + let pass = ctx.cumulative_pass_nr(); + armed.filter(|a| a.pass.saturating_add(1) >= pass) +} + +fn armed_for(ctx: &egui::Context, popup: egui::Id) -> bool { + armed_record(ctx).is_some_and(|a| a.popup == popup) +} + +/// Some picker's eyedropper is armed: a press on the Composition viewer samples the frame for it. +pub fn eyedropper_armed(ctx: &egui::Context) -> bool { + armed_record(ctx).is_some() +} + +fn arm(ctx: &egui::Context, popup: egui::Id) { + let pass = ctx.cumulative_pass_nr(); + ctx.data_mut(|d| d.insert_temp(armed_key(), Armed { popup, pass, sample: None })); +} + +fn disarm(ctx: &egui::Context, popup: egui::Id) { + let armed: Option = ctx.data(|d| d.get_temp(armed_key())); + if armed.is_some_and(|a| a.popup == popup) { + ctx.data_mut(|d| d.remove::(armed_key())); + } +} + +/// Keeps `popup`'s eyedropper armed while its picker is drawn. +fn refresh_armed(ctx: &egui::Context, popup: egui::Id) { + if let Some(armed) = armed_record(ctx).filter(|a| a.popup == popup) { + let pass = ctx.cumulative_pass_nr(); + ctx.data_mut(|d| d.insert_temp(armed_key(), Armed { pass, ..armed })); + } +} + +/// The colour the viewer sampled for `popup`, once: taking it disarms the eyedropper. +fn take_sample(ctx: &egui::Context, popup: egui::Id) -> Option<[f32; 3]> { + let armed = armed_record(ctx).filter(|a| a.popup == popup)?; + let sample = armed.sample?; + disarm(ctx, popup); + Some(sample) +} + +/// The viewer sampled `rgb` for the armed picker, which takes it on its next draw. +pub fn deliver_sample(ctx: &egui::Context, rgb: [f32; 3]) { + if let Some(armed) = armed_record(ctx) { + ctx.data_mut(|d| d.insert_temp(armed_key(), Armed { sample: Some(rgb), ..armed })); + ctx.request_repaint(); + } +} + +fn viewer_key() -> egui::Id { + egui::Id::new("color-picker-viewer") +} + +/// The Composition viewer's rect as it was drawn this pass: where an armed eyedropper samples. +pub fn note_viewer(ctx: &egui::Context, rect: Rect) { + let pass = ctx.cumulative_pass_nr(); + ctx.data_mut(|d| d.insert_temp(viewer_key(), (pass, rect))); +} + +fn overlay_key() -> egui::Id { + egui::Id::new("color-picker-overlays") +} + +/// A popup or dialog drawn over the viewer: a press inside `rect` is not a sample. +pub fn note_overlay(ctx: &egui::Context, rect: Rect) { + let pass = ctx.cumulative_pass_nr(); + ctx.data_mut(|d| { + // This pass's and the last's: the viewer may be drawn before an overlay in a pass. + let mut rects = d.get_temp::>(overlay_key()).unwrap_or_default(); + rects.retain(|(p, _)| p.saturating_add(1) >= pass); + rects.push((pass, rect)); + d.insert_temp(overlay_key(), rects); + }); +} + +/// Whether `pos` is over a popup or dialog drawn this pass or the last. +fn overlaid(ctx: &egui::Context, pos: egui::Pos2) -> bool { + let pass = ctx.cumulative_pass_nr(); + let rects: Option> = ctx.data(|d| d.get_temp(overlay_key())); + rects.unwrap_or_default().iter().any(|(p, r)| p.saturating_add(1) >= pass && r.contains(pos)) +} + +/// Where an armed eyedropper samples: the pointer, if it is over the viewer and not over a popup or +/// dialog. The pointer is read rather than the viewer's responses, since a modal dialog over the +/// viewer takes its clicks. +pub fn eyedropper_point(ctx: &egui::Context) -> Option { + let viewer: Option<(u64, Rect)> = ctx.data(|d| d.get_temp(viewer_key())); + let viewer = viewer.filter(|(p, _)| p.saturating_add(1) >= ctx.cumulative_pass_nr())?.1; + let pos = ctx.input(|i| i.pointer.interact_pos())?; + (viewer.contains(pos) && !overlaid(ctx, pos)).then_some(pos) +} + +/// The press that samples for an armed eyedropper this frame, if there is one. +pub fn sample_press(ctx: &egui::Context) -> Option { + if ctx.input(|i| i.pointer.primary_pressed()) { eyedropper_point(ctx) } else { None } +} + +fn automation_key() -> egui::Id { + egui::Id::new("color-picker-eyedropper-rect") +} + +/// The eyedropper's rect, for the automation registry; [`take_automation`] hands it over. +fn note_automation(ctx: &egui::Context, rect: Rect) { + ctx.data_mut(|d| d.insert_temp(automation_key(), rect)); +} + +/// The eyedropper rect the last picker drawing noted, once. Pickers have no registry in hand, so the +/// app registers it each frame. +pub fn take_automation(ctx: &egui::Context) -> Option { + ctx.data_mut(|d| { + let rect = d.get_temp::(automation_key()); + d.remove::(automation_key()); + rect + }) +} + +/// The eyedropper of the hex row. A press arms it, or disarms it while armed; the next press on the +/// Composition viewer then samples the colour under the pointer into this picker. +fn eyedropper(ui: &mut Ui, popup: egui::Id) { + let armed = armed_for(ui.ctx(), popup); + let (rect, resp) = ui.allocate_exact_size(vec2(18.0, 18.0), Sense::click()); + let resp = resp.on_hover_text(tr("Eyedropper: click, then click in the Composition panel")); + resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, ui.is_enabled(), "Eyedropper")); + let fill = if armed { + ui.visuals().selection.bg_fill + } else if resp.hovered() { + ui.visuals().widgets.hovered.bg_fill + } else { + Color32::TRANSPARENT + }; + ui.painter().rect_filled(rect, 2.0, fill); + let col = if armed { ui.visuals().selection.stroke.color } else { ui.visuals().strong_text_color() }; + draw_pipette(ui.painter(), rect, col); + if resp.clicked() { + if armed { + disarm(ui.ctx(), popup); + } else { + arm(ui.ctx(), popup); + } + } + note_automation(ui.ctx(), rect); +} + +/// A pipette: a diagonal glass tube with a bulb at the top right and a tip at the bottom left, drawn +/// within `rect`. +pub fn draw_pipette(p: &egui::Painter, rect: Rect, col: Color32) { + let r = rect.shrink(rect.width() * 0.2); + let at = |x: f32, y: f32| pos2(r.min.x + x * r.width(), r.min.y + y * r.height()); + p.line_segment([at(0.08, 0.92), at(0.62, 0.38)], Stroke::new(2.2, col)); + p.circle_filled(at(0.76, 0.24), r.width() * 0.2, col); + p.line_segment([at(0.5, 0.3), at(0.7, 0.5)], Stroke::new(1.4, col)); +} + /// What the picker did this frame. #[derive(Default)] struct Outcome { @@ -237,6 +430,7 @@ fn color_picker(ui: &mut Ui, id: egui::Id, c: &mut [f32; 3], original: [f32; 3]) if hex_field(ui, id.with("hex"), &mut rgb) { pick = Pick { hsv: keep_hue(rgb_to_hsv(rgb), pick.hsv), rgb }; } + eyedropper(ui, id); }); }); }); @@ -429,3 +623,30 @@ pub fn hsv_to_rgb([h, s, v]: [f32; 3]) -> [f32; 3] { }; [f(5.0), f(3.0), f(1.0)] } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn armed_eyedropper_takes_one_sample_then_disarms() { + let ctx = egui::Context::default(); + let popup = egui::Id::new("test-picker"); + arm(&ctx, popup); + assert!(eyedropper_armed(&ctx)); + assert_eq!(take_sample(&ctx, popup), None); + deliver_sample(&ctx, [0.1, 0.2, 0.3]); + assert_eq!(take_sample(&ctx, popup), Some([0.1, 0.2, 0.3])); + assert!(!eyedropper_armed(&ctx)); + assert_eq!(take_sample(&ctx, popup), None); + } + + #[test] + fn points_over_a_dialog_are_not_samples() { + let ctx = egui::Context::default(); + let dialog = Rect::from_min_size(pos2(100.0, 100.0), vec2(120.0, 80.0)); + note_overlay(&ctx, dialog); + assert!(overlaid(&ctx, pos2(150.0, 150.0))); + assert!(!overlaid(&ctx, pos2(20.0, 20.0))); + } +} diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 105a075e7..67fa405f7 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1614,6 +1614,9 @@ impl EffectcraftApp { self.apply_gpu_failure(&ctx); self.run_dialog_answers(); self.auto.begin_frame(); + if let Some(rect) = crate::color_picker::take_automation(&ctx) { + self.auto.add("colorPicker.eyedropper", rect, "Eyedropper"); + } self.frames.set_context(&ctx); if self.session.render_job.is_some() { self.session.poll_render(); diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index d773fc433..2ed25070e 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -213,6 +213,8 @@ pub(crate) fn modal_fit(ctx: &egui::Context, title: &str, size: egui::Vec2, t: & ui.add_space(10.0); let title_h = ui.cursor().top() - ui.min_rect().top(); body(ui, (size.y - 36.0 - title_h).max(40.0)); + // A colour picker's eyedropper samples the viewer under the dialog, but not the dialog itself. + crate::color_picker::note_overlay(ui.ctx(), ui.min_rect().expand(18.0)); }); } diff --git a/crates/ui-egui/src/panels/effect_controls.rs b/crates/ui-egui/src/panels/effect_controls.rs index bad1db39f..b42047c75 100644 --- a/crates/ui-egui/src/panels/effect_controls.rs +++ b/crates/ui-egui/src/panels/effect_controls.rs @@ -1455,6 +1455,7 @@ pub fn viewer_hook( ) { let t = app.tokens; let ctx = ui.ctx().clone(); + crate::color_picker::note_viewer(&ctx, map.area); let mut actions: Actions = vec![]; let mut point_preview = None; if let Some(pick) = app.ui.fx_pick.clone() { @@ -1574,6 +1575,31 @@ pub fn viewer_hook( } } } + // A colour picker's eyedropper is armed (its popup stays open): the viewer shows the colour under + // the pointer, and the next press on it samples the frame into the picker. The press is read from + // the pointer, since a modal dialog over the viewer takes its clicks. + if crate::color_picker::eyedropper_armed(&ctx) { + let _ = ui.interact(map.area, egui::Id::new("viewer-popup-eyedropper"), Sense::click()); + ctx.set_cursor_icon(egui::CursorIcon::Crosshair); + app.viewer_pixels(); + let chip = Rect::from_min_size(map.area.min + vec2(10.0, 10.0), vec2(220.0, 22.0)); + painter.rect_filled(chip, 11.0, Color32::from_black_alpha(180)); + painter.text(chip.center(), Align2::CENTER_CENTER, "Click to sample color", Tokens::ui(11.5), t.text); + if let Some(hp) = crate::color_picker::eyedropper_point(&ctx) + && let Some(img) = &app.viewer_image + && let Some(s) = fw::sample_frame(img, super::viewer_tools::shown_region(&ctx, map.comp), map.to_comp(hp)) + { + let sw = Rect::from_min_size(hp + vec2(14.0, 14.0), vec2(26.0, 18.0)); + painter.rect_filled(sw, 2.0, Color32::from_rgb((s[0] * 255.0) as u8, (s[1] * 255.0) as u8, (s[2] * 255.0) as u8)); + painter.rect_stroke(sw, 2.0, Stroke::new(1.0, Color32::WHITE), egui::StrokeKind::Outside); + } + if let Some(pos) = crate::color_picker::sample_press(&ctx) + && let Some(img) = &app.viewer_image + && let Some(s) = fw::sample_frame(img, super::viewer_tools::shown_region(&ctx, map.comp), map.to_comp(pos)) + { + crate::color_picker::deliver_sample(&ctx, [s[0], s[1], s[2]]); + } + } let picking = app.ui.fx_pick.is_some() || app.ui.text_color_pick.is_some(); let point_pick = app.ui.fx_pick.clone().filter(|p| p.kind == "point"); if app.ui.viewer.show_layer_controls || point_pick.is_some() { diff --git a/crates/ui-egui/src/panels/fx_widgets.rs b/crates/ui-egui/src/panels/fx_widgets.rs index ca49c3fd0..bce037a10 100644 --- a/crates/ui-egui/src/panels/fx_widgets.rs +++ b/crates/ui-egui/src/panels/fx_widgets.rs @@ -171,12 +171,7 @@ pub fn eyedropper_button(ui: &mut egui::Ui, rect: Rect, active: bool, id: egui:: } else { t.text_dim }; - // A pipette: a diagonal glass tube with a bulb at the top right and a tip at the bottom left. - let r = rect.shrink(rect.width() * 0.2); - let at = |x: f32, y: f32| pos2(r.min.x + x * r.width(), r.min.y + y * r.height()); - p.line_segment([at(0.08, 0.92), at(0.62, 0.38)], Stroke::new(2.2, col)); - p.circle_filled(at(0.76, 0.24), r.width() * 0.2, col); - p.line_segment([at(0.5, 0.3), at(0.7, 0.5)], Stroke::new(1.4, col)); + crate::color_picker::draw_pipette(p, rect, col); resp } diff --git a/crates/ui-egui/src/panels/text_panels.rs b/crates/ui-egui/src/panels/text_panels.rs index ae2294f1f..ed5c531cb 100644 --- a/crates/ui-egui/src/panels/text_panels.rs +++ b/crates/ui-egui/src/panels/text_panels.rs @@ -204,7 +204,8 @@ pub fn character(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } let er = Rect::from_min_size(pos2(fs.max.x + 4.0, y + 1.0), vec2(18.0, 18.0)); let active = app.ui.text_color_pick.as_ref().is_some_and(|pick| pick.layer == target.as_ref().map_or(0, |tt| tt.layer) && pick.field == key); - let eye = super::fx_widgets::eyedropper_button(ui, er, active, egui::Id::new(("char-eye", key)), &t); + let eye = super::fx_widgets::eyedropper_button(ui, er, active, egui::Id::new(("char-eye", key)), &t) + .on_hover_text(tr("Eyedropper: click, then click in the Composition panel")); app.auto.add(&format!("character.{key}.eyedropper"), er, &format!("{label} eyedropper")); if eye.clicked() { if active { From 6442465bf1adaab9bac97a7207b46ffe47f78ad7 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 19 Oct 2026 12:59:32 -0400 Subject: [PATCH 02/29] CI: build the desktop app and CLI on Windows with MSVC, GNU, and GNU with bundled LLD (refs #1886) --- .github/workflows/windows-toolchains.yml | 70 ++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 .github/workflows/windows-toolchains.yml diff --git a/.github/workflows/windows-toolchains.yml b/.github/workflows/windows-toolchains.yml new file mode 100644 index 000000000..9c51e8acc --- /dev/null +++ b/.github/workflows/windows-toolchains.yml @@ -0,0 +1,70 @@ +# Windows builds with the toolchains contributors use, not just the MSVC one releases use: build +# the desktop app and the CLI with MSVC, with GNU (mingw-w64 from MSYS2, the setup CONTRIBUTING.md +# describes), and with GNU linked by Rust's bundled LLD plus mingw-w64's import libraries (#1886), +# then run each binary with `--version`. Both packages build in one `cargo build`, the way people +# build them, so features unify and the CLI links everything the desktop app pulls in. +name: Windows toolchains + +# Nightly on main (a dependency update is what brings in a new system library), and on PRs that +# change this workflow or the lockfile. +on: + schedule: + - cron: "41 5 * * *" + pull_request: + paths: + - .github/workflows/windows-toolchains.yml + - Cargo.lock + workflow_dispatch: + +concurrency: + group: windows-toolchains-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +permissions: + contents: read + +jobs: + build: + name: ${{ matrix.name }} + runs-on: windows-latest + timeout-minutes: 90 + strategy: + fail-fast: false + matrix: + include: + - { name: msvc, target: x86_64-pc-windows-msvc } + - { name: gnu, target: x86_64-pc-windows-gnu } + - { name: gnu-rust-lld, target: x86_64-pc-windows-gnu, rust-lld: true } + defaults: + run: + shell: bash + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Rust (stable-${{ matrix.target }}) + run: | + rustup toolchain install stable-${{ matrix.target }} --profile minimal --no-self-update + rustup default stable-${{ matrix.target }} + rustc -vV + - name: mingw-w64 from MSYS2 + if: matrix.target == 'x86_64-pc-windows-gnu' + run: /c/msys64/usr/bin/pacman -Sy --noconfirm --needed mingw-w64-x86_64-gcc + - name: Put it first on PATH + if: matrix.target == 'x86_64-pc-windows-gnu' && !matrix.rust-lld + run: | + echo 'C:\msys64\mingw64\bin' >> "$GITHUB_PATH" + - name: Link with Rust's bundled LLD and mingw-w64's import libraries + if: matrix.rust-lld + run: | + echo 'CARGO_TARGET_X86_64_PC_WINDOWS_GNU_LINKER=rust-lld' >> "$GITHUB_ENV" + echo 'CARGO_TARGET_X86_64_PC_WINDOWS_GNU_RUSTFLAGS=-C link-self-contained=yes -L native=C:/msys64/mingw64/lib' >> "$GITHUB_ENV" + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + with: + key: ${{ matrix.name }} + - name: Build the desktop app and the CLI + run: | + gcc --version | head -1 || true + cargo build --locked --target ${{ matrix.target }} -p effectcraft -p effectcraft-cli + - name: Run them + run: | + target/${{ matrix.target }}/debug/effectcraft.exe --version + target/${{ matrix.target }}/debug/effectcraft-cli.exe --version From 1e1e3d830cde01d2b5801980811e7df7cb887a76 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 19 Oct 2026 12:59:50 -0400 Subject: [PATCH 03/29] Docs: building on Windows with MSVC, GNU, or GNU with bundled LLD (refs #1886) --- CONTRIBUTING.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dfa28c9f0..42f23f771 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,6 +13,37 @@ cargo test --workspace cargo xtask ci # what every commit must pass ``` +### Windows + +The MSVC toolchain, rustup's default on Windows, is what the releases use: with the Visual Studio +C++ build tools installed, the commands above work as they are. + +The GNU toolchain (`x86_64-pc-windows-gnu`) needs a current mingw-w64 GCC and binutils first on +`PATH`. They provide the linker and the Windows import libraries, such as `libshlwapi.a` for +opening links from the app. With [MSYS2](https://www.msys2.org/): + +```powershell +C:\msys64\usr\bin\pacman.exe -S --needed mingw-w64-x86_64-gcc +$env:PATH = "C:\msys64\mingw64\bin;$env:PATH" +cargo +stable-x86_64-pc-windows-gnu run -p effectcraft -- --demo +``` + +An older MinGW earlier on `PATH` can break the build: CodeBlocks' GCC 8.1 fails to link +`boa_engine` ([#1882](https://github.com/storytold/effectcraft/issues/1882)). + +Rust's bundled LLD and self-contained libraries don't include every import library EffectCraft +needs, so on their own they stop with `unable to find library -lshlwapi` +([#1886](https://github.com/storytold/effectcraft/issues/1886)). To link that way, add mingw-w64's +libraries to the search path in your own Cargo config (`%USERPROFILE%\.cargo\config.toml`): + +```toml +[target.x86_64-pc-windows-gnu] +linker = "rust-lld" +rustflags = ["-C", "link-self-contained=yes", "-L", "native=C:/msys64/mingw64/lib"] +``` + +The `Windows toolchains` workflow builds the app and the CLI each of these three ways every night. + ## Ground rules - **Clean room.** Work from After Effects' public documentation and its observable behaviour only. From f0af0f736af111508bdc657a3378dd0579603aa9 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 19 Oct 2026 13:01:43 -0400 Subject: [PATCH 04/29] CI: write the MSYS2 PATH entry with printf, for shellcheck (refs #1886) --- .github/workflows/windows-toolchains.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/windows-toolchains.yml b/.github/workflows/windows-toolchains.yml index 9c51e8acc..3d8b37ea8 100644 --- a/.github/workflows/windows-toolchains.yml +++ b/.github/workflows/windows-toolchains.yml @@ -51,7 +51,7 @@ jobs: - name: Put it first on PATH if: matrix.target == 'x86_64-pc-windows-gnu' && !matrix.rust-lld run: | - echo 'C:\msys64\mingw64\bin' >> "$GITHUB_PATH" + printf '%s\n' 'C:\msys64\mingw64\bin' >> "$GITHUB_PATH" - name: Link with Rust's bundled LLD and mingw-w64's import libraries if: matrix.rust-lld run: | From 82e383deb27abe3bf994d36ccaf5b30c29836d43 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 19 Oct 2026 13:26:19 -0400 Subject: [PATCH 05/29] CI: give bundled LLD only libshlwapi.a, not all of mingw-w64's libraries (refs #1886) --- .github/workflows/windows-toolchains.yml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/windows-toolchains.yml b/.github/workflows/windows-toolchains.yml index 3d8b37ea8..b6765455e 100644 --- a/.github/workflows/windows-toolchains.yml +++ b/.github/workflows/windows-toolchains.yml @@ -1,6 +1,6 @@ # Windows builds with the toolchains contributors use, not just the MSVC one releases use: build # the desktop app and the CLI with MSVC, with GNU (mingw-w64 from MSYS2, the setup CONTRIBUTING.md -# describes), and with GNU linked by Rust's bundled LLD plus mingw-w64's import libraries (#1886), +# describes), and with GNU linked by Rust's bundled LLD plus mingw-w64's `libshlwapi.a` (#1886), # then run each binary with `--version`. Both packages build in one `cargo build`, the way people # build them, so features unify and the CLI links everything the desktop app pulls in. name: Windows toolchains @@ -52,11 +52,15 @@ jobs: if: matrix.target == 'x86_64-pc-windows-gnu' && !matrix.rust-lld run: | printf '%s\n' 'C:\msys64\mingw64\bin' >> "$GITHUB_PATH" - - name: Link with Rust's bundled LLD and mingw-w64's import libraries + # Only libshlwapi.a, in a folder of its own: with all of mingw-w64's lib folder on the search + # path, its C runtime mixes with Rust's bundled one (undefined `DllEntryPoint`). + - name: Link with Rust's bundled LLD and mingw-w64's shlwapi import library if: matrix.rust-lld run: | + mkdir -p /c/gnu-import-libs + cp /c/msys64/mingw64/lib/libshlwapi.a /c/gnu-import-libs/ echo 'CARGO_TARGET_X86_64_PC_WINDOWS_GNU_LINKER=rust-lld' >> "$GITHUB_ENV" - echo 'CARGO_TARGET_X86_64_PC_WINDOWS_GNU_RUSTFLAGS=-C link-self-contained=yes -L native=C:/msys64/mingw64/lib' >> "$GITHUB_ENV" + echo 'CARGO_TARGET_X86_64_PC_WINDOWS_GNU_RUSTFLAGS=-C link-self-contained=yes -L native=C:/gnu-import-libs' >> "$GITHUB_ENV" - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 with: key: ${{ matrix.name }} From d5a7ab0a136a7f20e4dd1ecc755996b4b985d309 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 19 Oct 2026 13:26:19 -0400 Subject: [PATCH 06/29] Docs: bundled LLD takes libshlwapi.a in a folder of its own (refs #1886) --- CONTRIBUTING.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 42f23f771..5f36bc5cb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -33,13 +33,20 @@ An older MinGW earlier on `PATH` can break the build: CodeBlocks' GCC 8.1 fails Rust's bundled LLD and self-contained libraries don't include every import library EffectCraft needs, so on their own they stop with `unable to find library -lshlwapi` -([#1886](https://github.com/storytold/effectcraft/issues/1886)). To link that way, add mingw-w64's -libraries to the search path in your own Cargo config (`%USERPROFILE%\.cargo\config.toml`): +([#1886](https://github.com/storytold/effectcraft/issues/1886)). To link that way, copy +mingw-w64's `libshlwapi.a` into a folder of its own and add that folder to the search path in your +own Cargo config (`%USERPROFILE%\.cargo\config.toml`). Don't add all of mingw-w64's `lib` folder: +its C runtime clashes with Rust's bundled one (`undefined symbol: DllEntryPoint`). + +```powershell +New-Item -ItemType Directory -Force C:\gnu-import-libs +Copy-Item C:\msys64\mingw64\lib\libshlwapi.a C:\gnu-import-libs\ +``` ```toml [target.x86_64-pc-windows-gnu] linker = "rust-lld" -rustflags = ["-C", "link-self-contained=yes", "-L", "native=C:/msys64/mingw64/lib"] +rustflags = ["-C", "link-self-contained=yes", "-L", "native=C:/gnu-import-libs"] ``` The `Windows toolchains` workflow builds the app and the CLI each of these three ways every night. From 9d33c200d5df7bcd6df808b4ddd12e9234d3521d Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Sat, 24 Oct 2026 23:00:46 +0000 Subject: [PATCH 07/29] #1743: protect render sources and refresh proxies safely --- .../workflows/render-source-protection.yml | 34 ++ Cargo.lock | 1 + Cargo.toml | 1 + crates/engine/Cargo.toml | 4 + crates/engine/src/commands/comp_more.rs | 7 +- crates/engine/src/commands/proxy.rs | 2 + crates/engine/src/commands/render_queue.rs | 9 + crates/engine/src/lib.rs | 3 + crates/engine/src/output_guard.rs | 435 +++++++++++++ crates/engine/src/render_queue.rs | 95 +-- crates/engine/src/tests_render_overwrite.rs | 571 ++++++++++++++++++ crates/project/src/render_queue.rs | 80 ++- docs/agents.md | 4 + docs/render-source-protection.md | 26 + 14 files changed, 1230 insertions(+), 42 deletions(-) create mode 100644 .github/workflows/render-source-protection.yml create mode 100644 crates/engine/src/output_guard.rs create mode 100644 crates/engine/src/tests_render_overwrite.rs create mode 100644 docs/render-source-protection.md diff --git a/.github/workflows/render-source-protection.yml b/.github/workflows/render-source-protection.yml new file mode 100644 index 000000000..3c040d831 --- /dev/null +++ b/.github/workflows/render-source-protection.yml @@ -0,0 +1,34 @@ +name: Render source protection + +on: + pull_request: + paths: + - .github/workflows/render-source-protection.yml + - Cargo.toml + - Cargo.lock + - crates/engine/** + - crates/project/src/render_queue.rs + workflow_dispatch: + +permissions: + contents: read + +jobs: + regression: + name: Source guard (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + env: + CARGO_PROFILE_DEV_DEBUG: 0 + CARGO_PROFILE_TEST_DEBUG: 0 + CARGO_INCREMENTAL: 0 + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@1.95.0 + - run: cargo test --locked -p effectcraft-engine --lib output_guard::tests + - run: cargo test --locked -p effectcraft-engine --lib tests_render_overwrite + - run: cargo test --locked -p effectcraft-project render_queue::tests diff --git a/Cargo.lock b/Cargo.lock index f99e09a06..2f9163471 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1285,6 +1285,7 @@ dependencies = [ "kurbo", "log", "rayon", + "same-file", "serde", "serde_json", "thiserror 2.0.21", diff --git a/Cargo.toml b/Cargo.toml index a3935f047..bf3ac938f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -72,6 +72,7 @@ skrifa = { version = "0.44", default-features = false, features = ["std"] } harfrust = { version = "0.12", default-features = false, features = ["std"] } exr = "1.74" image = { version = "0.25", default-features = false, features = ["png", "jpeg"] } +same-file = "1.0" # media codecs + containers: FilmCraft's pure-Rust crates, pinned to a revision (plan/adr/0001). # Only `crates/media`, `crates/export` and `crates/interchange` may depend on these. diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index 7104bc1cc..4bb488569 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -39,3 +39,7 @@ rayon = { workspace = true } image = { workspace = true } web-time = { workspace = true } exr = { workspace = true } + +# Native only: the volume's own identity for a file, to tell that a hard link is the footage it leads to. +[target.'cfg(not(target_arch = "wasm32"))'.dependencies] +same-file = { workspace = true } diff --git a/crates/engine/src/commands/comp_more.rs b/crates/engine/src/commands/comp_more.rs index d6de320ec..fca36e670 100644 --- a/crates/engine/src/commands/comp_more.rs +++ b/crates/engine/src/commands/comp_more.rs @@ -93,8 +93,11 @@ fn queue_frame(s: &mut Session, p: &Value) -> Result { it.output = s.project.render_templates.default_output_module(TemplateSlot::Still); if let Some(path) = str_p(p, "path") { it.output.output = path.to_string(); - } else if !it.output.format.is_sequence() { - it.output.output = "[compName]_frame.[fileExtension]".into(); + } else { + if !it.output.format.is_sequence() { + it.output.output = "[compName]_frame.[fileExtension]".into(); + } + s.avoid_sources(&mut it); } let id = s.edit("Save Frame As", None, |proj, _| { it.id = proj.render_queue.iter().map(|i| i.id).max().unwrap_or(0) + 1; diff --git a/crates/engine/src/commands/proxy.rs b/crates/engine/src/commands/proxy.rs index b7d644b94..dfa259618 100644 --- a/crates/engine/src/commands/proxy.rs +++ b/crates/engine/src/commands/proxy.rs @@ -190,6 +190,8 @@ fn create_proxy(s: &mut Session, p: &Value) -> Result { it.output.channels = Channels::Rgba; if let Some(o) = str_p(p, "path").or(str_p(p, "output")) { it.output.output = o.to_string(); + } else { + s.avoid_sources(&mut it); } it.post_render = PostRenderAction::SetProxy; let id = s.edit("Create Proxy", None, |proj, _| { diff --git a/crates/engine/src/commands/render_queue.rs b/crates/engine/src/commands/render_queue.rs index 09a7c7d2f..d7e914b1a 100644 --- a/crates/engine/src/commands/render_queue.rs +++ b/crates/engine/src/commands/render_queue.rs @@ -532,6 +532,9 @@ fn add(s: &mut Session, p: &Value) -> Result { object.remove("template"); } apply_output(&mut it.output, &templates, s.state.region_of_interest, &output_params, "renderQueue.add")?; + if str_p(p, "output").or(str_p(p, "path")).is_none() { + s.avoid_sources(&mut it); + } if let Some(l) = enum_p(p, "log", "renderQueue.add", RenderLog::parse, "errorsOnly|plusSettings|plusPerFrameInfo")? { it.log = l; } @@ -770,6 +773,9 @@ fn pre_render(s: &mut Session, p: &Value) -> Result { it.output = templates.default_output_module(TemplateSlot::PreRender); it.output.output = "[compName]_prerender.[fileExtension]".into(); apply_output(&mut it.output, &templates, s.state.region_of_interest, p, "render.preRender")?; + if str_p(p, "output").or(str_p(p, "path")).is_none() { + s.avoid_sources(&mut it); + } it.post_render = PostRenderAction::ImportAndReplace; let id = s.edit("Pre-render", None, |proj, _| { it.id = proj.render_queue.iter().map(|i| i.id).max().unwrap_or(0) + 1; @@ -795,6 +801,9 @@ fn save_current_preview(s: &mut Session, p: &Value) -> Result { let mut item = RenderQueueItem::new(0, cid); item.output = OutputModule::for_format(format); item.output.output = path.clone(); + if let Some(refusal) = s.check_outputs(&[vec![(item.clone(), path.clone())]]).first() { + return Err(EngineError::Other(refusal.reason.clone())); + } let job = crate::ExportJob { project: &s.project, footage: s.footage.as_ref(), diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 382534a44..3f703129b 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -30,6 +30,7 @@ pub mod media_cache; pub mod menus; pub mod models; pub mod offload; +mod output_guard; pub mod perf; pub mod prefs; pub mod preview; @@ -1333,6 +1334,8 @@ mod tests_proxy; #[cfg(test)] mod tests_quick_offset; #[cfg(test)] +mod tests_render_overwrite; +#[cfg(test)] mod tests_replace_source; #[cfg(test)] mod tests_rig3d; diff --git a/crates/engine/src/output_guard.rs b/crates/engine/src/output_guard.rs new file mode 100644 index 000000000..5d4dc289e --- /dev/null +++ b/crates/engine/src/output_guard.rs @@ -0,0 +1,435 @@ +//! Protect files the project reads from render outputs (#1743). +//! +//! This guard is independent of replacement consent and export staging: it never authorizes +//! overwriting ordinary outputs or opens a second confirmation flow. Those belong to the +//! exporter. Existing aliases are compared by native file identity, including hard links. +//! Missing sources remain protected by their normalized path. Checks happen before dispatch; +//! filesystem changes between the check and the write require exporter-side revalidation. + +use std::collections::{HashMap, HashSet}; +use std::path::{Component, Path, PathBuf}; +use std::sync::Arc; + +use effectcraft_project::render_queue::{RenderQueueItem, log_path, numbered_template, sequence_frame, sequence_path}; +use effectcraft_project::{Footage, ItemKind}; + +use crate::Session; + +/// Volumes on Windows and macOS ignore case unless they were set up not to. +const IGNORES_CASE: bool = cfg!(any(windows, target_os = "macos")); + +/// How many frames are looked at one by one for a sequence whose folder cannot be read at once +/// (its frame number is in a folder's name). +const PROBE_LIMIT: u64 = 100_000; + +/// How far a default output name is numbered (`_1` … `_1000`) before it is left to the check at +/// render time. +const MAX_NUMBER: u32 = 1_000; + +/// What reads a file the project depends on. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum Source { + /// A footage item, by name. + Footage(Arc), + /// An item's proxy, by the item's name. + Proxy(Arc), + /// The project file. + Project, +} + +impl Source { + /// Why `path`, a file this reads, cannot be a render's output. + pub(crate) fn refusal(&self, path: &str) -> String { + match self { + Source::Footage(name) => format!("{path} is the footage “{name}” of this project, and a render never replaces footage; choose another Output To"), + Source::Proxy(name) => format!("{path} is the proxy of “{name}”, and a render never replaces a proxy; choose another Output To"), + Source::Project => format!("{path} is this project's own file, and a render never replaces it; choose another Output To"), + } + } +} + +/// The files the project reads: footage (every frame of a sequence), proxies and the project file. +#[derive(Default)] +pub(crate) struct Protected { + files: Vec<(PathBuf, Source)>, + by_path: HashMap, + /// Hashes of native identities, built once. Handles are closed after indexing so long + /// sequences do not exhaust the process's file descriptors. Hash hits are verified with + /// native handle equality; a hash collision cannot make another file a protected source. + #[cfg(not(target_arch = "wasm32"))] + by_identity: std::sync::OnceLock>>, + #[cfg(all(test, not(target_arch = "wasm32")))] + identity_reads: std::sync::atomic::AtomicUsize, +} + +impl Protected { + pub(crate) fn of(s: &Session) -> Protected { + let mut found = Protected::default(); + for item in s.project.items.values() { + let name: Arc = item.name.as_str().into(); + if let ItemKind::Footage(f) = &item.kind { + found.add_footage(s, f, &Source::Footage(name.clone())); + } + if let Some(proxy) = &item.proxy { + found.add_footage(s, &proxy.footage, &Source::Proxy(name)); + } + } + if let Some(path) = s.path.as_deref().filter(|p| !p.is_empty()) { + found.add(PathBuf::from(path), Source::Project); + } + found + } + + fn add_footage(&mut self, s: &Session, f: &Footage, source: &Source) { + for stored in std::iter::once(&f.path).chain(&f.sequence).filter(|p| !p.is_empty()) { + // The file the media layer opens for the path the project stores. + let path = s.stored_path(stored).unwrap_or_else(|_| PathBuf::from(stored)); + self.add(path, source.clone()); + } + } + + fn add(&mut self, path: PathBuf, source: Source) { + self.by_path.entry(key(&path)).or_insert(self.files.len()); + self.files.push((path, source)); + } + + pub(crate) fn is_empty(&self) -> bool { + self.files.is_empty() + } + + /// The source at this normalized path or native file identity. A new output only needs + /// its path lookup and one failed filesystem probe; existing outputs build the index once. + pub(crate) fn find(&self, path: &Path) -> Option<&Source> { + self.by_path.get(&key(path)).and_then(|i| self.files.get(*i)).map(|(_, source)| source).or_else(|| self.alias_of(path)) + } + + #[cfg(not(target_arch = "wasm32"))] + fn handle(&self, path: &Path) -> Option { + #[cfg(test)] + self.identity_reads.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + // Opening a FIFO can wait for a writer. Only regular files can be protected media; + // preserve the old size-indexed lookup's exclusion of special files and directories. + if !std::fs::metadata(path).ok()?.is_file() { + return None; + } + same_file::Handle::from_path(path).ok() + } + + /// Hashes narrow the search, while native handle equality also checks any hash collisions. + #[cfg(not(target_arch = "wasm32"))] + fn alias_of(&self, path: &Path) -> Option<&Source> { + let output = self.handle(path)?; + let indexed = self.by_identity.get_or_init(|| { + let mut ids: HashMap> = HashMap::new(); + for (i, (known, _)) in self.files.iter().enumerate() { + if let Some(handle) = self.handle(known) { + ids.entry(identity_hash(&handle)).or_default().push(i); + } + } + ids + }); + indexed.get(&identity_hash(&output))?.iter().find_map(|i| { + let (known, source) = self.files.get(*i)?; + (self.handle(known)? == output).then_some(source) + }) + } + + #[cfg(target_arch = "wasm32")] + fn alias_of(&self, _: &Path) -> Option<&Source> { + None + } +} + +#[cfg(not(target_arch = "wasm32"))] +fn identity_hash(handle: &same_file::Handle) -> u64 { + use std::hash::{Hash, Hasher}; + let mut hash = std::collections::hash_map::DefaultHasher::new(); + handle.hash(&mut hash); + hash.finish() +} + +/// `path` made absolute, with `.` and `..` resolved by name alone (no file is looked at). +fn lexical(path: &Path) -> PathBuf { + let absolute = std::path::absolute(path).unwrap_or_else(|_| path.to_path_buf()); + let mut out = PathBuf::new(); + for part in absolute.components() { + match part { + Component::CurDir => {} + Component::ParentDir => match out.components().next_back() { + Some(Component::Normal(_)) => { + out.pop(); + } + Some(Component::RootDir | Component::Prefix(_)) => {} + _ => out.push(".."), + }, + other => out.push(other.as_os_str()), + } + } + out +} + +/// The name a path is compared by: absolute, resolved by name, without case where volumes ignore it. +fn key(path: &Path) -> String { + let parts: Vec = lexical(path).components().map(|c| c.as_os_str().to_string_lossy().into_owned()).collect(); + fold(&parts.join("/")) +} + +fn fold(name: &str) -> String { + if IGNORES_CASE { name.to_lowercase() } else { name.to_string() } +} + +#[cfg(test)] +fn same_file(a: &Path, b: &Path) -> bool { + if key(a) == key(b) { + return true; + } + #[cfg(not(target_arch = "wasm32"))] + return same_file::is_same_file(a, b).unwrap_or(false); + #[cfg(target_arch = "wasm32")] + false +} + +/// Whether a file (or a link, even one leading nowhere) is at `path`. +fn exists(path: &str) -> bool { + std::fs::symlink_metadata(path).is_ok_and(|m| !m.is_dir()) +} + +/// The files of a numbered sequence (`count` frames from `first`) that are on disk: its folder is +/// read once, not asked about frame by frame. +fn frames_on_disk(path: &str, first: i64, count: u64) -> Result, String> { + let last = first.saturating_add(i64::try_from(count).unwrap_or(i64::MAX)); + let p = Path::new(path); + let dir = p.parent().filter(|d| !d.as_os_str().is_empty()).unwrap_or(Path::new(".")); + let name = p.file_name().map(|n| n.to_string_lossy().into_owned()); + let Some(name) = name.filter(|_| !dir.to_string_lossy().contains('#')) else { + // The frame number is in a folder's name: every frame has a folder of its own. + if count > PROBE_LIMIT { + return Err(format!( + "cannot safely check {count} output frames with a number in a folder name (limit {PROBE_LIMIT}); choose a fixed output folder or a shorter range" + )); + } + return Ok((0..count) + .map(|k| sequence_path(path, first.saturating_add(i64::try_from(k).unwrap_or(i64::MAX)))) + .filter(|frame| exists(frame)) + .map(PathBuf::from) + .collect()); + }; + let pattern = fold(&name); + let entries = match std::fs::read_dir(dir) { + Ok(entries) => entries, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(e) => return Err(format!("cannot check render output folder {}: {e}", dir.display())), + }; + Ok(entries + .filter_map(Result::ok) + .filter(|e| sequence_frame(&pattern, &fold(&e.file_name().to_string_lossy())).is_some_and(|frame| (first..last).contains(&frame))) + .filter(|e| !e.file_type().is_ok_and(|t| t.is_dir())) + .map(|e| e.path()) + .collect()) +} + +/// A queue item refused before any of its output modules are dispatched. +#[derive(Clone, Debug, PartialEq)] +pub(crate) struct Refused { + pub item: u64, + pub reason: String, +} + +impl Session { + /// The files the project reads. + pub(crate) fn protected_files(&self) -> Protected { + Protected::of(self) + } + + /// Reject protected outputs and ranges that cannot be checked completely. Ordinary-file + /// replacement consent is left to the exporter, independently of this source guard. + pub(crate) fn check_outputs(&self, items: &[Vec<(RenderQueueItem, String)>]) -> Vec { + if cfg!(target_arch = "wasm32") { + return Vec::new(); + } + let protected = self.protected_files(); + let mut seen = HashSet::new(); + let mut refused = Vec::new(); + for (item, path) in items.iter().flatten() { + if seen.contains(&item.id) { + continue; + } + let reason = match self.check_output(&protected, item, path) { + Ok(reason) => reason, + Err(reason) => Some(reason), + }; + if let Some(reason) = reason { + seen.insert(item.id); + refused.push(Refused { item: item.id, reason }); + } + } + refused + } + + fn check_output(&self, protected: &Protected, item: &RenderQueueItem, path: &str) -> Result, String> { + if protected.is_empty() { + return Ok(None); + } + let sequence = item.output.format.is_sequence(); + // Skip Existing Files leaves existing frames untouched, but not the render log. + let frames = if sequence && !item.settings.skip_existing { + match self.project.comp(item.comp) { + Some(comp) => frames_on_disk(path, item.settings.first_frame(comp), item.settings.frame_count(comp))?, + None => Vec::new(), + } + } else { + Vec::new() + }; + let log = log_path(path); + let movie = (!sequence).then_some(Path::new(path)); + Ok(frames + .iter() + .map(PathBuf::as_path) + .chain(movie) + .chain(Some(Path::new(&log))) + .find_map(|path| protected.find(path).map(|source| source.refusal(&path.to_string_lossy())))) + } + + /// A new item's default output must not be a file the project reads: `[compName]_1`, + /// `[compName]_2`… take its place (and avoid the queue's other outputs), as After Effects + /// numbers a second item of the same composition. An output nothing is in the way of keeps + /// the composition's own name, and one the caller chose is not touched: rendering it is + /// refused instead. + pub(crate) fn avoid_sources(&self, item: &mut RenderQueueItem) { + if cfg!(target_arch = "wasm32") { + return; + } + let protected = self.protected_files(); + if protected.is_empty() { + return; + } + let template = item.output.output.clone(); + let mut taken: Option> = None; + 'candidate: for n in 0..=MAX_NUMBER { + item.output.output = if n == 0 { template.clone() } else { numbered_template(&template, n) }; + let Ok(outputs) = self.outputs_of(item, None) else { break }; + for (module, path) in &outputs { + match self.check_output(&protected, module, path) { + Ok(Some(_)) => continue 'candidate, + Err(_) => break 'candidate, + Ok(None) => {} + } + } + // Only a name that had to move is kept clear of the other items. + let others = if n == 0 { None } else { Some(taken.get_or_insert_with(|| self.queue_output_keys())) }; + if others.is_none_or(|others| !outputs.iter().any(|(_, path)| others.contains(&key(Path::new(path))))) { + return; + } + } + item.output.output = template; + } + + /// [`key`] of every output of the queue's items. + fn queue_output_keys(&self) -> HashSet { + self.project.render_queue.iter().filter_map(|it| self.outputs_of(it, None).ok()).flatten().map(|(_, path)| key(Path::new(&path))).collect() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + #[cfg(not(target_arch = "wasm32"))] + fn unrelated_same_size_frames_are_indexed_once() { + let dir = std::env::temp_dir().join(format!("effectcraft-identity-index-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let mut protected = Protected::default(); + for i in 0..128 { + let source = dir.join(format!("source{i}")).join("plate.png"); + let output = dir.join(format!("output{i}")).join("plate.png"); + for path in [&source, &output] { + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, b"same-size frame").unwrap(); + } + protected.add(source, Source::Footage(format!("plate{i}").into())); + } + for i in 0..128 { + assert!(protected.find(&dir.join(format!("output{i}")).join("plate.png")).is_none()); + } + assert_eq!(protected.identity_reads.load(std::sync::atomic::Ordering::Relaxed), 256, "one source-index pass plus one lookup per output"); + let alias = dir.join("renamed.png"); + std::fs::hard_link(dir.join("source64").join("plate.png"), &alias).unwrap(); + assert_eq!(protected.find(&alias), Some(&Source::Footage("plate64".into()))); + assert_eq!(protected.identity_reads.load(std::sync::atomic::Ordering::Relaxed), 258, "native equality verifies a matching identity"); + std::fs::remove_dir_all(dir).unwrap(); + } + + #[test] + fn folder_numbered_sequences_are_never_partially_checked() { + let pattern = std::env::temp_dir().join("frames[######]").join("plate.png").to_string_lossy().into_owned(); + let error = frames_on_disk(&pattern, 0, PROBE_LIMIT + 1).unwrap_err(); + assert!(error.contains("cannot safely check") && error.contains("100000"), "{error}"); + assert!(frames_on_disk(&pattern, 0, u64::MAX).is_err()); + } + + #[test] + #[cfg(not(target_arch = "wasm32"))] + fn non_file_outputs_do_not_build_the_identity_index() { + let protected = Protected::default(); + assert!(protected.find(&std::env::temp_dir()).is_none()); + assert!(protected.by_identity.get().is_none()); + } + + #[test] + fn paths_compare_by_name_once_resolved() { + let here = std::env::current_dir().unwrap(); + assert_eq!(key(Path::new("a/b/../c.mp4")), key(Path::new("a/c.mp4"))); + assert_eq!(key(Path::new("./a/c.mp4")), key(&here.join("a/c.mp4"))); + assert_ne!(key(Path::new("a/c.mp4")), key(Path::new("b/c.mp4"))); + assert_eq!(key(Path::new(&format!("{}x", "../".repeat(64)))), key(Path::new(&format!("{}x", "../".repeat(80)))), "past the root stays at the root"); + assert_eq!(IGNORES_CASE, key(Path::new("A/C.MP4")) == key(Path::new("a/c.mp4"))); + } + + #[test] + fn a_file_is_the_same_file_under_every_spelling_the_volume_allows() { + let dir = std::env::temp_dir().join(format!("effectcraft-output-guard-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("sub")).unwrap(); + std::fs::write(dir.join("clip.mov"), b"x").unwrap(); + std::fs::write(dir.join("other.mov"), b"y").unwrap(); + assert!(same_file(&dir.join("clip.mov"), &dir.join("sub").join("..").join("clip.mov"))); + assert!(!same_file(&dir.join("clip.mov"), &dir.join("other.mov"))); + // Nothing there yet: only the name decides. + assert!(same_file(&dir.join("gone.mov"), &dir.join("sub").join("..").join("gone.mov"))); + assert!(!same_file(&dir.join("gone.mov"), &dir.join("clip.mov"))); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn a_sequences_folder_is_read_once_for_its_frames() { + let dir = std::env::temp_dir().join(format!("effectcraft-output-frames-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("a_00003.png")).unwrap(); // a folder is not a frame + for name in ["a_00001.png", "a_00002.png", "a_0004.png", "a_00004.jpg", "b_00002.png", "a_00009.png"] { + std::fs::write(dir.join(name), b"x").unwrap(); + } + let pattern = dir.join("a_[#####].png").to_string_lossy().into_owned(); + let names = |frames: Vec| { + let mut names: Vec = frames.iter().filter_map(|f| f.file_name()).map(|n| n.to_string_lossy().into_owned()).collect(); + names.sort(); + names + }; + assert_eq!( + names(frames_on_disk(&pattern, 0, 5).unwrap()), + ["a_00001.png", "a_00002.png"], + "frames 0-4: not the 4-digit one, the jpg, the folder or frame 9" + ); + assert_eq!(names(frames_on_disk(&pattern, 2, 1).unwrap()), ["a_00002.png"]); + assert_eq!(names(frames_on_disk(&pattern, 9, 100).unwrap()), ["a_00009.png"]); + assert_eq!(names(frames_on_disk(&pattern, 0, u64::MAX).unwrap()), ["a_00001.png", "a_00002.png", "a_00009.png"], "a hostile frame count is not walked"); + // A frame number in a folder's name: each frame has its own folder. + std::fs::create_dir_all(dir.join("f02")).unwrap(); + std::fs::write(dir.join("f02").join("x.png"), b"x").unwrap(); + let in_folder = dir.join("f[##]").join("x.png").to_string_lossy().into_owned(); + assert_eq!(names(frames_on_disk(&in_folder, 0, 4).unwrap()), ["x.png"]); + // A folder that is not there holds nothing. + assert!(frames_on_disk(&dir.join("none").join("a_[#####].png").to_string_lossy(), 0, 5).unwrap().is_empty()); + let _ = std::fs::remove_dir_all(&dir); + } +} diff --git a/crates/engine/src/render_queue.rs b/crates/engine/src/render_queue.rs index 74b333151..f36b500cc 100644 --- a/crates/engine/src/render_queue.rs +++ b/crates/engine/src/render_queue.rs @@ -432,10 +432,46 @@ impl Session { self.render_job.as_ref().map(|j| j.shared.snapshot()) } - /// Start rendering every queued item. `wait`: block until done (otherwise a background thread - /// renders, or the [`Session::offload`] worker; call [`Session::poll_render`] regularly). - /// Returns the ids being rendered. Without threads or an offload (wasm32) the render always - /// runs to completion before this returns. + /// The (item, resolved output path) of each output of `it`: one per output module and, with + /// Segment Sequences / Segment Movie Files, per segment. `formats`: the ones that can be + /// written (`None`: all). Fails with the status the item gets instead. + pub(crate) fn outputs_of(&self, it: &RenderQueueItem, formats: Option<&[OutputFormat]>) -> Result, RenderStatus> { + let mut modules = vec![]; + for om in it.output_modules() { + let mut m = it.clone(); + m.output = om.clone(); + match self.resolve_output(&m) { + Some(p) if formats.is_none_or(|f| f.contains(&om.format)) => { + // Settings ▸ Export: Append Bit Depth to File Name, Segment Sequences / + // Movie Files. + let bits = output_bits(om.format, self.project.settings.bit_depth); + let p = self.prefs.output_name(&p, bits); + modules.extend(self.segments(&m, &p)); + } + Some(_) => return Err(RenderStatus::Failed(format!("{} export is not available", om.format.label()))), + None if self.project.comp(it.comp).is_none() => return Err(RenderStatus::Failed("composition missing".into())), + None => return Err(RenderStatus::NeedsOutput), + } + } + Ok(modules) + } + + /// What the next Render would write: for each queued item its [`Session::outputs_of`], and + /// the status of each queued item that has none. + pub(crate) fn queued_outputs(&self, formats: &[OutputFormat]) -> (Vec>, Vec<(u64, RenderStatus)>) { + let (mut items, mut problems) = (Vec::new(), Vec::new()); + for it in self.project.render_queue.iter().filter(|i| i.is_queued()) { + match self.outputs_of(it, Some(formats)) { + Ok(modules) => items.push(modules), + Err(status) => problems.push((it.id, status)), + } + } + (items, problems) + } + + /// Start rendering every queued item, refusing outputs that replace footage, proxies or + /// the project file. Other existing outputs keep the exporter's usual replacement policy. + /// `wait`: block until done, otherwise render in the background and poll regularly. pub fn start_render(&mut self, wait: bool) -> Result, String> { let offload = self.offload.clone().filter(|_| !wait); let wait = wait || (cfg!(target_arch = "wasm32") && offload.is_none()); @@ -445,46 +481,33 @@ impl Session { self.poll_render(); let exporter = self.exporter.clone().ok_or("export is not available in this build")?; let formats = exporter.formats(); - let mut items = Vec::new(); - let mut needs_output = Vec::new(); - for it in self.project.render_queue.iter().filter(|i| i.is_queued()) { - let mut modules = vec![]; - let mut problem = None; - for om in it.output_modules() { - let mut m = it.clone(); - m.output = om.clone(); - match self.resolve_output(&m) { - Some(p) if formats.contains(&om.format) => { - // Settings ▸ Export: Append Bit Depth to File Name, Segment Sequences / - // Movie Files. - let bits = output_bits(om.format, self.project.settings.bit_depth); - let p = self.prefs.output_name(&p, bits); - modules.extend(self.segments(&m, &p)); - } - Some(_) => problem = Some(RenderStatus::Failed(format!("{} export is not available", om.format.label()))), - None if self.project.comp(it.comp).is_none() => problem = Some(RenderStatus::Failed("composition missing".into())), - None => problem = Some(RenderStatus::NeedsOutput), - } - if problem.is_some() { - break; - } - } - match problem { - Some(st) => needs_output.push((it.id, st)), - None => items.push(modules), - } - } - if !needs_output.is_empty() { + let (mut items, mut problems) = self.queued_outputs(&formats); + let refused = self.check_outputs(&items); + items.retain(|modules| modules.first().is_none_or(|(it, _)| !refused.iter().any(|r| r.item == it.id))); + problems.extend(refused.iter().map(|r| (r.item, RenderStatus::Failed(r.reason.clone())))); + if !problems.is_empty() { let p = Arc::make_mut(&mut self.project); - for (id, st) in needs_output { + for (id, st) in problems { if let Some(i) = p.render_queue.iter_mut().find(|i| i.id == id) { i.status = st; } } self.bump(); } + if let Some(first) = refused.first() { + let message = if refused.len() == 1 { + format!("Not rendered: {}", first.reason) + } else { + format!("{} items were not rendered; first: {}", refused.len(), first.reason) + }; + self.events.push(Event::Toast { message, error: true }); + } if items.is_empty() { - return Err("nothing is queued: add a composition (Composition ▸ Add to Render Queue) and tick Render".into()); + return Err(match refused.as_slice() { + [] => "nothing is queued: add a composition (Composition ▸ Add to Render Queue) and tick Render".into(), + [only] => only.reason.clone(), + [first, ..] => format!("{} items would replace files the project reads; first: {}", refused.len(), first.reason), + }); } let ids: Vec = items.iter().filter_map(|m| m.first().map(|(i, _)| i.id)).collect(); if let Some(off) = offload { diff --git a/crates/engine/src/tests_render_overwrite.rs b/crates/engine/src/tests_render_overwrite.rs new file mode 100644 index 000000000..a2473c6e8 --- /dev/null +++ b/crates/engine/src/tests_render_overwrite.rs @@ -0,0 +1,571 @@ +//! Rendering never replaces a file the project reads (#1743). Ordinary-file replacement +//! behavior stays with the exporter and its confirmation API. + +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; + +use effectcraft_color::Label; +use effectcraft_project::render_queue::{OutputFormat, RenderStatus, sequence_path}; +use effectcraft_project::{Footage, FootageKind, ItemId, ItemKind}; +use effectcraft_time::Tick; +use serde_json::{Value, json}; + +use crate::{ExportJob, ExportResult, Exporter, Session}; + +const SOURCE: &[u8] = b"SOURCE FOOTAGE"; +const EARLIER: &[u8] = b"AN EARLIER RENDER"; +const RENDERED: &[u8] = b"RENDERED"; + +/// An exporter that writes like the real one: `RENDERED` at the job's path, or at the path of +/// every frame for a sequence (leaving the frames that exist alone when the item skips them). +#[derive(Default)] +struct Writer { + jobs: Mutex>, +} + +impl Exporter for Writer { + fn formats(&self) -> Vec { + OutputFormat::ALL.to_vec() + } + fn export(&self, job: &ExportJob, _: &mut dyn FnMut(u64, u64) -> bool) -> Result { + self.jobs.lock().unwrap().push(job.path.to_string()); + let comp = job.project.comp(job.item.comp).ok_or("no comp")?; + let paths: Vec = if job.item.output.format.is_sequence() { + let first = job.item.settings.first_frame(comp); + (0..job.item.settings.frame_count(comp)).map(|k| sequence_path(job.path, first + k as i64)).collect() + } else { + vec![job.path.to_string()] + }; + for path in &paths { + if job.item.settings.skip_existing && Path::new(path).exists() { + continue; + } + if let Some(dir) = Path::new(path).parent() { + std::fs::create_dir_all(dir).map_err(|e| e.to_string())?; + } + std::fs::write(path, RENDERED).map_err(|e| e.to_string())?; + } + Ok(ExportResult { path: paths.first().cloned().unwrap_or_default(), ..Default::default() }) + } +} + +/// A session whose project is saved in a fresh folder (the footage goes there too), with an +/// exporter that writes real files. +struct Fixture { + s: Session, + writer: Arc, + dir: PathBuf, +} + +impl Fixture { + fn new(name: &str) -> Fixture { + let dir = std::env::temp_dir().join("effectcraft-render-overwrite").join(format!("{}-{name}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let mut s = Session::default(); + let writer = Arc::new(Writer::default()); + s.exporter = Some(writer.clone()); + s.execute("file.saveAs", json!({"path": dir.join("Project.ecproj").to_string_lossy()})).unwrap(); + Fixture { s, writer, dir } + } + + fn path(&self, file: &str) -> PathBuf { + self.dir.join(file) + } + + fn text(&self, file: &str) -> String { + self.path(file).to_string_lossy().into_owned() + } + + fn read(&self, file: &str) -> Vec { + std::fs::read(self.path(file)).unwrap() + } + + fn write(&self, file: &str, bytes: &[u8]) { + std::fs::write(self.path(file), bytes).unwrap(); + } + + /// A video file in the project's folder, imported. + fn footage(&mut self, file: &str) -> ItemId { + self.write(file, SOURCE); + let f = Footage { + path: self.text(file), + kind: FootageKind::Video, + width: 64, + height: 36, + has_video: true, + duration: Tick::from_seconds_f64(1.0), + ..Default::default() + }; + Arc::make_mut(&mut self.s.project).add_item(file, Label::None, None, ItemKind::Footage(f)) + } + + /// Numbered stills in the project's folder, imported as one image sequence. + fn sequence(&mut self, name: &str, frames: std::ops::Range) -> ItemId { + let files: Vec = frames + .map(|n| { + let file = format!("{name}_{n:05}.png"); + self.write(&file, SOURCE); + self.text(&file) + }) + .collect(); + let f = Footage { + path: files[0].clone(), + kind: FootageKind::Sequence, + sequence: files, + width: 16, + height: 16, + has_video: true, + duration: Tick::from_seconds_f64(0.5), + ..Default::default() + }; + Arc::make_mut(&mut self.s.project).add_item(&format!("{name}_[00000-00004].png"), Label::None, None, ItemKind::Footage(f)) + } + + /// A five-frame composition. + fn comp(&mut self, name: &str) { + self.s.execute("comp.new", json!({"name": name, "width": 16, "height": 16, "frameRate": 10, "duration": 0.5})).unwrap(); + } + + /// The output path of queue item `index` (0-based). + fn output(&self, index: usize) -> PathBuf { + PathBuf::from(self.s.resolve_output(&self.s.project.render_queue[index]).unwrap()) + } + + /// The paths the exporter was started for. + fn exported(&self) -> Vec { + self.writer.jobs.lock().unwrap().clone() + } + + fn render(&mut self, params: Value) -> crate::Result { + self.s.execute("renderQueue.render", params) + } +} + +impl Drop for Fixture { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.dir); + } +} + +fn link(target: &Path, link: &Path) -> std::io::Result<()> { + #[cfg(unix)] + return std::os::unix::fs::symlink(target, link); + #[cfg(windows)] + return std::os::windows::fs::symlink_file(target, link); + #[cfg(not(any(unix, windows)))] + { + let _ = (target, link); + Err(std::io::ErrorKind::Unsupported.into()) + } +} + +/// A folder that leads to `target`: a symbolic link, or on Windows, where that takes a privilege +/// most accounts lack, a junction. +fn link_dir(target: &Path, link: &Path) -> std::io::Result<()> { + #[cfg(unix)] + return std::os::unix::fs::symlink(target, link); + #[cfg(windows)] + return std::os::windows::fs::symlink_dir(target, link).or_else(|_| { + let made = std::process::Command::new("cmd").args(["/C", "mklink", "/J"]).arg(link).arg(target).output()?; + if made.status.success() { Ok(()) } else { Err(std::io::Error::other(String::from_utf8_lossy(&made.stdout).into_owned())) } + }); + #[cfg(not(any(unix, windows)))] + { + let _ = (target, link); + Err(std::io::ErrorKind::Unsupported.into()) + } +} + +/// The report: save a new project beside `footage.mp4`, import it, make a composition from it +/// (it is named after the file), add that to the Render Queue and click Render. The composition's +/// default output is `/footage.mp4`, the footage itself. +#[test] +fn a_comp_made_from_footage_does_not_render_over_it() { + let mut f = Fixture::new("comp-from-footage"); + let footage = f.footage("footage.mp4"); + f.s.state.project_selection = vec![footage]; + f.s.execute("file.newCompFromSelection", json!({"addToRenderQueue": true})).unwrap(); + assert_eq!(f.s.project.render_queue.len(), 1); + f.render(json!({})).unwrap(); + assert_eq!(f.read("footage.mp4"), SOURCE, "the footage is still the footage"); + assert_eq!(f.s.project.render_queue[0].status, RenderStatus::Done); + // The default name stepped aside: the render sits next to it. + assert_eq!(f.output(0), f.path("footage_1.mp4")); + assert_eq!(f.read("footage_1.mp4"), RENDERED); + // Another item for the same composition takes the next free number. + f.s.execute("renderQueue.add", json!({})).unwrap(); + assert_eq!(f.output(1), f.path("footage_2.mp4")); +} + +#[test] +fn an_output_that_is_the_footage_is_refused_with_or_without_overwrite() { + for (n, params) in [json!({}), json!({"overwrite": true})].into_iter().enumerate() { + let mut f = Fixture::new(&format!("explicit-footage-{n}")); + f.footage("clip.mov"); + f.comp("Main"); + let clip = f.text("clip.mov"); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": clip})).unwrap(); + let e = f.render(params).unwrap_err().to_string(); + assert!(e.contains("clip.mov") && e.contains("footage"), "{e}"); + assert_eq!(f.read("clip.mov"), SOURCE); + assert!(f.exported().is_empty(), "the exporter was not started"); + let RenderStatus::Failed(why) = &f.s.project.render_queue[0].status else { panic!("{:?}", f.s.project.render_queue[0].status) }; + assert!(why.contains("clip.mov"), "{why}"); + } +} + +/// An item that would replace the footage fails on its own; the rest of the queue renders. +#[test] +fn the_rest_of_the_queue_renders_when_one_item_would_replace_footage() { + let mut f = Fixture::new("one-refused"); + f.footage("clip.mov"); + f.comp("Main"); + let clip = f.text("clip.mov"); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": clip})).unwrap(); + let ok = f.text("main.mp4"); + f.s.execute("renderQueue.add", json!({"output": ok})).unwrap(); + let r = f.render(json!({})).unwrap(); + assert_eq!(f.read("clip.mov"), SOURCE); + assert_eq!(f.read("main.mp4"), RENDERED); + assert!(matches!(f.s.project.render_queue[0].status, RenderStatus::Failed(_))); + assert_eq!(f.s.project.render_queue[1].status, RenderStatus::Done); + assert!(r["items"].as_array().unwrap().iter().any(|i| i["status"] == "Done"), "{r}"); +} + +#[test] +fn the_project_file_is_never_an_output() { + let mut f = Fixture::new("project-file"); + // A project saved under a name a movie could have. + let name = f.text("final.mp4"); + f.s.execute("file.saveAs", json!({"path": name})).unwrap(); + f.comp("final"); + f.s.execute("renderQueue.add", json!({})).unwrap(); + assert_eq!(f.output(0), f.path("final_1.mp4"), "the default name moves aside"); + f.s.execute("renderQueue.remove", json!({"index": 1})).unwrap(); + f.s.execute("renderQueue.add", json!({"output": name})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("final.mp4") && e.contains("project"), "{e}"); + assert!(std::fs::read_to_string(f.path("final.mp4")).unwrap().contains("\"schema\""), "the project file is still a project"); +} + +#[test] +fn a_proxy_is_not_an_output_either() { + let mut f = Fixture::new("proxy"); + f.footage("clip.mov"); + f.write("proxy.mov", SOURCE); + f.comp("Main"); + let proxy = Footage { path: f.text("proxy.mov"), kind: FootageKind::Video, has_video: true, ..Default::default() }; + let comp = f.s.active_comp_id().unwrap(); + Arc::make_mut(&mut f.s.project).item_mut(comp).unwrap().proxy = Some(Box::new(effectcraft_project::Proxy { footage: proxy, enabled: true })); + let target = f.text("proxy.mov"); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": target})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("proxy"), "{e}"); + assert_eq!(f.read("proxy.mov"), SOURCE); +} + +/// Recreating a default proxy writes a new file and attaches it, without replacing the proxy +/// the project is currently reading. Explicitly choosing that protected path is still refused. +#[test] +fn creating_a_proxy_again_chooses_a_new_default_and_attaches_it() { + struct Probe; + impl crate::Importer for Probe { + fn probe(&self, path: &str) -> Result { + if !Path::new(path).is_file() { + return Err("proxy file missing".into()); + } + Ok(Footage { + path: path.into(), + kind: if path.ends_with(".png") { FootageKind::Still } else { FootageKind::Video }, + width: 16, + height: 16, + has_video: true, + ..Default::default() + }) + } + } + for kind in ["movie", "still"] { + let mut f = Fixture::new(&format!("recreate-proxy-{kind}")); + f.s.importer = Some(Arc::new(Probe)); + f.comp("Main"); + let comp = f.s.active_comp_id().unwrap(); + f.s.execute("file.createProxy", json!({"kind": kind})).unwrap(); + f.render(json!({})).unwrap(); + let first = f.s.project.item(comp).unwrap().proxy.as_ref().unwrap().footage.path.clone(); + assert_eq!(std::fs::read(&first).unwrap(), RENDERED); + + f.s.execute("file.createProxy", json!({"kind": kind})).unwrap(); + f.render(json!({})).unwrap(); + let second = f.s.project.item(comp).unwrap().proxy.as_ref().unwrap().footage.path.clone(); + assert_ne!(second, first, "the replacement proxy gets its own file"); + assert_eq!(std::fs::read(&second).unwrap(), RENDERED); + assert_eq!(std::fs::read(&first).unwrap(), RENDERED); + assert_eq!(f.exported().len(), 2); + + // For a still proxy, an explicit output is the sequence pattern, not its first + // rendered frame (a bare PNG name would have another frame number appended). + let protected_output = f.output(1); + f.s.execute("file.createProxy", json!({"kind": kind, "path": protected_output})).unwrap(); + let error = f.render(json!({})).unwrap_err().to_string(); + assert!(error.contains("never replaces a proxy"), "{error}"); + assert_eq!(f.exported().len(), 2); + } +} + +/// Exercise the engine's import and project serialization paths, rather than injecting a +/// Footage item directly. The importer reads dimensions from a real, generated PNG. +#[test] +fn imported_footage_stays_protected_after_the_project_is_reopened() { + struct PngProbe; + impl crate::Importer for PngProbe { + fn probe(&self, path: &str) -> Result { + let (width, height) = image::image_dimensions(path).map_err(|e| e.to_string())?; + Ok(Footage { path: path.into(), kind: FootageKind::Still, width, height, has_video: true, ..Default::default() }) + } + } + let mut f = Fixture::new("import-reopen"); + f.s.importer = Some(Arc::new(PngProbe)); + let path = f.path("plate_00002.png"); + image::RgbaImage::from_pixel(4, 4, image::Rgba([40, 80, 120, 255])).save(&path).unwrap(); + let original = std::fs::read(&path).unwrap(); + let result = f.s.execute("file.import", json!({"paths": [path]})).unwrap(); + assert_eq!(result["items"].as_array().unwrap().len(), 1); + f.comp("Main"); + f.s.execute("renderQueue.add", json!({"format": "png", "output": f.text("plate_[#####].png")})).unwrap(); + f.s.execute("file.save", json!({})).unwrap(); + f.s.execute("file.open", json!({"path": f.text("Project.ecproj")})).unwrap(); + let error = f.render(json!({})).unwrap_err().to_string(); + assert!(error.contains("plate_00002.png") && error.contains("footage"), "{error}"); + assert_eq!(std::fs::read(&path).unwrap(), original); + assert!(f.exported().is_empty()); +} + +#[test] +fn a_protected_frame_past_the_probe_limit_is_not_rendered() { + struct LastFrame(PathBuf); + impl Exporter for LastFrame { + fn formats(&self) -> Vec { + vec![OutputFormat::PngSequence] + } + fn export(&self, _: &ExportJob, _: &mut dyn FnMut(u64, u64) -> bool) -> Result { + // A synthetic exporter writes only the vulnerable late frame, so a broken guard + // can be reproduced without creating 100,001 output directories. + std::fs::write(&self.0, RENDERED).map_err(|e| e.to_string())?; + Ok(ExportResult { path: self.0.to_string_lossy().into_owned(), ..Default::default() }) + } + } + let mut f = Fixture::new("late-frame"); + std::fs::create_dir_all(f.path("f100000")).unwrap(); + f.footage("f100000/plate.png"); + f.comp("Main"); + f.s.execute("comp.settings", json!({"duration": 10000.1})).unwrap(); + f.s.exporter = Some(Arc::new(LastFrame(f.path("f100000/plate.png")))); + f.s.execute("renderQueue.add", json!({"format": "png", "output": f.text("f[######]/plate.png")})).unwrap(); + let error = f.render(json!({})).unwrap_err().to_string(); + assert!(error.contains("cannot safely check"), "{error}"); + assert_eq!(f.read("f100000/plate.png"), SOURCE); +} + +/// A numbered sequence is footage frame by frame: one colliding frame is enough, and nothing of +/// the item is rendered. +#[test] +fn a_sequence_never_replaces_one_of_its_footage_frames() { + let mut f = Fixture::new("sequence"); + f.sequence("shot", 0..5); + f.comp("shot"); + // The default name steps aside as a whole: shot_1_00000.png … + f.s.execute("renderQueue.add", json!({"format": "png"})).unwrap(); + assert_eq!(f.output(0), f.path("shot_1_[#####].png")); + f.render(json!({})).unwrap(); + for n in 0..5 { + assert_eq!(f.read(&format!("shot_{n:05}.png")), SOURCE, "frame {n}"); + assert_eq!(f.read(&format!("shot_1_{n:05}.png")), RENDERED, "frame {n}"); + } + + // Named by hand, one frame of five is a footage file. + let mut f = Fixture::new("sequence-one-frame"); + f.write("shot_00002.png", SOURCE); + let still = Footage { path: f.text("shot_00002.png"), kind: FootageKind::Still, width: 16, height: 16, has_video: true, ..Default::default() }; + Arc::make_mut(&mut f.s.project).add_item("shot_00002.png", Label::None, None, ItemKind::Footage(still)); + f.comp("shot"); + let pattern = f.text("shot_[#####].png"); + f.s.execute("renderQueue.add", json!({"format": "png", "output": pattern})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("shot_00002.png"), "{e}"); + assert_eq!(f.read("shot_00002.png"), SOURCE); + assert!(!f.path("shot_00000.png").exists(), "the frames before it were not rendered either"); + assert!(f.exported().is_empty()); +} + +#[test] +fn ordinary_existing_outputs_keep_the_exporters_replacement_policy() { + let mut f = Fixture::new("earlier-render"); + f.comp("Main"); + f.s.execute("renderQueue.add", json!({"output": f.text("render.mp4")})).unwrap(); + f.write("render.mp4", EARLIER); + f.render(json!({})).unwrap(); + assert_eq!(f.read("render.mp4"), RENDERED); + assert_eq!(f.s.project.render_queue[0].status, RenderStatus::Done); +} + +#[test] +fn a_render_with_nothing_in_the_way_needs_no_flag() { + let mut f = Fixture::new("free"); + f.comp("Main"); + f.s.execute("renderQueue.add", json!({})).unwrap(); + assert_eq!(f.output(0), f.path("Main.mp4"), "no footage in the way: the comp's own name"); + f.render(json!({})).unwrap(); + assert_eq!(f.read("Main.mp4"), RENDERED); +} + +/// Skip Existing Files leaves the frames that exist alone, so they need no answer. +#[test] +fn a_sequence_that_skips_existing_frames_needs_no_flag() { + let mut f = Fixture::new("skip-existing"); + f.comp("Main"); + let pattern = f.text("render_[#####].png"); + f.s.execute("renderQueue.add", json!({"format": "png", "output": pattern, "skipExisting": true})).unwrap(); + for n in 0..3 { + f.write(&format!("render_{n:05}.png"), EARLIER); + } + f.render(json!({})).unwrap(); + assert_eq!(f.read("render_00002.png"), EARLIER); + assert_eq!(f.read("render_00003.png"), RENDERED); + // Ordinary files still follow the exporter's replacement policy when skipping is off. + f.s.execute("renderQueue.setRenderSettings", json!({"index": 1, "skipExisting": false})).unwrap(); + f.render(json!({})).unwrap(); + assert_eq!(f.read("render_00002.png"), RENDERED); +} + +/// A link is another name for the file it points to. +#[test] +fn a_link_to_the_footage_is_the_footage() { + let mut f = Fixture::new("link"); + f.footage("clip.mov"); + f.comp("Main"); + let alias = f.path("render.mov"); + if link(&f.path("clip.mov"), &alias).is_err() { + eprintln!("this account cannot make symbolic links: skipped"); + return; + } + f.s.execute("renderQueue.add", json!({"format": "prores", "output": f.text("render.mov")})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("render.mov") && e.contains("footage"), "{e}"); + assert_eq!(f.read("clip.mov"), SOURCE); + assert!(std::fs::symlink_metadata(&alias).unwrap().file_type().is_symlink(), "the link was not replaced either"); +} + +/// A hard link to the footage under another name is the same file on disk: replacing it would truncate the footage. +#[test] +fn a_hard_link_to_the_footage_is_the_footage() { + let mut f = Fixture::new("hardlink"); + f.footage("clip.mov"); + f.comp("Main"); + if std::fs::hard_link(f.path("clip.mov"), f.path("render.mov")).is_err() { + eprintln!("this volume cannot make hard links: skipped"); + return; + } + f.s.execute("renderQueue.add", json!({"format": "prores", "output": f.text("render.mov")})).unwrap(); + // not even with overwrite + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("render.mov") && e.contains("footage"), "{e}"); + assert_eq!(f.read("clip.mov"), SOURCE); + assert_eq!(f.read("render.mov"), SOURCE, "the other name still leads to the footage"); + assert!(f.exported().is_empty(), "nothing was rendered"); +} + +/// A copy of the footage has its size and bytes, but it is another file: an earlier render may replace it. +#[test] +fn a_copy_of_the_footage_is_not_the_footage() { + let mut f = Fixture::new("copy"); + f.footage("clip.mov"); + f.comp("Main"); + std::fs::copy(f.path("clip.mov"), f.path("copy.mov")).unwrap(); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": f.text("copy.mov")})).unwrap(); + f.render(json!({})).unwrap(); + assert_eq!(f.read("copy.mov"), RENDERED); + assert_eq!(f.read("clip.mov"), SOURCE, "the footage is as it was"); +} + +/// Footage imported through a linked folder is the same file as the one in the real folder. +#[test] +fn footage_reached_through_a_linked_folder_is_still_footage() { + let mut f = Fixture::new("linked-folder"); + std::fs::create_dir_all(f.path("real")).unwrap(); + std::fs::write(f.path("real").join("clip.mov"), SOURCE).unwrap(); + if link_dir(&f.path("real"), &f.path("linked")).is_err() { + eprintln!("this account cannot link folders: skipped"); + return; + } + let via_link = + Footage { path: f.path("linked").join("clip.mov").to_string_lossy().into_owned(), kind: FootageKind::Video, has_video: true, ..Default::default() }; + Arc::make_mut(&mut f.s.project).add_item("clip.mov", Label::None, None, ItemKind::Footage(via_link)); + f.comp("Main"); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": f.path("real").join("clip.mov").to_string_lossy()})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("footage"), "{e}"); + assert_eq!(std::fs::read(f.path("real").join("clip.mov")).unwrap(), SOURCE); +} + +/// Where the volume ignores case, `CLIP.MOV` is `clip.mov`. +#[test] +fn another_spelling_of_the_footage_name_is_the_footage_where_the_volume_ignores_case() { + let mut f = Fixture::new("case"); + f.footage("clip.mov"); + f.comp("Main"); + if !f.path("CLIP.MOV").exists() { + eprintln!("this volume tells the two names apart: skipped"); + return; + } + let shouting = f.text("CLIP.MOV"); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": shouting})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("footage"), "{e}"); + assert_eq!(f.read("clip.mov"), SOURCE); +} + +#[test] +fn save_current_preview_never_replaces_footage() { + let mut f = Fixture::new("preview"); + f.footage("clip.mov"); + f.comp("Main"); + let clip = f.text("clip.mov"); + let e = f.s.execute("render.saveCurrentPreview", json!({"path": clip})).unwrap_err().to_string(); + assert!(e.contains("clip.mov") && e.contains("footage"), "{e}"); + assert_eq!(f.read("clip.mov"), SOURCE); + assert!(f.exported().is_empty()); + // Anywhere else it renders. + let elsewhere = f.text("preview.mp4"); + f.s.execute("render.saveCurrentPreview", json!({"path": elsewhere})).unwrap(); + assert_eq!(f.read("preview.mp4"), RENDERED); +} + +#[test] +fn save_current_preview_checks_sequence_frames_as_well_as_the_pattern() { + let mut f = Fixture::new("preview-sequence"); + f.sequence("plate", 0..5); + f.comp("Main"); + let error = f.s.execute("render.saveCurrentPreview", json!({"path": f.text("plate_[#####].png")})).unwrap_err().to_string(); + assert!(error.contains("plate_000") && error.contains("footage"), "{error}"); + for frame in 0..5 { + assert_eq!(f.read(&format!("plate_{frame:05}.png")), SOURCE); + } + assert!(f.exported().is_empty()); +} + +/// A watched folder renders unattended; delete the status file to render a project again. +#[test] +fn a_watched_folder_still_renders_a_project_again_over_its_outputs() { + let mut f = Fixture::new("watch"); + f.comp("Main"); + let out = f.text("Main.mp4"); + f.s.execute("renderQueue.add", json!({"output": out})).unwrap(); + f.s.execute("file.save", json!({})).unwrap(); + f.write("Main.mp4", EARLIER); + let folder = f.dir.to_string_lossy().into_owned(); + let done = crate::commands::watch_folder::poll(&mut f.s, &folder).unwrap(); + assert_eq!(done.len(), 1, "{done:?}"); + assert_eq!(done[0]["state"], "done", "{:?}", done[0]); + assert_eq!(f.read("Main.mp4"), RENDERED); +} diff --git a/crates/project/src/render_queue.rs b/crates/project/src/render_queue.rs index 2d3ab741b..c1cda7087 100644 --- a/crates/project/src/render_queue.rs +++ b/crates/project/src/render_queue.rs @@ -1455,22 +1455,56 @@ pub fn expand_template(template: &str, format: OutputFormat, v: &TemplateVars) - /// Replace the first run of `#` (optionally in brackets, `[####]`) with the zero-padded frame /// number. Paths without a run get `_NNNNN` before the extension. pub fn sequence_path(path: &str, frame: i64) -> String { + let (before, width, after) = sequence_parts(path); + format!("{before}{frame:0width$}{after}") +} + +/// A sequence output path around its frame number: what comes before it, how many digits it is +/// padded to and what comes after (`out/a_[#####].png` → `out/a_`, 5, `.png`). +pub fn sequence_parts(path: &str) -> (String, usize, String) { if let Some(start) = path.find('#') { let len = path[start..].chars().take_while(|c| *c == '#').count(); let (mut a, mut b) = (start, start + len); - if a > 0 && path.as_bytes()[a - 1] == b'[' && path.as_bytes().get(b) == Some(&b']') { + if a > 0 && path.as_bytes().get(a - 1) == Some(&b'[') && path.as_bytes().get(b) == Some(&b']') { a -= 1; b += 1; } - return format!("{}{:0width$}{}", &path[..a], frame, &path[b..], width = len); + return (path[..a].to_string(), len, path[b..].to_string()); } let p = std::path::Path::new(path); match p.extension() { - Some(ext) => format!("{}_{frame:05}.{}", p.with_extension("").to_string_lossy(), ext.to_string_lossy()), - None => format!("{path}_{frame:05}"), + Some(ext) => (format!("{}_", p.with_extension("").to_string_lossy()), 5, format!(".{}", ext.to_string_lossy())), + None => (format!("{path}_"), 5, String::new()), } } +/// The frame number `candidate` is the file of, when `path` (a sequence output path) writes it: +/// the inverse of [`sequence_path`]. +pub fn sequence_frame(path: &str, candidate: &str) -> Option { + let (before, width, after) = sequence_parts(path); + let digits = candidate.strip_prefix(before.as_str())?.strip_suffix(after.as_str())?; + let frame: i64 = digits.parse().ok()?; + (format!("{frame:0width$}") == digits).then_some(frame) +} + +/// `template` with `_n` added to the name: before the frame-number run of a sequence, else before +/// the extension (`[compName].[fileExtension]` → `[compName]_1.[fileExtension]`, +/// `[compName]_[#####].[fileExtension]` → `[compName]_1_[#####].[fileExtension]`). +pub fn numbered_template(template: &str, n: u32) -> String { + let at = match template.find('#') { + Some(hash) => { + // The run's `[`, then the separator that sets it off from the name. + let run = if template[..hash].ends_with('[') { hash - 1 } else { hash }; + if template[..run].ends_with(['_', '-', '.', ' ']) { run - 1 } else { run } + } + None => { + let name = template.rfind(['/', '\\']).map_or(0, |i| i + 1); + template[name..].rfind('.').map_or(template.len(), |i| name + i) + } + }; + format!("{}_{n}{}", &template[..at], &template[at..]) +} + /// Make a name safe as a file name component. pub fn sanitize(name: &str) -> String { name.chars().map(|c| if matches!(c, '/' | '\\' | ':' | '*' | '?' | '"' | '<' | '>' | '|') { '_' } else { c }).collect() @@ -1490,6 +1524,44 @@ mod tests { assert_eq!(sequence_path("a.png", 3), "a_00003.png"); } + /// `sequence_frame` reads back what `sequence_path` writes, and nothing else. + #[test] + fn sequence_frames_read_back() { + for path in ["out/a_[#####].png", "a###.tif", "a.png", "/x/[compName]/shot_[##].exr", "noext", "dir.v2/clip"] { + for frame in [0, 7, 42, 99_999, 123_456, -3] { + assert_eq!(sequence_frame(path, &sequence_path(path, frame)), Some(frame), "{path} {frame}"); + } + } + let (before, width, after) = sequence_parts("out/a_[#####].png"); + assert_eq!((before.as_str(), width, after.as_str()), ("out/a_", 5, ".png")); + let (before, width, after) = sequence_parts("a.png"); + assert_eq!((before.as_str(), width, after.as_str()), ("a_", 5, ".png")); + // Other numbers, other widths, other names are not frames of the sequence. + assert_eq!(sequence_frame("a_[#####].png", "a_0042.png"), None, "padded to four"); + assert_eq!(sequence_frame("a_[####].png", "a_+042.png"), None, "a sign is not padding"); + assert_eq!(sequence_frame("a_[#####].png", "a_00042.jpg"), None); + assert_eq!(sequence_frame("a_[#####].png", "b_00042.png"), None); + assert_eq!(sequence_frame("a_[#####].png", "a_00x42.png"), None); + assert_eq!(sequence_frame("a_[#####].png", "a_.png"), None); + assert_eq!(sequence_frame("a_[#####].png", "a_99999999999999999999.png"), None, "too big for a frame number"); + } + + #[test] + fn numbered_templates() { + assert_eq!(numbered_template(DEFAULT_TEMPLATE, 1), "[compName]_1.[fileExtension]"); + assert_eq!(numbered_template(DEFAULT_SEQUENCE_TEMPLATE, 2), "[compName]_2_[#####].[fileExtension]"); + assert_eq!(numbered_template("[compName]_frame.[fileExtension]", 3), "[compName]_frame_3.[fileExtension]"); + assert_eq!(numbered_template("out/[compName]###.png", 1), "out/[compName]_1###.png"); + assert_eq!(numbered_template("my.dir/clip", 4), "my.dir/clip_4", "a dot in a folder is not an extension"); + assert_eq!(numbered_template("clip.v2.mov", 1), "clip.v2_1.mov"); + assert_eq!(numbered_template("", 1), "_1"); + assert_eq!(numbered_template("#", 1), "_1#"); + assert_eq!(numbered_template("[#]", 1), "_1[#]"); + // A numbered default is still a default: it expands to the numbered name. + let v = TemplateVars { comp_name: "footage", project_name: "P", width: 64, height: 36, frame_rate: 30.0, start_frame: 0, end_frame: 29 }; + assert_eq!(expand_template(&numbered_template(DEFAULT_TEMPLATE, 1), OutputFormat::H264, &v), "footage_1.mp4"); + } + #[test] fn set_format_fixes_names() { let mut m = OutputModule::default(); diff --git a/docs/agents.md b/docs/agents.md index 5c58d24b7..183315366 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -4,6 +4,10 @@ Everything in EffectCraft is an engine command with a stable id and JSON params. panel gestures, the CLI, the control channel and MCP all go through the same commands, so anything a user can do, an agent can do too. There are three ways in: +Render-queue outputs cannot replace footage, proxies or the project file. See +[render source protection](render-source-protection.md) for aliases, default names and sequence +limits. This guard does not change ordinary-file overwrite defaults or add another confirmation API. + | Interface | Best for | Needs a window | |---|---|---| | **MCP** (`effectcraft-cli mcp`) | Claude Code and other MCP clients | no (headless), or yes with `--bridge` | diff --git a/docs/render-source-protection.md b/docs/render-source-protection.md new file mode 100644 index 000000000..238684ed4 --- /dev/null +++ b/docs/render-source-protection.md @@ -0,0 +1,26 @@ +# Render source protection + +The render queue refuses an output that would replace footage, an attached proxy, or the +project file. It checks every output module and segment before dispatching the item. A refused +item is marked Failed with its reason; other valid queued items can still render. Render logs +are checked too. No render parameter overrides this protection. + +Checks compare normalized paths and native file identities, so an existing symbolic link, +hard link, junction or Windows short name cannot disguise a source as another output file. +Source identities are indexed once per check and matching identities are verified. The index +does not keep file handles open for the lifetime of a long sequence. + +Default output names move aside when they would replace a protected file. This also applies to +Create Proxy: recreating a proxy chooses a new default name and attaches the new proxy, leaving +the previous proxy file intact. An explicit output path is kept as entered and refused if it +would replace a protected file. + +Image-sequence checks inspect existing frames in the requested range. Skip Existing Files leaves +existing frames alone. If a frame number appears in a folder name, the checker probes each +frame separately and refuses ranges over 100,000 frames instead of silently checking a prefix. +Choose a fixed output folder or a shorter range for those exports. + +This guard does not introduce an overwrite flag, confirmation dialog, or changed replacement +defaults for the CLI, scripts or MCP. Replacement consent and staging ordinary output files +belong to the exporter. The source check runs before export; filesystem changes between that +check and a write still require exporter-side revalidation. From 39137fbd784ca202142b349fa3c9db544b04f58c Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Sat, 24 Oct 2026 23:28:42 +0000 Subject: [PATCH 08/29] #1743: report refused queue items to CLI callers --- .../workflows/render-source-protection.yml | 2 + apps/effectcraft-cli/src/main.rs | 3 +- apps/effectcraft-cli/tests/it/cli.rs | 43 +++++++++++++++++++ crates/engine/src/commands/render_queue.rs | 12 +++++- crates/engine/src/tests_render_overwrite.rs | 35 +++++++++++++++ docs/render-source-protection.md | 4 ++ 6 files changed, 97 insertions(+), 2 deletions(-) diff --git a/.github/workflows/render-source-protection.yml b/.github/workflows/render-source-protection.yml index 3c040d831..a61c3bd4c 100644 --- a/.github/workflows/render-source-protection.yml +++ b/.github/workflows/render-source-protection.yml @@ -28,6 +28,8 @@ jobs: CARGO_INCREMENTAL: 0 steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: dtolnay/rust-toolchain@1.95.0 - run: cargo test --locked -p effectcraft-engine --lib output_guard::tests - run: cargo test --locked -p effectcraft-engine --lib tests_render_overwrite diff --git a/apps/effectcraft-cli/src/main.rs b/apps/effectcraft-cli/src/main.rs index 6e5b8d6cc..c858dec18 100644 --- a/apps/effectcraft-cli/src/main.rs +++ b/apps/effectcraft-cli/src/main.rs @@ -783,6 +783,7 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> { r["outputModuleSummary"].as_str().unwrap_or("") ); } + let attempted: std::collections::HashSet = s.project.render_queue.iter().filter(|i| i.is_queued()).map(|i| i.id).collect(); s.execute("renderQueue.render", json!({"wait": false})).map_err(err)?; let tty = std::io::IsTerminal::is_terminal(&std::io::stderr()); // `--progress`: one line per update for a program reading stderr (a terminal gets the live line). @@ -821,7 +822,7 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> { } let mut results = vec![]; let mut failed = vec![]; - for it in s.project.render_queue.iter().filter(|i| i.render && i.started.is_some()) { + for it in s.project.render_queue.iter().filter(|i| attempted.contains(&i.id)) { let name = s.project.item(it.comp).map(|i| i.name.clone()).unwrap_or_else(|| "?".into()); match &it.status { RenderStatus::Done => { diff --git a/apps/effectcraft-cli/tests/it/cli.rs b/apps/effectcraft-cli/tests/it/cli.rs index 538887d9b..97c604ce4 100644 --- a/apps/effectcraft-cli/tests/it/cli.rs +++ b/apps/effectcraft-cli/tests/it/cli.rs @@ -533,6 +533,49 @@ fn a_failed_render_under_run_and_exec_exits_1() { assert!(v["error"].as_str().is_some_and(|e| e.starts_with("T: ")), "{v}"); } +/// A source refusal happens before encoding and has no started timestamp. It must still +/// fail the CLI, while the valid queue item finishes and the imported frame stays intact. +#[test] +fn a_partly_protected_queue_exits_1_under_render_run_and_exec() { + let root = tmp("protected-mixed-queue"); + let _ = std::fs::remove_dir_all(&root); + std::fs::create_dir_all(&root).unwrap(); + let source = root.join("source_00000.png"); + image::RgbaImage::from_pixel(4, 4, image::Rgba([40, 80, 120, 255])).save(&source).unwrap(); + let original = std::fs::read(&source).unwrap(); + let project = root.join("q.ecproj"); + let good = root.join("good.gif"); + let import = json!({"paths": [source]}).to_string(); + let blocked = json!({"format": "png", "output": root.join("source_[#####].png")}).to_string(); + let valid = json!({"format": "gif", "output": good}).to_string(); + ok_json(&[ + "run", + "--empty", + "--save-as", + project.to_str().unwrap(), + "comp.new", + r#"{"name":"Main","width":16,"height":16,"frameRate":10,"duration":0.1}"#, + "layer.newSolid", + "{}", + "file.import", + &import, + "renderQueue.add", + &blocked, + "renderQueue.add", + &valid, + ]); + let p = project.to_str().unwrap(); + for args in [vec!["render", "--queue", "--project", p], vec!["run", p, "renderQueue.render", "{}"], vec!["exec", "renderQueue.render", "--project", p]] { + std::fs::write(&good, b"previous output").unwrap(); + let (code, result) = run_json(&args); + assert_eq!(code, 1, "{args:?}: {result}"); + assert!(result["error"].as_str().is_some_and(|e| e.contains("footage") && e.contains("source_00000.png")), "{result}"); + assert_eq!(std::fs::read(&source).unwrap(), original); + assert!(std::fs::read(&good).unwrap().starts_with(b"GIF8"), "the valid item still rendered"); + } + std::fs::remove_dir_all(root).unwrap(); +} + /// `render --bitrate` (#440) targets the bitrate in VP9 WebM. #[test] fn render_bitrate_applies_to_vp9_webm() { diff --git a/crates/engine/src/commands/render_queue.rs b/crates/engine/src/commands/render_queue.rs index d7e914b1a..5b71cfca5 100644 --- a/crates/engine/src/commands/render_queue.rs +++ b/crates/engine/src/commands/render_queue.rs @@ -681,8 +681,18 @@ fn render(s: &mut Session, p: &Value) -> Result { { log::warn!("{e}"); } + let queued: std::collections::HashSet = s.project.render_queue.iter().filter(|i| i.is_queued()).map(|i| i.id).collect(); let ids = s.start_render(wait).map_err(EngineError::Other)?; - let items: Vec = s.project.render_queue.iter().enumerate().filter(|(_, i)| ids.contains(&i.id)).map(|(k, i)| item_json(s, i, k)).collect(); + // Source refusals fail before the exporter starts. Report them just like encoding + // failures, so CLI/MCP callers do not mistake a partly refused queue for full success. + let items: Vec = s + .project + .render_queue + .iter() + .enumerate() + .filter(|(_, i)| ids.contains(&i.id) || (queued.contains(&i.id) && matches!(i.status, RenderStatus::Failed(_)))) + .map(|(k, i)| item_json(s, i, k)) + .collect(); Ok(json!({"rendering": s.is_rendering(), "items": items})) } diff --git a/crates/engine/src/tests_render_overwrite.rs b/crates/engine/src/tests_render_overwrite.rs index a2473c6e8..420763da2 100644 --- a/crates/engine/src/tests_render_overwrite.rs +++ b/crates/engine/src/tests_render_overwrite.rs @@ -230,6 +230,7 @@ fn the_rest_of_the_queue_renders_when_one_item_would_replace_footage() { assert_eq!(f.read("main.mp4"), RENDERED); assert!(matches!(f.s.project.render_queue[0].status, RenderStatus::Failed(_))); assert_eq!(f.s.project.render_queue[1].status, RenderStatus::Done); + assert!(r["items"].as_array().unwrap().iter().any(|i| i["status"].get("Failed").is_some()), "the reply includes the refused item: {r}"); assert!(r["items"].as_array().unwrap().iter().any(|i| i["status"] == "Done"), "{r}"); } @@ -313,6 +314,40 @@ fn creating_a_proxy_again_chooses_a_new_default_and_attaches_it() { } } +/// A pre-render becomes imported footage. Its old output stays protected even from the +/// same queue item; issuing Pre-render again supplies a fresh default output instead. +#[test] +fn refreshing_a_pre_render_uses_a_new_default_without_exempting_imported_footage() { + struct Probe; + impl crate::Importer for Probe { + fn probe(&self, path: &str) -> Result { + if !Path::new(path).is_file() { + return Err("pre-render missing".into()); + } + Ok(Footage { path: path.into(), kind: FootageKind::Video, width: 16, height: 16, has_video: true, ..Default::default() }) + } + } + let mut f = Fixture::new("refresh-pre-render"); + f.s.importer = Some(Arc::new(Probe)); + f.comp("Main"); + f.s.execute("render.preRender", json!({"format": "gif"})).unwrap(); + let first = f.output(0); + f.render(json!({})).unwrap(); + f.s.execute("renderQueue.setRender", json!({"index": 1, "render": true})).unwrap(); + let error = f.render(json!({})).unwrap_err().to_string(); + assert!(error.contains("footage"), "{error}"); + assert_eq!(std::fs::read(&first).unwrap(), RENDERED); + assert_eq!(f.exported().len(), 1); + f.s.execute("renderQueue.setRender", json!({"index": 1, "render": false})).unwrap(); + f.s.execute("render.preRender", json!({"format": "gif"})).unwrap(); + let second = f.output(1); + assert_ne!(first, second); + f.render(json!({})).unwrap(); + assert_eq!(std::fs::read(first).unwrap(), RENDERED); + assert_eq!(std::fs::read(second).unwrap(), RENDERED); + assert_eq!(f.exported().len(), 2); +} + /// Exercise the engine's import and project serialization paths, rather than injecting a /// Footage item directly. The importer reads dimensions from a real, generated PNG. #[test] diff --git a/docs/render-source-protection.md b/docs/render-source-protection.md index 238684ed4..cf15f6797 100644 --- a/docs/render-source-protection.md +++ b/docs/render-source-protection.md @@ -15,6 +15,10 @@ Create Proxy: recreating a proxy chooses a new default name and attaches the new the previous proxy file intact. An explicit output path is kept as entered and refused if it would replace a protected file. +After Pre-render imports its result, that file is footage too. Re-queueing the same item +with the same output is refused; run Pre-render again for a fresh default output, or choose +another Output To. An item's `last_output` is not permission to replace imported footage. + Image-sequence checks inspect existing frames in the requested range. Skip Existing Files leaves existing frames alone. If a frame number appears in a folder name, the checker probes each frame separately and refuses ranges over 100,000 frames instead of silently checking a prefix. From e50971022737863fe0f73f096298f393f3aa8a81 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 11:29:50 +0000 Subject: [PATCH 09/29] #712: register an automation id for the popup eyedropper's viewer surface Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01RDvxCS8xcR28VArjzn1x7U --- crates/ui-egui/src/panels/effect_controls/viewer.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/ui-egui/src/panels/effect_controls/viewer.rs b/crates/ui-egui/src/panels/effect_controls/viewer.rs index 5f471953c..c7f8981d1 100644 --- a/crates/ui-egui/src/panels/effect_controls/viewer.rs +++ b/crates/ui-egui/src/panels/effect_controls/viewer.rs @@ -186,6 +186,8 @@ fn popup_eyedropper(app: &mut EffectcraftApp, ui: &mut egui::Ui, painter: &egui: return; } let _ = ui.interact(map.area, egui::Id::unique("viewer-popup-eyedropper"), Sense::click()); + // The sampling surface, apart from the picker's button that arms it (`colorPicker.eyedropper`). + app.auto.add("viewer.colorPickerEyedropper", map.area, "Sample color"); ctx.set_cursor_icon(egui::CursorIcon::Crosshair); app.viewer_pixels(); let chip = Rect::from_min_size(map.area.min + vec2(10.0, 10.0), vec2(220.0, 22.0)); From d3d3b865395781551c0002aee6f03655d0a16815 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Thu, 29 Oct 2026 17:27:06 -0400 Subject: [PATCH 10/29] CI: the Windows toolchain jobs don't keep the checkout token on disk (refs #1886) --- .github/workflows/windows-toolchains.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/windows-toolchains.yml b/.github/workflows/windows-toolchains.yml index b6765455e..6d22ffc12 100644 --- a/.github/workflows/windows-toolchains.yml +++ b/.github/workflows/windows-toolchains.yml @@ -39,7 +39,10 @@ jobs: run: shell: bash steps: + # Build scripts run as this job's user; nothing here pushes, so don't leave the token on disk. - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - name: Rust (stable-${{ matrix.target }}) run: | rustup toolchain install stable-${{ matrix.target }} --profile minimal --no-self-update From 09d41e54ca799bac62bb22b7095aa3a53c64e6e8 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Thu, 29 Oct 2026 17:27:12 -0400 Subject: [PATCH 11/29] CI: bundled LLD also gets the ktmw32, propsys and version import libraries winsafe binds (refs #1886) --- .github/workflows/windows-toolchains.yml | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/.github/workflows/windows-toolchains.yml b/.github/workflows/windows-toolchains.yml index 6d22ffc12..1d00276c8 100644 --- a/.github/workflows/windows-toolchains.yml +++ b/.github/workflows/windows-toolchains.yml @@ -1,8 +1,9 @@ # Windows builds with the toolchains contributors use, not just the MSVC one releases use: build # the desktop app and the CLI with MSVC, with GNU (mingw-w64 from MSYS2, the setup CONTRIBUTING.md -# describes), and with GNU linked by Rust's bundled LLD plus mingw-w64's `libshlwapi.a` (#1886), -# then run each binary with `--version`. Both packages build in one `cargo build`, the way people -# build them, so features unify and the CLI links everything the desktop app pulls in. +# describes), and with GNU linked by Rust's bundled LLD plus the mingw-w64 import libraries its +# self-contained sysroot lacks (#1886), then run each binary with `--version`. Both packages build +# in one `cargo build`, the way people build them, so features unify and the CLI links everything +# the desktop app pulls in. name: Windows toolchains # Nightly on main (a dependency update is what brings in a new system library), and on PRs that @@ -55,13 +56,17 @@ jobs: if: matrix.target == 'x86_64-pc-windows-gnu' && !matrix.rust-lld run: | printf '%s\n' 'C:\msys64\mingw64\bin' >> "$GITHUB_PATH" - # Only libshlwapi.a, in a folder of its own: with all of mingw-w64's lib folder on the search - # path, its C runtime mixes with Rust's bundled one (undefined `DllEntryPoint`). - - name: Link with Rust's bundled LLD and mingw-w64's shlwapi import library + # Only the import libraries the self-contained sysroot lacks, in a folder of its own: with all + # of mingw-w64's lib folder on the search path, its C runtime mixes with Rust's bundled one + # (undefined `DllEntryPoint`). webbrowser, which opens links, binds shlwapi; winsafe, which the + # updater (craft-update) uses, binds shlwapi, ktmw32, propsys and version. + - name: Link with Rust's bundled LLD and mingw-w64's missing import libraries if: matrix.rust-lld run: | mkdir -p /c/gnu-import-libs - cp /c/msys64/mingw64/lib/libshlwapi.a /c/gnu-import-libs/ + for lib in shlwapi ktmw32 propsys version; do + cp "/c/msys64/mingw64/lib/lib$lib.a" /c/gnu-import-libs/ + done echo 'CARGO_TARGET_X86_64_PC_WINDOWS_GNU_LINKER=rust-lld' >> "$GITHUB_ENV" echo 'CARGO_TARGET_X86_64_PC_WINDOWS_GNU_RUSTFLAGS=-C link-self-contained=yes -L native=C:/gnu-import-libs' >> "$GITHUB_ENV" - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 From 6e7a008f6fcaf94730c772148b0ee67dad4da374 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Thu, 29 Oct 2026 17:27:26 -0400 Subject: [PATCH 12/29] Docs: bundled LLD needs mingw-w64's ktmw32, propsys and version import libraries too (refs #1886) --- CONTRIBUTING.md | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 840f29ed6..600239efa 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -56,15 +56,18 @@ An older MinGW earlier on `PATH` can break the build: CodeBlocks' GCC 8.1 fails `boa_engine` ([#1882](https://github.com/storytold/effectcraft/issues/1882)). Rust's bundled LLD and self-contained libraries don't include every import library EffectCraft -needs, so on their own they stop with `unable to find library -lshlwapi` -([#1886](https://github.com/storytold/effectcraft/issues/1886)). To link that way, copy -mingw-w64's `libshlwapi.a` into a folder of its own and add that folder to the search path in your -own Cargo config (`%USERPROFILE%\.cargo\config.toml`). Don't add all of mingw-w64's `lib` folder: -its C runtime clashes with Rust's bundled one (`undefined symbol: DllEntryPoint`). +needs, so on their own they stop with `unable to find library` for `-lshlwapi`, `-lktmw32`, +`-lpropsys` and `-lversion` ([#1886](https://github.com/storytold/effectcraft/issues/1886)). To +link that way, copy those four from mingw-w64 into a folder of its own and add that folder to the +search path in your own Cargo config (`%USERPROFILE%\.cargo\config.toml`). Don't add all of +mingw-w64's `lib` folder: its C runtime clashes with Rust's bundled one +(`undefined symbol: DllEntryPoint`). ```powershell New-Item -ItemType Directory -Force C:\gnu-import-libs -Copy-Item C:\msys64\mingw64\lib\libshlwapi.a C:\gnu-import-libs\ +foreach ($lib in "shlwapi", "ktmw32", "propsys", "version") { + Copy-Item "C:\msys64\mingw64\lib\lib$lib.a" C:\gnu-import-libs\ +} ``` ```toml From 0c35a7a889de94f8c02c2a5921a7af4d719f2af5 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 02:43:47 +0200 Subject: [PATCH 13/29] fix(effects): let CC Particle World's Textured Square use a Texture Layer (fixes #1465) --- crates/effects/src/catalog.rs | 1 + crates/effects/src/sim/particle_fx.rs | 52 ++++++++++++++++++++++++--- crates/effects/src/sim/registry.rs | 3 +- crates/effects/src/sim/sprites.rs | 2 +- crates/effects/src/sim/tests.rs | 47 ++++++++++++++++++++++++ docs/effects.md | 4 +-- 6 files changed, 100 insertions(+), 9 deletions(-) diff --git a/crates/effects/src/catalog.rs b/crates/effects/src/catalog.rs index 85be739ed..bac2bb065 100644 --- a/crates/effects/src/catalog.rs +++ b/crates/effects/src/catalog.rs @@ -89,6 +89,7 @@ const CPU_ONLY_CONTROLS: &[(&str, &[&str])] = &[ ("ec.distort.ccpowerpin", &["unstretch"]), ("ec.matte.refinesoft", &["reduceChatter", "useMotionBlur", "invert"]), ("ec.matte.refinehard", &["reduceChatter", "useMotionBlur", "invert"]), + ("ec.sim.ccparticleworld", &["particle/texture/textureLayer"]), ]; /// Whether the GPU kernel of effect `id` covers the instance's current settings (see diff --git a/crates/effects/src/sim/particle_fx.rs b/crates/effects/src/sim/particle_fx.rs index 440951c83..ca5c3c279 100644 --- a/crates/effects/src/sim/particle_fx.rs +++ b/crates/effects/src/sim/particle_fx.rs @@ -1,5 +1,5 @@ use super::engine::{MERCURY, PS2, PState, PW_EFFECT_CAMERA, Phys, norm3, particle_look, pw_state, type_sprite}; -use super::sprites::{Acc, Post, Sprite, SpritePlan, raster, splat}; +use super::sprites::{Acc, Post, Sprite, SpritePlan, aa, raster, splat, sprite_bbox}; use super::{Buf, EffectCtx, Image, IntoParallelRefIterator, ParallelIterator, SimCache, Value, unpremul}; // ------------------------------------------------------------------ CC Particle World @@ -35,7 +35,18 @@ pub(super) fn pw_phys(ctx: &EffectCtx) -> Phys { } } -pub(super) fn particle_world(ctx: &EffectCtx, b: Buf) -> Buf { +/// CC Particle World's Particle Type ▸ Textured Square. +const PW_TEXTURED_SQUARE: u32 = 8; +/// CC Particle World's Particle ▸ Texture ▸ Texture Layer. +pub(super) const PW_TEXTURE_LAYER: &str = "particle/texture/textureLayer"; + +/// Whether CC Particle World draws its Texture Layer on the particles (Textured Square with a +/// layer chosen). Only the CPU does: the Texture Layer is a CPU-only control in `catalog`. +fn particle_world_textured(ctx: &EffectCtx) -> bool { + ctx.params.e("particle/particleType") == PW_TEXTURED_SQUARE && ctx.params.get(PW_TEXTURE_LAYER).and_then(Value::as_layer).is_some() +} + +pub(super) fn particle_world(ctx: &EffectCtx, mut b: Buf) -> Buf { // #1195: with the comp's motion-blur shutter open, particles draw as motion trails: the // sprite fields of samples across the shutter are averaged, as Transform averages its // matrix samples. (The GPU compositor runs this case on the CPU: catalog::gpu_supported.) @@ -59,12 +70,43 @@ pub(super) fn particle_world(ctx: &EffectCtx, b: Buf) -> Buf { None => b, }; } - match particle_world_plan(ctx, &b) { - Some(plan) => plan.finish(b), - None => b, + let Some(plan) = particle_world_plan(ctx, &b) else { return b }; + // The texture at the current comp time. A layer the host can't give (the effect's own + // layer with its effects, a reference cycle) leaves the squares in the particle colour. + match particle_world_textured(ctx).then(|| ctx.layer_param(PW_TEXTURE_LAYER, true)).flatten() { + // The particles replace the layer ([`particle_sprites`]). + Some(tex) => { + b.img = splat_textured(b.img.width, b.img.height, &plan.sprites, plan.acc, &tex); + b + } + None => plan.finish(b), } } +/// Draw square sprites into a new transparent image of `w`×`h`, each showing the layer `tex` +/// stretched over the square (turned with it) at the sprite's opacity. +fn splat_textured(w: u32, hgt: u32, sprites: &[Sprite], acc: Acc, tex: &crate::LayerPixels) -> Image { + let mut img = Image::new(w, hgt); + let (tw, th) = (tex.size[0], tex.size[1]); + let shade = |s: &Sprite, x: f32, y: f32| { + // As [`sprite_shade`]'s Square, sub-pixel sprites included. + let (r, fade) = if s.r < 0.5 { (0.5, s.r / 0.5) } else { (s.r, 1.0) }; + let (sn, cs) = s.rot.sin_cos(); + let (dx, dy) = (x - s.x, y - s.y); + let (ux, uy) = (dx * cs + dy * sn, -dx * sn + dy * cs); + let e = r * 0.75; + let a = aa(e - ux.abs()) * aa(e - uy.abs()) * s.c[3] * fade; + if a <= 1e-6 { + return None; + } + let (u, v) = (((ux / e + 1.0) * 0.5).clamp(0.0, 1.0) as f64, ((uy / e + 1.0) * 0.5).clamp(0.0, 1.0) as f64); + let (px, py) = tex.buf.to_px([u * tw, v * th]); + Some(tex.buf.img.sample_bilinear_clamped(px, py).map(|c| c * a)) + }; + raster(&mut img, sprites, sprite_bbox, shade, acc); + img +} + /// Effect Camera ▸ FOV's default (degrees). pub(super) const PW_FOV: f64 = 50.0; diff --git a/crates/effects/src/sim/registry.rs b/crates/effects/src/sim/registry.rs index f29eb2653..f55b99e27 100644 --- a/crates/effects/src/sim/registry.rs +++ b/crates/effects/src/sim/registry.rs @@ -1,6 +1,6 @@ use super::common::{pct, pt, spec}; use super::engine::{PARTICLE_TYPES, PW_ANIMS}; -use super::particle_fx::{PW_FOV, mr_mercury, particle_systems2, particle_systems2_plan, particle_world, particle_world_plan}; +use super::particle_fx::{PW_FOV, PW_TEXTURE_LAYER, mr_mercury, particle_systems2, particle_systems2_plan, particle_world, particle_world_plan}; use super::sprites::SpritePlan; use super::weather::{bubbles, drizzle, rainfall, rainfall_plan, snowfall, snowfall_plan, star_burst, star_burst_plan}; use super::{Buf, EffectCtx, EffectSpec, ParamUi, Value, col, num, p, popup, slider}; @@ -151,6 +151,7 @@ pub fn specs() -> Vec { p("physics/gravityVector/gravityY", "Gravity Y", num(1.0), slider(-1.0, 1.0, -1.0, 1.0, 2)), p("physics/gravityVector/gravityZ", "Gravity Z", num(0.0), slider(-1.0, 1.0, -1.0, 1.0, 2)), p("particle/particleType", "Particle Type", Value::Enum(0), popup(PARTICLE_TYPES)), + p(PW_TEXTURE_LAYER, "Texture Layer", Value::Layer(None), ParamUi::Layer), p("particle/birthSize", "Birth Size", num(0.2), slider(0.0, 10.0, 0.0, 1.0, 3)), p("particle/deathSize", "Death Size", num(0.2), slider(0.0, 10.0, 0.0, 1.0, 3)), p("particle/sizeVariation", "Size Variation", num(0.0), pct()), diff --git a/crates/effects/src/sim/sprites.rs b/crates/effects/src/sim/sprites.rs index 78c405abd..55ba594b4 100644 --- a/crates/effects/src/sim/sprites.rs +++ b/crates/effects/src/sim/sprites.rs @@ -225,7 +225,7 @@ impl Post { } } -fn sprite_bbox(s: &Sprite) -> Option<[f32; 4]> { +pub(super) fn sprite_bbox(s: &Sprite) -> Option<[f32; 4]> { if s.c[3] <= 1e-5 || s.r <= 0.0 { return None; } diff --git a/crates/effects/src/sim/tests.rs b/crates/effects/src/sim/tests.rs index e368c7bbe..a1d2f14e2 100644 --- a/crates/effects/src/sim/tests.rs +++ b/crates/effects/src/sim/tests.rs @@ -329,6 +329,53 @@ fn particle_world_effect_camera() { assert_ne!(key(¶ms), key(&moved)); } +/// A host whose every layer is `img`. +struct TexHost(Image); +impl crate::EffectHost for TexHost { + fn layer(&self, _: u64, _: bool) -> Option { + let s = [self.0.width as f64, self.0.height as f64]; + Some(crate::LayerPixels { buf: Buf { img: self.0.clone(), offset: [0.0; 2], scale: 1.0 }, size: s }) + } + fn audio(&self, _: u64, _: f64, _: usize, _: u32) -> Option> { + None + } +} + +/// #1465: Textured Square draws the Texture Layer on each particle, not the particle colour. +#[test] +fn particle_world_textured_square_draws_the_texture_layer() { + // Left half green, right half blue; the particles themselves are red. + let mut tex = Image::new(8, 8); + for y in 0..8 { + for x in 0..8 { + tex.set(x, y, if x < 4 { [0.0, 1.0, 0.0, 1.0] } else { [0.0, 0.0, 1.0, 1.0] }); + } + } + let host = TexHost(tex); + let vals = |layer: Option| { + vec![ + ("birthRate", num(1.0)), + ("extras/randomSeed", num(4.0)), + ("particle/particleType", Value::Enum(8)), + ("particle/texture/textureLayer", Value::Layer(layer)), + ("particle/birthSize", num(1.0)), + ("particle/deathSize", num(1.0)), + ("particle/birthColor", col(1.0, 0.0, 0.0)), + ("particle/deathColor", col(1.0, 0.0, 0.0)), + ] + }; + let env = EffectEnv { host: Some(&host), ..Default::default() }; + let out = run_fx("ec.sim.ccparticleworld", &vals(Some(1)), Image::new(48, 48), 1.0, env).img; + let seen: Vec<_> = out.data.iter().filter(|p| p[3] > 0.05).collect(); + assert!(!seen.is_empty(), "particles drawn"); + assert!(seen.iter().all(|p| p[0] < 0.02 * p[3]), "no particle colour: {:?}", seen.iter().find(|p| p[0] >= 0.02 * p[3])); + // Both halves of the texture show: it is mapped across the square, not sampled once. + assert!(seen.iter().any(|p| p[1] > 2.0 * p[2]) && seen.iter().any(|p| p[2] > 2.0 * p[1]), "both texture halves"); + // Without a texture layer the squares keep the particle colour. + let plain = run_fx("ec.sim.ccparticleworld", &vals(None), Image::new(48, 48), 1.0, env).img; + assert!(plain.data.iter().any(|p| p[3] > 0.05 && p[0] > 0.9 * p[3])); +} + #[test] fn particle_systems2_seek_and_direction() { let v = vec![ diff --git a/docs/effects.md b/docs/effects.md index c6fb40922..b3adb46ec 100644 --- a/docs/effects.md +++ b/docs/effects.md @@ -3,7 +3,7 @@ -EffectCraft ships 308 effects with 3033 parameters in total, grouped into the same categories as the Effects & Presets panel. Every effect is our own implementation, written from public behaviour descriptions and standard image-processing literature. Parameter names, order, popup options, units, defaults and ranges follow the reference application so that projects, expressions and muscle memory carry over. +EffectCraft ships 308 effects with 3034 parameters in total, grouped into the same categories as the Effects & Presets panel. Every effect is our own implementation, written from public behaviour descriptions and standard image-processing literature. Parameter names, order, popup options, units, defaults and ranges follow the reference application so that projects, expressions and muscle memory carry over. - **GPU**: 280 effects also run on the GPU compositor with identical results. - **32**: 308 effects process 32-bit float (HDR, overbright) pixels without clamping. @@ -325,7 +325,7 @@ Effects are addressed by id (`ec..`) or display name in commands | CC Hair | `ec.sim.cchair` | 20 | GPU | 32 | Implemented | | CC Mr. Mercury | `ec.sim.ccmrmercury` | 23 | GPU | 32 | Implemented | | CC Particle Systems II | `ec.sim.ccparticlesystems2` | 21 | GPU | 32 | Implemented | -| CC Particle World | `ec.sim.ccparticleworld` | 35 | GPU | 32 | Implemented | +| CC Particle World | `ec.sim.ccparticleworld` | 36 | GPU | 32 | Implemented | | CC Pixel Polly | `ec.sim.ccpixelpolly` | 10 | GPU | 32 | Implemented | | CC Rainfall | `ec.sim.ccrainfall` | 14 | GPU | 32 | Implemented | | CC Scatterize | `ec.sim.ccscatterize` | 4 | GPU | 32 | Implemented | From 2a5e9066e5e74666776a651b75af5fa83a95586b Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 21 Oct 2026 10:33:36 +0200 Subject: [PATCH 14/29] fix(effects): draw Particle World's Texture Layer in the motion-trail samples too With the shutter open the textured squares fell back to the particle colour. Both branches now draw through the same sprite closure, and the sharp frame goes through plan.post like the trails. --- crates/effects/src/sim/particle_fx.rs | 29 ++++++++------ crates/effects/src/sim/tests.rs | 56 +++++++++++++++++++-------- 2 files changed, 56 insertions(+), 29 deletions(-) diff --git a/crates/effects/src/sim/particle_fx.rs b/crates/effects/src/sim/particle_fx.rs index ca5c3c279..a17a9f68b 100644 --- a/crates/effects/src/sim/particle_fx.rs +++ b/crates/effects/src/sim/particle_fx.rs @@ -46,20 +46,29 @@ fn particle_world_textured(ctx: &EffectCtx) -> bool { ctx.params.e("particle/particleType") == PW_TEXTURED_SQUARE && ctx.params.get(PW_TEXTURE_LAYER).and_then(Value::as_layer).is_some() } -pub(super) fn particle_world(ctx: &EffectCtx, mut b: Buf) -> Buf { +pub(super) fn particle_world(ctx: &EffectCtx, b: Buf) -> Buf { + // The texture at the current comp time, shared by every shutter sample. A layer the host + // can't give (the effect's own layer with its effects, a reference cycle) leaves the + // squares in the particle colour. + let tex = particle_world_textured(ctx).then(|| ctx.layer_param(PW_TEXTURE_LAYER, true)).flatten(); + let (w, h) = (b.img.width, b.img.height); + let draw = |plan: &SpritePlan| match &tex { + Some(t) => splat_textured(w, h, &plan.sprites, plan.acc, t), + None => splat(w, h, &plan.sprites, plan.acc), + }; // #1195: with the comp's motion-blur shutter open, particles draw as motion trails: the // sprite fields of samples across the shutter are averaged, as Transform averages its // matrix samples. (The GPU compositor runs this case on the CPU: catalog::gpu_supported.) if let Some((angle, phase, n)) = ctx.env.shutter.filter(|(a, _, _)| *a > 0.0) { let n = (n.clamp(2, 64) as usize).max(2); let fd = 1.0 / ctx.fps().max(0.001); - let mut acc = Image::new(b.img.width, b.img.height); + let mut acc = Image::new(w, h); let mut post: Option = None; for k in 0..n { let t = ctx.time + (phase + angle * k as f64 / (n - 1) as f64) / 360.0 * fd; let sub = EffectCtx { params: ctx.params, time: t, layer_size: ctx.layer_size, seed: ctx.seed, adjustment: ctx.adjustment, env: ctx.env }; if let Some(plan) = particle_world_plan(&sub, &b) { - let fx = splat(b.img.width, b.img.height, &plan.sprites, plan.acc); + let fx = draw(&plan); acc.data.iter_mut().zip(&fx.data).for_each(|(a, o)| (0..4).for_each(|c| a[c] += o[c])); post.get_or_insert(plan.post); } @@ -70,16 +79,12 @@ pub(super) fn particle_world(ctx: &EffectCtx, mut b: Buf) -> Buf { None => b, }; } - let Some(plan) = particle_world_plan(ctx, &b) else { return b }; - // The texture at the current comp time. A layer the host can't give (the effect's own - // layer with its effects, a reference cycle) leaves the squares in the particle colour. - match particle_world_textured(ctx).then(|| ctx.layer_param(PW_TEXTURE_LAYER, true)).flatten() { - // The particles replace the layer ([`particle_sprites`]). - Some(tex) => { - b.img = splat_textured(b.img.width, b.img.height, &plan.sprites, plan.acc, &tex); - b + match particle_world_plan(ctx, &b) { + Some(plan) => { + let fx = draw(&plan); + plan.finish_with(b, fx) } - None => plan.finish(b), + None => b, } } diff --git a/crates/effects/src/sim/tests.rs b/crates/effects/src/sim/tests.rs index a1d2f14e2..59780ca74 100644 --- a/crates/effects/src/sim/tests.rs +++ b/crates/effects/src/sim/tests.rs @@ -341,29 +341,36 @@ impl crate::EffectHost for TexHost { } } -/// #1465: Textured Square draws the Texture Layer on each particle, not the particle colour. -#[test] -fn particle_world_textured_square_draws_the_texture_layer() { - // Left half green, right half blue; the particles themselves are red. +/// A texture whose left half is green and right half blue. +fn two_tone_tex() -> Image { let mut tex = Image::new(8, 8); for y in 0..8 { for x in 0..8 { tex.set(x, y, if x < 4 { [0.0, 1.0, 0.0, 1.0] } else { [0.0, 0.0, 1.0, 1.0] }); } } - let host = TexHost(tex); - let vals = |layer: Option| { - vec![ - ("birthRate", num(1.0)), - ("extras/randomSeed", num(4.0)), - ("particle/particleType", Value::Enum(8)), - ("particle/texture/textureLayer", Value::Layer(layer)), - ("particle/birthSize", num(1.0)), - ("particle/deathSize", num(1.0)), - ("particle/birthColor", col(1.0, 0.0, 0.0)), - ("particle/deathColor", col(1.0, 0.0, 0.0)), - ] - }; + tex +} + +/// CC Particle World drawing red Textured Squares with Texture Layer `layer`. +fn pw_textured_vals(layer: Option) -> Vec<(&'static str, Value)> { + vec![ + ("birthRate", num(1.0)), + ("extras/randomSeed", num(4.0)), + ("particle/particleType", Value::Enum(8)), + ("particle/texture/textureLayer", Value::Layer(layer)), + ("particle/birthSize", num(1.0)), + ("particle/deathSize", num(1.0)), + ("particle/birthColor", col(1.0, 0.0, 0.0)), + ("particle/deathColor", col(1.0, 0.0, 0.0)), + ] +} + +/// #1465: Textured Square draws the Texture Layer on each particle, not the particle colour. +#[test] +fn particle_world_textured_square_draws_the_texture_layer() { + let host = TexHost(two_tone_tex()); + let vals = pw_textured_vals; let env = EffectEnv { host: Some(&host), ..Default::default() }; let out = run_fx("ec.sim.ccparticleworld", &vals(Some(1)), Image::new(48, 48), 1.0, env).img; let seen: Vec<_> = out.data.iter().filter(|p| p[3] > 0.05).collect(); @@ -376,6 +383,21 @@ fn particle_world_textured_square_draws_the_texture_layer() { assert!(plain.data.iter().any(|p| p[3] > 0.05 && p[0] > 0.9 * p[3])); } +/// #1465 with #1195's motion trails: with the shutter open the averaged samples still show +/// the Texture Layer, not the particle colour. +#[test] +fn particle_world_textured_square_motion_trails_draw_the_texture_layer() { + let host = TexHost(two_tone_tex()); + let env = EffectEnv { host: Some(&host), frame_rate: 10.0, shutter: Some((360.0, -180.0, 8)), ..Default::default() }; + let blurred = run_fx("ec.sim.ccparticleworld", &pw_textured_vals(Some(1)), Image::new(48, 48), 1.0, env).img; + let sharp = run_fx("ec.sim.ccparticleworld", &pw_textured_vals(Some(1)), Image::new(48, 48), 1.0, EffectEnv { shutter: None, ..env }).img; + assert_ne!(sharp.data, blurred.data, "the shutter smears the particles"); + let seen: Vec<_> = blurred.data.iter().filter(|p| p[3] > 0.02).collect(); + assert!(!seen.is_empty(), "particles drawn"); + assert!(seen.iter().all(|p| p[0] < 0.02 * p[3]), "no particle colour: {:?}", seen.iter().find(|p| p[0] >= 0.02 * p[3])); + assert!(seen.iter().any(|p| p[1] > 2.0 * p[2]) && seen.iter().any(|p| p[2] > 2.0 * p[1]), "both texture halves"); +} + #[test] fn particle_systems2_seek_and_direction() { let v = vec![ From a76c65bd007defda63dacdc04c82fed3c3d85075 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 21 Oct 2026 10:38:37 +0200 Subject: [PATCH 15/29] fix(effects): keep Particle World on the GPU unless Textured Square draws the Texture Layer A Texture Layer left set with another Particle Type forced the CPU for nothing. gpu_supported now asks the same predicate the CPU path uses. --- crates/effects/src/catalog.rs | 11 ++++++++--- crates/effects/src/sim.rs | 1 + crates/effects/src/sim/particle_fx.rs | 4 ++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/crates/effects/src/catalog.rs b/crates/effects/src/catalog.rs index bac2bb065..332968baf 100644 --- a/crates/effects/src/catalog.rs +++ b/crates/effects/src/catalog.rs @@ -89,12 +89,11 @@ const CPU_ONLY_CONTROLS: &[(&str, &[&str])] = &[ ("ec.distort.ccpowerpin", &["unstretch"]), ("ec.matte.refinesoft", &["reduceChatter", "useMotionBlur", "invert"]), ("ec.matte.refinehard", &["reduceChatter", "useMotionBlur", "invert"]), - ("ec.sim.ccparticleworld", &["particle/texture/textureLayer"]), ]; /// Whether the GPU kernel of effect `id` covers the instance's current settings (see -/// `CPU_ONLY_CONTROLS`; plus Cell Pattern's HQ variants and Turbulent Displace's locked -/// pinning). +/// `CPU_ONLY_CONTROLS`; plus Cell Pattern's HQ variants, Turbulent Displace's locked +/// pinning and CC Particle World's motion trails and textured squares). pub fn gpu_supported(id: &str, ctx: &crate::EffectCtx) -> bool { match id { "ec.generate.cellpattern" if (6..=10).contains(&ctx.params.e("cellPattern")) => return false, @@ -102,6 +101,8 @@ pub fn gpu_supported(id: &str, ctx: &crate::EffectCtx) -> bool { // #1195: CC Particle World's motion trails (the averaged shutter samples) render on the // CPU kernel until the GPU particle pass supports them. "ec.sim.ccparticleworld" if ctx.env.shutter.filter(|(a, _, _)| *a > 0.0).is_some() => return false, + // #1465: Textured Square's Texture Layer is drawn on the CPU only. + "ec.sim.ccparticleworld" if crate::sim::particle_world_textured(ctx) => return false, _ => {} } let Some((_, controls)) = CPU_ONLY_CONTROLS.iter().find(|(e, _)| *e == id) else { return true }; @@ -225,6 +226,10 @@ mod tests { assert!(!super::gpu_supported("ec.sim.ccparticleworld", &ctx)); } assert!(ok("ec.sim.ccparticleworld", &[])); + // #1465: the Texture Layer keeps CC Particle World on the CPU only when Textured Square draws it. + let tex = ("particle/texture/textureLayer", Value::Layer(Some(1))); + assert!(ok("ec.sim.ccparticleworld", &[tex.clone(), ("particle/particleType", Value::Enum(2))])); + assert!(!ok("ec.sim.ccparticleworld", &[tex, ("particle/particleType", Value::Enum(8))])); assert!(ok("ec.generate.cellpattern", &[("cellPattern", Value::Enum(3))])); assert!(!ok("ec.generate.cellpattern", &[("cellPattern", Value::Enum(9))])); assert!(!ok("ec.distort.turbulentdisplace", &[("pinning", Value::Enum(9))])); diff --git a/crates/effects/src/sim.rs b/crates/effects/src/sim.rs index 79a225f75..248a66178 100644 --- a/crates/effects/src/sim.rs +++ b/crates/effects/src/sim.rs @@ -32,6 +32,7 @@ use engine::{MERCURY, PS2, PW_EFFECT_CAMERA, Stepped, rate_history, sim_key, sim #[cfg(test)] pub(crate) use engine::{PState, PW_ANIMS, Phys, simulate}; pub(crate) use engine::{SPS, steps_at}; +pub(crate) use particle_fx::particle_world_textured; pub use particle_fx::{Blob, mercury_blobs}; #[cfg(test)] use particle_fx::{PW_RATE, mercury_phys, ps2_phys, pw_phys}; diff --git a/crates/effects/src/sim/particle_fx.rs b/crates/effects/src/sim/particle_fx.rs index a17a9f68b..382913964 100644 --- a/crates/effects/src/sim/particle_fx.rs +++ b/crates/effects/src/sim/particle_fx.rs @@ -41,8 +41,8 @@ const PW_TEXTURED_SQUARE: u32 = 8; pub(super) const PW_TEXTURE_LAYER: &str = "particle/texture/textureLayer"; /// Whether CC Particle World draws its Texture Layer on the particles (Textured Square with a -/// layer chosen). Only the CPU does: the Texture Layer is a CPU-only control in `catalog`. -fn particle_world_textured(ctx: &EffectCtx) -> bool { +/// layer chosen). Only the CPU does: `catalog::gpu_supported` keeps this case off the GPU. +pub(crate) fn particle_world_textured(ctx: &EffectCtx) -> bool { ctx.params.e("particle/particleType") == PW_TEXTURED_SQUARE && ctx.params.get(PW_TEXTURE_LAYER).and_then(Value::as_layer).is_some() } From efd6ab97dc082cf56baa008e17cbf2c2aabf60f0 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 21 Oct 2026 11:37:31 +0200 Subject: [PATCH 16/29] fix(effects): cover Particle World's Texture Layer buffer before sampling it Text and shape layers hand over a buffer that spans only their content; the clamped sampling then stretched its edge pixels across the squares. --- crates/effects/src/sim/particle_fx.rs | 8 ++++-- crates/effects/src/sim/tests.rs | 35 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/crates/effects/src/sim/particle_fx.rs b/crates/effects/src/sim/particle_fx.rs index 382913964..c2f493ab5 100644 --- a/crates/effects/src/sim/particle_fx.rs +++ b/crates/effects/src/sim/particle_fx.rs @@ -49,8 +49,12 @@ pub(crate) fn particle_world_textured(ctx: &EffectCtx) -> bool { pub(super) fn particle_world(ctx: &EffectCtx, b: Buf) -> Buf { // The texture at the current comp time, shared by every shutter sample. A layer the host // can't give (the effect's own layer with its effects, a reference cycle) leaves the - // squares in the particle colour. - let tex = particle_world_textured(ctx).then(|| ctx.layer_param(PW_TEXTURE_LAYER, true)).flatten(); + // squares in the particle colour. Authored buffers (text, shapes) span only their content: + // covering the whole layer keeps the clamped sampling from stretching their edge pixels. + let tex = particle_world_textured(ctx).then(|| ctx.layer_param(PW_TEXTURE_LAYER, true)).flatten().map(|mut t| { + t.buf.cover(t.size); + t + }); let (w, h) = (b.img.width, b.img.height); let draw = |plan: &SpritePlan| match &tex { Some(t) => splat_textured(w, h, &plan.sprites, plan.acc, t), diff --git a/crates/effects/src/sim/tests.rs b/crates/effects/src/sim/tests.rs index 59780ca74..7166e3f0a 100644 --- a/crates/effects/src/sim/tests.rs +++ b/crates/effects/src/sim/tests.rs @@ -398,6 +398,41 @@ fn particle_world_textured_square_motion_trails_draw_the_texture_layer() { assert!(seen.iter().any(|p| p[1] > 2.0 * p[2]) && seen.iter().any(|p| p[2] > 2.0 * p[1]), "both texture halves"); } +/// A host whose every layer is `0` as given (a buffer that may span only its content). +struct PixelsHost(crate::LayerPixels); +impl crate::EffectHost for PixelsHost { + fn layer(&self, _: u64, _: bool) -> Option { + Some(self.0.clone()) + } + fn audio(&self, _: u64, _: f64, _: usize, _: u32) -> Option> { + None + } +} + +/// #1465: a Texture Layer whose buffer spans only its content (text, shapes) is mapped as +/// the whole layer, transparent around the content, not with its edge pixels stretched. +#[test] +fn particle_world_texture_layer_with_a_content_sized_buffer() { + // An 8×8 layer with a green 4×4 square in the middle: once as the full layer, once as + // just the square's pixels. + let mut full = Image::new(8, 8); + for y in 2..6 { + for x in 2..6 { + full.set(x, y, [0.0, 1.0, 0.0, 1.0]); + } + } + let content = Image::filled(4, 4, [0.0, 1.0, 0.0, 1.0]); + let render = |buf: Buf| { + let host = PixelsHost(crate::LayerPixels { buf, size: [8.0, 8.0] }); + let env = EffectEnv { host: Some(&host), ..Default::default() }; + run_fx("ec.sim.ccparticleworld", &pw_textured_vals(Some(1)), Image::new(48, 48), 1.0, env).img + }; + let want = render(Buf { img: full, offset: [0.0; 2], scale: 1.0 }); + let got = render(Buf { img: content, offset: [-2.0, -2.0], scale: 1.0 }); + assert!(want.data.iter().any(|p| p[3] > 0.05), "particles drawn"); + assert_eq!(want.data, got.data, "the content-sized buffer draws as the full layer"); +} + #[test] fn particle_systems2_seek_and_direction() { let v = vec![ From 3cd7a197104730aefd05991b14c670cdcf43b415 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 23 Oct 2026 19:54:26 +0200 Subject: [PATCH 17/29] Character panel: All Fills Over All Strokes and All Strokes Over All Fills in the stroke order menu --- crates/ui-egui/src/panels/text_panels.rs | 70 +++++++++++++++-- crates/ui-egui/tests/it/main.rs | 1 + .../tests/it/ui_character_stroke_order.rs | 77 +++++++++++++++++++ 3 files changed, 141 insertions(+), 7 deletions(-) create mode 100644 crates/ui-egui/tests/it/ui_character_stroke_order.rs diff --git a/crates/ui-egui/src/panels/text_panels.rs b/crates/ui-egui/src/panels/text_panels.rs index 38c93ebd4..33e9703b0 100644 --- a/crates/ui-egui/src/panels/text_panels.rs +++ b/crates/ui-egui/src/panels/text_panels.rs @@ -173,21 +173,43 @@ fn kerning_field(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, } } -/// Stroke Over Fill / Fill Over Stroke (grid row 2, right). -fn stroke_order_field(app: &mut EffectcraftApp, ui: &mut egui::Ui, g: Grid, doc: &TextDoc, actions: &mut Vec) { +/// Stroke order (grid row 2, right): per character (Fill Over Stroke / Stroke Over Fill), or for +/// the whole layer, which is the layer's More Options > Fill & Stroke (`fill_stroke`, see +/// [`layer_fill_stroke`]). Returns the layer order picked, if any. +fn stroke_order_field( + app: &mut EffectcraftApp, + ui: &mut egui::Ui, + g: Grid, + doc: &TextDoc, + fill_stroke: u32, + actions: &mut Vec, +) -> Option { let t = app.tokens; let Grid { x0, y, w, enabled } = g; let fx = x0 + w / 2.0; let fy = y + 2.0 * 28.0; let dr = Rect::from_min_size(pos2(fx, fy), vec2(w / 2.0, 20.0)); - let labels = vec!["Fill Over Stroke".to_string(), "Stroke Over Fill".to_string()]; + let labels: Vec = + ["Fill Over Stroke", "Stroke Over Fill", "-", "All Fills Over All Strokes", "All Strokes Over All Fills"].iter().map(|s| s.to_string()).collect(); + let current = match fill_stroke { + 1 => 3, + 2 => 4, + _ => doc.stroke_over_fill as usize, + }; let pop = egui::Id::unique("char-stroke-order-pop"); - if widgets::dropdown(ui, dr, &labels[doc.stroke_over_fill as usize], &t, egui::Id::unique("char-stroke-order")).clicked() && enabled { + if widgets::dropdown(ui, dr, &labels[current], &t, egui::Id::unique("char-stroke-order")).clicked() && enabled { widgets::open_popup(ui, pop); } app.auto.add("character.strokeOverFill", dr, "Stroke order"); - if let Some(i) = widgets::popup_menu(ui, pop, dr.left_bottom(), &labels, Some(doc.stroke_over_fill as usize)) { - actions.push(json!({"strokeOverFill": i == 1})); + match widgets::popup_menu(ui, pop, dr.left_bottom(), &labels, Some(current)) { + Some(i @ (3 | 4)) => Some(i as u32 - 2), + Some(i) => { + actions.push(json!({"strokeOverFill": i == 1})); + // A per-character order puts the layer back on the Per Character Palette, or it + // would have no visible effect. + (fill_stroke != 0).then_some(0) + } + None => None, } } @@ -205,6 +227,36 @@ fn line_join_field(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte } } +/// Sets the layer's More Options > Fill & Stroke to `v` together with the per-character order +/// it changes, as one undo step. +fn apply_layer_fill_stroke(app: &mut EffectcraftApp, ctx: &egui::Context, tt: &TextTarget, v: u32, actions: Vec) { + let set = json!({"command": "prop.set", "params": {"layer": tt.layer, "path": "text/moreOptions/fillStroke", "value": v}}); + let steps: Vec = + std::iter::once(set).chain(actions.into_iter().map(|a| json!({"command": "layer.setText", "params": tt.params(a)}))).collect(); + if let Err(e) = crate::menus::invoke(app, ctx, "engine.batch", json!({"steps": steps, "label": "Stroke Order"})) { + app.ui.status = e; + } +} + +/// A text layer's More Options > Fill & Stroke in the active comp: 0 per character palette, +/// 1 all fills over all strokes, 2 all strokes over all fills. +fn layer_fill_stroke(app: &EffectcraftApp, layer: u64) -> u32 { + let (Some(comp), Some(cid)) = (app.session.active_comp(), app.session.active_comp_id()) else { return 0 }; + let Some(l) = comp.layer(effectcraft_engine::project::LayerId(layer)) else { return 0 }; + let Some(more) = l.props.sub("text").and_then(|t| t.sub("moreOptions")) else { return 0 }; + let ectx = EvalCtx { + project: &app.session.project, + comp_id: cid, + comp, + time: app.session.time(), + expr: app.session.expr.as_deref(), + footage: None, + property_overrides: &[], + media_overrides: &[], + }; + ectx.e(l, more, "fillStroke") +} + /// The Character panel for the text being edited (or the selected text layer): font family and /// style, size, leading, tracking, stroke, scale, baseline shift and other numeric fields, kerning, /// fill/stroke colours with eyedroppers, style toggles, OpenType features and variable-font axes. @@ -246,7 +298,8 @@ pub fn character(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let grid = Grid { x0, y, w, enabled }; numeric_fields(app, ui, &p, grid, &doc, &mut actions); kerning_field(app, ui, &p, grid, &doc, &mut actions); - stroke_order_field(app, ui, grid, &doc, &mut actions); + let fill_stroke = target.as_ref().map_or(0, |tt| layer_fill_stroke(app, tt.layer)); + let layer_fill_stroke_pick = stroke_order_field(app, ui, grid, &doc, fill_stroke, &mut actions); line_join_field(app, ui, &p, grid, &doc, &mut actions); y += 6.0 * 28.0 + 8.0; // Fill / stroke swatches with their enable boxes. @@ -383,6 +436,9 @@ pub fn character(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { _ => {} } let Some(tt) = target else { return }; + if let Some(v) = layer_fill_stroke_pick { + apply_layer_fill_stroke(app, &ctx, &tt, v, std::mem::take(&mut actions)); + } for a in actions { if let Err(e) = crate::menus::invoke(app, &ctx, "layer.setText", tt.params(a)) { app.ui.status = e; diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index 54f641687..c08b948a7 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -13,6 +13,7 @@ mod ui_batch_switches; mod ui_blank_context_menus; mod ui_camera_tracker; mod ui_character_eyedropper; +mod ui_character_stroke_order; mod ui_color_hex; mod ui_comp_presets; mod ui_control_modifiers; diff --git a/crates/ui-egui/tests/it/ui_character_stroke_order.rs b/crates/ui-egui/tests/it/ui_character_stroke_order.rs new file mode 100644 index 000000000..db89f7c1a --- /dev/null +++ b/crates/ui-egui/tests/it/ui_character_stroke_order.rs @@ -0,0 +1,77 @@ +//! The Character panel's stroke order menu: the two per-character orders and the two layer-wide +//! ones (All Fills Over All Strokes, All Strokes Over All Fills), which set the text layer's +//! More Options ▸ Fill & Stroke. +use effectcraft_engine::{Session, commands::text_edit::layer_doc, project::LayerId}; +use effectcraft_ui_egui::{EffectcraftApp, dock::PanelKind}; +use egui::{ + Event, Modifiers, PointerButton, Pos2, + accesskit::{Role, Toggled}, + pos2, vec2, +}; +use egui_kittest::{ + Harness, + kittest::{NodeT, Queryable}, +}; +use serde_json::json; + +fn click(h: &mut Harness<'_, EffectcraftApp>, pos: Pos2) { + h.event(Event::PointerMoved(pos)); + h.step(); + for pressed in [true, false] { + h.event(Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }); + h.step(); + } + h.run_steps(2); +} + +fn open_menu(h: &mut Harness<'_, EffectcraftApp>) { + let e = h.state().auto.find("character.strokeOverFill").unwrap_or_else(|| panic!("missing character.strokeOverFill")).clone(); + click(h, pos2(e.rect[0] + e.rect[2] * 0.5, e.rect[1] + e.rect[3] * 0.5)); +} + +fn pick(h: &mut Harness<'_, EffectcraftApp>, label: &str) { + open_menu(h); + // A menu row: the dropdown shows the current order under the same label. + h.get_by_role_and_label(Role::Button, label).click(); + h.run_steps(3); +} + +fn fill_stroke(s: &mut Session, layer: u64) -> serde_json::Value { + s.execute("prop.get", json!({"layer": layer, "path": "text/moreOptions/fillStroke"})).unwrap()["value"].clone() +} + +#[test] +fn character_stroke_order_menu_sets_layer_fill_and_stroke() { + let mut s = Session::default(); + s.execute("comp.new", json!({"width": 320, "height": 180, "duration": 2})).unwrap(); + let text = s.execute("layer.newText", json!({"text": "Caption", "font": "Inter"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("layer.setText", json!({"layer": text, "applyStroke": true, "strokeWidth": 8.0, "strokeOverFill": false})).unwrap(); + let mut app = EffectcraftApp::new(s); + app.set_workspace("Text"); + let builder = Harness::builder().with_size(vec2(1600.0, 1400.0)); + let mut h = if std::env::var_os("EC_SNAPSHOT_DIR").is_some() { builder.wgpu().build_eframe(|_| app) } else { builder.build_eframe(|_| app) }; + h.state_mut().show_panel(PanelKind::Character); + h.run_steps(4); + + pick(&mut h, "All Fills Over All Strokes"); + assert_eq!(fill_stroke(&mut h.state_mut().session, text), json!(1)); + pick(&mut h, "All Strokes Over All Fills"); + assert_eq!(fill_stroke(&mut h.state_mut().session, text), json!(2)); + // The menu shows the layer's order as the current one. + open_menu(&mut h); + assert_eq!(h.get_by_role_and_label(Role::Button, "All Strokes Over All Fills").accesskit_node().toggled(), Some(Toggled::True)); + assert_eq!(h.get_by_role_and_label(Role::Button, "Fill Over Stroke").accesskit_node().toggled(), Some(Toggled::False)); + if let Ok(dir) = std::env::var("EC_SNAPSHOT_DIR") { + h.render().unwrap().save(format!("{dir}/character-stroke-order.png")).unwrap(); + } + h.key_press(egui::Key::Escape); + h.run_steps(2); + + // A per-character order goes back to the Per Character Palette, in one undo step. + pick(&mut h, "Stroke Over Fill"); + assert_eq!(fill_stroke(&mut h.state_mut().session, text), json!(0)); + assert!(layer_doc(&h.state().session, LayerId(text)).unwrap().stroke_over_fill); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + assert_eq!(fill_stroke(&mut h.state_mut().session, text), json!(2)); + assert!(!layer_doc(&h.state().session, LayerId(text)).unwrap().stroke_over_fill); +} From 41d9083aea77fea2a9029dd0344e6fb093134ffe Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Thu, 29 Oct 2026 20:57:11 -0400 Subject: [PATCH 18/29] CI: drop the Windows toolchains workflow (refs #1886) --- .github/workflows/windows-toolchains.yml | 82 ------------------------ 1 file changed, 82 deletions(-) delete mode 100644 .github/workflows/windows-toolchains.yml diff --git a/.github/workflows/windows-toolchains.yml b/.github/workflows/windows-toolchains.yml deleted file mode 100644 index 1d00276c8..000000000 --- a/.github/workflows/windows-toolchains.yml +++ /dev/null @@ -1,82 +0,0 @@ -# Windows builds with the toolchains contributors use, not just the MSVC one releases use: build -# the desktop app and the CLI with MSVC, with GNU (mingw-w64 from MSYS2, the setup CONTRIBUTING.md -# describes), and with GNU linked by Rust's bundled LLD plus the mingw-w64 import libraries its -# self-contained sysroot lacks (#1886), then run each binary with `--version`. Both packages build -# in one `cargo build`, the way people build them, so features unify and the CLI links everything -# the desktop app pulls in. -name: Windows toolchains - -# Nightly on main (a dependency update is what brings in a new system library), and on PRs that -# change this workflow or the lockfile. -on: - schedule: - - cron: "41 5 * * *" - pull_request: - paths: - - .github/workflows/windows-toolchains.yml - - Cargo.lock - workflow_dispatch: - -concurrency: - group: windows-toolchains-${{ github.ref }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} - -permissions: - contents: read - -jobs: - build: - name: ${{ matrix.name }} - runs-on: windows-latest - timeout-minutes: 90 - strategy: - fail-fast: false - matrix: - include: - - { name: msvc, target: x86_64-pc-windows-msvc } - - { name: gnu, target: x86_64-pc-windows-gnu } - - { name: gnu-rust-lld, target: x86_64-pc-windows-gnu, rust-lld: true } - defaults: - run: - shell: bash - steps: - # Build scripts run as this job's user; nothing here pushes, so don't leave the token on disk. - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - - name: Rust (stable-${{ matrix.target }}) - run: | - rustup toolchain install stable-${{ matrix.target }} --profile minimal --no-self-update - rustup default stable-${{ matrix.target }} - rustc -vV - - name: mingw-w64 from MSYS2 - if: matrix.target == 'x86_64-pc-windows-gnu' - run: /c/msys64/usr/bin/pacman -Sy --noconfirm --needed mingw-w64-x86_64-gcc - - name: Put it first on PATH - if: matrix.target == 'x86_64-pc-windows-gnu' && !matrix.rust-lld - run: | - printf '%s\n' 'C:\msys64\mingw64\bin' >> "$GITHUB_PATH" - # Only the import libraries the self-contained sysroot lacks, in a folder of its own: with all - # of mingw-w64's lib folder on the search path, its C runtime mixes with Rust's bundled one - # (undefined `DllEntryPoint`). webbrowser, which opens links, binds shlwapi; winsafe, which the - # updater (craft-update) uses, binds shlwapi, ktmw32, propsys and version. - - name: Link with Rust's bundled LLD and mingw-w64's missing import libraries - if: matrix.rust-lld - run: | - mkdir -p /c/gnu-import-libs - for lib in shlwapi ktmw32 propsys version; do - cp "/c/msys64/mingw64/lib/lib$lib.a" /c/gnu-import-libs/ - done - echo 'CARGO_TARGET_X86_64_PC_WINDOWS_GNU_LINKER=rust-lld' >> "$GITHUB_ENV" - echo 'CARGO_TARGET_X86_64_PC_WINDOWS_GNU_RUSTFLAGS=-C link-self-contained=yes -L native=C:/gnu-import-libs' >> "$GITHUB_ENV" - - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 - with: - key: ${{ matrix.name }} - - name: Build the desktop app and the CLI - run: | - gcc --version | head -1 || true - cargo build --locked --target ${{ matrix.target }} -p effectcraft -p effectcraft-cli - - name: Run them - run: | - target/${{ matrix.target }}/debug/effectcraft.exe --version - target/${{ matrix.target }}/debug/effectcraft-cli.exe --version From 93b9451f3d69f67344d2b1f6777e0a7fa0ee730f Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Thu, 29 Oct 2026 20:57:11 -0400 Subject: [PATCH 19/29] Docs: the Windows section no longer mentions a toolchains workflow (refs #1886) --- CONTRIBUTING.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 600239efa..eff8208c9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -76,8 +76,6 @@ linker = "rust-lld" rustflags = ["-C", "link-self-contained=yes", "-L", "native=C:/gnu-import-libs"] ``` -The `Windows toolchains` workflow builds the app and the CLI each of these three ways every night. - ## Ground rules - **Clean room.** Work from After Effects' public documentation and its observable behaviour only. From 35d21d45bb849e5ff4be1ac623c8bb0e2cd7c8e7 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Thu, 29 Oct 2026 20:57:12 -0400 Subject: [PATCH 20/29] Docs: MSYS2 is plain text in the Windows section (refs #1886) --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index eff8208c9..29521ad73 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -44,7 +44,7 @@ C++ build tools installed, the commands above work as they are. The GNU toolchain (`x86_64-pc-windows-gnu`) needs a current mingw-w64 GCC and binutils first on `PATH`. They provide the linker and the Windows import libraries, such as `libshlwapi.a` for -opening links from the app. With [MSYS2](https://www.msys2.org/): +opening links from the app. With MSYS2: ```powershell C:\msys64\usr\bin\pacman.exe -S --needed mingw-w64-x86_64-gcc From 199af9213c629efbc69c2b73f452519afe5c531f Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Fri, 30 Oct 2026 02:10:36 +0100 Subject: [PATCH 21/29] #1743: use existing CI and consolidate render source documentation --- .../workflows/render-source-protection.yml | 36 ------------------- docs/agents.md | 2 +- docs/file-format-parity.md | 32 +++++++++++++++++ docs/render-source-protection.md | 30 ---------------- 4 files changed, 33 insertions(+), 67 deletions(-) delete mode 100644 .github/workflows/render-source-protection.yml delete mode 100644 docs/render-source-protection.md diff --git a/.github/workflows/render-source-protection.yml b/.github/workflows/render-source-protection.yml deleted file mode 100644 index 4bb44c8bf..000000000 --- a/.github/workflows/render-source-protection.yml +++ /dev/null @@ -1,36 +0,0 @@ -name: Render source protection - -on: - pull_request: - paths: - - .github/workflows/render-source-protection.yml - - Cargo.toml - - Cargo.lock - - crates/engine/** - - crates/project/src/render_queue.rs - workflow_dispatch: - -permissions: - contents: read - -jobs: - regression: - name: Source guard (${{ matrix.os }}) - runs-on: ${{ matrix.os }} - timeout-minutes: 30 - strategy: - fail-fast: false - matrix: - os: [ubuntu-latest, windows-latest] - env: - CARGO_PROFILE_DEV_DEBUG: 0 - CARGO_PROFILE_TEST_DEBUG: 0 - CARGO_INCREMENTAL: 0 - steps: - - uses: actions/checkout@v4 - with: - persist-credentials: false - - uses: dtolnay/rust-toolchain@1.95.0 - - run: cargo test --locked -p effectcraft-engine --lib output_guard::tests - - run: cargo test --locked -p effectcraft-engine --lib tests::render_overwrite - - run: cargo test --locked -p effectcraft-project render_queue::tests diff --git a/docs/agents.md b/docs/agents.md index 1781c7163..9672d9c42 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -5,7 +5,7 @@ panel gestures, the CLI, the control channel and MCP all go through the same com user can do, an agent can do too. There are three ways in: Render-queue outputs cannot replace footage, proxies or the project file. See -[render source protection](render-source-protection.md) for aliases, default names and sequence +[render source protection](file-format-parity.md#render-source-protection) for aliases, default names and sequence limits. This guard does not change ordinary-file overwrite defaults or add another confirmation API. | Interface | Best for | Needs a window | diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 9e2249e9b..19447ae3d 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -105,6 +105,37 @@ single-file video (Animation / PNG codecs), DPX / Targa / Photoshop sequences, M encoders ([hardware-parity.md](hardware-parity.md)), and rendering safely over an existing file (#325). +### Render source protection + +The render queue refuses an output that would replace footage, an attached proxy, or the +project file. It checks every output module and segment before dispatching the item. A refused +item is marked Failed with its reason; other valid queued items can still render. Render logs +are checked too. No render parameter overrides this protection. + +Checks compare normalized paths and native file identities, so an existing symbolic link, +hard link, junction or Windows short name cannot disguise a source as another output file. +Source identities are indexed once per check and matching identities are verified. The index +does not keep file handles open for the lifetime of a long sequence. + +Default output names move aside when they would replace a protected file. This also applies to +Create Proxy: recreating a proxy chooses a new default name and attaches the new proxy, leaving +the previous proxy file intact. An explicit output path is kept as entered and refused if it +would replace a protected file. + +After Pre-render imports its result, that file is footage too. Re-queueing the same item +with the same output is refused; run Pre-render again for a fresh default output, or choose +another Output To. An item's `last_output` is not permission to replace imported footage. + +Image-sequence checks inspect existing frames in the requested range. Skip Existing Files leaves +existing frames alone. If a frame number appears in a folder name, the checker probes each +frame separately and refuses ranges over 100,000 frames instead of silently checking a prefix. +Choose a fixed output folder or a shorter range for those exports. + +This guard does not introduce an overwrite flag, confirmation dialog, or changed replacement +defaults for the CLI, scripts or MCP. Replacement consent and staging ordinary output files +belong to the exporter. The source check runs before export; filesystem changes between that +check and a write still require exporter-side revalidation. + ## Estimate **≈ 130–230 h**: `.aep` / `.aepx` import 80–160 h (after the owner decision), wiring TGA / DPX / @@ -115,5 +146,6 @@ relinking 6–12 h, `.mogrt`-compatible templates 15–30 h (partly parallel wit | Date | Change | Summary | |---|---|---| +| 2026-10-30 | minor | Render-queue source-protection behavior and limits consolidated here from the standalone page | | 2026-10-12 | minor | #324 and #514 marked closed; open `.aep` PRs #251 and #860 noted | | 2026-10-10 | major | First file format parity document, from the After Effects 26.5 bundle and this repository's import/export registries | diff --git a/docs/render-source-protection.md b/docs/render-source-protection.md deleted file mode 100644 index cf15f6797..000000000 --- a/docs/render-source-protection.md +++ /dev/null @@ -1,30 +0,0 @@ -# Render source protection - -The render queue refuses an output that would replace footage, an attached proxy, or the -project file. It checks every output module and segment before dispatching the item. A refused -item is marked Failed with its reason; other valid queued items can still render. Render logs -are checked too. No render parameter overrides this protection. - -Checks compare normalized paths and native file identities, so an existing symbolic link, -hard link, junction or Windows short name cannot disguise a source as another output file. -Source identities are indexed once per check and matching identities are verified. The index -does not keep file handles open for the lifetime of a long sequence. - -Default output names move aside when they would replace a protected file. This also applies to -Create Proxy: recreating a proxy chooses a new default name and attaches the new proxy, leaving -the previous proxy file intact. An explicit output path is kept as entered and refused if it -would replace a protected file. - -After Pre-render imports its result, that file is footage too. Re-queueing the same item -with the same output is refused; run Pre-render again for a fresh default output, or choose -another Output To. An item's `last_output` is not permission to replace imported footage. - -Image-sequence checks inspect existing frames in the requested range. Skip Existing Files leaves -existing frames alone. If a frame number appears in a folder name, the checker probes each -frame separately and refuses ranges over 100,000 frames instead of silently checking a prefix. -Choose a fixed output folder or a shorter range for those exports. - -This guard does not introduce an overwrite flag, confirmation dialog, or changed replacement -defaults for the CLI, scripts or MCP. Replacement consent and staging ordinary output files -belong to the exporter. The source check runs before export; filesystem changes between that -check and a write still require exporter-side revalidation. From 6da89b6ea8e58be2da7f84d97b39f36f91c2a045 Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Fri, 30 Oct 2026 02:17:34 +0100 Subject: [PATCH 22/29] #712: share eyedropper previews and translate the sampling hint --- crates/ui-egui/src/i18n/ui.rs | 8 ++++ .../src/panels/effect_controls/viewer.rs | 40 ++++++++----------- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/crates/ui-egui/src/i18n/ui.rs b/crates/ui-egui/src/i18n/ui.rs index 46493296b..a7a9be62b 100644 --- a/crates/ui-egui/src/i18n/ui.rs +++ b/crates/ui-egui/src/i18n/ui.rs @@ -163,6 +163,7 @@ pub(crate) fn tr_args(text: &str, args: &[&dyn Display]) -> String { /// Japanese: the same rows, in the same order, as the Chinese catalogs below. const JAPANESE: &[(&str, &str)] = &[ + ("Click to sample color", "クリックして色をサンプル"), ("Appearance Settings…", "外観設定…"), ("Add to favorites", "お気に入りに追加"), ("Remove from favorites", "お気に入りから削除"), @@ -904,6 +905,7 @@ const JAPANESE: &[(&str, &str)] = &[ ]; const SIMPLIFIED_CHINESE: &[(&str, &str)] = &[ + ("Click to sample color", "单击以取样颜色"), ("Appearance Settings…", "外观设置…"), ("Add to favorites", "添加到收藏"), ("Remove from favorites", "从收藏中移除"), @@ -1644,6 +1646,7 @@ const SIMPLIFIED_CHINESE: &[(&str, &str)] = &[ ]; const TRADITIONAL_CHINESE: &[(&str, &str)] = &[ + ("Click to sample color", "按一下以取樣顏色"), ("Appearance Settings…", "外觀設定…"), ("Add to favorites", "加入最愛"), ("Remove from favorites", "從最愛移除"), @@ -2332,6 +2335,7 @@ const TRADITIONAL_CHINESE: &[(&str, &str)] = &[ /// Ukrainian (uk): original translations of the same interface rows as the Chinese catalogs. /// Technical names, key names and substituted project content keep their original spelling. const UKRAINIAN: &[(&str, &str)] = &[ + ("Click to sample color", "Клацніть, щоб узяти зразок кольору"), ("Appearance Settings…", "Налаштування вигляду…"), ("Add to favorites", "Додати до вибраного"), ("Remove from favorites", "Видалити з вибраного"), @@ -2906,6 +2910,7 @@ const UKRAINIAN: &[(&str, &str)] = &[ // Brazilian Portuguese. const PORTUGUESE: &[(&str, &str)] = &[ + ("Click to sample color", "Clique para obter uma amostra de cor"), (" OK ", " OK "), (" Delete ", " Excluir "), (" Open Auto-Save ", " Abrir salvamento automático "), @@ -3357,6 +3362,7 @@ const PORTUGUESE: &[(&str, &str)] = &[ /// Czech (cs): original translations covering every row in the Chinese panel catalogs. const CZECH: &[(&str, &str)] = &[ + ("Click to sample color", "Kliknutím naberete barvu"), (" OK ", " OK "), (" Delete ", " Odstranit "), (" Open Auto-Save ", " Otevřít automatickou zálohu "), @@ -3911,6 +3917,7 @@ const CZECH: &[(&str, &str)] = &[ /// order. Terms follow the Spanish menu catalog (composición, material de archivo, cola de procesamiento, /// ajuste preestablecido, método abreviado). Written from the English source. const SPANISH: &[(&str, &str)] = &[ + ("Click to sample color", "Haz clic para tomar una muestra de color"), ("Copy Source Path", "Copiar ruta de origen"), ("Appearance Settings…", "Ajustes de apariencia…"), ("Add to favorites", "Añadir a favoritos"), @@ -4467,6 +4474,7 @@ const SPANISH: &[(&str, &str)] = &[ /// Georgian (ka): every row of the Chinese catalogs, in the same order (the tests keep them in step). const GEORGIAN: &[(&str, &str)] = &[ + ("Click to sample color", "დააწკაპუნეთ ფერის ნიმუშის ასაღებად"), ("Appearance Settings…", "იერსახის პარამეტრები…"), ("Add to favorites", "რჩეულებში დამატება"), ("Remove from favorites", "რჩეულებიდან ამოშლა"), diff --git a/crates/ui-egui/src/panels/effect_controls/viewer.rs b/crates/ui-egui/src/panels/effect_controls/viewer.rs index c7f8981d1..7bf65d40c 100644 --- a/crates/ui-egui/src/panels/effect_controls/viewer.rs +++ b/crates/ui-egui/src/panels/effect_controls/viewer.rs @@ -119,6 +119,21 @@ fn fx_pick_session( ControlFlow::Continue(point_preview) } +/// Shared hint chip and sampled-color swatch for text and popup eyedroppers. +fn color_pick_preview(app: &EffectcraftApp, painter: &egui::Painter, map: &crate::panels::viewer::ViewerMap, hint: &str, pointer: Option) { + let chip = Rect::from_min_size(map.area.min + vec2(10.0, 10.0), vec2(220.0, 22.0)); + painter.rect_filled(chip, 11.0, Color32::from_black_alpha(180)); + painter.text(chip.center(), Align2::CENTER_CENTER, hint, Tokens::ui(11.5), app.tokens.text); + if let Some(pos) = pointer + && let Some(img) = &app.viewer_image + && let Some(sample) = fw::sample_frame(img, crate::panels::viewer_tools::shown_region(painter.ctx(), map.comp), map.to_comp(pos)) + { + let swatch = Rect::from_min_size(pos + vec2(14.0, 14.0), vec2(26.0, 18.0)); + painter.rect_filled(swatch, 2.0, Color32::from_rgb((sample[0] * 255.0) as u8, (sample[1] * 255.0) as u8, (sample[2] * 255.0) as u8)); + painter.rect_stroke(swatch, 2.0, Stroke::new(1.0, Color32::WHITE), egui::StrokeKind::Outside); + } +} + /// An armed text-colour pick: sample the shown frame on click, Esc cancels. fn text_color_pick( app: &mut EffectcraftApp, @@ -128,7 +143,6 @@ fn text_color_pick( ectx: &EvalCtx, actions: &mut Actions, ) { - let t = app.tokens; let ctx = ui.ctx().clone(); let Some(pick) = app.ui.text_color_pick.clone() else { return; @@ -140,20 +154,11 @@ fn text_color_pick( app.viewer_pixels(); let resp = ui.interact(map.area, egui::Id::unique("viewer-text-color-pick"), Sense::click()); app.auto.add("viewer.textColorPick", map.area, &pick.field); - let chip = Rect::from_min_size(map.area.min + vec2(10.0, 10.0), vec2(220.0, 22.0)); - painter.rect_filled(chip, 11.0, Color32::from_black_alpha(180)); - painter.text(chip.center(), Align2::CENTER_CENTER, crate::i18n::tr_args("Click to sample text {}", &[&pick.field]), Tokens::ui(11.5), t.text); + color_pick_preview(app, painter, map, &crate::i18n::tr_args("Click to sample text {}", &[&pick.field]), resp.hover_pos()); if let Some(pos) = resp.hover_pos() { ctx.set_cursor_icon(egui::CursorIcon::Crosshair); let c = map.to_comp(pos); app.pointer_comp = Some([c[0] as f32, c[1] as f32]); - if let Some(img) = &app.viewer_image - && let Some(s) = fw::sample_frame(img, crate::panels::viewer_tools::shown_region(&ctx, map.comp), c) - { - let sw = Rect::from_min_size(pos + vec2(14.0, 14.0), vec2(26.0, 18.0)); - painter.rect_filled(sw, 2.0, Color32::from_rgb((s[0] * 255.0) as u8, (s[1] * 255.0) as u8, (s[2] * 255.0) as u8)); - painter.rect_stroke(sw, 2.0, Stroke::new(1.0, Color32::WHITE), egui::StrokeKind::Outside); - } } if resp.clicked() && let Some(pos) = resp.interact_pointer_pos() @@ -180,7 +185,6 @@ fn text_color_pick( /// the pointer, and the next press on it samples the frame into the picker. The press is read from /// the pointer, since a modal dialog over the viewer takes its clicks. fn popup_eyedropper(app: &mut EffectcraftApp, ui: &mut egui::Ui, painter: &egui::Painter, map: &crate::panels::viewer::ViewerMap) { - let t = app.tokens; let ctx = ui.ctx().clone(); if !crate::color_picker::eyedropper_armed(&ctx) { return; @@ -190,17 +194,7 @@ fn popup_eyedropper(app: &mut EffectcraftApp, ui: &mut egui::Ui, painter: &egui: app.auto.add("viewer.colorPickerEyedropper", map.area, "Sample color"); ctx.set_cursor_icon(egui::CursorIcon::Crosshair); app.viewer_pixels(); - let chip = Rect::from_min_size(map.area.min + vec2(10.0, 10.0), vec2(220.0, 22.0)); - painter.rect_filled(chip, 11.0, Color32::from_black_alpha(180)); - painter.text(chip.center(), Align2::CENTER_CENTER, "Click to sample color", Tokens::ui(11.5), t.text); - if let Some(hp) = crate::color_picker::eyedropper_point(&ctx) - && let Some(img) = &app.viewer_image - && let Some(s) = fw::sample_frame(img, crate::panels::viewer_tools::shown_region(&ctx, map.comp), map.to_comp(hp)) - { - let sw = Rect::from_min_size(hp + vec2(14.0, 14.0), vec2(26.0, 18.0)); - painter.rect_filled(sw, 2.0, Color32::from_rgb((s[0] * 255.0) as u8, (s[1] * 255.0) as u8, (s[2] * 255.0) as u8)); - painter.rect_stroke(sw, 2.0, Stroke::new(1.0, Color32::WHITE), egui::StrokeKind::Outside); - } + color_pick_preview(app, painter, map, crate::i18n::tr("Click to sample color"), crate::color_picker::eyedropper_point(&ctx)); if let Some(pos) = crate::color_picker::sample_press(&ctx) && let Some(img) = &app.viewer_image && let Some(s) = fw::sample_frame(img, crate::panels::viewer_tools::shown_region(&ctx, map.comp), map.to_comp(pos)) From ccae8411b5d6d30410637619ddf03058d527c196 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 01:41:06 +0000 Subject: [PATCH 23/29] i18n(ja): fix one mistranslation and unify terms the menu and the panels spell differently "Warn User When Executing Files" said the cursor, not the user, was warned. The other 27 rows give the same English term one Japanese spelling in the menu table and another in the UI table (track/tracking, Region of Interest, Time Remapping, Separate Dimensions, Stabilize Motion, Hold, Brightness, Vector Layer, Small Screen, Straight, Expression and a few more), or read awkwardly ("({}) Selected Objects", "Interpret Unlabeled Alpha As"). Each now follows the spelling the rest of the file already uses. Only Japanese values change; no key or English text does. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01K5NRZNWfaSLBWVzV8hivyS --- crates/ui-egui/src/i18n.rs | 30 +++++++++++++++--------------- crates/ui-egui/src/i18n/ui.rs | 26 +++++++++++++------------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/crates/ui-egui/src/i18n.rs b/crates/ui-egui/src/i18n.rs index 14afe59cc..7f4a77989 100644 --- a/crates/ui-egui/src/i18n.rs +++ b/crates/ui-egui/src/i18n.rs @@ -1874,7 +1874,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("comp.settings", "Composition Settings...", "コンポジション設定..."), ("comp.setPosterTime", "Set Poster Time", "ポスター時間を設定"), ("comp.trimToWorkArea", "Trim Comp to Work Area", "コンポジションをワークエリアにトリミング"), - ("comp.cropToRegionOfInterest", "Crop Comp to Region of Interest", "コンポジションを目標範囲にクロップ"), + ("comp.cropToRegionOfInterest", "Crop Comp to Region of Interest", "コンポジションを関心領域にクロップ"), ("comp.cropToLayerBounds", "Crop Comp to Selected Layer(s) Bounds", "コンポジションを選択レイヤーの境界にクロップ"), ("render.addOutputModule", "Add Output Module", "出力モジュールを追加"), ("", "Preview", "プレビュー"), @@ -1927,7 +1927,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("layer.mask.reset", "Reset Mask", "マスクをリセット"), ("layer.mask.remove", "Remove Mask", "マスクを削除"), ("layer.mask.removeAll", "Remove All Masks", "すべてのマスクを削除"), - ("track.mask", "Track Mask", "マスクをトラック"), + ("track.mask", "Track Mask", "マスクをトラッキング"), ("", "Mode", "モード"), ("layer.mask.mode", "None", "なし"), ("layer.mask.mode", "Add", "加算"), @@ -1992,8 +1992,8 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("layer.transform", "Fit to Comp Height", "コンポジションの高さに合わせる"), ("layer.autoOrient", "Auto-Orient...", "自動方向..."), ("", "Time", "時間"), - ("layer.enableTimeRemap", "Enable Time Remapping", "タイムリマップ使用可能"), - ("layer.timeReverse", "Time-Reverse Layer", "時間反転レイヤー"), + ("layer.enableTimeRemap", "Enable Time Remapping", "時間リマップを有効化"), + ("layer.timeReverse", "Time-Reverse Layer", "レイヤーを時間反転"), ("layer.timeStretch", "Time Stretch...", "時間伸縮..."), ("layer.freezeFrame", "Freeze Frame", "フレームを固定"), ("layer.freezeOnLastFrame", "Freeze On Last Frame", "最後のフレームで固定"), @@ -2061,7 +2061,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("layer.trackMatte", "Matte with Layer Above", "上のレイヤーをマットにする"), ("layer.trackMatte", "Matte with Layer Below", "下のレイヤーをマットにする"), ("", "Layer Styles", "レイヤースタイル"), - ("layer.style.options", "Layer Style Options...", "レイヤースタイルオプション..."), + ("layer.style.options", "Layer Style Options...", "レイヤースタイルのオプション..."), ("layer.style.convertToEditable", "Convert to Editable Styles", "編集可能なスタイルに変換"), ("layer.style.showAll", "Show All", "すべてを表示"), ("layer.style.removeAll", "Remove All", "すべてを削除"), @@ -2090,7 +2090,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("layer.create", "Convert to Editable Text", "編集可能なテキストに変換"), ("layer.create", "Create Shapes from Text", "テキストからシェイプを作成"), ("layer.create", "Create Masks from Text", "テキストからマスクを作成"), - ("layer.create", "Create Shapes from Vector Layer", "ベクトルレイヤーからシェイプを作成"), + ("layer.create", "Create Shapes from Vector Layer", "ベクターレイヤーからシェイプを作成"), ("layer.create", "Create Keyframes from Data", "データからキーフレームを作成"), ("layer.create", "Null Controllers for Positional Points", "位置のポイント用ヌルコントローラー"), ("layer.create", "Null Controllers for Path Points", "パスのポイント用ヌルコントローラー"), @@ -2140,7 +2140,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("layer.applyTextPreset", "Jitter", "ジッター"), ("layer.applyTextPreset", "Drop In Lines", "行をドロップイン"), ("anim.addKeyframe", "Add Keyframe", "キーフレームを追加"), - ("keys.toggleHold", "Toggle Hold Keyframe", "停止したキーフレームの切り替え"), + ("keys.toggleHold", "Toggle Hold Keyframe", "ホールドキーフレームの切り替え"), ("keys.setMuted", "Mute Keyframes", "キーフレームをミュート"), ("keys.setMuted", "Unmute Keyframes", "キーフレームのミュートを解除"), ("keys.interpolation", "Keyframe Interpolation...", "キーフレーム補間法..."), @@ -2156,7 +2156,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("layer.sequence", "Sequence Layers...", "シーケンスレイヤー..."), ("keys.timeReverse", "Time-Reverse Keyframes", "時間反転キーフレーム"), ("", "Animate Text", "テキストをアニメート"), - ("layer.enablePerChar3D", "Enable Per-character 3D", "文字ごとの3Dを使用可能にする"), + ("layer.enablePerChar3D", "Enable Per-character 3D", "文字ごとの3Dを有効化"), ("layer.addTextAnimator", "Anchor Point", "アンカーポイント"), ("layer.addTextAnimator", "Position", "位置"), ("layer.addTextAnimator", "Scale", "スケール"), @@ -2168,7 +2168,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("layer.addTextAnimator", "RGB", "RGB"), ("layer.addTextAnimator", "Hue", "色相"), ("layer.addTextAnimator", "Saturation", "彩度"), - ("layer.addTextAnimator", "Brightness", "明度"), + ("layer.addTextAnimator", "Brightness", "明るさ"), ("", "Stroke Color", "線のカラー"), ("layer.addTextAnimator", "Stroke Width", "線幅"), ("", "Front", "前面"), @@ -2190,12 +2190,12 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("text.removeAllAnimators", "Remove All Text Animators", "すべてのテキストアニメーターを削除"), ("prop.setExpression", "Add Expression", "エクスプレッションを追加"), ("essential.addProperty", "Add Property to Essential Graphics", "エッセンシャルグラフィックスにプロパティを追加"), - ("prop.separateDimensions", "Separate Dimensions", "次元に分割"), - ("track.camera", "Track Camera", "カメラをトラック"), + ("prop.separateDimensions", "Separate Dimensions", "次元を分離"), + ("track.camera", "Track Camera", "カメラをトラッキング"), ("track.warpStabilizer", "Warp Stabilizer VFX", "ワープスタビライザーVFX"), - ("track.motion", "Track Motion", "モーションをトラック"), - ("track.stabilize", "Stabilize Motion", "モーションをスタビライズ"), - ("track.property", "Track this Property", "このプロパティをトラック"), + ("track.motion", "Track Motion", "モーションをトラッキング"), + ("track.stabilize", "Stabilize Motion", "モーションを安定化"), + ("track.property", "Track this Property", "このプロパティをトラッキング"), ("anim.reveal", "Reveal Properties with Keyframes", "キーフレームのあるプロパティを表示"), ("anim.reveal", "Reveal Properties with Animation", "アニメーションのあるプロパティを表示"), ("anim.reveal", "Reveal All Modified Properties", "変更されたすべてのプロパティを表示"), @@ -2275,7 +2275,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("window.workspace", "Default", "初期設定"), ("window.workspace", "Review", "レビュー"), ("window.workspace", "Learn", "学習"), - ("window.workspace", "Small Screen", "小画面"), + ("window.workspace", "Small Screen", "小さい画面"), ("window.workspace", "Standard", "標準"), ("window.workspace", "All Panels", "すべてのパネル"), ("window.workspace", "Animation", "アニメーション"), diff --git a/crates/ui-egui/src/i18n/ui.rs b/crates/ui-egui/src/i18n/ui.rs index 1da662aad..ab9b0eba2 100644 --- a/crates/ui-egui/src/i18n/ui.rs +++ b/crates/ui-egui/src/i18n/ui.rs @@ -455,7 +455,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("None", "なし"), ("Not applied: tick it in the list to add it.", "未適用：リストでチェックを入れると追加されます。"), ("Not assigned", "未割り当て"), - ("Note: Shadows are only cast from layers with Cast Shadows enabled.", "注意：シャドウは「シャドウを投影」が有効なレイヤーからのみ投影されます。"), + ("Note: Shadows are only cast from layers with Cast Shadows enabled.", "注意：「シャドウを落とす」が有効なレイヤーだけがシャドウを落とします。"), ("Notify", "通知"), ("OK", "OK"), ("Offset:", "オフセット:"), @@ -537,7 +537,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Select a command to change its shortcut.", "ショートカットを変更するコマンドを選択してください。"), ("Select a layer to see its effects", "レイヤーを選択するとエフェクトが表示されます"), ("Select a layer to see its properties", "レイヤーを選択するとプロパティが表示されます"), - ("({}) Selected Objects", "({}) 個の選択されたオブジェクト"), + ("({}) Selected Objects", "選択したオブジェクト（{}）"), ( "Select a layer with a mask or shape path (or the path property), or Puppet pins.", "マスクまたはシェイプパスのあるレイヤー（またはパスプロパティ）か、パペットピンを選択してください。", @@ -685,7 +685,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Import Image Sequence", "画像シーケンスを読み込み"), ("Import PDF / Illustrator File", "PDF / Illustratorファイルを読み込み"), ("Import Photoshop File", "Photoshopファイルを読み込み"), - ("Interpret Unlabeled Alpha As", "ラベルのないアルファを変換"), + ("Interpret Unlabeled Alpha As", "ラベルのないアルファの解釈"), ("Join the ArtCraft Discord", "ArtCraftのDiscordに参加"), ("Mercury GPU Acceleration ({})", "Mercury GPUアクセラレーション（{}）"), ("Mercury Software Only", "Mercuryソフトウェアのみ"), @@ -803,7 +803,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Cycle Mask Colors", "新規マスクに別のカラーを使用"), ("Use Gradients", "グラデーションを使用"), ("Menu Bar", "メニューバー"), - ("Grids & Guides", "グリッド＆ガイド"), + ("Grids & Guides", "グリッドとガイド"), ("Gridline Every", "グリッドの間隔"), ("Subdivisions", "分割数"), ("Style", "スタイル"), @@ -835,7 +835,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Ask User", "ユーザーに確認"), ("Guess", "推測"), ("Ignore", "無視"), - ("Straight", "そのまま"), + ("Straight", "ストレート"), ("Premultiplied", "プリマルチプライ"), ("Default Drag Import As", "複数アイテムのドラッグ読み込みの初期設定"), ("Footage", "フッテージ"), @@ -885,7 +885,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Scripting & Expressions", "スクリプトとエクスプレッション"), ("Application Scripting", "アプリケーションのスクリプト"), ("Allow Scripts to Write Files and Access Network", "スクリプトによるファイルへの書き込みとネットワークへのアクセスを許可"), - ("Warn User When Executing Files", "ファイルの実行時にカーソルに警告"), + ("Warn User When Executing Files", "ファイルの実行時にユーザーに警告"), ("Enable JavaScript Debugger", "JavaScript デバッガーを使用"), ("Expressions Editor", "エクスプレッションエディター"), ("Font Size", "フォントサイズ"), @@ -894,7 +894,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Auto-complete", "オートコンプリート"), ("Bracket Matching", "対応する括弧をハイライト"), ("Word Wrap", "折り返し"), - ("Show Expression Error Banner", "式のエラーがある場合に警告バナーを表示"), + ("Show Expression Error Banner", "エクスプレッションのエラーがある場合に警告バナーを表示"), ( "Expressions use the JavaScript engine. Script errors report their file and line in the Script Console; there is no step debugger.", "エクスプレッションはJavaScriptエンジンを使用します。スクリプトのエラーはファイルと行をスクリプトコンソールに報告します。ステップ実行デバッガーはありません。", @@ -1003,8 +1003,8 @@ const JAPANESE: &[(&str, &str)] = &[ ("Create Shapes from Vector Layer", "ベクターレイヤーからシェイプを作成"), ("Create Shapes from Text", "テキストからシェイプを作成"), ("Create Masks from Text", "テキストからマスクを作成"), - ("New Mask", "新しいマスク"), - ("Center In View", "ビュー内で中央揃え"), + ("New Mask", "新規マスク"), + ("Center In View", "ビューの中央に配置"), ("Fit to Comp", "コンポジションに合わせる"), ("Flip Horizontal", "水平方向に反転"), ("Flip Vertical", "垂直方向に反転"), @@ -1033,8 +1033,8 @@ const JAPANESE: &[(&str, &str)] = &[ ("All Transform Properties", "すべてのトランスフォームプロパティ"), ("Line Anchor", "行のアンカー"), ("Line Spacing", "行間"), - ("Character Offset", "文字オフセット"), - ("Character Value", "文字値"), + ("Character Offset", "文字のオフセット"), + ("Character Value", "文字の値"), ("Fill Color: RGB", "塗りの色: RGB"), ("Fill Color: Hue", "塗りの色: 色相"), ("Fill Color: Saturation", "塗りの色: 彩度"), @@ -1060,7 +1060,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("ScriptUI Panel", "ScriptUI パネル"), ("Lumetri Scopes", "Lumetri スコープ"), ("Media Browser", "メディアブラウザー"), - ("Content-Aware Fill", "コンテンツに応じた塗り"), + ("Content-Aware Fill", "コンテンツに応じた塗りつぶし"), ("Create Nulls From Paths", "パスからヌルを作成"), ("VR Comp Editor", "VR コンポジションエディター"), ("Media Duration", "メディアの長さ"), @@ -1095,7 +1095,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Background Color", "背景色"), ("Basic", "基本"), ("Shutter Angle", "シャッター角度"), - ("Shutter Phase", "シャッター位相"), + ("Shutter Phase", "シャッターフェーズ"), ("Samples Per Frame", "フレームあたりのサンプル数"), ("Adaptive Sample Limit", "適応サンプル数の上限"), ("X and Y", "X と Y"), From 6ed8f7a6d8c6d3365e74f15311989588bbc985ec Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Thu, 29 Oct 2026 21:44:30 -0400 Subject: [PATCH 24/29] deps: bump tar, zip, roxmltree, rustls-webpki and symphonia to their latest versions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `roxmltree`: 0.21.1 → 0.21.2 - `zip`: 9.1.0 → 9.1.3 Lockfile only. `symphonia` deliberately stays 0.5.5: effectcraft names it only to enable AIFF in filmcraft-codecs' symphonia, which is 0.5.5 at the pinned filmcraft rev. Moving to 0.6 waits for that pin to move. --- Cargo.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a48f2d44b..6befc768e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4243,7 +4243,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]] @@ -4835,9 +4835,9 @@ dependencies = [ [[package]] name = "roxmltree" -version = "0.21.1" +version = "0.21.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f1964b10c76125c36f8afe190065a4bf9a87bf324842c05701330bba9f1cacbb" +checksum = "9da7f59328b09405b181fee1018925fad50f157afb43c0c0f5c522c256dd5907" dependencies = [ "memchr", ] @@ -7420,9 +7420,9 @@ dependencies = [ [[package]] name = "zip" -version = "9.1.0" +version = "9.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2886b00003a72c4ccee3f42cfc34908d33b2508a74d827dc2a4244706475c704" +checksum = "1156b39d142a34b999c2310a50455e0cb3760bf7efad0122f796d813bee26ccb" dependencies = [ "crc32fast", "flate2", From 8a317537fa5332ba2c10f69e422f2dcd04ba1fc6 Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Fri, 30 Oct 2026 02:44:56 +0100 Subject: [PATCH 25/29] #1805: keep marker names and duration handles visible with shared GPU test gating --- crates/ui-egui/src/panels/markers_ui.rs | 57 +++++++++++++++---- crates/ui-egui/tests/it/gpu_test.rs | 43 ++++++++++++++ crates/ui-egui/tests/it/main.rs | 1 + .../ui-egui/tests/it/render_queue_snapshot.rs | 5 +- crates/ui-egui/tests/it/ui_3d.rs | 10 ++-- .../tests/it/ui_blank_context_menus.rs | 3 +- crates/ui-egui/tests/it/ui_camera_tracker.rs | 6 +- .../tests/it/ui_character_eyedropper.rs | 5 +- crates/ui-egui/tests/it/ui_dock_resize.rs | 4 +- crates/ui-egui/tests/it/ui_env_light_gpu.rs | 1 + crates/ui-egui/tests/it/ui_font_search.rs | 6 +- crates/ui-egui/tests/it/ui_gpu.rs | 1 + crates/ui-egui/tests/it/ui_guide_layers.rs | 3 +- .../tests/it/ui_layer_source_picker.rs | 4 +- crates/ui-egui/tests/it/ui_marker_names.rs | 33 +++++++++-- .../tests/it/ui_media_browser_cache.rs | 3 +- .../tests/it/ui_media_browser_copy_path.rs | 3 +- crates/ui-egui/tests/it/ui_menus.rs | 3 +- crates/ui-egui/tests/it/ui_panels.rs | 3 +- crates/ui-egui/tests/it/ui_presets_folder.rs | 5 +- crates/ui-egui/tests/it/ui_script_console.rs | 3 +- crates/ui-egui/tests/it/ui_scriptui.rs | 4 +- crates/ui-egui/tests/it/ui_tabs.rs | 3 +- crates/ui-egui/tests/it/ui_theme_favorites.rs | 3 +- crates/ui-egui/tests/it/ui_viewer.rs | 11 ++-- crates/ui-egui/tests/it/ui_viewers.rs | 3 +- crates/ui-egui/tests/it/ui_visual_review.rs | 9 +-- crates/ui-egui/tests/it/ui_web_gpu_storage.rs | 1 + crates/ui-egui/tests/it/ui_web_job_gpu.rs | 8 +-- .../ui-egui/tests/it/ui_workflow_selection.rs | 5 +- crates/ui-egui/tests/it/viewer_geometry.rs | 5 +- 31 files changed, 194 insertions(+), 60 deletions(-) create mode 100644 crates/ui-egui/tests/it/gpu_test.rs diff --git a/crates/ui-egui/src/panels/markers_ui.rs b/crates/ui-egui/src/panels/markers_ui.rs index cc08f223d..15bfa0f72 100644 --- a/crates/ui-egui/src/panels/markers_ui.rs +++ b/crates/ui-egui/src/panels/markers_ui.rs @@ -296,21 +296,13 @@ fn marker(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, comp: hx += 5.0; } } - p.line_segment([pos2(xe, y0), pos2(xe, y0 + h)], Stroke::new(1.0, col)); } - p.add(egui::Shape::convex_polygon(marker_shape(x, y0, h), col, Stroke::NONE)); if !m.comment.is_empty() { // The name sits in a small outlined box beside the flag, as in After Effects. It is drawn in the theme's field // colours, so it follows Dark, Darker and Light. let galley = p.layout_no_wrap(m.comment.lines().next().unwrap_or_default().to_string(), Tokens::ui(10.0), t.text); - let (box_w, mut box_x) = (galley.size().x + 8.0, x + 6.0); - // A duration that ends under the box would be hidden by its fill: the box steps past the end cap instead. - if let Some(xe) = end - && xe > box_x - && xe < box_x + box_w - { - box_x = xe + 3.0; - } + let box_w = galley.size().x + 8.0; + let box_x = name_box_x(x, box_w, end, p.clip_rect()); let box_rect = Rect::from_min_size(pos2(box_x, y0), vec2(box_w, h.max(galley.size().y + 2.0))); p.rect(box_rect, t.radius_sm, t.field_bg, Stroke::new(1.0, t.field_border), egui::StrokeKind::Inside); p.galley(pos2(box_rect.min.x + 4.0, box_rect.center().y - galley.size().y / 2.0), galley, t.text); @@ -385,6 +377,35 @@ fn marker(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, comp: }); } +fn name_box_x(x: f32, width: f32, end: Option, clip: Rect) -> f32 { + let start = x + 6.0; + if let Some(end) = end + && end > start + && end < start + width + { + let right = end + 3.0; + if right + width <= clip.right() { + return right; + } + let left = x - 6.0 - width; + if left >= clip.left() { + return left; + } + } + start +} + +/// Flags and duration handles sit above every name box in their marker strip. +fn marker_glyphs(p: &egui::Painter, tokens: &Tokens, tm: TMap, marker: &Marker, time: f64, y0: f32, y1: f32) { + let color = marker_color(tokens, marker.label); + let h = (y1 - y0).min(10.0); + p.add(egui::Shape::convex_polygon(marker_shape(tm.x(time), y0, h), color, Stroke::NONE)); + if marker.duration > Tick(0) { + let end = tm.x(time + marker.duration.seconds()); + p.line_segment([pos2(end, y0), pos2(end, y0 + h)], Stroke::new(1.0, color)); + } +} + /// Composition markers in the ruler strip `[y0, y1]`, protected regions over `rows`. pub(crate) fn comp_markers(app: &mut EffectcraftApp, ui: &mut egui::Ui, comp: &Comp, tm: TMap, strip: Rect, rows: Rect) { let p = ui.painter().with_clip_rect(strip); @@ -398,6 +419,9 @@ pub(crate) fn comp_markers(app: &mut EffectcraftApp, ui: &mut egui::Ui, comp: &C for (i, m) in comp.markers.iter().enumerate() { marker(app, ui, &p, comp, tm, m, m.time.seconds(), MarkerRef { layer: None, index: i }, strip.min.y, strip.max.y); } + for m in &comp.markers { + marker_glyphs(&p, &app.tokens, tm, m, m.time.seconds(), strip.min.y, strip.max.y); + } } /// The composition marker bin at the right end of the time ruler (`bin`), as in After Effects @@ -439,6 +463,9 @@ pub(crate) fn layer_markers(app: &mut EffectcraftApp, ui: &mut egui::Ui, clip: R for (i, m) in layer.markers.iter().enumerate() { marker(app, ui, &p, comp, tm, m, layer.comp_time(m.time).seconds(), MarkerRef { layer: Some(layer.id.0), index: i }, r.min.y + 3.0, r.max.y - 3.0); } + for m in &layer.markers { + marker_glyphs(&p, &app.tokens, tm, m, layer.comp_time(m.time).seconds(), r.min.y + 3.0, r.max.y - 3.0); + } } /// The nested comp's markers on a precomp layer's bar (`markers.nested`). @@ -488,6 +515,16 @@ fn nested_markers(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter mod tests { use super::*; + #[test] + fn duration_name_box_stays_visible_near_the_right_clip_edge() { + let clip = Rect::from_min_max(pos2(0.0, 0.0), pos2(100.0, 20.0)); + let x = name_box_x(70.0, 20.0, Some(85.0), clip); + assert!(x >= clip.left() && x + 20.0 <= clip.right()); + assert!(x + 20.0 < 70.0 - 4.0, "the left-hand box must not cover its own marker flag"); + assert!(85.0 < x || 85.0 > x + 20.0, "the end cap must remain outside the box"); + assert_eq!(name_box_x(20.0, 20.0, Some(35.0), clip), 38.0); + } + #[test] fn marker_dialog_reads_and_writes_every_field() { let mut s = effectcraft_engine::Session::default(); diff --git a/crates/ui-egui/tests/it/gpu_test.rs b/crates/ui-egui/tests/it/gpu_test.rs new file mode 100644 index 000000000..0156fbb4d --- /dev/null +++ b/crates/ui-egui/tests/it/gpu_test.rs @@ -0,0 +1,43 @@ +//! Shared device-creation gate for the integration-test executable. + +static GPU_TEST_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +thread_local! { + static HELD: std::cell::Cell = const { std::cell::Cell::new(false) }; +} + +pub(crate) struct Guard { + held: Option>, +} + +impl Drop for Guard { + fn drop(&mut self) { + if self.held.is_some() { + HELD.with(|held| held.set(false)); + } + } +} + +/// GPU-backed harnesses hold this for their whole test; screenshots may enter it again. +pub(crate) fn lock() -> Guard { + let held = if HELD.with(std::cell::Cell::get) { + None + } else { + let guard = GPU_TEST_LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + HELD.with(|held| held.set(true)); + Some(guard) + }; + Guard { held } +} + +/// Plain harnesses create their GPU renderer lazily on the first screenshot. +pub(crate) trait RenderSerial { + fn render_serialized(&mut self) -> Result; +} + +impl RenderSerial for egui_kittest::Harness<'_, State> { + fn render_serialized(&mut self) -> Result { + let _device = lock(); + self.render().map_err(|error| format!("{error:?}")) + } +} diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index 7d38e3089..17fe06cae 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -1,5 +1,6 @@ //! The crate's integration tests, one binary (each module is a former `tests/.rs`). +mod gpu_test; mod native_menu_text_keys; mod render_queue_snapshot; mod shared_docking_ui; diff --git a/crates/ui-egui/tests/it/render_queue_snapshot.rs b/crates/ui-egui/tests/it/render_queue_snapshot.rs index d054511af..27054119d 100644 --- a/crates/ui-egui/tests/it/render_queue_snapshot.rs +++ b/crates/ui-egui/tests/it/render_queue_snapshot.rs @@ -2,6 +2,7 @@ //! adapter); run with //! `RQ_SNAPSHOT=/abs/out.png cargo test -p effectcraft-ui-egui --test it render_queue_snapshot:: -- --ignored`. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::project::render_queue::RenderStatus; use effectcraft_ui_egui::EffectcraftApp; @@ -34,7 +35,7 @@ fn render_queue_panel_snapshot() { h.ctx.data_mut(|d| d.insert_temp(egui::Id::unique("rq-ui"), (ids, None::))); h.state_mut().show_panel(effectcraft_ui_egui::dock::PanelKind::RenderQueue); h.run_steps(4); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); let out = std::env::var("RQ_SNAPSHOT").unwrap_or_else(|_| concat!(env!("CARGO_MANIFEST_DIR"), "/../../target/test-out/render_queue.png").into()); if let Some(d) = std::path::Path::new(&out).parent() { std::fs::create_dir_all(d).unwrap(); @@ -57,7 +58,7 @@ fn templates_dialog_snapshot() { let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_| app.take().expect("app")); h.run_steps(4); assert!(h.state().dialog.is_some(), "the Templates dialog is open"); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); let out = std::env::var("RQ_TEMPLATES_SNAPSHOT").unwrap_or_else(|_| concat!(env!("CARGO_MANIFEST_DIR"), "/../../target/test-out/rq_templates.png").into()); if let Some(d) = std::path::Path::new(&out).parent() { std::fs::create_dir_all(d).unwrap(); diff --git a/crates/ui-egui/tests/it/ui_3d.rs b/crates/ui-egui/tests/it/ui_3d.rs index a654f745c..0f7a10b7e 100644 --- a/crates/ui-egui/tests/it/ui_3d.rs +++ b/crates/ui-egui/tests/it/ui_3d.rs @@ -2,6 +2,7 @@ //! with wgpu and writes PNGs when `EC_SNAPSHOT_DIR` is set (`cargo test -p effectcraft-ui-egui //! --test it ui_3d:: -- --ignored`); the other test only runs the UI logic. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::render::three_d::View3D; use effectcraft_ui_egui::EffectcraftApp; @@ -109,6 +110,7 @@ fn camera_and_light_dialogs_create_layers() { #[test] #[ignore = "renders with wgpu; run with --ignored (set EC_SNAPSHOT_DIR to keep PNGs)"] fn snapshot() { + let _device = crate::gpu_test::lock(); let dir = std::env::var("EC_SNAPSHOT_DIR").ok(); let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).with_pixels_per_point(1.0).wgpu().build_eframe(|_| app()); let shots: [(&str, Option<&str>); 4] = @@ -118,7 +120,7 @@ fn snapshot() { h.state_mut().session.execute(c, json!({})).unwrap(); } settle(&mut h); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); if let Some(d) = &dir { img.save(format!("{d}/ui3d_{name}.png")).unwrap(); } @@ -126,14 +128,14 @@ fn snapshot() { let ctx = h.ctx.clone(); effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "layer.newCamera", json!({})).unwrap(); h.run_steps(3); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); if let Some(d) = &dir { img.save(format!("{d}/ui3d_camera_dialog.png")).unwrap(); } h.state_mut().dialog = None; effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "layer.newLight", json!({})).unwrap(); h.run_steps(3); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); if let Some(d) = &dir { img.save(format!("{d}/ui3d_light_dialog.png")).unwrap(); } @@ -143,7 +145,7 @@ fn snapshot() { h.state_mut().session.execute("layer.select", json!({"layers": ["Card Blue"]})).unwrap(); effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "timeline.reveal.rotation", json!({})).unwrap(); settle(&mut h); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); if let Some(d) = &dir { img.save(format!("{d}/ui3d_reveal_rotation.png")).unwrap(); } diff --git a/crates/ui-egui/tests/it/ui_blank_context_menus.rs b/crates/ui-egui/tests/it/ui_blank_context_menus.rs index 52417326f..84080be08 100644 --- a/crates/ui-egui/tests/it/ui_blank_context_menus.rs +++ b/crates/ui-egui/tests/it/ui_blank_context_menus.rs @@ -1,4 +1,5 @@ //! Right-click actions in the blank Project and Timeline areas, and on Timeline property names. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::project::ItemKind; use effectcraft_ui_egui::dock::WorkspaceExt as _; @@ -20,7 +21,7 @@ fn rect(h: &Harness<'_, EffectcraftApp>, id: &str) -> Rect { } fn snapshot(h: &mut Harness<'_, EffectcraftApp>, name: &str) { if let Ok(dir) = std::env::var("BLANK_CONTEXT_MENU_SNAPSHOTS") { - h.render().unwrap().save(format!("{dir}/{name}.png")).unwrap(); + h.render_serialized().unwrap().save(format!("{dir}/{name}.png")).unwrap(); } } fn right_click(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) { diff --git a/crates/ui-egui/tests/it/ui_camera_tracker.rs b/crates/ui-egui/tests/it/ui_camera_tracker.rs index f7d349342..1f42879a9 100644 --- a/crates/ui-egui/tests/it/ui_camera_tracker.rs +++ b/crates/ui-egui/tests/it/ui_camera_tracker.rs @@ -3,6 +3,7 @@ //! The solve is a synthetic one written into the effect (the analysis itself is tested in the //! engine and track crates). +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::effects::camera_tracker as ct; use effectcraft_engine::track::camtrack::{CameraSolve, ShotType, SolveMethod, SolvedFrame, SolvedPoint}; @@ -217,6 +218,7 @@ fn settle(h: &mut Harness<'_, EffectcraftApp>) { #[test] #[ignore = "analyses the demo's 3D Showcase and renders with wgpu; run with --ignored (set EC_SNAPSHOT_DIR to keep PNGs)"] fn snapshot_showcase_track() { + let _device = crate::gpu_test::lock(); let dir = std::env::var("EC_SNAPSHOT_DIR").ok(); let mut s = Session::default(); s.execute("file.openDemoProject", json!({})).unwrap(); @@ -235,7 +237,7 @@ fn snapshot_showcase_track() { h.input_mut().events.push(egui::Event::PointerMoved(r.center() + egui::vec2(12.0, 12.0))); h.run_steps(2); if let Some(d) = &dir { - h.render().expect("render").save(format!("{d}/camera_tracker_hover.png")).unwrap(); + h.render_serialized().expect("render").save(format!("{d}/camera_tracker_hover.png")).unwrap(); } let sel: Vec = pts.iter().take(5).filter_map(|i| i.rsplit('.').next()?.parse().ok()).collect(); h.state_mut().session.execute("camera.selectPoints", json!({"points": sel})).unwrap(); @@ -243,6 +245,6 @@ fn snapshot_showcase_track() { h.state_mut().session.execute("layer.select", json!({"layers": ["#3"]})).unwrap(); settle(&mut h); if let Some(d) = &dir { - h.render().expect("render").save(format!("{d}/camera_tracker_solid.png")).unwrap(); + h.render_serialized().expect("render").save(format!("{d}/camera_tracker_solid.png")).unwrap(); } } diff --git a/crates/ui-egui/tests/it/ui_character_eyedropper.rs b/crates/ui-egui/tests/it/ui_character_eyedropper.rs index ea2578a0d..ea4eecd99 100644 --- a/crates/ui-egui/tests/it/ui_character_eyedropper.rs +++ b/crates/ui-egui/tests/it/ui_character_eyedropper.rs @@ -1,4 +1,5 @@ //! Character Fill and Stroke can sample a displayed Composition pixel. +use crate::gpu_test::RenderSerial; use effectcraft_engine::{Session, commands::text_edit::layer_doc, project::LayerId}; use effectcraft_ui_egui::{ EffectcraftApp, @@ -37,6 +38,7 @@ fn settle(h: &mut Harness<'_, EffectcraftApp>) { #[test] fn character_fill_eyedropper_samples_viewer_and_keeps_text_selection() { + let _device = crate::gpu_test::lock(); let mut s = Session::default(); s.execute("comp.new", json!({"width": 320, "height": 180, "duration": 2})).unwrap(); s.execute("layer.newSolid", json!({"name": "Background", "color": "#20c040"})).unwrap(); @@ -64,6 +66,7 @@ fn character_fill_eyedropper_samples_viewer_and_keeps_text_selection() { #[test] fn character_stroke_eyedropper_samples_viewer() { + let _device = crate::gpu_test::lock(); let mut s = Session::default(); s.execute("comp.new", json!({"width": 320, "height": 180, "duration": 2})).unwrap(); s.execute("layer.newSolid", json!({"name": "brat green", "color": "#8ACE00"})).unwrap(); @@ -79,7 +82,7 @@ fn character_stroke_eyedropper_samples_viewer() { settle(&mut h); click_auto(&mut h, "character.stroke.eyedropper"); if let Ok(dir) = std::env::var("EC_SNAPSHOT_DIR") { - h.render().unwrap().save(format!("{dir}/character-eyedropper.png")).unwrap(); + h.render_serialized().unwrap().save(format!("{dir}/character-eyedropper.png")).unwrap(); } let sample = comp_to_screen(&h.ctx, [20.0, 20.0]).unwrap(); click(&mut h, sample); diff --git a/crates/ui-egui/tests/it/ui_dock_resize.rs b/crates/ui-egui/tests/it/ui_dock_resize.rs index 863fe697e..11ca740cb 100644 --- a/crates/ui-egui/tests/it/ui_dock_resize.rs +++ b/crates/ui-egui/tests/it/ui_dock_resize.rs @@ -1,5 +1,6 @@ //! Shared pane resizing in the full application, including application shortcut ownership. +use crate::gpu_test::RenderSerial; use craft_ui::docking::{Divider, DockArea, Hit}; use effectcraft_engine::Session; use effectcraft_ui_egui::EffectcraftApp; @@ -16,7 +17,7 @@ fn snapshots(h: &mut Harness<'_, EffectcraftApp>, directory: Option<&std::path:: let ctx = h.ctx.clone(); h.state_mut().set_theme(&ctx, theme); h.run_steps(3); - h.render().unwrap().save(directory.join(format!("effectcraft-{phase}-{name}.png"))).unwrap(); + h.render_serialized().unwrap().save(directory.join(format!("effectcraft-{phase}-{name}.png"))).unwrap(); } let ctx = h.ctx.clone(); h.state_mut().set_theme(&ctx, ThemeKind::Dark); @@ -32,6 +33,7 @@ fn first_size(h: &Harness<'_, EffectcraftApp>) -> f32 { #[test] fn focused_dock_resize_owns_arrows_and_keeps_pointer_dragging() { + let _device = crate::gpu_test::lock(); let mut session = Session::default(); session.execute("comp.new", json!({"name": "Split test", "width": 160, "height": 90, "duration": 1})).unwrap(); session.execute("layer.newSolid", json!({"name": "Selected", "color": [1.0, 0.0, 0.0, 1.0]})).unwrap(); diff --git a/crates/ui-egui/tests/it/ui_env_light_gpu.rs b/crates/ui-egui/tests/it/ui_env_light_gpu.rs index 171cc5728..71e4fae67 100644 --- a/crates/ui-egui/tests/it/ui_env_light_gpu.rs +++ b/crates/ui-egui/tests/it/ui_env_light_gpu.rs @@ -42,6 +42,7 @@ fn same_adapter(name: String) -> eframe::egui_wgpu::WgpuSetup { /// viewer's GPU frames change when the light is added. #[test] fn environment_light_lights_the_scene_on_the_gpu() { + let _device = crate::gpu_test::lock(); let Some(probe) = effectcraft_gpu::Gpu::headless() else { eprintln!("no GPU adapter: skipping"); return; diff --git a/crates/ui-egui/tests/it/ui_font_search.rs b/crates/ui-egui/tests/it/ui_font_search.rs index 77984cf6b..44b650be4 100644 --- a/crates/ui-egui/tests/it/ui_font_search.rs +++ b/crates/ui-egui/tests/it/ui_font_search.rs @@ -1,4 +1,5 @@ //! Real keyboard/pointer coverage for the font menus in both text panels. +use crate::gpu_test::RenderSerial; use effectcraft_engine::{Session, commands::text_edit::layer_doc, project::LayerId}; use effectcraft_ui_egui::{EffectcraftApp, dock::PanelKind}; use egui::{Event, Key, Modifiers, PointerButton}; @@ -29,6 +30,7 @@ fn type_text(h: &mut Harness<'_, EffectcraftApp>, text: &str) { #[test] fn font_search_filters_selects_and_undoes_in_both_panels() { + let _device = crate::gpu_test::lock(); for (panel, prefix) in [(PanelKind::Character, "character.font"), (PanelKind::Properties, "properties.text.font")] { let mut s = Session::default(); s.execute("comp.new", json!({"width": 320, "height": 180})).unwrap(); @@ -56,7 +58,7 @@ fn font_search_filters_selects_and_undoes_in_both_panels() { assert!(!rows.is_empty()); assert!(rows.iter().all(|r| r.label.to_lowercase().contains("jetbrains"))); if let Ok(dir) = std::env::var("EC_SNAPSHOT_DIR") { - h.render().unwrap().save(format!("{dir}/{prefix}-search.png")).unwrap(); + h.render_serialized().unwrap().save(format!("{dir}/{prefix}-search.png")).unwrap(); } assert_eq!(layer_doc(&h.state().session, LayerId(lid)).unwrap().font, "Inter", "typing only filters"); key(&mut h, Key::Enter); @@ -76,7 +78,7 @@ fn font_search_filters_selects_and_undoes_in_both_panels() { // Reopening clears the previous query, and mouse selection still works. h.run_steps(2); if let Ok(dir) = std::env::var("EC_SNAPSHOT_DIR") { - h.render().unwrap().save(format!("{dir}/{prefix}-reopened.png")).unwrap(); + h.render_serialized().unwrap().save(format!("{dir}/{prefix}-reopened.png")).unwrap(); } assert!(h.state().auto.query(&format!("{prefix}.option.")).len() > 1); type_text(&mut h, "JetBrains"); diff --git a/crates/ui-egui/tests/it/ui_gpu.rs b/crates/ui-egui/tests/it/ui_gpu.rs index 355c35bf5..c8cdc428a 100644 --- a/crates/ui-egui/tests/it/ui_gpu.rs +++ b/crates/ui-egui/tests/it/ui_gpu.rs @@ -41,6 +41,7 @@ fn same_adapter(name: String) -> eframe::egui_wgpu::WgpuSetup { #[test] fn viewer_draws_gpu_frames_that_match_the_cpu() { + let _device = crate::gpu_test::lock(); let Some(probe) = effectcraft_gpu::Gpu::headless() else { eprintln!("no GPU adapter: skipping"); return; diff --git a/crates/ui-egui/tests/it/ui_guide_layers.rs b/crates/ui-egui/tests/it/ui_guide_layers.rs index 37158658f..92196f5f1 100644 --- a/crates/ui-egui/tests/it/ui_guide_layers.rs +++ b/crates/ui-egui/tests/it/ui_guide_layers.rs @@ -1,6 +1,7 @@ //! Guide Layer in the layer context menus (Timeline rows and the Composition viewer), a plain item //! (no check mark) that toggles the switch on the selection, and the guide mark by a guide layer's //! name. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::project::LayerId; use effectcraft_ui_egui::EffectcraftApp; @@ -45,7 +46,7 @@ fn guide(h: &Harness<'_, EffectcraftApp>, id: LayerId) -> bool { fn snapshot(h: &mut Harness<'_, EffectcraftApp>, name: &str) { if let Ok(dir) = std::env::var("GUIDE_LAYER_SNAPSHOTS") { - h.render().unwrap().save(format!("{dir}/{name}.png")).unwrap(); + h.render_serialized().unwrap().save(format!("{dir}/{name}.png")).unwrap(); } } diff --git a/crates/ui-egui/tests/it/ui_layer_source_picker.rs b/crates/ui-egui/tests/it/ui_layer_source_picker.rs index 78a0f8821..ca10e4249 100644 --- a/crates/ui-egui/tests/it/ui_layer_source_picker.rs +++ b/crates/ui-egui/tests/it/ui_layer_source_picker.rs @@ -3,6 +3,7 @@ //! Effect Controls' layer parameters (a Displacement Map's layer) pick through the same widget, //! and both write an undoable `prop.set`. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::keyframe::Value; use effectcraft_engine::project::LayerId; @@ -183,6 +184,7 @@ fn effect_controls_layer_param_picks_through_the_shared_widget() { #[test] #[ignore = "renders with wgpu; run with --ignored (set EC_SNAPSHOT_DIR to keep PNGs)"] fn snapshot_source_dropdown_open_and_chosen() { + let _device = crate::gpu_test::lock(); let mut s = Session::default(); s.execute("comp.new", json!({"name": "Main", "width": 1280, "height": 720, "frameRate": 30, "duration": 5})).unwrap(); s.execute("comp.settings", json!({"renderer": "advanced3D"})).unwrap(); @@ -200,7 +202,7 @@ fn snapshot_source_dropdown_open_and_chosen() { h.run_steps(4); let save = |h: &mut Harness<'_, EffectcraftApp>, name: &str| { if let Ok(dir) = std::env::var("EC_SNAPSHOT_DIR") { - h.render().expect("render").save(format!("{dir}/{name}")).expect("save"); + h.render_serialized().expect("render").save(format!("{dir}/{name}")).expect("save"); } }; save(&mut h, "source_unset.png"); diff --git a/crates/ui-egui/tests/it/ui_marker_names.rs b/crates/ui-egui/tests/it/ui_marker_names.rs index 99adf41f3..190bd77bb 100644 --- a/crates/ui-egui/tests/it/ui_marker_names.rs +++ b/crates/ui-egui/tests/it/ui_marker_names.rs @@ -8,16 +8,13 @@ use effectcraft_ui_egui::theme::ThemeKind; use egui_kittest::Harness; use serde_json::json; -/// One render at a time: two tests creating a GPU device together crash some Vulkan loaders. -static RENDER: std::sync::Mutex<()> = std::sync::Mutex::new(()); - fn close(got: [u8; 4], want: egui::Color32) -> bool { got[..3].iter().zip(want.to_array()).all(|(a, b)| a.abs_diff(b) <= 3) } #[test] fn the_name_box_follows_the_theme() { - let _one_at_a_time = RENDER.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let _one_at_a_time = crate::gpu_test::lock(); let mut s = Session::default(); s.execute("comp.new", json!({"name": "Song", "width": 640, "height": 360, "duration": 4})).unwrap(); s.execute("layer.newSolid", json!({"name": "Bed", "color": "#204060", "width": 640, "height": 360})).unwrap(); @@ -58,7 +55,7 @@ fn the_name_box_follows_the_theme() { /// stays visible: the cap is drawn, the bar shows left of it, and the box starts after it. #[test] fn a_short_duration_stays_visible_beside_the_name_box() { - let _one_at_a_time = RENDER.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let _one_at_a_time = crate::gpu_test::lock(); let mut s = Session::default(); s.execute("comp.new", json!({"name": "Song", "width": 640, "height": 360, "duration": 4})).unwrap(); s.execute("layer.newSolid", json!({"name": "Bed", "color": "#204060", "width": 640, "height": 360})).unwrap(); @@ -95,3 +92,29 @@ fn a_short_duration_stays_visible_beside_the_name_box() { let fill = at(xe + 3.0 + 2.5, mid); assert!(close(fill, tokens.field_bg), "the box begins after the end cap: {fill:?}, field_bg {:?}", tokens.field_bg); } + +#[test] +fn a_later_name_box_does_not_hide_an_earlier_duration_cap() { + let _one_at_a_time = crate::gpu_test::lock(); + let mut s = Session::default(); + s.execute("comp.new", json!({"width": 640, "height": 360, "duration": 4})).unwrap(); + s.execute("markers.set", json!({"new": true, "time": 1.0, "duration": 0.5, "label": "Red"})).unwrap(); + s.execute("markers.set", json!({"new": true, "time": 1.3, "comment": "This name overlaps the earlier endpoint", "label": "Blue"})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(move |_| EffectcraftApp::new(s)); + h.run_steps(5); + let a = h.state().auto.find("timeline.marker.0").unwrap().rect; + let b = h.state().auto.find("timeline.marker.1").unwrap().rect; + let x = |r: [f32; 4]| r[0] + r[2] / 2.0; + let end = x(a) + (x(b) - x(a)) * 0.5 / 0.3; + let y = a[1] + 1.0 + (a[3] - 1.0).min(10.0) / 2.0; + let image = h.render().unwrap(); + if let Some(dir) = std::env::var_os("MARKER_NAMES_SNAPSHOT") { + image.save(std::path::Path::new(&dir).join("marker-overlapping-cap.png")).unwrap(); + } + let scale = image.width() as f32 / 1600.0; + let cap = (-1..=1) + .map(|offset| image.get_pixel(((end + offset as f32) * scale) as u32, (y * scale) as u32).0) + .max_by_key(|pixel| i32::from(pixel[0]) - i32::from(pixel[1])) + .unwrap(); + assert!(i32::from(cap[0]) - i32::from(cap[1]) > 40, "the earlier red duration cap stays above the later name: {cap:?}"); +} diff --git a/crates/ui-egui/tests/it/ui_media_browser_cache.rs b/crates/ui-egui/tests/it/ui_media_browser_cache.rs index ddfc53e22..44ca9dd81 100644 --- a/crates/ui-egui/tests/it/ui_media_browser_cache.rs +++ b/crates/ui-egui/tests/it/ui_media_browser_cache.rs @@ -1,3 +1,4 @@ +use crate::gpu_test::RenderSerial; use std::sync::{ Arc, Mutex, atomic::{AtomicBool, Ordering}, @@ -140,6 +141,6 @@ fn browser_cache_refreshes_filters_navigates_and_recovers_from_errors() { assert_eq!(names(&listing(&h, false).1), ["Sub", "Added.csv", "Clip.csv", "From action.csv"]); assert!(h.state().auto.find("mediaBrowser.entry.Clip.csv").is_some()); if let Ok(path) = std::env::var("MEDIA_BROWSER_SNAPSHOT") { - h.render().unwrap().save(path).unwrap(); + h.render_serialized().unwrap().save(path).unwrap(); } } diff --git a/crates/ui-egui/tests/it/ui_media_browser_copy_path.rs b/crates/ui-egui/tests/it/ui_media_browser_copy_path.rs index dff60ddd0..8372b2be5 100644 --- a/crates/ui-egui/tests/it/ui_media_browser_copy_path.rs +++ b/crates/ui-egui/tests/it/ui_media_browser_copy_path.rs @@ -1,3 +1,4 @@ +use crate::gpu_test::RenderSerial; use std::sync::Arc; use effectcraft_engine::{ @@ -74,7 +75,7 @@ fn file_folder_and_virtual_paths_copy_from_the_context_menu_without_document_cha if name == "作品 clip.csv" && let Ok(file) = std::env::var("MEDIA_BROWSER_COPY_SNAPSHOT") { - h.render().unwrap().save(file).unwrap(); + h.render_serialized().unwrap().save(file).unwrap(); } assert_eq!(click(&mut h, &id, PointerButton::Primary), [path]); h.run_steps(3); diff --git a/crates/ui-egui/tests/it/ui_menus.rs b/crates/ui-egui/tests/it/ui_menus.rs index 424b75c48..ce0cbab1a 100644 --- a/crates/ui-egui/tests/it/ui_menus.rs +++ b/crates/ui-egui/tests/it/ui_menus.rs @@ -1,6 +1,7 @@ //! The menu bar from the keyboard, and the app's shortcuts waiting while a menu is open (#279) //! (egui_kittest, UI logic only). +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_ui_egui::EffectcraftApp; use egui::{Event, Key, Modifiers, Pos2, Rect, pos2, vec2}; @@ -226,7 +227,7 @@ fn relink_is_reachable_through_the_dependencies_menu() { h.run_steps(8); let relink = h.query_by_label_contains("Relink Missing Footage").unwrap().rect(); if let Some(path) = std::env::var_os("EFFECTCRAFT_RELINK_MENU_SNAPSHOT") { - h.render().unwrap().save(path).unwrap(); + h.render_serialized().unwrap().save(path).unwrap(); } click_at(&mut h, relink.center()); assert!(picked.get()); diff --git a/crates/ui-egui/tests/it/ui_panels.rs b/crates/ui-egui/tests/it/ui_panels.rs index db9ef7373..a29d3e358 100644 --- a/crates/ui-egui/tests/it/ui_panels.rs +++ b/crates/ui-egui/tests/it/ui_panels.rs @@ -2,6 +2,7 @@ //! registers its automation ids; the Footage panel's buttons edit into the comp; the Progress //! panel lists and cancels a job; Lumetri Scopes switch scope through their dropdown. +use crate::gpu_test::RenderSerial; use effectcraft_ui_egui::dock::WorkspaceExt as _; use std::sync::Arc; @@ -448,7 +449,7 @@ fn panels_snapshot() { } open(&mut h, panel); h.run_steps(3); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); img.save(format!("{dir}/{panel}{scope}.png")).unwrap(); } } diff --git a/crates/ui-egui/tests/it/ui_presets_folder.rs b/crates/ui-egui/tests/it/ui_presets_folder.rs index e11efe944..1bcd9189c 100644 --- a/crates/ui-egui/tests/it/ui_presets_folder.rs +++ b/crates/ui-egui/tests/it/ui_presets_folder.rs @@ -1,5 +1,6 @@ //! Effects & Presets: copy the configured folder path through the contents menu. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_ui_egui::panels::effects_presets::PresetFile; use effectcraft_ui_egui::{ControlRequest, EffectcraftApp, control, dock::PanelKind}; @@ -52,7 +53,7 @@ fn presets_menu_copies_the_configured_folder_without_editing_the_project() { click(&mut h, "effects.menu"); h.run_steps(3); if let Ok(path) = std::env::var("PRESETS_FOLDER_SNAPSHOT") { - h.render().unwrap().save(path).unwrap(); + h.render_serialized().unwrap().save(path).unwrap(); } assert_eq!(click(&mut h, "effects.copyPresetsFolderPath"), [expected.as_str()]); h.run_steps(2); @@ -156,7 +157,7 @@ fn shared_preset_rows_search_apply_and_undo_the_selected_definition() { assert_eq!(h.state().session.active_comp().unwrap().layers[0].props.prop("transform/opacity").unwrap().value.as_f64(), expected); assert_eq!(h.state().session.history.undo.len(), history + 1); if let Ok(dir) = std::env::var("PRESET_CACHE_SNAPSHOTS") { - h.render().unwrap().save(format!("{dir}/{name}.png")).unwrap(); + h.render_serialized().unwrap().save(format!("{dir}/{name}.png")).unwrap(); } h.state_mut().session.execute("edit.undo", json!({})).unwrap(); h.run_steps(2); diff --git a/crates/ui-egui/tests/it/ui_script_console.rs b/crates/ui-egui/tests/it/ui_script_console.rs index e705a20b7..9430bc7a2 100644 --- a/crates/ui-egui/tests/it/ui_script_console.rs +++ b/crates/ui-egui/tests/it/ui_script_console.rs @@ -1,6 +1,7 @@ //! Window ▸ Script Console: the panel shows, runs JavaScript through `script.run` (persistent //! context) and logs output, results and errors (egui_kittest, UI logic only). +use crate::gpu_test::RenderSerial; use effectcraft_ui_egui::EffectcraftApp; use effectcraft_ui_egui::dock::PanelKind; use egui_kittest::Harness; @@ -59,7 +60,7 @@ fn script_console_snapshot() { } h.state_mut().ui.script_console.input = "app.project.numItems".into(); h.run_steps(3); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); let out = std::env::var("SC_SNAPSHOT").unwrap_or_else(|_| concat!(env!("CARGO_MANIFEST_DIR"), "/../../target/test-out/script_console.png").into()); if let Some(d) = std::path::Path::new(&out).parent() { std::fs::create_dir_all(d).unwrap(); diff --git a/crates/ui-egui/tests/it/ui_scriptui.rs b/crates/ui-egui/tests/it/ui_scriptui.rs index 09986f6d1..6199462d0 100644 --- a/crates/ui-egui/tests/it/ui_scriptui.rs +++ b/crates/ui-egui/tests/it/ui_scriptui.rs @@ -2,6 +2,7 @@ //! windows (clicks reach the script's handlers), the branching History panel, and the Window / //! File ▸ Scripts menus listing scripts (egui_kittest, UI logic only). +use crate::gpu_test::RenderSerial; use effectcraft_ui_egui::EffectcraftApp; use effectcraft_ui_egui::dock::PanelKind; use effectcraft_ui_egui::dock::WorkspaceExt as _; @@ -210,6 +211,7 @@ fn history_panel_jumps_between_branches() { /// onDraw fills of concave paths and real images (drawImage, image controls), rendered. #[test] fn on_draw_concave_fills_and_images_render() { + let _device = crate::gpu_test::lock(); let dir = std::env::temp_dir().join(format!("effectcraft-scriptui-img-{}", std::process::id())); std::fs::create_dir_all(&dir).unwrap(); let png = dir.join("green.png"); @@ -244,7 +246,7 @@ fn on_draw_concave_fills_and_images_render() { let g = &win.root.children[0]; assert!(matches!(&g.draw[1], effectcraft_engine::scriptui::DrawOp::Image { image: Some(i), .. } if i.src.as_deref() == Some(png.as_str()))); assert_eq!(win.root.children[1].image.as_ref().and_then(|i| i.src.clone()).as_deref(), Some(png.as_str())); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); if let Ok(d) = std::env::var("EC_SNAPSHOT_DIR") { img.save(format!("{d}/scriptui_paint.png")).unwrap(); } diff --git a/crates/ui-egui/tests/it/ui_tabs.rs b/crates/ui-egui/tests/it/ui_tabs.rs index 5fc3d8598..cc9e56041 100644 --- a/crates/ui-egui/tests/it/ui_tabs.rs +++ b/crates/ui-egui/tests/it/ui_tabs.rs @@ -2,6 +2,7 @@ //! as its own Timeline tab (not a rename, and not in place of the comp shown), the tabs switch and //! close comps, and a dragged tab lands where its insertion mark shows. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::project::ItemId; use effectcraft_ui_egui::EffectcraftApp; @@ -135,7 +136,7 @@ fn tab_drag_snapshot() { } let dir = concat!(env!("CARGO_MANIFEST_DIR"), "/../../target/test-out"); std::fs::create_dir_all(dir).unwrap(); - h.render().expect("render").save(format!("{dir}/tab-drag.png")).unwrap(); + h.render_serialized().expect("render").save(format!("{dir}/tab-drag.png")).unwrap(); } /// A precomp layer's bar shows its comp's markers; a double-click opens that comp at the marker. diff --git a/crates/ui-egui/tests/it/ui_theme_favorites.rs b/crates/ui-egui/tests/it/ui_theme_favorites.rs index 1b700f51f..424b28c49 100644 --- a/crates/ui-egui/tests/it/ui_theme_favorites.rs +++ b/crates/ui-egui/tests/it/ui_theme_favorites.rs @@ -1,3 +1,4 @@ +use crate::gpu_test::RenderSerial; use effectcraft_engine::{Session, config::MemoryConfig, prefs::Prefs}; use effectcraft_ui_egui::{EffectcraftApp, menus}; use egui::{Event, Modifiers, PointerButton, Pos2, pos2, vec2}; @@ -49,7 +50,7 @@ fn star_controls_save_and_favorite_menu_selects_the_family() { assert_eq!(h.state().session.prefs.appearance.favorite_themes, vec!["light"]); if let Some(path) = std::env::var_os("FAVORITES_SCREENSHOT_DIR") { std::fs::create_dir_all(&path).unwrap(); - h.render().unwrap().save(std::path::Path::new(&path).join("effectcraft-favorites.png")).unwrap(); + h.render_serialized().unwrap().save(std::path::Path::new(&path).join("effectcraft-favorites.png")).unwrap(); } assert_eq!(h.state().session.prefs.appearance.appearance_mode, "dark"); let p = at(&h, "settings.ok"); diff --git a/crates/ui-egui/tests/it/ui_viewer.rs b/crates/ui-egui/tests/it/ui_viewer.rs index 41cf5fe43..48bc36a44 100644 --- a/crates/ui-egui/tests/it/ui_viewer.rs +++ b/crates/ui-egui/tests/it/ui_viewer.rs @@ -2,6 +2,7 @@ //! interest drawing, rulers and guides, snapping, the shape Pen and motion-path key drags //! (egui_kittest, UI logic only). +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::commands::shape_tool::PaintKind; use effectcraft_engine::project::LayerId; @@ -448,7 +449,7 @@ fn mid_drag(h: &mut Harness<'_, EffectcraftApp>, from: Pos2, to: Pos2, mods: egu h.input_mut().events.push(Event::PointerMoved(from + (to - from) * (i as f32 / 8.0))); h.step(); } - h.render().expect("the mid-drag frame") + h.render_serialized().expect("the mid-drag frame") } /// Release the button where the drag left it. @@ -490,7 +491,7 @@ fn shape_drag_ghost_shows_the_tools_own_shape_while_dragging() { h.state_mut().ui.tool = tool; settle_viewer(&mut h); let (a, b) = (screen(&h, from), screen(&h, to)); - let before = h.render().expect("the frame before the drag"); + let before = h.render_serialized().expect("the frame before the drag"); let mid = mid_drag(&mut h, a, b, egui::Modifiers::default()); let fill = fill_rgb(&h); @@ -526,7 +527,7 @@ fn shape_drag_ghost_normalises_the_same_way_as_the_release() { h.state_mut().ui.tool = Tool::Ellipse; settle_viewer(&mut h); let (a, b) = (screen(&h, from.map(|v| v as f32)), screen(&h, to.map(|v| v as f32))); - let before = h.render().expect("the frame before the drag"); + let before = h.render_serialized().expect("the frame before the drag"); // Measured with the button down, so this is the ghost and not the committed shape. let mid = mid_drag(&mut h, a, b, mods); let on = [centre[0] as f32 + size[0] as f32 / 4.0, centre[1] as f32 + size[1] as f32 / 4.0]; @@ -595,7 +596,7 @@ fn a_layer_drag_moves_the_picture_while_the_button_is_down() { h.input_mut().events.push(Event::PointerButton { pos: from, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() }); h.step(); // Where the layer is not (yet): the plate behind it. - let plate = px_at(&h.render().unwrap(), &h, [440.0, 200.0]); + let plate = px_at(&h.render_serialized().unwrap(), &h, [440.0, 200.0]); // Drag in small steps, still holding the button, and give the renders a chance to land. let mut moved = false; @@ -606,7 +607,7 @@ fn a_layer_drag_moves_the_picture_while_the_button_is_down() { std::thread::sleep(std::time::Duration::from_millis(15)); // 30 comp px behind the pointer: inside the layer, well away from its outline, its anchor // icon and where it started. - let at = px_at(&h.render().expect("mid-drag frame"), &h, [320.0 + 200.0 * (i as f32 / 40.0) - 30.0, 200.0]); + let at = px_at(&h.render_serialized().expect("mid-drag frame"), &h, [320.0 + 200.0 * (i as f32 / 40.0) - 30.0, 200.0]); if i >= 30 && delta(at, plate) > 30 { moved = true; break; diff --git a/crates/ui-egui/tests/it/ui_viewers.rs b/crates/ui-egui/tests/it/ui_viewers.rs index 833ab0a60..bfbcad51e 100644 --- a/crates/ui-egui/tests/it/ui_viewers.rs +++ b/crates/ui-egui/tests/it/ui_viewers.rs @@ -2,6 +2,7 @@ //! comp goes to an unlocked viewer (a new one when every viewer is locked); a click on another //! viewer makes it the active one; closing a viewer forgets it. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::project::ItemId; use effectcraft_ui_egui::EffectcraftApp; @@ -171,5 +172,5 @@ fn viewers_snapshot() { h.run_steps(4); let dir = concat!(env!("CARGO_MANIFEST_DIR"), "/../../target/test-out"); std::fs::create_dir_all(dir).unwrap(); - h.render().unwrap().save(format!("{dir}/viewers.png")).unwrap(); + h.render_serialized().unwrap().save(format!("{dir}/viewers.png")).unwrap(); } diff --git a/crates/ui-egui/tests/it/ui_visual_review.rs b/crates/ui-egui/tests/it/ui_visual_review.rs index 70cc5ed84..6d079f1f5 100644 --- a/crates/ui-egui/tests/it/ui_visual_review.rs +++ b/crates/ui-egui/tests/it/ui_visual_review.rs @@ -1,5 +1,6 @@ //! Opt-in visual review of every built-in panel, workspaces and common dialogs. //! Original fixture; captures are review outputs, not shipped assets. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_ui_egui::{Dialog, EffectcraftApp, dock::PanelKind}; use egui_kittest::Harness; @@ -19,18 +20,18 @@ fn all_ui_review_captures() { std::sync::Arc::make_mut(&mut s.project).settings.gpu_acceleration = false; let mut h = Harness::builder().with_size(size).build_eframe(|_| EffectcraftApp::new(s)); h.run_steps(4); - h.render().unwrap().save(out.join(format!("{label}-workspace.png"))).unwrap(); + h.render_serialized().unwrap().save(out.join(format!("{label}-workspace.png"))).unwrap(); let ctx = h.ctx.clone(); h.state_mut().set_theme(&ctx, effectcraft_ui_egui::theme::ThemeKind::Light); h.run_steps(4); - h.render().unwrap().save(out.join(format!("{label}-workspace-light.png"))).unwrap(); + h.render_serialized().unwrap().save(out.join(format!("{label}-workspace-light.png"))).unwrap(); h.state_mut().set_theme(&ctx, effectcraft_ui_egui::theme::ThemeKind::Dark); for p in PanelKind::ALL { h.state_mut().show_panel(p); h.state_mut().ui.maximized = Some(p); h.run_steps(4); assert!(h.state().auto.find(&format!("panel.{}", p.id())).is_some(), "{} opens", p.title()); - h.render().unwrap().save(out.join(format!("{label}-{}.png", p.id()))).unwrap(); + h.render_serialized().unwrap().save(out.join(format!("{label}-{}.png", p.id()))).unwrap(); } h.state_mut().ui.maximized = None; for (name, dialog) in [ @@ -42,7 +43,7 @@ fn all_ui_review_captures() { ] { h.state_mut().dialog = Some(dialog); h.run_steps(4); - h.render().unwrap().save(out.join(format!("{label}-dialog-{name}.png"))).unwrap(); + h.render_serialized().unwrap().save(out.join(format!("{label}-dialog-{name}.png"))).unwrap(); h.state_mut().dialog = None; h.run_steps(2); } diff --git a/crates/ui-egui/tests/it/ui_web_gpu_storage.rs b/crates/ui-egui/tests/it/ui_web_gpu_storage.rs index a742226de..024bd1cd0 100644 --- a/crates/ui-egui/tests/it/ui_web_gpu_storage.rs +++ b/crates/ui-egui/tests/it/ui_web_gpu_storage.rs @@ -122,6 +122,7 @@ fn cpu_pixels(f: &FrameImage) -> Vec { #[test] fn gpu_worker_renders_effect_frames_and_the_disk_cache_survives_a_reload() { + let _device = crate::gpu_test::lock(); let (Some(page_gpu), Some(worker_gpu)) = (Gpu::headless(), Gpu::headless_deferred()) else { eprintln!("no GPU adapter: skipped"); return; diff --git a/crates/ui-egui/tests/it/ui_web_job_gpu.rs b/crates/ui-egui/tests/it/ui_web_job_gpu.rs index 9060e1067..8b8914abc 100644 --- a/crates/ui-egui/tests/it/ui_web_job_gpu.rs +++ b/crates/ui-egui/tests/it/ui_web_job_gpu.rs @@ -10,10 +10,6 @@ use effectcraft_engine::offload::{Inbox, Offload, Post, WorkerReply, WorkerReque use effectcraft_gpu::Gpu; use serde_json::json; -/// One device at a time in this test binary: wgpu's OpenGL backend (Mesa llvmpipe on FreeBSD, -/// Linux without Vulkan) panics or crashes when tests create and drive devices concurrently. -static GPU_LOCK: Mutex<()> = Mutex::new(()); - /// Holds offloaded jobs until the test runs them. #[derive(Default)] struct Manual(Mutex)>>); @@ -63,7 +59,7 @@ fn page() -> (Session, Arc) { #[test] fn render_queue_jobs_render_on_the_worker_gpu() { - let _gpu = GPU_LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let _gpu = crate::gpu_test::lock(); let Some(g) = Gpu::headless_deferred() else { eprintln!("no GPU adapter: skipped"); return; @@ -102,7 +98,7 @@ fn render_queue_jobs_render_on_the_worker_gpu() { #[test] fn analyses_render_their_input_on_the_worker_gpu() { - let _gpu = GPU_LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let _gpu = crate::gpu_test::lock(); let Some(g) = Gpu::headless_deferred() else { eprintln!("no GPU adapter: skipped"); return; diff --git a/crates/ui-egui/tests/it/ui_workflow_selection.rs b/crates/ui-egui/tests/it/ui_workflow_selection.rs index 121c2b888..e7bcedb60 100644 --- a/crates/ui-egui/tests/it/ui_workflow_selection.rs +++ b/crates/ui-egui/tests/it/ui_workflow_selection.rs @@ -1,4 +1,5 @@ //! User-reported selection, Project deletion, and render-queue drop regressions. +use crate::gpu_test::RenderSerial; use effectcraft_engine::{Session, project::LayerId}; use effectcraft_ui_egui::{Dialog, EffectcraftApp, dock::PanelKind}; use egui::{Event, Modifiers, PointerButton, Pos2, Rect, pos2, vec2}; @@ -273,7 +274,7 @@ fn viewer_right_click_targets_hit_and_duplicate_preserves_group() { } assert!(h.state().frames.is_cached(&key)); h.run_steps(3); - h.render().expect("offscreen context menu render").save(out).unwrap(); + h.render_serialized().expect("offscreen context menu render").save(out).unwrap(); } h.get_by_label("Duplicate").click(); h.run_steps(3); @@ -433,7 +434,7 @@ fn workflow_snapshot() { } assert!(h.state().frames.is_cached(&key), "current composition rendered before capture"); h.run_steps(3); - h.render().expect("offscreen UI render").save(out).unwrap(); + h.render_serialized().expect("offscreen UI render").save(out).unwrap(); } /// Filled rects painted this frame (flattening nested shape lists). diff --git a/crates/ui-egui/tests/it/viewer_geometry.rs b/crates/ui-egui/tests/it/viewer_geometry.rs index e3e4128e8..d57a8aa9e 100644 --- a/crates/ui-egui/tests/it/viewer_geometry.rs +++ b/crates/ui-egui/tests/it/viewer_geometry.rs @@ -1,5 +1,6 @@ //! Inspect the actual viewer's painted colors and pointer-driven control geometry. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_ui_egui::{ EffectcraftApp, @@ -269,10 +270,10 @@ fn viewer_theme_and_width_visual_matrix() { } fn save_stable_snapshot(h: &mut Harness<'_, EffectcraftApp>, path: &str, width: f32) { - let image = h.render().unwrap(); + let image = h.render_serialized().unwrap(); image.save(path).unwrap(); h.run_steps(3); - let repeated = h.render().unwrap(); + let repeated = h.render_serialized().unwrap(); assert!(image == repeated, "settled synthetic viewer pixels must repeat exactly: {path}"); if width == 900.0 { let times: Vec<_> = h From 528a9ff7430743a7e2999bad05b7832ba2678edf Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Fri, 30 Oct 2026 02:58:56 +0100 Subject: [PATCH 26/29] #1805: keep ordinary marker names inside the viewport too --- crates/ui-egui/src/panels/markers_ui.rs | 27 ++++++++++++++----------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/crates/ui-egui/src/panels/markers_ui.rs b/crates/ui-egui/src/panels/markers_ui.rs index 15bfa0f72..03c45d32b 100644 --- a/crates/ui-egui/src/panels/markers_ui.rs +++ b/crates/ui-egui/src/panels/markers_ui.rs @@ -379,18 +379,16 @@ fn marker(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, comp: fn name_box_x(x: f32, width: f32, end: Option, clip: Rect) -> f32 { let start = x + 6.0; - if let Some(end) = end - && end > start - && end < start + width - { - let right = end + 3.0; - if right + width <= clip.right() { - return right; - } - let left = x - 6.0 - width; - if left >= clip.left() { - return left; - } + let right = match end { + Some(end) if end > start && end < start + width => end + 3.0, + _ => start, + }; + if right + width <= clip.right() { + return right; + } + let left = x - 6.0 - width; + if left >= clip.left() { + return left; } start } @@ -523,6 +521,11 @@ mod tests { assert!(x + 20.0 < 70.0 - 4.0, "the left-hand box must not cover its own marker flag"); assert!(85.0 < x || 85.0 > x + 20.0, "the end cap must remain outside the box"); assert_eq!(name_box_x(20.0, 20.0, Some(35.0), clip), 38.0); + for end in [None, Some(72.0), Some(110.0)] { + let x = name_box_x(90.0, 20.0, end, clip); + assert!(x >= clip.left() && x + 20.0 <= clip.right(), "all right-edge names must remain visible: {end:?}"); + assert!(x + 20.0 < 90.0 - 4.0); + } } #[test] From f3d46b0fa3a1ce9a871802ee1a2d96c83979e498 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Thu, 29 Oct 2026 22:21:27 -0400 Subject: [PATCH 27/29] deps: bump tar, zip, roxmltree, rustls-webpki and symphonia to their latest versions (#2633) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `roxmltree`: 0.21.1 → 0.21.2 - `zip`: 9.1.0 → 9.1.3 Lockfile only. `symphonia` deliberately stays 0.5.5: effectcraft names it only to enable AIFF in filmcraft-codecs' symphonia, which is 0.5.5 at the pinned filmcraft rev. Moving to 0.6 waits for that pin to move. --- Cargo.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 629fc65e9..5f4551c19 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4292,7 +4292,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]] @@ -4884,9 +4884,9 @@ dependencies = [ [[package]] name = "roxmltree" -version = "0.21.1" +version = "0.21.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f1964b10c76125c36f8afe190065a4bf9a87bf324842c05701330bba9f1cacbb" +checksum = "9da7f59328b09405b181fee1018925fad50f157afb43c0c0f5c522c256dd5907" dependencies = [ "memchr", ] @@ -7469,9 +7469,9 @@ dependencies = [ [[package]] name = "zip" -version = "9.1.0" +version = "9.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2886b00003a72c4ccee3f42cfc34908d33b2508a74d827dc2a4244706475c704" +checksum = "1156b39d142a34b999c2310a50455e0cb3760bf7efad0122f796d813bee26ccb" dependencies = [ "crc32fast", "flate2", From 725750eabb1e3d2a19162fa2de61465ad08af9b8 Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Fri, 30 Oct 2026 03:23:57 +0100 Subject: [PATCH 28/29] #1328: sample the displayed frame and guard modal ownership --- crates/ui-egui/src/app/frame.rs | 2 + crates/ui-egui/src/color_picker.rs | 116 +++++++++++++++++- crates/ui-egui/src/frames.rs | 16 ++- crates/ui-egui/src/frames/tests.rs | 6 + .../src/panels/effect_controls/viewer.rs | 9 +- crates/ui-egui/src/panels/fx_widgets.rs | 13 +- crates/ui-egui/src/panels/viewer_tools.rs | 80 ++++++++++++ 7 files changed, 223 insertions(+), 19 deletions(-) diff --git a/crates/ui-egui/src/app/frame.rs b/crates/ui-egui/src/app/frame.rs index f519bcdde..01245fde3 100644 --- a/crates/ui-egui/src/app/frame.rs +++ b/crates/ui-egui/src/app/frame.rs @@ -372,6 +372,7 @@ impl EffectcraftApp { // Video Preview is a second viewport: only a real pass can update it. prefs_live::frame(self, &ctx); self.auto.begin_frame(); + panels::viewer_tools::clear_sample_source(&ctx); if let Some(rect) = crate::color_picker::take_automation(&ctx) { self.auto.add("colorPicker.eyedropper", rect, "Eyedropper"); } @@ -398,6 +399,7 @@ impl EffectcraftApp { let header = egui::Rect::from_min_size(egui::pos2(full.min.x, top), egui::vec2(full.width(), header_h)); header::show(self, ui, header); let body = egui::Rect::from_min_max(egui::pos2(full.min.x + 4.0, header.max.y + 2.0), egui::pos2(full.max.x - 4.0, full.max.y - 4.0)); + crate::color_picker::note_dialog(&ctx, self.dialog); self.dock_area(ui, body); #[cfg(not(target_arch = "wasm32"))] prefs_live::dropped_files(self, ui, full); diff --git a/crates/ui-egui/src/color_picker.rs b/crates/ui-egui/src/color_picker.rs index b4fd70544..c75cc0242 100644 --- a/crates/ui-egui/src/color_picker.rs +++ b/crates/ui-egui/src/color_picker.rs @@ -122,12 +122,27 @@ struct Armed { popup: egui::Id, pass: u64, sample: Option<[f32; 3]>, + dialog: Option, } fn armed_key() -> egui::Id { egui::Id::unique("color-picker-eyedropper") } +fn dialog_key() -> egui::Id { + egui::Id::unique("color-picker-dialog") +} + +/// Record the modal before drawing the viewer. A picker may sample behind its own dialog, but +/// a new, unrelated modal must block it from its very first pass, before its rect is known. +pub(crate) fn note_dialog(ctx: &egui::Context, dialog: Option) { + ctx.data_mut(|data| data.insert_temp(dialog_key(), dialog)); +} + +fn current_dialog(ctx: &egui::Context) -> Option { + ctx.data(|data| data.get_temp::>(dialog_key())).flatten() +} + /// The armed eyedropper, if its picker was drawn in this pass or the last: one whose picker stops /// being drawn (its dialog gone) lapses. fn armed_record(ctx: &egui::Context) -> Option { @@ -147,7 +162,8 @@ pub fn eyedropper_armed(ctx: &egui::Context) -> bool { fn arm(ctx: &egui::Context, popup: egui::Id) { let pass = ctx.cumulative_pass_nr(); - ctx.data_mut(|d| d.insert_temp(armed_key(), Armed { popup, pass, sample: None })); + let dialog = current_dialog(ctx); + ctx.data_mut(|d| d.insert_temp(armed_key(), Armed { popup, pass, sample: None, dialog })); } fn disarm(ctx: &egui::Context, popup: egui::Id) { @@ -218,6 +234,9 @@ fn overlaid(ctx: &egui::Context, pos: egui::Pos2) -> bool { /// dialog. The pointer is read rather than the viewer's responses, since a modal dialog over the /// viewer takes its clicks. pub fn eyedropper_point(ctx: &egui::Context) -> Option { + if armed_record(ctx).is_some_and(|armed| armed.dialog != current_dialog(ctx)) { + return None; + } let viewer: Option<(u64, Rect)> = ctx.data(|d| d.get_temp(viewer_key())); let viewer = viewer.filter(|(p, _)| p.saturating_add(1) >= ctx.cumulative_pass_nr())?.1; let pos = ctx.input(|i| i.pointer.interact_pos())?; @@ -646,6 +665,101 @@ pub fn hsv_to_rgb([h, s, v]: [f32; 3]) -> [f32; 3] { mod tests { use super::*; + fn sampling_viewer() -> egui_kittest::Harness<'static, crate::EffectcraftApp> { + let mut session = effectcraft_engine::Session::default(); + session.execute("comp.new", serde_json::json!({"width": 320, "height": 180, "duration": 2})).unwrap(); + session.execute("layer.newSolid", serde_json::json!({"color": "#204060"})).unwrap(); + let mut harness = egui_kittest::Harness::builder().with_size(vec2(1200.0, 800.0)).build_eframe(|_| crate::EffectcraftApp::new(session)); + settle_sampling_viewer(&mut harness); + harness + } + + fn settle_sampling_viewer(harness: &mut egui_kittest::Harness<'_, crate::EffectcraftApp>) { + for _ in 0..200 { + harness.step(); + if harness.state().frames.inflight() == 0 && harness.state().viewer_image.is_some() { + harness.run_steps(3); + return; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + panic!("the synthetic viewer frame did not arrive"); + } + + fn press_sampling_viewer(harness: &mut egui_kittest::Harness<'_, crate::EffectcraftApp>) -> Option<[f32; 3]> { + let ctx = harness.ctx.clone(); + arm(&ctx, egui::Id::unique("test-popup")); + let point = crate::panels::viewer::comp_to_screen(&ctx, [20.0, 20.0]).unwrap(); + harness.event(egui::Event::PointerMoved(point)); + harness.event(egui::Event::PointerButton { pos: point, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE }); + harness.step(); + let sample = ctx.data(|data| data.get_temp::(armed_key())).and_then(|armed| armed.sample); + harness.event(egui::Event::PointerButton { pos: point, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE }); + harness.step(); + sample + } + + #[test] + fn popup_eyedropper_samples_the_displayed_channel_and_exposure() { + let mut harness = sampling_viewer(); + harness.state_mut().session.state.viewer.channel = effectcraft_engine::viewer::Channel::Green; + let green = press_sampling_viewer(&mut harness).unwrap(); + assert!(green.iter().all(|value| (*value - 64.0 / 255.0).abs() < 0.01), "{green:?}"); + let options = &mut harness.state_mut().session.state.viewer; + options.channel = effectcraft_engine::viewer::Channel::Rgb; + options.exposure = 1.0; + let exposed = press_sampling_viewer(&mut harness).unwrap(); + let mut expected = [[32, 64, 96, 255]]; + effectcraft_engine::viewer::display_transform(&mut expected, effectcraft_engine::viewer::Channel::Rgb, false, 1.0); + for (actual, expected) in exposed.iter().zip(expected[0]) { + assert!((*actual - expected as f32 / 255.0).abs() < 0.01, "{exposed:?}"); + } + } + + #[test] + fn popup_eyedropper_samples_snapshot_and_refuses_wireframe() { + let mut harness = sampling_viewer(); + harness.state_mut().session.execute("view.takeSnapshot", serde_json::json!({})).unwrap(); + harness.state_mut().session.execute("layer.newSolid", serde_json::json!({"color": "#a0b0c0"})).unwrap(); + settle_sampling_viewer(&mut harness); + harness.event(egui::Event::Key { key: egui::Key::F5, physical_key: None, pressed: true, repeat: false, modifiers: egui::Modifiers::NONE }); + // Let the snapshot texture's discard passes finish before arming this synthetic picker. + harness.step(); + let snapshot = press_sampling_viewer(&mut harness).unwrap(); + for (actual, expected) in snapshot.iter().zip([32, 64, 96]) { + assert!((*actual - expected as f32 / 255.0).abs() < 0.01, "{snapshot:?}"); + } + let options = &mut harness.state_mut().session.state.viewer; + options.show_snapshot = false; + options.fast_previews = effectcraft_engine::commands::viewer_cmds::FastPreviews::Wireframe; + harness.event(egui::Event::Key { key: egui::Key::F5, physical_key: None, pressed: false, repeat: false, modifiers: egui::Modifiers::NONE }); + assert_eq!(press_sampling_viewer(&mut harness), None, "wireframe must not sample an old pixel frame"); + } + + #[test] + fn a_new_modal_backdrop_cannot_deliver_a_pending_sample() { + let mut harness = sampling_viewer(); + harness.state_mut().dialog = Some(crate::Dialog::About); + assert_eq!(press_sampling_viewer(&mut harness), None, "the first modal pass must already exclude its backdrop"); + } + + #[test] + fn a_picker_can_sample_behind_its_own_dialog_but_not_a_new_modal() { + let ctx = egui::Context::default(); + let point = pos2(20.0, 20.0); + let mut output = ctx.run_ui(egui::RawInput { events: vec![egui::Event::PointerMoved(point)], ..Default::default() }, |ui| { + let ctx = ui.ctx(); + note_dialog(ctx, Some(crate::Dialog::SolidSettings)); + note_viewer(ctx, Rect::from_min_size(pos2(0.0, 0.0), vec2(320.0, 180.0))); + note_overlay(ctx, Rect::from_min_size(pos2(100.0, 100.0), vec2(120.0, 80.0))); + arm(ctx, egui::Id::unique("test-picker")); + assert_eq!(eyedropper_point(ctx), Some(point)); + note_dialog(ctx, Some(crate::Dialog::About)); + assert_eq!(eyedropper_point(ctx), None); + }); + output.textures_delta.clear(); + } + #[test] fn armed_eyedropper_takes_one_sample_then_disarms() { let ctx = egui::Context::default(); diff --git a/crates/ui-egui/src/frames.rs b/crates/ui-egui/src/frames.rs index a7023a494..53e2248c5 100644 --- a/crates/ui-egui/src/frames.rs +++ b/crates/ui-egui/src/frames.rs @@ -534,18 +534,16 @@ impl Default for Frames { /// Premultiplied f32 → egui premultiplied Color32 (alpha kept, so the viewer can show the /// transparency grid or the comp background colour underneath). pub fn to_color_image(img: &effectcraft_engine::render::Image) -> egui::ColorImage { - let px: Vec = img - .data - .par_iter() - .map(|p| { - let a = p[3].clamp(0.0, 1.0); - let c = |v: f32| (v.clamp(0.0, a) * 255.0 + 0.5) as u8; - egui::Color32::from_rgba_premultiplied(c(p[0]), c(p[1]), c(p[2]), (a * 255.0 + 0.5) as u8) - }) - .collect(); + let px: Vec = img.data.par_iter().map(|p| display_pixel(*p)).collect(); egui::ColorImage::new([img.width as usize, img.height as usize], px) } +pub(crate) fn display_pixel(pixel: [f32; 4]) -> egui::Color32 { + let alpha = if pixel[3].is_nan() { 0.0 } else { pixel[3].clamp(0.0, 1.0) }; + let component = |value: f32| (value.clamp(0.0, alpha) * 255.0 + 0.5) as u8; + egui::Color32::from_rgba_premultiplied(component(pixel[0]), component(pixel[1]), component(pixel[2]), (alpha * 255.0 + 0.5) as u8) +} + /// The whole factor an image of `size` is averaged down by for a texture: at least `min_k`, and /// the smallest that fits its sides in `max_side` (frames of comps wider than the GPU's texture /// limit, #201). diff --git a/crates/ui-egui/src/frames/tests.rs b/crates/ui-egui/src/frames/tests.rs index 6da1dc269..9a522dc72 100644 --- a/crates/ui-egui/src/frames/tests.rs +++ b/crates/ui-egui/src/frames/tests.rs @@ -1,5 +1,11 @@ use super::*; +#[test] +fn converting_a_frame_with_nan_alpha_keeps_the_viewer_alive() { + let image = effectcraft_engine::render::Image::filled(1, 1, [0.5, 0.25, 0.1, f32::NAN]); + assert_eq!(to_color_image(&image).pixels, [egui::Color32::TRANSPARENT]); +} + struct ProbeDisk { keys: Mutex>, probes: std::sync::atomic::AtomicUsize, diff --git a/crates/ui-egui/src/panels/effect_controls/viewer.rs b/crates/ui-egui/src/panels/effect_controls/viewer.rs index 7bf65d40c..6f59d8c96 100644 --- a/crates/ui-egui/src/panels/effect_controls/viewer.rs +++ b/crates/ui-egui/src/panels/effect_controls/viewer.rs @@ -125,8 +125,7 @@ fn color_pick_preview(app: &EffectcraftApp, painter: &egui::Painter, map: &crate painter.rect_filled(chip, 11.0, Color32::from_black_alpha(180)); painter.text(chip.center(), Align2::CENTER_CENTER, hint, Tokens::ui(11.5), app.tokens.text); if let Some(pos) = pointer - && let Some(img) = &app.viewer_image - && let Some(sample) = fw::sample_frame(img, crate::panels::viewer_tools::shown_region(painter.ctx(), map.comp), map.to_comp(pos)) + && let Some(sample) = crate::panels::viewer_tools::sample_displayed(app, painter.ctx(), map.to_comp(pos)) { let swatch = Rect::from_min_size(pos + vec2(14.0, 14.0), vec2(26.0, 18.0)); painter.rect_filled(swatch, 2.0, Color32::from_rgb((sample[0] * 255.0) as u8, (sample[1] * 255.0) as u8, (sample[2] * 255.0) as u8)); @@ -162,8 +161,7 @@ fn text_color_pick( } if resp.clicked() && let Some(pos) = resp.interact_pointer_pos() - && let Some(img) = &app.viewer_image - && let Some(s) = fw::sample_frame(img, crate::panels::viewer_tools::shown_region(&ctx, map.comp), map.to_comp(pos)) + && let Some(s) = crate::panels::viewer_tools::sample_displayed(app, &ctx, map.to_comp(pos)) { let mut params = json!({"layer": pick.layer}); params[&pick.field] = json!([s[0], s[1], s[2]]); @@ -196,8 +194,7 @@ fn popup_eyedropper(app: &mut EffectcraftApp, ui: &mut egui::Ui, painter: &egui: app.viewer_pixels(); color_pick_preview(app, painter, map, crate::i18n::tr("Click to sample color"), crate::color_picker::eyedropper_point(&ctx)); if let Some(pos) = crate::color_picker::sample_press(&ctx) - && let Some(img) = &app.viewer_image - && let Some(s) = fw::sample_frame(img, crate::panels::viewer_tools::shown_region(&ctx, map.comp), map.to_comp(pos)) + && let Some(s) = crate::panels::viewer_tools::sample_displayed(app, &ctx, map.to_comp(pos)) { crate::color_picker::deliver_sample(&ctx, [s[0], s[1], s[2]]); } diff --git a/crates/ui-egui/src/panels/fx_widgets.rs b/crates/ui-egui/src/panels/fx_widgets.rs index fd71461e6..b3140d394 100644 --- a/crates/ui-egui/src/panels/fx_widgets.rs +++ b/crates/ui-egui/src/panels/fx_widgets.rs @@ -196,14 +196,21 @@ pub fn layer_to_comp(l2c: &Mat3, p: [f64; 2]) -> [f64; 2] { /// `region` ([x, y, w, h] in comp pixels: the whole comp, or the region of interest or Extended /// Viewer area it was rendered for), maybe at a reduced resolution. `None` outside it. pub fn frame_pixel(img: &egui::ColorImage, region: [f64; 4], pt: [f64; 2]) -> Option { + img.pixels.get(frame_pixel_index(img.size, region, pt)?).copied() +} + +pub(crate) fn frame_pixel_index(size: [usize; 2], region: [f64; 4], pt: [f64; 2]) -> Option { + if size.contains(&0) { + return None; + } let [x, y, w, h] = region; let (fx, fy) = ((pt[0] - x) / w, (pt[1] - y) / h); if !(0.0..1.0).contains(&fx) || !(0.0..1.0).contains(&fy) { return None; } - let sx = ((fx * img.size[0] as f64).floor() as usize).min(img.size[0].saturating_sub(1)); - let sy = ((fy * img.size[1] as f64).floor() as usize).min(img.size[1].saturating_sub(1)); - img.pixels.get(sy.checked_mul(img.size[0])?.checked_add(sx)?).copied() + let sx = ((fx * size[0] as f64).floor() as usize).min(size[0].saturating_sub(1)); + let sy = ((fy * size[1] as f64).floor() as usize).min(size[1].saturating_sub(1)); + sy.checked_mul(size[0])?.checked_add(sx) } /// Straight RGBA (0..1) of the displayed frame at comp point `pt` ([`frame_pixel`]). diff --git a/crates/ui-egui/src/panels/viewer_tools.rs b/crates/ui-egui/src/panels/viewer_tools.rs index f86a7bfee..3429f5404 100644 --- a/crates/ui-egui/src/panels/viewer_tools.rs +++ b/crates/ui-egui/src/panels/viewer_tools.rs @@ -366,6 +366,7 @@ pub(crate) fn display_key(app: &EffectcraftApp, has_conversion: bool) -> String /// is set), or the snapshot while Show Snapshot is on / F5 is held, through Show Channel and the /// exposure. Fast Previews ▸ Wireframe draws layer outlines instead. pub(crate) fn draw_frame(app: &mut EffectcraftApp, ctx: &egui::Context, painter: &egui::Painter, comp_rect: Rect, cid: ItemId, ectx: &EvalCtx) { + clear_sample_source(ctx); let opts = app.session.state.viewer.clone(); let snap = showing_snapshot(app, ctx); // Settings ▸ Video ▸ Mirror on Computer Monitor off: playback goes to Video Preview only. @@ -393,6 +394,8 @@ pub(crate) fn draw_frame(app: &mut EffectcraftApp, ctx: &egui::Context, painter: let dc = vw::DisplayColor::of(&app.session); let dc_key = display_key(app, dc.is_some()); let plain = opts.channel == Channel::Rgb && opts.exposure == 0.0 && dc.is_none(); + let display = SampleDisplay { channel: opts.channel, colorized: opts.colorized, exposure: opts.exposure, conversion: dc.clone() }; + let region = shown_region(ctx, [ectx.comp.width as f32, ectx.comp.height as f32]); let uv = Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)); if snap { let Some(s) = app.session.snapshot.clone() else { return }; @@ -415,6 +418,12 @@ pub(crate) fn draw_frame(app: &mut EffectcraftApp, ctx: &egui::Context, painter: } }; painter.image(tex.id(), comp_rect, uv, Color32::WHITE); + note_sample_source( + ctx, + SampleSource::Snapshot(s.image.clone()), + [0.0, 0.0, ectx.comp.width as f64, ectx.comp.height as f64], + (!plain).then_some(display), + ); painter.text(comp_rect.left_top() + vec2(6.0, 6.0), Align2::LEFT_TOP, crate::i18n::tr("Snapshot"), Tokens::ui(11.0), SNAP_COLOR); return; } @@ -428,6 +437,9 @@ pub(crate) fn draw_frame(app: &mut EffectcraftApp, ctx: &egui::Context, painter: ctx.request_repaint(); } painter.image(id, comp_rect, uv, Color32::WHITE); + if let Some((_, key)) = app.viewer_shown { + note_sample_source(ctx, SampleSource::Gpu(key), region, None); + } return; } comp_rect @@ -444,6 +456,9 @@ pub(crate) fn draw_frame(app: &mut EffectcraftApp, ctx: &egui::Context, painter: if plain { let uv = app.viewer_image.as_ref().map_or(uv, |src| fitted_uv(ctx, src.size, minify)); painter.image(tex.id(), rect, uv, Color32::WHITE); + if let Some(image) = app.viewer_image.clone() { + note_sample_source(ctx, SampleSource::Cpu(image), region, None); + } return; } rect @@ -470,6 +485,71 @@ pub(crate) fn draw_frame(app: &mut EffectcraftApp, ctx: &egui::Context, painter: } }; painter.image(tex.id(), rect, fitted_uv(ctx, src.size, minify), Color32::WHITE); + note_sample_source(ctx, SampleSource::Cpu(src), region, Some(display)); +} + +#[derive(Clone)] +enum SampleSource { + Cpu(std::sync::Arc), + Gpu(crate::frames::FrameKey), + Snapshot(std::sync::Arc), +} + +#[derive(Clone)] +struct SampleDisplay { + channel: Channel, + colorized: bool, + exposure: f32, + conversion: Option, +} + +#[derive(Clone)] +struct DisplayedSample { + pass: u64, + source: SampleSource, + region: [f64; 4], + display: Option, +} + +fn sample_source_id() -> egui::Id { + egui::Id::unique("viewer-displayed-sample-source") +} + +pub(crate) fn clear_sample_source(ctx: &egui::Context) { + ctx.data_mut(|data| data.remove::(sample_source_id())); +} + +fn note_sample_source(ctx: &egui::Context, source: SampleSource, region: [f64; 4], display: Option) { + let sample = DisplayedSample { pass: ctx.cumulative_pass_nr(), source, region, display }; + ctx.data_mut(|data| data.insert_temp(sample_source_id(), sample)); +} + +/// Sample only a frame actually drawn this pass, with that draw's channel, exposure and +/// display conversion. Hidden and wireframe views deliberately have no sample source. +pub(crate) fn sample_displayed(app: &EffectcraftApp, ctx: &egui::Context, point: [f64; 2]) -> Option<[f32; 4]> { + let sample = ctx.data(|data| data.get_temp::(sample_source_id()))?; + if sample.pass != ctx.cumulative_pass_nr() { + return None; + } + let pixel = match sample.source { + SampleSource::Cpu(image) => super::fx_widgets::frame_pixel(&image, sample.region, point)?, + SampleSource::Gpu(key) => { + if app.viewer_shown.map(|(_, shown)| shown) != Some(key) { + return None; + } + super::fx_widgets::frame_pixel(app.viewer_image.as_ref()?, sample.region, point)? + } + SampleSource::Snapshot(image) => { + let index = super::fx_widgets::frame_pixel_index([image.width as usize, image.height as usize], sample.region, point)?; + crate::frames::display_pixel(*image.data.get(index)?) + } + }; + let image = egui::ColorImage::new([1, 1], vec![pixel]); + let image = match sample.display { + Some(display) => transformed(&image, display.channel, display.colorized, display.exposure, display.conversion), + None => image, + }; + super::fx_widgets::sample_frame(&image, [0.0, 0.0, 1.0, 1.0], [0.5, 0.5]) } /// The texture coordinates of a frame of `size` in its texture, fitted with at least `minify` From 36e18cfe6addf8d465a0bf5602d2b1ba1b301325 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Thu, 29 Oct 2026 22:25:26 -0400 Subject: [PATCH 29/29] CI: workflows only call craft-common-actions at @main (#2625) AGENTS.md (and CLAUDE.md where present) gets a CI workflows section: workflows only call craft-common-actions at @main, with no other jobs; feature and fix PRs don't touch CI and link or open a craft-common-actions issue (a PR only for a small fix that is safe for every app and does not increase CI time); legacy jobs aren't added to and move only as their own task, staying until a shared workflow runs them. windows-arm64.yml moves from its SHA pin to @main. The pinned revision is on craft-common-actions main and the workflow (and the actions it calls) is identical there, so nothing changes in behaviour. --- .github/workflows/windows-arm64.yml | 4 ++-- AGENTS.md | 5 +++++ CLAUDE.md | 5 +++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/windows-arm64.yml b/.github/workflows/windows-arm64.yml index 2a8144a5b..2cedc1a57 100644 --- a/.github/workflows/windows-arm64.yml +++ b/.github/workflows/windows-arm64.yml @@ -28,8 +28,8 @@ permissions: jobs: windows-arm64: - # Immutable supporting workflow revision until the new install contract lands upstream. - uses: storytold/craft-common-actions/.github/workflows/windows-arm64.yml@fa5ef18780cf5e7d5d3b8de068de56cffc83cac5 + # Intentionally tracked at @main (not SHA-pinned) so org-wide CI improvements, security fixes, and build optimizations propagate without PR churn across all craftapp repos. + uses: storytold/craft-common-actions/.github/workflows/windows-arm64.yml@main with: app-name: effectcraft app-dir: EffectCraft diff --git a/AGENTS.md b/AGENTS.md index 24abee6a1..24287ff9a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -144,3 +144,8 @@ as PhotoCraft does with `cargo xtask corpus`. GitHub profiles). Never hand-edit `contributors.json`. - Maintainers refresh the stats with `python3 ../../craftrules/scripts/contributors.py .` (it also re-verifies who wrote each `people.toml` entry; `--check` only verifies). + +## CI workflows +- Workflows in `.github/workflows/` call the shared workflows in [`storytold/craft-common-actions`](https://github.com/storytold/craft-common-actions) at `@main`, with their inputs and secrets. No other jobs: if no shared workflow does what's needed, it doesn't go here. +- Don't add or change CI in a feature or fix PR. If the existing workflows can't build or test a change, link an existing craft-common-actions issue or PR, or open a new issue, or a PR only for a small fix that is safe for every app and does not increase CI time. +- Some workflow files here still run their own jobs. Don't add to them. Moving a job into craft-common-actions is its own task, never part of another PR, and the job stays here until a shared workflow runs it. diff --git a/CLAUDE.md b/CLAUDE.md index 21f2df36b..7bd0e7422 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -30,3 +30,8 @@ EffectCraft is a clean-room, open-source, pure-Rust motion graphics and visual e - For UI work, **look at the result**. Headless (no window, works while the user is busy on the machine): `cargo run -p effectcraft-ui-egui --example snapshot -- --out ui.png --step '{"method":"engine.execute","params":{"command":"layer.select","params":{"layers":["#2"]}}}'` (steps are control-channel requests; `--script steps.jsonl`; `{"method":"snap","params":{"path":…}}` for intermediate shots). Or drive the live app via the control channel and take `ui.screenshot` (needs the window visible on screen). - After Effects reference: drive AE via ExtendScript (`osascript -e 'tell application "Adobe After Effects 2026" to DoScriptFile "x.jsx"'`) and capture only AE's window (`screencapture -l `); keep captures in `plan/aftereffects/ref/` (gitignored), never commit them, never capture the Home screen. - Parallel agents: separate git worktrees and `CARGO_TARGET_DIR=target/agent-`; keep every `Cargo.toml` valid (the `crates/*` glob). + +## CI workflows +- Workflows in `.github/workflows/` call the shared workflows in [`storytold/craft-common-actions`](https://github.com/storytold/craft-common-actions) at `@main`, with their inputs and secrets. No other jobs: if no shared workflow does what's needed, it doesn't go here. +- Don't add or change CI in a feature or fix PR. If the existing workflows can't build or test a change, link an existing craft-common-actions issue or PR, or open a new issue, or a PR only for a small fix that is safe for every app and does not increase CI time. +- Some workflow files here still run their own jobs. Don't add to them. Moving a job into craft-common-actions is its own task, never part of another PR, and the job stays here until a shared workflow runs it.