From 74517a52da4e35cd6804b7ff1d2ebccd7d139390 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Kozlovsk=C3=BD?= Date: Thu, 15 Oct 2026 15:37:14 +0200 Subject: [PATCH 01/37] Cryptomatte: the GPU path skips the ID colours unless the Colors display reads them, and the CPU path keys the layer in place instead of copying it (#473) Output is bit-identical (all displays, CPU and GPU). At 2560x1440 with 3 ranks and 2000 IDs on 8 cores the CPU effect drops from ~87-135 ms to ~44-54 ms and the GPU path's aux reduction from 38 to 8 ms for Matte, Preview and Matte Only. --- crates/effects/src/channel3d.rs | 28 +++++++++++++++++++++++----- crates/effects/src/lib.rs | 2 +- crates/gpu/src/fx_depth.rs | 5 ++++- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/crates/effects/src/channel3d.rs b/crates/effects/src/channel3d.rs index 7884e808..32fda883 100644 --- a/crates/effects/src/channel3d.rs +++ b/crates/effects/src/channel3d.rs @@ -413,10 +413,16 @@ fn crypto_colors(planes: &RankPlanes, i: usize) -> [f32; 3] { /// selection's coverage (A), computed in parallel with the rank planes resolved once. The GPU /// effect uploads this image and samples it per layer pixel. pub fn crypto_aux_image(a: &AuxChannels, layer: &str, sel: &[u32]) -> Image { + crypto_aux_image_with(a, layer, sel, true) +} + +/// [`crypto_aux_image`]; with `colors` false the RGB stays 0 and only the matte (A) is computed, +/// for the displays that never read the ID colours. +pub fn crypto_aux_image_with(a: &AuxChannels, layer: &str, sel: &[u32], colors: bool) -> Image { let planes = a.crypto_rank_planes(layer); let mut img = Image::new(a.width, a.height); img.data.par_iter_mut().enumerate().for_each(|(i, px)| { - let c = crypto_colors(&planes, i); + let c = if colors { crypto_colors(&planes, i) } else { [0.0; 3] }; *px = [c[0], c[1], c[2], crypto_matte(&planes, sel, i)]; }); img @@ -436,7 +442,6 @@ fn cryptomatte(ctx: &EffectCtx, mut b: Buf) -> Buf { let display = ctx.params.e("display"); let matte_only = ctx.params.b("matteOnly"); let planes = a.crypto_rank_planes(&layer); - let src = b.img.clone(); b.img.data.par_iter_mut().enumerate().for_each(|(i, px)| { let m = idx[i].map_or(0.0, |k| crypto_matte(&planes, &sel, k)); if matte_only { @@ -450,11 +455,11 @@ fn cryptomatte(ctx: &EffectCtx, mut b: Buf) -> Buf { let c = c.map(|v| v * (1.0 - 0.5 * m) + 0.5 * m); [c[0], c[1], c[2], 1.0] } - // Matte: the layer keyed by the selection. - 1 => src.data[i].map(|v| v * m), + // Matte: the layer keyed by the selection (each pixel only reads itself, so no copy). + 1 => px.map(|v| v * m), // Preview: the layer with the selection tinted. _ => { - let s = src.data[i]; + let s = *px; [s[0] * (1.0 - 0.5 * m) + 0.5 * m * s[3], s[1] * (1.0 - 0.5 * m) + 0.5 * m * s[3], s[2] * (1.0 - 0.5 * m), s[3]] } }; @@ -835,6 +840,19 @@ mod tests { } } + /// Without the ID colours the matte (A) is bit-identical and the RGB is 0, on the edge values. + #[test] + fn crypto_aux_image_without_colors_keeps_the_matte() { + let a = edge_aux(true); + let sel = edge_selection(); + let full = crypto_aux_image(&a, "CryptoObject", &sel); + let matte = crypto_aux_image_with(&a, "CryptoObject", &sel, false); + for (f, m) in full.data.iter().zip(&matte.data) { + assert_eq!(f[3].to_bits(), m[3].to_bits()); + assert_eq!(m[..3], [0.0; 3]); + } + } + /// The CPU effect on a 20×10 layer over the 6×2 edge aux: pixels inside the aux area match /// the lookup by name bit for bit in every display mode, pixels outside it (no aux index) /// get an empty matte and no ID colour. diff --git a/crates/effects/src/lib.rs b/crates/effects/src/lib.rs index 40ed79a7..4c1ea60b 100644 --- a/crates/effects/src/lib.rs +++ b/crates/effects/src/lib.rs @@ -110,7 +110,7 @@ pub use time_fx::{PixelMotion, TimewarpPlan, TwStep, pmb_plan, posterized_time, // effectcraft-gpu fx_tone. pub use ocio::color_stabilizer_maps; // GPU effects, part B (3D Channel, Immersive Video, colour management, simulation render passes). -pub use channel3d::{CRYPTO_LAYERS, crypto_aux_image, id_color, selection_hashes}; +pub use channel3d::{CRYPTO_LAYERS, crypto_aux_image, crypto_aux_image_with, id_color, selection_hashes}; pub use ocio::{ColorOp, Straight, Tf, color_program}; pub use sim::{Acc, Blob, Bubble, Post, Shape, Sprite, SpritePlan, Tint, bubble_list, drizzle_drops, mercury_blobs, sprite_plan}; pub use sim2::{ diff --git a/crates/gpu/src/fx_depth.rs b/crates/gpu/src/fx_depth.rs index 4931afe2..a74ff717 100644 --- a/crates/gpu/src/fx_depth.rs +++ b/crates/gpu/src/fx_depth.rs @@ -278,7 +278,10 @@ fn cryptomatte(e: &mut Enc, ctx: &EffectCtx, b: GBuf, a: &AuxChannels) -> Option let sel = effectcraft_effects::selection_hashes(pr.s("selection"), manifest); // Per aux pixel: coverage-weighted ID colours (RGB) and the selection's coverage (A), on all // cores with the rank planes resolved once. - let img = effectcraft_effects::crypto_aux_image(a, &layer, &sel); + // The ID colours are only read by the Colors display; Matte, Preview and Matte Only need the + // matte alone. + let colors = pr.e("display") == 0 && !pr.b("matteOnly"); + let img = effectcraft_effects::crypto_aux_image_with(a, &layer, &sel, colors); let tex = e.g.upload_image(&img)?; let plane = lookup(e, &b, a, &tex, [0.5, 0.5], [0.0; 4]); let mut p = op(7); From 34d5f00af86eada959d71c86bec7614a6654a979 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Kozlovsk=C3=BD?= Date: Thu, 15 Oct 2026 15:48:46 +0200 Subject: [PATCH 02/37] Cryptomatte: Colors remembers each rank's last ID colour instead of hashing every pixel (#473) Bit-identical output. At 2560x1440, 3 ranks, 2000 IDs on 8 cores: CPU Colors ~69 -> ~49 ms, the GPU path's aux reduction ~35 -> ~14 ms. --- crates/effects/src/channel3d.rs | 52 ++++++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/crates/effects/src/channel3d.rs b/crates/effects/src/channel3d.rs index 32fda883..26fbfe7a 100644 --- a/crates/effects/src/channel3d.rs +++ b/crates/effects/src/channel3d.rs @@ -395,13 +395,37 @@ fn crypto_matte(planes: &RankPlanes, sel: &[u32], i: usize) -> f32 { .clamp(0.0, 1.0) } +/// [`id_color`] of the last ID seen on each rank plane, per worker. Neighbouring pixels mostly +/// carry the same IDs, so this skips the hash and the HSL conversion for most of them. Keyed by the +/// ID's bits, so it returns exactly what [`id_color`] would. +/// +/// Keep the per-pixel loop below as it is: with NaN coverage, which NaN payload ends up in a +/// channel depends on the order the compiler adds the ranks in, and a row-wise loop let it reorder +/// them (`crypto_aux_image_is_bit_exact_on_edge_values` catches that). +#[derive(Default)] +struct IdColorMemo(Vec<(u32, [f32; 3])>); + +impl IdColorMemo { + #[inline] + fn get(&mut self, rank: usize, id: f32) -> [f32; 3] { + if self.0.len() <= rank { + self.0.resize(rank + 1, (id.to_bits(), id_color(id))); + } + let e = &mut self.0[rank]; + if e.0 != id.to_bits() { + *e = (id.to_bits(), id_color(id)); + } + e.1 + } +} + /// The coverage-weighted ID colours at aux pixel `i` (the spec's preview). #[inline] -fn crypto_colors(planes: &RankPlanes, i: usize) -> [f32; 3] { +fn crypto_colors(planes: &RankPlanes, i: usize, memo: &mut IdColorMemo) -> [f32; 3] { let mut c = [0.0f32; 3]; - for (id, cov) in planes { + for (rank, (id, cov)) in planes.iter().enumerate() { let (Some(&id), Some(&cov)) = (id.get(i), cov.get(i)) else { continue }; - let k = id_color(id); + let k = memo.get(rank, id); for j in 0..3 { c[j] += k[j] * cov; } @@ -421,8 +445,8 @@ pub fn crypto_aux_image(a: &AuxChannels, layer: &str, sel: &[u32]) -> Image { pub fn crypto_aux_image_with(a: &AuxChannels, layer: &str, sel: &[u32], colors: bool) -> Image { let planes = a.crypto_rank_planes(layer); let mut img = Image::new(a.width, a.height); - img.data.par_iter_mut().enumerate().for_each(|(i, px)| { - let c = if colors { crypto_colors(&planes, i) } else { [0.0; 3] }; + img.data.par_iter_mut().enumerate().for_each_init(IdColorMemo::default, |memo, (i, px)| { + let c = if colors { crypto_colors(&planes, i, memo) } else { [0.0; 3] }; *px = [c[0], c[1], c[2], crypto_matte(&planes, sel, i)]; }); img @@ -442,7 +466,7 @@ fn cryptomatte(ctx: &EffectCtx, mut b: Buf) -> Buf { let display = ctx.params.e("display"); let matte_only = ctx.params.b("matteOnly"); let planes = a.crypto_rank_planes(&layer); - b.img.data.par_iter_mut().enumerate().for_each(|(i, px)| { + b.img.data.par_iter_mut().enumerate().for_each_init(IdColorMemo::default, |memo, (i, px)| { let m = idx[i].map_or(0.0, |k| crypto_matte(&planes, &sel, k)); if matte_only { *px = [m, m, m, m]; @@ -451,7 +475,7 @@ fn cryptomatte(ctx: &EffectCtx, mut b: Buf) -> Buf { *px = match display { // Colors: coverage-weighted ID colours (the spec's preview), selection brighter. 0 => { - let c = idx[i].map_or([0.0; 3], |k| crypto_colors(&planes, k)); + let c = idx[i].map_or([0.0; 3], |k| crypto_colors(&planes, k, memo)); let c = c.map(|v| v * (1.0 - 0.5 * m) + 0.5 * m); [c[0], c[1], c[2], 1.0] } @@ -819,6 +843,20 @@ mod tests { a } + /// The memo hands out exactly `id_color`'s bits for every rank and pixel of the edge values + /// (NaN IDs with and without a payload, -0.0 and 0.0 are keyed by their bits). + #[test] + fn id_color_memo_matches_id_color() { + let a = edge_aux(true); + let planes = a.crypto_rank_planes("CryptoObject"); + let mut memo = IdColorMemo::default(); + for i in 0..12 { + for (r, (id, _)) in planes.iter().enumerate() { + assert_eq!(memo.get(r, id[i]).map(f32::to_bits), id_color(id[i]).map(f32::to_bits), "pixel {i} rank {r}"); + } + } + } + fn edge_selection() -> Vec { vec![crypto_hash("sphere"), f32::NAN.to_bits(), 0x7fc0_1234, (-0.0f32).to_bits()] } From 0fbe76b2bfbb6ab6d919ace5446c3d51ea5a981c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Kozlovsk=C3=BD?= Date: Sat, 17 Oct 2026 01:48:48 +0200 Subject: [PATCH 03/37] Cryptomatte: CryptoAux enum instead of a positional bool; GPU test for every display mode (#473) crypto_aux_image_with takes CryptoAux::{ColorsAndMatte, MatteOnly}, and CryptoAux::for_display holds the rule for which displays read the ID colours. A new GPU test checks every display x Matte Only against the CPU, so the GPU fails the test if the shader starts reading RGB in a mode that skips it (checked by mutation: forcing MatteOnly fails display 0). --- crates/effects/src/lib.rs | 2 +- crates/gpu/src/fx_depth.rs | 5 +++-- crates/gpu/src/tests_fx_depth.rs | 12 ++++++++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/crates/effects/src/lib.rs b/crates/effects/src/lib.rs index 4c1ea60b..5a083392 100644 --- a/crates/effects/src/lib.rs +++ b/crates/effects/src/lib.rs @@ -110,7 +110,7 @@ pub use time_fx::{PixelMotion, TimewarpPlan, TwStep, pmb_plan, posterized_time, // effectcraft-gpu fx_tone. pub use ocio::color_stabilizer_maps; // GPU effects, part B (3D Channel, Immersive Video, colour management, simulation render passes). -pub use channel3d::{CRYPTO_LAYERS, crypto_aux_image, crypto_aux_image_with, id_color, selection_hashes}; +pub use channel3d::{CRYPTO_LAYERS, CryptoAux, crypto_aux_image, crypto_aux_image_with, id_color, selection_hashes}; pub use ocio::{ColorOp, Straight, Tf, color_program}; pub use sim::{Acc, Blob, Bubble, Post, Shape, Sprite, SpritePlan, Tint, bubble_list, drizzle_drops, mercury_blobs, sprite_plan}; pub use sim2::{ diff --git a/crates/gpu/src/fx_depth.rs b/crates/gpu/src/fx_depth.rs index a74ff717..1bb0e49e 100644 --- a/crates/gpu/src/fx_depth.rs +++ b/crates/gpu/src/fx_depth.rs @@ -280,8 +280,9 @@ fn cryptomatte(e: &mut Enc, ctx: &EffectCtx, b: GBuf, a: &AuxChannels) -> Option // cores with the rank planes resolved once. // The ID colours are only read by the Colors display; Matte, Preview and Matte Only need the // matte alone. - let colors = pr.e("display") == 0 && !pr.b("matteOnly"); - let img = effectcraft_effects::crypto_aux_image_with(a, &layer, &sel, colors); + // `tests_fx_depth::cryptomatte_every_display_matches_the_cpu` guards this against the shader. + let what = effectcraft_effects::CryptoAux::for_display(pr.e("display"), pr.b("matteOnly")); + let img = effectcraft_effects::crypto_aux_image_with(a, &layer, &sel, what); let tex = e.g.upload_image(&img)?; let plane = lookup(e, &b, a, &tex, [0.5, 0.5], [0.0; 4]); let mut p = op(7); diff --git a/crates/gpu/src/tests_fx_depth.rs b/crates/gpu/src/tests_fx_depth.rs index b84b4849..8a666565 100644 --- a/crates/gpu/src/tests_fx_depth.rs +++ b/crates/gpu/src/tests_fx_depth.rs @@ -251,6 +251,18 @@ fn ids_and_cryptomatte() { case("ec.3d.cryptomatte", &[("selection", Value::Str("alpha".into())), ("matteOnly", on())]); } +/// The GPU skips the ID colours for displays that do not read them +/// ([`effectcraft_effects::CryptoAux::for_display`]): every display × Matte Only must still match +/// the CPU effect, so a shader that starts reading RGB in another mode fails here. +#[test] +fn cryptomatte_every_display_matches_the_cpu() { + for display in 0..3 { + for matte_only in [false, true] { + case("ec.3d.cryptomatte", &[("selection", Value::Str("beta, delta".into())), ("display", e(display)), ("matteOnly", Value::Bool(matte_only))]); + } + } +} + #[test] fn extractor() { case("ec.3d.extractor", &[("red", Value::Str("Z".into())), ("green", Value::Str("UV.U".into())), ("whitePoint", n(1000.0))]); From 14f4c56732c078ab16e52736696842c593bbbbe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Kozlovsk=C3=BD?= Date: Sun, 18 Oct 2026 16:13:39 +0200 Subject: [PATCH 04/37] Cryptomatte: define CryptoAux, which the previous commit used but never added (#473) crypto_aux_image_with now takes CryptoAux::{ColorsAndMatte, MatteOnly} instead of a bool, and CryptoAux::for_display holds the rule for which displays read the ID colours (Colors without Matte Only). IdColorMemo uses get_mut instead of indexing, and the CPU effect zips the pixels with the aux index map instead of indexing it. No behaviour change. --- crates/effects/src/channel3d.rs | 70 +++++++++++++++++++++++++-------- 1 file changed, 54 insertions(+), 16 deletions(-) diff --git a/crates/effects/src/channel3d.rs b/crates/effects/src/channel3d.rs index 26fbfe7a..a77d4497 100644 --- a/crates/effects/src/channel3d.rs +++ b/crates/effects/src/channel3d.rs @@ -408,14 +408,19 @@ struct IdColorMemo(Vec<(u32, [f32; 3])>); impl IdColorMemo { #[inline] fn get(&mut self, rank: usize, id: f32) -> [f32; 3] { - if self.0.len() <= rank { - self.0.resize(rank + 1, (id.to_bits(), id_color(id))); + let bits = id.to_bits(); + match self.0.get_mut(rank) { + Some(e) if e.0 == bits => e.1, + Some(e) => { + *e = (bits, id_color(id)); + e.1 + } + None => { + let c = id_color(id); + self.0.resize(rank + 1, (bits, c)); + c + } } - let e = &mut self.0[rank]; - if e.0 != id.to_bits() { - *e = (id.to_bits(), id_color(id)); - } - e.1 } } @@ -437,16 +442,37 @@ fn crypto_colors(planes: &RankPlanes, i: usize, memo: &mut IdColorMemo) -> [f32; /// selection's coverage (A), computed in parallel with the rank planes resolved once. The GPU /// effect uploads this image and samples it per layer pixel. pub fn crypto_aux_image(a: &AuxChannels, layer: &str, sel: &[u32]) -> Image { - crypto_aux_image_with(a, layer, sel, true) + crypto_aux_image_with(a, layer, sel, CryptoAux::ColorsAndMatte) } -/// [`crypto_aux_image`]; with `colors` false the RGB stays 0 and only the matte (A) is computed, -/// for the displays that never read the ID colours. -pub fn crypto_aux_image_with(a: &AuxChannels, layer: &str, sel: &[u32], colors: bool) -> Image { +/// What [`crypto_aux_image_with`] computes per aux pixel. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum CryptoAux { + /// The coverage-weighted ID colours (RGB) and the selection's coverage (A). + ColorsAndMatte, + /// Only the selection's coverage (A); the RGB stays 0. + MatteOnly, +} + +impl CryptoAux { + /// What the Cryptomatte effect reads for `display` and Matte Only: the ID colours are only + /// read by the Colors display (0) without Matte Only; Matte, Preview and Matte Only need the + /// matte alone. + pub fn for_display(display: u32, matte_only: bool) -> Self { + if display == 0 && !matte_only { Self::ColorsAndMatte } else { Self::MatteOnly } + } +} + +/// [`crypto_aux_image`]; with [`CryptoAux::MatteOnly`] the RGB stays 0 and only the matte (A) is +/// computed, for the displays that never read the ID colours. +pub fn crypto_aux_image_with(a: &AuxChannels, layer: &str, sel: &[u32], what: CryptoAux) -> Image { let planes = a.crypto_rank_planes(layer); let mut img = Image::new(a.width, a.height); img.data.par_iter_mut().enumerate().for_each_init(IdColorMemo::default, |memo, (i, px)| { - let c = if colors { crypto_colors(&planes, i, memo) } else { [0.0; 3] }; + let c = match what { + CryptoAux::ColorsAndMatte => crypto_colors(&planes, i, memo), + CryptoAux::MatteOnly => [0.0; 3], + }; *px = [c[0], c[1], c[2], crypto_matte(&planes, sel, i)]; }); img @@ -466,8 +492,8 @@ fn cryptomatte(ctx: &EffectCtx, mut b: Buf) -> Buf { let display = ctx.params.e("display"); let matte_only = ctx.params.b("matteOnly"); let planes = a.crypto_rank_planes(&layer); - b.img.data.par_iter_mut().enumerate().for_each_init(IdColorMemo::default, |memo, (i, px)| { - let m = idx[i].map_or(0.0, |k| crypto_matte(&planes, &sel, k)); + b.img.data.par_iter_mut().zip(idx.par_iter()).for_each_init(IdColorMemo::default, |memo, (px, &k)| { + let m = k.map_or(0.0, |k| crypto_matte(&planes, &sel, k)); if matte_only { *px = [m, m, m, m]; return; @@ -475,7 +501,7 @@ fn cryptomatte(ctx: &EffectCtx, mut b: Buf) -> Buf { *px = match display { // Colors: coverage-weighted ID colours (the spec's preview), selection brighter. 0 => { - let c = idx[i].map_or([0.0; 3], |k| crypto_colors(&planes, k, memo)); + let c = k.map_or([0.0; 3], |k| crypto_colors(&planes, k, memo)); let c = c.map(|v| v * (1.0 - 0.5 * m) + 0.5 * m); [c[0], c[1], c[2], 1.0] } @@ -884,13 +910,25 @@ mod tests { let a = edge_aux(true); let sel = edge_selection(); let full = crypto_aux_image(&a, "CryptoObject", &sel); - let matte = crypto_aux_image_with(&a, "CryptoObject", &sel, false); + let matte = crypto_aux_image_with(&a, "CryptoObject", &sel, CryptoAux::MatteOnly); for (f, m) in full.data.iter().zip(&matte.data) { assert_eq!(f[3].to_bits(), m[3].to_bits()); assert_eq!(m[..3], [0.0; 3]); } } + /// Only the Colors display without Matte Only reads the ID colours. + #[test] + fn crypto_aux_for_display_reads_colors_only_for_the_colors_display() { + assert_eq!(CryptoAux::for_display(0, false), CryptoAux::ColorsAndMatte); + assert_eq!(CryptoAux::for_display(0, true), CryptoAux::MatteOnly); + for display in [1, 2, 3, u32::MAX] { + for matte_only in [false, true] { + assert_eq!(CryptoAux::for_display(display, matte_only), CryptoAux::MatteOnly, "display {display}"); + } + } + } + /// The CPU effect on a 20×10 layer over the 6×2 edge aux: pixels inside the aux area match /// the lookup by name bit for bit in every display mode, pixels outside it (no aux index) /// get an empty matte and no ID colour. From 3b9d243ac516c790aa4e7905b6e0cd003426b157 Mon Sep 17 00:00:00 2001 From: Anubhav-Rai Date: Sun, 18 Oct 2026 22:46:28 +0530 Subject: [PATCH 05/37] =?UTF-8?q?Shift=20Channels:=20After=20Effects'=20Ta?= =?UTF-8?q?ke=20=E2=80=A6=20From=20menu=20order,=20so=2010=20is=20Full=20O?= =?UTF-8?q?ff=20(#1797)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The menu had an extra "Half On" at position 10, where After Effects has "Full Off", so a project or script written for After Effects set a channel to 50 % grey instead of turning it off, with no error: the RGB-split recipe (three copies, each keeping one channel and setting the other two to Full Off, blended with Add) rendered pastel instead of pure red, green and blue. After Effects' ten options now come first, in its order, and "Half On" sits last as an EffectCraft extra, so values 1 to 10 mean what they mean in After Effects and nothing is lost. Instances saved with the old order keep the option they chose: upgrade_instance already remaps popup values by label. The GPU path keeps its own copy of the table, so it is reordered to match; a GPU-vs-CPU parity case now covers the constant sources at the end of the menu, where the two tables could drift apart unnoticed. Both paths clamped the index to a hardcoded 10 and now use SHIFT_ORDER.len() - 1, so the clamp cannot go stale if the list changes again. Co-Authored-By: Claude Opus 5 --- crates/effects/src/channel.rs | 27 ++++++++++++++++++++++++--- crates/effects/src/migrate.rs | 16 ++++++++++++++++ crates/gpu/src/fx_key.rs | 7 +++++-- crates/gpu/src/tests_fx_key.rs | 3 +++ 4 files changed, 48 insertions(+), 5 deletions(-) diff --git a/crates/effects/src/channel.rs b/crates/effects/src/channel.rs index b763ac21..44e9dd89 100644 --- a/crates/effects/src/channel.rs +++ b/crates/effects/src/channel.rs @@ -49,12 +49,15 @@ fn set_channels(ctx: &EffectCtx, mut b: Buf) -> Buf { b } +/// Shift Channels' Take … From menu. After Effects' ten options come first, in its order, so a +/// project or script that sets 10 ("Full Off" there) turns the channel off here too; "Half On" is +/// an EffectCraft extra and sits last. const SHIFT_ORDER: [Src; 11] = - [Src::Alpha, Src::Red, Src::Green, Src::Blue, Src::Luminance, Src::Hue, Src::Lightness, Src::Saturation, Src::Full, Src::Half, Src::Off]; -const SHIFT_NAMES: [&str; 11] = ["Alpha", "Red", "Green", "Blue", "Luminance", "Hue", "Lightness", "Saturation", "Full On", "Half On", "Full Off"]; + [Src::Alpha, Src::Red, Src::Green, Src::Blue, Src::Luminance, Src::Hue, Src::Lightness, Src::Saturation, Src::Full, Src::Off, Src::Half]; +const SHIFT_NAMES: [&str; 11] = ["Alpha", "Red", "Green", "Blue", "Luminance", "Hue", "Lightness", "Saturation", "Full On", "Full Off", "Half On"]; fn shift_channels(ctx: &EffectCtx, mut b: Buf) -> Buf { - let s = ["takeRedFrom", "takeGreenFrom", "takeBlueFrom", "takeAlphaFrom"].map(|id| SHIFT_ORDER[(ctx.params.e(id) as usize).min(10)]); + let s = ["takeRedFrom", "takeGreenFrom", "takeBlueFrom", "takeAlphaFrom"].map(|id| SHIFT_ORDER[(ctx.params.e(id) as usize).min(SHIFT_ORDER.len() - 1)]); route(&mut b, s); b } @@ -548,6 +551,24 @@ mod tests { assert!(close(out.get(7, 0), [0.25, 0.25, 0.45, 0.5])); } + /// #1797: the Take … From menu keeps After Effects' order, where the tenth option (`Enum(9)` + /// here, 10 to a script) is Full Off, not Half On. + #[test] + fn shift_channels_menu_follows_after_effects_order() { + assert_eq!(SHIFT_NAMES[8..], ["Full On", "Full Off", "Half On"]); + // The sample's right-hand pixels are half transparent, so a straight 0.0 and 0.5 are + // stored as 0.0 and 0.25. + let off = run("ec.channel.shiftchannels", &[("takeGreenFrom", Value::Enum(9))], sample()); + assert!(close(off.get(0, 0), [0.2, 0.0, 0.9, 1.0]), "{:?}", off.get(0, 0)); + assert!(close(off.get(7, 0), [0.1, 0.0, 0.45, 0.5]), "{:?}", off.get(7, 0)); + let half = run("ec.channel.shiftchannels", &[("takeGreenFrom", Value::Enum(10))], sample()); + assert!(close(half.get(0, 0), [0.2, 0.5, 0.9, 1.0]), "{:?}", half.get(0, 0)); + assert!(close(half.get(7, 0), [0.1, 0.25, 0.45, 0.5]), "{:?}", half.get(7, 0)); + // A value past the last option stays on the last one instead of panicking. + let past = run("ec.channel.shiftchannels", &[("takeGreenFrom", Value::Enum(99))], sample()); + assert_eq!(past.get(0, 0), half.get(0, 0)); + } + #[test] fn channel_combiner_hls_round_trip() { let img = sample(); diff --git a/crates/effects/src/migrate.rs b/crates/effects/src/migrate.rs index 0e037022..b5976b76 100644 --- a/crates/effects/src/migrate.rs +++ b/crates/effects/src/migrate.rs @@ -499,6 +499,22 @@ mod tests { assert_eq!(pr.ui, popup.ui); } + /// #1797: Shift Channels' menu put Half On before Full Off; a project saved then keeps the + /// option it chose, by label. + #[test] + fn shift_channels_half_on_and_full_off_are_remapped_by_label() { + const OLD: [&str; 11] = ["Alpha", "Red", "Green", "Blue", "Luminance", "Hue", "Lightness", "Saturation", "Full On", "Half On", "Full Off"]; + for (saved, want) in [(9u32, 10u32), (10, 9), (1, 1)] { + let mut next = 1; + let (spec, mut g) = instance("ec.channel.shiftchannels", &mut next); + let pr = g.get_mut("takeGreenFrom").unwrap(); + pr.ui = ParamUi::Popup { options: OLD.iter().map(|s| s.to_string()).collect() }; + pr.value = Value::Enum(saved); + upgrade_instance(spec, &mut g, &mut Ids(&mut next), SIZE); + assert_eq!(g.get("takeGreenFrom").unwrap().value, Value::Enum(want), "saved {saved}"); + } + } + /// #514: Apply Color LUT's LUT was hidden (set only by commands) and the OCIO files were /// plain text; old projects get the Choose… file parameter, keeping the path. #[test] diff --git a/crates/gpu/src/fx_key.rs b/crates/gpu/src/fx_key.rs index 1099bccd..3bb5a87a 100644 --- a/crates/gpu/src/fx_key.rs +++ b/crates/gpu/src/fx_key.rs @@ -438,8 +438,10 @@ fn refine(e: &mut Enc, ctx: &EffectCtx, b: GBuf, hard: bool) -> Option { const SET_CHANNEL_LAYERS: [&str; 4] = ["sourceLayer1", "sourceLayer2", "sourceLayer3", "sourceLayer4"]; +/// Shift Channels' Take … From menu, in the order of `effects`' `SHIFT_ORDER`: the two must agree, +/// or the source depends on whether the effect renders on the GPU. const SHIFT_ORDER: [Src; 11] = - [Src::Alpha, Src::Red, Src::Green, Src::Blue, Src::Luminance, Src::Hue, Src::Lightness, Src::Saturation, Src::Full, Src::Half, Src::Off]; + [Src::Alpha, Src::Red, Src::Green, Src::Blue, Src::Luminance, Src::Hue, Src::Lightness, Src::Saturation, Src::Full, Src::Off, Src::Half]; const MATTE_ORDER: [Src; 10] = [Src::Red, Src::Green, Src::Blue, Src::Alpha, Src::Luminance, Src::Hue, Src::Lightness, Src::Saturation, Src::Full, Src::Off]; @@ -527,7 +529,8 @@ fn point(e: &mut Enc, id: &str, ctx: &EffectCtx, b: GBuf) -> Option { p.f[0] = [level, pr.f("softness").clamp(0.0, 100.0) as f32 / 100.0, 0.0, 0.0]; } "ec.channel.shiftchannels" => { - let s = ["takeRedFrom", "takeGreenFrom", "takeBlueFrom", "takeAlphaFrom"].map(|id| src_index(SHIFT_ORDER[(pr.e(id) as usize).min(10)])); + let s = ["takeRedFrom", "takeGreenFrom", "takeBlueFrom", "takeAlphaFrom"] + .map(|id| src_index(SHIFT_ORDER[(pr.e(id) as usize).min(SHIFT_ORDER.len() - 1)])); return route(e, b, s); } "ec.channel.removecolormatting" => { diff --git a/crates/gpu/src/tests_fx_key.rs b/crates/gpu/src/tests_fx_key.rs index 5ece27e6..35303bb0 100644 --- a/crates/gpu/src/tests_fx_key.rs +++ b/crates/gpu/src/tests_fx_key.rs @@ -155,6 +155,9 @@ fn refine_mattes() { fn channel_effects() { effect_case("ec.channel.setchannels", &[("setRedTo", e(2)), ("setGreenTo", e(4)), ("setBlueTo", e(5)), ("setAlphaTo", e(6))]); effect_case("ec.channel.shiftchannels", &[("takeRedFrom", e(3)), ("takeGreenFrom", e(7)), ("takeAlphaFrom", e(4))]); + // #1797: the constant sources at the end of the menu, where the GPU's order could drift from + // the CPU's (Full On, Full Off, Half On). + effect_case("ec.channel.shiftchannels", &[("takeRedFrom", e(8)), ("takeGreenFrom", e(9)), ("takeBlueFrom", e(10))]); effect_case("ec.channel.removecolormatting", &[("backgroundColor", c(0.3, 0.5, 0.7))]); effect_case("ec.channel.removecolormatting", &[("backgroundColor", c(0.8, 0.2, 0.1)), ("clipHdr", off())]); for op in [0, 2, 3, 4, 8, 10, 12] { From ae5d090aed089d007b2b8537ef80246e595da661 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Mon, 19 Oct 2026 09:16:41 +0200 Subject: [PATCH 06/37] =?UTF-8?q?Hold=20position=20between=20identical=20a?= =?UTF-8?q?uto-B=C3=A9zier=20spatial=20keys?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1675 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/keyframe/src/lib.rs | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/crates/keyframe/src/lib.rs b/crates/keyframe/src/lib.rs index e0ee137b..83774764 100644 --- a/crates/keyframe/src/lib.rs +++ b/crates/keyframe/src/lib.rs @@ -327,7 +327,12 @@ fn auto_spatial(keys: &[Keyframe], i: usize) -> ([f64; 3], [f64; 3]) { let a = v3(&keys[i.saturating_sub(1)].value); let c = v3(&keys[(i + 1).min(keys.len() - 1)].value); let tout = [(c[0] - a[0]) / 6.0, (c[1] - a[1]) / 6.0, (c[2] - a[2]) / 6.0]; - ([-tout[0], -tout[1], -tout[2]], tout) + // A side facing a key at the same position gets no tangent, so the segment between two + // identical keys has zero length and the layer holds still (#1675). + let p = v3(&keys[i].value); + let tin = if i > 0 && a == p { [0.0; 3] } else { [-tout[0], -tout[1], -tout[2]] }; + let tout = if i + 1 < keys.len() && c == p { [0.0; 3] } else { tout }; + (tin, tout) } /// Effective spatial tangents of key `i` (auto or stored). @@ -928,6 +933,26 @@ mod tests { assert_eq!(spatial_tangents(&still[..1], 0), ([0.0; 3], [0.0; 3])); } + #[test] + fn auto_spatial_holds_between_identical_keys() { + let path = [(0.0, [0.0, 540.0]), (0.6, [400.0, 540.0]), (4.4, [400.0, 540.0]), (5.0, [0.0, 540.0])]; + for eased in [false, true] { + let keys: Vec = path + .iter() + .map(|&(t, v)| { + let k = Keyframe::new(s(t), Value::Vec2(v)); + if eased { k.eased() } else { k } + }) + .collect(); + assert_eq!(spatial_tangents(&keys, 1).1, [0.0; 3]); + assert_eq!(spatial_tangents(&keys, 2).0, [0.0; 3]); + for t in [0.7, 1.0, 2.5, 4.0, 4.3] { + let p = evaluate(&keys, s(t), true).unwrap().as_vec3(); + assert!((p[0] - 400.0).abs() < 1e-6 && (p[1] - 540.0).abs() < 1e-6, "eased={eased} t={t} p={p:?}"); + } + } + } + #[test] fn set_key_replaces_same_time() { let mut keys = vec![]; From a3d5e55bc72b80a470d77b419b714c72ab5d4f28 Mon Sep 17 00:00:00 2001 From: Wes Campbell Date: Mon, 19 Oct 2026 14:33:59 -0500 Subject: [PATCH 07/37] EPS: Illustrator EPS files draw their artwork (#955) Illustrator EPS imported as an empty layer. Three gaps in the PostScript interpreter, each against the PostScript Language Reference: - An operator stored under another name (`/mo /moveto load def`, as Illustrator's prolog does for every path and paint operator) pushed the operator's name instead of running it, so nothing was drawn. - Device queries that return values (currentpagedevice, currentscreen, currentflat, currenthalftone, status, dictstack, ...) pushed nothing, shifting the stack under the procset's guards. They now push a plain device's defaults; setscreen pops its three operands. - `currentfile 0 (eod) /SubFileDecode filter` now skips to the end-of-data string, so an embedded XMP packet isn't run as PostScript. The regression test uses synthetic PostScript only. Co-Authored-By: Claude Opus 5.5 --- crates/pdf/src/ps.rs | 102 ++++++++++++++++++++++++++++++++- crates/pdf/src/tests_import.rs | 20 +++++++ 2 files changed, 121 insertions(+), 1 deletion(-) diff --git a/crates/pdf/src/ps.rs b/crates/pdf/src/ps.rs index 45db6c2e..984f7206 100644 --- a/crates/pdf/src/ps.rs +++ b/crates/pdf/src/ps.rs @@ -467,6 +467,10 @@ impl Interp { if let Some(v) = self.lookup(n) { return match v { V::Proc(_) => self.exec_obj(v, src), + // An operator stored under another name (`/mo /moveto load def`, as Illustrator's + // EPS prolog does for every path operator) runs that operator. It pushed the + // operator's name instead, so such files drew nothing (#955). + V::Name(op) => self.builtin(&op, src), other => { self.push(other); Ok(()) @@ -1203,6 +1207,96 @@ impl Interp { } } "currentglobal" | "currentpacking" | "currentshared" => self.push(V::Bool(false)), + // Operators that return values must push them, or the rest of the program reads a + // shifted stack: Illustrator's EPS prolog queries the device this way and then drew + // nothing (#955). Values are a plain device's defaults (PostScript Language + // Reference, 3rd ed., chapter 8). + "currentflat" => self.push(V::Num(1.0)), + "currentscreen" => { + self.push(V::Num(60.0)); + self.push(V::Num(45.0)); + self.push(V::Proc(Rc::new(vec![]))); + } + "currentblackgeneration" | "currentundercolorremoval" | "currenttransfer" => self.push(V::Proc(Rc::new(vec![]))), + "currentcolortransfer" => { + for _ in 0..4 { + self.push(V::Proc(Rc::new(vec![]))); + } + } + "currentpagedevice" | "currenthalftone" | "currentsystemparams" | "currentuserparams" | "currentdevparams" => { + if n == "currentdevparams" { + self.pop(); + } + self.push(V::Dict(new_dict())); + } + "setuserparams" | "setsystemparams" | "setcolortransfer" | "setblackgeneration" | "setundercolorremoval" => { + let k = if n == "setcolortransfer" { 4 } else { 1 }; + for _ in 0..k { + self.pop(); + } + } + // `file status` → false; `string status` → false (no such file). + "status" => { + self.pop(); + self.push(V::Bool(false)); + } + "dictstack" => { + self.pop(); + let ds: Vec = self.dicts.iter().map(|d| V::Dict(d.clone())).collect(); + self.push(array(ds)); + } + "maxlength" => { + let k = match self.pop() { + V::Dict(d) => d.borrow().len(), + _ => 0, + }; + self.push(V::Num(k.saturating_add(16) as f64)); + } + "internaldict" => { + self.pop(); + let d = self.dicts[0].clone(); + self.push(V::Dict(d)); + } + "undefineresource" => { + self.pop(); + self.pop(); + } + // `[ … /Name pdfmark`: a Distiller annotation; other interpreters discard it. + "pdfmark" => { + while let Some(v) = self.st.pop() { + if matches!(v, V::Mark) { + break; + } + } + } + // `source [params] /Name filter`. A SubFileDecode of the current file with a count of + // 0 reads to its end-of-data string: skip that data (an embedded XMP packet, which + // otherwise ran as PostScript). Other filters stand for the file they read. + "filter" => { + let name = self.pop().key(); + if name == "SubFileDecode" { + let eod = match self.pop() { + V::Str(s) => s.borrow().clone(), + _ => vec![], + }; + let count = self.popn(); + if let V::File = self.pop() + && count == 0.0 + && !eod.is_empty() + { + src.pos = match find_from(src.d, &eod, src.pos) { + Some(p) => p.saturating_add(eod.len()), + None => src.d.len(), + }; + } + } else { + if matches!(self.st.last(), Some(V::Dict(_))) { + self.pop(); + } + self.pop(); + } + self.push(V::File); + } "vmstatus" => { for _ in 0..3 { self.push(V::Num(0.0)); @@ -1236,9 +1330,15 @@ impl Interp { let a: Vec = self.pop().items().unwrap_or_default().iter().filter_map(V::num).collect(); self.gs.dash = (!a.is_empty() && a.iter().any(|v| *v > 0.0)).then_some((a, off)); } - "setflat" | "setstrokeadjust" | "setoverprint" | "setrenderingintent" | "setcolorrendering" | "sethalftone" | "setscreen" | "settransfer" => { + "setflat" | "setstrokeadjust" | "setoverprint" | "setrenderingintent" | "setcolorrendering" | "sethalftone" | "settransfer" => { self.pop(); } + // frequency angle proc + "setscreen" => { + for _ in 0..3 { + self.pop(); + } + } "setgray" => { let g = self.popn().clamp(0.0, 1.0); self.gs.color = [g; 3]; diff --git a/crates/pdf/src/tests_import.rs b/crates/pdf/src/tests_import.rs index b6a95859..7f46f6cf 100644 --- a/crates/pdf/src/tests_import.rs +++ b/crates/pdf/src/tests_import.rs @@ -25,6 +25,26 @@ fn ink_x(img: &effectcraft_raster::Image) -> (i64, i64) { (x0, x1) } +/// #955: EPS files that name their operators (`/mo /moveto load def`, as Illustrator's do), +/// query the device and embed metadata read through a SubFileDecode filter draw their artwork. +/// They imported as an empty layer. +#[test] +fn eps_with_aliased_operators_device_queries_and_embedded_data_draws() { + let square = "1 0 0 setrgbcolor 50 25 mo 150 25 li 150 75 li 50 75 li cp f\n"; + let aliases = "/mo /moveto load def /li /lineto load def /cp /closepath load def /f /fill load def\n"; + let doc = eps(format!("{aliases}{square}").as_bytes()); + assert!(coverage(&render(&doc), 50, 25, 150, 75) > 4900.0, "aliased operators draw the square"); + // Every operator that returns a value pushes it, so the stack stays aligned. + let queries = "mark currentflat currentscreen currentpagedevice currenthalftone currentblackgeneration \ + currentundercolorremoval currentcolortransfer currentsystemparams counttomark 13 eq /ok exch def cleartomark\n"; + let doc = eps(format!("{aliases}{queries}ok {{ {square} }} if\n").as_bytes()); + assert!(coverage(&render(&doc), 50, 25, 150, 75) > 4900.0, "device queries leave 13 values"); + // Data up to the end-of-data string isn't run: its unbalanced brace would swallow the rest. + let data = "currentfile 0 (%END-OF-DATA) /SubFileDecode filter pop\n { ( not PostScript\n%END-OF-DATA\n"; + let doc = eps(format!("{aliases}{data}{square}").as_bytes()); + assert!(coverage(&render(&doc), 50, 25, 150, 75) > 4900.0, "drawing after the embedded data"); +} + #[test] fn eps_text_with_bundled_fonts_and_show_variants() { // Not embedded: drawn with the bundled stand-in, like the same text in a PDF. From 49ec37ec3f1d5a1844b628652fecc63f76d6cbe5 Mon Sep 17 00:00:00 2001 From: ded Date: Tue, 20 Oct 2026 16:40:24 +0300 Subject: [PATCH 08/37] Disk cache: an edit that leaves the footage alone keeps its salt (#1926) Session::bump took the disk cache's footage salt at every edit, which reads the size and time of every footage file, each frame of an image sequence included. With three 5000-frame sequences a caret move or a typed character in a text field took ~0.6 s on the UI thread (9 ms with the disk cache off). The salt is now taken again only when the footage items changed, or when a project is opened, footage is reloaded or the disk cache attached. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/footage_reload.rs | 2 ++ crates/engine/src/lib.rs | 41 +++++++++++++++++++++++++-- crates/engine/src/session_settings.rs | 6 ++-- crates/engine/src/tests_sequence.rs | 28 ++++++++++++++++++ 4 files changed, 72 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/footage_reload.rs b/crates/engine/src/footage_reload.rs index fa12b27a..4d22ce0e 100644 --- a/crates/engine/src/footage_reload.rs +++ b/crates/engine/src/footage_reload.rs @@ -193,6 +193,8 @@ fn forget(s: &mut Session, paths: &BTreeSet) { s.footage.forget(p); } s.layer_cache.clear(); + // Reloaded files may keep their footage settings but not their size and time. + s.refresh_disk_salt(true); s.events.push(crate::Event::PurgeCaches); } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index cdfc7468..ff40d5e6 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -521,6 +521,8 @@ pub struct Session { pub footage_stamps: std::collections::HashMap, /// The stamps being taken on a worker thread ([`footage_reload::start_scan`]). pub footage_scan: Option, + /// The disk cache's footage salt and the footage it was taken for ([`Session::refresh_disk_salt`]). + pub disk_salt: Option<(u64, u64)>, /// How many script-issued commands are running ([`Session::execute_for_script`]): while one /// is, settings that guard what scripts may do can't change (see /// [`prefs::Scripting::same_permissions`]). @@ -618,6 +620,7 @@ impl Default for Session { check_footage_on_open: false, stored_paths: Default::default(), footage_stamps: Default::default(), + disk_salt: None, footage_scan: None, script_calls: ScriptCalls::default(), } @@ -736,12 +739,42 @@ impl Session { if Arc::ptr_eq(&self.project, &self.saved_project) { self.saved_revision = self.revision; } - if self.disk_cache.is_some() { - self.layer_cache.set_disk_salt(effectcraft_render::disk_cache::footage_salt(&self.project)); - } + self.refresh_disk_salt(false); self.events.push(Event::ProjectChanged { revision: self.revision }); } + /// Salt the disk cache's layer keys with the project's footage files + /// ([`effectcraft_render::disk_cache::footage_salt`]). Taking it reads the size and time of + /// every file, every frame of an image sequence included, so it is taken again only when the + /// footage items changed, or with `force` (a project opened, footage reloaded, the disk cache + /// attached), not at every edit: with a few large sequences that cost each keystroke in a + /// text field over half a second (#1926). + pub fn refresh_disk_salt(&mut self, force: bool) -> Option { + use std::hash::{Hash, Hasher}; + if self.disk_cache.is_none() { + self.disk_salt = None; + return None; + } + let mut h = std::collections::hash_map::DefaultHasher::new(); + for (id, it) in &self.project.items { + if let effectcraft_project::ItemKind::Footage(f) = &it.kind { + id.hash(&mut h); + format!("{f:?}").hash(&mut h); + } + } + let footage = h.finish(); + if let Some((seen, salt)) = self.disk_salt + && seen == footage + && !force + { + return Some(salt); + } + let salt = effectcraft_render::disk_cache::footage_salt(&self.project); + self.disk_salt = Some((footage, salt)); + self.layer_cache.set_disk_salt(salt); + Some(salt) + } + /// Set a comp's Resolution. As in After Effects it is saved with the project (which becomes /// modified) but is no undo step, and it changes no pixels: the comp's cached frames stay /// valid ([`effectcraft_project::Comp::same_pixels`]). @@ -1169,6 +1202,8 @@ impl Session { } self.layer_cache.clear(); self.project = Arc::new(p); + // Its files may have changed since they were last stamped. + self.disk_salt = None; self.history = History::default(); self.state = EditorState { snapping: true, view_layout: 1, ..Default::default() }; self.path = path; diff --git a/crates/engine/src/session_settings.rs b/crates/engine/src/session_settings.rs index ef3e748a..209dfeb6 100644 --- a/crates/engine/src/session_settings.rs +++ b/crates/engine/src/session_settings.rs @@ -102,7 +102,7 @@ impl Session { /// and attach it to the layer cache. Without a folder setting the platform cache folder is /// used, but only by frontends with a settings store (headless sessions stay off disk). pub fn configure_disk_cache(&mut self) { - use effectcraft_render::disk_cache::{DiskCache, default_folder, footage_salt}; + use effectcraft_render::disk_cache::{DiskCache, default_folder}; let d = &self.prefs.disk; let folder = if d.disk_cache_folder.trim().is_empty() { (self.config.is_some() && !cfg!(target_arch = "wasm32")).then(default_folder) @@ -131,7 +131,9 @@ impl Session { } }, }; - self.layer_cache.set_disk(Some((dc.clone(), footage_salt(&self.project)))); + self.disk_cache = Some(dc.clone()); + let salt = self.refresh_disk_salt(true).unwrap_or_default(); + self.layer_cache.set_disk(Some((dc.clone(), salt))); // Roto Brush / Object Matte segmentations persist there too (held weakly: switching the // disk cache off drops it). let store: std::sync::Arc = dc.clone(); diff --git a/crates/engine/src/tests_sequence.rs b/crates/engine/src/tests_sequence.rs index 7f937596..018e4327 100644 --- a/crates/engine/src/tests_sequence.rs +++ b/crates/engine/src/tests_sequence.rs @@ -207,3 +207,31 @@ fn dpx_and_cineon_runs_import_as_sequences() { got.sort(); assert_eq!(got, [("plate.[0001-0003].dpx".to_string(), 3), ("scan_[01-02].cin".to_string(), 2)], "{r}"); } + +/// #1926: an edit that leaves the footage alone doesn't stamp every frame of its image sequences +/// again for the disk cache's salt (with large sequences each keystroke in a text field took over +/// half a second); Reload Footage does, and so does new footage. +#[test] +fn edits_keep_the_disk_salt_and_reload_stamps_the_files_again() { + let dir = folder(&RUN); + let cache = std::env::temp_dir().join(format!("ec-seq-salt-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&cache); + let mut s = session(); + s.execute_checked("prefs.set", json!({"values": {"disk.diskCacheFolder": cache.to_string_lossy(), "disk.diskCacheEnabled": true}})).unwrap(); + s.execute("comp.new", json!({"name": "C", "width": 64, "height": 32, "frameRate": 30, "duration": 1})).unwrap(); + let r = s.execute("file.import", json!({"paths": [at(&dir, "shot_0002.png")], "sequence": true, "addToComp": true})).unwrap(); + let item = r["items"][0].clone(); + let salt = s.refresh_disk_salt(false).expect("a disk cache"); + // A frame rewritten in place: same footage settings, another size on disk. + std::fs::write(at(&dir, "shot_0004.png"), b"a new render of this frame").unwrap(); + s.execute("layer.newText", json!({"text": "title"})).unwrap(); + s.execute("layer.setText", json!({"text": "title 2"})).unwrap(); + assert_eq!(s.refresh_disk_salt(false), Some(salt), "edits don't stamp the files again"); + s.execute("file.reloadFootage", json!({"items": [item]})).unwrap(); + let reloaded = s.refresh_disk_salt(false).unwrap(); + assert_ne!(reloaded, salt, "Reload Footage stamps them again"); + std::fs::write(at(&dir, "extra.png"), b"").unwrap(); + s.execute("file.import", json!({"paths": [at(&dir, "extra.png")]})).unwrap(); + assert_ne!(s.refresh_disk_salt(false), Some(reloaded), "new footage counts"); + let _ = std::fs::remove_dir_all(&cache); +} From 3878fdf0ba7826337d7874e08e8f557013a947f0 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 20 Oct 2026 18:46:10 +0200 Subject: [PATCH 09/37] Make Gaussian Blur and Fast Box Blur continuous at small values (#1939) --- crates/effects/src/misc.rs | 68 +++++++++++++++++++++++++++++--- crates/effects/src/obsolete.rs | 18 ++++++++- crates/gpu/src/effects.rs | 11 +++++- crates/gpu/src/fx_extra.rs | 4 ++ crates/gpu/src/tests.rs | 23 +++++++++-- crates/gpu/src/tests_fx_extra.rs | 5 ++- crates/raster/src/blur.rs | 47 +++++++++++++++++++++- crates/raster/src/lib.rs | 2 +- 8 files changed, 162 insertions(+), 16 deletions(-) diff --git a/crates/effects/src/misc.rs b/crates/effects/src/misc.rs index c119118d..7468c6b8 100644 --- a/crates/effects/src/misc.rs +++ b/crates/effects/src/misc.rs @@ -35,22 +35,44 @@ fn gaussian(ctx: &EffectCtx, mut b: Buf) -> Buf { if !repeat && !ctx.adjustment { b.pad((s * 3.0).ceil() as u32); } - b.img = gaussian_blur(&b.img, s * kx, s * ky, repeat || ctx.adjustment); + let (sx, sy) = (s * kx, s * ky); + // Small sigmas: integer-radius box passes would step (and do nothing below ~0.6 px), so sample the kernel (#1939). + b.img = if effectcraft_raster::gaussian_is_exact(sx, sy) { + effectcraft_raster::gaussian_blur_exact(&b.img, sx, sy, repeat || ctx.adjustment) + } else { + gaussian_blur(&b.img, sx, sy, repeat || ctx.adjustment) + }; b } fn box_blur(ctx: &EffectCtx, mut b: Buf) -> Buf { - let r = (ctx.params.f("radius") * b.scale).round().max(0.0) as usize; - if r == 0 { + let radius = (ctx.params.f("radius") * b.scale).max(0.0); + let r = radius.floor() as usize; + let frac = (radius - r as f64) as f32; + // Fractional radius: blend the two neighbouring integer radii so the result changes continuously (#1939). + let frac = if frac < 1e-4 { 0.0 } else { frac }; + if r == 0 && frac == 0.0 { return b; } let it = ctx.params.f("iterations").round().clamp(1.0, 50.0) as usize; let (kx, ky) = dims_xy(ctx.params.e("dimensions")); let repeat = ctx.params.b("repeatEdge"); if !repeat && !ctx.adjustment { - b.pad((r * it) as u32 + 1); + let reach = if frac > 0.0 { r + 1 } else { r }; + b.pad((reach * it) as u32 + 1); + } + let edge = repeat || ctx.adjustment; + let lo = effectcraft_raster::box_blur(&b.img, r * kx as usize, r * ky as usize, it, edge); + if frac == 0.0 { + b.img = lo; + return b; + } + let hi = effectcraft_raster::box_blur(&b.img, (r + 1) * kx as usize, (r + 1) * ky as usize, it, edge); + for ((l, h), o) in lo.data.iter().zip(hi.data.iter()).zip(b.img.data.iter_mut()) { + for c in 0..4 { + o[c] = l[c] + (h[c] - l[c]) * frac; + } } - b.img = effectcraft_raster::box_blur(&b.img, r * kx as usize, r * ky as usize, it, repeat || ctx.adjustment); b } @@ -738,6 +760,42 @@ mod tests { use super::*; use crate::{EffectEnv, run_fx}; + /// White left half, black right half (one vertical edge at x = 20). + fn edge() -> effectcraft_raster::Image { + let mut img = effectcraft_raster::Image::filled(40, 8, [0.0, 0.0, 0.0, 1.0]); + for y in 0..8 { + for x in 0..20 { + img.set(x, y, [1.0, 1.0, 1.0, 1.0]); + } + } + img + } + + fn blurred_row(id: &str, param: &str, v: f64) -> Vec { + let out = run_fx(id, &[(param, num(v)), ("repeatEdge", Value::Bool(true))], edge(), 0.0, EffectEnv::default()); + (16..24).map(|x| out.img.get(x, 4)[0]).collect() + } + + #[test] + fn small_gaussian_blurriness_is_continuous() { + // #1939: 0.9 used to be identical to 0, and 1.2 / 1.6 / 2.0 identical to each other. + let row = |v| blurred_row("ec.blur.gaussian", "blurriness", v); + assert_ne!(row(0.0), row(0.9)); + assert_ne!(row(0.9), row(1.2)); + assert_ne!(row(1.2), row(1.6)); + assert_ne!(row(1.6), row(2.0)); + assert_ne!(row(2.0), row(2.4)); + } + + #[test] + fn fractional_box_blur_radius_is_continuous() { + let row = |v| blurred_row("ec.blur.fastbox", "radius", v); + assert_ne!(row(0.0), row(0.5)); + assert_ne!(row(0.5), row(0.9)); + assert_ne!(row(1.2), row(1.6)); + assert_ne!(row(1.6), row(2.4)); + } + /// A bright square on mid grey. fn scene() -> effectcraft_raster::Image { let mut img = effectcraft_raster::Image::filled(24, 24, [0.3, 0.3, 0.3, 1.0]); diff --git a/crates/effects/src/obsolete.rs b/crates/effects/src/obsolete.rs index c58f5ca6..a2b161b3 100644 --- a/crates/effects/src/obsolete.rs +++ b/crates/effects/src/obsolete.rs @@ -99,7 +99,13 @@ fn gaussian_legacy(ctx: &EffectCtx, mut b: Buf) -> Buf { if !ctx.adjustment { b.pad((s * 3.0).ceil() as u32); } - b.img = gaussian_blur(&b.img, s * kx, s * ky, ctx.adjustment); + let (sx, sy) = (s * kx, s * ky); + // Small sigmas use the sampled kernel, like Gaussian Blur (#1939). + b.img = if effectcraft_raster::gaussian_is_exact(sx, sy) { + effectcraft_raster::gaussian_blur_exact(&b.img, sx, sy, ctx.adjustment) + } else { + gaussian_blur(&b.img, sx, sy, ctx.adjustment) + }; b } @@ -175,6 +181,16 @@ mod tests { assert!(out.img.get(c as i64, c as i64)[3] < 0.5); } + #[test] + fn gaussian_legacy_small_blurriness_blurs() { + // #1939: Blurriness under ~1.2 used to be a no-op. + let mut img = Image::new(15, 15); + img.set(7, 7, [1.0, 1.0, 1.0, 1.0]); + let out = run_fx("ec.obsolete.gaussianlegacy", &[("blurriness", num(0.9))], img, 0.0, EffectEnv::default()); + let c = out.img.width / 2; + assert!(out.img.get(c as i64, c as i64)[3] < 0.9); + } + #[test] fn moved_effects_are_obsolete() { for id in ["ec.key.luma", "ec.key.spill", "ec.blur.reduceflicker"] { diff --git a/crates/gpu/src/effects.rs b/crates/gpu/src/effects.rs index 42d2bab7..09aec69d 100644 --- a/crates/gpu/src/effects.rs +++ b/crates/gpu/src/effects.rs @@ -261,6 +261,10 @@ fn gaussian(e: &mut Enc, ctx: &EffectCtx, mut b: GBuf) -> Option { return Some(b); } let (kx, ky) = dims_xy(ctx.params.e("dimensions")); + // Small sigmas use the CPU's sampled kernel (#1939). + if effectcraft_raster::gaussian_is_exact(s * kx, s * ky) { + return None; + } let repeat = ctx.params.b("repeatEdge"); if !repeat && !ctx.adjustment { b.pad(e, (s * 3.0).ceil() as u32)?; @@ -270,7 +274,12 @@ fn gaussian(e: &mut Enc, ctx: &EffectCtx, mut b: GBuf) -> Option { } fn box_blur(e: &mut Enc, ctx: &EffectCtx, mut b: GBuf) -> Option { - let r = (ctx.params.f("radius") * b.scale).round().max(0.0) as usize; + let radius = (ctx.params.f("radius") * b.scale).max(0.0); + // Fractional radii blend two integer radii on the CPU (#1939). + if radius - radius.floor() >= 1e-4 { + return None; + } + let r = radius.floor() as usize; if r == 0 { return Some(b); } diff --git a/crates/gpu/src/fx_extra.rs b/crates/gpu/src/fx_extra.rs index 2e8bb85b..cff51630 100644 --- a/crates/gpu/src/fx_extra.rs +++ b/crates/gpu/src/fx_extra.rs @@ -188,6 +188,10 @@ fn gaussian_legacy(e: &mut Enc, ctx: &EffectCtx, mut b: GBuf) -> Option { 2 => (0.0, 1.0), _ => (1.0, 1.0), }; + // Small sigmas use the CPU's sampled kernel (#1939). + if effectcraft_raster::gaussian_is_exact(s * kx, s * ky) { + return None; + } if !ctx.adjustment { b.pad(e, (s * 3.0).ceil() as u32)?; } diff --git a/crates/gpu/src/tests.rs b/crates/gpu/src/tests.rs index 24f8c961..de1a20df 100644 --- a/crates/gpu/src/tests.rs +++ b/crates/gpu/src/tests.rs @@ -491,12 +491,27 @@ pub(crate) fn c(r: f64, g: f64, b: f64) -> Value { Value::Color([r, g, b, 1.0]) } +#[test] +fn small_blurs_fall_back_to_the_cpu() { + // #1939: small Gaussian sigmas and fractional box radii render on the CPU kernel. + for (id, vals) in [("ec.blur.gaussian", [("blurriness", n(0.9))]), ("ec.blur.fastbox", [("radius", n(1.5))])] { + let mut s = Scene::new(BitDepth::Bpc32); + let bg = s.solid([0.15, 0.1, 0.2], 97, 61); + s.push(bg); + let mut l = s.footage(70, 44); + s.effect(&mut l, id, &vals); + s.push(l); + check(&format!("{id} {vals:?}"), compare_at(&s, opts(), Tick::ZERO), 0.0); + } +} + #[test] fn gpu_effects_match_cpu() { - effect_case("ec.blur.gaussian", &[("blurriness", n(9.0))]); - effect_case("ec.blur.gaussian", &[("blurriness", n(4.0)), ("dimensions", Value::Enum(1)), ("repeatEdge", Value::Bool(true))]); - effect_case("ec.blur.fastbox", &[("radius", n(5.0)), ("iterations", n(2.0))]); - effect_case("ec.blur.fastbox", &[("radius", n(3.0)), ("dimensions", Value::Enum(2)), ("repeatEdge", Value::Bool(true))]); + // Sigma 3 px and up and even radii, so the half-scale pass stays on the GPU too (#1939). + effect_case("ec.blur.gaussian", &[("blurriness", n(14.0))]); + effect_case("ec.blur.gaussian", &[("blurriness", n(12.0)), ("dimensions", Value::Enum(1)), ("repeatEdge", Value::Bool(true))]); + effect_case("ec.blur.fastbox", &[("radius", n(6.0)), ("iterations", n(2.0))]); + effect_case("ec.blur.fastbox", &[("radius", n(4.0)), ("dimensions", Value::Enum(2)), ("repeatEdge", Value::Bool(true))]); effect_case("ec.blur.directional", &[("direction", n(30.0)), ("length", n(12.0))]); effect_case("ec.stylize.glow", &[("threshold", n(40.0)), ("radius", n(14.0)), ("intensity", n(1.5))]); effect_case("ec.stylize.glow", &[("colors", Value::Enum(1)), ("operation", Value::Enum(1)), ("colorA", c(1.0, 0.8, 0.1))]); diff --git a/crates/gpu/src/tests_fx_extra.rs b/crates/gpu/src/tests_fx_extra.rs index b7fe6cbd..136a66d2 100644 --- a/crates/gpu/src/tests_fx_extra.rs +++ b/crates/gpu/src/tests_fx_extra.rs @@ -46,6 +46,7 @@ fn bevels_and_legacy_gaussian() { effect_case("ec.perspective.bevelalpha", &[("edgeThickness", n(2.0)), ("lightAngle", n(200.0)), ("lightColor", c(1.0, 0.8, 0.5))]); effect_case("ec.perspective.beveledges", &[("edgeThickness", n(0.1)), ("lightIntensity", n(0.5))]); effect_case("ec.perspective.beveledges", &[("edgeThickness", n(0.3)), ("lightAngle", n(130.0))]); - effect_case("ec.obsolete.gaussianlegacy", &[("blurriness", n(7.0))]); - effect_case("ec.obsolete.gaussianlegacy", &[("blurriness", n(5.0)), ("blurDimensions", e(2))]); + // Sigma 3 px and up at half scale too: smaller ones render on the CPU (#1939). + effect_case("ec.obsolete.gaussianlegacy", &[("blurriness", n(14.0))]); + effect_case("ec.obsolete.gaussianlegacy", &[("blurriness", n(12.0)), ("blurDimensions", e(2))]); } diff --git a/crates/raster/src/blur.rs b/crates/raster/src/blur.rs index 1c065d77..fcafee5d 100644 --- a/crates/raster/src/blur.rs +++ b/crates/raster/src/blur.rs @@ -1,5 +1,5 @@ -//! Blurs: separable box passes (exact running sums) approximating Gaussians, plus directional -//! and radial (spin/zoom) blurs. +//! Blurs: separable box passes (exact running sums) approximating Gaussians, a sampled Gaussian +//! kernel for small sigmas, plus directional and radial (spin/zoom) blurs. //! //! Box passes run straight on row-major pixels: horizontal passes per row, vertical passes over //! blocks of rows with a per-column running sum (no transposes), ping-ponging between two @@ -157,6 +157,49 @@ pub fn gaussian_blur(img: &Image, sigma_x: f64, sigma_y: f64, repeat: bool) -> I box_passes(img, &rx, &ry, repeat) } +/// Below this sigma (pixels) three integer-radius box passes are too coarse (sigma under ~0.6 is +/// a no-op and the result steps between a few shapes), so [`gaussian_blur_exact`] is used instead. +const EXACT_SIGMA_BELOW: f64 = 3.0; + +/// True when a Gaussian of these per-axis sigmas should use [`gaussian_blur_exact`]. +pub fn gaussian_is_exact(sigma_x: f64, sigma_y: f64) -> bool { + sigma_x.max(sigma_y) < EXACT_SIGMA_BELOW +} + +/// One separable pass of a sampled Gaussian (radius 3 sigma) along x or y. +fn gauss_pass(img: &Image, sigma: f64, vertical: bool, repeat: bool) -> Image { + if sigma <= 0.0 || img.is_empty() { + return img.clone(); + } + let r = (sigma * 3.0).ceil() as i64; + let mut k: Vec = (-r..=r).map(|i| (-((i * i) as f64) / (2.0 * sigma * sigma)).exp()).collect(); + let total: f64 = k.iter().sum(); + k.iter_mut().for_each(|v| *v /= total); + let w = img.width as usize; + let mut out = Image::new(img.width, img.height); + out.rows_mut().for_each(|(y, row)| { + for x in 0..w { + let mut acc = [0.0f64; 4]; + for (i, wt) in k.iter().enumerate() { + let d = i as i64 - r; + let (sx, sy) = if vertical { (x as i64, y as i64 + d) } else { (x as i64 + d, y as i64) }; + let p = if repeat { img.get_clamped(sx, sy) } else { img.get(sx, sy) }; + for c in 0..4 { + acc[c] += p[c] as f64 * wt; + } + } + row[x] = acc.map(|v| v as f32); + } + }); + out +} + +/// Gaussian blur as a true sampled kernel, continuous in sigma (for small sigmas, see +/// [`gaussian_is_exact`]). +pub fn gaussian_blur_exact(img: &Image, sigma_x: f64, sigma_y: f64, repeat: bool) -> Image { + gauss_pass(&gauss_pass(img, sigma_x, false, repeat), sigma_y, true, repeat) +} + /// Motion blur along `angle_deg` (0 = vertical in AE's Directional Blur, measured clockwise from /// up) over `length` pixels. pub fn directional_blur(img: &Image, angle_deg: f64, length: f64) -> Image { diff --git a/crates/raster/src/lib.rs b/crates/raster/src/lib.rs index f1760282..dce8dda8 100644 --- a/crates/raster/src/lib.rs +++ b/crates/raster/src/lib.rs @@ -16,7 +16,7 @@ pub mod warp; use effectcraft_color::{BlendMode, blend_pixel}; use rayon::prelude::*; -pub use blur::{box_blur, directional_blur, gaussian_blur, radial_blur}; +pub use blur::{box_blur, directional_blur, gaussian_blur, gaussian_blur_exact, gaussian_is_exact, radial_blur}; pub use channels3d::AuxChannels; pub use warp::{Sampling, WarpOpts, accumulate_warp, composite_warp, composite_warp_reference, resample}; From 89d954a10b72660baaf65cd005f7578ccf7deaeb Mon Sep 17 00:00:00 2001 From: Camille Moine Date: Wed, 21 Oct 2026 13:06:40 +0200 Subject: [PATCH 10/37] fix(effects): calibrate CC Blobbylize to the AE oracle Use the measured Softness kernel (sigma = 0.42 * Softness), threshold the blurred height with a ~1 px antialias, and allow Cut Away 100 to empty the matte. Split the source-matte displacement from the height-field RGB refraction: the matte uses a z=1 bevel normal fitted to the T2 step edges, while RGB follows the measured z=height refraction. Align the documented AE defaults and add CPU regression tests for the cut threshold, Cut Away 100, and the inward matte warp. Update the GPU kernel and parameters to match, keeping CPU/GPU parity. --- crates/effects/src/distort3.rs | 150 ++++++++++++++++++---- crates/gpu/src/fx_transition.rs | 6 +- crates/gpu/src/shaders/fx_transition.wgsl | 47 +++++-- 3 files changed, 159 insertions(+), 44 deletions(-) diff --git a/crates/effects/src/distort3.rs b/crates/effects/src/distort3.rs index 99139483..ca6e5ba8 100644 --- a/crates/effects/src/distort3.rs +++ b/crates/effects/src/distort3.rs @@ -107,7 +107,14 @@ fn norm3(v: [f64; 3]) -> [f64; 3] { [v[0] / l, v[1] / l, v[2] / l] } -fn dot3(a: [f64; 3], b: [f64; 3]) -> f64 { +/// f32 normal/dot helpers for the CC Blobbylize lighting pass. The GPU kernel runs in f32; +/// matching its precision here keeps specular highlights within the CPU/GPU parity tolerance. +fn norm3f(v: [f32; 3]) -> [f32; 3] { + let l = (v[0] * v[0] + v[1] * v[1] + v[2] * v[2]).sqrt().max(1e-12); + [v[0] / l, v[1] / l, v[2] / l] +} + +fn dot3f(a: [f32; 3], b: [f32; 3]) -> f32 { a[0] * b[0] + a[1] * b[1] + a[2] * b[2] } @@ -115,14 +122,17 @@ fn dot3(a: [f64; 3], b: [f64; 3]) -> f64 { fn cc_blobbylize(ctx: &EffectCtx, mut b: Buf) -> Buf { let soft = ctx.params.f("softness").max(0.0) * b.scale; - let cut = (ctx.params.f("cutAway") / 100.0).clamp(0.0, 0.99) as f32; + let cut = (ctx.params.f("cutAway") / 100.0).clamp(0.0, 1.0) as f32; let blob = layer_or_self(ctx, &b, "blobLayer", true, false); let prop = src_at(ctx.params.e("property")); let raw = Plane::from_image(&blob, |px| { let (c, a) = unpremul(px); pick(prop, c, a) * if matches!(prop, crate::util::Src::Alpha) { 1.0 } else { a } }); - let h = gauss_plane(&raw, soft, soft); + // Oracle (AE 26.2, 16 bpc): a step thresholded at Cut 25/75 with Softness 20 moves the + // 50% contour by 5.55 px. For a Gaussian that is 0.67449 sigma, so sigma = 0.42 Softness. + // The same probe shows the matte stays a ~1 px antialiased threshold, not a broad ramp. + let h = gauss_plane(&raw, soft * 0.42, soft * 0.42); let intensity = ctx.params.f("lightIntensity") / 100.0; let lc = ctx.params.color("lightColor"); let point = ctx.params.e("lightType") == 1; @@ -135,37 +145,69 @@ fn cc_blobbylize(ctx: &EffectCtx, mut b: Buf) -> Buf { let rough = ctx.params.f("roughness").clamp(0.001, 1.0); let metal = (ctx.params.f("metal") / 100.0) as f32; let shininess = (1.0 / rough).min(500.0); - let depth = (soft * 0.5).max(1.0); let src = b.img.clone(); let (w, hh) = (b.img.width as usize, b.img.height as usize); let span = (w.max(hh) as f64).max(1.0); + + // The source matte and the RGB refraction are separate in the oracle: + // - the matte edge of a self step moves 5/9/14/18 px at Softness 5/10/20/40. A normal + // with z = 1, a ~30 px depth at 1920 and a slope scale of 1.5 Softness reproduces that; + // - RGB follows a height-field refraction with z = height: on a linear ramp the measured + // horizontal offset is dx = -0.34 / h, independent of Softness. + let matte_slope = soft * 1.5; + let matte_depth = if soft > 0.05 { span / 64.0 } else { 0.0 }; + let rgb_depth = span * 0.34 * b.scale; + let light_slope = (soft * 0.65).max(1.0); + let limit = span * 0.5; b.img.rows_mut().for_each(|(y, row)| { for (x, px) in row.iter_mut().enumerate() { let (xi, yi) = (x as i64, y as i64); let hv = h.get(x, y); let gx = (h.get_clamped(xi + 1, yi) - h.get_clamped(xi - 1, yi)) as f64 * 0.5; let gy = (h.get_clamped(xi, yi + 1) - h.get_clamped(xi, yi - 1)) as f64 * 0.5; - // Refraction-like displacement down the slope, then the blobby matte. - let sx = x as f64 + 0.5 - gx * depth * 2.0; - let sy = y as f64 + 0.5 - gy * depth * 2.0; - let s = src.sample_bilinear(sx, sy); - let (c, a) = unpremul(s); - let m = ((hv - cut) / ((1.0 - cut) * 0.5).max(1e-3)).clamp(0.0, 1.0); - let alpha = a.max(s[3]) * m; + let grad = (gx * gx + gy * gy).sqrt(); + // One-pixel antialiased threshold. A flat field is either fully in or fully out. + let aa = (1.25 * grad).max(5e-4); + let m = if grad > 1e-9 { + (0.5 + (hv - cut) as f64 / aa).clamp(0.0, 1.0) as f32 + } else if hv > cut { + 1.0 + } else { + 0.0 + }; + if m <= 0.0 { + *px = [0.0; 4]; + continue; + } + // Warp the source matte with the softness-scaled bevel normal (z = 1). + let na = norm3([-gx * matte_slope, -gy * matte_slope, 1.0]); + let sa = src.sample_bilinear(x as f64 + 0.5 + na[0] * matte_depth, y as f64 + 0.5 + na[1] * matte_depth); + let alpha = sa[3].max(0.0) * m; if alpha <= 0.0 { *px = [0.0; 4]; continue; } - let n = norm3([-gx * depth, -gy * depth, 1.0]); - let l = if point { norm3([lpos.0 - x as f64, lpos.1 - y as f64, lheight / 100.0 * span]) } else { ldir }; - let ndl = dot3(n, l).max(0.0); - let hv3 = norm3([l[0], l[1], l[2] + 1.0]); - let sp = dot3(n, hv3).max(0.0).powf(shininess) * specular; + // Refract the colour through the height field (z = height for this pass). + let hz = (hv as f64).max(1e-3); + let rx = (-rgb_depth * gx / hz).clamp(-limit, limit); + let ry = (-rgb_depth * gy / hz).clamp(-limit, limit); + let srgb = src.sample_bilinear(x as f64 + 0.5 + rx, y as f64 + 0.5 + ry); + let (c, _) = unpremul(srgb); + let (gx32, gy32) = (gx as f32, gy as f32); + let n = norm3f([-gx32 * light_slope as f32, -gy32 * light_slope as f32, 1.0]); + let l = if point { + norm3f([lpos.0 as f32 - (x as f32 + 0.5), lpos.1 as f32 - (y as f32 + 0.5), (lheight / 100.0 * span) as f32]) + } else { + [ldir[0] as f32, ldir[1] as f32, ldir[2] as f32] + }; + let ndl = dot3f(n, l).max(0.0); + let hv3 = norm3f([l[0], l[1], l[2] + 1.0]); + let sp = dot3f(n, hv3).max(0.0).powf(shininess as f32) * specular as f32; let mut out = [0.0f32; 3]; for i in 0..3 { - let lit = (ambient + diffuse * ndl * intensity) as f32 * lc[i]; + let lit = (ambient + diffuse * ndl as f64 * intensity) as f32 * lc[i]; let spec_col = (1.0 - metal) * lc[i] + metal * c[i] * lc[i]; - out[i] = c[i] * lit + (sp * intensity) as f32 * spec_col; + out[i] = c[i] * lit + (sp as f64 * intensity) as f32 * spec_col; } *px = premul(out, alpha); } @@ -880,19 +922,19 @@ pub fn specs() -> Vec { "CC Blobbylize", vec![ p("blobLayer", "Blob Layer", Value::Layer(None), ParamUi::Layer), - p("property", "Property", Value::Enum(3), popup(&crate::util::SRC_NAMES[..8])), - p("softness", "Softness", num(6.0), slider(0.0, 200.0, 0.0, 50.0, 1)), - p("cutAway", "Cut Away", num(0.0), pct()), - p("lightIntensity", "Light Intensity", num(100.0), slider(0.0, 400.0, 0.0, 200.0, 1)), + p("property", "Property", Value::Enum(0), popup(&crate::util::SRC_NAMES[..8])), + p("softness", "Softness", num(20.0), slider(0.0, 500.0, 0.0, 50.0, 1)), + p("cutAway", "Cut Away", num(25.0), pct()), + p("lightIntensity", "Light Intensity", num(100.0), slider(0.0, 1000.0, 0.0, 200.0, 1)), p("lightColor", "Light Color", col(1.0, 1.0, 1.0), ParamUi::Color), p("lightType", "Light Type", Value::Enum(0), popup(&["Distant Light", "Point Light"])), - p("lightHeight", "Light Height", num(50.0), slider(0.0, 100.0, 0.0, 100.0, 1)), - p("lightPosition", "Light Position", pt(0.25, 0.25), ParamUi::Point), + p("lightHeight", "Light Height", num(65.0), slider(0.0, 100.0, 0.0, 100.0, 1)), + p("lightPosition", "Light Position", pt(0.5, 0.5), ParamUi::Point), p("lightDirection", "Light Direction", num(-45.0), ParamUi::Angle), - p("ambient", "Ambient", num(20.0), slider(0.0, 200.0, 0.0, 100.0, 1)), - p("diffuse", "Diffuse", num(70.0), slider(0.0, 200.0, 0.0, 100.0, 1)), - p("specular", "Specular", num(40.0), slider(0.0, 200.0, 0.0, 100.0, 1)), - p("roughness", "Roughness", num(0.05), slider(0.001, 0.5, 0.001, 0.5, 3)), + p("ambient", "Ambient", num(75.0), slider(0.0, 200.0, 0.0, 100.0, 1)), + p("diffuse", "Diffuse", num(25.0), slider(0.0, 200.0, 0.0, 100.0, 1)), + p("specular", "Specular", num(100.0), slider(0.0, 200.0, 0.0, 100.0, 1)), + p("roughness", "Roughness", num(0.025), slider(0.001, 1.0, 0.001, 1.0, 3)), p("metal", "Metal", num(100.0), pct()), ], cc_blobbylize, @@ -1127,6 +1169,58 @@ mod tests { } } + #[test] + fn blobbylize_cut_away_thresholds_a_height_ramp() { + // AE oracle: Cut Away is a linear height threshold with a ~1 px antialiased edge. + let im = crate::util::gen_image(1920, 8, |x, _| { + let h = x as f32 / 1920.0; + [h, h, h, 1.0] + }); + let o = run("ec.distort.ccblobbylize", &[("property", Value::Enum(0)), ("softness", num(0.0)), ("cutAway", num(25.0))], im, &[]); + assert_eq!(o.img.get(400, 4)[3], 0.0, "below Cut Away must be transparent"); + assert_eq!(o.img.get(560, 4)[3], 1.0, "above Cut Away must be opaque"); + let edge = o.img.get(480, 4)[3]; + assert!((0.45..=0.55).contains(&edge), "edge alpha {edge} is not a one-pixel threshold"); + let partial = (450..510).filter(|&x| (0.001..0.999).contains(&o.img.get(x, 4)[3])).count(); + assert!(partial <= 2, "{partial} antialiased columns instead of one"); + } + + #[test] + fn blobbylize_cut_away_100_is_empty() { + // Regression: the old clamp to 0.99 left the top of the ramp opaque at Cut Away 100. + let im = crate::util::gen_image(1920, 8, |x, _| { + let h = x as f32 / 1920.0; + [h, h, h, 1.0] + }); + let o = run("ec.distort.ccblobbylize", &[("property", Value::Enum(0)), ("softness", num(0.0)), ("cutAway", num(100.0))], im, &[]); + assert!(o.img.data.iter().all(|p| p[3] == 0.0), "Cut Away 100 left opaque pixels"); + } + + #[test] + fn blobbylize_softness_warps_a_step_matte_inward() { + // AE oracle T2: a self alpha step at x = 960, Softness 20, Cut Away 0 puts the matte + // edge at about x = 946, not at the source edge. The PR's matte-preserving version + // left it at 960; the effect must warp the source matte. + let im = crate::util::gen_image(1920, 32, |x, _| if x < 960 { [1.0; 4] } else { [0.0; 4] }); + let o = run( + "ec.distort.ccblobbylize", + &[ + ("property", Value::Enum(3)), + ("softness", num(20.0)), + ("cutAway", num(0.0)), + ("ambient", num(100.0)), + ("diffuse", num(0.0)), + ("specular", num(0.0)), + ("metal", num(0.0)), + ], + im, + &[], + ); + assert!(o.img.get(945, 16)[3] > 0.95, "the matte did not move inward: {}", o.img.get(945, 16)[3]); + assert!(o.img.get(949, 16)[3] < 0.05, "source alpha leaked past the warped edge"); + assert_eq!(o.img.get(960, 16)[3], 0.0, "the source matte was preserved instead of warped"); + } + /// A text layer's buffer: 20 × 10 opaque pixels at (90, 45)–(110, 55) of its comp-sized /// (200 × 100) layer bounds, after `id` with `set`. fn on_text(id: &str, set: &[(&str, Value)]) -> Buf { diff --git a/crates/gpu/src/fx_transition.rs b/crates/gpu/src/fx_transition.rs index a0ad5465..60378491 100644 --- a/crates/gpu/src/fx_transition.rs +++ b/crates/gpu/src/fx_transition.rs @@ -586,17 +586,19 @@ fn blobbylize(e: &mut Enc, ctx: &EffectCtx, b: GBuf) -> Option { let soft = pr.f("softness").max(0.0) * b.scale; let blob = layer_or_self(e, ctx, &b, "blobLayer")?; let raw = plane(e, &blob, pr.e("property").min(10)); - let h = gaussian_blur(e, &raw, soft, soft, true); + let h = gaussian_blur(e, &raw, soft * 0.42, soft * 0.42, true); let lpos = b.to_px(pr.v2("lightPosition")); let lheight = pr.f("lightHeight"); let (d, el) = (pr.f("lightDirection").to_radians(), (lheight / 100.0).clamp(0.0, 1.0) * std::f64::consts::FRAC_PI_2); let ldir = [d.sin() * el.cos(), -d.cos() * el.cos(), el.sin()]; let span = (b.img.width.max(b.img.height) as f64).max(1.0); let mut p = Params::default(); - p.f[0] = f4([(soft * 0.5).max(1.0), (pr.f("cutAway") / 100.0).clamp(0.0, 0.99), pr.f("lightIntensity") / 100.0, pr.f("ambient") / 100.0]); + p.f[0] = f4([(pr.f("cutAway") / 100.0).clamp(0.0, 1.0), pr.f("lightIntensity") / 100.0, pr.f("ambient") / 100.0, soft * 1.5]); p.f[1] = f4([pr.f("diffuse") / 100.0, pr.f("specular") / 100.0, (1.0 / pr.f("roughness").clamp(0.001, 1.0)).min(500.0), pr.f("metal") / 100.0]); p.f[2] = f4([lpos.0, lpos.1, lheight / 100.0 * span, (pr.e("lightType") == 1) as u32 as f64]); p.f[3] = f4([ldir[0], ldir[1], ldir[2], 0.0]); p.f[4] = rgb(pr.color("lightColor"), 0.0); + p.f[5] = f4([if soft > 0.05 { span / 64.0 } else { 0.0 }, span * 0.34 * b.scale, (soft * 0.65).max(1.0), 1.25]); + p.f[6] = f4([span * 0.5, 0.0, 0.0, 0.0]); run(e, "ftr_blob", &p, b, Some(&h), None) } diff --git a/crates/gpu/src/shaders/fx_transition.wgsl b/crates/gpu/src/shaders/fx_transition.wgsl index 379b6446..ef543633 100644 --- a/crates/gpu/src/shaders/fx_transition.wgsl +++ b/crates/gpu/src/shaders/fx_transition.wgsl @@ -569,9 +569,11 @@ fn ftr_bender(@builtin(global_invocation_id) gid: vec3) { } // ---------------------------------------------------------------- CC Blobbylize (distort3::cc_blobbylize) -// aux = the blurred blob plane (x). f[0] = (depth, cut away, intensity, ambient); -// f[1] = (diffuse, specular, shininess, metal); f[2] = (light x, y, point light height, -// point light); f[3] = distant light direction; f[4] = light colour. +// aux = the Softness-blurred blob plane (x). f[0] = (cut away, intensity, ambient, +// matte slope); f[1] = (diffuse, specular, shininess, metal); f[2] = (light x, y, +// point light height, point); f[3] = distant light direction; f[4] = light colour; +// f[5] = (matte depth, RGB depth, light slope, matte antialias width); +// f[6].x = displacement clamp. @compute @workgroup_size(16, 16) fn ftr_blob(@builtin(global_invocation_id) gid: vec3) { @@ -579,32 +581,49 @@ fn ftr_blob(@builtin(global_invocation_id) gid: vec3) { if (!ftr_inside(p)) { return; } - let depth = P.f[0].x; - let cut = P.f[0].y; + let cut = P.f[0].x; let hv = textureLoad(aux, p, 0).x; let g = ftr_grad(p); + let grad = length(g); + var m = 0.0; + if (grad > 1e-9) { + let aa = max(P.f[5].w * grad, 5e-4); + m = clamp(0.5 + (hv - cut) / aa, 0.0, 1.0); + } else if (hv > cut) { + m = 1.0; + } + if (m <= 0.0) { + textureStore(out, p, vec4(0.0)); + return; + } let f = ftr_centre(p); - let s = sample_bilinear(src, f.x - g.x * depth * 2.0, f.y - g.y * depth * 2.0); - let c = ftr_unpremul(s); - let a = max(s.w, 0.0); - let m = clamp((hv - cut) / max((1.0 - cut) * 0.5, 1e-3), 0.0, 1.0); - let alpha = max(a, s.w) * m; + // Softness-scaled bevel warp of the source matte (normal z = 1). + let na = normalize(vec3(-g.x * P.f[0].w, -g.y * P.f[0].w, 1.0)); + let sa = sample_bilinear(src, f.x + na.x * P.f[5].x, f.y + na.y * P.f[5].x); + let alpha = max(sa.w, 0.0) * m; if (alpha <= 0.0) { textureStore(out, p, vec4(0.0)); return; } - let n = normalize(vec3(-g.x * depth, -g.y * depth, 1.0)); + // Height-field refraction of the colour (normal z = height), independent of Softness. + let hz = max(hv, 1e-3); + let d = -g * P.f[5].y / hz; + let lim = P.f[6].x; + let disp = vec2(clamp(d.x, -lim, lim), clamp(d.y, -lim, lim)); + let srgb = sample_bilinear(src, f.x + disp.x, f.y + disp.y); + let c = ftr_unpremul(srgb); + let n = normalize(vec3(-g.x * P.f[5].z, -g.y * P.f[5].z, 1.0)); var l = P.f[3].xyz; if (P.f[2].w != 0.0) { - l = normalize(vec3(P.f[2].x - f32(p.x), P.f[2].y - f32(p.y), P.f[2].z)); + l = normalize(vec3(P.f[2].x - f.x, P.f[2].y - f.y, P.f[2].z)); } - let intensity = P.f[0].z; + let intensity = P.f[0].y; let ndl = max(dot(n, l), 0.0); let h3 = normalize(vec3(l.x, l.y, l.z + 1.0)); let sp = powz(max(dot(n, h3), 0.0), P.f[1].z) * P.f[1].y; let lc = P.f[4].xyz; let metal = P.f[1].w; - let lit = (P.f[0].w + P.f[1].x * ndl * intensity) * lc; + let lit = (P.f[0].z + P.f[1].x * ndl * intensity) * lc; let spec_col = (1.0 - metal) * lc + metal * c * lc; let o = c * lit + (sp * intensity) * spec_col; textureStore(out, p, vec4(o * alpha, alpha)); From 6e85c762c145e5a5ecf36f0f86d7869a8f468f85 Mon Sep 17 00:00:00 2001 From: Camille Moine Date: Wed, 21 Oct 2026 13:33:53 +0200 Subject: [PATCH 11/37] test(effects): pin the AE defaults of CC Blobbylize --- crates/effects/src/distort3.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/crates/effects/src/distort3.rs b/crates/effects/src/distort3.rs index ca6e5ba8..88be62e9 100644 --- a/crates/effects/src/distort3.rs +++ b/crates/effects/src/distort3.rs @@ -1169,6 +1169,22 @@ mod tests { } } + #[test] + fn blobbylize_defaults_match_ae() { + let s = crate::find("ec.distort.ccblobbylize").unwrap(); + let default = |id: &str| s.params.iter().find(|p| p.id == id).unwrap().default.clone(); + assert_eq!(default("property"), Value::Enum(0), "Property defaults to Red"); + assert_eq!(default("softness"), num(20.0)); + assert_eq!(default("cutAway"), num(25.0)); + assert_eq!(default("lightHeight"), num(65.0)); + assert_eq!(default("lightPosition"), pt(0.5, 0.5)); + assert_eq!(default("ambient"), num(75.0)); + assert_eq!(default("diffuse"), num(25.0)); + assert_eq!(default("specular"), num(100.0)); + assert_eq!(default("roughness"), num(0.025)); + assert_eq!(default("metal"), num(100.0)); + } + #[test] fn blobbylize_cut_away_thresholds_a_height_ramp() { // AE oracle: Cut Away is a linear height threshold with a ~1 px antialiased edge. From fda7eef73fe6a73b047f285bef39bf2271d3844e Mon Sep 17 00:00:00 2001 From: Camille Moine Date: Wed, 21 Oct 2026 14:34:08 +0200 Subject: [PATCH 12/37] contributors: credit ki2pixel --- contributors/people.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contributors/people.toml b/contributors/people.toml index 625fcdb7..1012a37b 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -95,3 +95,6 @@ sync_github_name = true [people.Silvertree2010] display_name = "Netter" sync_github_name = false + +[people.ki2pixel] +display_name = "ki2pixel" From 374c3bf6b9db9a01ce25a93139fc11bbbf4efc06 Mon Sep 17 00:00:00 2001 From: Camille Moine Date: Wed, 21 Oct 2026 14:46:21 +0200 Subject: [PATCH 13/37] fix(effects): clear Cut Away 100 opaque edges The gradient antialias could leave a half-covered pixel where an opaque source reaches h = 1. Add an endpoint guard on CPU and GPU and cover an opaque edge in the regression test. --- crates/effects/src/distort3.rs | 15 +++++++++++++++ crates/gpu/src/shaders/fx_transition.wgsl | 6 ++++++ crates/gpu/src/tests_fx_transition.rs | 1 + 3 files changed, 22 insertions(+) diff --git a/crates/effects/src/distort3.rs b/crates/effects/src/distort3.rs index 88be62e9..cb389d90 100644 --- a/crates/effects/src/distort3.rs +++ b/crates/effects/src/distort3.rs @@ -123,6 +123,12 @@ fn dot3f(a: [f32; 3], b: [f32; 3]) -> f32 { fn cc_blobbylize(ctx: &EffectCtx, mut b: Buf) -> Buf { let soft = ctx.params.f("softness").max(0.0) * b.scale; let cut = (ctx.params.f("cutAway") / 100.0).clamp(0.0, 1.0) as f32; + if cut >= 1.0 { + // Endpoint guard: at Cut Away 100 the gradient antialias must not leave a half-covered + // pixel where an opaque source reaches exactly 1.0. + b.img.data.fill([0.0; 4]); + return b; + } let blob = layer_or_self(ctx, &b, "blobLayer", true, false); let prop = src_at(ctx.params.e("property")); let raw = Plane::from_image(&blob, |px| { @@ -1212,6 +1218,15 @@ mod tests { assert!(o.img.data.iter().all(|p| p[3] == 0.0), "Cut Away 100 left opaque pixels"); } + #[test] + fn blobbylize_cut_away_100_clears_an_opaque_edge() { + // An opaque source reaches h = 1 on the high side of an edge; the gradient antialias + // used to leave m = 0.5 there, so Cut Away 100 still produced visible pixels. + let im = crate::util::gen_image(64, 8, |x, _| if x < 32 { [1.0; 4] } else { [0.0; 4] }); + let o = run("ec.distort.ccblobbylize", &[("property", Value::Enum(3)), ("softness", num(0.0)), ("cutAway", num(100.0))], im, &[]); + assert!(o.img.data.iter().all(|p| p[3] == 0.0), "Cut Away 100 left an opaque edge"); + } + #[test] fn blobbylize_softness_warps_a_step_matte_inward() { // AE oracle T2: a self alpha step at x = 960, Softness 20, Cut Away 0 puts the matte diff --git a/crates/gpu/src/shaders/fx_transition.wgsl b/crates/gpu/src/shaders/fx_transition.wgsl index ef543633..5037ec9a 100644 --- a/crates/gpu/src/shaders/fx_transition.wgsl +++ b/crates/gpu/src/shaders/fx_transition.wgsl @@ -582,6 +582,12 @@ fn ftr_blob(@builtin(global_invocation_id) gid: vec3) { return; } let cut = P.f[0].x; + if (cut >= 1.0) { + // Endpoint guard: an opaque source can reach h = 1.0, where the gradient antialias + // would otherwise leave a half-covered pixel at Cut Away 100. + textureStore(out, p, vec4(0.0)); + return; + } let hv = textureLoad(aux, p, 0).x; let g = ftr_grad(p); let grad = length(g); diff --git a/crates/gpu/src/tests_fx_transition.rs b/crates/gpu/src/tests_fx_transition.rs index b4b3572a..abe9273d 100644 --- a/crates/gpu/src/tests_fx_transition.rs +++ b/crates/gpu/src/tests_fx_transition.rs @@ -126,6 +126,7 @@ fn distort() { "ec.distort.ccblobbylize", &[("property", e(4)), ("softness", n(3.0)), ("cutAway", n(30.0)), ("lightType", e(1)), ("lightPosition", pt(20.0, 10.0))], ); + effect_case("ec.distort.ccblobbylize", &[("cutAway", n(100.0))]); effect_case("ec.distort.ccblobbylize", &[("property", e(5)), ("metal", n(30.0)), ("roughness", n(0.2))]); } From d2cb0946efed13b02d1f484413e5dd964efcb771 Mon Sep 17 00:00:00 2001 From: Camille Moine Date: Wed, 21 Oct 2026 16:20:44 +0200 Subject: [PATCH 14/37] test(gpu): assert Cut Away 100 is fully transparent --- crates/gpu/src/tests.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/gpu/src/tests.rs b/crates/gpu/src/tests.rs index 24f8c961..1fad0e6c 100644 --- a/crates/gpu/src/tests.rs +++ b/crates/gpu/src/tests.rs @@ -446,12 +446,16 @@ pub(crate) fn effect_direct(id: &str, vals: &[(&str, Value)], adjustment: bool) for (k, v) in vals { params.values.insert(k.to_string(), v.clone()); } + let expect_empty = id == "ec.distort.ccblobbylize" && vals.iter().any(|(k, v)| *k == "cutAway" && matches!(v, Value::Scalar(x) if *x == 100.0)); for scale in [1.0, 0.5] { let img = pattern(7, (size[0] * scale) as u32, (size[1] * scale) as u32); let buf = effectcraft_effects::Buf { img, offset: [0.0, 0.0], scale }; let ctx = || effectcraft_effects::EffectCtx { params: ¶ms, time: 0.25, layer_size: size, seed: 11, adjustment, env: Default::default() }; let cpu = (spec.render)(&ctx(), buf.clone()); let out = effectcraft_render::Accelerator::effects(g, &[effectcraft_render::FxStep { spec, ctx: ctx() }], &buf, None).expect("the GPU runs the effect"); + if expect_empty { + assert!(out.img.data.iter().all(|p| p[3] == 0.0), "{id} {vals:?} adj {adjustment} scale {scale}: Cut Away 100 produced non-zero alpha"); + } assert_eq!((out.offset, out.scale), (cpu.offset, cpu.scale), "{id}: geometry"); let d = diff(&cpu.img, &out.img, 1e-3); let allow = if on_gl() { (d.total as f64 * GL_BOUNDARY_FLIPS) as usize } else { 0 }; From ca88af2b5f271c5c8be324cbe0574e99ece07bf2 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Wed, 21 Oct 2026 21:34:45 +0530 Subject: [PATCH 15/37] Fractal Noise: resolve the Blending Mode once per layer, not per pixel `crates/effects/src/noise3.rs` only: - a `blend_mode(u32) -> Option` helper, which the CPU and GPU paths now share; - the CPU `render` resolves the mode once and passes `Option` to the per-pixel `composite`. The semantics are unchanged: None means the noise alone, and an unknown index means Normal. Co-Authored-By: Claude Opus 5.5 --- crates/effects/src/noise3.rs | 54 ++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/crates/effects/src/noise3.rs b/crates/effects/src/noise3.rs index 88a5c01d..0ea63dfe 100644 --- a/crates/effects/src/noise3.rs +++ b/crates/effects/src/noise3.rs @@ -406,15 +406,20 @@ pub fn fractal_gpu(ctx: &EffectCtx, offset: [f64; 2], scale: f64, turbulent: boo sub_rotation: fr.sub_rotation != 0.0, norm, octaves, - mode: match ctx.params.get("blendingMode").map(Value::as_enum).unwrap_or(BLEND_NONE) { - BLEND_NONE => None, - m => Some(BLEND_MODES.get(m as usize).and_then(|l| BlendMode::from_name(l)).unwrap_or(BlendMode::Normal)), - }, + mode: blend_mode(ctx.params.get("blendingMode").map(Value::as_enum).unwrap_or(BLEND_NONE)), opacity: (ctx.params.f("opacity") as f32 / 100.0).clamp(0.0, 1.0), blend: ctx.params.f("blend") as f32 / 100.0, } } +/// Resolve a Blending Mode index: None → `None`, unknown → Normal. +fn blend_mode(m: u32) -> Option { + match m { + BLEND_NONE => None, + m => Some(BLEND_MODES.get(m as usize).and_then(|l| BlendMode::from_name(l)).unwrap_or(BlendMode::Normal)), + } +} + /// Map a value by an [`OVERFLOWS`] option. pub fn overflow(v: f32, mode: u32) -> f32 { match mode { @@ -431,14 +436,11 @@ pub fn overflow(v: f32, mode: u32) -> f32 { /// Composite grey noise `val` over the original pixel by Blending Mode, Opacity and (ours) Blend /// With Original. The noise stays inside the layer's alpha, so a masked solid shows it only within /// its masks (#939). -pub(crate) fn composite(orig: [f32; 4], val: f32, opacity: f32, mode: u32, blend_orig: f32) -> [f32; 4] { +pub(crate) fn composite(orig: [f32; 4], val: f32, opacity: f32, mode: Option, blend_orig: f32) -> [f32; 4] { let src = [val * opacity, val * opacity, val * opacity, opacity]; let out = match mode { - BLEND_NONE => src, - m => { - let bm = BLEND_MODES.get(m as usize).and_then(|l| BlendMode::from_name(l)).unwrap_or(BlendMode::Normal); - blend_pixel(bm, orig, src, 0.5) - } + None => src, + Some(bm) => blend_pixel(bm, orig, src, 0.5), }; let k = 1.0 - blend_orig; [0, 1, 2, 3].map(|c| orig[c] * blend_orig + out[c] * orig[3] * k) @@ -447,7 +449,7 @@ pub(crate) fn composite(orig: [f32; 4], val: f32, opacity: f32, mode: u32, blend fn render(ctx: &EffectCtx, mut b: Buf, turbulent: bool) -> Buf { let fr = Fractal::from_params(ctx, &b, turbulent); let opacity = (ctx.params.f("opacity") as f32 / 100.0).clamp(0.0, 1.0); - let mode = ctx.params.get("blendingMode").map(Value::as_enum).unwrap_or(BLEND_NONE); + let mode = blend_mode(ctx.params.get("blendingMode").map(Value::as_enum).unwrap_or(BLEND_NONE)); let blend = ctx.params.f("blend") as f32 / 100.0; b.img.rows_mut().for_each(|(y, row)| { for (x, px) in row.iter_mut().enumerate() { @@ -665,4 +667,34 @@ mod tests { } assert!(axis / diag < 1.15, "gradient along the axes / along the diagonals: {}", axis / diag); } + + /// The base composite, resolving the Blending Mode by name on every call, kept only to prove + /// the hoisted lookup changes nothing. + fn old_composite(orig: [f32; 4], val: f32, opacity: f32, mode: u32, blend_orig: f32) -> [f32; 4] { + let src = [val * opacity, val * opacity, val * opacity, opacity]; + let out = match mode { + BLEND_NONE => src, + m => { + let bm = BLEND_MODES.get(m as usize).and_then(|l| BlendMode::from_name(l)).unwrap_or(BlendMode::Normal); + blend_pixel(bm, orig, src, 0.5) + } + }; + let k = 1.0 - blend_orig; + [0, 1, 2, 3].map(|c| orig[c] * blend_orig + out[c] * orig[3] * k) + } + + #[test] + fn resolved_blend_matches_per_pixel_lookup() { + for m in (0..BLEND_MODES.len() as u32).chain([24, 999]) { + for orig in [[0.2, 0.4, 0.6, 1.0], [0.9, 0.1, 0.5, 0.5], [0.0, 0.0, 0.0, 0.0]] { + for val in [0.0, 0.3, 0.75, 1.0] { + for (op, bl) in [(1.0, 0.0), (0.6, 0.25)] { + assert_eq!(composite(orig, val, op, blend_mode(m), bl), old_composite(orig, val, op, m, bl), "mode {m}"); + } + } + } + } + assert_eq!(blend_mode(BLEND_NONE), None); + assert_eq!(blend_mode(999), Some(BlendMode::Normal)); + } } From c9323afe1dfc3dac8fdf0954706dac65f4e51bf8 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Wed, 21 Oct 2026 21:34:59 +0530 Subject: [PATCH 16/37] Save: check the file's values without reparsing it in release builds - New `crates/project/src/file_check.rs`: a no-output `Serializer` that walks the value once. It refuses non-finite numbers and nesting past 127 levels, and it allocates only on error. - `to_file_json_with_view` runs that check, then writes the file. It also refuses a project schema newer than this build, the remaining thing `from_json` would reject in a file we wrote. - **Debug builds still reparse the text as well**, so a new kind of mismatch still shows up in tests. The bytes written are unchanged. Co-Authored-By: Claude Opus 5.5 --- crates/project/src/file_check.rs | 329 +++++++++++++++++++++++++++++++ crates/project/src/lib.rs | 19 +- crates/project/src/tests.rs | 23 +++ 3 files changed, 367 insertions(+), 4 deletions(-) create mode 100644 crates/project/src/file_check.rs diff --git a/crates/project/src/file_check.rs b/crates/project/src/file_check.rs new file mode 100644 index 00000000..f1b261c5 --- /dev/null +++ b/crates/project/src/file_check.rs @@ -0,0 +1,329 @@ +//! A cheap check that a project file will read back, without parsing it. +//! +//! `serde_json` writes a non-finite float as `null` and refuses to parse nesting deeper than 128 +//! containers, so a project holding either would be saved to a file that can't be opened. Walking +//! the value once, writing nothing, finds both without building the project again. + +use std::fmt; + +use serde::ser::{ + self, Serialize, SerializeMap, SerializeSeq, SerializeStruct, SerializeStructVariant, SerializeTuple, SerializeTupleStruct, SerializeTupleVariant, + Serializer, +}; + +/// serde_json refuses to parse the 128th nested array/object (its recursion limit is 128). +const MAX_DEPTH: u32 = 127; + +const NOT_FINITE: &str = "a number that isn't finite (NaN or infinity)"; +const TOO_DEEP: &str = "nesting deeper than 127 levels"; + +/// Why a value can't be written to a project file. +#[derive(Debug)] +pub(crate) struct Unstorable(String); + +impl fmt::Display for Unstorable { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.0) + } +} + +impl std::error::Error for Unstorable {} + +impl ser::Error for Unstorable { + fn custom(msg: T) -> Self { + Unstorable(msg.to_string()) + } +} + +/// Walk `value` as `serde_json` would write it, producing no text and allocating only on error. +pub(crate) fn check(value: &T) -> Result<(), Unstorable> { + let mut walker = Walker { depth: 0 }; + value.serialize(&mut walker) +} + +/// The current nesting depth of the value being walked. +struct Walker { + depth: u32, +} + +impl Walker { + /// Enter `levels` nested containers, refusing anything serde_json couldn't parse back. + fn enter(&mut self, levels: u32) -> Result<(), Unstorable> { + let depth = self.depth.checked_add(levels).ok_or_else(|| Unstorable(TOO_DEEP.into()))?; + if depth > MAX_DEPTH { + return Err(Unstorable(TOO_DEEP.into())); + } + self.depth = depth; + Ok(()) + } + fn leave(&mut self, levels: u32) { + self.depth = self.depth.saturating_sub(levels); + } +} + +/// One compound value being walked; leaving it when it ends keeps the depth exact. +struct Compound<'a> { + walker: &'a mut Walker, + levels: u32, +} + +impl<'a> Compound<'a> { + fn new(walker: &'a mut Walker, levels: u32) -> Result { + walker.enter(levels)?; + Ok(Self { walker, levels }) + } + fn value(&mut self, value: &T) -> Result<(), Unstorable> { + value.serialize(&mut *self.walker) + } + fn finish(self) -> Result<(), Unstorable> { + self.walker.leave(self.levels); + Ok(()) + } +} + +impl SerializeSeq for Compound<'_> { + type Ok = (); + type Error = Unstorable; + fn serialize_element(&mut self, value: &T) -> Result<(), Unstorable> { + self.value(value) + } + fn end(self) -> Result<(), Unstorable> { + self.finish() + } +} + +impl SerializeTuple for Compound<'_> { + type Ok = (); + type Error = Unstorable; + fn serialize_element(&mut self, value: &T) -> Result<(), Unstorable> { + self.value(value) + } + fn end(self) -> Result<(), Unstorable> { + self.finish() + } +} + +impl SerializeTupleStruct for Compound<'_> { + type Ok = (); + type Error = Unstorable; + fn serialize_field(&mut self, value: &T) -> Result<(), Unstorable> { + self.value(value) + } + fn end(self) -> Result<(), Unstorable> { + self.finish() + } +} + +impl SerializeTupleVariant for Compound<'_> { + type Ok = (); + type Error = Unstorable; + fn serialize_field(&mut self, value: &T) -> Result<(), Unstorable> { + self.value(value) + } + fn end(self) -> Result<(), Unstorable> { + self.finish() + } +} + +impl SerializeMap for Compound<'_> { + type Ok = (); + type Error = Unstorable; + fn serialize_key(&mut self, key: &T) -> Result<(), Unstorable> { + self.value(key) + } + fn serialize_value(&mut self, value: &T) -> Result<(), Unstorable> { + self.value(value) + } + fn end(self) -> Result<(), Unstorable> { + self.finish() + } +} + +impl SerializeStruct for Compound<'_> { + type Ok = (); + type Error = Unstorable; + fn serialize_field(&mut self, _key: &'static str, value: &T) -> Result<(), Unstorable> { + self.value(value) + } + fn end(self) -> Result<(), Unstorable> { + self.finish() + } +} + +impl SerializeStructVariant for Compound<'_> { + type Ok = (); + type Error = Unstorable; + fn serialize_field(&mut self, _key: &'static str, value: &T) -> Result<(), Unstorable> { + self.value(value) + } + fn end(self) -> Result<(), Unstorable> { + self.finish() + } +} + +impl<'a> Serializer for &'a mut Walker { + type Ok = (); + type Error = Unstorable; + type SerializeSeq = Compound<'a>; + type SerializeTuple = Compound<'a>; + type SerializeTupleStruct = Compound<'a>; + type SerializeTupleVariant = Compound<'a>; + type SerializeMap = Compound<'a>; + type SerializeStruct = Compound<'a>; + type SerializeStructVariant = Compound<'a>; + + fn serialize_bool(self, _v: bool) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_i8(self, _v: i8) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_i16(self, _v: i16) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_i32(self, _v: i32) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_i64(self, _v: i64) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_i128(self, _v: i128) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_u8(self, _v: u8) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_u16(self, _v: u16) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_u32(self, _v: u32) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_u64(self, _v: u64) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_u128(self, _v: u128) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_f32(self, v: f32) -> Result<(), Unstorable> { + if v.is_finite() { Ok(()) } else { Err(Unstorable(NOT_FINITE.into())) } + } + fn serialize_f64(self, v: f64) -> Result<(), Unstorable> { + if v.is_finite() { Ok(()) } else { Err(Unstorable(NOT_FINITE.into())) } + } + fn serialize_char(self, _v: char) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_str(self, _v: &str) -> Result<(), Unstorable> { + Ok(()) + } + /// serde_json writes bytes as an array of numbers: one level of nesting. + fn serialize_bytes(self, _v: &[u8]) -> Result<(), Unstorable> { + Compound::new(self, 1)?.finish() + } + /// Writes text for a `Display` value; it is a scalar, not a container. + fn collect_str(self, _v: &T) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_none(self) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_some(self, value: &T) -> Result<(), Unstorable> { + value.serialize(&mut *self) + } + fn serialize_unit(self) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_unit_struct(self, _name: &'static str) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_unit_variant(self, _name: &'static str, _index: u32, _variant: &'static str) -> Result<(), Unstorable> { + Ok(()) + } + fn serialize_newtype_struct(self, _name: &'static str, value: &T) -> Result<(), Unstorable> { + value.serialize(self) + } + fn serialize_newtype_variant(self, _name: &'static str, _index: u32, _variant: &'static str, value: &T) -> Result<(), Unstorable> { + let mut compound = Compound::new(self, 1)?; + compound.value(value)?; + compound.finish() + } + fn serialize_seq(self, _len: Option) -> Result, Unstorable> { + Compound::new(self, 1) + } + fn serialize_tuple(self, _len: usize) -> Result, Unstorable> { + Compound::new(self, 1) + } + fn serialize_tuple_struct(self, _name: &'static str, _len: usize) -> Result, Unstorable> { + Compound::new(self, 1) + } + fn serialize_tuple_variant(self, _name: &'static str, _index: u32, _variant: &'static str, _len: usize) -> Result, Unstorable> { + Compound::new(self, 2) + } + fn serialize_map(self, _len: Option) -> Result, Unstorable> { + Compound::new(self, 1) + } + fn serialize_struct(self, _name: &'static str, _len: usize) -> Result, Unstorable> { + Compound::new(self, 1) + } + fn serialize_struct_variant(self, _name: &'static str, _index: u32, _variant: &'static str, _len: usize) -> Result, Unstorable> { + Compound::new(self, 2) + } +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use super::check; + + #[test] + fn refuses_non_finite_numbers_anywhere() { + assert!(check(&1.5f64).is_ok()); + assert!(check(&vec![Some(1.0f64), None]).is_ok()); + assert!(check(&BTreeMap::from([("a".to_string(), 2.0f32)])).is_ok(), "finite floats in a map"); + assert!(check(&f64::NAN).is_err()); + assert!(check(&f64::INFINITY).is_err()); + assert!(check(&vec![Some(f64::NEG_INFINITY)]).is_err(), "inside an option in a list"); + assert!(check(&BTreeMap::from([("a".to_string(), f32::NAN)])).is_err(), "as a map value"); + } + + #[test] + fn depth_limit_matches_serde_json() { + for n in [127, 128] { + let mut v = serde_json::Value::Null; + for _ in 0..n { + v = serde_json::Value::Array(vec![v]); + } + let text = serde_json::to_string(&v).unwrap(); + let reads_back = serde_json::from_str::(&text).is_ok(); + assert_eq!(check(&v).is_ok(), reads_back, "depth {n} agrees with serde_json"); + assert_eq!(check(&v).is_ok(), n == 127, "depth {n}"); + } + } + + /// `n` nested lists around bytes, which serde_json writes as one more array. + struct BytesUnder(u32); + + impl serde::Serialize for BytesUnder { + fn serialize(&self, s: S) -> Result { + use serde::ser::SerializeSeq; + let Some(inner) = self.0.checked_sub(1) else { + return s.serialize_bytes(&[1]); + }; + let mut seq = s.serialize_seq(Some(1))?; + seq.serialize_element(&BytesUnder(inner))?; + seq.end() + } + } + + #[test] + fn bytes_count_as_one_level() { + for n in [126, 127] { + let text = serde_json::to_string(&BytesUnder(n)).unwrap(); + let reads_back = serde_json::from_str::(&text).is_ok(); + assert_eq!(check(&BytesUnder(n)).is_ok(), reads_back, "depth {n} agrees with serde_json"); + assert_eq!(check(&BytesUnder(n)).is_ok(), n == 126, "depth {n}"); + } + } +} diff --git a/crates/project/src/lib.rs b/crates/project/src/lib.rs index a4bffbe0..68772e00 100644 --- a/crates/project/src/lib.rs +++ b/crates/project/src/lib.rs @@ -8,6 +8,7 @@ pub mod build; pub mod essential; +mod file_check; pub mod limits; pub mod props; pub mod render_queue; @@ -1519,13 +1520,20 @@ impl Project { && self.render_prefs == other.render_prefs } /// The project file's text: the project with the version that wrote it (`savedBy`), checked - /// to read back, so a save never writes a file that can't be opened (JSON has no NaN or - /// infinity: such a value would be written as `null`). + /// to read back, so a save never writes a file that can't be opened. Values JSON can't hold + /// (NaN or infinity, written as `null`, and nesting too deep to parse) are refused on every + /// save; debug builds also read the text back. pub fn to_file_json(&self) -> Result { self.to_file_json_with_view(&SavedView::default()) } /// [`Project::to_file_json`] that also records the open compositions ([`SavedView`]). pub fn to_file_json_with_view(&self, view: &SavedView) -> Result { + if self.schema > SCHEMA_VERSION { + return Err(ProjectError::Invalid(format!( + "the project can't be saved: it holds a value a project file can't store (project schema {} is newer than this version supports ({SCHEMA_VERSION}))", + self.schema + ))); + } #[derive(Serialize)] struct File<'a> { #[serde(rename = "savedBy")] @@ -1535,8 +1543,11 @@ impl Project { #[serde(flatten)] project: &'a Project, } - let text = serde_json::to_string_pretty(&File { saved_by: APP_VERSION, view, project: self }) - .map_err(|e| ProjectError::Invalid(format!("the project can't be saved: {e}")))?; + let file = File { saved_by: APP_VERSION, view, project: self }; + file_check::check(&file) + .map_err(|e| ProjectError::Invalid(format!("the project can't be saved: it holds a value a project file can't store ({e})")))?; + let text = serde_json::to_string_pretty(&file).map_err(|e| ProjectError::Invalid(format!("the project can't be saved: {e}")))?; + #[cfg(debug_assertions)] Project::from_json(&text) .map_err(|e| ProjectError::Invalid(format!("the project can't be saved: it holds a value a project file can't store ({e})")))?; Ok(text) diff --git a/crates/project/src/tests.rs b/crates/project/src/tests.rs index 8cd52092..3c40e862 100644 --- a/crates/project/src/tests.rs +++ b/crates/project/src/tests.rs @@ -166,10 +166,33 @@ fn project_files_name_their_version_and_always_read_back() { p.comp_mut(cid).unwrap().layers[0].props.prop_mut("transform/opacity").unwrap().value = Value::Scalar(f64::NAN); let e = p.to_file_json().unwrap_err().to_string(); assert!(e.contains("can't be saved"), "{e}"); + // Infinity too, in either profile: the check doesn't rely on reading the file back. + p.comp_mut(cid).unwrap().layers[0].props.prop_mut("transform/opacity").unwrap().value = Value::Scalar(f64::INFINITY); + let e = p.to_file_json().unwrap_err().to_string(); + assert!(e.contains("can't be saved"), "{e}"); assert!(crate::is_newer_version("0.2.0", "0.1.9") && crate::is_newer_version("1.0.0-rc.1", "0.9.9")); assert!(!crate::is_newer_version("0.1.1", "0.1.1") && !crate::is_newer_version("0.1.0", "0.1.1")); } +#[test] +fn project_files_refuse_newer_schemas() { + let p = Project { schema: crate::SCHEMA_VERSION + 1, ..Project::default() }; + let expected = format!( + "the project can't be saved: it holds a value a project file can't store (project schema {} is newer than this version supports ({}))", + p.schema, + crate::SCHEMA_VERSION + ); + assert_eq!(p.to_file_json().unwrap_err().to_string(), expected); + assert_eq!(p.to_file_json_with_view(&crate::SavedView::default()).unwrap_err().to_string(), expected); +} + +#[test] +fn saved_files_reopen_in_pretty_and_compact_form() { + let (p, _cid) = project_with_layer(); + assert_eq!(Project::from_json(&p.to_file_json().unwrap()).unwrap(), p, "the pretty project file"); + assert_eq!(Project::from_json(&p.to_json_compact()).unwrap(), p, "compact JSON"); +} + #[test] fn project_files_remember_the_open_comps() { let (p, cid) = project_with_layer(); From 2b3732153b10f8e3e306808d8df4b80b522ae613 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Wed, 21 Oct 2026 21:45:28 +0530 Subject: [PATCH 17/37] GPU: devices favour memory use, so Purge and Close give video memory back Both devices EffectCraft creates now ask for `MemoryHints::MemoryUsage`: - the window device (`apps/effectcraft/src/main.rs`, through a new `device_descriptor` helper); - the headless compositor device (`crates/gpu/src/context.rs`). The limits, labels, features and error paths are unchanged. The devices tests create stay as they were. Co-Authored-By: Claude Opus 5.5 --- apps/effectcraft/src/main.rs | 28 +++++++++++++++++++++++----- crates/gpu/src/context.rs | 11 +++++++---- crates/gpu/src/tests.rs | 10 ++++++++++ 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/apps/effectcraft/src/main.rs b/apps/effectcraft/src/main.rs index a1fb6e4a..2bc3265d 100644 --- a/apps/effectcraft/src/main.rs +++ b/apps/effectcraft/src/main.rs @@ -371,15 +371,22 @@ fn wgpu_options(backends: Option) -> eframe::WgpuConfigu // Iris Pro: the device is lost and the window never opens. // `WGPU_VALIDATION_INDIRECT_CALL=1` turns the check back on. setup.instance_descriptor.flags = setup.instance_descriptor.flags.difference(eframe::wgpu::InstanceFlags::VALIDATION_INDIRECT_CALL).with_env(); - setup.device_descriptor = std::sync::Arc::new(|adapter| eframe::wgpu::DeviceDescriptor { - label: Some("EffectCraft device"), - required_limits: device_limits(adapter.limits()), - ..Default::default() - }); + setup.device_descriptor = std::sync::Arc::new(|adapter| device_descriptor(adapter.limits())); } config } +/// The window device's descriptor: the adapter's limits ([`device_limits`]) and `MemoryUsage`, +/// so memory the compositor frees on the shared device goes back to the driver. +fn device_descriptor(adapter: eframe::wgpu::Limits) -> eframe::wgpu::DeviceDescriptor<'static> { + eframe::wgpu::DeviceDescriptor { + label: Some("EffectCraft device"), + required_limits: device_limits(adapter), + memory_hints: eframe::wgpu::MemoryHints::MemoryUsage, + ..Default::default() + } +} + /// The limits to request from an adapter that offers `adapter`: all of them, with textures /// capped at 16384 px like the headless renderer's device. fn device_limits(adapter: eframe::wgpu::Limits) -> eframe::wgpu::Limits { @@ -539,6 +546,17 @@ mod tests { assert_eq!(super::device_limits(big).max_texture_dimension_2d, 16384); } + /// The window device asks for `MemoryUsage`: the adapter's limits, unchanged. + #[test] + fn the_window_device_favours_memory_usage() { + use eframe::wgpu::Limits; + let big = Limits { max_texture_dimension_2d: 32768, ..Limits::default() }; + let d = super::device_descriptor(big); + assert!(matches!(d.memory_hints, eframe::wgpu::MemoryHints::MemoryUsage)); + assert_eq!(d.required_limits.max_texture_dimension_2d, 16384); + assert_eq!(d.label, Some("EffectCraft device")); + } + /// A positional .ecprojx is a project file, not an unsupported media import (#528). #[test] fn launch_classifies_xml_and_json_projects_as_projects() { diff --git a/crates/gpu/src/context.rs b/crates/gpu/src/context.rs index 7229aa5a..2fbf86bc 100644 --- a/crates/gpu/src/context.rs +++ b/crates/gpu/src/context.rs @@ -462,6 +462,12 @@ pub(crate) fn buffer_init(device: &wgpu::Device, queue: &wgpu::Queue, label: Opt buffer } +/// The descriptor for a device the compositor owns: `MemoryUsage` so freed textures give their +/// allocator blocks back instead of a few long-lived objects pinning large ones. +pub(crate) fn device_descriptor(required_limits: wgpu::Limits) -> wgpu::DeviceDescriptor<'static> { + wgpu::DeviceDescriptor { label: Some("effectcraft gpu"), required_limits, memory_hints: wgpu::MemoryHints::MemoryUsage, ..Default::default() } +} + impl GpuContext { /// [`buffer_init`] on this context's device. pub(crate) fn buffer_init(&self, label: Option<&str>, contents: &[u8], usage: wgpu::BufferUsages) -> wgpu::Buffer { @@ -870,10 +876,7 @@ impl GpuContext { max_buffer_size: limits.max_buffer_size, ..wgpu::Limits::default() }; - let (device, queue) = adapter - .request_device(&wgpu::DeviceDescriptor { label: Some("effectcraft gpu"), required_limits, ..Default::default() }) - .await - .map_err(|e| match by_name { + let (device, queue) = adapter.request_device(&device_descriptor(required_limits)).await.map_err(|e| match by_name { true => format!("no device on {} (picked by WGPU_ADAPTER_NAME): {e}", adapter.get_info().name), false => format!("no device: {e}"), })?; diff --git a/crates/gpu/src/tests.rs b/crates/gpu/src/tests.rs index 24f8c961..b466e539 100644 --- a/crates/gpu/src/tests.rs +++ b/crates/gpu/src/tests.rs @@ -1071,3 +1071,13 @@ fn upload_cache_drops_textures_of_freed_buffers() { let kept = g.ctx.upload_entries(); assert!(kept <= before + 1, "the upload cache kept {kept} textures of freed buffers (had {before})"); } + +/// The device the compositor owns asks for `MemoryUsage` so freed textures give +/// their allocator blocks back, while its label and limits stay as they were. +#[test] +fn owned_devices_favour_memory_usage() { + let d = crate::context::device_descriptor(wgpu::Limits::default()); + assert!(matches!(d.memory_hints, wgpu::MemoryHints::MemoryUsage)); + assert_eq!(d.label, Some("effectcraft gpu")); + assert_eq!(d.required_limits.max_texture_dimension_2d, wgpu::Limits::default().max_texture_dimension_2d); +} From 17d57a67058b1fe52fd600d76564bcfb16bbb4bd Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Wed, 21 Oct 2026 12:41:50 -0400 Subject: [PATCH 18/37] Light Settings: list Environment and keep an existing light's type (#1588) The dialog offered four hard-coded light types and reopened an Environment light as Spot, so OK turned it into one. It now lists every light type the engine has, keeps the light's own type, and shows Falloff and Shadows only for lights that have them. --- crates/ui-egui/src/panels/dialogs_3d.rs | 36 +++++++++++++++---------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/crates/ui-egui/src/panels/dialogs_3d.rs b/crates/ui-egui/src/panels/dialogs_3d.rs index a1a67bc6..01fd5b9e 100644 --- a/crates/ui-egui/src/panels/dialogs_3d.rs +++ b/crates/ui-egui/src/panels/dialogs_3d.rs @@ -29,7 +29,7 @@ pub struct CameraDraft { pub struct LightDraft { pub layer: Option, pub name: String, - pub kind: usize, + pub kind: LightKind, pub color: [f32; 3], pub intensity: f64, pub cone_angle: f64, @@ -42,9 +42,14 @@ pub struct LightDraft { pub shadow_diffusion: f64, } -const KINDS: [LightKind; 4] = [LightKind::Parallel, LightKind::Spot, LightKind::Point, LightKind::Ambient]; const FALLOFFS: [&str; 3] = ["None", "Smooth", "Inverse Square Clamped"]; +/// Whether a light type has Falloff and Shadows options. Ambient and Environment lights come +/// from no one place, so they have neither. +fn falls_off(kind: LightKind) -> bool { + !matches!(kind, LightKind::Ambient | LightKind::Environment) +} + fn comp_w(app: &EffectcraftApp) -> f64 { app.session.active_comp().map(|c| c.width as f64).unwrap_or(1920.0) } @@ -105,7 +110,7 @@ pub fn open_light(app: &mut EffectcraftApp, layer: Option) -> Result<(), St LightDraft { layer: Some(l.id.0), name: l.name.clone(), - kind: KINDS.iter().position(|k| *k == kind).unwrap_or(1), + kind, color: [c[0], c[1], c[2]], intensity: f("intensity", 100.0), cone_angle: f("coneAngle", 90.0), @@ -123,7 +128,7 @@ pub fn open_light(app: &mut EffectcraftApp, layer: Option) -> Result<(), St LightDraft { layer: None, name: format!("Light {n}"), - kind: 1, + kind: LightKind::Spot, color: [1.0, 1.0, 1.0], intensity: 100.0, cone_angle: 90.0, @@ -263,11 +268,13 @@ pub fn light(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { crate::widgets::focus_on_open(ui.ctx(), &r, &d.name); ui.end_row(); ui.label(tr("Light Type")); - egui::ComboBox::from_id_salt("light-type").selected_text(KINDS[d.kind.min(3)].label()).show_ui(ui, |ui| { - for (i, k) in KINDS.iter().enumerate() { - ui.selectable_value(&mut d.kind, i, k.label()); + let combo = egui::ComboBox::from_id_salt("light-type").selected_text(tr(d.kind.label())).show_ui(ui, |ui| { + for (i, k) in LightKind::ALL.into_iter().enumerate() { + let item = ui.selectable_value(&mut d.kind, k, tr(k.label())); + app.auto.add(&format!("dialog.light.typeItem.{i}"), item.rect, k.label()); } }); + app.auto.add_response("dialog.light.type", &combo.response, "Light Type"); ui.end_row(); ui.label(tr("Color")); crate::widgets::srgb_color_button(ui, &mut d.color); @@ -275,8 +282,7 @@ pub fn light(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { ui.label(tr("Intensity")); ui.add(crate::widgets::drag_value(egui::DragValue::new(&mut d.intensity).range(-10000.0..=10000.0).suffix(" %"))); ui.end_row(); - let kind = KINDS[d.kind.min(3)]; - if kind == LightKind::Spot { + if d.kind == LightKind::Spot { ui.label(tr("Cone Angle")); ui.add(crate::widgets::drag_value(egui::DragValue::new(&mut d.cone_angle).range(0.0..=180.0).suffix("°"))); ui.end_row(); @@ -284,7 +290,7 @@ pub fn light(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { ui.add(crate::widgets::drag_value(egui::DragValue::new(&mut d.cone_feather).range(0.0..=100.0).suffix(" %"))); ui.end_row(); } - if kind != LightKind::Ambient { + if falls_off(d.kind) { ui.label(tr("Falloff")); egui::ComboBox::from_id_salt("light-falloff").selected_text(FALLOFFS[d.falloff.min(2)]).show_ui(ui, |ui| { for (i, f) in FALLOFFS.iter().enumerate() { @@ -312,15 +318,17 @@ pub fn light(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { } }); ui.add_space(14.0); - ui.label(egui::RichText::new(tr("Note: Shadows are only cast from layers with Cast Shadows enabled.")).font(Tokens::ui(11.0)).color(t.text_dim)); - ui.add_space(8.0); + if falls_off(d.kind) { + ui.label(egui::RichText::new(tr("Note: Shadows are only cast from layers with Cast Shadows enabled.")).font(Tokens::ui(11.0)).color(t.text_dim)); + ui.add_space(8.0); + } rects = buttons(ui, t, &mut ok, &mut close); }); app.auto.add("dialog.light.ok", rects.0, "OK"); app.auto.add("dialog.light.cancel", rects.1, "Cancel"); app.dialog_state.light = d.clone(); if ok || super::dialogs::ui_enter(ctx) { - let kind = KINDS[d.kind.min(3)]; + let kind = d.kind; let mut p = json!({ "name": d.name, "kind": kind.label(), @@ -331,7 +339,7 @@ pub fn light(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { p["coneAngle"] = json!(d.cone_angle); p["coneFeather"] = json!(d.cone_feather); } - if kind != LightKind::Ambient { + if falls_off(kind) { p["falloff"] = json!(d.falloff); p["radius"] = json!(d.radius); p["falloffDistance"] = json!(d.falloff_distance); From b9929c2d7108ea30b062ba2d34460efd3cfc1120 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Wed, 21 Oct 2026 12:41:50 -0400 Subject: [PATCH 19/37] Light Settings: test Environment lights and the other types through the dialog (#1588) --- crates/ui-egui/tests/ui_light_settings.rs | 172 ++++++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 crates/ui-egui/tests/ui_light_settings.rs diff --git a/crates/ui-egui/tests/ui_light_settings.rs b/crates/ui-egui/tests/ui_light_settings.rs new file mode 100644 index 00000000..a4284423 --- /dev/null +++ b/crates/ui-egui/tests/ui_light_settings.rs @@ -0,0 +1,172 @@ +//! Light Settings dialog (Layer ▸ New ▸ Light, Layer Settings on a light): every light type the +//! engine has, Environment included, and an existing light's type kept when it's reopened (#1588). + +use effectcraft_engine::Session; +use effectcraft_engine::project::{Layer, LayerId, LayerSource, LightKind}; +use effectcraft_ui_egui::{Dialog, EffectcraftApp}; +use egui_kittest::Harness; +use serde_json::{Value, json}; + +fn harness() -> Harness<'static, EffectcraftApp> { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Lights", "width": 640, "height": 360, "duration": 2})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(s)); + h.step(); + h +} + +/// An automation element's rect in the last frame drawn (a dialog settles its place in its +/// first frames, so the frame before can be stale). +fn element(h: &Harness<'_, EffectcraftApp>, id: &str) -> Option { + let auto = &h.state().auto; + auto.elements.iter().chain(auto.previous.iter()).find(|e| e.id == id).cloned() +} + +fn click_id(h: &mut Harness<'_, EffectcraftApp>, id: &str) { + let [x, y, w, hh] = element(h, id).unwrap_or_else(|| panic!("{id} is registered")).rect; + let p = egui::pos2(x + w / 2.0, y + hh / 2.0); + h.input_mut().events.push(egui::Event::PointerMoved(p)); + h.step(); + h.input_mut().events.push(egui::Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE }); + h.step(); + h.input_mut().events.push(egui::Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE }); + h.run_steps(3); +} + +fn invoke(h: &mut Harness<'_, EffectcraftApp>, id: &str, params: Value) { + let ctx = h.ctx.clone(); + effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, id, params).unwrap(); + h.run_steps(2); +} + +/// Picks the dialog's Light Type `kind` from its menu. +fn choose(h: &mut Harness<'_, EffectcraftApp>, kind: LightKind) { + click_id(h, "dialog.light.type"); + let i = LightKind::ALL.iter().position(|k| *k == kind).unwrap(); + click_id(h, &format!("dialog.light.typeItem.{i}")); +} + +fn ok(h: &mut Harness<'_, EffectcraftApp>) { + click_id(h, "dialog.light.ok"); + assert_eq!(h.state().dialog, None); +} + +fn layer(h: &Harness<'_, EffectcraftApp>, id: u64) -> Layer { + h.state().session.active_comp().unwrap().layer(LayerId(id)).unwrap().clone() +} + +fn kind(h: &Harness<'_, EffectcraftApp>, id: u64) -> LightKind { + match layer(h, id).source { + LayerSource::Light { kind } => kind, + other => panic!("not a light: {other:?}"), + } +} + +fn last_layer(h: &Harness<'_, EffectcraftApp>) -> u64 { + h.state().session.active_comp().unwrap().layers.iter().map(|l| l.id.0).max().unwrap() +} + +/// Opens Light Settings for an existing light, as Layer Settings does. +fn reopen(h: &mut Harness<'_, EffectcraftApp>, id: u64) { + invoke(h, "layer.settings", json!({"layer": id})); + assert_eq!(h.state().dialog, Some(Dialog::LightSettings)); +} + +/// An Environment light lit from a solid, its image turned 30°. +fn environment_light(h: &mut Harness<'_, EffectcraftApp>) -> (u64, u64) { + let s = &mut h.state_mut().session; + let sky = s.execute("layer.newSolid", json!({"width": 256, "height": 128, "name": "Sky"})).unwrap()["layer"].as_u64().unwrap(); + let env = s.execute("layer.newLight", json!({"kind": "Environment", "name": "Env", "intensity": 80})).unwrap()["layer"].as_u64().unwrap(); + s.execute("prop.set", json!({"layer": env, "path": "lightOptions/source", "value": sky})).unwrap(); + s.execute("prop.set", json!({"layer": env, "path": "lightOptions/rotation", "value": 30})).unwrap(); + h.run_steps(2); + (env, sky) +} + +#[test] +fn the_light_type_menu_lists_every_light_type() { + let mut h = harness(); + invoke(&mut h, "layer.newLight", json!({})); + click_id(&mut h, "dialog.light.type"); + for (i, k) in LightKind::ALL.iter().enumerate() { + let item = element(&h, &format!("dialog.light.typeItem.{i}")).map(|e| e.label); + assert_eq!(item.as_deref(), Some(k.label())); + } +} + +#[test] +fn new_light_creates_an_environment_light() { + let mut h = harness(); + invoke(&mut h, "layer.newLight", json!({})); + choose(&mut h, LightKind::Environment); + ok(&mut h); + let id = last_layer(&h); + assert_eq!(kind(&h, id), LightKind::Environment); + let l = layer(&h, id); + assert!(l.props.prop("lightOptions/source").is_some() && l.props.prop("lightOptions/falloff").is_none()); + // One undoable step, redone as Environment. + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + assert!(h.state().session.active_comp().unwrap().layer(LayerId(id)).is_none()); + h.state_mut().session.execute("edit.redo", json!({})).unwrap(); + assert_eq!(kind(&h, id), LightKind::Environment); +} + +#[test] +fn reopening_an_environment_light_and_pressing_ok_keeps_it() { + let mut h = harness(); + let (env, sky) = environment_light(&mut h); + let before = layer(&h, env); + reopen(&mut h, env); + ok(&mut h); + let after = layer(&h, env); + assert_eq!(kind(&h, env), LightKind::Environment, "not turned into a Spot light"); + assert_eq!(after.props.prop("lightOptions/source").unwrap().value.as_layer(), Some(sky)); + for path in ["lightOptions/rotation", "lightOptions/intensity"] { + assert_eq!(after.props.prop(path).unwrap().value, before.props.prop(path).unwrap().value, "{path}"); + } +} + +#[test] +fn cancelling_light_settings_changes_nothing() { + let mut h = harness(); + let (env, _) = environment_light(&mut h); + let before = serde_json::to_value(layer(&h, env)).unwrap(); + let steps = h.state().session.history.undo.len(); + reopen(&mut h, env); + choose(&mut h, LightKind::Spot); + click_id(&mut h, "dialog.light.cancel"); + assert_eq!(h.state().dialog, None); + assert_eq!(serde_json::to_value(layer(&h, env)).unwrap(), before); + assert_eq!(h.state().session.history.undo.len(), steps); +} + +#[test] +fn changing_a_light_to_environment_undoes_and_redoes() { + let mut h = harness(); + let id = h.state_mut().session.execute("layer.newLight", json!({"kind": "Spot", "coneAngle": 45})).unwrap()["layer"].as_u64().unwrap(); + h.run_steps(2); + reopen(&mut h, id); + choose(&mut h, LightKind::Environment); + ok(&mut h); + assert_eq!(kind(&h, id), LightKind::Environment); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + assert_eq!(kind(&h, id), LightKind::Spot); + assert_eq!(layer(&h, id).props.prop("lightOptions/coneAngle").unwrap().value.as_f64(), 45.0); + h.state_mut().session.execute("edit.redo", json!({})).unwrap(); + assert_eq!(kind(&h, id), LightKind::Environment); +} + +#[test] +fn the_other_light_types_are_created_and_kept() { + let mut h = harness(); + for k in [LightKind::Parallel, LightKind::Spot, LightKind::Point, LightKind::Ambient] { + invoke(&mut h, "layer.newLight", json!({})); + choose(&mut h, k); + ok(&mut h); + let id = last_layer(&h); + assert_eq!(kind(&h, id), k); + reopen(&mut h, id); + ok(&mut h); + assert_eq!(kind(&h, id), k, "{} kept on reopening", k.label()); + } +} From 83cd74c7305d5b2ce14630828579e556ff9b9ec0 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Wed, 21 Oct 2026 13:14:46 -0400 Subject: [PATCH 20/37] Light Settings: test which options each light type shows, and undoing Environment to Spot (#1588) --- crates/ui-egui/tests/ui_light_settings.rs | 31 ++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/tests/ui_light_settings.rs b/crates/ui-egui/tests/ui_light_settings.rs index a4284423..fe1392be 100644 --- a/crates/ui-egui/tests/ui_light_settings.rs +++ b/crates/ui-egui/tests/ui_light_settings.rs @@ -4,7 +4,7 @@ use effectcraft_engine::Session; use effectcraft_engine::project::{Layer, LayerId, LayerSource, LightKind}; use effectcraft_ui_egui::{Dialog, EffectcraftApp}; -use egui_kittest::Harness; +use egui_kittest::{Harness, kittest::Queryable as _}; use serde_json::{Value, json}; fn harness() -> Harness<'static, EffectcraftApp> { @@ -170,3 +170,32 @@ fn the_other_light_types_are_created_and_kept() { assert_eq!(kind(&h, id), k, "{} kept on reopening", k.label()); } } + +#[test] +fn falloff_and_shadow_options_show_only_for_lights_that_have_them() { + let mut h = harness(); + invoke(&mut h, "layer.newLight", json!({})); + for k in LightKind::ALL { + choose(&mut h, k); + let shown = !matches!(k, LightKind::Ambient | LightKind::Environment); + for label in ["Falloff", "Casts Shadows", "Shadow Darkness"] { + assert_eq!(h.query_by_label(label).is_some(), shown, "{label} for {}", k.label()); + } + } +} + +#[test] +fn changing_an_environment_light_to_spot_undoes_to_its_source_and_rotation() { + let mut h = harness(); + let (env, sky) = environment_light(&mut h); + reopen(&mut h, env); + choose(&mut h, LightKind::Spot); + ok(&mut h); + assert_eq!(kind(&h, env), LightKind::Spot); + assert!(layer(&h, env).props.prop("lightOptions/source").is_none()); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + let l = layer(&h, env); + assert_eq!(kind(&h, env), LightKind::Environment); + assert_eq!(l.props.prop("lightOptions/source").unwrap().value.as_layer(), Some(sky)); + assert_eq!(l.props.prop("lightOptions/rotation").unwrap().value.as_f64(), 30.0); +} From 174f401fe17e09ad60df7e68967ff16874acfecc Mon Sep 17 00:00:00 2001 From: Christopher Linnett Date: Fri, 23 Oct 2026 01:05:06 +0800 Subject: [PATCH 21/37] timeline: pick layer-valued properties from a layer drop-down (#2093) The Timeline drew a layer-valued property (an Environment light's Source, a Displacement Map's layer parameter) as read-only text holding the raw layer id, so a light's Source could not be chosen in the UI at all - only through prop.set from the command line, scripting or MCP. Effect Controls already had a working drop-down for exactly these values: extract it into one shared widgets::layer_picker (the current entry, "None" first, then "N. name" for every layer of the comp, the list built only while the popup is open) and draw it from both panels. In the Timeline the pick writes through the same set closure as every other value arm - an undoable prop.set (layer id, or null for None) - and registers timeline.prop..value like its neighbours, so the row names the layer instead of exposing its id. Effect Controls keeps its rect, automation id and write; its closed dropdown's "None", previously a hardcoded literal, now follows the interface language. The dead "Layer {n}" id text leaves the i18n catalogs with it. ui_layer_source_picker drives both panels headlessly (open, pick, undo, dismiss, current-entry highlight, translated None, picking None to clear a set value), and an ignored wgpu snapshot test keeps the visual record reproducible. --- crates/ui-egui/src/i18n/ui.rs | 7 - crates/ui-egui/src/panels/effect_controls.rs | 13 +- crates/ui-egui/src/panels/timeline.rs | 17 +- crates/ui-egui/src/widgets.rs | 38 ++++ .../ui-egui/tests/ui_layer_source_picker.rs | 211 ++++++++++++++++++ 5 files changed, 261 insertions(+), 25 deletions(-) create mode 100644 crates/ui-egui/tests/ui_layer_source_picker.rs diff --git a/crates/ui-egui/src/i18n/ui.rs b/crates/ui-egui/src/i18n/ui.rs index 434f2b79..c0b61996 100644 --- a/crates/ui-egui/src/i18n/ui.rs +++ b/crates/ui-egui/src/i18n/ui.rs @@ -294,7 +294,6 @@ const JAPANESE: &[(&str, &str)] = &[ ("Latest auto-save: {}", "最新の自動保存：{}"), ("Layer", "レイヤー"), ("Layer controls", "レイヤーコントロール"), - ("Layer {}", "レイヤー{}"), ("Layer:", "レイヤー:"), ("Layer: {}", "レイヤー：{}"), ("Layer Styles", "レイヤースタイル"), @@ -1029,7 +1028,6 @@ const SIMPLIFIED_CHINESE: &[(&str, &str)] = &[ ("Latest auto-save: {}", "最新的自动保存：{}"), ("Layer", "图层"), ("Layer controls", "图层控制"), - ("Layer {}", "图层 {}"), ("Layer:", "图层："), ("Layer: {}", "图层：{}"), ("Layer Styles", "图层样式"), @@ -1769,7 +1767,6 @@ const TRADITIONAL_CHINESE: &[(&str, &str)] = &[ ("Latest auto-save: {}", "最新的自動儲存：{}"), ("Layer", "圖層"), ("Layer controls", "圖層控制"), - ("Layer {}", "圖層 {}"), ("Layer:", "圖層："), ("Layer: {}", "圖層：{}"), ("Layer Styles", "圖層樣式"), @@ -2512,7 +2509,6 @@ const UKRAINIAN: &[(&str, &str)] = &[ ("Latest auto-save: {}", "Останнє автозбереження: {}"), ("Layer", "Шар"), ("Layer controls", "Керування шарами"), - ("Layer {}", "Шар {}"), ("Layer:", "Шар:"), ("Layer: {}", "Шар: {}"), ("Layer Styles", "Стилі шару"), @@ -3070,7 +3066,6 @@ const PORTUGUESE: &[(&str, &str)] = &[ ("Latest auto-save: {}", "Último salvamento automático: {}"), ("Layer", "Camada"), ("Layer controls", "Controles da camada"), - ("Layer {}", "Camada {}"), ("Layer:", "Camada:"), ("Layer: {}", "Camada: {}"), ("Layer Styles", "Estilos de camada"), @@ -3529,7 +3524,6 @@ const CZECH: &[(&str, &str)] = &[ ("Latest auto-save: {}", "Nejnovější automatická záloha: {}"), ("Layer", "Vrstva"), ("Layer controls", "Ovládací prvky vrstvy"), - ("Layer {}", "Vrstva {}"), ("Layer:", "Vrstva:"), ("Layer: {}", "Vrstva: {}"), ("Layer Styles", "Styly vrstvy"), @@ -4092,7 +4086,6 @@ const SPANISH: &[(&str, &str)] = &[ ("Latest auto-save: {}", "Último guardado automático: {}"), ("Layer", "Capa"), ("Layer controls", "Controles de capa"), - ("Layer {}", "Capa {}"), ("Layer:", "Capa:"), ("Layer: {}", "Capa: {}"), ("Layer Styles", "Estilos de capa"), diff --git a/crates/ui-egui/src/panels/effect_controls.rs b/crates/ui-egui/src/panels/effect_controls.rs index 03b3a1ac..f930aff2 100644 --- a/crates/ui-egui/src/panels/effect_controls.rs +++ b/crates/ui-egui/src/panels/effect_controls.rs @@ -441,19 +441,10 @@ fn prop_row( let comp = app.session.active_comp_arc(); if let Some(comp) = comp { let dr = Rect::from_min_size(pos2(vx, cy - 9.0), vec2(((r.max.x - vx) * 0.55).clamp(90.0, 170.0), 18.0)); - let idx_of = |id: u64| comp.layers.iter().position(|x| x.id.0 == id); - let name = l.and_then(|id| idx_of(id).map(|i| format!("{}. {}", i + 1, comp.layers[i].name))).unwrap_or_else(|| "None".into()); - let pop = egui::Id::new(("ec-lpop", uid)); - if widgets::dropdown(ui, dr, &name, &t, egui::Id::new(("ec-l", uid))).clicked() { - widgets::open_popup(ui, pop); + if let Some(chosen) = widgets::layer_picker(ui, dr, &comp.layers, *l, &t, egui::Id::new(("ec-l", uid))) { + set(actions, chosen.map(|id| json!(id)).unwrap_or(serde_json::Value::Null)); } app.auto.add(&format!("effectControls.prop.{uid}.value"), dr, &prop.name); - let mut opts = vec!["None".to_string()]; - opts.extend(comp.layers.iter().enumerate().map(|(i, l)| format!("{}. {}", i + 1, l.name))); - let cur = l.and_then(idx_of).map(|i| i + 1).unwrap_or(0); - if let Some(i) = widgets::popup_menu(ui, pop, dr.left_bottom(), &opts, Some(cur)) { - set(actions, if i == 0 { serde_json::Value::Null } else { json!(comp.layers[i - 1].id.0) }); - } // Source / Masks / Effects & Masks (the hidden companion parameter). if let Some(src) = group.get(&effectcraft_engine::effects::layer_source_id(&prop.match_id)) { let si = src.value.as_enum() as usize; diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index b34fbd9d..a48cc3e4 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -4233,13 +4233,16 @@ fn value_editor( } Value::Text(_) => {} Value::Layer(l) => { - p.text( - pos2(x, at.y), - Align2::LEFT_CENTER, - l.map(|l| crate::i18n::tr_args("Layer {}", &[&l])).unwrap_or(crate::i18n::tr("None").into()), - Tokens::ui(12.0), - t.hot_text, - ); + // Layer references (a light's Source, a Displacement Map's layer…) pick from the + // comp's layers, as in Effect Controls; the choice is written through the same + // undoable prop.set as the other value arms. + let r = Rect::from_min_size(pos2(x, at.y - 9.0), vec2(150.0, 18.0)); + if let Some(comp) = app.session.active_comp_arc() + && let Some(chosen) = widgets::layer_picker(ui, r, &comp.layers, *l, &t, egui::Id::new(("tl-lay", uid))) + { + set(actions, chosen.map(|id| json!(id)).unwrap_or(serde_json::Value::Null)); + } + app.auto.add(&format!("timeline.prop.{uid}.value"), r, &prop.name); } Value::Gradient(_) => { // Opens the Gradient Editor at the current time (#612). diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index c005224d..f74cc048 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -557,6 +557,44 @@ pub fn popup_menu(ui: &mut Ui, id: egui::Id, pos: egui::Pos2, options: &[String] }) } +/// The drop-down for a layer-valued property (`ParamUi::Layer`): the current layer shown as +/// "N. name" (or "None"), and a popup of "None" plus every layer of the comp. Effect Controls +/// and the Timeline share it so a layer reference (an Environment light's Source, a Displacement +/// Map's layer) is picked the same way in every panel, and the reference is shown as its layer's +/// index and name rather than its internal id. Returns `Some(chosen)` when the user picked — +/// the layer's id, or `None` for "None" — and `None` while nothing is chosen; the caller writes +/// it as an undoable `prop.set` (id, or null for "None"). +pub fn layer_picker( + ui: &mut Ui, + rect: Rect, + layers: &[effectcraft_engine::project::Layer], + current: Option, + t: &Tokens, + id: egui::Id, +) -> Option> { + let index_of = |wanted: u64| layers.iter().position(|l| l.id.0 == wanted); + let entry = |i: usize| layers.get(i).map(|l| format!("{}. {}", i + 1, l.name)); + let current_index = current.and_then(index_of); + let label = current_index.and_then(entry).unwrap_or_else(|| crate::i18n::tr("None").to_string()); + let pop = id.with("pop"); + if dropdown(ui, rect, &label, t, id).clicked() { + open_popup(ui, pop); + } + if !popup_is_open(ui, pop) { + return None; + } + // Built only while open: a comp can hold hundreds of layers. + let mut options: Vec = Vec::with_capacity(layers.len() + 1); + options.push(crate::i18n::tr("None").to_string()); + options.extend((0..layers.len()).filter_map(entry)); + // The current entry is highlighted (the first option is "None"). + match popup_menu(ui, pop, rect.left_bottom(), &options, Some(current_index.map_or(0, |i| i + 1))) { + Some(0) => Some(None), + Some(picked) => layers.get(picked - 1).map(|l| Some(l.id.0)), + None => None, + } +} + /// The open popup list `id` at `pos`, drawn by `body` (`rows` entries and `seps` separators, /// for its height): as tall as the window allows, scrolling beyond with a scroll bar (a shape /// layer's Add menu has 20 entries, the blend modes 40, the fonts hundreds), and moved up when diff --git a/crates/ui-egui/tests/ui_layer_source_picker.rs b/crates/ui-egui/tests/ui_layer_source_picker.rs new file mode 100644 index 00000000..9b952de7 --- /dev/null +++ b/crates/ui-egui/tests/ui_layer_source_picker.rs @@ -0,0 +1,211 @@ +//! Picking a layer-valued property with the shared drop-down: the Timeline draws a layer-valued +//! Light Options property (an Environment light's Source) as a drop-down of the comp's layers, +//! Effect Controls' layer parameters (a Displacement Map's layer) pick through the same widget, +//! and both write an undoable `prop.set`. + +use effectcraft_engine::Session; +use effectcraft_engine::keyframe::Value; +use effectcraft_engine::project::LayerId; +use effectcraft_ui_egui::EffectcraftApp; +use effectcraft_ui_egui::dock::PanelKind; +use egui::{Pos2, Rect, pos2, vec2}; +use egui_kittest::Harness; +use egui_kittest::kittest::NodeT; +use egui_kittest::kittest::Queryable; +use serde_json::json; + +fn click(h: &mut Harness<'_, EffectcraftApp>, at: Pos2) { + h.event(egui::Event::PointerMoved(at)); + h.step(); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() }); + h.step(); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }); + h.run_steps(3); +} + +fn rect_of(h: &Harness<'_, EffectcraftApp>, id: &str) -> Rect { + let e = h.state().auto.find(id).unwrap_or_else(|| panic!("no automation element {id}")).clone(); + Rect::from_min_size(pos2(e.rect[0], e.rect[1]), vec2(e.rect[2], e.rect[3])) +} + +/// Whether `want` is painted inside `row` (the drop-down's current-value text). +fn painted_in(h: &Harness<'_, EffectcraftApp>, row: Rect, want: &str) -> bool { + h.output().shapes.iter().any(|c| match &c.shape { + egui::Shape::Text(t) => t.galley.text() == want && row.expand(3.0).contains(t.pos), + _ => false, + }) +} + +/// The current value of a layer-valued property, by uid. +fn layer_value(h: &Harness<'_, EffectcraftApp>, layer: u64, prop: u64) -> Option { + let comp = h.state().session.active_comp().unwrap(); + match &comp.layer(LayerId(layer)).unwrap().props.find(prop).unwrap().value { + Value::Layer(v) => *v, + other => panic!("property {prop} is not layer-valued: {other:?}"), + } +} + +/// The "N. name" a drop-down shows for a layer. +fn entry_for(h: &Harness<'_, EffectcraftApp>, layer: u64) -> String { + let comp = h.state().session.active_comp().unwrap(); + let i = comp.layers.iter().position(|l| l.id.0 == layer).unwrap(); + format!("{}. {}", i + 1, comp.layers[i].name) +} + +/// Puts the layer-valued `prop` of `layer` back to `value` with a direct `prop.set` (no merge key, +/// so the pick below records its own step instead of folding into a picker write), picks `none` +/// from the drop-down at `value_id`, and checks the clear takes one undo step that restores `value`. +#[track_caller] +fn none_pick_clears(h: &mut Harness<'_, EffectcraftApp>, value_id: &str, none: &str, layer: u64, prop: u64, value: u64) { + h.state_mut().session.execute("prop.set", json!({"layer": layer, "prop": prop, "value": value})).unwrap(); + h.run_steps(2); + assert_eq!(layer_value(h, layer, prop), Some(value)); + let steps = h.state().session.history.undo.len(); + let at = rect_of(h, value_id).center(); + click(h, at); + h.get_by_label(none).click(); + h.run_steps(3); + assert_eq!(layer_value(h, layer, prop), None, "selecting None clears the value"); + assert_eq!(h.state().session.history.undo.len(), steps + 1, "one clear undo step"); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + h.run_steps(2); + assert_eq!(layer_value(h, layer, prop), Some(value), "undo restores the selected layer"); +} + +/// A light's Source (a layer-valued Light Options property) picks a comp layer from the +/// Timeline and undo restores None. +#[test] +fn timeline_source_drop_down_picks_the_layer_and_undo_restores_none() { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Main", "width": 640, "height": 360, "frameRate": 30, "duration": 4})).unwrap(); + s.execute("comp.settings", json!({"renderer": "advanced3D"})).unwrap(); + let sky = s.execute("layer.newSolid", json!({"name": "Sky HDRI", "color": "#66aaff"})).unwrap()["layer"].as_u64().unwrap(); + let light = s.execute("layer.newLight", json!({"kind": "Environment", "name": "Env Light"})).unwrap()["layer"].as_u64().unwrap(); + let (group, source) = { + let comp = s.active_comp().unwrap(); + let opts = comp.layer(LayerId(light)).unwrap().props.sub("lightOptions").unwrap(); + (opts.uid, opts.get("source").unwrap().uid) + }; + let mut app = EffectcraftApp::new(s); + app.ui.timeline.open_layers.insert(light); + app.ui.timeline.open_groups.insert(group); + let mut h = Harness::builder().with_size(vec2(1600.0, 1000.0)).build_eframe(|_| app); + h.run_steps(4); + + let value_id = format!("timeline.prop.{source}.value"); + // Unset: the row is a drop-down reading "None" — an element to click, not bare text. + assert_eq!(layer_value(&h, light, source), None); + assert!(painted_in(&h, rect_of(&h, &value_id), "None"), "the Source row shows a None drop-down"); + + // Opening it lists "None" and every layer of the comp, by index and name. + let at = rect_of(&h, &value_id).center(); + click(&mut h, at); + let entry = entry_for(&h, sky); + assert!(h.query_by_label("None").is_some(), "the list starts with None"); + assert!(h.query_by_label(&entry).is_some(), "the comp's layers are listed ({entry})"); + + let steps = h.state().session.history.undo.len(); + h.get_by_label(&entry).click(); + h.run_steps(3); + assert_eq!(layer_value(&h, light, source), Some(sky), "the Source holds the layer id"); + assert_eq!(h.state().session.history.undo.len(), steps + 1, "one undo step"); + // The row names the layer (index and name), not its internal id. + assert!(painted_in(&h, rect_of(&h, &value_id), &entry), "the Source row shows the layer's name"); + + // Dismissing the list without choosing writes nothing; reopening marks the chosen layer as + // the current entry (and "None" as not current). + click(&mut h, pos2(1500.0, 300.0)); + h.run_steps(2); + assert_eq!(layer_value(&h, light, source), Some(sky), "dismissing the list writes nothing"); + let reopen = rect_of(&h, &value_id).center(); + click(&mut h, reopen); + assert_eq!(h.get_by_label(&entry).accesskit_node().toggled(), Some(egui::accesskit::Toggled::True), "the current entry is highlighted"); + assert_eq!(h.get_by_label("None").accesskit_node().toggled(), Some(egui::accesskit::Toggled::False), "None is not the current entry"); + click(&mut h, pos2(1500.0, 300.0)); + h.run_steps(2); + + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + h.run_steps(2); + assert_eq!(layer_value(&h, light, source), None, "undo restores None"); + + none_pick_clears(&mut h, &value_id, "None", light, source, sky); +} + +/// The shared widget is the one Effect Controls already used: a Displacement Map's layer +/// parameter still picks from the drop-down and still undoes. The interface language is +/// Japanese: "None" is the catalog's word there, like every other label drawn in the panel. +#[test] +fn effect_controls_layer_param_picks_through_the_shared_widget() { + let mut s = Session::default(); + s.prefs.general.language = "ja".to_string(); + s.execute("comp.new", json!({"name": "Main", "width": 640, "height": 360, "frameRate": 30, "duration": 4})).unwrap(); + let map = s.execute("layer.newSolid", json!({"name": "Disp", "color": "#ff0000"})).unwrap()["layer"].as_u64().unwrap(); + let base = s.execute("layer.newSolid", json!({"name": "Base", "color": "#20c040"})).unwrap()["layer"].as_u64().unwrap(); + let fx = s.execute("effect.apply", json!({"layers": [base], "effect": "Displacement Map"})).unwrap()["effects"][0].as_u64().unwrap(); + let prop = { + let comp = s.active_comp().unwrap(); + comp.layer(LayerId(base)).unwrap().effects().unwrap().find_group(fx).unwrap().get("displacementMapLayer").unwrap().uid + }; + s.state.selected_layers = vec![LayerId(base)]; + let mut app = EffectcraftApp::new(s); + app.show_panel(PanelKind::EffectControls); + let mut h = Harness::builder().with_size(vec2(1600.0, 1100.0)).build_eframe(|_| app); + h.run_steps(4); + + let value_id = format!("effectControls.prop.{prop}.value"); + // The closed drop-down is the translated "None" (なし), not a hardcoded English word. + let row = rect_of(&h, &value_id); + assert!(painted_in(&h, row, "なし"), "the parameter row shows the translated None"); + click(&mut h, row.center()); + let entry = entry_for(&h, map); + assert!(h.query_by_label(&entry).is_some(), "the comp's layers are listed ({entry})"); + let steps = h.state().session.history.undo.len(); + h.get_by_label(&entry).click(); + h.run_steps(3); + assert_eq!(layer_value(&h, base, prop), Some(map), "the parameter holds the layer id"); + assert_eq!(h.state().session.history.undo.len(), steps + 1, "one undo step"); + // The row names the picked layer, as the Timeline's Source row does. + assert!(painted_in(&h, rect_of(&h, &value_id), &entry), "the parameter row shows the layer's name"); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + h.run_steps(2); + assert_eq!(layer_value(&h, base, prop), None, "undo restores None"); + + none_pick_clears(&mut h, &value_id, "なし", base, prop, map); +} + +/// The visual record (the `ui_3d` ignored-snapshot convention): the Source drop-down open over +/// the layer list, and the row after a layer is chosen. Renders with wgpu when `EC_SNAPSHOT_DIR` +/// is set: `EC_SNAPSHOT_DIR=shots cargo test -p effectcraft-ui-egui --test ui_layer_source_picker -- --ignored`. +#[test] +#[ignore = "renders with wgpu; run with --ignored (set EC_SNAPSHOT_DIR to keep PNGs)"] +fn snapshot_source_dropdown_open_and_chosen() { + 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(); + let sky = s.execute("layer.newSolid", json!({"name": "Sky HDRI", "color": "#66aaff"})).unwrap()["layer"].as_u64().unwrap(); + let light = s.execute("layer.newLight", json!({"kind": "Environment", "name": "Env Light"})).unwrap()["layer"].as_u64().unwrap(); + let (group, source) = { + let comp = s.active_comp().unwrap(); + let opts = comp.layer(LayerId(light)).unwrap().props.sub("lightOptions").unwrap(); + (opts.uid, opts.get("source").unwrap().uid) + }; + let mut app = EffectcraftApp::new(s); + app.ui.timeline.open_layers.insert(light); + app.ui.timeline.open_groups.insert(group); + let mut h = Harness::builder().with_size(vec2(1600.0, 1000.0)).with_pixels_per_point(2.0).wgpu().build_eframe(|_| app); + 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"); + } + }; + save(&mut h, "source_unset.png"); + let at = rect_of(&h, &format!("timeline.prop.{source}.value")).center(); + click(&mut h, at); + save(&mut h, "source_dropdown_open.png"); + let entry = entry_for(&h, sky); + h.get_by_label(&entry).click(); + h.run_steps(3); + assert_eq!(layer_value(&h, light, source), Some(sky)); + save(&mut h, "source_chosen.png"); +} From a07eb51dbfe469f4f00e1e1e3e5eaca4de38e082 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:04:20 +0200 Subject: [PATCH 22/37] Fix u32 overflows in PDF CID width tables and CMap ranges (#2147) --- crates/pdf/src/font.rs | 18 ++++++++++------- crates/pdf/src/tests.rs | 44 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/crates/pdf/src/font.rs b/crates/pdf/src/font.rs index 296878c3..2c27838a 100644 --- a/crates/pdf/src/font.rs +++ b/crates/pdf/src/font.rs @@ -263,8 +263,8 @@ fn parse_cmap(data: &[u8]) -> CMap { } Obj::Array(list) => { for (k, s) in list.iter().enumerate() { - if let Obj::Str(s) = s { - m.unicode.insert(lo + k as u32, utf16(s)); + if let (Obj::Str(s), Some(c)) = (s, lo.checked_add(k as u32)) { + m.unicode.insert(c, utf16(s)); } } } @@ -401,13 +401,15 @@ impl Font { match w.get(i + 1).map(|x| file.resolve(x)) { Some(Obj::Array(list)) => { for (k, v) in list.iter().enumerate() { - f.cid_widths.insert(first + k as u32, file.resolve(v).num().unwrap_or(0.0) / 1000.0); + if let Some(cid) = first.checked_add(k as u32) { + f.cid_widths.insert(cid, file.resolve(v).num().unwrap_or(0.0) / 1000.0); + } } i += 2; } Some(Obj::Num(last)) => { let wv = w.get(i + 2).and_then(|x| file.resolve(x).num()).unwrap_or(0.0) / 1000.0; - for c in first..=(*last as u32).min(first + 65535) { + for c in first..=(*last as u32).min(first.saturating_add(65535)) { f.cid_widths.insert(c, wv); } i += 3; @@ -432,13 +434,15 @@ impl Font { Some(Obj::Array(list)) => { let v: Vec = list.iter().filter_map(|x| file.resolve(x).num()).collect(); for (k, t) in v.as_chunks::<3>().0.iter().enumerate() { - f.cid_vmetrics.insert(first + k as u32, (t[0] / 1000.0, t[1] / 1000.0, t[2] / 1000.0)); + if let Some(cid) = first.checked_add(k as u32) { + f.cid_vmetrics.insert(cid, (t[0] / 1000.0, t[1] / 1000.0, t[2] / 1000.0)); + } } i += 2; } Some(Obj::Num(last)) => { let (Some(w1), Some(vx), Some(vy)) = (num(i + 2), num(i + 3), num(i + 4)) else { break }; - for c in first..=(last.max(0.0) as u32).min(first + 65535) { + for c in first..=(last.max(0.0) as u32).min(first.saturating_add(65535)) { f.cid_vmetrics.insert(c, (w1 / 1000.0, vx / 1000.0, vy / 1000.0)); } i += 5; @@ -609,7 +613,7 @@ impl Font { if self.cid_ranges.is_empty() { return code; } - self.cid_ranges.iter().find(|(lo, hi, _)| code >= *lo && code <= *hi).map(|(lo, _, c)| c + (code - lo)).unwrap_or(0) + self.cid_ranges.iter().find(|(lo, hi, _)| code >= *lo && code <= *hi).and_then(|(lo, _, c)| c.checked_add(code - lo)).unwrap_or(0) } /// Vertical metrics of a code in text space (before the font size): the vertical diff --git a/crates/pdf/src/tests.rs b/crates/pdf/src/tests.rs index f2eff046..b15996a1 100644 --- a/crates/pdf/src/tests.rs +++ b/crates/pdf/src/tests.rs @@ -1090,3 +1090,47 @@ fn retained_layers_preserve_artwork_beyond_the_bounds_depth_limit() { assert_eq!(cropped.pixel_size(), (120, 80), "unknown bounds keep the full page"); assert_eq!(offset, [0.0, 0.0]); } + +#[test] +fn cid_width_entries_at_the_top_of_the_cid_range_do_not_overflow() { + // A first CID of u32::MAX (larger values saturate to it) used to overflow `first + k` and `first + 65535`: + // a panic in debug builds; in release builds the list wrapped round to CID 0 and the range was dropped. + let load = |entry: &str| { + let type0 = "<< /Type /Font /Subtype /Type0 /BaseFont /X /Encoding /Identity-H /DescendantFonts [13 0 R] >>"; + let desc = format!("<< /Type /Font /Subtype /CIDFontType2 {entry} >>"); + let file = object::File::parse(&page_pdf("", "", vec![(10, type0.into(), None), (13, desc, None)])).unwrap(); + let d = file.objects.get(&(10, 0)).and_then(object::Obj::dict).unwrap(); + font::Font::load(&file, d, &mut vec![]) + }; + let max = u32::MAX; + // c [w1 w2 ...] + let f = load("/W [4294967295 [500 600 700]]"); + assert_eq!((f.width(max), f.width(0)), (0.5, 1.0)); + // cfirst clast w + assert_eq!(load("/W [4294967295 4294967295 400]").width(max), 0.4); + // c [w1y vx vy ...] + let f = load("/W2 [4294967295 [1 2 3 4 5 6]]"); + assert_eq!((f.vmetrics(max), f.vmetrics(0)), ((0.001, 0.002, 0.003), (-1.0, 0.5, 0.88))); + // cfirst clast w1y vx vy + assert_eq!(load("/W2 [4294967295 4294967295 1 2 3]").vmetrics(max), (0.001, 0.002, 0.003)); +} + +#[test] +fn cmap_entries_at_the_top_of_the_code_range_do_not_overflow() { + // A bfrange array starting at code 0xFFFFFFFF and a cidrange mapping onto CID u32::MAX used to overflow + // `lo + k` and `c + (code - lo)`: a panic in debug builds, a wrap round to code / CID 0 in release builds. + let to_unicode = "beginbfrange [<0041> <0042>] endbfrange"; + let encoding = "begincodespacerange <0000> endcodespacerange begincidrange <0000> 4294967295 endcidrange"; + let objs: Objs = vec![ + (10, "<< /Type /Font /Subtype /Type0 /Encoding 15 0 R /ToUnicode 14 0 R /DescendantFonts [13 0 R] >>".into(), None), + (13, "<< /Type /Font /Subtype /CIDFontType2 /W [0 [700 800] 4294967295 [500]] >>".into(), None), + (14, "<< >>".into(), Some(to_unicode.as_bytes().to_vec())), + (15, "<< /Type /CMap >>".into(), Some(encoding.as_bytes().to_vec())), + ]; + let file = object::File::parse(&page_pdf("", "", objs)).unwrap(); + let d = file.objects.get(&(10, 0)).and_then(object::Obj::dict).unwrap(); + let f = font::Font::load(&file, d, &mut vec![]); + assert_eq!((f.unicode(0xFFFF_FFFF), f.unicode(0)), ("A".to_string(), String::new())); + // Code 0 is CID u32::MAX; code 2 would be past it, so it falls back to CID 0 like an unmapped code. + assert_eq!((f.width(0), f.width(2)), (0.5, 0.7)); +} From 88045641f32a1b880beb70f506f752bbe668d9e0 Mon Sep 17 00:00:00 2001 From: Anubhav-Rai Date: Tue, 20 Oct 2026 20:38:58 +0530 Subject: [PATCH 23/37] script: comp.width and comp.height resize the frame without moving the layers (#1942) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The setters go through `comp.settings`, which defaults to the Composition Settings dialog's centre anchor, so `comp.width = 800` on a 400-wide comp shifted every unparented layer — and all of its position keys — by 200 px. A script that resized a precomp then found its pixel coordinates had moved under it. After Effects' width and height setters only change the frame; moving the contents is what "resize from centre" scripts do for themselves. The setters now pass `anchor: 0` (top left). The dialog, which is where the anchor belongs, is untouched. Co-Authored-By: Claude Opus 5 --- crates/script/src/prelude.js | 7 +++++-- crates/script/src/tests.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/crates/script/src/prelude.js b/crates/script/src/prelude.js index 52712bc0..2354830c 100644 --- a/crates/script/src/prelude.js +++ b/crates/script/src/prelude.js @@ -1506,8 +1506,11 @@ function AVItem() {} AVItem.prototype = Object.create(Item.prototype); (function () { function def(name, get, set) { Object.defineProperty(AVItem.prototype, name, { get: get, set: set }); } - def("width", function () { return this.__info().width; }, function (v) { this.__settings({ width: Math.round(__num(v, "width")) }); }); - def("height", function () { return this.__info().height; }, function (v) { this.__settings({ height: Math.round(__num(v, "height")) }); }); + // `anchor: 0` (top left): After Effects' width and height setters resize the frame and leave the + // layers at their coordinates. Re-anchoring the old frame in the new one belongs to the + // Composition Settings dialog, which is what `comp.settings` defaults to (#1942). + def("width", function () { return this.__info().width; }, function (v) { this.__settings({ width: Math.round(__num(v, "width")), anchor: 0 }); }); + def("height", function () { return this.__info().height; }, function (v) { this.__settings({ height: Math.round(__num(v, "height")), anchor: 0 }); }); def("pixelAspect", function () { return this.__info().pixelAspect; }, function (v) { this.__settings({ pixelAspect: __num(v) }); }); def("duration", function () { return this.__info().duration; }, function (v) { this.__settings({ duration: __num(v, "duration") }); }); def("frameRate", function () { return this.__info().frameRate; }, function (v) { this.__settings({ frameRate: __num(v, "frameRate") }); }); diff --git a/crates/script/src/tests.rs b/crates/script/src/tests.rs index a1e96194..a32289aa 100644 --- a/crates/script/src/tests.rs +++ b/crates/script/src/tests.rs @@ -1623,3 +1623,30 @@ fn set_value_rejects_an_array_of_the_wrong_dimension() { let (_, c) = comp_named(&s, "Dims"); assert_eq!(layer(c, "s").props.prop("transform/position").unwrap().value, KV::Vec3([120.0, 34.0, 0.0])); } + +/// #1942: the scripting width and height setters resize the frame only. Re-anchoring the old frame +/// in the new one, which moves every unparented layer, belongs to the Composition Settings dialog. +#[test] +fn setting_comp_width_and_height_leaves_the_layers_where_they_are() { + let mut s = session(); + let o = ok( + &mut s, + r#" + var c = app.project.items.addComp("w", 400, 200, 1, 1, 25); + var l = c.layers.addSolid([1, 1, 1], "s", 50, 50, 1, 1); + var pos = l.property("ADBE Transform Group").property("ADBE Position"); + pos.setValue([100, 100]); + var k = c.layers.addSolid([1, 1, 1], "k", 50, 50, 1, 1); + var kpos = k.property("ADBE Transform Group").property("ADBE Position"); + kpos.setValueAtTime(0, [10, 20]); + kpos.setValueAtTime(1, [30, 40]); + var before = "" + pos.value; + c.width = 800; + c.height = 400; + [before, "" + pos.value, "" + kpos.keyValue(1), "" + kpos.keyValue(2), c.width, c.height] + "#, + ); + assert_eq!(o.result, json!(["100,100,0", "100,100,0", "10,20,0", "30,40,0", 800, 400])); + let (_, c) = comp_named(&s, "w"); + assert_eq!(layer(c, "s").props.prop("transform/position").unwrap().value, KV::Vec3([100.0, 100.0, 0.0])); +} From 6e3b5eb3e35510d36c456e634f7890a5060045db Mon Sep 17 00:00:00 2001 From: Anubhav-Rai Date: Fri, 23 Oct 2026 04:37:38 +0530 Subject: [PATCH 24/37] engine: deleting a layer clears the references to it (#2153) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing cleared a `Value::Layer` reference when its layer was deleted, so the id stayed behind: `env_pixels` resolved it to nothing and rendered no environment at all, while the UI showed "None", which is documented as "fall back to the comp's Environment Layer". The delete path already unparents children and drops a track matte that pointed at a deleted layer; a layer reference now goes the same way, in the same history step, so undo restores the layer and the reference together. This covers every layer-reference property, not only an Environment light's Source: 73 of them across the effects (Clone Source, Set Matte, Back Layer, Water Surface, Sky, Ground…). Keyframes carry the same value kind as the property, as in `essential.rs`, and are fixed too. Co-Authored-By: Claude Opus 5 --- crates/engine/src/commands/edit.rs | 16 ++++++++++++++++ crates/engine/src/tests_3d.rs | 17 +++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/crates/engine/src/commands/edit.rs b/crates/engine/src/commands/edit.rs index 019d56d1..4bfe6fa7 100644 --- a/crates/engine/src/commands/edit.rs +++ b/crates/engine/src/commands/edit.rs @@ -1,6 +1,7 @@ //! Edit menu. use effectcraft_color::Label; +use effectcraft_keyframe::Value as KV; use effectcraft_project::{Comp, Expression, ItemKind, Layer, LayerId, LayerSource, TrackMatte}; use effectcraft_time::Tick; use serde_json::{Value, json}; @@ -201,6 +202,21 @@ fn delete(s: &mut Session, p: &Value) -> Result { if l.track_matte.is_some_and(|m| ids.contains(&m.layer)) { l.track_matte = None; } + // A layer-reference property (an Environment light's Source, an effect's Clone Source, + // Back Layer…) must read as None once its layer is gone, in this same history step: a + // dead id renders as nothing while the UI shows "None", which means "fall back to the + // comp's Environment Layer" (#2153). Keys carry the same value kind as the property. + l.props.walk_mut(&mut |p| { + let dead = |v: &KV| matches!(v, KV::Layer(Some(id)) if ids.contains(&LayerId(*id))); + if dead(&p.value) { + p.value = KV::Layer(None); + } + for k in &mut p.keys { + if dead(&k.value) { + k.value = KV::Layer(None); + } + } + }); } st.selected_layers.clear(); st.selected_props.clear(); diff --git a/crates/engine/src/tests_3d.rs b/crates/engine/src/tests_3d.rs index cf9773f9..07a48af1 100644 --- a/crates/engine/src/tests_3d.rs +++ b/crates/engine/src/tests_3d.rs @@ -131,6 +131,23 @@ fn environment_light_is_listed_and_takes_a_source_layer() { assert_eq!(prop(&s, env, "lightOptions/source").as_layer(), None); } +/// Deleting the layer a Source points at leaves no dangling id: "None" means the comp's +/// Environment Layer, and a dead id rendered as nothing at all (#2153). +#[test] +fn deleting_a_source_layer_clears_the_reference() { + let mut s = session(); + let sky = s.execute("layer.newSolid", json!({"width": 256, "height": 128, "name": "Sky"})).unwrap()["layer"].as_u64().unwrap(); + let env = s.execute("layer.newLight", json!({"kind": "Environment", "name": "Env"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("prop.set", json!({"layer": env, "path": "lightOptions/source", "value": sky})).unwrap(); + assert_eq!(prop(&s, env, "lightOptions/source").as_layer(), Some(sky)); + s.execute("edit.clear", json!({"layers": [sky]})).unwrap(); + assert_eq!(prop(&s, env, "lightOptions/source").as_layer(), None, "the reference goes with the layer"); + // One history step with the delete, so undo brings both back. + s.undo(); + assert_eq!(prop(&s, env, "lightOptions/source").as_layer(), Some(sky)); + assert!(s.active_comp().unwrap().layer(effectcraft_project::LayerId(sky)).is_some()); +} + #[test] fn three_d_switch_undo_redo() { let mut s = session(); From 8ac7b3f50876c0444d277e539cef30720c5face1 Mon Sep 17 00:00:00 2001 From: Markus Ang Date: Thu, 22 Oct 2026 19:29:42 -0400 Subject: [PATCH 25/37] #2135: reject non-ASCII thumbnail text safely --- crates/engine/src/templates.rs | 10 ++++++++-- crates/ui-egui/src/panels/home.rs | 11 +++++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/templates.rs b/crates/engine/src/templates.rs index a8009f47..500308b7 100644 --- a/crates/engine/src/templates.rs +++ b/crates/engine/src/templates.rs @@ -486,10 +486,10 @@ pub fn encode_thumb(w: u32, h: u32, rgba: &[u8]) -> String { /// Decode a thumbnail to RGB bytes (THUMB_W×THUMB_H×3). pub fn decode_thumb(text: &str) -> Option> { let t = text.trim(); - if t.len() != THUMB_W * THUMB_H * 6 { + if t.len() != THUMB_W * THUMB_H * 6 || !t.is_ascii() { return None; } - (0..t.len()).step_by(2).map(|i| u8::from_str_radix(&t[i..i + 2], 16).ok()).collect() + (0..t.len()).step_by(2).map(|i| u8::from_str_radix(t.get(i..i + 2)?, 16).ok()).collect() } /// Render a project's main comp at its poster time into a thumbnail. @@ -943,6 +943,12 @@ mod tests { use super::*; use crate::config::{DirConfig, MemoryConfig}; + #[test] + fn non_ascii_thumbnail_is_rejected_without_panicking() { + let malformed = format!("aé{}", "0".repeat(THUMB_W * THUMB_H * 6 - 3)); + assert!(decode_thumb(&malformed).is_none()); + } + #[test] fn every_builtin_builds_with_controls_and_renders() { let s = Session::default(); diff --git a/crates/ui-egui/src/panels/home.rs b/crates/ui-egui/src/panels/home.rs index 46bbb44f..5c7fd982 100644 --- a/crates/ui-egui/src/panels/home.rs +++ b/crates/ui-egui/src/panels/home.rs @@ -154,12 +154,13 @@ pub fn encode_thumb(img: &egui::ColorImage) -> String { out } +/// Decode a stored hex thumbnail, rejecting malformed or non-ASCII text. pub fn decode_thumb(text: &str) -> Option { let t = text.trim(); - if t.len() != THUMB_W * THUMB_H * 6 { + if t.len() != THUMB_W * THUMB_H * 6 || !t.is_ascii() { return None; } - let b: Vec = (0..t.len()).step_by(2).map(|i| u8::from_str_radix(&t[i..i + 2], 16)).collect::>().ok()?; + let b: Vec = (0..t.len()).step_by(2).map(|i| u8::from_str_radix(t.get(i..i + 2)?, 16).ok()).collect::>()?; let px = b.as_chunks::<3>().0.iter().map(|c| Color32::from_rgb(c[0], c[1], c[2])).collect(); Some(egui::ColorImage::new([THUMB_W, THUMB_H], px)) } @@ -484,6 +485,12 @@ fn run_actions(app: &mut EffectcraftApp, ctx: &egui::Context, actions: Vec<(&str mod tests { use super::*; + #[test] + fn non_ascii_thumbnail_is_rejected_without_panicking() { + let malformed = format!("aé{}", "0".repeat(THUMB_W * THUMB_H * 6 - 3)); + assert!(decode_thumb(&malformed).is_none()); + } + #[test] fn recent_list_comes_from_settings() { let mut prefs = effectcraft_engine::prefs::Prefs::default(); From 058fe9a0054a992f405868e09dbbfa490eeaf35e Mon Sep 17 00:00:00 2001 From: heojunfo Date: Fri, 23 Oct 2026 10:21:35 +0900 Subject: [PATCH 26/37] Reload a numbered still as its own file (#2074) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reload Footage probed every item the way File ▸ Import does by default, so a still such as Photo_2.jpg with Photo_1.jpg beside it was grouped into a run and measured from the run's first file. The still took Photo_1's size. Sequence items still probe as a run; everything else reads its one file. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/engine/src/footage_reload.rs | 15 ++++++++++----- crates/engine/src/tests_sequence.rs | 19 +++++++++++++++++-- docs/footage.md | 3 ++- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/footage_reload.rs b/crates/engine/src/footage_reload.rs index fa12b27a..4b900d08 100644 --- a/crates/engine/src/footage_reload.rs +++ b/crates/engine/src/footage_reload.rs @@ -129,17 +129,22 @@ pub fn reload_items(s: &mut Session, ids: &[ItemId]) -> crate::Result { /// writing it) is left as it was. Returns the number reloaded and the files that couldn't be read /// (only without `undoable`). fn reload(s: &mut Session, ids: &[ItemId], undoable: bool) -> crate::Result<(usize, BTreeSet)> { - if s.importer.is_none() { + let Some(importer) = s.importer.clone() else { return Err(crate::EngineError::Other("media import is not available in this build".into())); - } + }; let mut updates = vec![]; let mut paths = BTreeSet::new(); let mut failed = BTreeSet::new(); for i in ids { if let Some(ItemKind::Footage(f)) = s.project.item(*i).map(|x| &x.kind) { - // Probed the way it imports by default, so a sequence is found as one. A path that - // is not followed is not read. - let probed = s.stored_paths.check(f).map_err(|(p, why)| crate::doc_path::refusal(&p, why)).and_then(|_| s.probe_footage(&f.path)); + // A sequence is probed the way it imports by default, so its run is found again; any + // other item is its one file, not the run a numbered still sits in (#2074). A path + // that is not followed is not read. + let probed = s + .stored_paths + .check(f) + .map_err(|(p, why)| crate::doc_path::refusal(&p, why)) + .and_then(|_| if f.kind == FootageKind::Sequence { s.probe_footage(&f.path) } else { importer.probe(&f.path) }); let nf = match probed { Ok(p) => refreshed(s, f, p), Err(_) if undoable => Footage { missing: true, ..f.clone() }, diff --git a/crates/engine/src/tests_sequence.rs b/crates/engine/src/tests_sequence.rs index 7f937596..ee5fbf1c 100644 --- a/crates/engine/src/tests_sequence.rs +++ b/crates/engine/src/tests_sequence.rs @@ -8,14 +8,16 @@ use serde_json::{Value, json}; use crate::Session; -/// Probes every file as a 64×32 still (the media layer's single-file probe). +/// Probes every file as a still 64 + its byte length wide and 32 high (the media layer's +/// single-file probe). struct Stills; impl crate::Importer for Stills { fn probe(&self, path: &str) -> Result { if !std::path::Path::new(path).is_file() { return Err(format!("{path}: no such file")); } - Ok(Footage { path: path.into(), kind: FootageKind::Still, width: 64, height: 32, has_video: true, codec: "PNG".into(), ..Default::default() }) + let width = 64 + std::fs::metadata(path).map_or(0, |m| m.len() as u32); + Ok(Footage { path: path.into(), kind: FootageKind::Still, width, height: 32, has_video: true, codec: "PNG".into(), ..Default::default() }) } } @@ -196,6 +198,19 @@ fn replace_footage_and_reload_keep_sequences() { assert_eq!(f.duration, FrameRate::from_f64(12.0).tick_of(5)); } +/// #2074: reloading a numbered still probed it as its run, so it took the first file's size. +#[test] +fn reloading_a_numbered_still_measures_its_own_file() { + let dir = folder(&RUN); + let mut s = session(); + let item = s.execute("file.import", json!({"paths": [at(&dir, "shot_0002.png")], "sequence": false})).unwrap()["items"][0].clone(); + std::fs::write(at(&dir, "shot_0002.png"), [0; 16]).unwrap(); + s.execute("file.reloadFootage", json!({"items": [item]})).unwrap(); + let (name, f) = footage(&s, &item); + assert_eq!((name.as_str(), f.kind, f.width, f.sequence.len()), ("shot_0002.png", FootageKind::Still, 80, 0)); + assert_eq!(f.path, at(&dir, "shot_0002.png")); +} + /// #756: numbered DPX and Cineon scans import as sequences, as in After Effects (`.cin` was not /// an image to the Media Browser, so its runs came in one file at a time). #[test] diff --git a/docs/footage.md b/docs/footage.md index 84d4c49e..2281589e 100644 --- a/docs/footage.md +++ b/docs/footage.md @@ -129,7 +129,8 @@ including files overwritten at the same path and byte size. File ▸ Open tries each missing file at the same place under the project's new folder, without marking the project modified. - **File ▸ Reload Footage** (Ctrl+Alt+L) reads the files again. A sequence picks up frames - added to its folder (a render still in progress) and keeps its interpretation. + added to its folder (a render still in progress) and keeps its interpretation; a still is + read as its one file, even when numbered files sit next to it. **File ▸ Dependencies ▸ Relink Missing Footage…** searches a chosen folder and its subfolders for missing source files on the desktop. It relinks unique exact filenames whose decoded type, From 9c0597fbfdfdeb5f80f86e281c91aefb831baa19 Mon Sep 17 00:00:00 2001 From: Zaxaerith Date: Fri, 23 Oct 2026 22:45:35 +0800 Subject: [PATCH 27/37] fix(ui): add Duplicate to project item context menu (#2207) --- crates/ui-egui/src/panels/project.rs | 14 ++ crates/ui-egui/tests/ui_project_duplicate.rs | 173 +++++++++++++++++++ 2 files changed, 187 insertions(+) create mode 100644 crates/ui-egui/tests/ui_project_duplicate.rs diff --git a/crates/ui-egui/src/panels/project.rs b/crates/ui-egui/src/panels/project.rs index c4e9894c..4686f6e2 100644 --- a/crates/ui-egui/src/panels/project.rs +++ b/crates/ui-egui/src/panels/project.rs @@ -870,6 +870,20 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { ctx.data_mut(|d| d.insert_temp::(edit_id(), (id.0, "name".into(), it.name.clone()))); ui.close(); } + let duplicate_items = targets(app, *id, selected); + // Like Edit > Duplicate: folders disable the whole operation, including mixed selections. + let can_duplicate = !duplicate_items.is_empty() + && duplicate_items.iter().all(|id| app.session.project.item(ItemId(*id)).is_some_and(|i| !i.is_folder())); + let mut duplicate = egui::Button::new(crate::i18n::tr("Duplicate")); + if let Some(shortcut) = app.session.shortcuts().shortcut_of("edit.duplicate", &serde_json::Value::Null) { + duplicate = duplicate.shortcut_text(crate::menus::shortcut_text(shortcut)); + } + let response = ui.add_enabled(can_duplicate, duplicate); + app.auto.add("project.context.duplicate", response.rect, "Duplicate"); + if response.clicked() { + actions.push(("project.duplicate".into(), json!({"items": duplicate_items}))); + ui.close(); + } if it.parent.is_some() && ui.button(crate::i18n::tr("Move to Project Root")).clicked() { actions.push(("project.move".into(), json!({"items": targets(app, *id, selected), "folder": null}))); ui.close(); diff --git a/crates/ui-egui/tests/ui_project_duplicate.rs b/crates/ui-egui/tests/ui_project_duplicate.rs new file mode 100644 index 00000000..2dac885b --- /dev/null +++ b/crates/ui-egui/tests/ui_project_duplicate.rs @@ -0,0 +1,173 @@ +//! #2207: Duplicate is reachable from a Project row's context menu, with the same targets, +//! naming, undo and shortcut preset as Edit > Duplicate. + +use effectcraft_engine::Session; +use effectcraft_engine::project::ItemId; +use effectcraft_ui_egui::EffectcraftApp; +use effectcraft_ui_egui::dock::PanelKind; +use effectcraft_ui_egui::menus::{parse_shortcut, shortcut_text}; +use egui::{Event, Key, Modifiers, PointerButton, Rect, Shape, pos2}; +use egui_kittest::Harness; +use serde_json::json; + +const DUPLICATE: &str = "project.context.duplicate"; + +fn harness() -> (Harness<'static, EffectcraftApp>, ItemId, ItemId, ItemId) { + let mut s = Session::default(); + let mut comp = |name: &str| { + ItemId(s.execute("comp.new", json!({"name": name, "width": 64, "height": 64, "duration": 1})).unwrap()["comp"].as_u64().unwrap()) + }; + let main = comp("Main"); + let other = comp("Other"); + let folder = ItemId(s.execute("project.newFolder", json!({"name": "Bin"})).unwrap()["item"].as_u64().unwrap()); + s.execute("project.select", json!({"items": []})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(s)); + h.run_steps(3); + (h, main, other, folder) +} + +fn rect(h: &Harness<'_, EffectcraftApp>, id: &str) -> Rect { + let e = h.state().auto.find(id).unwrap_or_else(|| panic!("no {id}")); + Rect::from_min_size(pos2(e.rect[0], e.rect[1]), egui::vec2(e.rect[2], e.rect[3])) +} + +fn click(h: &mut Harness<'_, EffectcraftApp>, id: &str, button: PointerButton, modifiers: Modifiers) { + let p = rect(h, id).center(); + h.input_mut().events.push(Event::ModifiersChanged(modifiers)); + h.input_mut().events.push(Event::PointerMoved(p)); + h.step(); + h.input_mut().events.push(Event::PointerButton { pos: p, button, pressed: true, modifiers }); + h.step(); + h.input_mut().events.push(Event::PointerButton { pos: p, button, pressed: false, modifiers }); + h.run_steps(3); + h.input_mut().events.push(Event::ModifiersChanged(Modifiers::NONE)); + h.step(); +} + +fn row(h: &mut Harness<'_, EffectcraftApp>, item: ItemId, button: PointerButton, modifiers: Modifiers) { + click(h, &format!("project.item.{}.name", item.0), button, modifiers); +} + +fn key(h: &mut Harness<'_, EffectcraftApp>, key: Key, modifiers: Modifiers) { + h.input_mut().events.push(Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers }); + h.step(); + h.input_mut().events.push(Event::Key { key, physical_key: None, pressed: false, repeat: false, modifiers }); + h.run_steps(2); +} + +fn copy_name(h: &Harness<'_, EffectcraftApp>, id: ItemId) -> String { + h.state().session.project.item(id).unwrap().name.clone() +} + +fn menu_text(h: &Harness<'_, EffectcraftApp>, text: &str) -> bool { + fn contains(shape: &Shape, bounds: Rect, text: &str) -> bool { + match shape { + Shape::Text(t) => bounds.contains(t.pos) && t.galley.text() == text, + Shape::Vec(shapes) => shapes.iter().any(|s| contains(s, bounds, text)), + _ => false, + } + } + let bounds = rect(h, DUPLICATE); + h.output().shapes.iter().any(|s| contains(&s.shape, bounds, text)) +} + +#[test] +fn unselected_row_duplicates_only_itself_and_undo_restores_the_project() { + let (mut h, main, other, _) = harness(); + row(&mut h, other, PointerButton::Primary, Modifiers::NONE); + let before = h.state().session.project.clone(); + let undo = h.state().session.history.undo.len(); + row(&mut h, main, PointerButton::Secondary, Modifiers::NONE); + assert_eq!(h.state().session.state.project_selection, [main]); + assert!(menu_text(&h, &shortcut_text("Cmd+D")), "the default platform shortcut is shown"); + click(&mut h, DUPLICATE, PointerButton::Primary, Modifiers::NONE); + let selected = h.state().session.state.project_selection.clone(); + let [copy] = selected.as_slice() else { panic!("one selected copy") }; + assert!(!before.items.contains_key(copy), "the copy has a fresh ID"); + assert_eq!(copy_name(&h, *copy), "Main 2"); + assert_eq!(copy_name(&h, other), "Other"); + assert_eq!(h.state().session.project.items.len(), before.items.len() + 1); + assert_eq!(h.state().session.history.undo.len(), undo + 1); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + assert_eq!(h.state().session.project, before); +} + +#[test] +fn selected_rows_duplicate_together_in_selection_order_with_numbered_names() { + let (mut h, main, other, _) = harness(); + // Deliberately reverse alphabetical row order: command targets retain selection order. + row(&mut h, other, PointerButton::Primary, Modifiers::NONE); + row(&mut h, main, PointerButton::Primary, Modifiers::COMMAND); + assert_eq!(h.state().session.state.project_selection, [other, main]); + let before = h.state().session.project.clone(); + let undo = h.state().session.history.undo.len(); + row(&mut h, main, PointerButton::Secondary, Modifiers::NONE); + click(&mut h, DUPLICATE, PointerButton::Primary, Modifiers::NONE); + let selected = h.state().session.state.project_selection.clone(); + let [a, b] = selected.as_slice() else { panic!("two selected copies") }; + assert_ne!(a, b); + assert!(selected.iter().all(|id| !before.items.contains_key(id))); + assert_eq!((copy_name(&h, *a), copy_name(&h, *b)), ("Other 2".to_string(), "Main 2".to_string())); + assert_eq!(h.state().session.project.items.len(), before.items.len() + 2); + assert_eq!(h.state().session.history.undo.len(), undo + 1); + // Duplicate the selected copies again: the existing engine numbering continues. + row(&mut h, *b, PointerButton::Secondary, Modifiers::NONE); + click(&mut h, DUPLICATE, PointerButton::Primary, Modifiers::NONE); + let names: Vec<_> = h.state().session.state.project_selection.iter().map(|id| copy_name(&h, *id)).collect(); + assert_eq!(names, ["Other 3", "Main 3"]); +} + +#[test] +fn folders_and_mixed_selections_disable_duplicate_without_partial_changes() { + for mixed in [false, true] { + let (mut h, main, _, folder) = harness(); + row(&mut h, main, PointerButton::Primary, Modifiers::NONE); + if mixed { + row(&mut h, folder, PointerButton::Primary, Modifiers::COMMAND); + } + // An unselected folder replaces the selection; a selected non-folder keeps the mixed set. + row(&mut h, if mixed { main } else { folder }, PointerButton::Secondary, Modifiers::NONE); + let before = h.state().session.project.clone(); + let selected = h.state().session.state.project_selection.clone(); + assert!(selected.contains(&folder)); + let undo = h.state().session.history.undo.len(); + let status = h.state().ui.status.clone(); + click(&mut h, DUPLICATE, PointerButton::Primary, Modifiers::NONE); + assert_eq!(h.state().session.project, before, "mixed={mixed}: no partial copy"); + assert_eq!(h.state().session.state.project_selection, selected); + assert_eq!(h.state().session.history.undo.len(), undo); + assert_eq!(h.state().ui.status, status, "no failing engine command was dispatched"); + } +} + +#[test] +fn duplicate_shortcut_still_dispatches_and_the_menu_tracks_rebinding_and_unbinding() { + let (mut h, main, _, _) = harness(); + row(&mut h, main, PointerButton::Primary, Modifiers::NONE); + assert_eq!(h.state().ui.focused, PanelKind::Project); + let (d, command) = parse_shortcut("Cmd+D").unwrap(); + key(&mut h, command, d); + let selected = h.state().session.state.project_selection.clone(); + let [copy] = selected.as_slice() else { panic!("one shortcut copy") }; + assert_ne!(*copy, main); + assert_eq!(copy_name(&h, *copy), "Main 2"); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + h.run_steps(2); + h.state_mut().session.execute("shortcuts.set", json!({"command": "edit.duplicate", "keys": "F6"})).unwrap(); + row(&mut h, main, PointerButton::Secondary, Modifiers::NONE); + assert!(menu_text(&h, "F6")); + assert!(!menu_text(&h, &shortcut_text("Cmd+D"))); + key(&mut h, Key::Escape, Modifiers::NONE); + row(&mut h, main, PointerButton::Primary, Modifiers::NONE); + let before = h.state().session.project.clone(); + key(&mut h, command, d); + assert_eq!(h.state().session.project, before, "the former shortcut stays unbound"); + key(&mut h, Key::F6, Modifiers::NONE); + assert_eq!(h.state().session.project.items.len(), before.items.len() + 1); + h.state_mut().session.execute("shortcuts.set", json!({"command": "edit.duplicate", "keys": null})).unwrap(); + row(&mut h, main, PointerButton::Secondary, Modifiers::NONE); + assert!(!menu_text(&h, "F6")); + assert!(!menu_text(&h, &shortcut_text("Cmd+D"))); + click(&mut h, DUPLICATE, PointerButton::Primary, Modifiers::NONE); + assert_eq!(h.state().session.project.items.len(), before.items.len() + 2, "unbinding keeps the menu action usable"); +} From 6d87dd62177d6263104860f828ab8ee7b4226d1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Lu=C3=ADs=20da=20Silva?= Date: Fri, 23 Oct 2026 01:27:53 -0300 Subject: [PATCH 28/37] effects: CC Radial Fast Blur grows its buffer so streaks pass the layer edge (#2095) Content r from Center smears out to r / (1 - s), so the buffer is padded by the farthest corner's reach (capped at 4096, skipped on adjustment layers), on the CPU and GPU paths alike. --- crates/effects/src/blur2.rs | 30 +++++++++++++++++++++++++++++ crates/effects/src/lib.rs | 2 +- crates/gpu/src/fx_generate.rs | 5 ++++- crates/gpu/src/tests_fx_generate.rs | 2 ++ 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/crates/effects/src/blur2.rs b/crates/effects/src/blur2.rs index e4f2b034..0e8c748a 100644 --- a/crates/effects/src/blur2.rs +++ b/crates/effects/src/blur2.rs @@ -509,11 +509,24 @@ fn camera_lens_blur(ctx: &EffectCtx, mut b: Buf) -> Buf { // --------------------------------------------------------------------------------------------- // CC Radial Fast Blur +/// Transparent pixels CC Radial Fast Blur adds on every side of a `w` × `h` buffer with Center +/// at `c` (buffer pixels) and smear `s`: content `r` from Center streaks out to `r / (1 − s)`, +/// so the farthest corner reaches `d · s / (1 − s)` past itself (at most 4096, as the other +/// growing effects). Shared with the GPU path so both grow the same way. +pub fn radial_fast_pad(c: (f64, f64), w: u32, h: u32, s: f64) -> u32 { + let (w, h) = (f64::from(w), f64::from(h)); + let far = [(0.0, 0.0), (w, 0.0), (0.0, h), (w, h)].iter().map(|&(x, y)| (x - c.0).hypot(y - c.1)).fold(0.0, f64::max); + (far * s / (1.0 - s)).ceil().min(4096.0) as u32 + 1 +} + fn radial_fast_blur(ctx: &EffectCtx, mut b: Buf) -> Buf { let s = (ctx.params.f("amount") / 100.0).clamp(0.0, 1.0) * 0.5; if s <= 0.0 { return b; } + if !ctx.adjustment { + b.pad(radial_fast_pad(b.to_px(ctx.params.v2("center")), b.img.width, b.img.height, s)); + } let c = b.to_px(ctx.params.v2("center")); let mode = ctx.params.e("zoom"); let src = b.img.clone(); @@ -906,6 +919,23 @@ mod tests { assert_eq!(circle.len(), 21); } + #[test] + fn radial_fast_blur_streaks_past_the_layer_edge() { + // Center left of an opaque square: the streaks run right, beyond the layer rectangle (#2095). + let img = Image::filled(20, 20, [1.0, 1.0, 1.0, 1.0]); + let out = run("ec.blur.ccradialfast", &img, &[("center", Value::Vec2([-20.0, 10.0])), ("amount", num(80.0))]); + assert!(out.img.width > 20 && out.img.height > 20, "{}x{}", out.img.width, out.img.height); + let (o, d) = (out.offset[0] as i64, out.offset[1] as i64); + assert!(out.img.get(30 + o, 10 + d)[3] > 0.1, "{:?}", out.img.get(30 + o, 10 + d)); + // Its streak samples only the square: still opaque, so the content keeps its place. + assert!((out.img.get(19 + o, 10 + d)[3] - 1.0).abs() < 1e-5, "{:?}", out.img.get(19 + o, 10 + d)); + // An adjustment layer is comp-sized already; its buffer is not grown. + let s = find("ec.blur.ccradialfast").unwrap(); + let params = Params { values: [("center".into(), Value::Vec2([-20.0, 10.0])), ("amount".into(), num(80.0))].into_iter().collect() }; + let ctx = EffectCtx { params: ¶ms, time: 0.0, layer_size: [20.0, 20.0], seed: 1, adjustment: true, env: Default::default() }; + assert_eq!((s.render)(&ctx, Buf { img, offset: [0.0, 0.0], scale: 1.0 }).img.width, 20); + } + #[test] fn reduce_flicker_blurs_vertically_only() { let mut img = Image::filled(8, 8, [0.0, 0.0, 0.0, 1.0]); diff --git a/crates/effects/src/lib.rs b/crates/effects/src/lib.rs index 40ed79a7..f4a92b7d 100644 --- a/crates/effects/src/lib.rs +++ b/crates/effects/src/lib.rs @@ -90,7 +90,7 @@ pub use sim::particle_state; pub use sim3::{PgBlit, PgPlan, playground_plan, playground_state}; pub use stylize2::strobe_on; // CPU helpers the GPU kernels share (effectcraft-gpu). -pub use blur2::{camera_lens_plain, camera_lens_spans}; +pub use blur2::{camera_lens_plain, camera_lens_spans, radial_fast_pad}; pub use generate::gen_mode; pub use generate2::pattern_kind as cell_pattern_kind; pub use generate2::{BoltSeg, Contour, Ghost, RadioPlan, Wave, flare_ghosts, lightning_segments, radio_plan}; diff --git a/crates/gpu/src/fx_generate.rs b/crates/gpu/src/fx_generate.rs index 3744ec4f..3e466dcd 100644 --- a/crates/gpu/src/fx_generate.rs +++ b/crates/gpu/src/fx_generate.rs @@ -117,11 +117,14 @@ fn radial_blur(e: &mut Enc, ctx: &EffectCtx, b: GBuf) -> Option { run(e, "gen_radial_blur", &p, b, None, Some(&data)) } -fn radial_fast(e: &mut Enc, ctx: &EffectCtx, b: GBuf) -> Option { +fn radial_fast(e: &mut Enc, ctx: &EffectCtx, mut b: GBuf) -> Option { let s = (ctx.params.f("amount") / 100.0).clamp(0.0, 1.0) * 0.5; if s <= 0.0 { return Some(b); } + if !ctx.adjustment { + b.pad(e, effectcraft_effects::radial_fast_pad(b.to_px(ctx.params.v2("center")), b.img.width, b.img.height, s))?; + } let c = b.to_px(ctx.params.v2("center")); let mut p = Params::default(); p.u[0][0] = ctx.params.e("zoom"); diff --git a/crates/gpu/src/tests_fx_generate.rs b/crates/gpu/src/tests_fx_generate.rs index e15abb37..ebe2f2bf 100644 --- a/crates/gpu/src/tests_fx_generate.rs +++ b/crates/gpu/src/tests_fx_generate.rs @@ -31,6 +31,8 @@ fn cc_radial_fast_blur() { effect_case("ec.blur.ccradialfast", &[("amount", n(40.0))]); effect_case("ec.blur.ccradialfast", &[("amount", n(70.0)), ("zoom", e(1)), ("center", pt(22.4, 16.1))]); effect_case("ec.blur.ccradialfast", &[("amount", n(25.0)), ("zoom", e(2))]); + // Center outside the layer: the buffer grows so the streaks pass its edge (#2095). + effect_case("ec.blur.ccradialfast", &[("amount", n(80.0)), ("center", pt(-30.0, 20.0))]); } #[test] From 9199fc20b3e513a567af6413bc40f4abe6bc6cfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Lu=C3=ADs=20da=20Silva?= Date: Fri, 23 Oct 2026 12:29:01 -0300 Subject: [PATCH 29/37] effects: keep CC Radial Fast Blur's pad within 4096 including the extra pixel (#2095) --- crates/effects/src/blur2.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/effects/src/blur2.rs b/crates/effects/src/blur2.rs index 0e8c748a..929f4155 100644 --- a/crates/effects/src/blur2.rs +++ b/crates/effects/src/blur2.rs @@ -516,7 +516,7 @@ fn camera_lens_blur(ctx: &EffectCtx, mut b: Buf) -> Buf { pub fn radial_fast_pad(c: (f64, f64), w: u32, h: u32, s: f64) -> u32 { let (w, h) = (f64::from(w), f64::from(h)); let far = [(0.0, 0.0), (w, 0.0), (0.0, h), (w, h)].iter().map(|&(x, y)| (x - c.0).hypot(y - c.1)).fold(0.0, f64::max); - (far * s / (1.0 - s)).ceil().min(4096.0) as u32 + 1 + ((far * s / (1.0 - s)).ceil() + 1.0).min(4096.0) as u32 } fn radial_fast_blur(ctx: &EffectCtx, mut b: Buf) -> Buf { From 3b3b1846e5a3be4e45fea775fb146ae2b35fa531 Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 01:16:29 +0000 Subject: [PATCH 30/37] Batch fix: i18n: drop the Georgian "Layer {}" row the Timeline no longer asks for (#2155) --- crates/ui-egui/src/i18n/ui.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/ui-egui/src/i18n/ui.rs b/crates/ui-egui/src/i18n/ui.rs index 52f4e26e..a9c45bb9 100644 --- a/crates/ui-egui/src/i18n/ui.rs +++ b/crates/ui-egui/src/i18n/ui.rs @@ -4671,7 +4671,6 @@ const GEORGIAN: &[(&str, &str)] = &[ ("Latest auto-save: {}", "ბოლო ავტოშენახვა: {}"), ("Layer", "შრე"), ("Layer controls", "შრის მართვის ელემენტები"), - ("Layer {}", "შრე {}"), ("Layer:", "შრე:"), ("Layer: {}", "შრე: {}"), ("Layer Styles", "შრის სტილები"), From 546a9720b00cb43e40f92d3615d7c0b9860ec4b4 Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 01:16:38 +0000 Subject: [PATCH 31/37] Batch fix: cargo fmt --- crates/ui-egui/src/panels/project.rs | 4 ++-- crates/ui-egui/tests/it/ui_project_duplicate.rs | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/panels/project.rs b/crates/ui-egui/src/panels/project.rs index 0ad462a3..7f66a00d 100644 --- a/crates/ui-egui/src/panels/project.rs +++ b/crates/ui-egui/src/panels/project.rs @@ -910,8 +910,8 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } let duplicate_items = targets(app, *id, selected); // Like Edit > Duplicate: folders disable the whole operation, including mixed selections. - let can_duplicate = !duplicate_items.is_empty() - && duplicate_items.iter().all(|id| app.session.project.item(ItemId(*id)).is_some_and(|i| !i.is_folder())); + let can_duplicate = + !duplicate_items.is_empty() && duplicate_items.iter().all(|id| app.session.project.item(ItemId(*id)).is_some_and(|i| !i.is_folder())); let mut duplicate = egui::Button::new(crate::i18n::tr("Duplicate")); if let Some(shortcut) = app.session.shortcuts().shortcut_of("edit.duplicate", &serde_json::Value::Null) { duplicate = duplicate.shortcut_text(crate::menus::shortcut_text(shortcut)); diff --git a/crates/ui-egui/tests/it/ui_project_duplicate.rs b/crates/ui-egui/tests/it/ui_project_duplicate.rs index 2dac885b..2037c278 100644 --- a/crates/ui-egui/tests/it/ui_project_duplicate.rs +++ b/crates/ui-egui/tests/it/ui_project_duplicate.rs @@ -14,9 +14,8 @@ const DUPLICATE: &str = "project.context.duplicate"; fn harness() -> (Harness<'static, EffectcraftApp>, ItemId, ItemId, ItemId) { let mut s = Session::default(); - let mut comp = |name: &str| { - ItemId(s.execute("comp.new", json!({"name": name, "width": 64, "height": 64, "duration": 1})).unwrap()["comp"].as_u64().unwrap()) - }; + let mut comp = + |name: &str| ItemId(s.execute("comp.new", json!({"name": name, "width": 64, "height": 64, "duration": 1})).unwrap()["comp"].as_u64().unwrap()); let main = comp("Main"); let other = comp("Other"); let folder = ItemId(s.execute("project.newFolder", json!({"name": "Bin"})).unwrap()["item"].as_u64().unwrap()); From 3385eafe1f72f65b01a52f998e665519ff478247 Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 01:21:11 +0000 Subject: [PATCH 32/37] Batch fix: blur: a NaN or vanishing sigma skips the Gaussian pass (#1968) --- crates/raster/src/blur.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/raster/src/blur.rs b/crates/raster/src/blur.rs index fcafee5d..46b749a9 100644 --- a/crates/raster/src/blur.rs +++ b/crates/raster/src/blur.rs @@ -168,7 +168,8 @@ pub fn gaussian_is_exact(sigma_x: f64, sigma_y: f64) -> bool { /// One separable pass of a sampled Gaussian (radius 3 sigma) along x or y. fn gauss_pass(img: &Image, sigma: f64, vertical: bool, repeat: bool) -> Image { - if sigma <= 0.0 || img.is_empty() { + // Below 1e-3 the kernel is the identity in f64; tiny sigmas would also make 2σ² underflow to 0 (NaN taps). + if sigma.is_nan() || sigma <= 1e-3 || img.is_empty() { return img.clone(); } let r = (sigma * 3.0).ceil() as i64; @@ -271,6 +272,17 @@ mod tests { assert!(b.get(32, 32)[3] < 0.05); } + #[test] + fn vanishing_or_nan_sigma_leaves_the_image_unchanged() { + let mut img = Image::new(5, 5); + img.set(2, 2, [1.0, 0.5, 0.25, 1.0]); + for s in [1e-200, f64::MIN_POSITIVE, f64::NAN] { + let b = gaussian_blur_exact(&img, s, s, false); + assert!(b.data.iter().all(|p| p.iter().all(|v| v.is_finite())), "sigma {s}"); + assert_eq!(b, img, "sigma {s}"); + } + } + #[test] fn flat_image_stays_flat_with_repeat() { let img = Image::filled(20, 10, [0.5, 0.5, 0.5, 1.0]); From 69d6e3d28a57d244d14d75c30cad4967c8a804ff Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 01:22:55 +0000 Subject: [PATCH 33/37] Batch fix: EPS: filter keeps its source, so an image's data source isn't skipped twice (#1909) --- crates/pdf/src/ps.rs | 20 +++++++++++--------- crates/pdf/src/tests_import.rs | 10 ++++++++++ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/crates/pdf/src/ps.rs b/crates/pdf/src/ps.rs index 984f7206..88385186 100644 --- a/crates/pdf/src/ps.rs +++ b/crates/pdf/src/ps.rs @@ -1271,31 +1271,33 @@ impl Interp { } // `source [params] /Name filter`. A SubFileDecode of the current file with a count of // 0 reads to its end-of-data string: skip that data (an embedded XMP packet, which - // otherwise ran as PostScript). Other filters stand for the file they read. + // otherwise ran as PostScript). Other filters read what their source reads. "filter" => { let name = self.pop().key(); - if name == "SubFileDecode" { + let mut skipped = false; + let source = if name == "SubFileDecode" && !matches!(self.st.last(), Some(V::Dict(_))) { let eod = match self.pop() { V::Str(s) => s.borrow().clone(), _ => vec![], }; let count = self.popn(); - if let V::File = self.pop() - && count == 0.0 - && !eod.is_empty() - { + let source = self.pop(); + if matches!(source, V::File) && count == 0.0 && !eod.is_empty() { src.pos = match find_from(src.d, &eod, src.pos) { Some(p) => p.saturating_add(eod.len()), None => src.d.len(), }; + skipped = true; } + source } else { if matches!(self.st.last(), Some(V::Dict(_))) { self.pop(); } - self.pop(); - } - self.push(V::File); + self.pop() + }; + // Data already stepped over leaves nothing for `image` to skip. + self.push(if skipped { V::Null } else { source }); } "vmstatus" => { for _ in 0..3 { diff --git a/crates/pdf/src/tests_import.rs b/crates/pdf/src/tests_import.rs index 7f46f6cf..71b93a78 100644 --- a/crates/pdf/src/tests_import.rs +++ b/crates/pdf/src/tests_import.rs @@ -45,6 +45,16 @@ fn eps_with_aliased_operators_device_queries_and_embedded_data_draws() { assert!(coverage(&render(&doc), 50, 25, 150, 75) > 4900.0, "drawing after the embedded data"); } +#[test] +fn eps_filter_on_a_string_source_does_not_skip_the_program() { + let square = "1 0 0 setrgbcolor 50 25 moveto 150 25 lineto 150 75 lineto 50 75 lineto closepath fill\n"; + let img = "gsave 1 1 8 [1 0 0 1 0 0] (FF) /ASCIIHexDecode filter image grestore\n"; + // Images aren't drawn yet, so this parses without `eps`'s nothing-skipped check. + let doc = parse(format!("%!PS-Adobe-3.0 EPSF-3.0\n%%BoundingBox: 0 0 200 100\n{img}{square}").as_bytes()).unwrap(); + assert_eq!(doc.skipped, ["image"]); + assert!(coverage(&render(&doc), 50, 25, 150, 75) > 4900.0, "drawing after an image read from a string"); +} + #[test] fn eps_text_with_bundled_fonts_and_show_variants() { // Not embedded: drawn with the bundled stand-in, like the same text in a PDF. From c028b49f99338d93bb6cd86b0b4b5f42828dfb76 Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 01:23:26 +0000 Subject: [PATCH 34/37] Batch fix: EPS: executable names count against the PostScript call depth --- crates/pdf/src/ps.rs | 12 +++++++++++- crates/pdf/src/tests_import.rs | 8 ++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/pdf/src/ps.rs b/crates/pdf/src/ps.rs index 88385186..e8612e42 100644 --- a/crates/pdf/src/ps.rs +++ b/crates/pdf/src/ps.rs @@ -454,7 +454,17 @@ impl Interp { self.calls -= 1; r } - V::Name(n) => self.exec_name(&n, src), + // An executable name can run another (`exec` of `/exec cvx`), so names count too. + V::Name(n) => { + if self.calls >= MAX_CALL_DEPTH { + self.b.skip("PostScript (recursion limit)"); + return Err(Flow::Quit); + } + self.calls += 1; + let r = self.exec_name(&n, src); + self.calls -= 1; + r + } other => { self.push(other); Ok(()) diff --git a/crates/pdf/src/tests_import.rs b/crates/pdf/src/tests_import.rs index 71b93a78..ca5e2417 100644 --- a/crates/pdf/src/tests_import.rs +++ b/crates/pdf/src/tests_import.rs @@ -55,6 +55,14 @@ fn eps_filter_on_a_string_source_does_not_skip_the_program() { assert!(coverage(&render(&doc), 50, 25, 150, 75) > 4900.0, "drawing after an image read from a string"); } +/// Each `exec` of the executable name `exec` runs the next one: the chain is cut off at the call +/// depth instead of overflowing the stack. +#[test] +fn eps_exec_of_executable_names_stops_at_the_call_depth() { + let doc = parse(b"%!PS-Adobe-3.0 EPSF-3.0\n%%BoundingBox: 0 0 200 100\n/exec cvx 99999 {dup} repeat exec\n").unwrap(); + assert!(doc.skipped.iter().any(|s| s.contains("recursion limit")), "{:?}", doc.skipped); +} + #[test] fn eps_text_with_bundled_fonts_and_show_variants() { // Not embedded: drawn with the bundled stand-in, like the same text in a PDF. From 80a08e3a3f01ed2ad379ef49f5531b3a31ea021c Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 01:29:13 +0000 Subject: [PATCH 35/37] Batch fix: pdf: the u32 overflow tests reach their fonts from the page, as the object loader needs (#2163) --- crates/pdf/src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/pdf/src/tests.rs b/crates/pdf/src/tests.rs index b15996a1..40f064af 100644 --- a/crates/pdf/src/tests.rs +++ b/crates/pdf/src/tests.rs @@ -1098,7 +1098,7 @@ fn cid_width_entries_at_the_top_of_the_cid_range_do_not_overflow() { let load = |entry: &str| { let type0 = "<< /Type /Font /Subtype /Type0 /BaseFont /X /Encoding /Identity-H /DescendantFonts [13 0 R] >>"; let desc = format!("<< /Type /Font /Subtype /CIDFontType2 {entry} >>"); - let file = object::File::parse(&page_pdf("", "", vec![(10, type0.into(), None), (13, desc, None)])).unwrap(); + let file = object::File::parse(&page_pdf("", "/Font << /F1 10 0 R >>", vec![(10, type0.into(), None), (13, desc, None)])).unwrap(); let d = file.objects.get(&(10, 0)).and_then(object::Obj::dict).unwrap(); font::Font::load(&file, d, &mut vec![]) }; @@ -1127,7 +1127,7 @@ fn cmap_entries_at_the_top_of_the_code_range_do_not_overflow() { (14, "<< >>".into(), Some(to_unicode.as_bytes().to_vec())), (15, "<< /Type /CMap >>".into(), Some(encoding.as_bytes().to_vec())), ]; - let file = object::File::parse(&page_pdf("", "", objs)).unwrap(); + let file = object::File::parse(&page_pdf("", "/Font << /F1 10 0 R >>", objs)).unwrap(); let d = file.objects.get(&(10, 0)).and_then(object::Obj::dict).unwrap(); let f = font::Font::load(&file, d, &mut vec![]); assert_eq!((f.unicode(0xFFFF_FFFF), f.unicode(0)), ("A".to_string(), String::new())); From a407281852a011d013f0e1d29aed579a89168aca Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 01:39:09 +0000 Subject: [PATCH 36/37] Batch fix: tests: the web-job GPU page blurs by 12, which the GPU still runs after #1968 (#1968) --- crates/ui-egui/tests/it/ui_web_job_gpu.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 615d4fbd..9060e106 100644 --- a/crates/ui-egui/tests/it/ui_web_job_gpu.rs +++ b/crates/ui-egui/tests/it/ui_web_job_gpu.rs @@ -57,7 +57,7 @@ fn page() -> (Session, Arc) { s.execute("prop.addKey", json!({"layer": l, "path": "effects/#1/transform/offset", "time": 0.0, "value": [0.5, 0.5]})).unwrap(); s.execute("prop.addKey", json!({"layer": l, "path": "effects/#1/transform/offset", "time": 0.5, "value": [0.6, 0.56]})).unwrap(); s.execute("effect.apply", json!({"layers": [l], "effect": "ec.blur.gaussian"})).unwrap(); - s.execute("prop.set", json!({"layer": l, "path": "effects/#2/blurriness", "value": 3.0})).unwrap(); + s.execute("prop.set", json!({"layer": l, "path": "effects/#2/blurriness", "value": 12.0})).unwrap(); (s, off) } From 2cca29830e64c793d105da77ce4160b1c89da59b Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 01:43:40 +0000 Subject: [PATCH 37/37] Batch fix: EPS: load returns an operator value, so an executable-name alias runs what its name is bound to (#1909) --- crates/pdf/src/ps.rs | 23 +++++++++++++++++++---- crates/pdf/src/tests_import.rs | 11 +++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/crates/pdf/src/ps.rs b/crates/pdf/src/ps.rs index e8612e42..dd735075 100644 --- a/crates/pdf/src/ps.rs +++ b/crates/pdf/src/ps.rs @@ -28,6 +28,8 @@ enum V { Name(Rc), /// A literal name (`/name`). Lit(Rc), + /// A built-in operator, as `load` returns it for a name no dictionary defines. + Op(Rc), Str(Rc>>), Proc(Rc>), Array(Rc>>), @@ -47,7 +49,7 @@ impl V { } fn key(&self) -> String { match self { - V::Name(n) | V::Lit(n) => n.to_string(), + V::Name(n) | V::Lit(n) | V::Op(n) => n.to_string(), V::Str(s) => String::from_utf8_lossy(&s.borrow()).into_owned(), V::Num(n) => format!("{n}"), V::Bool(b) => format!("{b}"), @@ -441,6 +443,12 @@ impl Interp { break; } } + V::Op(op) => { + r = self.spend(1).and_then(|()| self.builtin(op, src)); + if r.is_err() { + break; + } + } // Other items count against the budget as names do. other => { r = self.spend(1); @@ -465,6 +473,10 @@ impl Interp { self.calls -= 1; r } + V::Op(op) => { + self.spend(1)?; + self.builtin(&op, src) + } other => { self.push(other); Ok(()) @@ -480,7 +492,9 @@ impl Interp { // An operator stored under another name (`/mo /moveto load def`, as Illustrator's // EPS prolog does for every path operator) runs that operator. It pushed the // operator's name instead, so such files drew nothing (#955). - V::Name(op) => self.builtin(&op, src), + V::Op(op) => self.builtin(&op, src), + // An executable name (`/y /x cvx def`) runs what that name is bound to. + V::Name(_) => self.exec_obj(v, src), other => { self.push(other); Ok(()) @@ -977,7 +991,7 @@ impl Interp { } "load" => { let k = self.pop().key(); - let v = self.lookup(&k).unwrap_or(V::Name(k.into())); + let v = self.lookup(&k).unwrap_or(V::Op(k.into())); self.push(v); } "undef" => { @@ -1174,13 +1188,14 @@ impl Interp { } "xcheck" => { let v = self.pop(); - self.push(V::Bool(matches!(v, V::Proc(_) | V::Name(_)))); + self.push(V::Bool(matches!(v, V::Proc(_) | V::Name(_) | V::Op(_)))); } "type" => { let t = match self.pop() { V::Num(_) => "realtype", V::Bool(_) => "booleantype", V::Name(_) | V::Lit(_) => "nametype", + V::Op(_) => "operatortype", V::Str(_) => "stringtype", V::Proc(_) | V::Array(_) => "arraytype", V::Dict(_) => "dicttype", diff --git a/crates/pdf/src/tests_import.rs b/crates/pdf/src/tests_import.rs index ca5e2417..11ef72f8 100644 --- a/crates/pdf/src/tests_import.rs +++ b/crates/pdf/src/tests_import.rs @@ -55,6 +55,17 @@ fn eps_filter_on_a_string_source_does_not_skip_the_program() { assert!(coverage(&render(&doc), 50, 25, 150, 75) > 4900.0, "drawing after an image read from a string"); } +/// A loaded operator runs that operator even after its name is redefined (the prolog idiom +/// `/_m /moveto load def /moveto {… _m} def`), and an executable name bound with `cvx` runs what +/// its name is bound to. +#[test] +fn eps_loaded_operators_and_executable_name_aliases() { + let saved = "/_m /moveto load def /moveto {_m} def 1 0 0 setrgbcolor 50 25 moveto 150 25 lineto 150 75 lineto 50 75 lineto closepath fill\n"; + assert!(coverage(&render(&eps(saved.as_bytes())), 50, 25, 150, 75) > 4900.0, "a redefined moveto that calls the saved one"); + let alias = "/sq {50 25 moveto 150 25 lineto 150 75 lineto 50 75 lineto closepath fill} def /y /sq cvx def 1 0 0 setrgbcolor y\n"; + assert!(coverage(&render(&eps(alias.as_bytes())), 50, 25, 150, 75) > 4900.0, "an executable name bound to a procedure's name"); +} + /// Each `exec` of the executable name `exec` runs the next one: the chain is cut off at the call /// depth instead of overflowing the stack. #[test]