Merge pull request #344 from MindlessRuff/fix/exr-over-range

fix(export): keep values above 1.0 in EXR output (#339)

crates/engine/src/commands/frame_export.rs

@@ -165,7 +165,7 @@ pub fn layered_exr(s: &Session, cid: ItemId, t: Tick, scale: f64) -> Result<(Vec
             let sp = straight(*p);
             let a = sp[3];
             for c in 0..3 {
-                out[c].push(srgb_to_linear(sp[c].clamp(0.0, 1.0)) * a);
+                out[c].push(srgb_to_linear(sp[c].max(0.0)) * a);
             }
             out[3].push(a);
         }

crates/engine/src/tests_frame_export.rs

@@ -76,3 +76,48 @@ fn proexr_writes_layer_prefixed_channels_and_the_composite() {
     assert!((at("A", 16, 16) - 1.0).abs() < 1e-3);
     assert!(at("R", 16, 16) < 0.5, "multiplied by 50% blue (linear)");
 }
+
+/// Save a 32×32 comp holding one opaque solid of `color` (an sRGB-encoded triple) as ProEXR and
+/// return the centre pixel of the composite and of the solid's own layer: `[R, G, B]` each.
+fn proexr_centre(color: [f64; 3], bit_depth: u64) -> ([f32; 3], [f32; 3]) {
+    use exr::prelude::*;
+    let mut s = Session::default();
+    s.execute("comp.new", json!({"name": "Bright", "width": 32, "height": 32, "frameRate": 24, "duration": 1})).unwrap();
+    s.execute("file.projectSettings", json!({"bitDepth": bit_depth})).unwrap();
+    s.execute("layer.newSolid", json!({"name": "Bright", "color": color})).unwrap();
+    let dir = out_dir(&format!("frame-exr-over-{}", color[0]));
+    let path = dir.join(format!("frame{bit_depth}.exr")).to_string_lossy().to_string();
+    s.execute("comp.saveFrameAsExr", json!({"path": path})).unwrap();
+    let img = read().no_deep_data().largest_resolution_level().all_channels().all_layers().all_attributes().from_file(&path).unwrap();
+    let layer = &img.layer_data[0];
+    let at = |n: &str| {
+        let c = layer.channel_data.list.iter().find(|c| c.name.to_string() == n).unwrap_or_else(|| panic!("no channel {n}"));
+        match &c.sample_data {
+            FlatSamples::F32(v) => v[16 * 32 + 16],
+            o => panic!("{o:?}"),
+        }
+    };
+    ([at("R"), at("G"), at("B")], [at("Bright.R"), at("Bright.G"), at("Bright.B")])
+}
+
+#[test]
+fn proexr_keeps_values_above_one_in_32_bpc() {
+    // Issue #339: over-range colour reaches the EXR in scene-linear light (sRGB decode, uncapped).
+    let src = [1.5, 1.25, 0.75];
+    let (comp, layer) = proexr_centre(src, 32);
+    for i in 0..3 {
+        let want = effectcraft_color::srgb_to_linear(src[i] as f32);
+        for got in [comp[i], layer[i]] {
+            assert!((got - want).abs() < 1e-3 * want.max(1.0), "channel {i}: got {got}, want {want} ({comp:?} / {layer:?})");
+        }
+    }
+    assert!(comp[0] > 1.5 && comp[1] > 1.25, "{comp:?}");
+}
+
+#[test]
+fn proexr_values_at_or_below_one_are_unchanged() {
+    let (comp, layer) = proexr_centre([1.0, 0.4, 0.0], 32);
+    for got in [comp, layer] {
+        assert!((got[0] - 1.0).abs() < 1e-4 && (got[1] - 0.1329).abs() < 1e-3 && got[2].abs() < 1e-6, "{got:?}");
+    }
+}

crates/export/src/lib.rs

@@ -297,6 +297,7 @@ fn write_still<W: std::io::Write + std::io::Seek>(
     match fmt {
         OutputFormat::ExrSequence => {
             // Linear light, premultiplied (the EXR convention); RGB keeps alpha = 1 over the background.
+            // Colour is floored at 0 but not capped at 1: a 32 bpc frame's over-range values survive (#339).
             let lin = |v: f32| if v <= 0.04045 { v / 12.92 } else { ((v + 0.055) / 1.055).powf(2.4) };
             let mut data = Vec::with_capacity((w * h * 4) as usize);
             for y in 0..h {
@@ -309,12 +310,12 @@ fn write_still<W: std::io::Write + std::io::Seek>(
                         }
                         Channels::Rgba => {
                             let a = p[3].clamp(0.0, 1.0);
-                            let s = |c: f32| if a > 0.0 { lin((c / a).clamp(0.0, 1.0)) * a } else { 0.0 };
+                            let s = |c: f32| if a > 0.0 { lin((c / a).max(0.0)) * a } else { 0.0 };
                             ([s(p[0]), s(p[1]), s(p[2])], a)
                         }
                         Channels::Rgb => {
                             let k = 1.0 - p[3].clamp(0.0, 1.0);
-                            let c = |i: usize| lin((p[i] + comp.background[i] * k).clamp(0.0, 1.0));
+                            let c = |i: usize| lin((p[i] + comp.background[i] * k).max(0.0));
                             ([c(0), c(1), c(2)], 1.0)
                         }
                     };

crates/export/tests/roundtrip.rs

@@ -282,6 +282,71 @@ fn exr_sequence_is_linear() {
     assert!((c[2] - 0.1329).abs() < 0.01 && (c[3] - 1.0).abs() < 1e-6, "{c:?}");
 }
 
+/// A 32 bpc project whose only layer is a full-frame opaque solid of the given colour.
+fn bright_project(color: [f32; 3]) -> (Project, ItemId) {
+    let mut p = Project::default();
+    p.settings.bit_depth = effectcraft_project::BitDepth::Bpc32;
+    let comp = Comp::new(W, H, FrameRate::new(10, 1), Tick::from_seconds_f64(0.1));
+    let sid = p.add_item("Bright", Label::Red, None, ItemKind::Solid(Solid { color, width: W, height: H, pixel_aspect: 1.0 }));
+    let mut comp = comp;
+    let l = build::layer(&mut p, &comp, "Bright", LayerSource::Solid { item: sid }, (W, H), None);
+    comp.layers.push(l);
+    let cid = p.add_item("Comp 1", Label::Sandstone, None, ItemKind::Comp(comp.into()));
+    (p, cid)
+}
+
+/// Export one frame of a 32 bpc solid to an EXR sequence and read its centre pixel.
+fn exr_centre(color: [f32; 3], channels: Channels, name: &str) -> [f32; 4] {
+    let (p, cid) = bright_project(color);
+    let d = out_dir(name);
+    let mut om = OutputModule::for_format(OutputFormat::ExrSequence);
+    om.channels = channels;
+    let s = RenderSettings { time_span: TimeSpan::LengthOfComp, ..Default::default() };
+    let path = d.join("f_[#####].exr").to_string_lossy().to_string();
+    let job = Job {
+        project: &p,
+        footage: &NoFootage,
+        expr: None,
+        accel: None,
+        comp: cid,
+        settings: &s,
+        output: &om,
+        path: &path,
+        sink: None,
+        options: Default::default(),
+        nested_switches: true,
+    };
+    export(&job, &mut |_| true).expect("export");
+    let img = image::open(d.join("f_00000.exr")).expect("exr").to_rgba32f();
+    img.get_pixel(W / 2, H / 2).0
+}
+
+/// The sRGB decode of the frame the writer is handed (the solid is sRGB-encoded in the frame).
+fn lin(v: f32) -> f32 {
+    effectcraft_color::srgb_to_linear(v)
+}
+
+#[test]
+fn exr_sequence_keeps_values_above_one() {
+    // Issue #339: a 32 bpc project's over-range colour must reach the EXR, in scene-linear light.
+    let src = [1.5, 1.25, 0.75];
+    for channels in [Channels::Rgba, Channels::Rgb] {
+        let c = exr_centre(src, channels, "seq-exr-over");
+        for i in 0..3 {
+            assert!((c[i] - lin(src[i])).abs() < 1e-3 * lin(src[i]).max(1.0), "{channels:?} channel {i}: {c:?}");
+        }
+        assert!(c[0] > 1.5 && c[1] > 1.25, "{channels:?} not clamped: {c:?}");
+        assert!((c[3] - 1.0).abs() < 1e-6, "{c:?}");
+    }
+}
+
+#[test]
+fn exr_sequence_values_at_or_below_one_are_unchanged() {
+    for channels in [Channels::Rgba, Channels::Rgb] {
+        let c = exr_centre([1.0, 0.4, 0.0], channels, "seq-exr-in-range");
+        assert!((c[0] - 1.0).abs() < 1e-4 && (c[1] - lin(0.4)).abs() < 1e-4 && c[2].abs() < 1e-6, "{channels:?}: {c:?}");
+    }
+}
 #[test]
 fn gif_roundtrip() {
     let (p, cid) = project(None);