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

Render a film stock on the GPU, and let it take over the rendering

Browse Source 
The stock model landed in dr-film with no way to see it. This is the
pipeline node, the two texture bindings it reads, and the end-to-end test
that proves the shader agrees with the model.

The design point is that a film simulation is not an adjustment. Every
other node changes a picture; this one makes it. A stock's characteristic
curve does the camera profile's base curve's job -- from measurements
rather than from a curve somebody drew -- so running both renders the
scene twice: the camera's rendering, and then a film's rendering of that.
It looks like neither, and it reads as a colour-management bug with no
colour-management bug to find.

So `Operation::renders` is new. A node declaring it takes camera RGB and
hands back linear sRGB, and the composer emits neither the base curve nor
the conversion out of camera space. Both halves move together, and the
composer keeps them as one string precisely so that getting half of it
right is impossible.

The tables are not parameters, for the reason vignetting's coefficients
are not: they are measurements. dr-pipeline declares the layout as a plain
struct and keeps its no-dependency property; the two crates share no types
on purpose. `EditGraph::set_film_tables` offers them to every node rather
than to the one that wants them, because knowing which concrete type is
which is what the graph is organised not to know.

Bindings 4 and 5 follow the masks precedent: declared unconditionally so
one bind group layout serves every generated shader, bound to 1x1
placeholders when no stock is loaded. Both are interpolated by hand with
textureLoad -- this pipeline binds no sampler, and adding one for two
lookups would cost a binding in every shader. Uploads are keyed on content
so an unchanged stock does not push half a megabyte across the bus per
frame.

