Declare a develop operation in YAML, and generate the rest

An operation was, in the overwhelming majority of cases, four facts: what
its parameters are, what uniforms they compute, what WGSL those uniforms
drive, and where it sits in the chain. Written in Rust those four facts
arrived wrapped in ninety lines of trait implementation — a match on
parameter id to a struct field, another match back, an is_active comparing
each field to its default, a Vec<Uniform> built by hand. All mechanical,
and each one a place to make a silent mistake: a param() arm returning the
wrong field reads perfectly and breaks the sidecar round-trip.

So the four facts are the file now. core/dr-pipeline/ops/<id>.yaml is a
node, build.rs compiles it into the same Operation impl as before, and the
result lands in OUT_DIR — the same reasoning as style.yaml -> theme.slint,
including why it does not land beside the sources it would look exactly
like. Nothing downstream can tell a declared node from a hand-written one:
same &'static OpDescriptor, same fused-shader composition, same sidecar.

Nine nodes moved: exposure, white_balance, contrast, highlights_shadows,
blacks_whites, brilliance, vibrance, saturation, and the shared WGSL
helper registry. Their prose came with them, and so did their tests —
set/expect/expect_active/expect_wgsl in the declaration compile to real
#[test]s, so a node file carries its own proof rather than leaving it
behind in a file that no longer exists.

Two stayed in Rust and say so with `rust:`. The tone curve's neutral is a
relationship between five interpolated points rather than a set of values;
the colour mixer generates thirty-six faceted parameters from twelve
computed hue bands. A schema stretched to cover either would be a worse
language than Rust aimed at one caller. They still declare their position
here, because the chain's *order* is the one thing a reader comes to this
directory to learn, and an order written half in YAML and half in Rust
would be worse than either alone. default_chain() is generated from it.

Uniforms are derived by a small expression language — exp2(exposure),
blacks / 100 * 0.02 — compiled to Rust rather than interpreted, so an
unknown name or a wrong arity is a build error naming the file and the key
and the arithmetic costs nothing at runtime. The build script refuses a
duplicate order, a filename disagreeing with its id, a default outside its
own range, a test value the graph would clamp before the node saw it, a
helper that does not define the function it names, and a declared node
colliding with a file in src/ops.

Verified by adding a scratch node and removing it again: one file, no
other edit, and it joined the chain at its declared order with its test
running. 237 tests pass in dr-pipeline, clippy and fmt clean.

