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

Merge branch 'worktree-agent-a75dc051d9bf691de' into integration

Browse Source 
# Conflicts:
#	docs/traceability.md
This commit is contained in:
dtourolle
2026-08-22 15:34:29 +02:00
parent 610f679881 7407a82aa7
commit 7f60a2547c
13 changed files with 3060 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/dr-pipeline/src/operation.rs
+323 -16
View File
@@ -28,19 +28,182 @@ use crate::descriptor::{OpDescriptor, ParamId, Presentation};
use crate::framing::{Framing, FRAMING_UNIFORM_FIELDS};
use crate::mask::MaskStack;
/// TRACES: FR-DEV-3d
/// What an operation's parameters affect, for cache invalidation scoping.
///
/// Adjusting exposure must not invalidate the demosaic result; this is what
/// lets the tile cache reuse everything up to the first changed stage
/// (ARCH §5.3).
///
/// **The ordering is the pipeline order**, which is why this derives `Ord`
/// rather than merely `Eq`: geometry decides which source pixel a colour comes
/// from, the fused colour pass transforms it, and the detail stage reads the
/// neighbourhood the colour pass produced. A change at one stage invalidates
/// that stage and every later one, and nothing earlier — see [`Invalidation`],
/// which is where that rule is actually written down and tested.
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub enum Affects {
/// Per-pixel colour only. Everything in this milestone.
Colour,
/// Pixel positions — crop, rotate. Invalidates geometry-dependent caches.
/// Pixel positions — crop, rotate, straighten. The framing prologue, which
/// also decides the resolution everything downstream runs at.
Geometry,
/// Per-pixel colour. Every operation fused into the single adjust
/// dispatch, and every mask layer's chain.
Colour,
/// TRACES: FR-DEV-3d
/// A pixel's *neighbourhood* — sharpening, noise reduction, clarity,
/// texture, dehaze, spot removal.
///
/// The seam `docs/requirements.md` §3.3 designed and nothing cut until
/// [`crate::detail`] existed. It is a separate variant rather than a flavour
/// of `Colour` because it is a separate *dispatch*: a fragment in the fused
/// pass is handed a colour and has no way back to a coordinate, so a
/// kernel cannot be expressed there at any price.
///
/// What the distinction buys, concretely: the fused pass's result is held
/// in a linear intermediate, so dragging a sharpening slider re-runs the
/// detail dispatches and **not** the colour pass — which is exactly the
/// reuse FR-DEV-3d asks for, and it is asserted in `dr-gpu`'s
/// `detail_stage` tests rather than merely hoped for.
Detail,
}
/// TRACES: FR-DEV-3d
/// One cache key per pipeline stage, derived from the edit.
///
/// # The rule
///
/// A cached result for stage *S* stays valid while *S*'s own key and the keys
/// of every stage **before** it are unchanged. [`Self::of`] is the first half;
/// [`Self::through`] folds in the second and is what a cache should actually
/// store.
///
/// That reads as pedantry until it is applied, at which point it settles the
/// two questions FR-DEV-3d asks:
///
/// - **Changing a detail parameter must not re-run demosaic**, or the framing,
/// or the fused colour pass. It does not: `of(Detail)` moves and
/// `through(Colour)` does not, so the linear intermediate the colour pass
/// wrote is still good and only the detail dispatches run again.
///
/// - **Changing exposure must not re-run anything upstream of colour.** It
/// does not: `through(Geometry)` is untouched, so a tile cache keyed on it
/// survives, and the demosaiced texture — which no key here mentions at all
/// — is never in question.
///
/// It also settles what is *not* true, and the temptation is real: changing
/// exposure **does** re-run the detail passes, because the detail stage reads
/// what the colour pass wrote and that changed. There is no arrangement of
/// keys that avoids it while keeping sharpening after the tone curve, and
/// sharpening after the tone curve is the correct place (see
/// [`crate::detail`]). Anyone who wants exposure to leave the detail stage
/// alone is asking for detail to run *before* tone, which is a different
/// pipeline and a worse picture.
///
/// # Why the demosaic is not in here
///
/// Because no parameter in this graph can change it. The demosaiced texture is
/// a function of the file and the decode settings, both of which live outside
/// the edit graph; a caller keying a cache on it mixes in whatever names the
/// photograph — a `VersionId` — and these keys ride on top.
///
/// # Integer state only
///
/// Every value folded in here is a parameter: a slider position or a number
/// from a sidecar, never a float that came back from the GPU. That is what
/// ARCH §6.13 requires of a cache key, and it is why hashing the raw bit
/// patterns is sound rather than reckless. Negative zero is canonicalised on
/// the way in, because `-0.0 == 0.0` while their bit patterns differ, and a
/// slider that arrived at zero from below would otherwise invalidate a cache
/// that is perfectly valid.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct Invalidation {
geometry: u64,
colour: u64,
detail: u64,
}
impl Invalidation {
/// Build from the three per-stage hashes. [`crate::EditGraph::invalidation`]
/// is what computes them; this is public so a caller with its own notion
/// of a stage can construct one.
pub fn new(geometry: u64, colour: u64, detail: u64) -> Self {
Self {
geometry,
colour,
detail,
}
}
/// The key for `stage`'s own parameters, ignoring everything upstream.
///
/// Useful for asserting that a change was correctly *scoped* — that moving
/// a detail slider left the colour stage's parameters alone. Not a cache
/// key: a stage whose own parameters are unchanged still has to re-run if
/// its input changed, which is what [`Self::through`] is for.
pub fn of(&self, stage: Affects) -> u64 {
match stage {
Affects::Geometry => self.geometry,
Affects::Colour => self.colour,
Affects::Detail => self.detail,
}
}
/// The key for the **output** of `stage` — this stage and everything
/// upstream of it. What a cached texture should be keyed on.
pub fn through(&self, stage: Affects) -> u64 {
let mut h = FNV_OFFSET;
h = mix(h, self.geometry);
if stage >= Affects::Colour {
h = mix(h, self.colour);
}
if stage >= Affects::Detail {
h = mix(h, self.detail);
}
h
}
}
/// Fold one operation's identity and settings into a running hash.
///
/// Shared by the stage keys so that two stages cannot come to disagree about
/// what "this operation's state" means — which would show as a cache that is
/// occasionally, unreproducibly stale.
pub(crate) fn hash_op(h: u64, op: &dyn Operation) -> u64 {
let desc = op.descriptor();
let mut h = hash_bytes(h, desc.id.0.as_bytes());
for p in desc.params {
h = hash_bytes(h, p.id.0.as_bytes());
h = mix(h, u64::from(canonical_bits(op.param(p.id))));
}
h
}
/// A parameter's bits, with negative zero folded onto zero.
///
/// `-0.0 == 0.0` as far as every operation is concerned — a slider that
/// reached zero from below produces the same shader and the same picture — but
/// the two have different bit patterns. Hashing them apart would invalidate a
/// cache for a change that is not one.
pub(crate) fn canonical_bits(v: f32) -> u32 {
if v == 0.0 {
0
} else {
v.to_bits()
}
}
pub(crate) fn hash_bytes(mut h: u64, bytes: &[u8]) -> u64 {
for byte in bytes {
h ^= u64::from(*byte);
h = h.wrapping_mul(0x100_0000_01b3);
}
h
}
/// FNV-1a's offset basis. No dependency, and stable across runs and platforms,
/// which a cache key requires.
pub(crate) const FNV_OFFSET: u64 = 0xcbf2_9ce4_8422_2325;
/// A single scalar a fragment reads from the generated uniform block.
///
/// Operations declare uniforms by name and value; the composer assigns them
@@ -85,6 +248,10 @@ pub trait Operation: Send + Sync {
///
/// The fragment runs inside its own block, so locals need no unique
/// names.
///
/// Never called on an operation that declares a [`Self::detail`] stage —
/// a neighbourhood operation is a dispatch of its own and contributes
/// nothing to the fused shader, so it returns an empty string.
fn wgsl_body(&self) -> String;
/// Uniform values this operation's fragment reads.
@@ -95,6 +262,26 @@ pub trait Operation: Send + Sync {
Affects::Colour
}
/// TRACES: FR-DEV-3 | FR-DEV-8
/// This operation's neighbourhood stage, if it has one.
///
/// `None` — the default, and true of every operation that is a function of
/// one colour — means the operation is fused into the single adjust
/// dispatch in the ordinary way.
///
/// `Some` means the opposite: the operation reads pixels it is not
/// writing, cannot be a fragment in a fused shader, and runs as its own
/// dispatch or dispatches after the colour pass. See [`crate::detail`] for
/// where that sits and why, and for what a sharpening operation has to
/// write. An operation returning `Some` must also return
/// [`Affects::Detail`] from [`Self::affects`], which
/// `detail_operations_agree_with_themselves` checks — the two saying
/// different things would leave the operation in neither stage, silently
/// doing nothing.
fn detail(&self) -> Option<&dyn crate::detail::DetailStage> {
None
}
/// Any WGSL helper functions the fragment calls.
///
/// Emitted once per *distinct* function name even if several operations
@@ -128,6 +315,32 @@ pub struct Helper {
pub source: &'static str,
}
/// TRACES: FR-DEV-2 | FR-DEV-3d
/// What the fused pass writes, and therefore what has to be bound to it.
///
/// The fused shader ends one of two ways, and the difference is not cosmetic —
/// it decides the storage texture's format, so a shader composed for one and
/// dispatched against the other is a validation failure rather than a wrong
/// picture.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum OutputMode {
/// `rgba8unorm`, display-encoded in the composed output space. What the
/// pass has always written, and still writes for the overwhelmingly common
/// edit that has no detail stage: one dispatch, one read, one write.
Encoded,
/// `rgba16float`, linear sRGB, **unclipped**, scene-referred.
///
/// Emitted when the edit has an active neighbourhood operation. The detail
/// passes read this, and the last of them performs the output transform,
/// so the pipeline still quantises exactly once (FR-DEV-2) — it simply
/// happens two dispatches later.
///
/// Unclipped matters: a recovered highlight is above 1.0 here, and
/// clamping before a sharpener sees it would draw a hard edge at precisely
/// the luminance a sharpener is most visible at.
LinearWorking,
}
/// The result of composing a set of operations into one shader.
#[derive(Debug, Clone, PartialEq)]
pub struct ComposedShader {
@@ -139,6 +352,8 @@ pub struct ComposedShader {
/// not their values. Two edits differing only in slider positions share
/// a compiled pipeline and differ only in the uniform upload.
pub structure_hash: u64,
/// What this shader writes. See [`OutputMode`].
pub output_mode: OutputMode,
}
/// Fields the generated uniform struct always carries, before op uniforms.
@@ -211,12 +426,33 @@ pub fn compose_full(
output: ColourSpace,
masks: &MaskStack,
) -> ComposedShader {
// Active *point* operations. A neighbourhood operation is filtered out
// here rather than asked for a fragment it cannot write: it reads pixels
// it is not writing, so it belongs to the detail stage that runs after
// this one (see `crate::detail`). Filtering on the declared stage rather
// than on `affects()` means the shader and the stage agree by
// construction — there is one place an operation says which it is.
let active: Vec<&dyn Operation> = ops
.iter()
.map(|o| o.as_ref())
.filter(|o| o.is_active())
.filter(|o| o.is_active() && o.detail().is_none())
.collect();
// Whether a detail stage follows. If one does, this pass stops short of
// the output transform and hands on a linear intermediate; the last detail
// pass finishes the job. Decided from the operations themselves rather
// than from a flag the caller sets, because a caller that got the flag
// wrong would produce a shader whose storage format does not match the
// texture bound to it.
let output_mode = if ops
.iter()
.any(|o| o.is_active() && o.detail().is_some())
{
OutputMode::LinearWorking
} else {
OutputMode::Encoded
};
let mut uniform_fields = String::new();
let mut uniform_values: Vec<f32> = Vec::new();
let mut body = String::new();
@@ -328,8 +564,40 @@ pub fn compose_full(
""
};
let to_output = primaries_conversion(output);
let encode_output = encode_output_fn(output);
// The tail, and it is the whole of the difference between the two output
// modes. Everything above — the prologue, the fragments, the mask layers,
// the camera matrix — is emitted identically either way, so an operation
// cannot tell whether a detail stage follows it and does not have to.
let (store_format, to_output, encode_output, store) = match output_mode {
OutputMode::Encoded => (
"rgba8unorm",
primaries_conversion(output),
encode_output_fn(output),
" // Clip to the output gamut and encode. The clip is last for the reason the
// matrix above is: a colour outside sRGB is still inside a wider space, and
// clipping before the conversion would throw it away for no one's benefit.
c = clamp(c, vec3<f32>(0.0), vec3<f32>(1.0));
textureStore(output, vec2<i32>(gid.xy), vec4<f32>(encode_output(c), 1.0));"
.to_string(),
),
OutputMode::LinearWorking => (
"rgba16float",
String::new(),
String::new(),
" // Stop here: a detail stage follows, and it needs linear values it
// can average. No primaries conversion, no clip and no encode — the
// last detail pass performs all three, so the pipeline still quantises
// exactly once (FR-DEV-2).
//
// Deliberately *not* clamped. A recovered highlight is above 1.0 at this
// point and an out-of-gamut colour can be below 0.0; clipping them here
// would put a hard edge into the very neighbourhood the next pass is
// about to convolve, which is how sharpeners come to draw dark rings
// around specular highlights.
textureStore(output, vec2<i32>(gid.xy), vec4<f32>(c, 1.0));"
.to_string(),
),
};
let source = format!(
"// GENERATED — do not edit.
@@ -344,7 +612,7 @@ struct Params {{
@group(0) @binding(0) var source: texture_2d<f32>;
@group(0) @binding(1) var<uniform> u: Params;
@group(0) @binding(2) var output: texture_storage_2d<rgba8unorm, write>;
@group(0) @binding(2) var output: texture_storage_2d<{store_format}, write>;
// The local adjustment masks, one array layer each, rasterised by a separate
// pass (ARCH §5.4). Declared unconditionally even when no layer is active, so
// that every generated shader shares one bind group layout — a layout that
@@ -430,11 +698,7 @@ fn main(@builtin(global_invocation_id) gid: vec3<u32>) {{
dot(u.cam_to_srgb_2.rgb, c),
);
{to_output}
// Clip to the output gamut and encode. The clip is last for the reason the
// matrix above is: a colour outside sRGB is still inside a wider space, and
// clipping before the conversion would throw it away for no one's benefit.
c = clamp(c, vec3<f32>(0.0), vec3<f32>(1.0));
textureStore(output, vec2<i32>(gid.xy), vec4<f32>(encode_output(c), 1.0));
{store}
}}
",
active.len()
@@ -473,6 +737,7 @@ fn main(@builtin(global_invocation_id) gid: vec3<u32>) {{
source,
uniforms: uniform_values,
structure_hash,
output_mode,
}
}
@@ -489,7 +754,7 @@ fn main(@builtin(global_invocation_id) gid: vec3<u32>) {{
/// value it already had. The identity is detected rather than special-cased by
/// name, so a space that happens to share sRGB's primaries would be spared
/// too.
fn primaries_conversion(output: ColourSpace) -> String {
pub(crate) fn primaries_conversion(output: ColourSpace) -> String {
let m = output.from_linear_srgb();
const IDENTITY: [f32; 9] = [1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0];
// A tolerance rather than equality: the matrix is an inverse multiplied by
@@ -528,7 +793,7 @@ fn primaries_conversion(output: ColourSpace) -> String {
///
/// Named `encode_output` whatever the space, so the call site at the end of
/// `main` does not have to know which one it got.
fn encode_output_fn(output: ColourSpace) -> String {
pub(crate) fn encode_output_fn(output: ColourSpace) -> String {
let body = match output.transfer() {
Transfer::Srgb => " let lo = c * 12.92;
let hi = 1.055 * pow(max(c, vec3<f32>(0.0031308)), vec3<f32>(1.0 / 2.4)) - 0.055;
@@ -626,7 +891,7 @@ const BILINEAR_HELPER: &str = "fn sample_bilinear(uv: vec2<f32>, dims: vec2<u32>
";
/// Fold a value into a hash. FNV-1a's mixing step, over eight bytes.
fn mix(mut h: u64, value: u64) -> u64 {
pub(crate) fn mix(mut h: u64, value: u64) -> u64 {
for byte in value.to_le_bytes() {
h ^= u64::from(byte);
h = h.wrapping_mul(0x100_0000_01b3);
@@ -644,7 +909,7 @@ fn mix(mut h: u64, value: u64) -> u64 {
/// Still integer state hashed on the CPU, as ARCH §6.13 requires of a cache
/// key: the text is generated from parameters that are neutral or not, never
/// from a rendered float.
fn hash_source(source: &str) -> u64 {
pub(crate) fn hash_source(source: &str) -> u64 {
// FNV-1a: no dependency, stable across runs and platforms, which the
// shader cache key requires.
let mut h: u64 = 0xcbf2_9ce4_8422_2325;
@@ -1051,4 +1316,46 @@ mod tests {
let shader = compose(&[fake(&DESC_A, 1.0, false)]);
assert!(shader.source.starts_with("// GENERATED"));
}
#[test]
fn detail_operations_agree_with_themselves() {
// An operation says which stage it belongs to in two places — through
// `affects()` and through `detail()` — and the two must say the same
// thing. Disagreement is the worst possible failure mode here, because
// it is silent: an operation claiming `Affects::Detail` while
// returning `None` from `detail()` is fused as a point op and asked
// for a fragment it does not have, and one returning `Some` while
// claiming `Affects::Colour` is filtered out of the fused pass and put
// in the wrong invalidation bucket. Either way the slider moves and
// nothing happens.
//
// Checked over the real chain, plus the test consumer, so that a
// sharpening operation added later is covered by this without anyone
// remembering to extend it.
let mut ops = crate::ops::chain();
ops.push(Box::new(crate::detail::probe::BoxBlur::new()));
for op in &ops {
let id = op.descriptor().id;
assert_eq!(
op.detail().is_some(),
op.affects() == Affects::Detail,
"{id} disagrees with itself about whether it is a \
neighbourhood operation"
);
}
}
#[test]
fn a_detail_operation_never_contributes_a_fused_uniform() {
// Slot order in the generated block is emission order, and nothing
// addresses a slot by number — so an operation that contributed a
// uniform without contributing the fragment that reads it would shift
// every later operation's uniforms out from under its shader. The
// filter in `compose_full` prevents it; this is the assertion that the
// filter is on the right side of the loop.
let mut ops = crate::ops::chain();
ops.push(Box::new(crate::detail::probe::BoxBlur::with_radius(0.05)));
let before = compose(&crate::ops::chain()).uniforms.len();
assert_eq!(compose(&ops).uniforms.len(), before);
}
}