dtourolle/DarkRoom
1
0
Fork 0
Code Issues 39 Pull Requests Actions Packages Projects Releases 20 Wiki Activity

Tell the shader which colour space it is encoding for

Browse Source 
The generated shader ended with `encode_srgb` and a clamp, so every
photograph leaving DarkRoom had been through sRGB's gamut whatever the
settings page said. Export refused the other three spaces rather than
tag clipped pixels with a gamut they did not contain — correct, and
not something an encoder could fix.

So the output space becomes a parameter of composition. `compose_for`
emits a constant primaries matrix after the camera matrix and before
the clip, and generates the transfer function to match: the sRGB curve
for sRGB and Display P3, a pure 2.199 gamma for Adobe RGB, 1.8 with a
linear toe for ProPhoto. The ordering the camera matrix depends on is
untouched — operations still run in camera space — and sRGB emits no
conversion at all, so the shader compiled on nearly every frame is
byte-for-byte what it was.

The numbers live in dr-types, derived from four chromaticity pairs per
space rather than tabulated. That is not tidiness: the shader encodes
the pixels and the ICC profile describes them, and a file whose profile
disagrees with its own contents is worse than one with no profile. One
derivation makes them agree by construction, and can be checked against
the values the specifications publish.

Profiles are generated here too — minimal v2 matrix/TRC, about 2 KB,
pure Rust, no lcms to satisfy under the NDK. A JPEG carries it in APP2,
a PNG in iCCP, a TIFF in tag 34675. sRGB gets one as well, because
untagged does not mean sRGB, it means guess.

