2eb06b106495226aea62d286d37831af9fa24bcc
139
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 dtourolle
|
a7b090cf36 |
Ship presets with the application instead of seeding them
The six starter presets were copied into the photographer's own library on a first run and were theirs from then on. That cannot grow into a real collection: a copy is frozen at the release that wrote it, so an improved preset reaches nobody who had the old one, and re-seeding would overwrite a preset someone had tuned. `dr_pipeline::bundled` now holds the shipped presets as `.drpl` files compiled into the binary, in sections — Essentials (the former six) and three sections of film presets, one per measured stock in dr-film, printed on the paper its profile names — and never writes them to the user's file. Every shipped preset is a look (`Reach::Named`), so applying one keeps the corrections a photograph already has. A name links a photographer's copy to a shipped preset. Saving over a shipped name makes their version the one that name applies; it is listed in the shipped section, marked as changed, and deleting it reverts to the shipped one. Renaming it makes it one of their own and the shipped preset reappears. Keyed on the name because that is what the photographer sees and chooses by. Copies an older first run seeded are forgotten on load where they are still exactly as seeded — otherwise all six would list as changed and stay frozen at their old values. A tuned one is kept and now overrides. The sheet lists "Yours" first, then each shipped section, with headings. Shipped rows apply and nothing else; a changed row offers Revert where the photographer's own offer Delete. A dr-ui test checks every shipped film names a stock this build can bake, on that stock's own paper, because dr-pipeline does not link the profile database. The film presets name stocks by id; the measurements behind them are spektrafilm's (CC BY-SA 4.0), attributed in each file as in dr-film. |
||
|
dtourolle
|
90c0695c05 |
Let a preset name its film, and let a look reach only what it names
A preset could not choose a film stock. The stock is a choice of material rather than a parameter, so `Preset` — a map of `op.param = value` — had nowhere to hold it, and "Portra 400, printed" could not be saved, copied or shipped as a look. Worse, the film node's own sliders *were* parameters: a paste moved one stock's exposure and push onto whatever stock the target was on, and left the target's tables baked from the values it had just replaced. A preset now carries a `FilmRef` beside its parameters. It travels under whichever scope carries the film node, so the stock and its sliders are never split, and by the replacement rule every other parameter follows: applied at that scope, a preset without a film develops the target without one. `Preset::apply` returns the `FilmRebake` it owes, as `EditGraph::set_state` already did, because this crate cannot bake a stock; the develop session pays it before recording the step, and the batch paste writes the stock into each sidecar through `film_for`. The library file spells it `film =` / `film_print =`, as a sidecar does, and an older build keeps those lines as ones it does not understand. `EditState` keeps the film in its own field only: the parameters it captures leave it out, so one edit has one place to say which stock it is on. Second, a preset now has a reach. Replacement is right for a copy of a whole edit — "make these match" — and wrong for a look: a stock-only "Portra 400" applied that way would put the photograph's exposure, white balance and noise reduction back to default. `Reach::Named` replaces only the operations a preset names (whole operations, so a look that sets the blacks resets the whites beside them) and the film only if it names one. Saved edits and the clipboard keep `Reach::Whole`; the line `reach = named` is written only for the other, so existing libraries write the same bytes. |
||
|
dtourolle
|
d430ec9528 |
Drop a detail pass that changes nothing when it sits between two others
Capture sharpening at a scale too coarse to draw its radius emits one pass with an empty body (`nothing_to_sharpen`), so that a chain still ends in something that performs the output transform. At fit on any modern sensor that is most of the time. When another neighbourhood operation follows it - dehaze, clarity, texture - the pass is not last and does nothing: it reads the rgba16float intermediate and writes the same texels to the other one. It still cost a full render-sized read and write every frame: scene before after sharpen+clarity 2560x1600 fit 18.66 ms 14.16 ms sharpen+clarity 3840x2160 fit 37.93 ms 27.88 ms every operation 2560x1600 fit 71.80 ms 67.24 ms clarity alone 2560x1600 fit 14.23 ms 14.20 ms (control) sharpen+clarity 2560x1600 1:1 23.01 ms 22.97 ms (control: resolves) (Laptop RTX 3050 held at 420/810 MHz by its power cap, synthetic 60 MP source, median of five alternated runs of forty frames each.) `compose_detail_with` now drops such a pass where dropping it is exact: not the last pass, whose output transform would otherwise move onto the previous pass's f32 result and round differently; and not a pass right after a reduced one, because a full-resolution pass is what closes the reduced chain for the operation after it. `DetailPass::is_identity` says what "changes nothing" means: full size, nothing bound at binding 3, and a body with no code in it. The rgba8 output is bit-identical: every scene above hashed the same before and after, and the sharpen+clarity frame hashes the same as clarity on its own, which is the claim in one line. A new dr-pipeline test pins the three cases - dropped ahead of another operation, kept when last, kept when alone. |
||
|
dtourolle
|
1dc7b45cfe |
Read the fit view's source gather once per framing, not once per frame
At fit, every output pixel of the fused pass loads one texel from a source three or four times its width, on a stride. The memory system fetches the texels it skips along with the one it wanted, so on a 60 MP rgba16float source that gather was most of what the fused pass cost: 10.6 ms of a 2560x1600 frame against 3.8 ms for the same shader reading a contiguous window (the 1:1 view). At 3840x2160 it was 21.1 ms. Those are the laptop RTX 3050 with its clocks held at 420/810 MHz by the power cap; unthrottled the same frames were about 2.0 and 3.2 ms, and the gather is the same share of them. Which texel an output pixel reads depends only on the framing prologue, the framing and warp uniforms, the source and the render size. None of those move during a slider drag, so the gather is the same work every frame. The fused shader now takes a render-sized rgba16float cache of it (bindings 6 and 7, declared in every generated shader like the masks) and a pair of uniform flags: write what was gathered, or read it back at the pixel's own coordinate. AdjustPass keeps the cache and decides per dispatch. The composer supplies `ComposedShader::sample_key`, a hash of the prologue and those uniforms, and AdjustPass adds the image and the size; an image gets a process-unique id for this rather than being held alive by the key. The picture is bit-for-bit the same. The source is rgba16float and so is the cache, so the stored texel is the texel, and only the path that reads a texel whole takes part: an interpolated sample (straightening, lens warps, CA) is a blend that f16 could not hold exactly, so the composer gives it no key and it reads directly as before. The cache is written on the second frame with a given key, not the first: a crop or zoom drag changes the key every frame, and writing then would add a render-sized write to exactly the gestures that can afford it least. It is kept only up to 3840x2400, so an export never parks a full-frame copy on the device, and `release_caches` drops it. Measured with a scratch probe rendering the synthetic 60 MP frame from examples/frame_budget.rs, forty frames per run after six warm-up, five runs of each binary alternated, median of the per-run p50 (GPU idle apart from the power cap): scene before after neutral 2560x1600 fit 10.62 ms 3.88 ms exposure 2560x1600 fit 10.83 ms 3.87 ms nr chroma 2560x1600 fit 19.84 ms 12.69 ms neutral 3840x2160 fit 21.05 ms 7.11 ms exposure 3840x2160 fit 21.08 ms 6.94 ms clarity 3840x2160 fit 42.20 ms 27.88 ms neutral 2560x1600 1:1 3.83 ms 3.84 ms (control: nothing to gain) The rgba8 output of every scene hashed identically before and after, in isolated runs and across all 38 scene/size/view combinations of the probe. New tests walk a pass through direct, write and read frames, a slider move, a neighbourhood operation and a framing change, and compare every frame with a fresh pass that can only have read directly. |
||
|
dtourolle
|
5a500118ae |
Correct converging verticals with a keystone in framing
There was no perspective transform anywhere in the pipeline: framing offered a ±45° straighten, quarter turns and flips, and a building shot looking up kept its leaning walls. Framing gains a vertical and a horizontal keystone (-100..100). They are parameters of framing rather than a new stage, so they carry its Compose attribute, persist in the sidecar under framing, and are withheld from a default paste exactly as the crop is. In the prologue the keystone runs after the crop and the straightening and before the stored orientation and the lens warp, so "vertical" is the photograph's displayed height and the lens still sees its whole frame. The map takes the output frame onto a trapezoid inside the source, built as a homography from four corners and uploaded as three columns in the framing uniform block (which grows from two vec4s to five). A keystone on its own therefore never exposes an empty corner and leaves any crop valid. Combined with a straightening angle the empty area is a pulled-back quadrilateral the closed-form inscribed rectangle cannot describe, so max_inscribed_crop searches for the largest centred rectangle whose corners all have a source pixel behind them. source_at and output_at apply the same map, so masks, gradients and spot handles follow it. |
||
|
dtourolle
|
d748527a4c |
Keep colour labels in the sidecar so they survive and travel
A rating and a flag are written to DarkRoom's sidecar as well as the catalog, because the catalog is a disposable index and the sidecar is how a judgement reaches the photographer's other devices. A label had no place there, so once labels could be set, one would have lived only in the catalog of the device it was set on and gone with it. The sidecar version now carries `label` (0 none, 1-5 as the catalog codes it), written only when set. It merges under the rating's rule, so a device that never labelled a frame cannot clear another device's label, and a code this build does not know reads as none rather than as some other colour. A judgement write carries the catalog's label with the stars, and the scan takes a sidecar's label into the catalog when it has one. An older build keeps the line as an unknown key and writes it back. |
||
|
dtourolle
|
fc0ea8824d |
Format the crop-orphan measurement
rustfmt wraps two tuples in dr_pipeline::orphan that the previous commit left on one line past the width limit. No change in behaviour. |
||
|
dtourolle
|
9772785f81 |
Measure which mask layers a crop takes out of the frame
Mask geometry is stored in source coordinates, so re-cropping tighter never destroys a layer. It makes it invisible: the layer stays in the panel and the sidecar, its adjustment lands on pixels nobody will see, and nothing says so. The spec had no clause for this; FR-DEV-17 now states it, under the ID issue #10 reserved. dr_pipeline::orphan samples each layer's mask on a 64x64 lattice over the source, with the gradient, radial, brush and model-raster geometry the mask shader uses, folds the parts by their joins and inversions, and maps the samples through the framing to see how much of the coverage the crop keeps. `hidden_by_crop` reports the layers whose share fell below a tenth, and only those the change newly hid, so an already stranded layer is not announced again on every later adjustment. Ranges follow the picture and region selections need a label map this crate does not hold, so a layer that adds either is never reported: a false alarm on the common path would teach the notice to be dismissed unread. |
||
|
dtourolle
|
8cdad3863d |
Keep only where two selections agree, as a third way to join a mask part
A layer's parts could be added to the mask or taken out of it, and nothing else. The selections that need composing most are the ones that are neither: the sky that is also bright, the subject that is also skin. With union and subtract alone, "this and that" had to be spelled as "this minus everything that is not that", which needs a second part that selects the complement and rarely exists. Join gains Intersect, stored as "intersect" in the part block of a sidecar. It is the product of the two coverages, dst * src, which is one more fixed-function blend state beside union's max and subtract's dst * (1 - src) (mask-editing.md 5.2): the same scratch texture, the same three vertices, no shader arithmetic. The product equals the minimum wherever either side is fully in or out, and is the softer reading where two soft edges overlap. Join::apply spells the three operations on the CPU so the GPU tests can be held to one definition. A layer that intersects with a part covering nothing now reports that it covers nothing, so it is not rasterised as an empty slice. Old sidecars never contain the word, so they read as before; a build from before this reads "intersect" as a union, the existing unknown-join fallback, which keeps the part visible rather than dropping it. Join::ALL keeps union and subtract at indices 0 and 1 so a stored panel index still means the same join. |
||
|
dtourolle
|
84fade99ec |
Put the developer docs under docs/dev and index the folder for users first
docs/ had 26 developer documents flat beside the manual, and the two audiences are very differently sized: most readers want the manual and the gesture reference, a few want the register, the designs and the measurements. The manual and gestures.md stay at the top; everything for someone changing the code moves to docs/dev/, and the two documents that name their own successors — the v0.1 milestone and the UI-refinement plan — go to docs/dev/archive/ rather than being deleted, since both are still cited. docs/README.md is the index, users first. Every reference follows: code comments, Cargo manifests, the workflows, the pre-commit hook, the bench and traceability tools (which locate the repo root by docs/dev/requirements.md now), packaging, the Docker READMEs, CLAUDE.md, CONTRIBUTING.md and the README. The matrix links one level deeper and is regenerated. Links out of the moved documents into the tree gain a level; a link checker over every Markdown file finds none broken. |
||
|
dtourolle
|
12f8990e09 |
Average a patch under the white balance picker, not one photosite
The probe's comment said a 192px render "averages a small neighbourhood into each of its pixels". It does not: the composed shader fetches the source at one position per output pixel - nearest for an unrotated frame, four photosites blended otherwise - so the probe was a point sample of a noisy sensor, and two painted-white air conditioners on the same wall answered +37 and -50. The tap is now narrowed to the patch of the canvas around the click, a couple of percent of its width and square on screen, and rendered at 64x64 with interpolation forced on, which puts a sample on every sensor pixel under it at any ordinary zoom. The samples are averaged, with the void and clipped ones left out rather than allowed to pull the mean, and fewer than half surviving is refused. compose_camera_probe takes the patch; the merge's compose_camera_linear keeps its nearest sampling. The readback shrinks from six megabytes to sixty-four kilobytes. A frame of alternating warm and cool columns, averaging neutral, moves the controls by at most two units; a point sample swung them to sixty. |
||
|
dtourolle
|
4576499c3b |
Refuse a clipped highlight as a neutral
Sampling the overcast sky on a Canon 6D frame set tint to -100 and temperature to -15 for a patch the canvas showed as pure white. A clipped photosite is sensor white, not a colour: every channel stopped counting, so what the tap hands back is the as-shot multipliers themselves, which are strongly magenta, and the solver dutifully drove green to its stop. The display shader already fades such a pixel to a neutral of the same brightness before any operation runs, so the picker was balancing against something the photographer could not see. The probe now refuses a sample with any channel at or above the onset the shader fades from, the way the solver already refuses black. The threshold is one constant, CLIP_ONSET, formatted into the shader and read by the probe, so the two cannot drift apart. |
||
|
dtourolle
|
2f47087223 |
Measure the white balance probe in camera RGB, where the gains multiply
Pressing "pick" and clicking a near-neutral wall on a Canon 6D frame set tint to -77 and turned the whole photograph green. The white balance operation runs first in the chain, on camera RGB, before the body's base curve and colour matrix; the probe was read off a display render after all three, and the solve treated that sRGB triple as if the gains multiplied it directly. On a JPEG the two spaces coincide, which is why the existing tests passed while the picker was broken on every raw file. The probe now reads the camera-space tap a merge stitches from, composed under the edit's own framing so a fraction of the canvas is a fraction of the probe, and puts the as-shot balance on itself - exactly the value the operation's gains are about to multiply. No operations run in the tap, so nothing has to be stripped and restored, and the display target is left alone, so a sample that found nothing usable no longer needs a redraw. A raw-frame test with the 6D's matrix and a typical as-shot balance samples a warm grey and asserts the rendered pixel comes back neutral; it fails on the previous probe. |
||
|
dtourolle
|
2fd7690b6f |
Mark a pixel the lens correction pushed off the sensor with alpha 0 in the camera-space tap
The fused shader stored black with alpha 1 for a pixel whose source coordinate left the frame, and the merge's warp averaged it in like any other: a dark, badly interpolated fringe along every frame's edge, visible as a seam wherever a frame ended and, later, as the edge the border fill continued. The display keeps its opaque black; CameraLinear stores alpha 0 and the warp weights each sample by the alpha it interpolated, dropping a sample that has none. |
||
|
dtourolle
|
42d11d919b |
cargo fmt and clippy across the panorama work, and one lint master carried
The dr-face comparison is master's: a negated partial-order test on the eye box's width, rewritten as the two conditions it meant. |
||
|
dtourolle
|
75d2ceb23c |
Provenance in the sidecar, a launch hook for the page, and where it stands
derived_from and merge are top-level sidecar fields (FR-MRG-6): one line per source in order, and how the composite was made. A build that predates them keeps the lines as unknown and writes them back. The job writes the sidecar beside the composite and stages it with its own record when the composite goes through the outbox. DARKROOM_START_MERGE=a.CR2,b.CR2 lands on the merge page at startup with the job running on local files, on the model of DARKROOM_START_IDENTITY, for looking at the page where synthetic clicks do not reach it. The fetch and the start are shared with the grid's button. panorama.md §11 records what exists, the fixture's figures, and the six things still open, auto-crop first. |
||
|
dtourolle
|
9b6b4942cf |
The camera-space tap: OutputMode::CameraLinear, composed with no operations
compose_camera_linear composes the fused pass with an empty operation list, the file's orientation as the baseline, a view rect for the tile, and a store of rgba32float. On the GPU, render_camera_linear is the only entry that accepts it: it fills the profile uniforms neutral — unit white balance, identity matrix, curve off — so what lands in the texture is the sensor's numbers after the lens warp and nothing else (FR-MRG-2). A third bind-group layout carries the format, as the linear one does, and the readback is generalised to any pixel width for the f32 copy. Thirty-two bits because the composite is written back at the sensor's scale: a 14-bit sensor has 16 384 steps to white and f16 keeps 2 048 of them in the top octave. |
||
|
dtourolle
|
ed4460cb9c |
Tag three requirements the code already meets
R5 says in its own note that zoom_resolution.rs establishes it as a pixel equality; that file was tagged FR-DSP-5 alone. FR-DEV-19's three sub-clauses carry eighty-three tags between them while the parent had none; MaskLayer, which is the thing they edit, now carries it. And NFR-R3 — a crash in decode does not take down the application, the image is marked failed — is exactly what the decoder's panic guard and the face sweep's unreadable mark do, tagged FR-RAW-4 and NFR-SEC-1 and not the clause that asked for them. |
||
|
dtourolle
|
696bafa9d5 |
Undefer AI subject masking, which shipped, and give it a clause
§7 still listed "AI subject masking — deferred per D11" while MaskSource::Subject and MaskSource::Category, backed by dr-segment's instance and semantic models, had been the primary way a local adjustment is made for weeks. The code was tagged FR-DEV-3, which names gradients and brushes and says nothing about a model. FR-DEV-3i now states what exists: a subject or a category found by a local model, stored as identity with the run's signature so that it merges per field and reads as stale rather than wrong, then treated as any other layer by the edge, stroke, composition and reveal clauses. The one place it departs from FR-DEV-19 — coverage written run-length coded beside the layer, so a stored subject renders without a model — is recorded in the clause instead of left for the next audit to find. The segmentation crate and the UI's selection module are tagged to it. |
||
|
dtourolle
|
d3b6127db6 |
Let a photographer name the state they liked, and go back to it or look at it
FR-DEV-5 asked for named snapshots of an edit state and FR-DEV-7 for a comparison against a chosen one, and neither existed. The history stack is per sitting and forgotten with it, on purpose — the gap that mattered was an automatically saved mis-drag with no way back, and that was closed first. What was left was the other half: a state the photographer wants to keep *because* it is worth keeping, which is a different thing from a step and is not served by making the steps last longer. A snapshot is an edit state, and an edit state is exactly what a sidecar version stores, so it is stored as one: a `[version]` block carrying `snapshot-of = <uuid>`. The parameters, the masks and their parts, the repairs and the film all arrive through the blocks that already carry them, a merge keys on the uuid as it does for any version, and a build that predates the key reads the block as a named version and keeps it — the right failure. Only the pointer is new. The one reader that has to know is `default_version`, which must never answer with a snapshot: a file whose edit is missing is not a file whose edit is one of its saved moments. The snapshots of an edit are listed by that pointer, oldest first, the same on every device. Writing them back removes what this sitting deleted and puts in what it holds, and leaves standing whatever it never saw — a snapshot the other device took since the photograph was opened here is not this device's to remove by not knowing about it. That is the rule the version merge already keeps, applied one level down, and it is why the save carries the deleted ids rather than replacing the list wholesale as the masks are. Each is re-pointed at the uuid the save settled on, because the default may have been fused onto its canonical identity since the snapshot was taken. Restoring is one history step, so undo takes it back whole, as a paste is. Taking and deleting are not steps: they change nothing about the photograph, and an undo that removed a snapshot would be undoing a decision to remember. Holding the eye beside one renders the snapshot and hands the edit straight back — the same suspension "Before" uses, against a point the photographer chose rather than the file. Two sessions on the same photograph get ids that cannot collide, stamped with the second and a random word, because the merge folds equal ids into one. |
||
|
dtourolle
|
4574c35236 |
Let a part be left out of a mask without being taken out of it
A layer built from parts was missing the one control a correction most often wants: seeing what it did. The question a subtracted gradient raises is whether it took only the sky, and the question a stroke raises is whether it filled the shoulder — and the only way to ask either was to remove the part and look, which answered the question and lost the part. The layer's own ring answers a different question, about the adjustment, and hiding eight layers to check one correction is not an A/B anybody performs. So a part carries `hidden`. It is an edit and a history step, as the layer's switch is, and it is folded into the render fingerprint because hiding a part changes the mask as surely as removing it does. Where the mask is built the shown parts are walked rather than the parts, which is what makes a hidden base hand the fold to the first part that is shown — and a revealed layer whose every part is hidden clears its slice rather than leaving whatever the last rasterisation put there to be read back. `covers` asks the same shown parts, so a layer whose only adding part is hidden costs no slice at all. In the sidecar the key is `hidden`, in the part's block or, for the base, in the mask block — under a word that cannot be confused with the layer's `enabled`, which has always meant the layer. Absent means shown, so no file written before the switch existed reads any differently. The row wears the same ring the layer does, one row down, because it is the same question about a smaller thing. |
||
|
dtourolle
|
a87139b838 |
Give every mask an eye and a colour, and put the brush where the mask is
The first build of seeing a mask showed the selected layer's, in one global style, from a strip at the top of the panel. It answered the wrong question and answered it somewhere nobody looked. What a photographer asks of two masks is how they meet — where the sky's edge sits against the building's — and that needs both on screen at once, in colours that can be told apart. So each row of the stack has an eye, drawn in the colour its mask is shown in, and each mask has six swatches to choose that colour from. Several can be open at once; a new one comes up open, in the first colour nothing else is using. The style — tint, alpha, outline — is the one setting that stays global, above the stack, because three styles at once are three pictures that cannot be read against each other. Alpha now draws every shown mask, each in its colour, on black. In the pipeline a `Reveal` is a list of `(layer, colour)` rather than one layer, and every reveal block carries its own colour. The brush moves too. Select, Paint and Erase and the three sliders under them sat at the top of the panel, appeared only once a row was selected, and said nothing about which mask they acted on — so "how do I paint" and "how do I correct the model's outline" both had the same answer and nobody found it. They sit under the selected mask's parts now, beside the swatches, and on a subject or a category the hint says what a stroke there does: it becomes a part of this mask, joined to the model's, and can be taken out again. Eyes and colours are viewing state, on the session and not on the layer, so a photograph reopened has every eye closed — the stored-mask round-trip test asserts it. |
||
|
dtourolle
|
c045702a47 |
Show the photographer the mask they are shaping
Nobody can refine an edge they are not being shown. The only thing drawn on the canvas was the region overlay — a false-coloured picture of what the model *detected* — which knows nothing of a layer's feather, its falloff, its morphology, its invert or its opacity, and nothing at all about a gradient, a range or a stroke. Every control added for mask editing therefore acted on something invisible, which is why the whole feature reads as absent rather than as unfinished. A layer's finished mask now draws over the photograph in one of three styles: a tint for whether the right thing is selected, an alpha for where the edge is, an outline for whether that edge is registered against the detail the other two hide. The hard part is not the shader. A selection with no adjustment on it changes no pixel, so it is not active, so it holds no slice of the mask array and is never rasterised — and that is exactly the layer somebody wants to look at, for the whole of the time between choosing a subject and deciding what to do to it. So `MaskStack::rendered` is `active()` plus the layer being looked at, and the rasteriser, the composer and the distance-field builder all index by position in it. Which is also why the design's "two uniforms, no recompile" is not available: a uniform can select a slot, it cannot conjure one. The reveal is never on the graph. It reaches the pipeline as an argument to `compose_revealing`, and `compose_for` — which the exporter, the thumbnail and the neutral probe all call — has no way to ask for one. A flag on the graph would have been shorter, would have type-checked, and would have been one forgotten reset away from a red tint baked into an exported file. And the tools that shape a mask now arm. `Masking.tool` is an `in` property only Rust may write, and the handler wrote nothing back, so the strip reported "Select" however many times Paint was pressed and the paint area was never enabled — the brush, the parts and the whole of FR-DEV-19b reachable from no control in the application. The region overlay stands down while a mask is being shown, and its button now says what it hides: two overlays that look alike and mean different things is worse than either. |
||
|
dtourolle
|
df741a8a49 |
Let one mask be built from more than one selection, and paint into it
A mask the model draws arrives approximately right — stopping inside a shoulder, leaking into the hair — and FR-DEV-3's edge controls move the *whole* boundary, so no value of feather or dilation fixes two errors that go opposite ways. What fixes them is a second selection joined to the first, and a layer that held exactly one source had nowhere to put one. The brush the core has had all along was reachable from no control in the application. A layer is now an ordered list of parts. Each names a source and how it joins the mask before it — added to it, or taken out of it — and carries its own edge treatment, because a model's soft coverage and a stroke painted where it stopped short do not want the same feather. Invert and opacity stay on the layer, where the composed shader already reads them. The sidecar grows `[part]` blocks and nothing else. A layer of one part writes exactly the bytes it always did; a mask block with no part blocks after it reads back as one part; and a stroke, a join or a source this build cannot read costs that part rather than the layer. So every sidecar in every library still parses to the edit it always was. On the device the parts fold into the layer's one slice, so eight layers still cost eight channels: union is a `max` blend and subtraction is the erase blend the brush already used. A part is drawn into a scratch texture before it is joined, and that is not incidental — an erase stroke means a hole in *that part*, not a hole in the mask, and drawn straight onto the accumulator it would punch through the subject underneath. A layer of one part skips all of it and takes the path it always took. In the interface: a part list under the selected layer with a chip saying which way each joins, Add and Subtract beside it, a Select/Paint/Erase strip with the brush's size, hardness and flow, and a drag on the photograph that paints. Pressing Paint on a mask that cannot hold a stroke joins a part that can, rather than explaining that a subject is not a brush. A whole stroke is one step in the history. The edge controls now shape the part that is selected rather than the layer, which is the one behaviour change to an existing control: with a correction selected, the feather slider softens the correction and leaves the model's mask alone. |
||
|
dtourolle
|
af89433aee |
Offer the lens profile as a tick box, since applying it silently reads as absent
The develop panel's Optics group is three manual sliders: distortion, chromatic aberration and lens vignetting. The automatic correction was already there — the file's EXIF lens is matched against the bundled Lensfun database on open and the coefficients are fanned out to all three — but nothing in the interface said so except a line of grey text under the camera reading "· corrected", and there was no way to decline it. From the outside that is indistinguishable from the feature not existing, which is how it was read. `dr-lens` states the rule this breaks: an automatic correction that silently does nothing is worse than one the user can see is unavailable. The caption satisfied the letter of it and not the point — a photographer looking for "apply the lens profile" found three sliders and no switch. So the profile is now a control. It is a capability rather than a flag on the session, because everything a photographer sets travels one road: the capability list feeds the generated panel, `Preset` captures it, the sidecar stores it and the undo stack replays it. A bool on the side would have needed adding to each of those four by hand and would have been forgotten in at least one — which is exactly how the mask stack came to be missing from the history. It is on by default, which is what `switch_on` is for: the coefficients are a measurement of the lens that took the photograph, so accepting them is neutral and declining them is the edit. The sidecar therefore stores nothing for the ordinary case and the correction still happens. The switch appears only where a profile was matched. A tick box on a photograph whose lens the database has never heard of would be a control that looks available and does nothing, which is the failure the rule above names rather than an instance of following it — those photographs are told "· no profile" in words instead, and one whose box is unticked now says "· profile off", which is a third fact and not either of the other two. Two things had to be built underneath. `ParamKind::Bool` was in the core's closed enum and mapped to a row kind here, and had no control behind it in `adjust.slint`: a parameter declaring itself a switch was flattened into a row that drew nothing at all. Nothing shipped had one until now, so the gap cost nothing and was invisible. And `Check` self-toggled, which is right for a settings page that owns its value and wrong for a panel row that is a view of the edit graph — the click would have answered by replacing the binding with a literal, and the next undo or pasted preset would have moved the value with the tick left where the finger put it. It now takes `controlled`, and the generated row uses it. The manual sliders are unchanged and still trim whatever the profile leaves, so switching it off is "correct this by hand" rather than "stop correcting". |
||
|
dtourolle
|
901f51e6c4 |
Point at something grey and let the pipeline work out the rest
FR-DEV-3 has asked for "white balance (temperature/tint, and picker)" since it was written, and only the first half existed. `WidgetKind::WhitePoint` was in the vocabulary and `develop::supported` answered false for it, so the node degraded to two sliders — correct behaviour that had quietly become the only behaviour. Sampling a neutral is the first move of the global tonal pass and every colour judgement afterwards is measured against where the grey was put, so guessing at two sliders until a wall stops looking green is the wrong way round. The awkward part is that a picker genuinely needs to know how far a hundred units of temperature move red against blue, and that number is declared in the node's own file. So the inversion lives in `dr_pipeline::neutral` rather than in the interface: the canvas hands over a colour, the core finds the operation that asked to be driven by a pixel and bisects its declared response until the sample comes back grey. Nothing in `ui/` names white balance, and nothing holds a second copy of a response that would be wrong the first time somebody adjusted the range. A bisection rather than a closed-form inverse because only monotonicity is part of the bargain — the expression is free to become a table tomorrow. The result is rounded to the precision the control is drawn at, which is not cosmetic: unrounded, sampling something already neutral lands a ten-thousandth off zero, and the photograph comes back modified with an undo step for a correction of nothing. On the panel side this needed one distinction the generated path was missing. `is_on_canvas` was being read as "and so the panel draws nothing for it", which is right for a crop — four edge fractions are not controls anyone drags in a list — and wrong for an eyedropper, which *writes* temperature and tint and leaves them exactly the controls a photographer reaches for next. So a sampling widget keeps its sliders and puts the affordance that arms the canvas in the group's heading, built like the reset beside it. One click, one sample, one history step: `Edit::Action` never coalesces, and there is no hover preview to fill the stack with temperatures nobody chose. Declaring the presentation also groups temperature and tint under one undo step, where they were two. That follows from what `Presentation` means and reads correctly — white balance is one decision — but it is a change, and worth saying so. |
||
|
dtourolle
|
68ebf5d78b |
Let a mask start from a tone or a colour, not only a shape
Every local adjustment began from a shape: painted, drawn with a handle, or found by a model. So the only way to hold back a sky was to draw a line near where it ended, and the only way to warm skin was to paint round it — both of which put the edit's edge where the photographer put a gesture rather than where the picture changes. A gradient across a treeline halos, and an adjustment traced round a face stops on the outline of a hand. MaskSource grows two variants that select by what a pixel *is*. Luminance carries two bounds on the perceptual tone scale plus a softness; Colour carries an arc of hue, a range of chroma, and one softness for every edge of both. Five floats and three, so they diff, sync and merge per field under FR-NC-9 exactly as a gradient's geometry does — the property a stored raster has none of, and the reason the model's coverage had to sit beside its source rather than inside it. The pixels are the shader's business and nowhere else's. `mask.wgsl` takes the demosaiced source as a sixth binding and two new modes read it: decode, balance, pull a clipped photosite back to neutral, apply the camera matrix, then weigh the band. Nothing crosses to the CPU but the numbers and the matrix, and each mask texel averages its own footprint in the source, so a band lands on the tone an area is rather than on whichever texel a proxy grid happened to land on. The photograph it measures is the one the camera recorded, before this edit. A band over the edited result would slide out from under the edit as the edit was made — raising the highlights would change which pixels counted as highlights, and the slider would chase its own mask. Feather, falloff and morphology stay off a range layer, which is what `shapeable` already meant. All three are functions of the signed distance from a boundary, and a range has no boundary to be at a distance from; its edge is the softness of its own band, in the band's units. Offering them would be four controls that move and change nothing. |
||
|
dtourolle
|
81b1ae8c42 |
Measure the haze from the picture, and divide it back out
Four files named dehaze as a member of the compositional detail family — `detail.rs` twice, `dr-gpu`'s detail module, `ops/README.md` and `capture_sharpen.rs` — and no such node existed. Every one of them was describing the family by listing clarity, texture and a control the photographer could not reach. Haze is the one degradation the controls already in the chain cannot remove, and the reason is spatial rather than tonal. Scattering composites an airlight over the scene in proportion to distance, so the lift is per-pixel: a black point that clears the mountains crushes the foreground, and a contrast curve that clears the mountains does the same. So the node has to estimate the transmission at every pixel, which is the dark-channel prior — the local minimum over the channels and over a patch is the airlight that has been added there — and then invert the scattering model with it. The airlight is taken as neutral and as unit, which removes the one part of the published method this stage cannot perform. Estimating it properly is a whole-frame reduction, and the detail chain has none: it hands each pass the pass before it. It is also unnecessary, because white balance is the first node in the chain and has already driven the illuminant to grey, so only the magnitude is unknown — and an unknown magnitude on the veil is a scale factor on the amount slider, which the photographer is setting by eye regardless. The patch is a fraction of the frame's shorter edge, through `RenderScale::frame_fraction`, and never a count of pixels. It has to be wide enough to contain something dark and narrow enough that what it measures is still local, and both of those are statements about how much of the composition it covers — so it must cover the same proportion of the picture on a proxy as in the export, or the file is sharpened for a patch three times narrower than the one that was tuned on screen. Affording it needs an identity a Gaussian does not have. Erosions compose by adding their structuring elements, so the minimum over a run of d followed by the minimum over k points spaced d apart is the exact minimum over the whole kd window. At the square root that is 16 taps rather than 61 at 4K, and it is the same filter rather than an approximation of one — which is the difference from the strided kernel `local_contrast` refuses, where sampling an image that is not band-limited aliases into the base and comes back as mottling. It runs first among the compositional detail nodes, at order 125: after noise reduction, because dividing by a transmission below one amplifies the noise in the veiled distance by exactly the factor it recovers the contrast by, and before clarity and texture, coarse before fine, so that their base is computed on the picture the veil has left rather than on a modelling about to be divided out. What it cannot honour is the placement dehaze most wants. It shifts colour — it subtracts a grey term and rescales, so saturation changes wherever the veil is thick — and the colour work would ideally be correcting the picture that leaves here. The detail stage runs as a group after every point operation, because a neighbourhood pass is a separate dispatch over a texture the fused pass has finished writing, so an order placing this node ahead of `vibrance` would be a lie the chain cannot tell. Interleaving would mean splitting the fused pass in half around it, at the cost of a second full-frame dispatch and intermediate for every edit in the catalogue whether it dehazes or not. The declaration records that rather than leaving it to be rediscovered. FR-DEV-18 is added to the requirements register alongside it. The tag had nowhere to point, and an orphan tag fails the traceability gate rather than quietly counting for nothing. |
||
|
dtourolle
|
7c3e1d2c54 |
Let the shadows and the highlights carry a colour the picture never had
The colour mixer is the only chromatic control in the chain, and it can only turn a hue that is already in the frame. Ask it for cool shadows against warm highlights and it has nothing to take hold of: the shadows of a correctly balanced photograph are near enough neutral that there is no band there to turn, and a monochrome conversion hands it a picture with no hue in it at all. Split toning is the oldest look in the book and every developer worth comparing against ships it; there was no way to reach it from here. So colour_grading, declared like any other node — a hue and a strength for the shadows, the midtones and the highlights, and a global cast over the frame. It targets a tonal range rather than a hue, which is the whole difference between the two controls: it puts colour where none was rather than turning what it finds. It sits at 105, after the mixer has had the last word on the colours that are in the picture and before the detail stage. The mechanism is one helper. Three cosines 120 degrees apart are the hue wheel written directly as an RGB direction, and their sum is zero at every angle, so exp2 turns them into three gains whose product is exactly one — a cast tilts the balance without moving the level. A grade that doubled as an exposure change is the failure that has the photographer chasing brightness with a colour slider, and it is corrected with a control that cannot reach it. The three tonal weights partition the scale rather than overlapping, the midtones being whatever the two ends leave, so setting all three to one hue is exactly the global cast and a split tone does not colour its own midtones as a side effect of its halves meeting. Full strength is half a stop on the leading channel, the ceiling white balance already holds itself to. Neutral is declared rather than inferred, which is what `active:` is for. A hue with no strength behind it is a direction with no distance, so under the default rule nudging one would have put the node into every fused shader for a change nobody can see. Summing the strengths is zero exactly when all four are, and they cannot go negative to cancel each other. The opposite reading — neutral as "nothing has been touched" — fails the other way round: red is hue zero, so a grade toward red never moves a hue off its default and would never have been applied at all. It asks for a colour wheel, the widget the descriptor vocabulary has been carrying with no operation behind it. Nothing draws one yet, and that is fine by construction: the panel takes the first widget it implements and falls through to sliders otherwise, so this arrives as eight ordinary controls that work. Each parameter is named for its own range for exactly that reason — in a flat list, four sliders called "Hue" are four controls nobody can tell apart. FR-DEV-12 is written into requirements.md beside it. A TRACES tag naming a requirement that is not defined there is an orphan, and the traceability gate fails on those rather than quietly counting them. The label catalogue gets one line for the operation's display name; the eight parameters derive correctly and are left to. |
||
|
dtourolle
|
efa9d84aad |
Correct the lens first and settle the grain last
`Attribute::ALL` has claimed since it was written to be roughly the order a photographer works in, and |
||
 dtourolleandClaude Opus 5
