Shift Channels: After Effects' Take … From menu order, so 10 is Full Off (#1797)

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 <noreply@anthropic.com>

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();

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]

crates/gpu/src/fx_key.rs

@@ -438,8 +438,10 @@ fn refine(e: &mut Enc, ctx: &EffectCtx, b: GBuf, hard: bool) -> Option<GBuf> {
 
 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<GBuf> {
             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" => {

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