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] {