Files
DarkRoom/docs/spot-removal.md
T
dtourolleandClaude Opus 5 ef07e6ca3e
Build and test / Desktop (Linux) (push) Failing after 1h14m38s
Build and test / Layer separation (push) Successful in 48s
🐳 Android image / Build and push (push) Successful in 16m30s
Build and test / android-image (push) Successful in 16m31s
Traceability / Requirement traces (push) Successful in 1m47s
Build and test / Android (aarch64) (push) Successful in 1h0m21s
Give the canvas tools a rail of their own, and the column one width
Crop, Local and Repair were chips at the head of the develop column, sharing a
row with the adjustment groups and told apart from them by the shape of their
highlight. Three things followed from that, and only the last is cosmetic: the
column closes, so the way out of a mode went away with the way in — hence the
duplicate "Done Cropping" over the canvas; the chips are generated from the
operation set, so the widest thing in the sidebar was a row nobody had chosen
the contents of; and a mode and a filter are different kinds of state wearing
one control.

They are a fixed 60px rail down the left now, generated from a single table in
toolrail.slint. A tool is one row of it plus a drawing plus a ViewMode variant;
nothing in app.slint is touched to add one. What is left of the strip is the
group filters, so it is GroupStrip.

The column stops measuring itself. Every panel published a content-width and
declared it as min-width, and the column took the largest — which spent the
photograph's pixels on whatever happened to be widest, and moved the image
sideways when switching tools swapped one set of panels for another. It is
panel-width now, one number in style.yaml.

That number is 360 and it is measured, not picked: the contents report a
minimum of 344 in every mode, and they do not compress below it because a Text
that does not elide reports the same minimum as preferred. 320 was tried and
sliced Paste down the middle. The Flickable's viewport is floored at the
layout's minimum rather than its preferred width for the same reason — content
that is never told how much room it has cannot adapt to having less.

Removing the eight content-width declarations repairs three comments an
earlier edit had spliced sentences into. The raw histogram's note on keeping
its hint short is rewritten rather than dropped: an over-long hint no longer
widens the column, it pushes the column's minimum past the width it has and
clips the panel, which makes that constraint sharper rather than obsolete.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-30 09:59:50 +02:00

576 lines
29 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Spot removal
**Status:** Draft · 2026-08-26
**Companion to:** [requirements.md](requirements.md) §3.3 FR-DEV-8 · [architecture.md](architecture.md) §5.2
The last develop feature the requirements ask for that nothing in the tree
implements. FR-DEV-8 states the shape — "non-destructive clone and heal spots
stored as parameters in the edit graph (target, radius, feather, source offset,
opacity, mode), with automatic source placement and manual override, plus a
visualise-spots mode" — and this document is how that lands on the pipeline
that exists now.
---
## 1. Why it is worth the work
Sensor dust is unavoidable with interchangeable lenses, and a dust spot is the
most common reason a photographer leaves a RAW editor for a pixel editor
mid-workflow. Every other develop operation in this application can be the best
one in its class and the workflow still breaks at the first frame with a mark on
the sky.
It is also, unusually, a feature whose cost has already been paid twice over.
The neighbourhood stage exists ([`crate::detail`](../core/dr-pipeline/src/detail.rs)),
the convention for storing geometry in normalised source coordinates exists
([`mask.rs`](../core/dr-pipeline/src/mask.rs)), the canvas-drag pattern exists
([`gradient.rs`](../ui/dr-ui/src/gradient.rs)), and the merge-by-id rule exists
([`sidecar.rs`](../core/dr-pipeline/src/sidecar.rs)). What is genuinely new is
small and is named in §3.
## 2. Non-goals
- **Not layer-based pixel editing.** §1.3 of the requirements excludes that and
this does not reopen it. A spot is a handful of numbers in the edit graph; no
pixels are stored, and the original file is never touched.
- **Not content-aware fill.** The source is a patch from the same photograph,
chosen by an offset. Synthesising texture that is not in the frame is a
different problem with a different budget.
- **Not a general clone brush.** A spot is a disc, not a stroke. A dragged
clone brush is expressible on top of this (a stroke *is* a run of discs) and
is deliberately left until the disc is finished and used.
- **Not automatic dust detection.** Finding spots without being asked is a
reasonable later feature and a bad first one: a false positive silently alters
a photograph, which is the failure this application must not have.
## 3. What is new, precisely
Four things, and it is worth being blunt about them because everything else in
this document is assembly of parts that already work:
1. **A detail pass with variable-length data.** Every [`DetailPass`] today
carries a `Vec<f32>` of uniforms fixed by its own structure. A spot list is
neither fixed nor small. §7.
2. **A neighbourhood operation whose reach is not a small kernel.** Every
existing pass declares a halo of a few pixels. A spot reads from wherever its
source is, which may be a third of the frame away. §5.3.
3. **Undo over something that is not a parameter.** [`History`] snapshots a
[`Preset`], which is a map of scalars — so mask edits are already outside
undo, and spots must not be. §10.4.
4. **A canvas mode that *creates* objects.** Crop edits one rect; local selects
a region; gradient drags an existing shape. Nothing yet makes a new thing
where the pointer went down. §10.
## 4. The model
```rust
/// TRACES: FR-DEV-8
pub struct Spot {
/// Stable across devices; see below.
pub id: String,
/// What is being covered, in normalised **source** coordinates.
pub centre: (f32, f32),
/// What covers it, as an offset from `centre` in **frame units**
/// (y spans 0..1, x spans 0..aspect — the mask convention).
pub offset: (f32, f32),
/// The radius of the disc, in frame units.
pub radius: f32,
/// Fraction of `radius` over which the edge falls away. 0 is hard.
pub feather: f32,
/// How much of the patch is laid down. 1.0 is opaque.
pub opacity: f32,
pub mode: SpotMode, // Heal | Clone
pub enabled: bool,
}
```
**Units follow the mask rule, for the mask reason.** A length stored in pixels
is a length that means something different in the preview and in the export
([`RenderScale`]'s whole documentation is this argument). `centre` is normalised
source, so a crop, a zoom, a pan and a rotation move the spot with the
photograph and no arithmetic is needed to keep it there.
Every *length* — radius, feather, offset — is in the frame's **isotropic
units**, `MaskSource::Radial`'s convention, where y spans `0..1` and x spans
`0..aspect`. Only in those units is a disc a disc: normalised coordinates would
make a spot on a 3:2 frame an ellipse half again wider than it is tall. They are
lengths against the *source* frame rather than the rendered region, so cropping
does not resize a spot already placed — a dust mark is a fact about the sensor,
not about the composition.
One unit for all three, deliberately. A radius in shorter-edge fractions beside
an offset in frame units agrees on a landscape frame and silently disagrees on a
portrait one, which is a bug that stays invisible until somebody rotates a
photograph.
**`offset` is a vector, not a second point.** Dragging the destination moves the
source with it, which is what a photographer expects when they nudge a spot half
a pixel and do not want to re-place the source. Moving the source alone is
editing `offset`.
**The id is derived, not counted.** `MaskStack::next_id` numbers layers, which
is fine for a stack a user names, and wrong here: two devices that each place a
spot offline would both produce `spot3`, and the merge in §11 would treat two
different marks as one. So the id is a short base-36 hash of the centre at
creation, and two devices that place a spot in the same place produce the same
id — which is the correct outcome, because they removed the same piece of dust.
```rust
pub struct SpotSet {
spots: Vec<Spot>, // in creation order; the order matters, see §5.2
}
```
### 4.1 Why it lives beside `ops`, not in it
The [`Operation`] trait takes a `ParamId` and returns an `f32`, and the whole
generic machinery above it — the panel, the sidecar, the presets, the history —
is built on that being true. A spot list is not scalars, and the trait says so
explicitly where it refuses a downcast for film tables.
[`EditGraph`] already holds three things that are not operations for exactly
this reason: `framing`, `masks` and `film`. `spots` is the fourth, and the
argument is the same one `masks` makes — a stack of layers is not a slider, and
folding it into the list would make every consumer that walks `ops` know that
some entries are not really operations.
Bounds, following `mask.rs`'s example of bounding what a sidecar can grow to:
| Constant | Value | Why |
|---|---|---|
| `MAX_SPOTS` | 64 | Beyond a few dozen the answer is to clean the sensor. Refuses rather than dropping, as `MaskStack::push` does. |
| `MAX_SOURCE_DISTANCE` | 0.5 | Frame units. Bounds the halo in §5.3, which is otherwise unbounded. |
| `DEFAULT_RADIUS` | 0.012 | Frame units — about 25 px on a 24 MP frame's short edge, which is a dust mark. |
| `MIN_RADIUS` / `MAX_RADIUS` | 0.001 / 0.5 | Not zero, because a spot that repairs nothing reads as a broken tool; not larger, because the halo bound has to mean something. |
| `DEFAULT_FEATHER` | 0.35 | Fraction of the radius. Soft enough that a heal on a gradient sky has no visible boundary. |
## 5. Where it runs
### 5.1 First in the detail chain
ARCH §5.2 draws spot removal *after* texture and clarity and *before* sharpen
and NR. That diagram is already out of step with the operation set — the
`order:` keys in `core/dr-pipeline/ops/` put noise reduction at 110 and capture
sharpening at 120, ahead of clarity at 130 and texture at 140 — so it needs a
correction anyway, and the correction should put spot removal **first among the
neighbourhood passes**, at a notional order of 105.
The reason is the halo. Sharpening a dust spot before removing it amplifies its
edge, and the amplified edge is wider than the spot: the sharpening kernel has
already smeared a dark ring into pixels that the spot's own disc does not cover,
so the heal leaves a faint circle of over-sharpened background around a patch
that is otherwise perfect. Removing the mark first means every later pass sees a
photograph with no mark in it, which is also the photograph the photographer
thinks they are sharpening.
Since the spot set is not in `ops`, `EditGraph::compose_detail_for` splices its
passes in front of the ops' passes rather than sorting by a declared order. That
is a two-line change and it is stated here so nobody looks for a `spots.yaml`.
### 5.2 Rounds, because sources can read destinations
Every pass reads one texture and writes another. So within a single pass, every
spot reads the *unhealed* image — and a spot whose source overlaps an earlier
spot's destination copies the mark the earlier spot was removing.
The fix is not to run one pass per spot (64 dispatches for a frame that needs
one). It is to group: walking the spots in creation order, a spot joins the
current round unless its source disc intersects the destination disc of a spot
already in that round, in which case it opens a new one. One pass per round, and
the common case — spots scattered over a sky, sources near their own
destinations — is a single round. The grouping is plain CPU code over at most 64
discs and belongs in `SpotSet`, with a test that says an overlapping pair
produces two rounds and a disjoint pair produces one.
### 5.3 The halo, honestly
[`DetailPass::radius`] is "the furthest this pass reads from the pixel it
writes", and it exists so that ARCH §5.3's tile scheduler knows how far to grow
a tile. For a spot pass that is `max(|offset| + radius)` over the pass's spots,
in render pixels — which with `MAX_SOURCE_DISTANCE` at 0.5 can approach half the
frame.
That is a real cost and it should be written down rather than discovered: a
frame with a long-armed spot is close to untileable for that one pass, so the
tiled path will compute it whole-frame. Two things keep it affordable. The pass
is cheap per pixel (§6.4), and it is only the *spot* passes that carry the halo
— the sharpening pass after it still declares its three pixels and still tiles.
The alternative, clamping the source distance to something tile-sized, would
make the tool useless exactly where it is most needed: a mark on a face is
healed from the other cheek, and that is a long way.
## 6. What a spot does to the pixels
Both modes work on the same disc. For a pixel at render coordinate `p` inside a
spot centred at `d` with radius `r`, with the source at `s = d + offset`:
```
w = falloff(|p - d| / r) // 1 at the centre, 0 at the rim
patch = bilinear(source_texture, p - d + s)
c = mix(c, patch + membrane, w * opacity)
```
`falloff` is a smoothstep over the outer `feather` fraction of the radius; a
feather of 0 is a hard disc. `bilinear` is four `tap`s and two lerps, because
the generated preamble offers `textureLoad` only and the offset is fractional in
render space — it becomes a [`Helper`], deduplicated across passes like the
existing luminance helper.
`membrane` is what separates the two modes, and it is zero for `Clone`.
### 6.1 Heal, without a Poisson solve — **implemented**
The classic heal is Poisson blending: copy the *gradients* of the source and
solve for the image whose gradients they are, subject to matching the
destination on the boundary. Solved properly that is an iterative linear system
— tens of Jacobi passes over the disc — and each iteration is a dispatch in this
architecture. Sixty dispatches to remove a dust spot is not a frame budget.
What that solve produces is a smooth membrane interpolating the boundary
difference, and a membrane can be interpolated directly instead of solved. The
shipped form samples the difference between destination and source at `K` points
around the rim and interpolates them into the interior by inverse square
distance:
```
for k in 0..K:
b_k = tap(rim_k) - tap(rim_k + offset) // boundary difference
w_k = 1 / max(|p - rim_k|², 1)
membrane = Σ w_k·b_k / Σ w_k
```
`K = 24` — `RIM_SAMPLES` in `core/dr-pipeline/src/spot.rs`, a uniform rather
than a constant in the source, so tuning it uploads a buffer instead of
recompiling. The cost is `2K` bilinear samples per pixel *inside a disc*, and
nothing at all outside one.
**What was originally specified here was mean-value seamless cloning** (Farbman
et al., 2009), whose weights are half-angle tangents over the rim rather than
inverse squares. The difference matters when the boundary difference varies
sharply around the rim; on the case that actually arises — a repair on a
smoothly varying background — both reduce to the same answer, and the inverse
square form costs two transcendentals per sample fewer. The measurement in
`core/dr-gpu/tests/spot_removal.rs` is what decides whether that trade stays
good: on a linear ramp steep enough to make a clone wrong by 38 levels out of
255, the heal is wrong by **0**. If a case turns up where it is not, the weights
are four lines and the tests are already written.
### 6.2 Why not compute the boundary statistics on the CPU
Because that means reading the rendered image back, and FR-DEV-4 forbids it in
the render path for reasons ARCH §6.1 spends a page on. A per-frame readback to
find out what colour a sky is would reintroduce exactly the stall the whole
architecture exists to avoid. The mean-value form needs no reduction at all,
which is most of why it is the right answer here.
### 6.3 Clone
`membrane = 0`. Kept because heal is wrong on a boundary: a spot straddling a
horizon healed by mean-value blending smears the horizon's contrast into the
disc, and the honest tool then is a straight copy from a matching part of the
frame. This is why FR-DEV-8 asks for both, and it costs one branch in the shader
and one segmented control in the panel.
### 6.4 Cost
Per pixel, per pass: a rejection test per spot in the pass (a squared distance
and a compare), and for the pixels actually inside a disc, `4 + 2K` taps. With
64 spots the rejection cost is the dominant term and it is about 64 × 4 ALU ops
on every pixel of the frame — call it a millisecond at 2 MP on integrated
graphics, which is affordable but not free.
If it proves not to be, the fix is the one `mask.rs` already uses for strokes: a
bounding box per spot and a dispatch sized to it. That needs the detail runner
to dispatch something other than the whole frame, which is a change to its
shape, and it is deliberately not being made until a measurement asks for it.
## 7. Getting a spot list to the GPU
[`DetailPass`] gains one field:
```rust
/// Per-instance data too large or too variable for the uniform block.
pub storage: Option<Vec<[f32; 4]>>,
```
and the generated preamble gains one binding:
```wgsl
@group(0) @binding(3) var<storage, read> instances: array<vec4<f32>>;
```
In `dr-gpu`, both bind group layouts gain a read-only storage entry at binding
3, and a pass that declares no storage binds a shared one-element dummy buffer.
wgpu permits a layout entry the shader does not use, so the two layouts stay two
rather than four, and no existing pass changes at all.
**A spot is two `vec4`s**: `(centre.x, centre.y, radius, feather)` and
`(offset.x, offset.y, opacity, flags)`, all in render pixels except `flags`,
converted on the CPU in `compose_detail_for` where the framing is in scope. This
matters: the shader never sees a normalised coordinate and never has to know
about crop, rotation or zoom — `Framing::output_at` does that map on the way in,
exactly as `gradient.rs` does it for handles. The framing is an affine
similarity, so a disc stays a disc and one radius scales by one factor:
`radius_px = radius × min(source_w, source_h) × scale.ratio()`.
**The source list does not recompile anything.** The WGSL is identical for one
spot and for sixty-four — the count is a uniform and the loop is over the
buffer — so `structure_hash` is unchanged as spots are placed, and placing the
tenth spot re-uploads a 512-byte buffer. This is the same property the fused
pass has for slider movement and it is worth a test that asserts
`cached_pipelines()` does not grow while spots are added.
**Rejected: packing spots into the uniform block.** It would touch no bind group
layout, which is genuinely attractive. It also requires the composer to emit
`vec4` uniform fields (it emits scalars), forces a fixed `MAX_SPOTS`-sized array
and its fixed upload cost into every spot pass, and gives the next operation
that wants a table — a LUT, a curve, a lens grid — nothing to build on. The
storage buffer is a few more lines once and useful again later.
**Invalidation.** The spot set folds into the `detail` key in
`EditGraph::invalidation`, alongside the detail operations. Dragging a spot
therefore re-runs the detail chain and *not* the fused colour pass or the
demosaic, which is exactly the reuse FR-DEV-3d asks for and is the difference
between a spot that follows the finger and one that stutters.
## 8. Automatic source placement
FR-DEV-8 asks for automatic placement with manual override. Two stages, because
the useful half is much cheaper than the good half.
**Stage one — a placed default.** A new spot's source is offset by `2.5 × radius`
in the direction that keeps it furthest inside the frame, biased towards the
frame centre. For dust on a sky, which is the overwhelming majority of spots,
this is right often enough to be worth having, and it is wrong in a way that is
immediately visible and one drag from fixed.
**Stage two — a scored search.** A compute dispatch per new spot scores candidate
offsets on two rings around the destination (say 32 candidates), each scored by
the sum of squared differences over the annulus just outside the destination
disc — the ring is what has to match, since the disc's interior is being
replaced anyway. Penalise candidates whose disc overlaps another spot's
destination, or the frame edge. The winner's offset is read back **once**, when
the spot is created, through `readback.rs` — a few hundred bytes, which is the
size the histogram already moves and completes in well under a frame — and
written into the spot.
Two rules about that readback, both of which are the difference between a
feature and a bug:
- It is **not** in the render loop. `readback.rs` blocks with a deadline, and
that is tolerable exactly once per placement and intolerable per frame. It
happens on the gesture, and the render that follows uses whatever the spot
currently holds.
- The result is **stored**, and the search is never re-run behind the user. A
spot whose source moved on its own when the file was reopened would be an edit
changing itself, and non-destructive editing means the sidecar decides what
the picture is.
If the readback fails, the stage-one default stands. There is no state in which
a spot has no source.
## 9. Visualising spots
FR-DEV-8's "visualise-spots mode" is two different things, and conflating them
is how one of them ends up missing:
**The overlay** — where the spots *are*. Circles for the destination, a fainter
circle for the source, a line between them for the selected spot. Drawn in Slint
over the canvas, alongside the gradient handles and by the same coordinate map,
so it costs the render path nothing and cannot leak into an export.
**The reveal** — where the spots *should be*. Lightroom's "Visualize Spots": a
high-contrast, desaturated view of the frame's high-frequency content, in which
sensor dust on a smooth sky is obvious and in a normal view is nearly invisible.
It is a detail pass appended to the chain:
```
c = abs(c - blur(c)) stretched by a threshold, greyscale, inverted
```
It is a **view**, not an edit. So it is not in the graph and not in the sidecar:
`compose_detail_for` takes a `DetailView` (`Normal` | `RevealSpots`) and the
export path passes `Normal`. A flag on the session would work until the day
somebody exports while the mode is on, and then it would produce a black-and-
white file that looks like corruption. Making the export call site name it is
what stops that from ever being possible.
## 10. Interaction
### 10.1 The mode
A `ViewMode::spots`, and a row in `ToolRail`'s table beside Crop and Local.
That table's own documentation already predicts this shape for the brush; the
spot tool is the same shape and arrives first.
(It landed as a third chip in `ModeStrip`, at the head of the develop column.
The three canvas tools have since moved out to a fixed rail down the left of
the develop view — `ui/dr-ui/ui/toolrail.slint` carries why — and what remains
of that strip is the adjustment-group filters, now `GroupStrip`. Nothing about
the mode itself changed in the move.)
### 10.2 Gestures
| Gesture | Effect |
|---|---|
| Tap / click on the photograph | Place a spot at the current radius, source auto-placed (§8), and select it |
| Drag from a spot's centre | Move the destination; the source follows |
| Drag from the source circle | Change the offset |
| Drag *out* from a fresh placement | Set the source directly, without the auto-placement |
| Scroll / pinch on a selected spot | Radius |
| Tap a spot | Select it; the panel scopes to it |
| `Delete` / `Backspace` | Remove the selected spot |
| Alt-click a spot | Remove it without selecting first |
| `Esc` | Leave the mode |
A drag is a displacement from the press, not a snap to the pointer — the rule
`gradient.rs` states and for the same reason: a finger-sized touch target snapped
to the pointer jumps by half a target the instant it is grabbed.
### 10.3 The panel
While a spot is selected, the adjust column shows radius, feather, opacity and
a Heal/Clone control for *that* spot, exactly as selecting a mask layer
re-scopes the column today. With nothing selected it shows the defaults new
spots will be created with, plus the reveal toggle.
### 10.4 Undo
[`History`] snapshots a [`Preset`], which is a parameter map — so today mask
edits are not undoable, and spot placement must not inherit that. `History`
should hold `(Preset, SpotSet)` and restore both.
That is a narrow change with a wide benefit: the same door lets the mask stack
join later, which closes a gap FR-DEV-5 has open right now. The coalescing rule
needs one addition — a drag of one spot's handle is one step, keyed by the spot
id in the same way a slider drag is keyed by its control — and placement,
deletion and mode changes each open a step of their own.
### 10.5 Touch
Every handle is a `Theme.touch-target`, per FR-UI-3. On a phone the destination
and source circles of a small spot overlap at that size, so the source handle is
drawn at a minimum arm length from the centre while the stored offset is
untouched — the trick `gradient.rs` uses with `MIN_ARM`, for the identical
reason: a handle that cannot be grabbed again is a one-way edit.
## 11. Persistence
One line per spot in the version block:
```
[version 8f04c0e2-…]
exposure.exposure = 0.75
spot.3f9k = 0.4213 0.2871 0.0120 0.35 0.0310 -0.0180 1 heal
```
Fields in order: `centre.x centre.y radius feather offset.x offset.y opacity
mode`. A line rather than a block because a spot is eight numbers and sixty-four
blocks would bury the rest of the file; a line *per spot* rather than one line
for the set because the line is the unit of merge and of a readable diff — the
same reasoning `write_strokes` gives for a line per stroke.
Coordinates are written at the same precision they are held at, as strokes are,
so a round trip is exact and two devices do not generate a diff of noise in the
sixth decimal.
**A malformed line costs that spot and not the file.** A truncated line is
dropped with a warning, exactly as `parse_stroke` drops a bad stroke: a spot
that silently lands somewhere the user never put it is worse than a spot that is
missing, because only one of the two is noticeable.
**Merge** follows `merge_masks` precisely: by id, disjoint survives, a spot both
sides edited resolves wholesale to the higher revision. Half of one device's
offset with the other's radius is a repair neither photographer made. Deletion
propagates through the base comparison exactly as a layer's does.
**Presets do not carry spots** in the first version — a preset is a look, and a
look does not include where the dust was. But dust is in the *same place on
every frame from that body*, which makes "copy spot removal to the selection"
genuinely valuable, and it is a stage of its own (§12, S7) rather than a
surprise inside the existing paste.
## 12. Stages
Each stage is shippable and each has something to look at. Test names are the
files they belong in.
**S1 — The model.** *(Done.)* `spot.rs` in `dr-pipeline`: `Spot`, `SpotMode`, `SpotSet`,
the bounds, id derivation, round grouping (§5.2). No GPU, no UI.
*Tests:* `core/dr-pipeline/tests/spots.rs` — id stability across two identical
placements, `MAX_SPOTS` refuses rather than drops, overlapping sources produce
two rounds, disjoint produce one.
**S2 — Persistence.** *(Done.)* Sidecar write, parse, round trip, merge.
*Acceptance:* a hand-written sidecar with three spots survives a load/save round
trip byte-identically, and two devices that each add a spot offline end with
both.
**S3 — The storage binding.** *(Done.)* `DetailPass::storage`, the preamble's binding 3,
the dummy buffer, the two layouts.
*Acceptance:* every existing detail test still passes untouched, and a synthetic
pass reading the buffer gets what was uploaded.
**S4 — Clone.** *(Done.)* The disc, the feather, the bilinear helper, the pass grouping,
the halo declaration, spliced first into the chain.
*Acceptance:* `core/dr-gpu/tests/spot_removal.rs` — a synthetic frame with a
black disc on a flat grey field is clean to within a tolerance after one clone
spot; the same edit at a one-quarter proxy and at full size land the disc in the
same *normalised* place; adding spots does not grow `cached_pipelines()`.
**S5 — Heal.** The membrane, the mode switch. **Done** — §6.1, and the measurement came out at 0 levels of error against a clone's 38.
*Acceptance:* a dark spot on a linear grey **gradient** — the case clone fails —
is clean to within a tolerance, and the residual at the disc boundary is below
the residual a clone leaves by an order of magnitude. This is the measurement
that decides `K`.
**S6 — The tool.** `ViewMode::spots`, the chip, placement, handles, selection,
the panel scope, deletion, history carrying the spot set, the stage-one source
default. **Done, except the reveal view** — §9's second half is the one piece
of S6 not built, and it is separable: it is a view mode over the detail chain
rather than part of the tool.
*Acceptance:* a dust mark on a real frame is gone in one click, the edit survives
a restart, and undo takes it back.
*What was verified, and how.* Everything below the interface is under test —
the model, the sidecar, the merge, the passes, both blend modes, and undo. The
interface itself was compiled, laid out and photographed: the strip renders
`Crop | Local | Repair` and the column re-scopes. It was **not** driven, because
synthetic clicks do not reach this application (the compositor refuses them),
so the gestures in §10 are as-written rather than as-felt. A first pass with a
real pointer is the outstanding work on this stage.
**S7 — The rest of FR-DEV-8.** The scored source search (§8 stage two), and
copying a spot set across a selection.
S1–S6 is the requirement met in the sense a photographer would recognise; S7 is
the sentence in FR-DEV-8 about automatic placement met in the sense the document
means it.
## 13. Documents to amend
- **requirements.md** — FR-DEV-8 has no *Acceptance:* line; every other
requirement of its weight does. Proposed: *"a dust mark on a smooth sky is
removed in one click with no visible boundary at 1:1, the spot survives a
crop, a rotation and an export at another size, and the exported file matches
the preview."*
- **architecture.md §5.2** — the stage list is out of step with the `order:`
keys in `ops/` and does not show spot removal first among the neighbourhood
passes. §5.1 above is the correction.
- **traceability.md** — regenerated, as ever, rather than edited. FR-DEV-8's row
currently points only at two comments that mention it.
## 14. Open questions
1. ~~**`K = 24`?**~~ Settled by S5's measurement: 24 samples, inverse square weights, zero error on the case the mode exists for.
2. **Does the reveal view belong to spot mode only,** or is it a view mode of
its own that a photographer can turn on while doing something else? It is
cheap to allow both; the risk is a mode nobody remembers turning on. Still
open, and now the only part of §9 unbuilt.
3. **Should a spot be clamped inside the crop?** A spot outside the current crop
costs nothing to render and is invisible, and re-cropping should bring it
back rather than find it deleted. Leaning strongly towards no clamp.
4. **One radius, or an ellipse?** Lightroom's spot tool is circular and its
users cope. An ellipse doubles the handle count for a case a second spot
already covers.