Files
DarkRoom/core/dr-pipeline/tests/declared_parity.rs
T
dtourolleandClaude Opus 5 e17b909d41 Connect the lens vignetting correction to the pipeline
`ops/vignetting.rs` has carried a complete descriptor, polynomial, helper
and test suite without an entry in `ops/`, so it was never in `chain()`.
It reached no photograph and no panel, and `Attribute::Optics` was an empty
category in consequence — filtered out of the tab strip for having no rows,
by a chain that had never been given its only member.

Declaring it needs the one thing the operation was written against and
which did not exist. `wgsl_body` reads `radius`, and the module claimed
"the composer publishes `radius` in the shader prologue for exactly this
reason". It did not. `sample_source` now does, in both sampling branches,
beside the `source_px` it already published for the same class of caller.

It is corner-normalised there, which is the part that is easy to leave out.
`p` spans ±0.5·aspect, so its length at the corner is 0.5·length(aspect) —
about 0.901 on a 3:2 frame, not 1. Lensfun's polynomials are fitted against
a corner radius of 1, so passing `length(p)` straight in evaluates every one
of them short of where it was measured, by a factor that changes with the
aspect ratio. It would have read as a correction that is simply too weak,
which is indistinguishable from a bad profile. Both `lens.rs` and
`framing.rs` asserted the normalisation `p` does not have; corrected.

`order: 5` puts the correction ahead of the tonal stages, and the ordering
is load-bearing rather than tidy. Recovering a corner means dividing by an
attenuation below one — about two stops for a fast prime wide open — so run
after the highlights have been rolled off and clipped, the lift has nowhere
to go and the corners posterise instead of brightening.

`layer_chain` now drops `Optics` as well as the neighbourhood operations.
A local vignetting slider would have worked, which is what makes it worth
excluding: `radius` measures from the centre of the whole photograph and a
mask cannot move the optical axis, so it would lay a frame-centred radial
ramp across the picture and multiply it by the mask. The existing exclusion
covers operations that move and do nothing; this one covers an operation
that moves and does something its name does not promise. The rule both
share is now written down.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-05 14:34:24 +02:00

437 lines
16 KiB
Rust