.yaml joins the traceability tool's scanned suffixes, because a node's
Rust now lives in OUT_DIR where a tag could never be linked from the
report. Coverage 47.7% -> 48.3%.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-16 21:08:16 +02:00
co-authored by Claude Opus 5
parent 65e6a96a65
commit 7c57f490fe
15 changed files with 483 additions and 1455 deletions
+157 -19
View File
@@ -1,35 +1,173 @@
//! The develop operations.
//!
//! Each operation is a self-contained file. Adding one means writing that file
//! and adding it to [`crate::graph::EditGraph::default_chain`] — no central
//! shader to edit, no UI change (FR-DEV-3c).
//! # Adding one
//!
//! Most implement [`crate::operation::Operation`], a function from colour to
//! colour. The optical corrections ([`distortion`]) implement
//! [`crate::lens::Warp`] instead, because they rewrite *coordinates* before
//! the source is sampled rather than transforming a colour after it. Both
//! publish the same [`crate::descriptor::OpDescriptor`], so the UI builds
//! controls for them identically and never learns the difference.
//! Write `ops/<id>.yaml` and rebuild. That is the whole procedure: the node
//! appears in the chain at its declared `order`, the develop panel grows the
//! controls its parameters describe (FR-DEV-3c), the sidecar persists them
//! because they are ordinary parameters, and its declared tests run with
//! everything else.
//!
//! There is no list to extend here, no shader to edit, and no UI change.
//! `build.rs` compiles each declaration into a module implementing
//! [`crate::operation::Operation`], and [`chain`] is generated from the
//! `order:` each node carries.
//!
//! # The two kinds of node
//!
//! **Declared** nodes are the majority: parameters, uniform expressions over
//! those parameters, and a WGSL fragment. Nothing about them is Rust.
//!
//! **Hand-written** nodes are the exceptions, and they are exceptions for a
//! reason rather than for want of migrating. The tone curve interpolates
//! between five points and its neutral is a *relationship* between them; the
//! colour mixer generates thirty-six faceted parameters from twelve computed
//! hue bands; [`vignetting`] carries lens-profile coefficients that are not
//! parameters at all. A schema stretched to cover those would be a worse
//! language than Rust, aimed at one caller each.
//!
//! Both publish the same [`crate::descriptor::OpDescriptor`], so nothing
//! downstream can tell them apart. A hand-written node still declares its
//! place in the chain in `ops/<id>.yaml` with `rust:`, so the directory
//! remains the one place the pipeline's order is written down.
//!
//! # The optical corrections
//!
//! [`distortion`] and [`aberration`] implement [`crate::lens::Warp`] rather
//! than `Operation`, because they rewrite *coordinates* before the source is
//! sampled rather than transforming a colour after it. They are not part of
//! the develop chain and do not appear in `ops/`.
// Hand-written nodes. Each is listed in `ops/` with `rust:`, which is what
// places it in the chain; these are the implementations that entry points at.
pub mod aberration;
pub mod colour;
pub mod colour_mixer;
pub mod contrast;
pub mod curve;
pub mod distortion;
pub mod exposure;
pub mod helpers;
pub mod tone;
pub mod vignetting;
pub mod white_balance;
pub use aberration::Aberration;
pub use colour::{Brilliance, Saturation, Vibrance};
pub use colour_mixer::ColourMixer;
pub use contrast::Contrast;
pub use curve::ToneCurve;
pub use distortion::Distortion;
pub use exposure::Exposure;
pub use tone::{BlacksWhites, HighlightsShadows};
pub use vignetting::Vignetting;
// The declared nodes, plus `helpers` and `chain`. Generated into OUT_DIR by
// `build.rs` from `ops/*.yaml` — see that file for why it does not land here
// beside the sources it looks exactly like.
include!(concat!(env!("OUT_DIR"), "/nodes.rs"));
// Re-exported so a caller writes `ops::Exposure` as it did when these were
// hand-written files, and so the chain reads the same either way.
pub use blacks_whites::BlacksWhites;
pub use brilliance::Brilliance;
pub use contrast::Contrast;
pub use exposure::Exposure;
pub use highlights_shadows::HighlightsShadows;
pub use saturation::Saturation;
pub use vibrance::Vibrance;
pub use white_balance::WhiteBalance;
#[cfg(test)]
mod tests {
use super::*;
use std::collections::BTreeSet;
#[test]
fn the_chain_is_what_the_declarations_say_it_is() {
// The only check available on a `rust:` node: `build.rs` cannot read
// the Rust type's descriptor, so it emits the declared id and this
// asserts the type agrees. A `rust:` entry whose id drifts from its
// implementation would otherwise reorder the pipeline silently.
let built: Vec<&str> = chain().iter().map(|o| o.descriptor().id.0).collect();
assert_eq!(built, DECLARED_IDS);
}
#[test]
fn every_helper_defines_the_function_it_names() {
// A mismatch between the dedup key and the function actually emitted
// would produce either a duplicate definition or a missing one.
// `build.rs` rejects this at the declaration; this asserts the
// generated registry kept the property.
for h in helpers::ALL {
assert!(
h.source.contains(&format!("fn {}(", h.name)),
"helper {} does not define fn {}",
h.name,
h.name
);
}
}
#[test]
fn no_two_helpers_share_a_name() {
// The drift the single-source-of-truth rule exists to prevent: same
// name, different source, and the composer silently picks one.
let mut names = BTreeSet::new();
for h in helpers::ALL {
assert!(
names.insert(h.name),
"two helpers are both called {}",
h.name
);
}
}
#[test]
fn a_node_only_requests_helpers_that_exist() {
// Follows from the build-time check, but asserted end to end: a
// fragment calling a function no helper defines compiles here and
// fails in the shader, which is the expensive place to find it.
let known: BTreeSet<&str> = helpers::ALL.iter().map(|h| h.name).collect();
for op in chain() {
for h in op.helpers() {
assert!(
known.contains(h.name) || h.source.contains(&format!("fn {}(", h.name)),
"{} requests helper {}, which is neither shared nor \
defined by the node",
op.descriptor().id,
h.name
);
}
}
}
#[test]
fn every_node_starts_neutral() {
// An unedited image must be the image. A node whose defaults are not
// its neutral would apply itself to every photograph on open.
for op in chain() {
assert!(
!op.is_active(),
"{} is active at its defaults",
op.descriptor().id
);
}
}
#[test]
fn every_declared_parameter_round_trips() {
// The generated `set_param`/`param` pair is mechanical, which is
// exactly why it is worth checking: a wrong field in one arm reads
// perfectly and silently breaks the sidecar.
for mut op in chain() {
let descriptor = op.descriptor();
for p in descriptor.params {
let crate::descriptor::ParamKind::Scalar { min, max, .. } = p.kind else {
continue;
};
// A value inside the range and away from the default, so a
// stuck field cannot pass by returning the default.
let target = (p.default + (max - p.default) * 0.5).clamp(min, max);
op.set_param(p.id, target);
assert_eq!(
op.param(p.id),
target,
"{}.{} did not round-trip",
descriptor.id,
p.id
);
}
}
}
}