# 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` 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, // 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>, ``` and the generated preamble gains one binding: ```wgsl @group(0) @binding(3) var instances: array>; ``` 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.