Draw the repairs, before anything sharpens what they removed

A spot set now composes detail passes of its own, one per round, and they
go ahead of every operation's kernel. That placement is the decision worth
recording: a sharpening pass reads a neighbourhood, so sharpening a dust
mark before removing it smears its edge into pixels the repair's disc does
not cover, and what survives is a faint over-sharpened ring around an
otherwise perfect patch. It also disagrees with ARCH §5.2, which draws
spot removal after clarity — docs/spot-removal.md §5.1 is where that is
argued out.

Every length reaching the shader is in render pixels, converted here where
the framing is in scope. Both the centre and the source go through
`Framing::output_at` — the same map the fused pass applies to every pixel
— so a rotated photograph rotates the offset with no trigonometry, and the
radius is found by mapping a point one radius above the centre and
measuring, rather than by multiplying by a ratio this function has no
business knowing about. The tests turn and crop the frame and expect the
mark to stay gone, which is the property that arrangement buys.

compose_full now takes the spot set, because a photograph with a repair
and no sharpening still has a detail stage: a fused pass that encoded its
own output there would quantise twice and bind to a texture of the wrong
format. compose_detail_for takes the source size for the same kind of
reason — a RenderScale describes the region on screen, and a spot is
stored against the photograph.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

core/dr-pipeline/src/detail.rs

