From 1b2c5d1944cf3038dd9802b0c754e0b872d1a45d Mon Sep 17 00:00:00 2001 From: MindlessRuff <40220862+MindlessRuff@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:22:49 -0700 Subject: [PATCH] fix(export): keep values above 1.0 in EXR output (#339) The EXR sequence writer and comp.saveFrameAsExr clamped colour to 0..1 before the sRGB decode, so a 32 bpc project lost every over-range value. The lower bound stays at 0 and alpha stays clamped. Tests cover Rgba and Rgb sequences and saveFrameAsExr. --- crates/engine/src/commands/frame_export.rs | 2 +- crates/engine/src/tests_frame_export.rs | 45 +++++++++++++++ crates/export/src/lib.rs | 5 +- crates/export/tests/roundtrip.rs | 65 ++++++++++++++++++++++ 4 files changed, 114 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/commands/frame_export.rs b/crates/engine/src/commands/frame_export.rs index 77e0f5d6..c939d67e 100644 --- a/crates/engine/src/commands/frame_export.rs +++ b/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); } diff --git a/crates/engine/src/tests_frame_export.rs b/crates/engine/src/tests_frame_export.rs index 4866619d..3f319b83 100644 --- a/crates/engine/src/tests_frame_export.rs +++ b/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:?}"); + } +} diff --git a/crates/export/src/lib.rs b/crates/export/src/lib.rs index 81e0f5c2..723a418b 100644 --- a/crates/export/src/lib.rs +++ b/crates/export/src/lib.rs @@ -293,6 +293,7 @@ fn write_still( 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 { @@ -305,12 +306,12 @@ fn write_still( } 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) } }; diff --git a/crates/export/tests/roundtrip.rs b/crates/export/tests/roundtrip.rs index cf533031..3d4e8fcb 100644 --- a/crates/export/tests/roundtrip.rs +++ b/crates/export/tests/roundtrip.rs @@ -277,6 +277,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);