Give the tone curve a curve for each colour channel

The declaration in ops/tone_curve.yaml has claimed per-channel curves since
it was written — it is the justification for the operation carrying both
`tone` and `colour`. Only the master curve existed. This is the other three.

The master runs first and the channels grade its result. Both orders are
real images and they differ visibly, so the choice is made and written down
rather than left to the loop: a point placed on the blue curve should act on
the tone the photographer can see, which is what the master has already
produced. The other order anchors the grade to tones the master is about to
move, so adjusting contrast slides a warm shadow up into the midtones.

Every id that existed before today is spelled exactly as it was. The master
curve keeps `p2_y` and the new curves take `r_`, `g_` and `b_` prefixes, so
a sidecar written when there was one curve loads, means what it meant, and
renders the same shader — asserted on the generated source, not on the
parameter values. Nothing needed a version check because nothing was
renamed.

Each curve reaches the shader only when it has been moved off the diagonal,
so an S-curve and no colour work generates what it generated when this
operation held ten parameters instead of forty, down to the uniform names.
The monotonicity guarantee is enforced per curve: a coincident pair on blue
divides by zero exactly as thoroughly as one on the master.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-22 16:06:13 +02:00
co-authored by Claude Opus 5
parent 586698db00
commit d64a61d677
6 changed files with 1339 additions and 174 deletions
+139
View File
@@ -0,0 +1,139 @@
//! TRACES: FR-DEV-3
//! The tone curve's four curves, on a device.
//!
//! `dr-pipeline` asserts that the right WGSL is generated and `dr-gpu`'s other
//! tests assert that a shader runs; neither notices a fragment that says
//! exactly what it should and does not compile, or one that compiles and puts
//! the red curve's uniforms into the blue slot. So this renders flat grey
//! through each curve and looks at what came out.
//!
//! Flat grey because it makes every assertion a comparison between the three
//! components of one pixel: a curve that is meant to be chromatic must move
//! them apart, and one that is meant to be tonal must not.
use dr_gpu::{AdjustPass, DemosaicedImage, GpuContext};
use dr_pipeline::ops::curve::{self, Axis, Channel};
use dr_pipeline::EditGraph;
const SIZE: u32 = 8;
fn ctx() -> Option<GpuContext> {
pollster::block_on(GpuContext::new_headless()).ok()
}
/// The centre pixel's red, green and blue, after `graph` has run over flat
/// mid-grey.
fn rendered(ctx: &GpuContext, graph: &EditGraph) -> (u8, u8, u8) {
let data: Vec<u8> = (0..SIZE * SIZE).flat_map(|_| [128, 128, 128, 255]).collect();
let source = DemosaicedImage::from_rgba8(ctx, &data, SIZE, SIZE).expect("upload");
// Composed the way the display path composes it. A curve that generates
// invalid WGSL fails at `render` below, which is the point of running this
// on a device at all.
let shader = graph.compose();
let mut adjust = AdjustPass::new(ctx);
adjust.render(&source, &shader, SIZE, SIZE).expect("render");
let pixels = adjust.export_pixels().expect("readback").0;
let at = ((SIZE / 2 * SIZE + SIZE / 2) * 4) as usize;
(pixels[at], pixels[at + 1], pixels[at + 2])
}
/// A curve with its mid-point lifted — the simplest edit that is unmistakably
/// an edit.
fn lifted(channel: Channel) -> EditGraph {
let mut graph = EditGraph::default_chain();
graph.set_param(
curve::ID,
curve::coordinate(channel, 2, Axis::Y),
0.75,
);
graph
}
#[test]
fn the_master_curve_lifts_every_component_together() {
let Some(ctx) = ctx() else {
eprintln!("no adapter; skipping");
return;
};
let (r0, g0, b0) = rendered(&ctx, &EditGraph::default_chain());
let (r, g, b) = rendered(&ctx, &lifted(Channel::Master));
assert!(r > r0, "the master curve did not lift the image: {r} vs {r0}");
// Grey in, grey out: the master curve is applied as a ratio over
// luminance, so it changes tone and not hue. A tolerance of one code
// value, because the components travel through the ratio separately and
// the result is quantised to eight bits.
assert!(
r.abs_diff(g) <= 1 && g.abs_diff(b) <= 1,
"the master curve tinted a neutral pixel: {r},{g},{b}"
);
assert_eq!((g0, b0), (r0, r0), "the unedited image is neutral");
}
#[test]
fn a_channel_curve_lifts_only_its_own_component() {
let Some(ctx) = ctx() else {
eprintln!("no adapter; skipping");
return;
};
let (r0, g0, b0) = rendered(&ctx, &EditGraph::default_chain());
for (channel, name) in [
(Channel::Red, "red"),
(Channel::Green, "green"),
(Channel::Blue, "blue"),
] {
let (r, g, b) = rendered(&ctx, &lifted(channel));
// The component the curve names moves; the other two stay exactly
// where they were. This is what catches a fragment whose uniforms are
// wired to the wrong curve — it would still lift *something*.
let (moved, still) = match channel {
Channel::Red => (r > r0, g == g0 && b == b0),
Channel::Green => (g > g0, r == r0 && b == b0),
_ => (b > b0, r == r0 && g == g0),
};
assert!(moved, "the {name} curve changed nothing: {r},{g},{b}");
assert!(
still,
"the {name} curve moved a component that was not its own: \
{r},{g},{b} from {r0},{g0},{b0}"
);
}
}
#[test]
fn the_master_and_the_channels_compose_in_one_pass() {
// All four curves at once: the case where the generated fragment is
// longest, every helper is present, and forty uniforms are in the block.
// Mostly a compile check, which is why the assertion is only that the
// result is a colour and not the one we started with.
let Some(ctx) = ctx() else {
eprintln!("no adapter; skipping");
return;
};
let mut graph = EditGraph::default_chain();
graph.set_param(curve::ID, curve::P1_Y, 0.15);
graph.set_param(curve::ID, curve::P3_Y, 0.85);
for (channel, y) in [
(Channel::Red, 0.55),
(Channel::Green, 0.5),
(Channel::Blue, 0.62),
] {
graph.set_param(curve::ID, curve::coordinate(channel, 2, Axis::Y), y);
}
let (r0, _, _) = rendered(&ctx, &EditGraph::default_chain());
let (r, g, b) = rendered(&ctx, &graph);
assert!(
(r, g, b) != (r0, r0, r0),
"four active curves left the image untouched"
);
// Red and blue were pushed apart from green, which is the chromatic half
// doing its work on top of the tonal one.
assert!(b > g, "blue was lifted above green: {r},{g},{b}");
}
+4 -3
View File
@@ -209,9 +209,10 @@ They still belong in this directory, because the pipeline's **order** is the
one thing a reader comes here to learn, and an order written half in YAML and
half in Rust would be worse than either alone.
Currently hand-written: `tone_curve` (a curve widget over five interpolated
points), `colour_mixer` (thirty-six faceted parameters from twelve computed hue
bands). `vignetting` is hand-written too but is not in the develop chain — it
Currently hand-written: `tone_curve` (one widget over four curves of five
interpolated points — master, red, green, blue — each reaching the shader only
when it has been moved), `colour_mixer` (thirty-six faceted parameters from
twelve computed hue bands). `vignetting` is hand-written too but is not in the develop chain — it
carries lens-profile coefficients that are not parameters. `distortion` and
`aberration` are `Warp`s rather than operations: they rewrite coordinates
before sampling rather than transforming a colour after it.
+18 -5
View File
@@ -16,13 +16,26 @@ attributes: [tone, colour]
rust: ToneCurve
why_rust: |
Five control points presented as one curve widget, with an interpolator and
a monotonicity guarantee behind it. Its neutral is a *relationship* between
parameters rather than a set of values — the identity diagonal — which is
not something the declarative `active:` rule can express, and its
`presentation()` spans parameters rather than describing one.
Four curves — master, red, green, blue — of five control points each,
presented as one widget, with an interpolator and a monotonicity guarantee
behind them. Its neutral is a *relationship* between parameters rather than
a set of values — the identity diagonal, on every channel — which is not
something the declarative `active:` rule can express; its `presentation()`
spans forty parameters rather than describing one; and its fragment is
*assembled* rather than written, because each curve reaches the shader only
when it has been moved off the diagonal. A declared node's `wgsl:` is one
fixed block of text, which is the right shape for nearly everything here and
the wrong one for a node whose cost has to follow what the photographer
actually touched.
placement: |
After the fixed-weight region controls, so the curve is the final word on
tone: a photographer reaches for it to fix what those controls could not
place exactly.
Within the node, the master curve runs before the per-channel ones. Both
orders are visibly different images and the reasons for this one are written
out in `src/ops/curve.rs`: the master is tonal and hue-preserving, the
per-channel curves are the chromatic grade over the tones it produced, and a
point placed on a channel curve should act on the tone the photographer can
see rather than on the one the master is about to move.
File diff suppressed because it is too large Load Diff
+85 -1
View File
@@ -1004,7 +1004,7 @@ impl std::error::Error for ParseError {}
mod tests {
use super::*;
use crate::framing;
use crate::ops::{exposure, saturation, white_balance};
use crate::ops::{curve, exposure, saturation, white_balance};
fn edited() -> EditGraph {
let mut g = EditGraph::default_chain();
@@ -1189,6 +1189,90 @@ mod tests {
assert_eq!(once, twice);
}
/// TRACES: FR-DEV-3 | FR-CAT-8
/// A file written before the tone curve had per-channel curves.
///
/// Spelled out as literal text rather than produced by `to_text`, because
/// the claim is about *those bytes*: a sidecar generated by this build
/// would agree with this build by construction, and would go on agreeing
/// with it through a rename that broke every file on disk.
#[test]
fn a_sidecar_from_before_the_channel_curves_still_names_the_master() {
let text = "drsc 1\n\n[version u1]\nname = Default\nrevision = 4\nmodified = 9\n\
tone_curve.p1_y = 0.15\ntone_curve.p3_y = 0.85\n";
let parsed = Sidecar::parse(text).expect("valid");
let mut g = EditGraph::default_chain();
parsed.default_version().expect("a version").apply(&mut g);
// The S-curve the file describes, on the master curve and nowhere
// else.
assert_eq!(g.param(curve::ID, curve::P1_Y), Some(0.15));
assert_eq!(g.param(curve::ID, curve::P3_Y), Some(0.85));
for channel in [curve::Channel::Red, curve::Channel::Green, curve::Channel::Blue] {
for point in 0..curve::POINTS {
for axis in [curve::Axis::X, curve::Axis::Y] {
let id = curve::coordinate(channel, point, axis);
let expected = g
.capabilities()
.iter()
.find(|c| c.id == curve::ID)
.and_then(|c| c.params.iter().find(|p| p.id == id))
.map(|p| p.default);
assert_eq!(
g.param(curve::ID, id),
expected,
"{id} moved, and no line in the file mentions it"
);
}
}
}
// And writing it back produces the same two lines: the curves the file
// never mentioned are still at their defaults, so they are still
// absent (`only_non_default_values_are_written`).
let written = Sidecar::parse(&Sidecar::parse(text).expect("valid").to_text())
.expect("valid")
.to_text();
assert!(written.contains("tone_curve.p1_y = 0.15"), "{written}");
assert!(written.contains("tone_curve.p3_y = 0.85"), "{written}");
assert!(
!written.contains("tone_curve.r_"),
"an untouched channel curve was written out:\n{written}"
);
}
/// TRACES: FR-DEV-3
/// The other direction: the new curves persist like any other parameter.
#[test]
fn a_per_channel_curve_survives_the_round_trip() {
let mut g = EditGraph::default_chain();
// A faded shadow: blue lifted at the black point, red pulled down.
let blue = curve::coordinate(curve::Channel::Blue, 0, curve::Axis::Y);
let red = curve::coordinate(curve::Channel::Red, 4, curve::Axis::Y);
g.set_param(curve::ID, blue, 0.08);
g.set_param(curve::ID, red, 0.92);
g.set_param(curve::ID, curve::P2_Y, 0.55);
let mut sidecar = Sidecar::new();
sidecar.put(version_of(&g));
let text = sidecar.to_text();
// Keyed by the channel-prefixed id, which is what makes the master's
// unprefixed ones safe to leave alone.
assert!(text.contains("tone_curve.b_p0_y = 0.08"), "{text}");
let parsed = Sidecar::parse(&text).expect("valid");
let mut restored = EditGraph::default_chain();
parsed
.default_version()
.expect("a version")
.apply(&mut restored);
assert_eq!(restored.param(curve::ID, blue), Some(0.08));
assert_eq!(restored.param(curve::ID, red), Some(0.92));
assert_eq!(restored.param(curve::ID, curve::P2_Y), Some(0.55));
assert!(!restored.is_neutral());
}
#[test]
fn an_unknown_operation_survives_a_round_trip() {
// The data-loss case that matters: a device running an older build
+202
View File
@@ -0,0 +1,202 @@
//! TRACES: FR-DEV-3
//! The tone curve's four curves, seen from outside the crate.
//!
//! The unit tests beside the operation assert what one `ToneCurve` does. These
//! assert the two properties that only show up once it is a node in a chain
//! with a sidecar under it: that a file written before the per-channel curves
//! existed still describes the edit it described, and that the three new
//! curves cost a photograph that does not use them precisely nothing.
use dr_pipeline::ops::curve::{self, Axis, Channel};
use dr_pipeline::{EditGraph, Sidecar};
/// A version block carrying `params`, in the on-disk spelling.
fn sidecar_with(params: &[(&str, f32)]) -> String {
let mut text = String::from("drsc 1\n\n[version u1]\nname = Default\nrevision = 2\nmodified = 0\n");
for (key, value) in params {
text.push_str(&format!("{key} = {value}\n"));
}
text
}
fn apply(text: &str) -> EditGraph {
let sidecar = Sidecar::parse(text).expect("a valid sidecar");
let mut graph = EditGraph::default_chain();
sidecar
.default_version()
.expect("a default version")
.apply(&mut graph);
graph
}
/// TRACES: FR-CAT-8
/// **The compatibility guarantee, end to end.**
///
/// An edit made before this build existed has to produce the same image now.
/// Asserted on the generated shader rather than on the parameter values,
/// because that is what the photograph is actually made of: same source, same
/// uniforms, same picture.
#[test]
fn an_edit_written_before_the_channel_curves_renders_as_it_did() {
let from_file = apply(&sidecar_with(&[
("tone_curve.p1_y", 0.15),
("tone_curve.p3_y", 0.85),
]));
let mut by_hand = EditGraph::default_chain();
by_hand.set_param(curve::ID, curve::P1_Y, 0.15);
by_hand.set_param(curve::ID, curve::P3_Y, 0.85);
let restored = from_file.compose();
let expected = by_hand.compose();
assert_eq!(restored.source, expected.source);
assert_eq!(restored.uniforms, expected.uniforms);
assert_eq!(restored.structure_hash, expected.structure_hash);
}
/// **Neutral means absent, and stays absent with four times as much to be
/// neutral about.**
///
/// The curve carries forty parameters now. An image edited with an S-curve and
/// nothing else must generate the shader it generated when it carried ten:
/// three untouched curves are not three identity evaluations, they are nothing
/// at all.
#[test]
fn three_untouched_curves_cost_nothing() {
let mut graph = EditGraph::default_chain();
graph.set_param(curve::ID, curve::P2_Y, 0.62);
let shader = graph.compose();
assert!(
shader.source.contains("---- tone_curve ----"),
"the master curve must reach the shader"
);
assert!(
!shader.source.contains("channel_curve"),
"an untouched channel curve reached the shader:\n{}",
shader.source
);
for prefix in ["r_", "g_", "b_"] {
assert!(
!shader.source.contains(&format!("tone_curve_{prefix}")),
"an untouched channel curve declared uniforms:\n{}",
shader.source
);
}
}
/// And with none of them touched, the operation is not there at all — the
/// property `a_fresh_graph_is_neutral` asserts for the chain, restated for the
/// node that just quadrupled in size.
#[test]
fn a_curve_at_its_defaults_is_absent_from_the_shader() {
let graph = EditGraph::default_chain();
assert!(graph.is_neutral());
assert!(!graph.compose().source.contains("tone_curve"));
// Including when every one of the forty parameters has been explicitly
// written to its own default, which is what a sidecar round trip through
// a build with a different idea of "default" would produce.
let mut written = EditGraph::default_chain();
for cap in written.capabilities() {
if cap.id != curve::ID {
continue;
}
for p in &cap.params {
written.set_param(curve::ID, p.id, p.default);
}
}
assert!(written.is_neutral());
}
/// A grade with no tonal work is a real edit, and it must not drag the
/// luminance path in behind it.
#[test]
fn a_channel_curve_reaches_the_shader_on_its_own() {
let mut graph = EditGraph::default_chain();
graph.set_param(
curve::ID,
curve::coordinate(Channel::Blue, 0, Axis::Y),
0.08,
);
let shader = graph.compose();
assert!(shader.source.contains("c.b = channel_curve(c.b,"));
assert!(
!shader.source.contains("apply_tone_gain"),
"the identity master curve reached the shader:\n{}",
shader.source
);
// Blue's ten points, and nothing else: the other two curves are at the
// identity and contribute no slot to the uniform block.
for prefix in ["r_", "g_"] {
assert!(
!shader.source.contains(&format!("tone_curve_{prefix}")),
"an untouched channel curve declared uniforms:\n{}",
shader.source
);
}
for point in 0..curve::POINTS {
assert!(
shader.source.contains(&format!("tone_curve_b_x{point}")),
"blue's point {point} is missing from the uniform block"
);
}
}
/// Each curve is its own shader, so the pipeline cache cannot hand the red
/// curve's compiled program to an edit that moved the green one.
#[test]
fn every_curve_generates_a_distinct_shader() {
let mut hashes: Vec<u64> = Vec::new();
for channel in Channel::ALL {
let mut graph = EditGraph::default_chain();
graph.set_param(curve::ID, curve::coordinate(channel, 2, Axis::Y), 0.62);
hashes.push(graph.compose().structure_hash);
}
let before = hashes.len();
hashes.sort_unstable();
hashes.dedup();
assert_eq!(before, hashes.len(), "two curves share a compiled shader");
}
/// The forty parameters all persist, and the file names each one once.
#[test]
fn every_point_of_every_curve_round_trips_through_a_sidecar() {
let mut graph = EditGraph::default_chain();
// A different y per coordinate, so a point wired to the wrong channel
// cannot pass by holding the value it was supposed to hold anyway.
// Thirty-secondths because they survive both the decimal the file is
// written in and the binary32 it is read back into exactly — this test is
// about which parameter a value lands in, and a rounding difference here
// would fail it for an unrelated reason.
let mut step = 1;
for channel in Channel::ALL {
for point in 0..curve::POINTS {
graph.set_param(
curve::ID,
curve::coordinate(channel, point, Axis::Y),
step as f32 / 32.0,
);
step += 1;
}
}
let mut sidecar = Sidecar::new();
sidecar.put(dr_pipeline::sidecar::Version::from_graph(
"u1", "Default", &graph,
));
let restored = apply(&sidecar.to_text());
for channel in Channel::ALL {
for point in 0..curve::POINTS {
let id = curve::coordinate(channel, point, Axis::Y);
assert_eq!(
restored.param(curve::ID, id),
graph.param(curve::ID, id),
"{id} did not survive the sidecar"
);
}
}
}