46 Commits

Include renames

Author	SHA1	Message	Date
dtourolleandClaude Opus 5	33847a0bbc	Open one GPU device for the tests, and stop the checkout dying over LFS ... Two CI failures, unrelated except that both were mine. **The test binary faulted under parallel threads.** Every GPU test opened its own `GpuContext`, and `cargo test` runs on as many threads as there are cores — so a full run asked the driver to bring up a dozen Vulkan devices at once and died with SIGSEGV. Serially it passed, which made it look like flakiness rather than a fault in the harness. One device now, behind a `OnceLock`: a `GpuContext` is an `Arc<Device>` and an `Arc<Queue>`, so sharing it is a refcount, and the losers of the race block until the winner is done. 416 tests now pass in parallel, in half the time twenty-six devices took. **`lfs: true` on the checkout made the checkout fail.** The intent was right — the model is in LFS, a plain checkout writes a 133-byte pointer, and the build script panics on it — but on this server `git lfs fetch` is rejected at `/info/lfs/objects/<oid>` with a client error: the credential `actions/checkout` installs for git is not one the LFS endpoint accepts. So a fetch problem presented as a checkout problem and took the whole job with it. The object is on the server; a clean clone over SSH with `git lfs install --local` pulls all 11 MB of it. It is now its own step with an explicit token, and `continue-on-error` so a credential problem cannot masquerade as a broken checkout — if it fails, the build still runs and fails with the build script's own message, which names the real problem. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-24 19:36:34 +02:00
dtourolleandClaude Opus 5	3a42c63b5b	Format the refine tests the way the gate asks for it ... `cargo fmt --check` is a required step and the mask-refine work landed with a test body it disagrees with. Whitespace only — kept as its own commit so it can be skipped wholesale rather than read for a change that matters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-23 13:24:48 +02:00
dtourolleandClaude Sonnet 5	b3641b5307	Let a subject's mask be refined, and let several masks be edited at once ... Two related gaps in the mask panel, from the same conversation: a subject's outline is only ever as sharp as the whole-frame pass that found it, and a change meant for several layers had to be dragged once per layer. ## Refine mask A "Refine mask" button on a subject layer re-runs detection on a padded crop around that instance's own box instead of the whole frame — the subject reaches the model at its own size rather than squeezed into the model's fixed 640x640 window alongside everything else in the photograph. `RefineJob` mirrors `SegmentationJob`'s split (built on the session, run off it, adopted back), and the crop itself is rendered through `Framing::set_view` — the same ephemeral viewport the interactive zoom already uses to render a region above proxy resolution, so no new render path and no change to the model's own input size was needed. `dr-segment` is untouched: `Tiling::Whole` already treats whatever buffer it is handed as the one window. The result is still downsampled onto the shared proxy grid every instance's mask lives on, but from a sharper source than the whole-frame pass ever saw for that subject, which is what the edge actually reads out of. ## Multi-select `active_mask: Option<String>` is now `active_masks: Vec<String>`. A plain click still replaces the selection; a control- or command-click toggles one layer in or out of it. `set_param` and `reset_op` fan out to every selected layer, each set to the exact value the slider now shows rather than offset by however far it already was — one slider, one reading, applied everywhere selected. Dragging a gradient's on-canvas handle is deliberately not extended to multi-select: several gradients have no single geometry a shared handle could move, so `gradient_handles` stays empty unless exactly one layer is selected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>	2026-08-23 13:17:07 +02:00
dtourolleandClaude Opus 5	fbf286d504	Keep the mask when the photograph is closed ... A local adjustment survived until the session ended and then did not exist. Nothing reported it, because nothing had failed. The sidecar format has carried masks since they were added, `Version::apply` restores them into the graph, and `Version::update` captures them — that work landed complete and was never called. The autosave writes `copy_settings()`, which is a `Preset`: a map from (operation, parameter) to a number. A mask is not a parameter. It is a rule about *where*, with a chain of its own, so it fell outside the only thing being written, and the read path had nothing to read. The save now carries the stack beside the preset, on both the local and the remote path. Deliberately not merged but replaced wholesale: this is the stack as it stands, so a layer the user deleted has to leave the file too. Merging two devices' stacks is `Sidecar::merge`'s job and belongs to sync (FR-NC-9). A paste still carries no masks, and the `Option` is how that is said. Settings travel between photographs; a mask does not, because it is drawn against one frame and describes nothing on another — and `Scope` cannot express that, since it filters parameters and a mask is not one. The test fails against the old save path with "the mask must come back with the photograph", which is the whole of the defect: not a crash, not an error, just an edit that was not there in the morning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-23 12:29:41 +02:00
dtourolleandClaude Opus 5	c75849040c	Format the tree the way the gate asks for it ... `cargo fmt --check` is a required step and had drifted across 45 files. Most of it arrived this week: several operations were written in parallel worktrees and merged by hand, and a hand-merge resolves conflicts without ever running the formatter over the result. No behaviour changes — this is `cargo fmt --all` and nothing else, kept as its own commit so the next reader can skip it wholesale rather than search it for one that matters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-22 21:16:34 +02:00
dtourolle	5852e14a5c	Merge branch 'worktree-agent-a75c901968abfa183' into integration ... # Conflicts: # core/dr-gpu/src/adjust.rs # core/dr-pipeline/ops/README.md # core/dr-pipeline/src/lib.rs # core/dr-pipeline/src/ops/mod.rs # ui/dr-ui/src/develop.rs	2026-08-22 19:29:39 +02:00
dtourolleandClaude Opus 5	b321556dbe	Sharpen the capture with a separable unsharp mask ... Capture sharpening as a two-pass unsharp mask in the detail stage: blur along x, then along y, each pass applying a one-dimensional high-pass to luminance so the composite preserves a flat field exactly and matches the textbook kernel on any locally one-dimensional edge. The radius is stated in source pixels and converted once per render, so a radius tuned on a fit view is the radius the exported file gets. Below one render pixel the operation declines to draw rather than showing sharpening the file will not contain, and emits a single pass-through that still carries the output transform. The develop session now renders through render_detailed, which is what lets an active neighbourhood operation reach the screen at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-22 19:13:00 +02:00
dtourolle	5db234bdf3	Sync with integration	2026-08-22 19:04:24 +02:00
dtourolle	b4e55b47c1	WIP: noise reduction ... Checkpoint committed by the coordinator, not by the authoring agent: the session hit its API limit mid-task and left this work uncommitted. Committed so it survives, NOT because it is finished - expect failing tests and half-applied changes. The agent resumes from here.	2026-08-22 19:01:18 +02:00
dtourolle	0d9910efc6	Merge branch 'worktree-agent-a89309a856c8f4947' into integration	2026-08-22 19:00:56 +02:00
dtourolleandClaude Opus 5	13d003b89d	Let the curve widget plot whichever curve is asked for ... An operation with four curves and a panel that draws one plot needs a way to say which. The panel finds out the way it finds out everything else: the points are faceted with the subject they act on, consecutive parameters sharing a subject are one curve, and a widget spanning several of them gets a selector over their names. Nothing in ui/ contains the word "red", and an operation that grows a fifth curve arrives with a fifth chip. The names ride on the panel rather than on the curve's row, because a Slint model is compared by identity: a fresh list built on every parameter event would make the row look changed every time, and rewriting a row rebuilds the element holding the drag in progress. That is the hazard the in-place point update already exists to avoid. Which curve is on show is interface state, not an edit. It changes no pixel, so it takes no history step, reaches no sidecar, and redraws nothing — the photograph on screen is already right. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-22 16:06:21 +02:00
dtourolle	610f679881	Reconcile the three branches ... Two faults textual merge could not see. Both agents added a `mod tests` to masks_ui.rs — the module boundary was an artefact of them being written apart, and the tests do not overlap, so they fold into one. And `segment` lost its context argument when the work moved to a worker, which a test written on another branch still passed.	2026-08-22 13:33:55 +02:00
dtourolle	5bcd0e0269	Merge branch 'worktree-agent-a22a049c461818dbe' into integration ... # Conflicts: # core/dr-pipeline/tests/mask_sidecar.rs	2026-08-22 13:23:34 +02:00
dtourolleandClaude Opus 5	96a7b405c2	Say which photograph the sliders are pointed at ... Selecting a mask layer silently re-points about thirty controls at that layer's chain. Same panel, same order, same sliders, different meaning — and the only thing that said so was a sentence in the panel above, which a photographer reaching for the exposure slider has no reason to read. An exposure change lands on the whole frame when it was meant for a face, or the reverse; both are silent, and both are discovered later. `ui-navigation.md` §1.1 calls it the dangerous one and it is: the others in that document cost time, this one costs work. The remedy is the classic one for a modal fault — make the mode visible — and the application already had the pattern. Crop arms a canvas interaction, draws an overlay, gives the column one job and is left by the control that entered it. Local masking is the same animal built as a peer panel, and that is what created the ambiguity. So `crop-mode` stops being a bare boolean and becomes one value of a three-state mode, which is the point: two modes could both be on before, and now that is not a state the interface can be in rather than one it is tested against. **One strip, not two.** The mode control was going to sit beside the group strip that filters the adjustments, which is two controls above one column answering the same question — what am I working on. They are one control now, `Crop · Local │ All · Light · Colour`, which is the shape Lightroom Mobile's bottom strip has for the same reason. The two halves are different kinds of state and are drawn differently: a mode is a chip that fills with the accent when it is on, a group is a word with a rule under it. That difference is what lets both be read at once, which they routinely are — picking Light while a mask is selected filters *that layer's* chain and does not leave the mode. Dropping the scope on a group press would be the same fault coming back from the other end, and would make Light mean two things depending on where it was pressed. The strip stays pinned above the develop column rather than moving to the top of the canvas as the document proposed. The half that filters the column belongs to the column, and the photograph is the subject. The canvas keeps one button, which now names the mode it leaves rather than saying "Done" — that was unambiguous with one mode and would not be with two — because the column can be closed on a narrow window and no mode may be inescapable. Entering a mode is a side effect, so Rust owns it rather than the strip writing the property: crop drops the zoom, local turns the overlay on, and leaving clears the selection. That last one is the fix. The "Overlay" and "Select" toggles are gone because they armed things that are simply what the mode *is* — a mode that has to be switched on separately is one you can enter and have do nothing. Escape and the Android back gesture join `back_step` as one `LeaveMode` rather than a second exit concept, and the mode is left before the zoom is: it was entered later, and it is the bigger step back. The heading is where the scope goes. Not a caption beside the panel, the heading *of* the panel that changed — `ADJUST` becomes the layer's name, the same string the selected row in the stack shows. That is the difference between describing a hazard and removing it. **Handles on the photograph.** A linear or radial mask could be created and then not moved, so a radial sat at the centre of the frame at its default size for ever. Three faults stood in the way of drawing one. The first is that a gradient did not render at all until the model had run. The rasteriser was built on the way out of `segment` and the array's size was read *off* the segmentation, so a gradient added to an unsegmented photograph produced nothing — silently, in the same way exports and thumbnails once did: the shader still emits the layer's block and the empty placeholder multiplies it by zero. The proxy size is a property of the photograph. Both are derived from it now, and deliberately at the same size rather than by coincidence, because a subject's distance field is sampled against that array. The second is hit-testing. A handle is drawn in output coordinates and stored in source ones, and between them lie the crop, the zoom, the pan, the straightening and the turns. `Framing::source_at` is `wgsl_prologue` evaluated on the CPU, kept in that file beside it so that keeping the two in step is one file's problem — a handle mapped through anything less drifts off the mask the moment the view moves, which is exactly what masks are rasterised in source space to avoid. The third is that a drag is a displacement, not a destination. Each handle answers to the movement of the pointer since the press, applied to where the mask was when the press landed. Snapping the handle to the pointer instead jerks it by up to half a touch target on the first press, and the target is finger-sized because a tablet has no hover to reveal a control and no modifier to qualify it. A ramp gets three handles — centre, width, angle. An ellipse gets three too: centre and one per semi-axis, the major one carrying the direction as well as the length, because where an axis is put says both. It had a fourth, and it is gone: standing off the shape by a fixed distance, the rotation arm began outside the photograph at the size a new radial is created at, so the first thing anyone saw was a control they could not reach without first shrinking the mask. Two faults here were found by looking at the screen rather than at the source, both of the kind that cannot be found any other way. A `1px` rule with a size and no position is *centred* by Slint, so the seam between the photograph and the column was a hairline down the middle of the panel, through the histogram and every slider under it — twice, once in `app.slint` and once in `AdjustPanel`. And handing Slint a fresh model for the handles on every pointer event made the repeater rebuild its items, taking the `TouchArea` holding the gesture with them: the handle jumped once and then went dead under a finger that was still down. `develop.rs` carries the same warning about the parameter rows, where it broke slider drags; the model is rewritten in place now. The tests worth having are the ones about ambiguity and about the map. That the same row reads the frame's value, then the layer's, then the frame's again is §1.1 in one assertion. That dragging a handle onto another gradient's matching handle *produces* that gradient closes the loop between the two directions of the framing map, through a view that is cropped, zoomed, panned, straightened and quarter-turned at once — a one-legged map is invisible when the framing is neutral, because then both legs are the identity. Not done here: the histogram still reports the whole frame while the sliders edit a layer. That disagreement is real and is N3's, which this unblocks. The strip has room for a Brush entry beside Crop and Local when the painted masks land in the core, and it needs nothing here but the canvas interaction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-22 13:20:41 +02:00
dtourolleandClaude Opus 5	c0e1179936	Find the subjects without stopping the window ... "Find subjects" took the UI thread for two thirds of a second on a 22 MP frame — a proxy render, a readback and a YOLO pass through `ort` — and for that time the interface was simply gone. The panel apologised for it rather than hiding it: a "Looking…" label, and a 16 ms `single_shot` so the label reached the screen before the freeze began, with a comment saying the obvious fix needed the develop session restructured and was not being taken. The obstacle was never `Send`. `DevelopSession` is `Send` — the device, the source texture and the passes all are. What cannot go to a worker is the `Rc<RefCell<Option<DevelopSession>>>` that every callback in the window reaches through, and the window has to keep reaching through it while the work runs. Handing the session over would freeze the interface exactly as thoroughly as blocking on it did. So the work takes a copy of what it needs instead. A `SegmentationJob` is the device, the demosaiced source behind an `Arc`, and the name of the session that asked. Taking one is two `Arc` bumps; running one is 495 ms on this desktop; none of it touches the session, and there is deliberately no `&mut DevelopSession` in scope for a caller to hold across it. The proxy render travels with it rather than staying behind — a `GpuContext` and a texture handle are both `Send`, and the model was never the only expensive half. So does building the mask rasteriser, which is a shader compile: adopting the result was costing 23 ms, a dropped frame on the one redraw the user is waiting for, and the rasteriser is needed exactly when the subjects arrive and never before. What is left on the UI thread is a microsecond. The answer comes back through a channel a `slint::Timer` polls, which is the shape `apply_when_ready` already uses for a sidecar fetch. **A result can outlive the photograph it describes.** Two thirds of a second is long enough to press the button, think better of it and swipe to the next frame — and the result landing then would fill the panel with subjects that are not in the picture, drawing outlines around a dog two photographs back. Nothing downstream can tell: the masks rasterise and the overlay draws either way. So every session is minted with an id, a job carries the id it was taken from, and `delivery` compares the two before anything is applied. An id rather than a counter beside the session slot, because that slot is written from four places in `lib.rs` and the fifth would be the one that forgot. A discard touches nothing on the way out. `segmenting` belongs to whichever photograph is open now, which may well have a run of its own going, and clearing it would re-enable a button that is correctly insensitive. One run at a time, and abandonment is what stops that being a trap. A job left over from a photograph the user has left is displaced rather than waited for — otherwise the next frame's "Find subjects" would do nothing for the length of a run nobody wants, which is the wait this exists to remove. `ort` offers no way into the inference, so abandoning is checked at the seams there are: before the job starts, and between the readback and the model. Abandoned early it costs nothing, abandoned mid-inference it costs the run it was already committed to, and either way the answer is dropped at the channel. `DevelopSession::segment` survives as a test-only convenience. Left public it is precisely the shape that put two thirds of a second on the UI thread in the first place, and the next caller would reach for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-22 12:26:11 +02:00
dtourolle	1d7106c94d	Apply the masks to the thumbnail and the export, not only the screen ... Reported as a thumbnail bug; the export had it too, which is the serious half. You would have exported a photograph missing every local adjustment. Both called the unmasked `render`, and the failure is silent by construction: the generated shader always declares the mask binding and always emits a block per active layer, so binding the empty placeholder multiplies each of them by zero. No error, no warning, no missing texture — the adjustments are simply not there. From inside either path there is nothing to see. Every path that produces pixels now goes through one helper that binds the array, and that is the point of it being one helper rather than three correct call sites. The array is rasterised in source space at proxy size and sampled through the framing map, so one array serves every output size: a 256px thumbnail and a 24 MP export bind the same texture. Three tests, and the first is the fault stated directly — render the same edit with and without the array and assert they *differ*. If binding it ever stops mattering, the masks have stopped reaching the shader. The third checks the masked share of the frame is the same at 32px and 128px, because "both non-empty" would pass while a mask that scaled wrongly still ruined every thumbnail.	2026-08-22 10:38:49 +02:00
dtourolle	924a837389	Group the panel by what operations say they are about ... A strip of groups over the adjust panel — Light, Colour, Detail — derived from the attributes the operations declare. `adjust.slint` names none of them: the strings arrive resolved and the panel only draws them, so a new operation joins the right group by saying what it is and this file does not change (FR-DEV-3a). A group nothing carries is not offered, so a tab never opens onto nothing. Geometry is left out because its one operation prefers an on-canvas widget and is skipped by the row builder — a Geometry tab would be empty while `GeometryPanel` holds the real controls. The strip appears only when there is more than one group to choose between; a single tab is a control with one option. The selected group is underlined rather than filled. The accent means *modified* everywhere else in this interface, and spending it on "which tab" would blunt the one signal the panel has. **The trap, and it nearly bit again.** `op_index` on a row counts over every capability, not over the ones a filter kept — it is how a row routes back to the core. Renumbering it while filtering would make a slider drive a different operation, which looks like a rendering fault rather than a routing one. `rows_filtered` keeps `enumerate` over the full list and only `group_head` is a position within the emitted rows; a test moves a value through a filtered row and checks it lands where it was asked to. Six tests, including that a nonsense index falls back to showing everything rather than to showing nothing.	2026-08-22 10:22:47 +02:00
dtourolle	7421837c8a	Let an operation say what it is about, so the panel can group without naming ... Tool tabs need a taxonomy, and the taxonomy was the problem: a table in `ui/` mapping operation to tab breaks FR-DEV-3a, and a `group:` field risks what `ui-refinement.md` condemned `starts-group` for — the core deciding where the panel draws things. `Attribute` threads the needle. It says what an operation *is* — tone, colour, detail, optics, geometry, effect — which is the same category as `ParamKind` and squarely on the core's side of ARCH §4.3a's line. What is drawn, where it sits and whether it is visible stay the frontend's. There is no attribute for "the third tab", the enum's order is declaration order rather than screen order, and a frontend may render these as tabs, as headings, or ignore them. The payoff is that a tab strip can be *derived*: the groups are the attributes present in the capability list, so the interface names no operation and needs no table to keep in step. An operation joins the right group by declaring what it is, which is the one thing its author is well placed to say. Plural, because the tone curve is genuinely both — an RGB curve is tonal and the per-channel curves are chromatic, and filing it under one would hide it from half the people looking for it. Required and non-empty, enforced in `build.rs`, and the failure was checked by removing the line rather than assumed. An operation with no attribute is invisible to a panel that groups by them; a build that stops costs ten seconds, a control nobody can find costs more. The vocabulary is closed for the same reason: a typo would otherwise invent a category holding exactly one operation, which looks like a deliberate one until somebody counts. Six tests over the real chain, including the hand-written operations that `build.rs` never sees and so cannot check.	2026-08-22 10:10:00 +02:00
dtourolle	e7dbdeb21f	Keep the overlay on the photograph when the view moves ... The overlay is a source-space picture; the canvas beside it shows whatever the crop, the zoom and the pan selected out of that same space. Drawn whole it stayed frame-sized while the photograph moved underneath, so zooming in left a map of the whole picture stretched over a detail of it. It now reports the visible rectangle as a clip, which the compositor applies for nothing. Resampling on the CPU instead would mean rebuilding a megapixel image on every frame of a drag, and putting it on the GPU would add a second texture to keep in step with the view. Pushed from the render path rather than the panel's sync: a pan changes no mask and no row, so nothing else needs to run, and rebuilding the row models on every frame of a drag would be waste. Straightening is handled by rotating the image. A quarter turn or a flip permutes the axes and a clip rectangle cannot say that — noted where it happens rather than left to be discovered. The proper fix is to run the overlay through the same shader prologue the photograph goes through, which is the right answer and a larger one than this. Four tests, and the one that matters asserts the clip *narrows* when zoomed — which is precisely what it failed to do.	2026-08-22 08:53:02 +02:00
dtourolle	37f63edbf9	Take the watershed out of the product path ... It does not work on a photograph, so nothing should offer it. `Segmentation` is now one model pass and what it recognised: no region field, no merge tree, no label upload, no granularity slider, and no readback of the whole proxy to build a graph that collapses. A click means "the object under the cursor". The region-selection path went with the hierarchy it indexed — including the shift-click add/subtract, which has no meaning for a whole object and would have been a modifier that silently did nothing. The passes, the hierarchy and the semantic prior stay in `dr-gpu` and `dr-segment`, tested and documented. It is the *merge criterion* that fails — the saddle is the minimum gradient along a boundary, so one weak pixel merges two regions and real gradient noise puts a weak pixel on every boundary. That is one function to replace, and the evidence for replacing it is worth keeping. What is gone is the wiring, the option, and the control that offered a user a choice with no outcome. `MaskSource::Regions` remains in the pipeline: it is tested, it round-trips through the sidecar, and a stored layer that names regions must still load and be reported stale rather than failing to parse.	2026-08-22 08:39:17 +02:00
dtourolle	6163b63895	Put the edge controls where the mask is ... Feather, falloff, grow/shrink/close/open and their amount, on the selected layer. All four read the one distance field, so all four are live — nothing recomputes except a compound morphology, and the session keys on that separately so a feather drag rebuilds nothing. Shown only for sources that go through the distance field. A gradient carries its own falloff in its geometry, and offering a second one would be two controls fighting over the same edge. Picking an operation seeds a small amount if none is set. Selecting "Grow" and seeing nothing happen would read as a broken control rather than as a radius of zero.	2026-08-22 08:39:17 +02:00
dtourolle	ec713585a5	Measure the distance to the edge, and get four controls for one transform ... Feathering, growing, shrinking, closing and opening are the same number read differently. With the signed distance from the boundary in hand, dilation is the set where d >= -r, erosion where d >= +r, and a feather of any shape is a function of d. So the field is computed once and the controls are arithmetic on it. The **field** is what reaches the GPU, not a finished alpha, and that is the point: growing a mask or changing its falloff then costs a uniform upload and no recomputation, which is what makes them live controls rather than ones that stall on every drag. Only closing and opening rebuild, because after the first threshold the shape has changed and the old distances describe the old one. Exact Euclidean, via Felzenszwalb's separable transform — not a chamfer approximation, which leaves a mask visibly octagonal once grown more than a few pixels. A test asserts the diagonal is √2 rather than 1 or 2. It runs on the CPU, which ARCH §5.4 forbids for masks. The rule is about brush lag — a stroke rasterised per frame — and this is a different operation: once per mask edit, on input the model already produced here, producing a field the GPU then samples for free. What it buys is exact determinism, which matters because masks reach the sidecar as indices and a field that varied by vendor would mean a mask meaning one thing on the desktop and another on the phone. The half-pixel in `signed_distance` is not a detail, and a test caught it. Measuring to the nearest opposite pixel *centre* puts the smallest magnitude at 1 either side, so the boundary is nowhere and **eroding by less than a pixel removes nothing**. A control whose first notch does nothing is a broken control. Half a pixel off each side puts the boundary where it physically is, and eroding by 1 takes exactly the outermost ring. Every falloff curve is 0.5 at the boundary by construction, asserted for all five: changing the curve should change how the transition looks and never where it sits.	2026-08-22 08:39:17 +02:00
dtourolle	ee10097435	Mask the subject the model found, not the regions underneath it ... The watershed hierarchy does not survive a photograph, so local masking stops depending on it. A layer can now be one recognised object, and the object's own coverage is the mask. `Options::watershed` defaults off. It costs ~80 ms plus a full-resolution readback to produce a ladder that collapses, and paying that on every photograph buys a control that misleads. Kept switchable rather than deleted: the passes and the hierarchy are correct in themselves and it is the merge criterion that fails, which is a change to one function. Masks now rasterise in **source** space at proxy resolution and are sampled by the composed shader after the framing map. That fixes a real bug: they were rasterised in output space, so zooming slid the photograph underneath a mask that stayed pinned to the viewport, and cropping moved every adjustment to a different part of the picture. Doing it this way also leaves the framing map in exactly one place — a second copy in the mask shader would have been a second thing to keep in step, failing only when straightened. A subject is stored as identity, not pixels: the mask is megabytes and is reproducible by running the same model over the same image, so the sidecar carries the index, the class and the score, and the session carries the pixels. The class is there to be checked — if instance 3 comes back a "car" where it was a "dog", something changed and the layer is stale rather than silently masking the wrong thing. The overlay now draws instances and is transparent everywhere else. The region version covered every pixel and so hid the photograph it was drawn over; the question it exists to answer is whether an outline follows the subject, which you can only answer by seeing both. `examples/local.rs` is the worked example: subject in colour with the rest monochrome, and the subject lifted out of its background. Run on a 5472x3648 CR2 it finds two people and two cars, and the colour-pop keeps her hat and hair while the wall and grass behind go grey.	2026-08-22 08:39:17 +02:00
dtourolle	9b4f0815e5	Show the regions, click one, and adjust it ... The local panel sits above the adjust panel because it decides what those sliders act on; below it, a photographer would set an exposure and only then discover which scope it landed in. Selecting a layer re-scopes the existing controls to that layer's chain — there is no second set of sliders, and there must not be, or every operation added to `ops/` would need a local twin. The overlay is drawn over the canvas rather than blended into the render, because it is a diagnostic and not an edit: it must not reach the histogram, an export, or the texture handed to the compositor. Nearest- neighbour always — the map's values are *names*, so smoothing between region 4 and region 9 invents a colour belonging to neither and softens exactly the edge the overlay exists to show. Picking gets its own touch area above the pan handler. Panning wants press-drag-release and picking wants a click; interleaving them in one handler is how a drag ends up selecting a region the user was scrolling past. Shift is tracked as window state because a TouchArea's click carries no modifiers. Three states a layer can be in are worth distinguishing, and each has a different remedy: stale needs re-segmenting, "no adjustment yet" needs a slider moved, and the ordinary case needs nothing said. A bare selection renders nothing and looks identical to a broken mask, which is the first thing a new user will hit. Known rough edge, commented where it happens: segmentation blocks the UI thread for about half a second. Moving it to a worker needs the develop session — GPU resources behind a RefCell shared with every callback — to be reachable from another thread, which is a restructuring rather than a change to the call. The button says "Finding regions…" first so the stall is announced rather than looking like a hang.	2026-08-22 08:39:17 +02:00
dtourolle	5ecb35864f	Put the region map behind the sliders that were already there ... A mask layer holds a real develop chain, so the develop panel can edit one with no new controls: select a layer and the same sliders read and write its chain instead of the graph's. An operation declared in `ops/` tomorrow becomes locally adjustable by existing, which is the payoff for making a layer a chain rather than a handful of special-cased parameters. `segmentation.rs` joins the two arms into the one thing the view needs. The model reads the image through a neutral graph rather than the edited one, so a segmentation survives an exposure change instead of being invalidated by every slider. Arm B failing is not fatal: a missing or unreadable model leaves a working watershed map, because refusing to segment at all would trade a working feature for a strict one. The overlay colours groups by a golden-angle walk over hue. Deterministic rather than random, so a region keeps its colour across a level change and the eye can track it; boundaries drawn black over the fill, because two adjacent groups landing on near hues read as one region and telling them apart is the whole reason to look at it. Clicking the photograph creates the layer if none is selected — that is how a local adjustment begins, and making the user press "add layer" first would be a step with no decision in it. Shift-click extends, and clicking a region already selected removes it, so one gesture both adds and corrects. `segment-readback` is a new dr-gpu feature and not a loosening of `readback`. The region-graph transfer is once per image on a worker; the one AC-8 forbids is per frame in the render loop. Sharing a switch would have forced a build wanting local masking to unlock the other. F3 still stands and the feature name says so.	2026-08-22 08:39:17 +02:00
dtourolle	caf61d41a5	Re-thumbnail a photograph from its own edit ... A thumbnail comes from the file's embedded preview, which is the camera's idea of the photograph and knows nothing about what has been done to it since. So a frame could be cropped, turned upright and pulled two stops back, and the grid would go on showing the original — making the library, where a photographer spends most of their time, the one view in which an edit is invisible. The render is the framed output, not the sensor: `output_size` is what a crop, a quarter turn, a flip and a straighten all act on, so a thumbnail taken from the raw frame would be the right pixels in the wrong shape and still the wrong way up. It is the same path an export takes, at a size the store wants rather than at full resolution, and always sRGB — this is a JPEG in a shard that syncs between devices and is drawn as a cell, not a file anyone is finishing. Both size classes are replaced. The store keys on the class, so refreshing only the one the grid happens to be drawing leaves the other holding the unedited preview, and a zoom across the boundary would show the edit undoing itself. Each is rendered rather than downscaled from the larger, which would be a second and worse resampler than the GPU has already applied. It runs on the way out of develop, after the sidecar write is queued and never instead of it — the edit is what must not be lost, and a render that failed must not take the save down with it. Two cases are worth the work: an edit made in this sitting, which `can-undo` records even when it ends back at neutral, and an image opened with an edit already in its sidecar and left untouched, whose cached thumbnail has never shown that edit at all. A neutral image nobody touched fails both and costs nothing. Not covered: a batch paste onto a selection, which deliberately never opens a session — there is no rendered frame to take a thumbnail from, and downloading forty RAWs to make forty is exactly what that path exists to avoid.	2026-08-21 22:48:10 +02:00
dtourolle	cfff6a3302	Merge branch 'zero-copy-display' ... # Conflicts: # core/dr-gpu/src/adjust.rs # ui/dr-ui/src/develop.rs	2026-08-17 10:04:14 +02:00
dtourolle	9fc8721fa8	Merge branch 'histogram' ... # Conflicts: # ui/dr-ui/src/lib.rs	2026-08-17 10:00:27 +02:00
dtourolleandClaude Opus 5	0233df4bf2	See what the highlights are doing: a live histogram (FR-DSP-7) ... Exposure, blacks and whites were set by eye. Nothing said a highlight had blown — the canvas shows white where a channel is at 250 and white where it is at 255, and the difference is the whole question. **Counted on the GPU, not on the readback.** There is a full frame sitting in CPU memory on every canvas update right now — `AdjustPass::read_output`, the bridge spike S1 removes — and walking it would have been thirty lines and no shader. FR-DSP-7 states the mechanism and not just the feature: "these derive from a GPU-side reduction into a small buffer. Per-frame CPU readback of image data is prohibited." A histogram founded on the bridge would be correct today and deleted by S1, and would meanwhile be the reason the bridge could not go. What crosses the bus here is 4104 bytes whatever the image size. The reduction tallies into workgroup memory first and merges once per workgroup. A photograph is not noise: a clear sky puts tens of thousands of adjacent pixels in one bin, and contending for that single global atomic serialises the dispatch. **On the settled frame only.** `render_now` already knows whether a gesture is still moving — `draft` is the flag `redraw` derives from `was_coalesced` — so the dispatch and its transfer happen once when the slider stops rather than on each of the forty frames a drag emits. Nothing is lost: a histogram flickering past under a finger is not a reading anyone takes. FR-DSP-7 requires exactly this, that it not extend the FR-DSP-3 frame budget. Luma is weighted in 8.8 fixed point — 54, 183, 19, summing to 256 exactly — rather than in floats. Not thrift: it makes the shader's arithmetic reproducible bit for bit, which is what lets the test below be an `assert_eq` against a CPU count rather than a tolerance. ARCH §6.13's line about integer state, applied where it happens to also be free. **What the numbers were checked against.** A flat frame must put all 4096 pixels in one bin and one only. A 256-wide ramp must occupy every level with exactly the same count, which is what catches an off-by-one in the quantisation — a `floor` where a rounding was needed shifts the whole photograph one bin left and looks like nothing at all. And a 101x37 frame of seeded pseudo-random pixels — deliberately not a multiple of the 16x16 workgroup, so the edge tiles run off the image — is compared slot for slot against a second, obvious CPU implementation. Exact equality, no tolerance. The CPU version is a deliberate reimplementation rather than shared code: the bugs worth catching here are ones shared code would commit identically on both sides. Above that, the presentation arithmetic is unit-tested headless, because it is where a wrong answer is invisible. A histogram of the wrong shape looks exactly as plausible as one of the right shape. So: 64 columns because it divides 256 and an uneven fold draws an even ramp as a comb; the peak excludes the end columns, or a night scene scaled against its own black spike is a flat line with no information in it; heights are clamped into the plot; and "0%" is kept distinct from "<0.1%" and from "—", since an indicator reading "clipped" over a figure reading "none" is a panel contradicting itself. Clipping counts a *pixel* with any channel at an extreme, not a channel. Any, because a blown red has no gradation left in it however much green and blue still hold — and it is the saturated highlight, the sunset and the red jersey, that clips first and recovers worst. Per pixel, because counting channels can report 200% of a frame clipped, and a percentage above 100 is a readout nobody trusts again. Two affordances for it, which NFR-A11Y-3 asks for: a bar standing at the end of the plot the tones are piling against, and a figure saying how much. Either alone reads. The panel sits directly under the capture metadata and above every control, because it is what the controls are judged against. It is hand-built rather than generated, and ARCH §4.3a is untroubled: a histogram is not an operation — no parameters, changes nothing, answers a question rather than asking one — and nothing in it reads a parameter out of a descriptor. Three plot colours and a neutral luma trace join the palette. That is the swatch's exception rather than a second one: a per-channel histogram has to say which channel, and no achromatic treatment distinguishes red from blue, so the hue is data exactly as the image beside it is. Held well back from full strength for the reason the theme preamble gives. The bounded, non-parking map wait moves out of `AdjustPass` into `readback::await_mapping`, shared with the histogram's transfer. Thirty lines of load-bearing reasoning about frozen interfaces and lost devices, and two copies of it would have drifted. The histogram describes the frame on the canvas, so it is in the output colour space FR-DSP-7 asks for, and when zoomed it describes the visible region — a photographer inspecting a highlight at 4x is asking about that highlight. A device that cannot build the reduction loses the histogram and keeps the photograph. Still to do for FR-DSP-7: the pixel colour readout under the cursor. 324 tests pass, clippy and fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 09:55:40 +02:00
dtourolleandClaude Opus 5	cf8f5b632f	Show the develop frame itself, instead of a photocopy of it ... The oldest open item in the project (ARCH §6.1, spike S1, AC-8). Every frame in develop was read off the GPU into a `SharedPixelBuffer` and handed back to Slint to upload again: ~7 ms at 4K against a 0.28 ms compute pass, 96% of the frame spent carrying pixels to the CPU and back so they could be drawn where they already were. Slint 1.17 will adopt a `wgpu::Texture` directly, and the whole of what that needs is arrangement rather than code. **One device, made before the window.** A texture belongs to the device that allocated it, so the compute passes and the compositor cannot each open their own. `GpuContext::new_shared` opens one and hands back the instance and adapter alongside it; `dr_ui::shared_gpu` gives all four to `BackendSelector::require_wgpu_29(WGPUConfiguration::Manual { .. })`. That call has to come before the first window, because creating one selects a backend for you — which is why the GPU is now opened at the top of `run` rather than two hundred lines down beside the other controllers. dr-gpu still names no UI type. It hands out raw wgpu and does not ask who is compositing (ARCH §6.5a). **Vulkan only on the shared path**, where headless keeps its GL fallback. wgpu's GL backend reaches its display through EGL at instance creation, and before a window exists there is no display handle to give it — so a GL instance cannot later produce the window surface Slint needs from it. A machine with no Vulkan gets no shared device and browses without develop, which is the same degradation as no adapter at all. **`renderer-femtovg` becomes `renderer-femtovg-wgpu`.** The old one is FemtoVG over OpenGL and cannot be handed a wgpu texture at all. It is not kept alongside as a fallback: FemtoVG-over-GL has no branch for an imported texture, falls through to "render this image into a buffer", gets nothing, and draws nothing — a blank canvas with no error, which is worse than the failure it would be papering over. The consequence is stated plainly in the manifest: the desktop app now needs a working wgpu adapter to open a window. **Two output textures, not one, and this is the part that is not obvious.** Slint repaints when the image property *changes*, and it decides that with `PartialEq` — which for two images over the same `wgpu::Texture` says "unchanged". A pass that reused a single target would have rendered every slider move correctly on the GPU and shown none of them: right, and invisible. `AdjustPass` alternates between two targets, so consecutive frames are genuinely different values. It also settles the read-while-write question that one queue was already answering. `RENDER_ATTACHMENT` is added to both render targets. Neither pass uses it; Slint rejects an imported texture without it, on the reasoning that a compositor handed a texture may need to draw into it. **`AdjustPass::read_output` is deleted rather than gated.** It and `export_pixels` were the same transfer under two names, and the comments explaining why they were separate are the point of the whole criterion: reading pixels back to *display* them is the defect, reading them back to *encode a file* is the only way a file is made. The display twin is now gone outright, which is stronger than a feature flag — it cannot be turned back on. `export_pixels` is untouched and still ungated. The `readback` feature comes off dr-ui, darkroom-desktop and darkroom-android; it stays in dr-gpu, where it still gates `RenderTarget::read_pixels` and the segmentation field readback. `examples/develop` moves to `export_pixels`, which is honest — it writes a PPM — and so no longer needs the feature. Four tests, each named for what it protects and each of which fails without a screen if the property it guards breaks: - the adjust target satisfies every condition Slint's import checks, asserted in the crate that owns the descriptor, because a descriptor that drifts fails at runtime on a real display and nothing else would notice; - consecutive renders are different textures, and the third is the first again, so the alternation is a rotation and not an allocation per frame; - the develop canvas has no CPU pixel buffer and does have a wgpu texture — AC-8 itself, in the terms Slint uses; - consecutive frames compare unequal as `slint::Image`, which is the property the repaint actually depends on. The zoom test's readback moves into the test module. It has to: there is no library function that copies a displayed frame to the CPU any more, and that is the point — the round-trip now exists in the test binary and nowhere a shipping build can reach. **What is not proven.** No GUI was run. What is verified is that the texture satisfies the import contract, that the import succeeds, that the canvas is a texture rather than a buffer, and that consecutive frames are distinguishable. What is unverified is everything that needs a display: that Slint's FemtoVG wgpu renderer adopts the Manual configuration on a real surface, that the picture appears the right way up and the right colour, and the frame timing that motivated the whole exercise. Android is untouched by testing — the android backend routes a WGPU29 request to Skia, whose wgpu surface does handle imported textures, but that is read from the source, not observed. 56 dr-gpu tests and 255 dr-ui tests pass, clippy clean under `-D warnings`, fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 09:54:00 +02:00
dtourolleandClaude Opus 5	4b36ca66aa	Render an export into the colour space its file will claim ... The colour-managed export branch left one call site deliberately unfixed, and this is it. `render_for_export` composed with the default sRGB shader, so a Display P3 export failed with an accurate error rather than producing a mislabelled file — the right way to leave a half-finished path, and no way to leave it. The space is chosen at render time because that is the only time it can be: the conversion happens in the shader, before the clip to 0..1, so by the time pixels reach an encoder they are in exactly one space and the only honest thing left is to label them. `Frame::in_space` carries which, and a mismatch between what was rendered and what was asked for stays a typed error. Also regenerates the traceability matrix over the four merged branches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 09:34:57 +02:00
dtourolleandClaude Opus 5	7f524d2fd0	Give a mis-drag a way back ... Develop edits now save themselves to a sidecar the moment you leave the image, so until this there was no way to undo one — the mistake was persisted and the only recourse was to remember the old number. The history is a stack of snapshots, because the edit graph is already plain data: `Preset::capture` reduces it to what differs from default and `Preset::apply` puts it back, so undo is those two calls and nothing else. A command object per action, with an inverse beside it, would have been a second thing every operation had to register — and operations are declared in YAML precisely so that a new one needs no code written for it. A snapshot cannot fall behind them. The interesting part is coalescing. A slider drag emits an event per frame and must be one step, not forty. Nothing in the interface reports a gesture boundary — the same wall the render coalescing hit, and it is answered the same way rather than by threading a "finger is down" out of every slider, curve point and crop handle. What stands in for the boundary is the control plus recency: changes to the same control within 700 ms amend one step. Which control is "the same" is asked of the graph, not listed: an operation whose declared presentation claims a parameter is one where a single gesture moves several — a curve point carries an x and a y — so those coalesce as one widget. Nothing in the history names the tone curve. The compromise, and it is a real one: a control let go of and picked up again within the window is one step rather than two. Buying the other answer costs a gesture-boundary signal on every control, which is more surface than the difference is worth. The stack is bounded at 64 states for NFR-RES-1 — a develop session stays open for hours. Sixty-four rather than a byte cap: what is being bounded is steps a photographer would want back, and a byte cap would give the elaborate edit the shallowest history, which is exactly backwards. The session owns its history and every mutator records into it, so the callbacks in `lib.rs` cannot change the edit and forget to — with a dozen generic callbacks that would have been one press of undo away from wrong every time a control was added. Opening a photograph makes its stored edit the floor rather than a step: it is not work done in this sitting, and an undo reaching behind it would discard a previous session's edit and then save that on the way out. Not yet done, from FR-DEV-5: history is per-session and in memory, and there are no named snapshots. What mattered was that a saved mis-drag had no way back at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 09:03:34 +02:00
dtourolleandClaude Opus 5	2330ed25e9	Let a grouped slider be dragged, by flattening the panel that drew it ... White balance, highlights and shadows, and the mixer took a press, jumped once, and went dead under the finger. Exposure and contrast dragged perfectly — which is what made it read as a slider bug rather than a layout one. The panel nested. A group's head row drew the *whole* group, repeating over `row.group-len` and indexing back into `root.rows`; every other row drew nothing. So the inner repeater's model was read off the head row, and depended on that row's identity. Moving any parameter in the group rewrites that row — its own value changed, or `group-modified` flipped for its neighbours — which re-evaluated the repeater, rebuilt its items, and destroyed the `TouchArea` holding the live gesture. A lone parameter had no inner repeater, so the five single-parameter operations were never affected. Now each row draws only itself, so an update touches one control and nothing structural. The facet heading comes off `starts-facet`, which Rust already marks on the first row of a run, and the group heading is drawn by the row that heads it. It also retires the old hazard of a head row building every control in its group — thirty-six live TouchAreas behind the mixer's twelve visible ones. The same identity hazard reached the rows themselves through `ModelRc`, which compares by identity rather than contents: a fresh empty model per row per call made every row differ from itself, so `sync_rows` rewrote all of them on every event. `points` now shares one empty model as `choices` already did. Both are held by tests, because the failure is invisible in a still — every value is right and the panel looks perfect. Curve rows are excluded: their points model carries live coordinates, is rebuilt by design, and `sync_rows` writes values through the existing model rather than swapping it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 07:23:01 +02:00
dtourolleandClaude Opus 5	cb1d2be240	Choose the export folder by walking the server, not by typing it ... The destination for a Nextcloud export was a text field. Nobody recalls the exact spelling of a path three levels down, and getting it wrong does not fail — `create_dir` makes whatever was typed, so a misremembered folder becomes a new one at the root and the exports are somewhere nobody looks. So it is picked the way the library root is picked, using the same `FolderBrowser` model the launch screen drives: up, into, and "use this folder", confirming the folder currently *shown* rather than one selected in the list. Same rule in both places, so the phrase means one thing. The model is shared; the worker is not. `settings_ui::spawn_folder_list` is a near-twin of the launch screen's, because that one reaches into the `LaunchController` for its session and reports onto the launch screen's error line, while this one is handed credentials and writes to the settings page. Factoring them together needs a function taking both controllers or a trait implemented twice to abstract two call sites — more machinery than the twenty lines it saves. What matters is shared already: navigation behaves identically because both drive the same model. The callbacks are wired in `lib.rs` rather than in `settings_ui::wire`, because listing a remote folder needs credentials and the settings page holds no session on purpose — it is reachable before a library is opened and must not depend on one existing. With no account the picker says to sign in first, rather than showing an empty list that reads as a server with no folders. Details that are decisions rather than accidents: the picker opens at the library root rather than at whatever half-typed path is in the field, which would list nothing and look broken. The listing area is a fixed 180px, since a folder with sixty children would otherwise push the rest of the settings page off the bottom. "Up" is disabled at the root rather than hidden, so the row does not jump as the user navigates. A failed listing leaves the picker open on the folder it was showing — where the user had got to is not something to discard over a dropped request. And the chosen folder saves immediately like every other setting on a page that has no Save button. The poll timer lives on the controller for the reason `LaunchController` keeps its own there: a `slint::Timer` stops when dropped, so one local to the function that starts it would be collected before the listing arrived. Carries in-flight work from a parallel session — a segmentation pass in dr-gpu, a sidecar cache, and the develop panel's continuing changes. 1020 tests pass, fmt clean. One clippy warning remains and is not mine: `sidecar_cache::dir` is unused while that work is in progress. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 07:12:01 +02:00
dtourolleandClaude Opus 5	e00c99b864	Let a photograph leave: an export button, and a cache to leave from ... dr-export could turn a frame into bytes and nothing could ask it to. This is the button, and the place the bytes go. **Everything is staged first.** An export bound for the server is written to a local outbox and uploaded afterwards; offline is not a special case, it is the same path with a drain that finds the server absent. Doing it the other way — upload directly, stage only on failure — makes the failure path the one that is rarely exercised and always broken, and a network drop mid-batch leaves some exports existing and some not with nothing recording which. Staged first, an export is finished the moment it is written and the upload is a promise kept later. The outbox sits beside the catalog rather than under the cache. dr_catalog's cache already draws that line: passive entries are a convenience and go under LRU, pinned ones are a promise and never do. An export awaiting upload is a promise — the user was told it succeeded — and sweeping it for disk would destroy the only copy. Bytes are written before the destination record, so a kill between the two leaves an orphan the drain ignores rather than a record pointing at nothing. The status line says "Queued for Exports/2026", never "Exported to Nextcloud", until it has actually landed. There is a test asserting that wording, because the tempting shorter sentence is a claim the app cannot keep. The drain runs on the sync pass, before the shards: a thumbnail shard can be rebuilt from the originals and the catalog is an index, but a queued export exists nowhere else. `DevelopSession::render_for_export` renders the framed size rather than reusing the frame on screen, which is deliberately viewport-sized (FR-DSP-1) — encoding that would hand the user a soft, screen-sized file with nothing to say anything had been lost (FR-EXP-9). One compromise, recorded rather than hidden: the export runs synchronously on the UI thread, so the window is unresponsive for the few hundred milliseconds a full-resolution render and encode takes. Moving a DevelopSession and its GPU pass to a worker is a larger change than one button earns, and it is batch export that makes the wait intolerable rather than merely noticeable. Still missing: the Nextcloud folder *picker*. The destination is typed into Settings for now. `FolderBrowser` in launch.rs is already the reusable model for it — it browses a remote tree and nothing about it is specific to choosing a library root — but wiring it into the settings page needs a listing worker and browser UI there, which is its own piece of work. Carries in-flight work from a parallel session — presets, the develop copy and paste, and the node schema's `presentation` and `enum` support. One misplaced callback in settings_ui.rs is moved from `render` to `wire`: registered in `render` it borrowed a `&SettingsController` into a 'static closure and would not compile, and that file's own docs say render pushes properties while wire connects callbacks. 992 tests pass, clippy and fmt clean. Traceability 48.3% -> 51.0%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-16 23:58:17 +02:00
dtourolleandClaude Opus 5	69b12e327f	Let framing say it wants the canvas, instead of the panel knowing ... The generated panel opened with a special case: if op.id == dr_pipeline::framing::ID { continue; } and a paragraph explaining that framing's eight parameters are eight bad controls — four crop edges you would have to type coordinates into, a "rotate" slider running 0..3, two switches — so `GeometryPanel` presents them as the gestures they are instead. Every word of that is true, and none of it was the frontend's to know. It is a fact about the operation, and ARCH §4.3a is explicit that a frontend deciding things by *naming a stage* is the boundary being crossed: a second frontend would have had to learn the same special case, and nothing in the capability output said why it existed. Framing now declares a `Presentation` preferring `WidgetKind::CropOverlay`, with a demand of two-dimensional dragging and — deliberately — no precise pointing, since FR-UI-7 grows the handles to the modality and a crop is forgiving. The widget owns all eight parameters rather than only the rect: a frontend taking this on takes the whole framing control surface, and leaving rotation and the flips behind would scatter them into the generated panel underneath a crop control that already exists. The panel's rule is now general. `supported` answers whether a kind is implemented *anywhere* — drawn in the panel like the tone curve, or hosted on the canvas like the crop — and `is_on_canvas` settles which afterwards, so the two cannot disagree about the same kind. Any stage preferring an on-canvas widget is skipped, with nothing named. A frontend that implements neither still gets the eight sliders: tedious, complete, and the guarantee the whole hint mechanism rests on. **The tests were asserting against the wrong thing.** `rows_of` was a hand-written simulation of the row generator, complete with its own copy of the framing skip, so the suite was checking a second implementation kept in step by hand. It was not in step: giving framing a presentation changed the real panel and the simulation disagreed, which is exactly how a green suite hides a regression. It now calls `rows_from`, and `rows_of_unfiltered` is gone. `framing_is_not_generated_as_sliders` survives but asserts by routing rather than by counting — no row may carry framing's capability index — so it cannot be satisfied by two miscounts cancelling out. Alongside it, an invented stage preferring a widget this frontend lacks, falling back to one the canvas hosts, must also be skipped: if that ever needs a name added to pass, the special case has grown back. Not included: moving the crop overlay's markup out of app.slint into controls.slint. It is cosmetic next to the above and app.slint is in another session's working set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-16 23:29:23 +02:00
dtourolleandClaude Opus 5	0a331c717e	Give the controls a vocabulary, and let a node ask for one ... widgets.slint set the rule — screens consume components, and a bare `Theme.*` at a call site means a component is missing — and it set it for chrome only. The controls never got the same treatment, so they were written wherever they were first needed and copied from there. **The slider was private to the develop panel.** `SliderTrack`, with the fifty-line preamble explaining how it wrests a drag away from a Flickable, lived inside adjust.slint and no other screen could reach it. It shows: export quality is a 1-to-100 value, and the settings page offered a free-text box for it, with the range written in a hint and enforced nowhere. `to-float()` answers 0 for anything it cannot parse, so a typo saved a quality of 0 and the page displayed the 0 back as though it had been asked for. The tick-box was written twice, in launch.slint and settings.slint, from the same 18px box and the same handler; the second carried a comment deferring the lift until a third caller appeared. The label-and-hint header was written three times inside settings.slint alone. controls.slint is the input layer beside widgets.slint's chrome layer, and the constraint that makes it reusable is that **nothing in it knows about `ParamRow`** — that struct is the develop panel's flattening of the capability model, and a control that imported it could only ever be used by the develop panel. The primitives take plain numbers; the ParamRow-shaped wrappers stay in the panel that owns the model. 658 lines came out of the three screens. `SliderRow` is the slider-plus-number-box ARCH §4.3 names as the pointer presentation of a bounded scalar, and quality is its first adopter. It commits on gesture end rather than on every movement, because the settings page saves to disk on change and a two-second drag is a couple of hundred writes where a text field committed once. The develop panel keeps the live stream — that is what its pipeline is for — so `SliderTrack` now reports both. **The other half is the descriptor.** FR-DEV-3a and ARCH §4.3a already specify more than was built: an ordered preference list of widgets rather than one, the demands a widget makes, and kinds beyond scalar and bool. - `Presentation.widgets` is now a list, walked by `choose`, falling back to plain sliders. Falling off the end is not an error, and there is a test asserting an operation asking only for an unimplemented widget still yields one control per parameter. - `WidgetDemand` carries what a widget inherently needs — two-dimensional dragging, precise pointing — and no pixels, breakpoints or platform names. - `WidgetKind` grows to the specified set. There is deliberately no `Colour` *kind*: a colour is three numbers, and a value type that is not an `f32` would reach through the graph, the uniform block and the sidecar format to buy what `ColourWheel` over three scalars already describes. Every widget here is a hint over ordinary scalars, which is what keeps the fallback honest. - `ParamKind::Enum` is the one new shape, and it fits because a variant index is exact in binary32. `kind: enum` with a `variants:` list works in `ops/*.yaml`, so a node declaring one gets a segmented control with no UI file edited — which is the promise ops/mod.rs already makes. The panel's dispatch was duplicated: a lone parameter and a grouped one each wrote out their own list of kinds, so `enum` would have had to be added twice and a kind added to one would appear or vanish depending on how many parameters its operation happened to declare. `ParamControl` is now the only such chain. `rows_from` is free-standing rather than a method, which is what lets the FR-DEV-3c acceptance test requirements.md asks for actually be written: an operation the frontend has never heard of, appearing in a generated panel, with no GPU in sight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-16 23:21:35 +02:00
dtourolleandClaude Opus 5	9b2ee0d0eb	Show the colour mixer as three runs of twelve, each row a colour ... The mixer was thirty-six sliders reading "Hue / Sat / Lum" twelve times over with nothing saying which band any row belonged to. The identity was there all along — the descriptor declares param.mixer.orange.sat and BANDS carries orange at 30° — and was discarded on the way out: labels.rs had no mixer entries, so every key fell through to a derived label that yields the bare channel name. A parameter can now say which aspect it adjusts and which subject it adjusts it on, with the subject's hue where the subject is a colour (descriptor::Facet). That is data about what the operation does, not a layout: the mixer genuinely weights pixels around 30°. What to draw from 30°, and in what order to stack the runs, stay in dr-ui (ARCH §4.3a) — develop.rs brings rows sharing an aspect together and marks the first of each, and adjust.slint names the run once and draws a swatch, a track and a readout on one line. Grouped by channel rather than by band because an edit is almost never "everything about orange"; it is the saturation of the greens, made by comparing one channel across neighbouring bands. Twelve band sections put those twelve rows in twelve different places. The swatch is the label, which is what makes twelve rows fit where four did. The band name is not lost: it is the row's accessible label, so the control is not colour-only, and labels.rs is where the mapping is written down — including chartreuse as "Yellow-Green" and spring as "Blue-Green", since nobody hunting foliage scans a list for "Spring". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-16 16:14:13 +02:00
dtourolleandClaude Opus 5	489465faf0	Show the photograph the way it was taken ... Nothing read EXIF orientation, so every frame from a body held sideways lay on its side — in the grid, in develop, and in the read-only preview. The tag is honoured as part of *reading the file*, at the same standing as a RAW's masked-photosite crop, never as an edit. It lives as a baseline on Framing rather than as a starting value for quarter_turns, which is what keeps four things true: a sideways file opens unmodified, reset returns it to upright rather than to the sensor's scan order, its sidecar stays empty, and the rotate button still moves the image 90° whatever the file underneath it says. Framing::effective composes the baseline with the user's own turns through the group law rather than by adding turns and OR-ing flags. The naive version gets one case wrong — an odd baseline turn plus a user mirror — and gets it wrong quietly, because the result is still a plausible orientation. The composition collapses to a single permutation, so obeying the tag costs nothing per pixel. dr_decode::orientation is a header-only IFD walk, separate from metadata() for the reason the entry points are separate at all: the grid asks once per cell and must not build a rawler decoder to get one tag. CR3 and RAF fall back to the full read, being neither TIFF nor JPEG. Written down as FR-DEV-3h. Known gap: thumbnails cached before this stay sideways. The store is keyed by file and size, and its shards sync — invalidating them would have every client re-download 25 MB a shard, which is not this commit's call to make. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-16 15:37:05 +02:00
dtourolleandClaude Opus 5	03326242a1	Make the CI checks say what they mean, and format the workspace ... The Android job's "Verify minimum API level" step has never verified the minimum API level. It took the first `*.so` anywhere under the target directory, which is a host proc-macro from debug/deps — an x86-64 object built by the runner's gcc, whose .comment section cannot mention Android and so can never contradict the expected value. It now reads the artifact under the target triple, compares against MIN_API parsed from the Dockerfile rather than a second copy of the number, and fails on a mismatch. Both sides are checked non-empty first: two failed parses would otherwise compare equal and pass, which is the same silent success in a new costume. The Android image installs one SDK package per layer and keeps the output. sdkmanager is a JVM program that aborts when it cannot get memory, and the single `> /dev/null` step reported that as a bare "exit code 134" while a retry re-downloaded everything that had already succeeded. tools/ci-local.sh runs all four jobs — desktop, android, layering, traceability — against the host toolchain, which is pinned to the same 1.92.0 CI installs. Its matrix check compares regeneration against the working tree rather than against HEAD: CI starts from a clean checkout, so git's answer is the right one there and reports every local run stale here. The rest is rustfmt across the workspace, and the clippy findings that surfaced once it did: manual_contains in dr-thumbs and collections_ui, a map iterated as pairs for its keys, an index loop over a slice, and two runtime assertions on a constant now made at compile time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-16 12:02:51 +02:00
dtourolleandClaude Opus 5	cd75e5a4c6	Run the library from local data when the server is unreachable ... Also carries in-flight work that shared these files: the zoom structure-key fix in the adjust pipeline, nearest-neighbour filtering past 1:1, the timeline scrub marker correction, the 423-Locked retry in the metadata sweep, and the thumbnail size-class migration. # Offline mode (FR-CAT-9) The app previously assumed the server was reachable and treated its absence as a series of unrelated per-operation failures. A launch without a connection produced an empty grid, even with a complete catalog on disk and every thumbnail already in the shards. Reachability is now inferred from traffic the app was already making, rather than probed for. `RemoteError::indicates_offline` draws the line that makes this possible: a dead connection is offline, a 403 or a 500 is not — the server answered, so blanking the library over one forbidden file would be a worse error than the one being reported. `Reachability` turns those outcomes into a state, so a library browsing happily never issues a probe at all. Going offline takes one failure, because the user is already experiencing it. Coming back requires evidence — a completed scan or a fetched thumbnail — with a capped exponential backoff behind the manual retry, so twelve sweep lanes failing together do not schedule twelve immediate probes. What keeps working: the catalog opens even when the scan that normally provides it failed, so the grid fills from the last successful scan. Thumbnails come from the shards. Rating, flagging and collecting are catalog writes that never touched the network. What stops is opening an original that was never stored locally, and it now says so in those words instead of reporting "network error: connection refused" over a photograph. Work that is pure network is refused rather than left to fail slowly: the metadata sweep, derived sync, and sidecar writes. The sweep would otherwise spend a timeout per image across the whole library while the progress bar implied something was happening. Deferring sidecars is a real gap rather than a hidden one — a rating made offline reaches its sidecar only when that image is judged again while connected — and it is recorded as such at the call site. # The "On this device" filter A chip beside the rating filters, narrowing the grid to images whose original is held locally. It composes with the rating terms rather than replacing them, so "five-star frames I can actually edit on this train" is one filter. The predicate is SQL, like the rating terms and for the same reason: the count in the header has to agree with the cells drawn. It reads `image_cache.tier_actual`, which nothing writes yet — the next commit fills it. Until then the chip honestly reports zero. `Tier` gains an explicit on-disk encoding. The variants are ordered by generosity and the derived `Ord` invites reordering them, which would silently reinterpret every cached row; the round-trip test is what holds the two in agreement. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-11 20:36:50 +02:00
dtourolle	8ad5c86ff9	Add the library, collections, and trash views; theme from style.yaml ... The UI gains the views the catalog work was building toward: a windowed library grid with ratings and flags, the collection tree with drag-to-add, and trash with restore. derived_sync pushes thumbnail shards and the catalog snapshot to the server's derived folder. Tokens now have one source of truth. build.rs reads style.yaml and generates theme.slint into OUT_DIR, which answers every existing `import { Theme } from "theme.slint"` unchanged, because Slint resolves imports against the importing file's directory first and the include paths after. Generating into OUT_DIR rather than beside the hand-written Slint is the point: a generated file sitting in ui/ looks exactly like the files around it that are meant to be edited, and an edit to it would survive until the next touch of style.yaml — a bug that hides for weeks. build.rs fails loudly if a stale ui/theme.slint exists, which would otherwise shadow the generated one silently and make every palette change vanish with no error. The palette moves to near-neutral dark with achromatic signalling, so the accent means "modified" or "active" rather than "heading". Shared components land in widgets.slint: a token that binds several values into one concept is a component, not a row in a YAML file. Adds an optional live-style feature that makes the tokens in-out so they can be written at startup — a feature rather than the default because it stops the properties being constant-folded. serde_norway is the YAML crate: serde_yaml and serde_yml are both deprecated, and its mappings preserve insertion order, which is what lets the generated Slint keep the token ordering the author chose. Assisted-by: LLM	2026-08-09 21:11:38 +02:00
dtourolle	5365123d92	Fix the folder picker stuck on "Loading…" ... The picker set loading=true and nothing ever cleared it, because browser_loaded() was never called — my earlier edit to replace the stub silently failed to apply after cargo fmt reindented the code it matched against. The stale stub was still writing folder names into the status line, which is the list visible under the selector. Verified by grep before and after: browser_loaded had zero call sites, now two. Two related defects fixed while in there. Both timers polled their channel with `if let Ok(..)`, so a worker thread that died without sending left the screen on "Loading…" or "Connecting…" forever. Both now treat Disconnected as terminal. The first attempt at that fix introduced a bug of its own: a separate probe call consumed a pending message and discarded it, so a successful login would have vanished. The login poller now drains in one loop with disconnection as a match arm, and only reports failure when the flow had not already completed. Lesson worth recording: verify a replacement applied rather than trusting the edit succeeded. A silently-skipped replace looks identical to a successful one until the feature is used. 43 tests in dr-ui.	2026-08-09 16:01:48 +02:00
dtourolle	2ca1716a29	Make "Choose folder" an actual folder picker ... It previously fetched the folder list and threw it away into a status line — a button that looked like it worked and did not. Now it opens a browsable picker: click a folder to descend, ".." to go back, "Use this folder" to select, "Cancel" to leave the root unchanged. Descends one level per click because that is what the backend supports: Depth: infinity is frequently disabled server-side and prohibitively expensive where it is not (ARCH §8.4). The chosen root persists immediately on confirm, so it survives a crash before the library is opened. Confirming at the account root is allowed — a user may legitimately keep everything at the top level — and cancelling leaves any previous selection untouched, which a test asserts. Verified against nextcloud.tourolle.paris at both depths: 30 folders at the root, 21 year-folders inside PhotosRaw. 19 launch tests, 38 in dr-ui.	2026-08-09 15:50:40 +02:00
dtourolle	09e3043f4c	Add secure credential storage, sessions, and a launch screen ... Login now persists properly rather than through the JSON file the test harness was using. dr-plat SecretStore trait plus a Secret Service backend. Verified against the live GNOME Keyring: store, retrieve, delete, confirm-gone all round-trip. Session/SessionStore splits credentials from settings — the app password goes to the keyring (FR-NC-2), while server, login, chosen root and format selection are ordinary config. A test asserts the credential never appears in the config file. LaunchModel the launch-screen state machine, testable without a display server: sign in, approve in browser, choose folder, tick formats, sign out. launch.slint the screen itself, in its own file. Absence of a secrets daemon is an explicit degraded mode, not a silent fallback to plaintext — the screen says sign-in will not persist rather than letting the user find out next launch. Android's Keystore backend fails loudly for the same reason: a no-op store would look like it worked and then lose the credential. Two bugs caught by tests rather than by running it: - fail() after busy() signed the user out, because busy() had already discarded the session. A failed *scan* would have logged you out. Busy now carries the session. - normalise_server upgrades http:// to https:// rather than accepting it. NFR-SEC-3 requires TLS, and silently sending a credential in the clear is not a decision to make on the user's behalf. launch.slint is not yet wired into app.slint. Calling slint_build::compile twice replaces the generated module rather than adding to it, which broke the other in-flight work on dr-ui; I reverted that immediately. Wiring it needs an import inside app.slint, which is that work's file to change. 419 tests passing across ten crates.	2026-08-09 15:20:39 +02:00
dtourolleandClaude Opus 5	78e3e6b846	Add the develop pipeline: demosaic and seven raw adjustments ... Decode through display, on the GPU: black/white normalisation, Bayer demosaic, camera colour transform, and the first seven adjustment operations — white balance, exposure, highlights/shadows, blacks/whites, brilliance, vibrance, saturation. Composable shaders. Each operation contributes a WGSL fragment rather than owning a pass, and dr-pipeline fuses the *active* ones into a single compute shader. One texture read and one write per frame regardless of how many adjustments are in play, while the operations stay independent in Rust — adding one is a new file, with no central shader to edit. An operation at neutral settings contributes no code, no uniform and no branch. Uniforms are prefixed per operation so two may both declare `amount`; helpers dedupe by name from a single source of truth. Pipelines cache on a structure hash covering the op-set and its order but not the values, so dragging a slider uploads uniforms and reuses the compiled pipeline. Measured on a 24 MP CR2: 0.60 ms re-render, one pipeline compiled across ten slider positions. The UI is generated, not written. EditGraph::capabilities() reports parameters with their kinds, ranges, defaults and current values; the panel builds one control per entry chosen by ParamKind. No file in ui/ names an operation, and dr-pipeline has no wgpu dependency, so codegen is testable without a device (ARCH §6.5a). Three defects found against real files, each silent: - rawler 0.7.2's `xyz_to_cam` is all zeros — deprecated and no longer populated. The live matrices are in `color_matrix`, keyed by illuminant. Reading the old field yields no colour transform at all. - `cam_to_xyz_normalized()` returns all NaN on any Bayer sensor: it divides each of four rows by its own sum, and the unused fourth (emerald) row sums to zero. Inverting the 3x3 ourselves avoids it. `wb_coeffs[3]` is NaN for the same reason and is normalised at decode. - As-shot white balance reached the uniform block but no shader read it, so the first render of a real CR2 came out violently green. Green photosites collect roughly twice the signal of red and blue. Now applied unconditionally before any operation, with tests on ordering. Demosaic is Malvar-He-Cutler rather than bilinear: gradient-corrected interpolation at one 5x5 neighbourhood per pixel, where bilinear leaves visible zippering on any high-contrast edge at 1:1. Two of the four packed CFA constants were wrong on the first attempt, so all four layouts are asserted to reconstruct the same colour. Crop origins at odd coordinates re-phase the pattern; without that, red and blue swap. X-Trans reports GpuError::UnsupportedCfa rather than approximating with the Bayer path, which would look like a corrupt file. 206 tests, including GPU tests proving every operation and the full seven-operation chain generate compilable WGSL. Known gaps: the display path still reads back to the CPU each frame, which ARCH §6.1 forbids and AC-8 asserts against — it is gated behind the `readback` feature and waits on spike S1 wiring Slint's texture import. Curve shapes are a first draft and want tuning against real photographs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-09 11:37:58 +02:00