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

Replace the per-body base curve with a scene-referred view transform

Browse Source 
The base curve was a five-point spline on the unit square, flat past its
last point: every value above 1.0 left it as the same number, per
channel. Exposure and highlight recovery put values up there, and the
curve threw them away, then handed the result on as though it were
still scene-linear. The six per-body curves were also, by their own
file's account, hand-tuned shapes rather than measurements, and not
enough is known about where they came from to keep them (D19).

In their place, one view transform for every body (FR-DEV-3j): a
log-logistic sigmoid per channel, with the middle channel put back
between the other two so a hue survives the shoulder. Its two free
constants are solved from two conditions rather than set: scene grey
0.13, where the retired default curve put it, lands on display 0.18,
and the scene white four stops above grey lands on 1.0. So a highlight
a stop past sensor saturation still rolls into white, and the midtones
stay within 0.26 EV of the retired default between scene 0.03 and 1.0.
`dr_pipeline::view` holds the CPU reference and the WGSL, and the tests
there are FR-DEV-3j's acceptance criteria.

It is still fixed and still in the fused pass's tail, so a detail stage
still sees rendered values; the next commits make it an operation and
move it after the detail stage. It is skipped for a JPEG, as the base
curve was, and absent from the camera-space tap.

The base curve's database, its lookup and its twelve uniform slots go.
`RawImage` and `DemosaicedImage` lose the field, and the GPU test that
proved a curve reached the shader is replaced by one that renders the
view transform against the CPU reference and shows two highlights above
1.0 still render apart. The JPEG-and-sensor test now asserts the two
differ by exactly the view transform, where before an identity fixture
curve had made them match.
This commit is contained in:
dtourolle
2026-09-27 16:52:53 -04:00
parent db7b84795c
commit 37a6d99dc4
23 changed files with 564 additions and 1442 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/dr-pipeline/src/operation.rs
+67 -163
View File
@@ -557,12 +557,12 @@ pub struct ComposedShader {
/// Fields the generated uniform struct always carries, before op uniforms.
///
/// WGSL requires a uniform struct to be non-empty and 16-byte aligned; these
/// are needed by every generated shader in any case.
/// are needed by every generated shader in any case: the matrix, the as-shot
/// balance and the sample cache's flags. Framing's block follows.
///
/// Twelve of the twenty-eight are the camera profile's base curve
/// ([`BASE_CURVE_UNIFORM_FIELDS`]); the rest are the matrix, the as-shot
/// balance and framing's own block.
const BASE_UNIFORM_FIELDS: usize = 16 + SAMPLE_CACHE_UNIFORM_FIELDS + BASE_CURVE_UNIFORM_FIELDS;
/// Twelve fewer than before D19, which retired the base curve that sat at the
/// end of this block.
const BASE_UNIFORM_FIELDS: usize = 16 + SAMPLE_CACHE_UNIFORM_FIELDS;
/// Slots the sample cache's two flags occupy: read, write, and two spare to
/// keep the block a whole `vec4`. See [`ComposedShader::sample_key`].
@@ -571,36 +571,11 @@ const SAMPLE_CACHE_UNIFORM_FIELDS: usize = 4;
/// Where the sample cache's flags sit in the generated uniform block: `x` says
/// read the source colour from the cache, `y` says write it there.
///
/// Exported for the reason [`BASE_CURVE_UNIFORM_OFFSET`] is — `dr-gpu` writes
/// Exported for the reason [`RESERVED_UNIFORM_FIELDS`] is — `dr-gpu` writes
/// these by index — and zero in every block the composer hands out, so a
/// caller that never heard of the cache gets the direct read it always had.
pub const SAMPLE_CACHE_UNIFORM_OFFSET: usize = 16;
/// TRACES: FR-DEV-3e
/// Slots the base curve occupies: five `(x, y)` points and an active flag.
///
/// Twelve rather than eleven so the block stays a whole number of `vec4`s,
/// which is what std140 requires of a uniform struct's members. The spare
/// float is left zero rather than repurposed — a uniform slot that means one
/// thing today and two things next year is how a shader comes to read a
/// highlight rolloff out of a crop rectangle.
const BASE_CURVE_UNIFORM_FIELDS: usize = 12;
/// TRACES: FR-DEV-3e
/// Where the base curve's slots begin in the generated uniform block.
///
/// Exported for the same reason [`RESERVED_UNIFORM_FIELDS`] is: `dr-gpu`
/// writes these by index, and an offset computed independently at both ends is
/// an offset that will eventually disagree with itself.
pub const BASE_CURVE_UNIFORM_OFFSET: usize =
SAMPLE_CACHE_UNIFORM_OFFSET + SAMPLE_CACHE_UNIFORM_FIELDS;
/// How many control points a base curve carries.
///
/// The same five the tone curve widget has, deliberately — see the helper
/// selection in [`compose_full`].
pub const BASE_CURVE_POINTS: usize = 5;
/// Where the highlight desaturation begins: the fraction of the white level
/// above which a photosite is treated as clipped.
///
@@ -869,42 +844,20 @@ fn compose_inner(
\x20 // The sample cache (see `ComposedShader::sample_key`): `.x` reads\n\
\x20 // the source colour from `sampled`, `.y` writes it to\n\
\x20 // `sample_out`. Zero for both is the direct read.\n\
\x20 sample_cache: vec4<f32>,\n\
\x20 // The camera profile's base curve (FR-DEV-3e): five points on a\n\
\x20 // monotone spline, packed as x0..x3, y0..y3, then (x4, y4, on).\n\
\x20 // `.z` of the last is the flag, not padding — it is 0 for a\n\
\x20 // body with no profile and for an already-rendered source.\n\
\x20 base_curve_x: vec4<f32>,\n\
\x20 base_curve_y: vec4<f32>,\n\
\x20 base_curve_last: vec4<f32>,\n",
\x20 sample_cache: vec4<f32>,\n",
);
uniform_values.resize(BASE_UNIFORM_FIELDS, 0.0);
// TRACES: FR-DEV-3e
// The spline the base curve is evaluated on is the *tone curve's* spline,
// reached through the trait rather than reimplemented here.
//
// Two reasons, and the second is the one that matters. The obvious one is
// that a shader carrying two `curve_eval`s would not compile, and the
// composer's helper de-duplication is what makes both stages able to ask
// for it. The real one is that a profile author placing a control point
// and a photographer dragging one must mean the same thing by it — down to
// the Fritsch-Carlson tangent limiting, which is what decides how a
// shoulder actually rolls off. Two implementations that agreed today would
// be two that could disagree later, and the disagreement would show up as
// a body whose profile renders subtly differently from the curve someone
// drew to match it.
//
// Emitted unconditionally, unlike an operation's helpers. The base curve
// is active for every RAW frame — an unprofiled body still gets the
// database's default rendering — so making the shader's shape depend on it
// would split the pipeline cache in two for no benefit. The uniform flag
// above turns it off for the cases that are genuinely already rendered,
// and a branch on a uniform is coherent across the whole dispatch.
for h in crate::ops::ToneCurve::new().helpers() {
if matches!(h.name, "curve_span" | "curve_eval") {
helpers.push(*h);
}
// TRACES: FR-DEV-3j
// The view transform's function, whenever the composer emits the view
// transform — which is every render but the camera-space tap and one a
// rendering operation has taken over. See `rendering_tail` below.
let views = !op_renders && output_mode != OutputMode::CameraLinear;
if views {
helpers.push(Helper {
name: "view_sigmoid",
source: crate::view::VIEW_SIGMOID_WGSL,
});
}
// Framing's block follows the base one at a fixed offset, for the same
@@ -1161,47 +1114,32 @@ fn compose_inner(
),
};
// The base curve, which an operation may have taken over.
let rendering_tail = if op_renders {
" // The base curve is absent: an operation declaring\n // `Operation::renders` has done its job, and doing it again would render\n // the picture twice.\n"
// The view transform, which an operation may have taken over.
let rendering_tail = if !views {
" // No view transform: an operation declaring `Operation::renders` has\n // mapped the scene to a display range itself, or this is the\n // camera-space tap, which stores the sensor's own numbers.\n"
.to_string()
} else {
" // ==== camera profile: the base curve (FR-DEV-3e) ====
let curve = crate::view::Sigmoid::default_curve();
format!(
" // ==== the view transform (FR-DEV-3j) ====
//
// 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.
// an edit apart from the rendering of one.
//
// After every operation, and — since D19 moved the matrix to the front of
// the chain — on working-space colour rather than the camera RGB it was
// tuned against. An interim placement: the per-body curve is being
// replaced by a view transform after the detail stage (FR-DEV-3j).
// The one stage allowed to map scene-linear colour to a display range
// (D19, ARCH §6.14), after every operation. Everything above it is
// unbounded; everything the shoulder has not brought under 1.0 is
// clipped by the output transform, at the last moment.
//
// 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,
),
);
}
"
.to_string()
// Skipped for an already-rendered source — a JPEG is a display rendering
// already, and rendering it again would compress it twice.
if (!non_linear) {{
c = view_sigmoid(c, {:?}, {:?}, {:?});
}}
",
curve.n, curve.inv_k, curve.w
)
};
// Formatted with Rust's `Display` so the shader reads the same threshold
@@ -2270,10 +2208,10 @@ mod tests {
}
#[test]
fn a_rendering_operation_takes_over_the_base_curve() {
// TRACES: FR-DEV-3e | FR-DEV-3f
// A film stock's characteristic curve does the base curve's job.
// Emitting the profile's rendering as well would render the scene
fn a_rendering_operation_takes_over_the_view_transform() {
// TRACES: FR-DEV-3j | FR-DEV-3f
// A film stock's characteristic curve does the view transform's job.
// Emitting the default rendering as well would render the scene
// 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.
@@ -2299,8 +2237,8 @@ mod tests {
.find("---- film_sim ----")
.expect("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"
!source.contains("view_sigmoid"),
"the view transform is still being applied on top of the film"
);
// D19: the film no longer converts out of camera space itself. The
// composer does, once, ahead of it — the film is handed working-space
@@ -2323,7 +2261,7 @@ mod tests {
// suppressed the tail unconditionally renders every ordinary edit
// flat and uncorrected, which reads as a broken camera profile.
let source = compose(&[fake(DESC_A.clone(), 2.0, false)]).source;
assert!(source.contains("base_curve_last.z > 0.5"));
assert!(source.contains("c = view_sigmoid("));
assert!(source.contains("camera profile: the matrix"));
}
@@ -2335,7 +2273,7 @@ mod tests {
// 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.clone(), 2.0, false)]).source;
assert!(source.contains("base_curve_last.z > 0.5"));
assert!(source.contains("c = view_sigmoid("));
assert!(source.contains("camera profile: the matrix"));
}
@@ -2374,8 +2312,8 @@ mod tests {
}
#[test]
fn the_base_curve_runs_after_the_operations() {
// TRACES: FR-DEV-3e
fn the_view_transform_runs_after_the_operations() {
// TRACES: FR-DEV-3j
// Exposure and the tonal controls are corrections to capture, and
// they are only meaningful on linear values. A stop is a doubling; run
// exposure after a curve and it is not one any more, and every slider
@@ -2383,66 +2321,23 @@ mod tests {
let ops = vec![fake(DESC_A.clone(), 2.0, false)];
let source = compose(&ops).source;
let op = source.find("---- op_a ----").expect("op present");
let curve = source
.find("if (u.base_curve_last.z > 0.5)")
.expect("base curve applied");
assert!(op < curve, "the base curve must come after the operations");
}
#[test]
fn the_base_curve_reaches_a_shader_with_no_operations_at_all() {
// TRACES: FR-DEV-3e
// The same property as as-shot white balance, and for the same reason:
// it is part of interpreting the file, not part of the edit. An
// unedited RAW must open looking like a photograph rather than like a
// scan of one.
let shader = compose(&[]);
assert!(shader.source.contains("u.base_curve_x"));
let view = source.find("c = view_sigmoid(").expect("view applied");
assert!(
shader.source.contains("fn curve_eval("),
"the spline it is evaluated on must be emitted too"
op < view,
"the view transform must come after the operations"
);
}
#[test]
fn the_base_curve_and_the_tone_curve_share_one_spline() {
// TRACES: FR-DEV-3e
// Two `curve_eval`s in one shader would not compile — but the reason
// the helper is *shared* rather than merely renamed is that a profile
// author placing a control point and a photographer dragging one must
// mean the same thing by it, down to the tangent limiting that decides
// how a shoulder rolls off.
let mut curve = crate::ops::ToneCurve::new();
curve.set_param(crate::ops::curve::P2_Y, 0.7);
assert!(
curve.is_active(),
"the fixture must actually reach the shader"
);
let source = compose(&[Box::new(curve)]).source;
assert_eq!(
source.matches("fn curve_eval(").count(),
1,
"the spline must be declared exactly once"
);
assert_eq!(source.matches("fn curve_span(").count(), 1);
}
#[test]
fn the_base_curve_owns_the_slots_dr_gpu_writes() {
// TRACES: FR-DEV-3e
// `dr-gpu` fills these by index. The offset is exported rather than
// recomputed there, and this asserts the exported number still points
// at the block the shader declares — the failure otherwise is a
// highlight rolloff read out of a crop rectangle, which renders as
// nonsense rather than as an error.
assert_eq!(
BASE_CURVE_UNIFORM_OFFSET + BASE_CURVE_UNIFORM_FIELDS,
BASE_UNIFORM_FIELDS,
"the base curve must be the last thing in the base block"
);
assert_eq!(BASE_CURVE_POINTS * 2 + 1, BASE_CURVE_UNIFORM_FIELDS - 1);
assert!(compose(&[]).uniforms.len() >= BASE_UNIFORM_FIELDS);
fn the_view_transform_reaches_a_shader_with_no_operations_at_all() {
// TRACES: FR-DEV-3j
// An unedited RAW must open looking like a photograph rather than
// like a scan of one, and it is skipped only for a source that is
// already a rendering.
let source = compose(&[]).source;
assert!(source.contains("c = view_sigmoid("));
assert!(source.contains("fn view_sigmoid("));
assert!(source.contains("if (!non_linear)"));
}
/// Compose with neutral framing into a chosen output space.
@@ -2450,6 +2345,15 @@ mod tests {
compose_with_framing(ops, &Framing::new(), output)
}
#[test]
fn the_camera_space_tap_has_no_view_transform() {
// TRACES: FR-MRG-2
// A merge stitches the sensor's own numbers; a rendering in them
// would be developed a second time when the composite is opened.
let source = compose_camera_probe(&[], &crate::framing::Framing::default()).source;
assert!(!source.contains("view_sigmoid"));
}
#[test]
fn an_srgb_render_is_byte_for_byte_what_it_was_before_output_spaces_existed() {
// The display path is the shader compiled on nearly every frame, and