//! TRACES: FR-PLG-2
//! The generated path and the interpreted path produce the same operation.
//!
//! # Why this test is the point
//!
//! FR-PLG-2 says a bundled operation and a third-party plugin are the same
//! kind of thing, differing only in where the file was found. Two
//! implementations sit behind that claim: `build.rs` compiles `ops/*.yaml`
//! into Rust, and [`DeclaredOp`] interprets the same declaration at run time.
//!
//! **If the two ever disagree, the claim fails quietly.** A plugin would be a
//! second-class kind of node — one whose colours come out fractionally
//! different, or whose fragment lands in the shader with a different comment,
//! or whose uniform arrives in a different slot — and nothing would say so.
//! The photographer would see a look they could not reproduce with a built-in
//! and would have no way to find out why.
//!
//! So this parses every built-in declaration at run time and asserts the
//! composed WGSL is **byte for byte** what the generated implementation
//! produces, with the uniform block bit for bit identical, at a spread of
//! parameter values.
//!
//! # Byte-for-byte, and bit-for-bit, on purpose
//!
//! Not "equivalent", not "within an epsilon". A tolerance is where a real
//! divergence hides: the arithmetic in a declaration is `f32` at both ends and
//! there is no reason for a single bit to differ, so any difference at all is
//! a bug in one of the two backends and should read as one. The one place
//! this bites is number literals, which is why `expr::as_f32` rounds a decimal
//! exactly once — see its own documentation.
//!
//! # What is not covered, and why that is honest
//!
//! A `rust:` node — `tone_curve`, `colour_mixer`, `film_sim`,
//! `capture_sharpen`, `noise_reduction`, `clarity`, `texture` — names a
//! hand-written type and has no declaration to interpret. It is not skipped
//! silently: [`every_declared_node_is_checked`] asserts the two sets partition
//! `ops/` between them, so a node that stops being declared cannot quietly
//! drop out of this file's coverage.
use std::collections::BTreeMap;
use std::path::{Path, PathBuf};
use dr_pipeline::declared::{builtin_helpers, decl, DeclaredOp, Node};
use dr_pipeline::descriptor::ParamKind;
use dr_pipeline::operation::{compose, ComposedShader, Operation};
use dr_pipeline::ops;
use dr_pipeline::ParamId;
/// The declarations this crate ships, read from disk rather than embedded.
///
/// From disk deliberately: `build.rs` reads these very files, so reading the
/// same bytes is what makes the comparison a comparison of the two *readers*
/// rather than of two snapshots that were taken at different times.
fn ops_dir() -> PathBuf {
Path::new(env!("CARGO_MANIFEST_DIR")).join("ops")
}
/// Every `<id>.yaml` in `ops/`, keyed by id, excluding `_helpers.yaml`.
fn declaration_files() -> BTreeMap<String, String> {
let mut out = BTreeMap::new();
for entry in std::fs::read_dir(ops_dir()).expect("ops/ is readable") {
let path = entry.expect("a directory entry").path();
if path.extension().and_then(|e| e.to_str()) != Some("yaml") {
continue;
}
let stem = path
.file_stem()
.and_then(|s| s.to_str())
.expect("a file name")
.to_string();
if stem.starts_with('_') {
continue;
}
out.insert(stem, std::fs::read_to_string(&path).expect("readable"));
}
assert!(!out.is_empty(), "ops/ declares no nodes");
out
}
/// Parse one declaration the way both backends do.
fn read(id: &str, text: &str) -> Node {
let library = builtin_helpers();
decl::read_node(text, &format!("ops/{id}.yaml"), &library.names())
.unwrap_or_else(|e| panic!("ops/{id}.yaml: {e}"))
}
/// The generated implementation of one node, taken out of the default chain.
///
/// Out of `chain()` rather than constructed by name, because that is how the
/// application gets one: if the two ever differed, the chain's version is the
/// one a photograph would be developed with.
fn generated(id: &str) -> Box<dyn Operation> {
let chain = ops::chain();
let index = chain
.iter()
.position(|o| o.descriptor().id.0 == id)
.unwrap_or_else(|| panic!("`{id}` is not in the default chain"));
let mut chain = chain;
chain.remove(index)
}
/// Values worth setting a parameter to, spanning its declared range.
///
/// Both ends, because that is where a rounding difference in a `min:` or a
/// `max:` would show; the default, because that is the neutral every
/// `is_active` is written against; and two interior points that are not
/// round numbers, because a value like 37.5 exercises the arithmetic in a way
/// 0 and 100 do not.
fn probe_values(kind: &ParamKind, default: f32) -> Vec<f32> {
match kind {
ParamKind::Scalar { min, max, .. } => {
let span = max - min;
vec![
default,
*min,
*max,
min + span * 0.375,
min + span * 0.8125,
// A value that is not representable as a short decimal, to
// catch a backend that round-trips a uniform through text.
min + span / 3.0,
]
}
ParamKind::Bool => vec![0.0, 1.0],
ParamKind::Enum { variants } => (0..variants.len()).map(|i| i as f32).collect(),
}
}
/// Put every parameter back where it started.
fn reset(op: &mut dyn Operation) {
let descriptor = op.descriptor();
for p in &descriptor.params {
op.set_param(p.id, p.default);
}
}
/// Assert the two operations are indistinguishable at their current settings.
fn assert_same(id: &str, setting: &str, generated: &dyn Operation, declared: &dyn Operation) {
let g = generated.descriptor();
let d = declared.descriptor();
assert_eq!(*g, *d, "{id} [{setting}]: the descriptors differ");
assert_eq!(
generated.is_active(),
declared.is_active(),
"{id} [{setting}]: the two disagree about whether the operation is doing anything"
);
assert_eq!(
generated.wgsl_body(),
declared.wgsl_body(),
"{id} [{setting}]: the fragment bodies differ"
);
assert_eq!(
generated.helpers(),
declared.helpers(),
"{id} [{setting}]: the helper sets differ"
);
let (gu, du) = (generated.uniforms(), declared.uniforms());
assert_eq!(
gu.len(),
du.len(),
"{id} [{setting}]: different numbers of uniforms"
);
for (a, b) in gu.iter().zip(&du) {
assert_eq!(
a.name, b.name,
"{id} [{setting}]: uniforms in a different order"
);
// Bits, not values: `assert_eq!` on `f32` would call two NaNs unequal
// and would call `0.0` and `-0.0` equal, and both of those are
// differences worth failing on.
assert_eq!(
a.value.to_bits(),
b.value.to_bits(),
"{id} [{setting}]: uniform `{}` is {} generated and {} declared",
a.name,
a.value,
b.value
);
}
}
/// Assert two compositions are the same shader.
fn assert_same_shader(what: &str, a: &ComposedShader, b: &ComposedShader) {
// The source first, and compared as whole strings: a diff in the middle of
// several kilobytes of WGSL is unreadable as an assertion message, so the
// failure below points at the first differing line instead.
if a.source != b.source {
let line = a
.source
.lines()
.zip(b.source.lines())
.position(|(x, y)| x != y);
match line {
Some(n) => panic!(
"{what}: the composed WGSL differs at line {}:\n generated: {:?}\n declared: {:?}",
n + 1,
a.source.lines().nth(n).unwrap_or(""),
b.source.lines().nth(n).unwrap_or(""),
),
None => panic!(
"{what}: the composed WGSL differs in length: {} generated, {} declared",
a.source.len(),
b.source.len()
),
}
}
assert_eq!(
a.uniforms.len(),
b.uniforms.len(),
"{what}: different uniform block sizes"
);
for (i, (x, y)) in a.uniforms.iter().zip(&b.uniforms).enumerate() {
assert_eq!(
x.to_bits(),
y.to_bits(),
"{what}: uniform slot {i} is {x} generated and {y} declared"
);
}
// The hash is taken over the source, so it follows — but it is what the
// pipeline cache keys on, and asserting it says that a declared node and
// its generated twin would share a compiled pipeline rather than quietly
// splitting the cache in two.
assert_eq!(a.structure_hash, b.structure_hash, "{what}: structure hash");
assert_eq!(a.output_mode, b.output_mode, "{what}: output mode");
}
/// Compose a single operation, the way the display path composes a chain.
fn compose_one(op: Box<dyn Operation>) -> (ComposedShader, Box<dyn Operation>) {
let shader = compose(std::slice::from_ref(&op));
(shader, op)
}
/// TRACES: FR-PLG-2
/// Every declared built-in composes to the same shader either way.
#[test]
fn a_declaration_read_at_run_time_composes_byte_for_byte_as_the_generated_one() {
let library = builtin_helpers();
let mut checked = 0;
for (id, text) in declaration_files() {
let Node::Declared(declaration) = read(&id, &text) else {
continue;
};
let mut declared =
DeclaredOp::new(&declaration, library).unwrap_or_else(|e| panic!("ops/{id}.yaml: {e}"));
let mut generated = generated(&id);
// The neutral state first: it is the state every image starts in, and
// an operation that is inactive in one path and active in the other
// would put a whole fragment into one shader and not the other.
assert_same(&id, "neutral", generated.as_ref(), &declared);
let descriptor = declared.descriptor();
for p in &descriptor.params {
for value in probe_values(&p.kind, p.default) {
reset(generated.as_mut());
reset(&mut declared);
generated.set_param(p.id, value);
declared.set_param(p.id, value);
let setting = format!("{} = {value}", p.id);
assert_same(&id, &setting, generated.as_ref(), &declared);
let (gs, back) = compose_one(generated);
generated = back;
let (ds, _) = compose_one(Box::new(declared.clone()));
assert_same_shader(&format!("{id} [{setting}]"), &gs, &ds);
checked += 1;
}
}
// And every parameter moved at once, which is the only case that
// exercises the *order* uniforms are emitted in.
reset(generated.as_mut());
reset(&mut declared);
for p in &descriptor.params {
let value =
probe_values(&p.kind, p.default)[3.min(probe_values(&p.kind, p.default).len() - 1)];
generated.set_param(p.id, value);
declared.set_param(p.id, value);
}
assert_same(&id, "all parameters moved", generated.as_ref(), &declared);
let (gs, _) = compose_one(generated);
let (ds, _) = compose_one(Box::new(declared));
assert_same_shader(&format!("{id} [all parameters moved]"), &gs, &ds);
checked += 1;
}
// A test that silently checked nothing would pass forever. There are eight
// declared nodes and several settings each, so this is a floor rather than
// a count anybody has to maintain.
assert!(
checked > 20,
"only {checked} comparisons ran; the declarations were not found"
);
}
/// TRACES: FR-PLG-2
/// The whole chain composes identically with the declared nodes swapped in.
///
/// The single-operation test above is the sharper one — it isolates each node
/// — but it cannot see an interaction. This composes the *default develop
/// chain*, with every declared node replaced by its interpreted twin and the
/// `rust:` nodes left alone, so it covers uniform slot ordering across
/// operations, helper de-duplication between them, and the order the fragments
/// land in the shader.
#[test]
fn the_whole_chain_composes_identically_with_interpreted_nodes() {
let library = builtin_helpers();
let files = declaration_files();
let mut generated_chain = ops::chain();
let mut declared_chain = ops::chain();
let mut swapped = 0;
for i in 0..declared_chain.len() {
let id = declared_chain[i].descriptor().id.0.to_string();
let text = files
.get(&id)
.unwrap_or_else(|| panic!("`{id}` is in the chain but has no ops/{id}.yaml"));
if let Node::Declared(declaration) = read(&id, text) {
declared_chain[i] = Box::new(
DeclaredOp::new(&declaration, library)
.unwrap_or_else(|e| panic!("ops/{id}.yaml: {e}")),
);
swapped += 1;
}
// Move every operation off neutral, declared or not, so the chain is
// not a list of fragments that were all omitted. A neutral chain
// composes to a shader with no operation blocks in it at all, which
// would make this test pass while asserting nothing.
let descriptor = generated_chain[i].descriptor();
for p in &descriptor.params {
let value = probe_values(&p.kind, p.default)[1];
generated_chain[i].set_param(p.id, value);
declared_chain[i].set_param(p.id, value);
}
}
assert!(
swapped >= 8,
"only {swapped} nodes were swapped for declared ones"
);
assert_same_shader(
"the default chain",
&compose(&generated_chain),
&compose(&declared_chain),
);
}
/// TRACES: FR-PLG-2
/// Nothing in `ops/` escapes this file unnoticed.
///
/// The coverage guard. A declaration that stopped parsing, or a node that
/// quietly became `rust:`, would otherwise reduce what the parity test covers
/// without anything failing — which is exactly the silent divergence the whole
/// file exists to prevent.
#[test]
fn every_declared_node_is_checked() {
let files = declaration_files();
let mut declared = Vec::new();
let mut hand_written = Vec::new();
for (id, text) in &files {
match read(id, text) {
Node::Declared(_) => declared.push(id.clone()),
Node::Rust { ty, .. } => hand_written.push((id.clone(), ty)),
}
}
// Every file is one or the other, and the chain holds exactly them.
assert_eq!(declared.len() + hand_written.len(), files.len());
assert_eq!(
ops::DECLARED_IDS.len(),
files.len(),
"the chain and ops/ hold different numbers of nodes"
);
for id in ops::DECLARED_IDS {
assert!(
files.contains_key(*id),
"`{id}` is in the chain but not in ops/"
);
}
// Named rather than counted, so that a node changing sides is a failure
// somebody reads rather than a number they update.
let hand: Vec<&str> = hand_written.iter().map(|(id, _)| id.as_str()).collect();
assert_eq!(
hand,
[
"capture_sharpen",
"clarity",
"colour_mixer",
"film_sim",
"noise_reduction",
"texture",
"tone_curve",
"vignetting",
],
"the set of hand-written nodes changed; if that is deliberate, update \
this list and the module documentation above"
);
assert!(
declared.len() >= 8,
"only {} declared nodes: {declared:?}",
declared.len()
);
}
/// TRACES: FR-PLG-2
/// A declared operation is addressed by the ids the generated one uses.
///
/// The practical form of "indistinguishable downstream": the sidecar stores
/// parameters by `(op_id, param_id)` text, so a declared node whose interned
/// ids did not compare equal to the generated constants would load an edit
/// that silently did nothing.
#[test]
fn an_interpreted_node_answers_to_the_generated_parameter_ids() {
let mut declared = DeclaredOp::from_yaml(
&std::fs::read_to_string(ops_dir().join("exposure.yaml")).expect("readable"),
"ops/exposure.yaml",
builtin_helpers(),
)
.expect("exposure is a declaration");
declared.set_param(ops::exposure::EXPOSURE, 1.5);
assert_eq!(declared.param(ParamId("exposure")), 1.5);
assert_eq!(declared.descriptor().id, ops::exposure::ID);
}