The refusal survives in a sharper form. A `Frame` now carries the space
it was rendered in, and export refuses to label it anything else. The
develop session still composes for sRGB, so a P3 export from the
interface fails with an accurate error instead of producing a file that
lies — the frontend half is a separate change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-17 09:04:04 +02:00
co-authored by Claude Opus 5
parent 2330ed25e9
commit 914d14ec0d
11 changed files with 1672 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/dr-export/src/lib.rs
+78 -15
View File
@@ -1,4 +1,4 @@
//! TRACES: FR-EXP-1 | FR-EXP-3 | FR-EXP-4 | FR-EXP-6 | FR-EXP-9
//! TRACES: FR-EXP-1 | FR-EXP-2 | FR-EXP-3 | FR-EXP-4 | FR-EXP-6 | FR-EXP-9
//! Turning a rendered frame into a file's worth of bytes.
//!
//! # What this crate is, and is not
@@ -26,6 +26,7 @@ use dr_types::{ColourSpace, ExportFormat, ExportSettings};
mod encode;
mod error;
pub mod icc;
mod name;
mod sharpen;
mod size;
@@ -36,7 +37,7 @@ pub use size::target_size;
/// A rendered frame, as the adjust pass produced it.
///
/// 8-bit RGBA, display-encoded sRGB — the format
/// 8-bit RGBA, display-encoded in [`Self::space`] — the format
/// [`dr_gpu::AdjustPass`](../dr_gpu/struct.AdjustPass.html) writes. Alpha is
/// carried but never meaningful: the pipeline writes 1.0 everywhere, and no
/// operation produces transparency.
@@ -46,10 +47,42 @@ pub struct Frame {
pub height: u32,
/// Tightly packed RGBA8, `width * height * 4` bytes.
pub rgba: Vec<u8>,
/// TRACES: FR-EXP-2
/// The space the shader encoded these pixels into.
///
/// Travels with the pixels rather than being asserted at the point of
/// encoding, because it is a fact about them and not a preference. The
/// conversion happened in the generated shader, before the clip to 0..1,
/// and nothing downstream can undo or redo it — a frame clipped to sRGB
/// has already lost whatever a wider space would have carried.
///
/// Making it a field is what lets [`export`] refuse to label a frame as
/// something it is not, rather than trusting a caller to have rendered
/// what it asked for.
pub space: ColourSpace,
}
impl Frame {
/// A frame the pipeline rendered in sRGB — what
/// [`EditGraph::compose`](../dr_pipeline/struct.EditGraph.html#method.compose)
/// produces, and so what the display path hands over.
///
/// An export in a wider space must render its own frame with
/// `compose_for` and declare it through [`Self::in_space`]. Defaulting
/// here rather than demanding the space at every call site keeps the
/// common case honest by construction: a caller that has not thought
/// about colour is describing sRGB, and sRGB is what it rendered.
pub fn new(width: u32, height: u32, rgba: Vec<u8>) -> Result<Self, ExportError> {
Self::in_space(width, height, rgba, ColourSpace::Srgb)
}
/// A frame rendered into a stated colour space.
pub fn in_space(
width: u32,
height: u32,
rgba: Vec<u8>,
space: ColourSpace,
) -> Result<Self, ExportError> {
let expected = width as usize * height as usize * 4;
if rgba.len() != expected {
return Err(ExportError::FrameSize {
@@ -64,6 +97,7 @@ impl Frame {
width,
height,
rgba,
space,
})
}
}
@@ -100,17 +134,22 @@ pub fn export(
settings: &ExportSettings,
name: String,
) -> Result<Encoded, ExportError> {
// Refused rather than mislabelled. The pipeline's final stage encodes to
// sRGB and clamps to its gamut (see `encode_srgb` in the generated
// shader), so the pixels arriving here have already lost anything a wider
// space could have carried. Tagging them Display P3 would produce a file
// that claims a gamut it does not contain — worse than not offering it,
// because the claim survives into everything downstream.
// TRACES: FR-EXP-2
// Refused rather than mislabelled. Every space the settings page offers
// now works, but only if the *frame* was rendered into it: the conversion
// and the clip both happen in the generated shader, so pixels that arrive
// clipped to sRGB have already lost whatever a wider space would have
// carried, and no amount of profile-writing here brings it back.
//
// Honouring the other spaces is a pipeline change, not an encoder one:
// the shader has to be told what to encode to (FR-EXP-2).
if settings.colour_space != ColourSpace::Srgb {
return Err(ExportError::ColourSpaceUnsupported(settings.colour_space));
// The caller's fix is to compose with `EditGraph::compose_for(space)`
// before rendering. Until it does, this is an accurate error where the
// alternative would be a file that claims a gamut it does not contain —
// and that claim survives into everything downstream.
if frame.space != settings.colour_space {
return Err(ExportError::ColourSpaceMismatch {
rendered: frame.space,
requested: settings.colour_space,
});
}
if matches!(settings.format, ExportFormat::Avif | ExportFormat::JpegXl) {
@@ -247,17 +286,41 @@ mod tests {
}
#[test]
fn a_colour_space_the_pipeline_cannot_produce_is_refused_not_mislabelled() {
fn a_frame_rendered_in_one_space_is_not_labelled_another() {
// A file tagged Display P3 carrying sRGB-clipped pixels is a lie that
// survives into everything downstream. Better to fail loudly.
// survives into everything downstream. The frame carries the space it
// was rendered in precisely so this cannot be waved through.
let mut s = settings(ExportFormat::Jpeg);
s.colour_space = ColourSpace::DisplayP3;
assert!(matches!(
export(&frame(8, 8), &s, "a".into()),
Err(ExportError::ColourSpaceUnsupported(_))
Err(ExportError::ColourSpaceMismatch { .. })
));
}
#[test]
fn every_colour_space_exports_when_the_frame_was_rendered_in_it() {
// The other side of the refusal above, and what FR-EXP-2 actually
// asks for: a frame the pipeline encoded into a wide space reaches a
// file, in every format that has an encoder.
for space in ColourSpace::ALL {
for format in [
ExportFormat::Jpeg,
ExportFormat::Png,
ExportFormat::Tiff8,
ExportFormat::Tiff16,
] {
let mut s = settings(format);
s.colour_space = space;
let mut f = frame(8, 8);
f.space = space;
let out = export(&f, &s, "a".into())
.unwrap_or_else(|e| panic!("{space:?} as {format:?}: {e}"));
assert!(!out.bytes.is_empty());
}
}
}
#[test]
fn the_formats_without_an_encoder_say_so() {
for format in [ExportFormat::Avif, ExportFormat::JpegXl] {