@@ -489,10 +489,45 @@ pub fn compose_detail(
     ops: &[Box<dyn Operation>],
     scale: RenderScale,
     output: ColourSpace,
+) -> ComposedDetail {
+    compose_detail_with(ops, &[], scale, output)
+}
+
+/// TRACES: FR-DEV-8
+/// The detail stage with a set of repairs ahead of the operations.
+///
+/// `spots` are already-built passes, from [`crate::SpotSet::passes`], and they
+/// go **first** — before sharpening, before noise reduction, before every
+/// kernel in `ops`.
+///
+/// That placement is a decision rather than an ordering convenience. A
+/// sharpening kernel reads a neighbourhood, so sharpening a dust mark before
+/// removing it smears the mark's edge into pixels the repair's own disc does
+/// not cover: what is left afterwards is a faint over-sharpened ring around an
+/// otherwise perfect patch, which is exactly the artefact that reads as broken
+/// software. Removing the mark first means every later pass sees the
+/// photograph the photographer thinks they are sharpening.
+///
+/// It also means ARCH §5.2's stage list, which draws spot removal after
+/// texture and clarity, is not what this does — see `docs/spot-removal.md`
+/// §5.1, which is where the disagreement is written down.
+pub fn compose_detail_with(
+    ops: &[Box<dyn Operation>],
+    spots: &[DetailPass],
+    scale: RenderScale,
+    output: ColourSpace,
 ) -> ComposedDetail {
     // Every pass of every active detail operation, flattened, carrying the
     // operation it came from for the uniform prefix and the helper set.
     let mut planned: Vec<(&'static str, &'static [Helper], DetailPass, usize)> = Vec::new();
+    for (index, pass) in spots.iter().enumerate() {
+        planned.push((
+            crate::spot::SPOT_ID,
+            crate::spot::SPOT_HELPERS,
+            pass.clone(),
+            index,
+        ));
+    }
     for op in ops {
         if !op.is_active() {
             continue;
@@ -829,6 +864,7 @@ mod tests {
             &crate::Framing::new(),
             dr_types::ColourSpace::Srgb,
             &crate::mask::MaskStack::new(),
+            &crate::spot::SpotSet::new(),
         )
     }
 

core/dr-pipeline/src/graph.rs

@@ -443,7 +443,7 @@ impl EditGraph {
     /// graph renders to the screen and to a file in the same breath, and the
     /// two want different answers.
     pub fn compose_for(&self, output: dr_types::ColourSpace) -> ComposedShader {
-        compose_full(&self.ops, &self.framing, output, &self.masks)
+        compose_full(&self.ops, &self.framing, output, &self.masks, &self.spots)
     }
 
     /// TRACES: FR-DSP-1
@@ -484,9 +484,10 @@ impl EditGraph {
     /// single encoded dispatch it always has.
     pub fn compose_detail(
         &self,
-        scale: crate::detail::RenderScale,
+        source: (u32, u32),
+        render: (u32, u32),
     ) -> crate::detail::ComposedDetail {
-        self.compose_detail_for(scale, dr_types::ColourSpace::Srgb)
+        self.compose_detail_for(source, render, dr_types::ColourSpace::Srgb)
     }
 
     /// TRACES: FR-EXP-2
@@ -497,12 +498,21 @@ impl EditGraph {
     /// transform — the fused pass stops at linear working values. Composing
     /// the two halves for different spaces would encode the edit twice, or
     /// not at all.
+    /// `source` is the demosaiced image's size and `render` the size being
+    /// drawn. The scale is worked out here rather than handed in, because the
+    /// repairs need the *source* size as well — a spot is stored in normalised
+    /// source coordinates and has to be put through the framing to find out
+    /// where it lands on this render, and a [`crate::detail::RenderScale`]
+    /// describes the region on screen rather than the photograph.
     pub fn compose_detail_for(
         &self,
-        scale: crate::detail::RenderScale,
+        source: (u32, u32),
+        render: (u32, u32),
         output: dr_types::ColourSpace,
     ) -> crate::detail::ComposedDetail {
-        crate::detail::compose_detail(&self.ops, scale, output)
+        let scale = self.render_scale(source, render);
+        let spots = self.spots.passes(&self.framing, source, scale);
+        crate::detail::compose_detail_with(&self.ops, &spots, scale, output)
     }
 
     /// TRACES: FR-DEV-3d

core/dr-pipeline/src/lib.rs

@@ -152,8 +152,7 @@ mod tests {
         // source pixels, so on a proxy it may honestly decline to draw at all
         // (`RenderScale::resolves`) — which would put it in neither half and
         // make the assertion fail for a reason that is not a defect.
-        let scale = g.render_scale((4000, 3000), (4000, 3000));
-        let detail = g.compose_detail(scale);
+        let detail = g.compose_detail((4000, 3000), (4000, 3000));
         let mut fused_blocks = 0;
         for desc in g.descriptors() {
             let id = desc.id.0;

core/dr-pipeline/src/operation.rs

@@ -467,7 +467,13 @@ pub fn compose_with_framing(
     framing: &Framing,
     output: ColourSpace,
 ) -> ComposedShader {
-    compose_full(ops, framing, output, &MaskStack::new())
+    compose_full(
+        ops,
+        framing,
+        output,
+        &MaskStack::new(),
+        &crate::spot::SpotSet::new(),
+    )
 }
 
 /// TRACES: FR-DEV-3
@@ -487,6 +493,7 @@ pub fn compose_full(
     framing: &Framing,
     output: ColourSpace,
     masks: &MaskStack,
+    spots: &crate::spot::SpotSet,
 ) -> ComposedShader {
     // Active *point* operations. A neighbourhood operation is filtered out
     // here rather than asked for a fragment it cannot write: it reads pixels
@@ -506,11 +513,18 @@ pub fn compose_full(
     // 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
-    };
+    //
+    // TRACES: FR-DEV-8
+    // The repairs count too, and they are the reason this takes a spot set at
+    // all: a photograph with a spot on it and no sharpening still has a detail
+    // stage, and a fused pass that encoded its own output there would quantise
+    // twice and be bound to a texture of the wrong format.
+    let output_mode =
+        if ops.iter().any(|o| o.is_active() && o.detail().is_some()) || !spots.is_neutral() {
+            OutputMode::LinearWorking
+        } else {
+            OutputMode::Encoded
+        };
 
     // Whether an operation has taken over the rendering. Decided from the
     // operations for the same reason `output_mode` is: a caller that got it

core/dr-pipeline/src/ops/capture_sharpen.rs

@@ -541,7 +541,10 @@ mod tests {
     }
 
     fn chain_at(graph: &EditGraph, scale: RenderScale) -> crate::detail::ComposedDetail {
-        graph.compose_detail_for(scale, ColourSpace::Srgb)
+        // The scale is what these tests vary, so it is rebuilt into the two
+        // sizes it stands for rather than handed over: a render of the full
+        // frame at `render_size`, from a source of `full_size`.
+        graph.compose_detail_for(scale.full_size(), scale.render_size(), ColourSpace::Srgb)
     }
 
     #[test]

core/dr-pipeline/src/spot.rs

@@ -44,7 +44,7 @@
 //! nobody downstream should have to repeat: where a spot's source is
 //! ([`Spot::source`]), and which spots may share a pass ([`SpotSet::rounds`]).
 
-use crate::operation::{canonical_bits, hash_bytes, mix, FNV_OFFSET};
+use crate::operation::{canonical_bits, hash_bytes, mix, Helper, FNV_OFFSET};
 
 /// The most spots one edit holds.
 ///
@@ -490,3 +490,206 @@ impl SpotSet {
         mix(h, self.spots.len() as u64)
     }
 }
+
+/// The id the generated passes are labelled and prefixed with.
+///
+/// Not an operation id — no `ops/*.yaml` declares it and nothing in the chain
+/// answers to it — for the same reason [`crate::detail`]'s resolve pass has one
+/// of its own: a label reading `spot/round0` sends a reader to this module
+/// rather than to whichever operation happened to lend its name.
+pub const SPOT_ID: &str = "spot";
+
+/// Bilinear sampling, which the generated preamble does not offer.
+///
+/// `tap` takes an integer offset from the pixel being written, and a repair
+/// reads from wherever its source is — a fractional position in render space,
+/// because the offset was stored as a fraction of the frame and multiplied up.
+/// Sampling it nearest-neighbour would make a repair jitter by a pixel as the
+/// view is zoomed, which on a face is the difference between a repair and a
+/// smudge.
+pub const SPOT_HELPERS: &[Helper] = &[Helper {
+    name: "spot_tap",
+    source: "\
+// A bilinear sample at an arbitrary position, clamped to the edge.
+//
+// Clamped rather than zero-filled, exactly as `tap` is: a source dragged partly
+// off the frame must read the pixels that exist rather than fade into black,
+// which would draw a dark crescent inside the repair.
+fn spot_tap(p: vec2<f32>) -> vec3<f32> {
+    let last = vec2<i32>(textureDimensions(source)) - vec2<i32>(1);
+    // Pixel centres sit at half-integers, so the texel below and left of a
+    // position is `floor(p - 0.5)`. Getting this wrong shifts every repair by
+    // half a pixel — invisible in a test that checks a mean, obvious on a face.
+    let q = p - vec2<f32>(0.5);
+    let base = floor(q);
+    let f = q - base;
+    let i0 = clamp(vec2<i32>(base), vec2<i32>(0), last);
+    let i1 = clamp(i0 + vec2<i32>(1), vec2<i32>(0), last);
+    let s00 = textureLoad(source, vec2<i32>(i0.x, i0.y), 0).rgb;
+    let s10 = textureLoad(source, vec2<i32>(i1.x, i0.y), 0).rgb;
+    let s01 = textureLoad(source, vec2<i32>(i0.x, i1.y), 0).rgb;
+    let s11 = textureLoad(source, vec2<i32>(i1.x, i1.y), 0).rgb;
+    return mix(mix(s00, s10, f.x), mix(s01, s11, f.x), f.y);
+}",
+}];
+
+/// The WGSL every spot pass runs. See [`SpotSet::passes`] for the record layout
+/// it reads, which is where the meaning of each lane is written down.
+const SPOT_BODY: &str = "\
+// Each repair is two records: the disc it covers, and where it reads from.
+let repairs = instance_count / 2u;
+// The centre of this pixel. Half-integer, because a disc of radius 1.5 centred
+// on a pixel should cover that pixel whole rather than half of it.
+let here = vec2<f32>(f32(coord.x) + 0.5, f32(coord.y) + 0.5);
+
+for (var i = 0u; i < repairs; i = i + 1u) {
+    let disc = instances[i * 2u];
+    let src = instances[i * 2u + 1u];
+
+    // The rejection test. It is what every pixel outside every repair pays,
+    // and a repair covers a few thousand pixels of a few million.
+    let delta = here - disc.xy;
+    let dist = length(delta);
+    if (dist >= disc.z) {
+        continue;
+    }
+
+    // One inside the solid core, falling to zero at the rim. `disc.w` is where
+    // the fall begins, worked out on the CPU so the shader never divides by a
+    // feather that might be zero.
+    let cover = (1.0 - smoothstep(disc.w, disc.z, dist)) * src.z;
+    if (cover <= 0.0) {
+        continue;
+    }
+
+    // The same displacement within the disc, read from beside it: the patch is
+    // a translation of the photograph, so its texture arrives unrotated and
+    // unscaled.
+    // `replacement`, not `patch`: WGSL reserves that word, and a reserved
+    // keyword in generated code is a compile error a long way from its cause.
+    let replacement = spot_tap(src.xy + delta);
+    c = mix(c, replacement, cover);
+}";
+
+impl SpotSet {
+    /// TRACES: FR-DEV-8 | FR-DSP-1
+    /// The passes that draw these repairs at this size.
+    ///
+    /// Shaped like [`crate::detail::DetailStage::passes`] and called in the
+    /// same place for the same reason, but deliberately not an implementation
+    /// of it: that trait belongs to operations, and a spot set is not one.
+    ///
+    /// # Everything the shader sees is in render pixels
+    ///
+    /// The conversion happens here, where the [`crate::Framing`] is in scope,
+    /// and never in WGSL. That is what keeps the shader ignorant of crops,
+    /// zooms, rotations and flips: a repair's centre and its source both go
+    /// through [`crate::Framing::output_at`] — the same map the fused pass
+    /// applies to every pixel — so a rotated photograph rotates the offset with
+    /// no trigonometry here at all, and a repair panned off screen lands
+    /// outside the target and draws nothing.
+    ///
+    /// The radius goes through the same map rather than being multiplied by a
+    /// ratio: a point one radius above the centre is mapped too, and the
+    /// distance between the two answers *is* the radius in render pixels.
+    /// Anything cheaper would need this function to know that the framing is a
+    /// similarity, which is not its business to know.
+    ///
+    /// # The record layout
+    ///
+    /// Two `vec4`s per repair, because a repair does not fit in one:
+    ///
+    /// | | x | y | z | w |
+    /// |---|---|---|---|---|
+    /// | 0 | centre x | centre y | radius | where the edge starts falling |
+    /// | 1 | source x | source y | opacity | 1 for heal, 0 for clone |
+    pub fn passes(
+        &self,
+        framing: &crate::Framing,
+        source: (u32, u32),
+        scale: crate::detail::RenderScale,
+    ) -> Vec<crate::detail::DetailPass> {
+        use crate::detail::DetailPass;
+
+        let (sw, sh) = (source.0.max(1), source.1.max(1));
+        let aspect = sw as f32 / sh as f32;
+        let (rw, rh) = scale.render_size();
+        let (rw, rh) = (rw as f32, rh as f32);
+        let to_px = |uv: (f32, f32)| (uv.0 * rw, uv.1 * rh);
+
+        let active: Vec<&Spot> = self.active().collect();
+        let mut passes = Vec::new();
+
+        for (round, group) in self.rounds(aspect).into_iter().enumerate() {
+            let mut storage: Vec<[f32; 4]> = Vec::with_capacity(group.len() * 2);
+            let mut reach: f32 = 0.0;
+
+            for index in group {
+                let spot = active[index];
+                let centre = to_px(framing.output_at(spot.centre, sw, sh));
+                let from = to_px(framing.output_at(spot.source(aspect), sw, sh));
+
+                // One radius along y in frame units is one radius along y in
+                // normalised coordinates, which is why the probe point is built
+                // this way rather than from the offset.
+                let rim = (spot.centre.0, spot.centre.1 + spot.radius);
+                let rim_px = to_px(framing.output_at(rim, sw, sh));
+                let radius = (rim_px.0 - centre.0).hypot(rim_px.1 - centre.1);
+
+                // Where the edge begins to fall away. Always at least half a
+                // pixel inside the rim: a disc with a genuinely hard edge
+                // aliases into a visible polygon, and half a pixel of ramp is
+                // finer than any feather control can ask for anyway.
+                let inner = (radius * (1.0 - spot.feather)).min(radius - 0.5).max(0.0);
+
+                storage.push([centre.0, centre.1, radius, inner]);
+                storage.push([
+                    from.0,
+                    from.1,
+                    spot.opacity,
+                    match spot.mode {
+                        SpotMode::Heal => 1.0,
+                        SpotMode::Clone => 0.0,
+                    },
+                ]);
+
+                // How far this pass reads from a pixel it writes: across to the
+                // source, plus the disc it reads there. Stated honestly even
+                // though it is large — an understated radius shows as a seam at
+                // every tile boundary, which reads as a driver bug (ARCH §5.3).
+                let across = (from.0 - centre.0).hypot(from.1 - centre.1);
+                reach = reach.max(across + radius);
+            }
+
+            if storage.is_empty() {
+                continue;
+            }
+
+            passes.push(DetailPass {
+                label: round_label(round),
+                radius: reach.ceil() as u32,
+                wgsl: SPOT_BODY.to_string(),
+                uniforms: Vec::new(),
+                storage,
+            });
+        }
+
+        passes
+    }
+}
+
+/// A static label for round `n`.
+///
+/// Static because a pass label is a `&'static str`, and rounds past the few
+/// named here are rare enough — each one needs a source deliberately placed
+/// over an earlier repair — that sharing a label between them costs nothing but
+/// a slightly vaguer line in a profiler.
+fn round_label(round: usize) -> &'static str {
+    match round {
+        0 => "round0",
+        1 => "round1",
+        2 => "round2",
+        3 => "round3",
+        _ => "round",
+    }
+}