The end-to-end test earned its place immediately: it found the density
lookup being filled z-fastest while a 3D texture upload wants x-fastest,
so the red and blue axes were transposed. Green matched exactly, which is
what that bug looks like -- a plausible photograph of the wrong colour,
and one that every unit test on either side of the seam passes. dr-film
now pins the layout in a test that needs no device, and states it where
the field is declared.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-25 15:12:56 +02:00
co-authored by Claude Opus 5
parent b6a95e1965
commit 4a2fcb6d22
14 changed files with 1222 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/dr-pipeline/src/operation.rs
+190 -63
View File
@@ -262,6 +262,40 @@ pub trait Operation: Send + Sync {
Affects::Colour
}
/// TRACES: FR-DEV-3f
/// Hand this operation a stock's measured tables, if it wants them.
///
/// Default: ignore them, which is right for every operation that is a
/// function of its parameters alone.
///
/// A named method rather than a downcast or a bag of profiles, because
/// there is one caller and inventing a general mechanism for it would be
/// guessing at the shape of the next one. `vignetting`, `distortion` and
/// `aberration` already carry lens measurements through `set_profile` and
/// are not yet reached from the graph at all; when they are, this is the
/// shape it should take.
fn set_film_tables(&mut self, _tables: Option<&crate::ops::film_sim::FilmTables>) {}
/// TRACES: FR-DEV-3e | FR-DEV-3f
/// Whether this operation *is* the rendering, rather than an adjustment to
/// one.
///
/// Almost everything returns `false`. An operation that returns `true`
/// takes camera RGB and hands back linear sRGB, and in exchange the
/// composer emits neither the camera profile's base curve nor the
/// conversion out of camera space — because this operation has done both.
///
/// The reason it is a trait method and not a flag the caller sets is the
/// one [`compose_full`] gives for deciding the output mode the same way: a
/// caller that got it wrong would produce a shader that compiles, runs, and
/// renders the picture twice. `film_sim` is the operation this exists for —
/// a stock's characteristic curve does the base curve's job, from
/// measurements, and running both is the camera's rendering of the scene
/// followed by a film's rendering of *that*.
fn renders(&self) -> bool {
false
}
/// TRACES: FR-DEV-3 | FR-DEV-8
/// This operation's neighbourhood stage, if it has one.
///
@@ -478,6 +512,11 @@ pub fn compose_full(
OutputMode::Encoded
};
// Whether an operation has taken over the rendering. Decided from the
// operations for the same reason `output_mode` is: a caller that got it
// wrong would produce a shader that compiles and renders the picture twice.
let op_renders = ops.iter().any(|o| o.is_active() && o.renders());
let mut uniform_fields = String::new();
let mut uniform_values: Vec<f32> = Vec::new();
let mut body = String::new();
@@ -658,6 +697,82 @@ pub fn compose_full(
),
};
// The camera profile's rendering, which an operation may have taken over.
//
// Emitted as a unit because the two halves belong together: the base curve
// is defined in camera RGB and the matrix is what leaves it, so an
// operation that replaces one has necessarily replaced the other. Keeping
// them as one string is what makes that impossible to get half right.
let rendering_tail = if op_renders {
" // The camera profile's base curve and the conversion out of camera\n // space are both absent: an operation declaring `Operation::renders`\n // has done both, and doing them again would render the picture twice.\n"
.to_string()
} else {
format!(
" // ==== camera profile: the base curve (FR-DEV-3e) ====
//
// Marked with `====` and not the `----` an operation block carries: this
// is not one, and the difference is what several tests count on to tell
// an edit apart from the reading of a file.
//
// The stage between demosaic and the working space that turns a correct
// exposure into a photograph. Sensor data is scene-referred and nearly
// linear; nothing anybody looks at is. Rendering it straight out is the
// dcraw default, and it is flat, dark through the midtones and clips its
// highlights instead of rolling them off.
//
// **In camera RGB, and after the adjustments**, which is a deliberate pair
// of choices:
//
// - Before the matrix, because that is where a base curve is defined and
// where every other converter applies one. The curve was tuned against
// this body's own primaries; moving it after the conversion would apply
// a Canon rendering to sRGB values and change what it does.
// - After exposure and the tonal operations, because those are corrections
// to *capture* and are only meaningful on linear values. A stop is a
// doubling; run exposure after a curve and it stops being one.
//
// Per channel rather than on luminance. It desaturates the extremes
// slightly, and that is the point — it is what makes a blown sky roll
// toward white rather than toward a saturated corner of the gamut, and it
// is what the camera's own JPEG does.
//
// The branch is on a uniform, so the whole dispatch takes the same path.
// It is off for a JPEG and any other already-rendered source, which must
// not be rendered twice, and for a body the profile database declines to
// offer any curve for at all.
if (u.base_curve_last.z > 0.5) {{
c = vec3<f32>(
curve_eval(
u.base_curve_x.x, u.base_curve_y.x, u.base_curve_x.y, u.base_curve_y.y,
u.base_curve_x.z, u.base_curve_y.z, u.base_curve_x.w, u.base_curve_y.w,
u.base_curve_last.x, u.base_curve_last.y, c.r,
),
curve_eval(
u.base_curve_x.x, u.base_curve_y.x, u.base_curve_x.y, u.base_curve_y.y,
u.base_curve_x.z, u.base_curve_y.z, u.base_curve_x.w, u.base_curve_y.w,
u.base_curve_last.x, u.base_curve_last.y, c.g,
),
curve_eval(
u.base_curve_x.x, u.base_curve_y.x, u.base_curve_x.y, u.base_curve_y.y,
u.base_curve_x.z, u.base_curve_y.z, u.base_curve_x.w, u.base_curve_y.w,
u.base_curve_last.x, u.base_curve_last.y, c.b,
),
);
}}
// Camera space -> linear sRGB. Applied after the adjustments so white
// balance and exposure act on sensor-native values, which is where they
// are physically meaningful.
//
// Identity for a non-linear source, which is already in sRGB primaries.
c = vec3<f32>(
dot(u.cam_to_srgb_0.rgb, c),
dot(u.cam_to_srgb_1.rgb, c),
dot(u.cam_to_srgb_2.rgb, c),
);
")
};
let source = format!(
"// GENERATED — do not edit.
//
@@ -678,6 +793,13 @@ struct Params {{
// changed with the edit would mean rebuilding the pipeline layout, and the
// cost of the unused declaration is a 1x1 placeholder texture.
@group(0) @binding(3) var masks: texture_2d_array<f32>;
// A film stock's baked tables (FR-DEV-3f): the characteristic curves, and the
// density lookup that carries everything downstream of them. Declared
// unconditionally for the same reason the masks above are — one bind group
// layout for every generated shader — and bound to 1x1 placeholders when no
// stock is loaded, which costs eight bytes and no branch.
@group(0) @binding(4) var film_curves: texture_2d<f32>;
@group(0) @binding(5) var film_lut_texture: texture_3d<f32>;
{sampler_helper}{helper_src}{encode_output}
// Display-encoded sRGB back to linear, for sources that arrive that way.
@@ -746,69 +868,7 @@ fn main(@builtin(global_invocation_id) gid: vec3<u32>) {{
c = mix(c, neutral, clipped);
}}
{body}
// ==== camera profile: the base curve (FR-DEV-3e) ====
//
// Marked with `====` and not the `----` an operation block carries: this
// is not one, and the difference is what several tests count on to tell
// an edit apart from the reading of a file.
//
// The stage between demosaic and the working space that turns a correct
// exposure into a photograph. Sensor data is scene-referred and nearly
// linear; nothing anybody looks at is. Rendering it straight out is the
// dcraw default, and it is flat, dark through the midtones and clips its
// highlights instead of rolling them off.
//
// **In camera RGB, and after the adjustments**, which is a deliberate pair
// of choices:
//
// - Before the matrix, because that is where a base curve is defined and
// where every other converter applies one. The curve was tuned against
// this body's own primaries; moving it after the conversion would apply
// a Canon rendering to sRGB values and change what it does.
// - After exposure and the tonal operations, because those are corrections
// to *capture* and are only meaningful on linear values. A stop is a
// doubling; run exposure after a curve and it stops being one.
//
// Per channel rather than on luminance. It desaturates the extremes
// slightly, and that is the point — it is what makes a blown sky roll
// toward white rather than toward a saturated corner of the gamut, and it
// is what the camera's own JPEG does.
//
// The branch is on a uniform, so the whole dispatch takes the same path.
// It is off for a JPEG and any other already-rendered source, which must
// not be rendered twice, and for a body the profile database declines to
// offer any curve for at all.
if (u.base_curve_last.z > 0.5) {{
c = vec3<f32>(
curve_eval(
u.base_curve_x.x, u.base_curve_y.x, u.base_curve_x.y, u.base_curve_y.y,
u.base_curve_x.z, u.base_curve_y.z, u.base_curve_x.w, u.base_curve_y.w,
u.base_curve_last.x, u.base_curve_last.y, c.r,
),
curve_eval(
u.base_curve_x.x, u.base_curve_y.x, u.base_curve_x.y, u.base_curve_y.y,
u.base_curve_x.z, u.base_curve_y.z, u.base_curve_x.w, u.base_curve_y.w,
u.base_curve_last.x, u.base_curve_last.y, c.g,
),
curve_eval(
u.base_curve_x.x, u.base_curve_y.x, u.base_curve_x.y, u.base_curve_y.y,
u.base_curve_x.z, u.base_curve_y.z, u.base_curve_x.w, u.base_curve_y.w,
u.base_curve_last.x, u.base_curve_last.y, c.b,
),
);
}}
// Camera space -> linear sRGB. Applied after the adjustments so white
// balance and exposure act on sensor-native values, which is where they
// are physically meaningful.
//
// Identity for a non-linear source, which is already in sRGB primaries.
c = vec3<f32>(
dot(u.cam_to_srgb_0.rgb, c),
dot(u.cam_to_srgb_1.rgb, c),
dot(u.cam_to_srgb_2.rgb, c),
);
{to_output}
{rendering_tail}{to_output}
{store}
}}
",
@@ -1325,6 +1385,73 @@ mod tests {
assert!(wb < op, "as-shot white balance must precede the operations");
}
#[test]
fn a_rendering_operation_takes_over_the_base_curve_and_the_camera_matrix() {
// TRACES: FR-DEV-3e | FR-DEV-3f
// A film stock's characteristic curve does the base curve's job, and
// the film node converts out of camera space itself. Emitting the
// profile's rendering as well would render the scene twice and convert
// it twice — a picture that comes out looking like neither the camera's
// rendering nor the film's, with a colour-management bug's signature
// and no colour-management bug to find.
let mut film = crate::ops::FilmSim::new();
film.set_film_tables(Some(&crate::ops::FilmTables {
exposure_matrix: [[5.0, 0.5, 0.2], [0.1, 5.0, 0.3], [0.2, 0.5, 4.0]],
curves: vec![[0.5, 0.5, 0.5]; crate::ops::film_sim::CURVE_SAMPLES],
curve_log_min: -3.0,
curve_log_max: 4.0,
lut: vec![[0.5, 0.5, 0.5]; 32 * 32 * 32],
density_max: 3.0,
lut_size: 32,
}));
assert!(film.is_active(), "the fixture did not load");
let source = compose(&[Box::new(film) as Box<dyn Operation>]).source;
assert!(
source.contains("---- film_sim ----"),
"the operation itself must still be emitted"
);
assert!(
!source.contains("base_curve_last.z > 0.5"),
"the base curve is still being applied on top of the film"
);
// Asserted on the composer's own comment, not on the conversion
// itself: the film fragment performs exactly the same three dot
// products, so a substring search cannot tell the composer's copy from
// the operation's. What must be gone is the *second* one.
assert!(
!source.contains("Camera space -> linear sRGB"),
"the composer converted out of camera space after the film already had"
);
assert_eq!(
source.matches("dot(u.cam_to_srgb_0.rgb, c)").count(),
1,
"camera space is left exactly once, and it is the film that does it"
);
}
#[test]
fn an_operation_that_does_not_render_leaves_the_profile_alone() {
// The other half, and the one that would fail silently: a bug that
// suppressed the tail unconditionally renders every ordinary edit
// flat and uncorrected, which reads as a broken camera profile.
let source = compose(&[fake(&DESC_A, 2.0, false)]).source;
assert!(source.contains("base_curve_last.z > 0.5"));
assert!(source.contains("Camera space -> linear sRGB"));
}
#[test]
fn an_inactive_film_node_leaves_the_profile_alone() {
// `renders()` is a property of the type, but the suppression must key
// off whether it is *active*. A film node sitting in the chain with no
// stock loaded is the default state of every photograph in the
// catalogue, and it must not disturb the camera's own rendering.
let film: Box<dyn Operation> = Box::new(crate::ops::FilmSim::new());
let source = compose(&[film, fake(&DESC_A, 2.0, false)]).source;
assert!(source.contains("base_curve_last.z > 0.5"));
assert!(source.contains("Camera space -> linear sRGB"));
}
#[test]
fn the_camera_matrix_is_applied_after_the_operations() {
// Adjustments are meaningful in sensor-native space, where highlight