|
e235e99cce |
Move the film to Effect in the descriptor that is actually read
An earlier commit claimed to move `film_sim` from `[tone, colour]` to `[effect]` and did not. It edited `ops/film_sim.yaml`, where `attributes:` is read, validated against the vocabulary, and then dropped: a `rust:` node publishes its own descriptor, and the type still said tone and colour. The stock went on appearing in the Light group beside exposure and again in Colour beside white balance, exactly as before, and every test passed. Nothing caught it because nothing could. The declaration parsed, the parity tests compare ids rather than attributes, and an operation filed under the wrong groups renders perfectly. It surfaced only on screen, as a missing Effects tab — which is indistinguishable from a category that genuinely has nothing in it, and is precisely how `Optics` looked for as long as it was empty. So three changes rather than one: `FilmSim`'s descriptor declares `Attribute::Effect`, which is the move the earlier commit described. `attributes:` joins the keys a `rust:` node may not carry, beside `params`, `uniforms`, `wgsl`, `helpers`, `define` and `label`. The rule was already written — "its descriptor comes from the type" — and attributes were the one field that slipped past it. A key that is silently ignored is worse than one that is rejected, because it reads as though it worked; the eight hand-written declarations lose a line that never did anything. And a test asserts that every attribute the chain carries reaches the tab strip. That is the property that was actually broken, and its failure mode is invisible from every direction: the controls exist, they are in the shader, and there is no way to filter to them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
474dcf0bf6 |
Let Compose lead the attributes, as their own doc always said
`Attribute::ALL` claims to be "roughly the order a photographer works in" and then listed framing fifth, behind tone, colour and detail. Framing is the first decision made about a photograph and the one every later judgement is made inside — there is no sense balancing tones across a frame about to lose a third of its width. The contradiction was harmless while the list only fed a row of chips nobody reads in order. It stops being harmless now that the same list drives a column read top to bottom. `declared::Attr::ALL` moves with it. The two are separate spellings of one vocabulary and a test asserts they agree, which is what caught this rather than the order silently disagreeing between the YAML front end and the crate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
601c984894 |
Fold a photograph's rival default versions back into one
Picking the newer of two default versions stopped the wrong edit being shown, but it did not close the split: the losing version stayed in the file, and a device holding disjoint work — a crop made here, an exposure change made there — still contributed only one of the two. Worse, the next write made it larger. `amend` looks its version up by uuid, neither of the two was ours, so the miss minted a *third* `default = 1` block and the file grew one rival per device per photograph. `Sidecar::fuse_default_versions` folds them down. The version with the highest `(revision, modified)` is the accumulator and every other default is merged into it as the remote, which is what makes the fold order-independent — `Version::merge` raises its own revision to `max + 1` as it goes, so merging a chain in ascending order stops being ascending after the first step and a third device would be dropped. Contested values resolve to the winner, disjoint keys survive from both sides because the merge is key-wise, and ratings come across under `merge_judgement`, so a device that never judged the frame cannot erase one that did. The result is a function of the file's bytes alone, so two devices that fuse independently reach the same document and converge instead of overwriting each other. Called wherever a sidecar is parsed: - `amend`, with the write's own uuid, so the fold lands on the identity this device is about to use and the lookup below it hits instead of missing. - `spawn_sidecar_fetch`, so opening a photograph shows everything done to it rather than whichever half won. - `drain_one`, because `merge_into` reconciles by uuid and would otherwise publish the split rather than resolve it. - `presets::load_local` and `save_local` — a local sidecar's folder may be synced by something else entirely, and gets the same split. A file with one default under the expected uuid comes back byte-identical, so this costs nothing on the ordinary write and no sidecar is uploaded merely for having been read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
98fcf8e98e |
Ask which edit is newer, not which uuid sorts first
A photograph edited on two devices ends up with two `[version]` blocks in one sidecar, both marked `default = 1`. Five of the thirty-eight sidecars in the local cache are in that state right now. `default_version` answered with the first `is_default` it met in map order, and the map is keyed on uuid — so which device's work the photographer saw was decided by which randomly minted uuid happened to sort lower. On `IMG_20130625_0033` that is `0545c20a` over `679fe872`: a four-star rating from the twenty-first of August standing in front of the one-star made on the thirtieth, with nothing anywhere saying the newer judgement existed. Resolved by `(revision, modified)` instead, which is the discriminator `Version::merge` already uses — revision first so that a device with a skewed clock cannot win by claiming a later timestamp (FR-NC-8), and the timestamp only to break an exact tie. This makes the reader pick the right one. It does not make the two converge: the edit that lost is still in the file, and a device that holds disjoint work — a crop here, an exposure change there — still only contributes one of them. That is the next commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
a56baa9042 |
Let rustfmt have the assertion it reflowed
The closure taken down to `&dyn Operation` needed its call site re-wrapped, and I wrapped it by hand rather than letting rustfmt decide: it fits on one line at the workspace width. `cargo clippy` was run on the change and `cargo fmt --check` was not, which is the whole of how it got through — the two catch different things and CI runs both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
2841eaf9a1 |
Take the layer-chain test's closure down to &dyn Operation
`clippy::borrowed_box` is denied by the workspace lint set, and the closure added with the optics exclusion took `&Box<dyn Operation>` — a borrow of the box rather than of the thing in it, which says nothing the plain trait object does not. Caught by `cargo clippy --workspace --all-targets -- -D warnings`, which is what CI runs and what the workspace tests do not: a lint on test code only appears when the tests are compiled as a clippy target. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
c4ddcbe0f7 |
Look the lens up and say plainly whether one was found
`dr-lens` has held a complete Lensfun lookup — distortion, TCA and vignetting coefficients from a lens name, a focal length and an aperture — with no dependents anywhere in the workspace. The three corrections it feeds now exist in the graph, so this connects the two and finishes the chain. The coefficient structs stay duplicated. `dr-pipeline` is organised around having no dependencies so its codegen is testable without a device or a database (ARCH §6.5a), and `dr-lens` carries an XML parser and 5.5 MB of profile data. Neither crate can convert to the other, so the conversion goes above both, in `develop.rs`, which is the only place that sees them together. Both traits grow the same defaulted door. The optical corrections do not sit on the same side of the fetch — distortion and CA rewrite coordinates and are `Warp`s, vignetting applies a gain to the pixel already there and is an ordinary node — and fanning a profile out by which trait each happens to implement would make the caller reason about that distinction. Each correction takes its own share of the whole profile instead, and `set_lens_profile` walks both lists identically. The lookup happens in `set_source_metadata` rather than in its caller, because that is the one place a session is told which file it came from. Doing it there makes it unforgettable, in the shape `FilmRebake` already uses for the other derived thing — and, more to the point, makes *clearing* unforgettable: a session that opened a second photograph while still holding the first one's profile would correct it for the wrong optics, invisibly, in a way that looks exactly like the lens. It needs the whole shot and not just a name. Distortion is interpolated across a zoom's focal range and vignetting depends strongly on aperture — a fast prime can be two stops down in the corners wide open and clean by f/8 — so a lookup missing either returns coefficients measured for a shot nobody took. Missing any of the three refuses rather than guesses. A profile is derived, not persisted: it comes from the file's EXIF and a database, so it is not a parameter, not in the sidecar and not undoable. What is an edit is the manual trim beside it, which each correction composes with the measurement — so a photographer can lean on it, override it, or work without one. `InfoPanel` gains a lens line, and it distinguishes three cases rather than two. `dr-lens` states the rule it exists for: an automatic correction that silently did nothing is worse than one the user can see is unavailable. A session with no header draws nothing, a header naming no lens reads "Lens not recorded", and a lens the database has never heard of reads "· no profile". Collapsing the last two would send somebody hunting for a profile that was never missing — which, for third-party and adapted glass, is the ordinary case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
a1165ef182 |
Put the coordinate-domain lens corrections into the graph
`lens.rs` has held a `Warp` trait, a composer and two implementations — distortion and lateral chromatic aberration — since they were written, and `compose_warps` was called by nothing outside its own tests. The corrections existed, were correct, and never touched a photograph. `EditGraph` now holds them, and `compose_full` emits them between the framing prologue and the fetch. Distortion first, then CA: each warp receives the position the previous one produced, and lateral CA is a magnification about the optical axis of the *undistorted* frame, so measured on a barrel-distorted one it would be fitted to a radius no profile describes. They reach the panel the way framing already does — through `capabilities`. That was the one open question and existing practice answered it: framing is also not an `Operation`, also has parameters a photographer sets, and also arrives through that list. Because `Preset::capture` walks the same list, the sidecar, the clipboard and the undo stack carry a warp's parameters with nothing registered anywhere, and no file under `ui/` names one (FR-DEV-3a). `state()` destructures `EditGraph` field by field precisely so that a new field cannot be forgotten, and it was not. Chromatic aberration is the only thing that samples per channel, and `splits_channels` is what keeps everything else from paying for it. Red and blue are fetched from positions green is not — green is the reference and never moves, so a wrong correction still leaves one channel sharp rather than softening all three. With no CA in the chain the single-fetch path is emitted instead. The interpolating sampler is now chosen by framing *or* an active warp. Asking framing alone would have nearest-neighboured a distortion correction on an unstraightened frame, and that aliasing reads as a bad profile rather than as a missing filter. The warps go in the geometry invalidation key rather than the colour one: they decide which source pixel a colour is read from, so a tile cached across a distortion change would keep drawing the previous correction. The pipeline cache needs nothing new — `hash_source` already covers the generated body, and uniform values never enter it, so arming a warp recompiles and dragging it does not. Both are asserted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
e17b909d41 |
Connect the lens vignetting correction to the pipeline
`ops/vignetting.rs` has carried a complete descriptor, polynomial, helper and test suite without an entry in `ops/`, so it was never in `chain()`. It reached no photograph and no panel, and `Attribute::Optics` was an empty category in consequence — filtered out of the tab strip for having no rows, by a chain that had never been given its only member. Declaring it needs the one thing the operation was written against and which did not exist. `wgsl_body` reads `radius`, and the module claimed "the composer publishes `radius` in the shader prologue for exactly this reason". It did not. `sample_source` now does, in both sampling branches, beside the `source_px` it already published for the same class of caller. It is corner-normalised there, which is the part that is easy to leave out. `p` spans ±0.5·aspect, so its length at the corner is 0.5·length(aspect) — about 0.901 on a 3:2 frame, not 1. Lensfun's polynomials are fitted against a corner radius of 1, so passing `length(p)` straight in evaluates every one of them short of where it was measured, by a factor that changes with the aspect ratio. It would have read as a correction that is simply too weak, which is indistinguishable from a bad profile. Both `lens.rs` and `framing.rs` asserted the normalisation `p` does not have; corrected. `order: 5` puts the correction ahead of the tonal stages, and the ordering is load-bearing rather than tidy. Recovering a corner means dividing by an attenuation below one — about two stops for a fast prime wide open — so run after the highlights have been rolled off and clipped, the lift has nowhere to go and the corners posterise instead of brightening. `layer_chain` now drops `Optics` as well as the neighbourhood operations. A local vignetting slider would have worked, which is what makes it worth excluding: `radius` measures from the centre of the whole photograph and a mask cannot move the optical axis, so it would lay a frame-centred radial ramp across the picture and multiply it by the mask. The existing exclusion covers operations that move and do nothing; this one covers an operation that moves and does something its name does not promise. The rule both share is now written down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
8e7b1350bf |
Name the frame's category for the decision, not the maths
`Attribute::Geometry` becomes `Attribute::Compose`, and `film_sim` moves from `[tone, colour]` to `[effect]`. Two categories were doing the wrong job. "Geometry" describes what crop, straighten and the quarter turns do to coordinates — but it describes lens distortion correction exactly as well, and that is not a compositional choice at all. Naming the attribute for the photographer's decision is what separates it from `Optics`: one is what the lens did, the other is what they chose. The maths the two have in common is not the thing worth filing them under. A film stock declared both `tone` and `colour`, so "Kodachrome" appeared in the Light group beside exposure and again in Colour beside white balance — two places, neither of which is where anyone looks for it. It is neither: `Effect` is defined in this same file as "applied rather than corrected — a look, not a fix", which is what a stock is. That it moves tone and colour is true of every look, and is not what the attribute is for. `from_name` still accepts "geometry" on the way in. That string is persisted in `develop.copy_attributes`, and an entry it fails to parse is not an error — `presets::scope_for` logs it and drops it — so without the alias an existing settings file would have quietly narrowed what a paste carries. `name` writes the current spelling, so the file migrates itself the first time it is saved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolle
|
89c4ff1820 |
Store what the model found, so a reopened photograph keeps its masks
A subject or category layer was written to the sidecar as identity alone —
which run, which instance, which category — on the reasoning that the pixels
are reproducible by running the same model over the same image. They are, but
only by *running the model*, and nothing runs one except a photographer
pressing "find subjects". So on every path that did not already have a run in
memory the layer resolved to no coverage, `MaskPass::render` logged "has no
distance field; skipping", and the adjustment was silently absent:
- reopening an edited photograph rendered it without its local adjustments,
and then saved that state back on the way out;
- a batch export from the grid could not have them at any point, because
`render_from_library` opens a session, applies a version and renders, and
there is no model anywhere on that path. Three hundred files written
without the edits their photographer made, over a log warning.
Neither failure announced itself. The generated shader still emits the layer's
block and the empty placeholder multiplies it by zero, so the result is a
well-formed frame that is simply missing an edit — `mask_is_stale` already
named the state and called it "not stale, just unrenderable".
The coverage now travels in the file, as one `coverage = w h levels payload`
line at the end of the layer's block.
Two levels, and that is not a compromise. The model hands out a byte per pixel
but `Shaped::build` measures its distance field from `coverage >= 128` and
throws the shoulder away on the first line; everything soft about the rendered
edge comes afterwards from the layer's feather and falloff, which are read off
the distance. So one bit per pixel is not an approximation of what the model
said — it is exactly the part of it that reaches a pixel, and the stored mask
renders the identical frame. Storing all 256 levels would have stored 1.7 MB
of bilinear interpolation to reconstruct a predicate, and would not even have
compressed: a model mask is a bilinear upsample of a coarse grid, so almost no
two adjacent bytes are alike. Measured on a simulated sky and a simulated
figure at 1600x1067, against 1.71 MB raw: 4.0 kB and 6.5 kB at two levels,
46 kB and 76 kB at sixteen, 835 kB and 1.43 MB at all 256. The level count is
still written into the line, so a later build that finds a use for the
shoulder can write sixteen and this one will read them rather than misreading
a stream of lengths as pairs.
The coder is hand-rolled — run-length pairs in a base-64 varint — because
`dr-pipeline` links nothing, which is the property that lets the descriptor
and codegen logic be tested without a device. `flate2` would have been fewer
lines and a dependency in the one crate that has none.
Where it lives matters more than how it is coded. The raster sits on
`MaskLayer` beside the source, not inside `MaskSource::Subject`: the source is
*identity*, which is what makes it diff as a handful of numbers and merge per
field under FR-NC-9, and a raster in there would have given the merge a binary
blob to arbitrate. It takes no part in `MaskLayer`'s equality for the same
reason — a device that has run the model and one that has not hold the same
edit, and counting the difference would raise a conflict over a cache and let
`remote_wins` answer it by discarding the only copy of the pixels.
Encoding happens in `masks_for_storage`, on the save path, rather than in
`ensure_subject_fields` where every coverage already funnels through.
`ensure_subject_fields` runs on a drag — dilating a mask with a compound
morphology rebuilds the field every frame — and encoding a megapixel raster
per frame is the kind of work NFR-P5 exists to keep off a gesture. Saving
happens once, when the photograph stops being the open one, and already costs
a network round trip.
Version skew holds both ways. A file with no `coverage` line reads exactly as
it did before, which is a layer that needs the model run; an unreadable one
costs the pixels and not the layer, because the layer is the edit and the
raster is a cache of it. An old build reading a new file drops the key it does
not understand, which costs a model run and no work. And a payload that will
not compress is refused rather than truncated: a checkerboard would encode to
twice the raster it came from, so past 64 kB nothing is stored and the
behaviour falls back to what it was — half a mask would render as a mask that
is confidently wrong, which is the failure that tells nobody.
|
||
|
dtourolleandClaude Opus 5
|
3d248cfb79 |
Export the photograph, not the canvas
Zooming the develop view changed the exported file. `Framing::view` is kept out of the sidecar, out of `is_active` and out of `output_size` precisely so that it cannot — but those exclusions keep it out of the *edit*, and an export is a *render*. `visible_rect` deliberately folds the view into the single rect the fused shader's prologue samples, so `render_for_export` inherited it: at 4:1 it wrote the middle of the frame, magnified to fill the file at the full output size, with the detail kernels scaled four times over because `render_scale` folds the view in as well. `render_thumbnail` did the same to the grid. `render_uncropped` already suspends the view for this exact reason, so the fix is its pattern: one `render_the_file` that both file-producing paths go through, composing inside the suspension since the view reaches the shader as a uniform baked at composition time. Restored whatever happens — leaving the graph un-zoomed after a failed export would throw away where the photographer was looking. Nothing caught it because the guard checked the wrong things. `zooming_does_not_change_the_exported_image` asserted the output size and the crop; both held perfectly throughout. Renamed to `zooming_does_not_change_the_size_or_the_crop`, which is what it tests, and the pixels are now guarded where pixels exist. The new test uses a ramp rather than quadrants deliberately: a four-quadrant frame is self-similar under a centred zoom, and the first version of this test passed against the bug because of it. Traces FR-EXP-9, which asks for the full-quality pipeline "regardless of what the display was showing". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
4efab496c2 |
Put the refinement on a slider, per layer
Benchmarks / CPU and I/O (per commit) (push) Successful in 3m27s
Benchmarks / Frame budget (on demand) (push) Skipped
🐳 Android image / Build and push (push) Successful in 3s
Build and test / android-image (push) Successful in 3s
Build and test / Desktop (Linux) (push) Failing after 1h14m30s
Build and test / Layer separation (push) Successful in 43s
Traceability / Requirement traces (push) Failing after 46s
Build and test / Android (aarch64) (push) Successful in 1h8m56s
The evidence was being gathered and spent immediately at one strictness nobody could see or change. This makes it a control. `MaskLayer::refine` is shaping, like the feather, and it lives on the layer for the layer's reason: two layers may sit on the same category and want different amounts of it, and the model ran once for both. ## What the segmentation now stores `CategorySummary` keeps the **coarse** mask and the `Refinement` beside it, rather than a refined mask. That is what gives the control an off position that is bit-for-bit the model's own weighting, and what stops a strictness change from needing the model again. `category_mask_at` borrows at zero and wherever no refinement could be fitted, so a layer nobody has touched costs nothing over the old path. The fit moved to the far side of the orientation permutation. The verdict is a per-pixel field over the same grid as the mask it gates, so fitting it upright would mean permuting a proxy-sized buffer afterwards to match — a second rotation, and a second chance to get one wrong. One logit cell is the same number of pixels either way: the letterbox scales by the longer edge and a permutation does not change which edge that is. The two frames became a `Frames` struct rather than six parameters. This function reads the picture twice for opposite purposes — the model needs it upright or it recognises far less, the refinement needs the sensor's grid — and a transposed pair produces a plausible mask over slightly the wrong pixels, which is the failure this module is most prone to. ## It rebuilds the field, and the signature says so `refine` is mixed into `subject_signature`. Unlike a feather, which is read off a field that is already correct, this changes which pixels are in the mask at all — so it changes the coverage the field is measured from. Omitting it is the bug where the slider moves and nothing happens until some unrelated control invalidates the cache. That puts it in the same cost class as a close or an open, which is why the row takes `SliderRow::changed` — already once-per-gesture, since that row takes `SliderTrack`'s `committed` internally — rather than a live stream. The slider is offered only where there is something to move: a category source, *and* a refinement the frame actually gave enough to fit. A control that moves and does nothing is worse than an absent one. ## Two defaults that are deliberately different A layer added from the panel starts at 4.0, because a category's edges are twenty proxy pixels wide before anything is done to them and a photographer adding a sky mask wants the sky rather than the sky plus every chimney in it. A layer read from a sidecar with no `refine` key starts at **zero**. A file written before this control existed has to render as it did then, and a default of 4 on absence would quietly re-grade every stored category mask in the catalogue. `a_categorys_refine_strictness_survives_and_defaults_off` holds both halves, and `an_out_of_range_refine_is_clamped` holds the file to the scale — past the top of it every colour fails and the mask deletes itself, which reads as lost work rather than as a bad file. `MAX_REFINE` is dr-pipeline's own constant mirroring `dr_segment::STRICTNESS_MAX`, following `Falloff` and `Morphology`: this crate holds the description of an edit and must not depend on the crate that runs a model. dr-ui is where the two meet, and the only place that converts. Verified: fmt clean, clippy --workspace -D warnings clean, 488 dr-pipeline and 60 dr-segment tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolle
|
328fda6f7c |
Merge: mask a whole category, not just one instance
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> # Conflicts: # apps/darkroom-desktop/Cargo.toml # docs/traceability.md |
||
|
dtourolleandClaude Opus 5
|
485297f0c6 |
Let a mask cover a whole category, not just one instance
`MaskSource` could say "instance 3 of that segmentation run" but had no way
to say "the sky". Adding `Category { signature, name }` beside `Subject` is
what lets a local adjustment attach to a semantic category at all.
Stored as identity like a subject, and for the same reason: the coverage is
megabytes and is reproducible by running the same model over the same image,
so the sidecar carries what finds it again and the session carries pixels.
## A name rather than an index
An index would be smaller and would match `Subject`. It would also be a bug.
The grouping lives in `models/scene/categories.txt`, which is editable by
design — adding one category to it renumbers every category after it, and
every stored layer would silently start grading something else. A name that
no longer exists is simply not found and the layer reads as stale, which is
the failure that announces itself.
Staleness is otherwise identical to a subject's: the coverage buffer is in
the session, never the sidecar, so a signature from another run points at
pixels that were never computed.
## Two tests, and the second one caught a real shape
Round-tripping the name matters more than usual here, because the whole
argument for storing a name instead of an index is worthless if the sidecar
is what drops it.
The multi-word case is the one worth having: `category = swimming pool` is
written on one line, and a reader splitting on whitespace would have
truncated it to a category no model has — a layer that silently masks
nothing. `category` is also its own key rather than a reuse of `class`,
because a file conflating them would round-trip a subject into a category.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
dtourolleandClaude Opus 5
|
99563b33f7 |
Record the one clause of FR-PLG-8 that is built, and say what it is short of
FR-PLG-8 reads as unbuilt, and most of it is: no aggregated missing-plugin notice, no catalog-wide view, no export gate, no mark on an image whose operation is unavailable. But its opening sentence is a claim about existing behaviour — "the sidecar already preserves lines it does not understand verbatim and writes them back untouched" — and it goes on to say that property "is now load-bearing and shall be treated as such". Two tests treat it as such, and neither was tagged. `sidecar.rs` keeps an unknown operation across a parse-and-write, and keeps it out of the edit graph so that preserving it is safe rather than merely tidy. Both would fail if the verbatim path were removed, which is what CONTRIBUTING.md asks a tag to mean. The tag sits on the two tests rather than on the module, so the matrix points at the clause that is closed rather than at the requirement as a whole. It is still short of the requirement's own acceptance criterion, and the tag comment says so: FR-PLG-8 asks that a sidecar written with a plugin, opened and saved without it, be byte-identical to the original, and the test asserts `contains`. Both halves exist separately — `writing_the_same_state_twice_is_byte_identical` proves byte identity for content this build understands — and nothing joins them into the single claim the requirement makes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolle
|
91fe6cf300 |
Merge master into partial-preset-scope
🐳 Android image / Build and push (push) Successful in 8s
Build and test / android-image (push) Successful in 8s
Build and test / Desktop (Linux) (push) Failing after 1h17m12s
Build and test / Layer separation (push) Successful in 56s
Traceability / Requirement traces (push) Successful in 1m33s
Build and test / Android (aarch64) (push) Successful in 1h0m38s
# Conflicts: # docs/traceability.md # ui/dr-ui/ui/app.slint |
||
|
dtourolleandClaude Opus 5
|
754ab91347 |
Bring a Lightroom library across, and start with something in the list
Two halves of the same complaint: a preset sheet that opens on "No presets yet" is homework, and a photographer with ten years of presets in Lightroom has no way to bring them. `dr-preset-xmp` reads Camera Raw `.xmp`. The mapping turned out to be mostly a rename rather than a conversion, because Adobe and this pipeline already agree: exposure is in stops in both, and contrast, the four recovery controls, clarity, texture, vibrance and saturation are all ±100 in both. That is not imitation, it is the convention raw developers converged on — `highlights_shadows.yaml` cites it in as many words. Only sharpening needed arithmetic, Adobe's 0…150 against our 0…100. The white balance does not come across, and says so rather than guessing. Adobe writes absolute Kelvin for a raw file where ours is a relative nudge from what the camera recorded, so converting needs the *target image's* as-shot white balance — exactly what a preset cannot carry, since the same preset lands on a frame shot at 3200K and one shot at 7000K. A guess would be wrong on most images and invisibly so. A folder is read as readily as a file, nested, because that is the shape an exported preset folder is in and importing ninety files one at a time is asking someone not to bother. `dr_pipeline::starter` is six presets a first run begins with, written against this pipeline in its units and deliberately mild — a starting point, not a caricature. They are seeded when the library *file* does not exist rather than when the library is empty, so deleting all six does not hand them back on the next launch. Both of these name operations, and `ui_names_no_operation` was right to stop them living in `ui/`. That test exists because the failure is silent and cumulative, and it caught exactly what it was written for: a preset called "Punch" is a statement about contrast, clarity and vibrance, and a table mapping Adobe's vocabulary to ours is a statement about the pipeline. Neither is a fact about an interface. So the starter set went into `dr-pipeline`, and the importer into its own crate — between two walls, since `dr-pipeline` depends on nothing on purpose and XMP is real XML not worth hand-rolling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
bb35665bd2 |
Let a paste carry some kinds of edit and not others
FR-DEV-6 asks for presets "covering a subset of the edit graph". What landed with the named presets covered two subsets: everything, and everything but the crop. "Match the colour but not the sharpening" had no way to be said. `Scope` is now a set of `Attribute` — the same six kinds every operation already declares and the develop panel already builds its tabs from. The photographer ticking "tone and colour" is naming the groups they navigate by, and neither this module nor the interface has to name an operation to do it (FR-DEV-3c). The pleasing part is what left. Framing used to be excluded by an explicit test against one operation's id; it is now excluded because Geometry is not in the default set. The special case dissolved into the general rule, and the argument for it — a crop is a decision about *this* photograph, and carrying it across forty destroys forty compositions — is now a statement about a kind of edit rather than about a node. All thirty-three existing preset tests pass unchanged, which is the evidence that the generalisation kept its promises. One decision that is a field rather than a rule, because the two cases genuinely differ. An operation this build cannot classify — from a newer version, arriving over sync — travels under "everything" and "everything but the crop", because those are claims about the whole edit and an unrecognised operation is part of it (FR-NC-8). It does not travel under a hand-picked set, because that is a claim about kinds, and an unknown kind is not one of the kinds that were ticked. The settings page's "Copy crop and rotation" checkbox is gone, replaced by the same chips the preset sheet draws. It asked the right first question — geometry is the kind whose accidental travel destroys work — but it was the only question a boolean could ask. The field stays in `Settings`, read exactly once to seed the new set, so anyone who had ticked it keeps their behaviour. The chips are deliberately not in the develop column. Six of them there would set the width of the whole sidebar, which is the bug `ChipGrid`'s comment records at length. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
ab0ef6a26d |
Say which requirements the code was already satisfying
Thirteen requirements were surveyed as built but untagged. Eight of them were: R3, R6, FR-DEV-1, FR-UI-6, FR-NC-6d, NFR-OPS-3, NFR-PORT-2 and NFR-SEC-3. Each was read against its full text in requirements.md and against the code before the tag was added, because a tag that is wrong is worse than an absent one — it turns a visible gap into an invisible one. The five that were refused, and why, because the reasoning is the part worth keeping: R2 carries "(figure TBD)" in its own acceptance criterion and asks for a stated prefetch margin and cache-hit rate; neither figure exists anywhere in the tree and neither quantity is measured, while TD-2 and TD-3 both describe the thumbnail path falling short of it. R5 asks for three things and the code does one. The display pipeline does run at viewport resolution, but "only visible tiles are computed" and "panning recomputes only newly exposed tiles" need a tile scheduler that does not exist — and frame_budget.rs currently argues for striking tiled computation from the interactive path rather than building it. FR-RAW-2 asks for a trait taking a SourceRef, so that a second decoder can be added without changing callers. What exists is free functions over &[u8]. That meets the requirement's stated *purpose* — the same decoder serves a local file, a SAF document and a byte range, which is exactly why it takes bytes — but there is no trait and no second implementation seam, so the requirement should probably be amended rather than tagged. NFR-ARCH-1 asks for named executors with stated thread counts. architecture.md §7.1 states the table; nothing implements it. Workers are twenty-odd ad-hoc std::thread::spawn sites, each building its own one-worker tokio runtime, with no decode pool, no GPU-submit executor and no I/O pool. The requirement's own text says R4 and NFR-P9 "assert an outcome with no stated means", and that is still true. NFR-SEC-4 is satisfied by absence — there is no telemetry — and absence has no module to tag. A tag would point at nothing. NFR-OPS-3 was the closest call of the eight taken. The store is single, separate from the catalog, survives a catalog rebuild and does not sync between devices; it has no version *field*, deliberately, and settings.rs argues why and names the condition that would need one. The substance is met and the reasoning is recorded where it belongs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |