DarkRoom/docs/dev/spot-removal.md

Spot removal

Status: Draft · 2026-08-26 Companion to: requirements.md §3.3 FR-DEV-8 · 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), the convention for storing geometry in normalised source coordinates exists (mask.rs), the canvas-drag pattern exists (gradient.rs), and the merge-by-id rule exists (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

/// 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.

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 taps 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:

/// 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:

@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 vec4s: (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.