86 Commits

Include renames

Author	SHA1	Message	Date
dtourolleandClaude Opus 5	c396a22dfd	Paint a mask without ever rasterising one on the CPU ... The last line of FR-DEV-3, and the mask ARCH §5.4 was written for. darktable rasterises drawn masks on the CPU and users call the result unworkable; the architecture's answer is that a stroke arrives as *parameters* and the device draws it. This is that, from the model through the sidecar to the pixels — but not the finger: the canvas is somebody else's change, and this leaves it a seam rather than reaching into it. **A stroke is a swept disc along a polyline**, plus erase, radius, hardness and flow. `MaskSource::Brush` holds an ordered list of them, and the order is the mask: an erase after an add takes it away and the same pair reversed does not. Nothing about it is pixels, which is what makes a mask that costs a line of text, diffs by the gesture, and survives a crop, a straighten and an export at any size — the properties a stored raster has none of, and the same argument the region ids were chosen for. Two things keep the point count honest. While the finger is down, a position closer to the last than an eighth of the radius is dropped: a touch screen reports 120 a second, so a finger held still for five seconds is six hundred points in the same place, and simplification would only remove them once the gesture had ended — after every frame in between had drawn all of them. When it ends, Douglas–Peucker at an eighth of the radius removes what a disc that wide cannot express: a swept circle moved by r/8 moves its own edge by r/8, which is inside the soft part of any brush. Coordinates snap to a ten-thousandth of the frame on the way in *and* are written at that precision, so a round trip is exact rather than nearly exact — a file that drifts in the sixth decimal every save is a per-field merge conflict a day, over nothing. **Cost is why the strokes are not drawn by the full-screen triangle the other masks use.** A swept disc is the minimum distance to any of its segments, so a stroke over the whole frame costs `pixels × segments` and both terms grow together — the quadratic that is darktable's problem moved onto the GPU rather than solved. Each stroke is instead drawn over its own bounding box, grown by the radius, so the rasteriser never invokes the shader for a pixel the stroke cannot reach: `area(box) × segments`, which for a dab or a swipe is a small fraction of the frame. A gesture past 256 points continues as a second stroke for the same reason, since a shorter stroke has a smaller box. Add and erase are `dst + a(1 - dst)` and `dst(1 - a)`, which are exactly a source-over and a one-minus-source blend — so they are blend state, not arithmetic, and no pass ever reads the slice it is writing. That is what permits one draw per stroke at all. Within a stroke the coverage is the *minimum* distance over its segments rather than a sum: a path that crosses itself must not build up where it did, or every circle and every scribble would be blotchy wherever consecutive dabs overlap, which is everywhere. Not a distance field, deliberately. `dr-segment`'s transform documents the two conditions that make CPU work right there — once per mask edit, over input already CPU-side — and a stroke fails both: it changes while the finger moves, and its input is a handful of coordinates that never needed to be pixels. It also needs no transform, because the distance to a swept disc is closed form. A stroke is the one mask whose distance field is known without computing one. An unpainted brush layer is inactive rather than empty, which is not an optimisation: `invert` turns empty into everything, so a layer created with invert already set would apply its adjustment to the whole photograph before a single stroke was made. That is the loud, confident kind of wrong this codebase refuses everywhere else a mask can go missing, and there is a rendered test for it. The tests read pixels back off a device rather than checking that the two halves agree with each other. What they pin down is what is silent when wrong: the y flip between mask space and clip space, which a centred stroke would not notice; a bounding box not grown by the radius, which makes a tap draw nothing at all; an aspect ratio ignored, which makes a dab an ellipse on any frame that is not square; a stroke doubling back and building up; and an erase that lost its place in the order and put back paint the user had taken off. Not done here: the interaction. The canvas needs to begin, extend and end a stroke on the active layer, and `DevelopSession::rasterise_masks` still returns early without a segmentation — it takes the proxy size from one, and a brush needs no model to have run over the photograph first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-22 12:37:42 +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	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	b1433ad4a9	Give a mask an edge treatment, and find out the watershed has none worth having ... Two things, and the second is why the first matters more than expected. Mask layers gain a feather, a falloff curve and a morphology, all defined against a signed distance from the boundary rather than as separate features — one exact distance field answers "how soft" and "how far" at once, so dilation is a threshold at -r, erosion one at +r, and closing and opening are one of each in sequence. The compound pair costs a second distance field, which is why they are named rather than presented as a radius that happens to be signed. Types, defaults and sidecar round-trip only; the field itself is next. `edge-feather` and `edge-falloff`, not `feather` and `falloff`, because a radial mask already writes `feather` for the fraction of its radius it ramps over. Same word, different quantity, different units — sharing the key would have made an existing file ambiguous. The diagnostic that provoked this is committed as an ignored test, because "does the ladder land on things a person means" is the question S15 exists to answer and it should not depend on whoever still has the script. On bus.jpg it answers badly: 35,075 regions at blur 2 over an 810x1080 frame, and cutting that to 400 gives *one* region covering nearly the whole picture plus 399 noise specks. Not over-segmentation — collapse. Almost every saddle is near zero, so the merge order joins everything meaningful before it joins anything spurious, and a global cut spends its entire budget on grain. So the granularity ladder does not currently work on a photograph, and the region masks built on it inherit that. Recorded rather than worked around: the next commits move local masking onto the model's instances, where the edge treatment above is what makes a quarter-resolution mask usable.	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	94cfea4748	Keep the mask when the app closes, and when two devices disagree ... The sidecar is authoritative — the catalog is a disposable index and the RAW is never written — so a mask that does not round-trip is not a persistence bug, it is lost work. Layers get their own `[mask <version> <id>]` blocks rather than being flattened into dotted keys. A layer is not a scalar: it carries a selection, a geometry and a chain of its own, and encoding a region set as `m1.region.0 = 12` would be neither readable nor mergeable. The version uuid is repeated in the header instead of relying on the block following its version, because "belongs to whichever version appeared above me" is a relationship that hand-editing, merging and older builds each break quietly. Region ids sort and deduplicate on read rather than being trusted from the file. The mask's identity is the *set*, so two devices writing the same selection in different orders must produce the same mask rather than argue about a difference that is not one. Masks merge by layer id under FR-NC-9, which is the disjoint-survives rule the parameters already follow one level up: a layer added on the phone and one added on the desktop both survive. A layer *both* sides edited resolves wholesale to the higher revision, because half of one selection plus half of another's opacity is a layer neither person made. A remote deletion is honoured, or a mask the user removed returns on every sync. An unknown mask source is skipped rather than guessed at. Applying a newer format's mask type as the nearest one this build knows would put a confidently wrong adjustment on the photograph, which is worse than applying none. 29 new tests. The interesting ones are about silence: a maskless version clearing the previous image's layers, a bare selection persisting even though it renders nothing, and a mask naming a version that is not in the file being dropped instead of landing on whichever block was open.	2026-08-22 08:39:17 +02:00
dtourolle	c6a846a1f9	Brighten her face without touching the sky behind her ... A mask layer is an ordinary develop chain plus a rule about where it applies. Nothing in the chain knows it is being masked, so every operation that works globally now works locally and a newly declared op in `ops/` arrives with local support already done. The composer emits each layer after the global chain and before the conversion out of camera space, which is what a photographer means by "and *then* lift the shadows on her face". Op fragments write to a `c` they expect to own, so a layer block shadows it and copies the result back out through a carrier — assigning the outer one from inside is impossible precisely because it is shadowed. The fused dispatch survives: three global adjustments and two masked ones remain one shader, one read, one write. Masks rasterise on the GPU and never exist in CPU memory (ARCH §5.4). That is the whole reason darktable's brush masks lag, and it is architectural rather than tuning, so it is not a thing to inherit and fix later. The rasteriser is a render pass rather than the compute shader it obviously wants to be, and the format is why: R8Unorm is not a core storage format, so a compute path has to widen masks to four bytes per pixel — 768 MB across eight layers of a 24 MP export, against 192 MB at one byte. A colour attachment takes R8Unorm happily. The array slice comes from the attached view, so no slot uniform exists to disagree with where the pass writes. Region masks index a compacted label field rather than the watershed's raw basin roots, because a root is a sparse index into pixel space and indexing a per-region array by one would need a table the size of the image. Changing a selection then costs a few kilobytes, not a re-upload. Stored as region ids, not as pixels: diffable, mergeable per-field under FR-NC-9, and cheap in a sidecar. The ids only mean anything alongside the segmentation that produced them, so each layer carries that signature and is treated as stale rather than applied when it does not match — a confidently wrong mask being much worse than an absent one. Seven device tests render actual frames and read them back. The unit tests either side check halves that would both pass if the two agreed with each other and were both wrong; a mask sampled with x and y swapped satisfies them and fails these.	2026-08-22 08:39:16 +02:00
dtourolle	0da8271836	Let the model say what a thing is and the watershed say where it ends ... Local masking needs to know where an image's regions are. The watershed spike (S15 arm A) found the boundaries but had no idea what any of them enclosed; its coarse levels were geometric accidents. This adds the other half and the thing that joins them. `core/dr-segment` is where region reasoning now lives — the hierarchy moves out of `dr-gpu`, which keeps only the pixel passes that are genuinely shaders. The new crate is device-free and, without its default features, model-free too: 20 of its tests need neither an adapter nor 11 MB of weights. Arm B runs YOLO26n-seg through `ort`. D13 framed inference as a choice between `ort`'s C++ runtime and the pure-Rust dependency policy; that was a false choice. `ort`'s `alternative-backend` feature unlinks the C entirely and `ort-tract` supplies the API from tract, which is pure Rust. Measured before committing to it: zero unsupported operators, 420 ms for 640x640, and correct masks on bus.jpg. No NDK problem to solve, so D13's largest tolerated exception is not needed. Arm C is `prior.rs`, and it ships because the two arms fail in opposite directions. Instance membership re-weights the merge saddles, so region pairs the model believes share an object merge early and pairs straddling its edge merge late. No boundary moves — only the order in which they dissolve — which is how the result stays pixel-accurate at every level while its coarse levels become named things. Two things the spec assumed that turned out to be false, both recorded in models/LICENCE.md: there is no usable ADE20K-trained YOLO, so the shipped vocabulary is COCO's 80 subjects and *stuff* like sky and foliage must come from arm A; and tract cannot parse a dynamic-shape export, so the graph's input is fixed and tiling is the only route to more semantic resolution. Weights are AGPL-3.0, which GPLv3 §13 permits and which makes the combined work effectively AGPL. Deliberate, not accidental. They live in Git LFS, and a build script fails with an instruction rather than embedding a pointer file when the clone lacks them.	2026-08-22 08:39:16 +02:00
dtourolle	ecd6df686c	Read the defect map a raw file carries ... First step of dead pixel removal, and the one that decides whether the rest is worth building: where the map comes from. `rawler` is no help. It knows `OpcodeList1/2/3` exist — it copies them through when *writing* a DNG — but it never decodes them, and the `dng_tags` map it exposes is only ever filled by callers, never by a decoder. So the bytes are read from the IFD directly, which this module was already walking for previews, including the SubIFDs where a DNG keeps its raw IFD. `OpcodeList1` specifically: lists 2 and 3 run after demosaic and after the colour transform, so neither can carry a correction that has to happen on the mosaic. Two opcodes describe defects — `FixBadPixelsList`, which is explicit coordinates plus whole dead rows and columns, and `FixBadPixelsConstant`, which names a sentinel value rather than any coordinates and is left unimplemented until there is a stage to consume it. A half-implementation that guessed at coordinates would be worse than the absence, because it would look like it worked. Two things the tests pin down because both are silent when wrong: a point is stored (row, column) and reading it the other way round lands the correction on the wrong photosite — invisibly, on a square crop — and opcode payloads are big-endian whatever the container's byte order is, so a little-endian TIFF still writes these the other way round. An unknown opcode is stepped over using its declared length rather than abandoning the list, because a camera that corrected its lens as well as its sensor writes both, and losing the map whenever a warp is present would be losing it on most files that have one. Includes `--example defects`, because whether any of this fires is a question about a particular library rather than about the specification.	2026-08-21 23:00:10 +02:00
dtourolleandClaude Opus 5	ca833b6d2b	Give every colour band its own compiled shader ... The colour mixer emits a code block and a uniform only for the bands that are set, so which bands are adjusted is part of the shader's structure. The pipeline cache key was not: it hashed the set of *active operations*, which is "colour_mixer" whichever band that is. So a red adjustment and a blue one hashed alike. The second render was handed the first's compiled pipeline while its uniform was uploaded into a slot that shader had assigned to another band — whichever band compiled first kept acting on every subsequent move, and every other slider did nothing at all. Red is the first band declared, and the one reported as the only one working. The hash is now taken over the generated WGSL, because the source is what gets compiled and therefore is the structure. A summary of what went into it has to be kept in step with every operation's code generation by hand, and this one had fallen out of step. Values still do not enter it: no operation writes a parameter value into its source, so a slider drag regenerates identical text and reuses the pipeline, and one that did inline a value would have to recompile to be correct anyway. `each_colour_band_gets_its_own_pipeline` in dr-gpu renders a blue pixel through one pass with red set first and then blue, and fails on the old hash with the reported symptom — the blue slider returning the pixel unchanged to the byte. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-18 15:23:04 +02:00
dtourolleandClaude Opus 5	f76e024f41	Straighten a portrait frame in the frame the user is looking at ... The framing prologue built the centred position `p` by scaling with the source's aspect, then straightened, then permuted the quarter turns. On a landscape frame those are one space and it worked. Once a turn has swapped the axes — the rotate button, or a file whose EXIF tag says the camera was held sideways — they are not: `p` was measured with the source's ruler on a frame that is no longer that shape, stretching one axis against the other by (w/h)², which is 2.25 on a 3:2 photograph. The quarter-turn permutation happened to undo that stretch, so rotation alone looked right, which is how this survived. The straighten in between did not, and a rotation in a space whose axes carry different scales is a shear. `p` is now built in `frame_aspect` — the frame as the user sees it — and the permutation becomes the one place the two rulers meet: each axis divided by the aspect it is read from, multiplied by the aspect it is written to. Asserted on pixels rather than on the generated WGSL, because reading the shader and reasoning about which space `p` lives in is how the wrong formula got written in the first place: a disc, straightened by 20° on a turned 3:2 frame, must come back circular by every route to a swapped frame — the button, the tag, and the two composed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-18 15:17:54 +02:00
dtourolleandClaude Opus 5	02d629922f	Draw the date histogram over the collection you are looking at ... The timeline counted the whole library whatever the grid was showing, so opening a collection left a fortnight in Arosa as one column of a fifteen-year axis — an axis describing photographs that were not on screen. Scope the buckets and the span to the same collection and rating filter the grid uses. `timeline_range` counts `images` alone and cannot express the membership join, so the scoped query lives beside the other scoped readers in the UI and shares their descendants-of-scope rule. `catalog_span` now delegates to the same scoped reader. Zoom and scrub measured the full library while the bars were scoped, so a scrub could land on an instant the collection did not contain and send the view somewhere the user had not asked to go. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 22:15:59 +02:00
dtourolleandClaude Opus 5	b0206cbc7a	Let a sub-collection stay under its parent through a sync ... The merge inserted every incoming collection with `parent_id = NULL` and never set it on update, so the hierarchy flattened on each round trip: a collection nested on one device came back from the server at the top level. `r.parent_id` was selected and then not read. The id could not be copied — row ids are local, and the remote's integer names a different collection here, or none. So carry the parent's uuid and resolve it locally, in a second pass: rows arrive in whatever order the query returns, and a child can precede its parent. Guard the resolution against cycles. Each tree is acyclic alone, but the union need not be — we may hold A above B while the remote holds B above A — and closing that loop would make every tree walk spin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 21:55:09 +02:00
dtourolleandClaude Opus 5	5700c37016	Look before overwriting the file we are asking about ... The probe PUT its bytes first and read the outcome, which answers the question by destroying the evidence: pointed at a real sidecar it would replace an edit with the word "probe", and on success delete it outright. PROPFIND first. Permissions and status usually settle create-versus-update on their own, and a path that already exists is now reported and left alone. `--write` still forces the update test for a file worth losing, and the cleanup DELETE fires only for a path the probe itself created. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 20:40:36 +02:00
dtourolleandClaude Opus 5	18f20170b3	Ask the file, not its folder, why the write was refused ... The 403 probe read `oc:permissions` off the parent collection and warned when `W` was missing. But Nextcloud reports `W` on files and `CK` on collections, so a directory legitimately lacks `W`: the warning fired on a healthy share and pointed at a mount that was fine. Probe the file itself. Its permissions answer the question that matters, and the status distinguishes the two cases the parent could not: a 404 means the sidecar does not exist and the refusal was about creating it, while a 200 without `W` means it exists and cannot be updated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 20:38:48 +02:00
dtourolleandClaude Opus 5	6621c11ad6	Diagnose the refused sidecar: the library mount is create-only ... Ratings and edits made on the tablet queue and are then refused on reconnect, on a credential that pushes the catalog to the same library root in the same sync pass. Chased on the device, since that is where the account lives. A failed PUT now logs the server's own words, and on a 403 asks the parent what rights it reports. The answer: PUT .../PhotosRaw/2026/2026-08-03/_MG_9221.drsc -> 403 <s:exception>Sabre\DAV\Exception\Forbidden</s:exception> parent permissions: MGNVCK `M` mounted, `G` readable, `N` renameable, `V` moveable, `CK` create files and folders. Absent: `W`, update an existing file, and `D`, delete. So `PhotosRaw` is a mounted share that accepts a file once and refuses every change to it afterwards. That is the whole bug, and it is not one this side can retry its way out of. A sidecar is rewritten on every rating and every edit, so the first judgement on a photograph is written and every later one is refused — which reads as sync being broken rather than as a share missing one permission. **The fix is to grant update, and ideally delete, on that mount.** `PermissionDenied` now says so, rather than "not allowed to write here", which sent the reader to re-check a login that was working. Its test asserts the intent — points at the folder, never at the credential — rather than a phrase, so saying it better cannot read as a regression. Also adds a `put_probe` example that makes the same request from a stored session, for diagnosing this from a desktop when one is signed in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 20:19:27 +02:00
dtourolleandClaude Opus 5	71554714e7	Log why the server refused a write, in its own words ... A queued sidecar fails to upload with 403 on a credential that pushes the catalog to the same library root in the same pass. `map_status` reduces every non-success to a typed error, which is right for the application and leaves nothing to work from: a read-only share, a file access control rule and a lock all arrive as `PermissionDenied`. Sabre says which in the response body. It is now logged on any failed PUT — the URL, the status, and the first line naming the exception or message, capped at 300 characters because an error page can be a whole document. Only on failure; a success has no body worth reading. Also adds a `put_probe` example that makes the same request from a stored session and prints the reason, for diagnosing this from a desktop rather than from a tablet's logcat. It needs a session on the machine it runs on, which is why the log line above exists as well. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 20:08:07 +02:00
dtourolleandClaude Opus 5	dea826811e	Render a blown highlight white instead of magenta ... Every clipped sky came out bright pink. Measured, not guessed: developing _MG_8596.CR2 and looking at the export, the subject renders correctly and only the saturated region is wrong. A fully clipped pixel reaches the shader as (1, 1, 1) — three photosites that stopped counting, carrying no colour at all. The as-shot multipliers are not neutral, so balancing sends it to (1.93, 1.00, 1.68) on this body, and the camera matrix turns that into R 2.88, G 0.51, B 2.03. Red and blue clip at one; green, whose matrix row is far less positive-heavy, does not. Red and blue high with green low is magenta. Nothing upstream was at fault, which is why the two previous attempts missed it: the white balance is correct, the matrix is correct, and the sensor normalisation is correct. The input simply was not a colour, and correct arithmetic on a non-colour produces a confident wrong answer. So saturation is detected before the balance is applied — the last 1.5% of range, smoothstepped rather than switched, because a hard threshold draws a visible rim around every highlight and a backlit edge on skin is where that shows. Above it the pixel is pulled to the neutral of its own brightness, so it keeps its luminance and loses only the cast. Verified end to end on the file itself: the sky is white, the skin, the black dresses and the stone are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 13:17:15 +02:00
dtourolleandClaude Opus 5	31e20399c8	Read the true white level, and clamp the sensor stage at both ends ... Two corrections to the sensor stage, found while chasing magenta highlights. Neither is the cause of that — see below — but both are wrong on their own terms. `white_level` took the *first* of rawler's per-channel saturation points. On a Canon 6D that reports 15070 while the data reaches 16383, so every sample above it was treated as brighter than white. It takes the maximum now. The normalisation clamped its floor and not its ceiling, so those over-white samples passed through as values above 1.0. Clamped at both ends. **This does not fix the pink.** Measured on _MG_8596.CR2, exported and looked at: the subject renders correctly and only the blown sky is magenta. A fully clipped pixel is (1,1,1) in raw, the as-shot balance multiplies it to (1.93, 1.00, 1.68), and the camera matrix turns that into R 2.88, G 0.51, B 2.03 — red and blue clip at one, green does not, and the result is magenta. It is correct white balance applied to already-saturated data, which is the classic highlight-clipping cast and needs highlight desaturation to fix: a pixel at saturation carries no colour information and must be rendered neutral, not balanced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 13:13:17 +02:00
dtourolleandClaude Opus 5	7d0fb710a6	Bound a readback by time, so a full-resolution export can finish ... Exporting a real 20 MP CR2 failed every time with "readback did not complete", while the copy itself was perfectly healthy. The bound was 100,000 non-blocking polls. That sounds generous and is not: a `Poll` that finds nothing returns immediately, so the loop spent its entire budget in a few milliseconds. Small transfers — the histogram's 4 KB, a viewport-sized frame — happened to land inside it. An 80 MB frame never could. It is a deadline now, thirty seconds, which is the only thing the bound was ever for: catching a lost device that will never deliver the callback. A one-millisecond pause after the first sixty-four spins stops the loop saturating a core for the length of the copy, while keeping a small transfer as immediate as it was. Found by developing /home/dtourolle/Downloads/_MG_8596.CR2 through the export example: 5472×3648 renders in 127 ms and writes all five formats. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 13:13:17 +02:00
dtourolleandClaude Opus 5	f1f528fc42	Stop rendering a missing white balance as neutral, which came out pink ... `sane_wb` replaced any coefficient it could not use with 1.0. That reads as a safe default and is not one. A Bayer sensor's green photosites collect roughly twice the signal of its red and blue, so unbalanced data is strongly green — and the camera matrix is built assuming the data reaching it has already been balanced. Fed green-heavy input it subtracts green as designed, overshoots, and the frame lands in magenta. Bodies whose as-shot coefficients rawler does not report came out pink, and nothing anywhere said why. The fallback is now the camera's own response to daylight, which `cam_to_srgb_from` was already computing on its way to balancing the matrix and then discarding. `daylight_wb` exposes it, and both callers read the same matrix through the same illuminant preference — so the multipliers neutralise exactly the white the matrix expects to be neutral, by construction rather than by coincidence. With no matrix either, the body is unknown and neutral is the honest answer: uncalibrated beats wrong in a specific direction. A test caught me returning the response rather than its reciprocal, which inverts the correction — a sensor is *least* sensitive to the channel needing the largest multiplier, so that version boosted precisely the wrong one. The doc comment now says which of the two it returns, because they differ by an inversion and look alike. Four tests, on a real matrix (Canon 6D, D65) rather than a contrived one: the fallback is nowhere near neutral, lifts both red and blue against green, stays green-normalised, and — the property that makes it consistent rather than merely plausible — balancing by it and then applying the matrix maps the camera's white to a neutral sRGB. `daylight_wb` is also the anchor the white-balance presets need: a preset in kelvin requires an absolute illuminant to be a preset *of*, and the temperature control is currently a relative offset from whatever the camera chose. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 13:03:03 +02:00
dtourolleandClaude Opus 5	654c11300e	Pin the framing geometry on pixels, not on the generated shader ... Chasing a reported shear on rotate and straighten. Two tests, and what they prove is that the pipeline is not where it comes from. A circle is the shape that makes anisotropy unmissable: any transform scaling the axes unequally returns an ellipse, and the ratio of its axes is the error. Both a quarter turn and a 20° straighten, on a 3:2 frame, return a circle within 8%. Worth recording because I had a confident and wrong hypothesis first. The quarter turn carries `* aspect.x` on one component and `/ aspect.x` on the other, which reads like an anisotropy of a-squared, and the reasoning that `p` is already isotropic is plausible enough that I changed it. The existing `a_quarter_turn_corrects_for_aspect_across_the_swap` caught that immediately, and these tests then showed the original was right all along: the crop rect is expressed in the *turned* frame and the output axes swap with it, so the factors are the conversion between those spaces rather than a mistake. Reading the shader and reasoning about which space `p` lives in is exactly how a plausible formula gets written twice. These assert on real pixels off a real adapter instead, so the next person to suspect this transform can rule it out in one command. The shear is therefore in the display path — the fit from the framed size to the viewport, or the crop overlay's uncropped render — and not in the geometry the pipeline computes. Not yet fixed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 12:46:17 +02:00
dtourolleandClaude Opus 5	02ae92ba0d	Tidy what the seven-branch merge left behind ... Three lints, all from merged work rather than from any one branch: `terrace` and `disc` were steps on the way to the ramp the plateau test now uses, and the reasoning that discarded them lives in docs/segmentation.md §12 rather than needing the code; two mechanical clippy suggestions in segment and cache. 1176 tests pass, clippy and fmt clean, traceability regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 12:36:15 +02:00
dtourolle	4e62b89d17	Merge branch 'android-collections' ... Touch multi-selection in the grid, filing a selection into a collection without a drag, and taking a collection offline from a held row.	2026-08-17 12:27:49 +02:00
dtourolleandClaude Opus 5	d913e50948	Select photographs with a finger, and take a collection with you ... Two things a tablet could not do. Both existed for a pointer and had no touch form at all, which on Android meant the collection sidebar was somewhere to look at rather than somewhere to file into. **Selecting more than one.** Ctrl-click and shift-click are the only ways into a multi-selection, and touch has neither. Holding a cell now enters selection mode, where a tap toggles — reported to Rust as a ctrl-press, so it goes through the same `apply_press` as everything else rather than growing a second copy of the selection rules. A double tap takes the run between where selecting began and there: the touch form of shift-click, and the reason the anchor from *before* the double tap has to be remembered, since both of its taps move the anchor onto the cell being tapped. A "Select" button does the same thing where a gesture would go undiscovered (FR-UI-4). **Filing without a drag.** A one-finger drag beginning in the grid belongs to the Flickable that scrolls it — that is the arbitration working, not a bug to route around — so the selection can now be filed from a sheet listing the sidebar's own rows. Copy by default, as the drag has always been; moving out of the collection being shown is a switch, because it is the one that takes something away. **Taking a collection offline.** The machinery was there and reachable only by scoping the grid to a collection and finding a button behind a disclosure. Holding a collection's name now asks the question directly, and the tray on a row and the header button ask the same one — three affordances doing two different things is how a user comes to avoid all three. The question is asked rather than a toggle flipped because both answers are expensive: one downloads gigabytes, the other deletes them, and the counts and sizes go in the buttons where they are read before the tap. `Cache::release` is new and is the destructive half `unpin` deliberately is not. "Remove the local copies" is asked by someone whose device is full, and withdrawing a promise while leaving the bytes for a future eviction to notice is not an answer to it. It unpins before forgetting, or the next pin fetch would dutifully download everything it just deleted. The sidebar's trays read `tier_actual`, never `tier_desired`: the question is whether these will open on the aeroplane, and a pin whose download has not run yet answers no. TRACES: FR-CAT-7 | FR-NC-6a | FR-NC-6b | FR-NC-6c | FR-UI-2 | FR-UI-3 | FR-UI-4 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 12:27:29 +02:00
dtourolle	76bb6b2847	Merge branch 'worktree-watershed-plateaux'	2026-08-17 12:25:41 +02:00
dtourolleandClaude Opus 5	0b20436445	Prove the plateau pass does nothing, and stop paying for it ... Picks up the lower-completion work a crashed session left mid-debug, with one failing test and no diagnosis. The diagnosis is that the pass is a no-op. Not "does not reduce basin count" — it changes *no pixel's basin at all*, zero of 9216, comparing one plateau iteration against sixty-four. That assertion is the substance of this commit: the original test asserted a consequence (fewer basins) which a working pass need not produce, so it could have been satisfied by weakening it. A no-op check cannot pass vacuously, and it is what turned an opinion into a fact. Three candidate causes were tried and none was it. Exact float equality is genuinely wrong and is fixed regardless — a gradient computed from 8-bit samples is never exactly equal across a region the eye calls flat, so `==` never fires and `<` fires everywhere; `LEVEL_EPS` now sits behind all three comparisons. The test image is not it either: a flat disc, a terraced disc and a constant-slope ramp all behave the same. The finding worth keeping is about the domain rather than the code. On a gradient-magnitude watershed every flat region of the picture is at gradient zero, the global minimum, and a plateau with no descending exit is a minimum — one basin already, nothing to resolve. The plateaux lower-completion is defined for are regions of constant non-zero gradient, which are rarer in a photograph than F1's phrasing implies. That may be the whole answer, or it may be hiding a fourth cause; I could not close it. So `plateau_iterations` defaults to 0. The implementation stays, correct as far as it goes and costing nothing until someone finishes it; the test stays, ignored with its reason; docs/segmentation.md §12 records what was ruled out so the next attempt starts further along than this one did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 12:25:28 +02:00
dtourolleandClaude Opus 5	a8b28136a6	Merge the batch export, and settle the seven-branch merge ... Resolves the last of the parallel work. Two conflicts worth recording, because both were semantic rather than textual: `render_for_export` gained a colour space on master while the batch branch was rewriting the single-image export path around it. Kept both: the batch request supersedes the synchronous path, and the space still has to be chosen at render time because the conversion happens in the shader before the clip to 0..1. `render_open_frame` takes it as an argument rather than reaching for a controller it does not hold. The map-wait moved into `readback::await_mapping` on one branch while another was editing the constant it used, so `READBACK_POLL_LIMIT` survived the merge with no callers. Removed rather than left for clippy to find later. 1164 tests pass, clippy clean, fmt clean. Traceability 53.0% -> 54.3%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 10:11:44 +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
dtourolle	1c16ca3d27	Merge branch 'colour-managed-export'	2026-08-17 09:28:22 +02:00
dtourolle	8e330a24e9	Merge branch 'undo-redo'	2026-08-17 09:26:23 +02:00
dtourolle	c26b6082a2	Merge branch 'local-libraries'	2026-08-17 09:26:03 +02:00
dtourolle	9f577ed6a2	Merge branch 'xtrans-demosaic'	2026-08-17 09:25:28 +02:00
dtourolleandClaude Opus 5	8ea92545df	Stop the export folder forgetting itself, and let Android reach one ... Three faults, compounding into an export that could not be made to work on a tablet at all and a destination that appeared to reset on its own. **Switching target destroyed the destination.** One field held both a filesystem path and a remote folder, so changing the target had to clear it — `/home/x/Exports` carried to the server would have offered to create a folder called `home` at the library root. The consequence was that merely looking at the other option threw away the destination already chosen, which reads, correctly, as a setting that will not stick. There are two fields now. Each target remembers where it was pointed and switching is free; `active_destination` picks between them so no caller can reach for the wrong one. **The library root read as "unset".** The picker opens at the root, so confirming it where it opens stored an empty string — indistinguishable from "ask each time" and looking exactly like the picker had done nothing. Empty now means the library root for a server destination, which is a real folder and the one the photographs are already in; it means "ask" only for a device folder, where no path is worth assuming. The page labels it so. **Android defaulted to a target it cannot use.** A device folder there means the Storage Access Framework, which provides no filesystem path (ARCH §6.9) and is not implemented — so the default target could never succeed however the destination was filled in. The export button said "no export folder is set", the settings page offered no way to choose one, and the only way out was to guess that the other target was the working one. The device target is now absent from `ExportTarget::available()` on Android and the default there is the server, which needs no platform work at all. A settings file carrying an unreachable target — copied from a desktop, say — is corrected on read rather than left to fail at the last step. Seven tests, each named for the fault it prevents returning. The compatibility one matters most: a file written before `remote_destination` existed keeps its device path and gains an empty remote one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 09:25:15 +02:00
dtourolleandClaude Opus 5	914d14ec0d	Tell the shader which colour space it is encoding for ... The generated shader ended with `encode_srgb` and a clamp, so every photograph leaving DarkRoom had been through sRGB's gamut whatever the settings page said. Export refused the other three spaces rather than tag clipped pixels with a gamut they did not contain — correct, and not something an encoder could fix. So the output space becomes a parameter of composition. `compose_for` emits a constant primaries matrix after the camera matrix and before the clip, and generates the transfer function to match: the sRGB curve for sRGB and Display P3, a pure 2.199 gamma for Adobe RGB, 1.8 with a linear toe for ProPhoto. The ordering the camera matrix depends on is untouched — operations still run in camera space — and sRGB emits no conversion at all, so the shader compiled on nearly every frame is byte-for-byte what it was. The numbers live in dr-types, derived from four chromaticity pairs per space rather than tabulated. That is not tidiness: the shader encodes the pixels and the ICC profile describes them, and a file whose profile disagrees with its own contents is worse than one with no profile. One derivation makes them agree by construction, and can be checked against the values the specifications publish. Profiles are generated here too — minimal v2 matrix/TRC, about 2 KB, pure Rust, no lcms to satisfy under the NDK. A JPEG carries it in APP2, a PNG in iCCP, a TIFF in tag 34675. sRGB gets one as well, because untagged does not mean sRGB, it means guess. The refusal survives in a sharper form. A `Frame` now carries the space it was rendered in, and export refuses to label it anything else. The develop session still composes for sRGB, so a P3 export from the interface fails with an accurate error instead of producing a file that lies — the frontend half is a separate change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 09:04:04 +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	bb71f141e7	Let a folder on this machine be scanned into the catalog ... `dr_catalog::scan` has known since it was written what a changed directory means — when to prune, when to list, and the one question that decides whether a deletion sweep is safe. It was fully tested and nothing called it, because walking a real directory "belongs to the platform layer" and the platform layer was eleven lines re-exporting `secrets`. So every photograph in DarkRoom arrived over WebDAV, and a user without a Nextcloud account saw nothing at all. This is the missing half: a `Storage` trait, a filesystem implementation of it, and the driver that pours one into the other. The trait is shaped by the platform it does *not* yet support. Android's SAF gives no filesystem path, which is why `SourceRef` exists; less obviously, it gives no way to *compose* one either — a document id is opaque, and the only way to learn a child's id is the children query that returned it. So a listing hands back the reference to each entry rather than a name for the caller to join onto a parent, and there is deliberately no "path + name" helper anywhere above `LocalStorage`. That single restriction is what makes SAF a second implementation rather than a second set of call sites. A reference is otherwise an opaque `(RootId, key)` pair the catalog stores verbatim and rebuilds later, which a persisted tree grant supports exactly as a relative path does. A `Path` now appears in one place: `LocalStorage::grant`, where the folder the user picked is handed in. Everything above it addresses a `RootId`. `dr_catalog::walk` is the seam. It probes a directory, asks `scan` what that means, lists only when told to, and reconciles what it found against the rows it holds. Two things it does are worth saying out loud, because both are ways to lose a library: Absence only counts where absence was observed. A listed folder proves its missing images are gone; a pruned one proves nothing about its contents, and a scan that was cancelled or that failed part-way proves nothing about folders it never reached. So the file sweep runs per listed folder, the folder sweep runs once at the end and only after a complete scan, and a root that cannot be reached at all marks its images offline and deletes nothing — FR-CAT-9's line between proven-absent and merely-unreachable, which is the difference between unplugging a drive and losing everything on it. A trashed image is absent from its folder on purpose. It is exempt from both sweeps, and detached from a folder about to be deleted rather than cascaded away with it, or a soft delete would come undone the first time the folder it came from was rescanned. Two things the tests taught, both changes to what was there before: Modification times are now milliseconds, not seconds. Change detection asks whether a timestamp moved, so the unit's granularity is the width of the window in which a change is invisible — and a second is long enough to copy a card and start a scan. The test that caught it looked like a test bug; it was not. SAF reports milliseconds natively, so this is also the unit that needs no conversion on the platform with the coarser clock. And an in-place rewrite of an existing file is invisible to directory-level pruning, because writing to a file moves neither its directory's mtime nor its entry count. That is a real limit, now documented and held by a test rather than left to be discovered. It bites less than it reads: an export, a restore, `mv`, and every editor that saves safely write beside the file and rename over it, which does move both. Narrowing the format filter no longer deletes what it stops matching, which fell out of the same principle: unticking JPEG says stop looking for new ones, not discard the hundred already rated. The files are sitting right there. `DirState` and `DirEntry` move to `dr-types`. They are the sentence the platform says to the catalog and both crates need the same one; `scan` re-exports them so nothing that used them has changed. Not done: the UI. The launch screen's "Open library" flow is account-shaped from the first field to the thumbnail worker, and giving it a local branch is its own piece of work rather than a button. `cargo run -p dr-catalog --example scan_local -- ~/Pictures` scans a real folder and reports what it cost; run it twice to see the second run list nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 08:59:17 +02:00
dtourolleandClaude Opus 5	1c0994c807	Demosaic a Fujifilm sensor instead of refusing it ... Every RAF stopped at the embedded preview, because the demosaicer had one kernel and it was a Bayer kernel. D11 makes Fujifilm first-class and FR-RAW-5 asks for it by name, so a hard error there was a promise we had not kept. X-Trans is a 6x6 tile, and nothing in the Bayer path survives that: the missing channels sit at different offsets at all 36 positions, so there is no fixed kernel to write. The new shader fits a weighted plane through each channel's samples in a 5x5 window and carries the other two channels across as the difference between those planes, keeping the pixel's own measured value untouched. A plane rather than a mean because the three channels are sampled at different places in the tile: a mean compares a red taken slightly left of the pixel with a green taken slightly right of it, and that offset is a colour cast that follows every gradient in the frame. The fit is done in white-balanced space, where the constant-colour-difference model it rests on is actually true of a neutral subject; that alone halves the error at a luminance edge. Two compromises, both deliberate. It is not Markesteijn. There are no directional hypotheses and no homogeneity map, so it does not resolve detail finer than the CFA period and a hard edge arrives about two pixels wide. It cannot ring — the output is bounded by the local sample range — so it does not produce the worms FR-RAW-5 exists to avoid, but the quality that requirement asks for is still owed. The tile's phase is guessed rather than known. rawler has each body's pattern exactly, as a 36-character string, but CfaPattern::XTrans throws it away before dr-gpu sees the file, and it is not a constant to hard-code: the bodies in that database start the tile at four different origins. So the phase is read back out of the pixels, by grouping the 36 per-position means and taking the grouping with the least spread. That part needs nothing from the scene. Telling red from blue does — shifting the tile by half a tile turns it into itself with red and blue swapped, so no geometry can decide it — and the as-shot white balance is what breaks the tie. A frame that is almost entirely one colour can defeat that; widening dr-decode to carry the pattern string would retire the guess altogether. The tests assert reconstruction, not success: a flat patch comes back exactly at all six phases tested, and a linear ramp comes back exactly too, which is the property the plane fit exists for and the one a mean would fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-17 08:46:28 +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	23f0c4b76a	Let a declared node ask for a widget, and write down how ... Two gaps in the control vocabulary, both found by reading back what 0a331c7 actually shipped against what it claimed. **`kind: enum` worked and was undocumented.** The whole point of that kind is that a node author can declare a control without touching the interface, and ops/README.md is where a node author looks. A kind absent from the table is one nobody will use. It is now in the table, with the property that makes it cheap spelled out: the value is the chosen index carried as an `f32` like every other parameter, so a uniform expression can read it, the sidecar stores it unchanged, and nothing along the way needed a second kind of value. **A declared node could not ask for a widget at all.** `Presentation` gained an ordered preference list and a demand, and `tone_curve` and `framing` use them — but both are hand-written Rust. A YAML node had no way to say that several of its parameters form one control, so the generated half of the pipeline was locked out of the half of FR-DEV-3a that makes widgets extensible. Nodes now take a `presentation:` block: presentation: widgets: [colour_wheel] demand: { two_dimensional: true } params: [hue, strength] `demand` carries exactly two flags and there is deliberately no key for a pixel width, a breakpoint or a platform name — those are the frontend's to decide, and ARCH §4.3a is explicit that a core reasoning about them will eventually be wrong about a display it never saw. Widget names are spelled in the YAML the way `WidgetKind` spells them, so a declaration and descriptor.rs cannot drift into two vocabularies for one idea, and an unknown one is a build error listing the six that exist. `params:` is checked against the node's own parameters: the widget claims what it names and the generic path skips what was claimed, so a typo would silently drop a parameter out of *both* — the one mistake here that produces no error and no control. Verified by declaring a presentation on a node, confirming the generated `fn presentation` matched, and confirming both error paths report the file and the key; then reverted, since no node in the chain wants a widget yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-16 23:54:10 +02:00
dtourolleandClaude Opus 5	5e4b8de18d	Stop the mixer's bands from dividing one budget between them ... Each band's delta was scaled by the summed weight of the bands that happened to be adjusted: d_hue = d_hue / w_total; d_sat = d_sat / w_total; The divisor is the wrong quantity, and it fails in two directions. A band adjusted on its own divides by its own weight and cancels it — w*v/w is v — so the falloff does nothing. Green saturation at +100 hit a pixel at 179° exactly as hard as one at 120° and not at all at 181°: full strength across the whole window, then a cliff. That seam is the thing the overlap exists to prevent, and it was there the moment a single band was touched. Worse, the divisor counts a band's weight even for a channel that band says nothing about, so the controls compete. `red_hue` at +100 shifts a pure red pixel +30°. Set `orange_sat` as well and the same pixel shifts +20°, while orange's saturation bleeds into pure red at a third strength. Hue and saturation on *neighbouring* colours were trading against each other — the two sliders behaving as though only one of them could be spent. The real fault is upstream of the division: the falloff window was ±60° while the bands sit 30° apart, so the twelve weights sum to 2.0 rather than 1.0 and *something* had to correct for it. Narrowing the window to the band spacing makes them a partition of unity, and then nothing has to. The division is gone; `w_total` stays, demoted to what it should always have been — the test for whether any adjusted band reaches this pixel at all. Everything the module header claimed is now true rather than aspirational: a band reaches zero at its neighbours' centres, a hue halfway between two gets half of each, twelve bands at +100 equals global +100, and a band pushed alone reaches the full 30° of travel its own comment documents instead of whatever fraction the other sliders left it. `overlapping_weights_are_normalised` asserted the broken arithmetic verbatim, so it is replaced rather than repaired. In its place: no delta may be divided by w_total, the guard must survive, two adjacent bands must emit independent terms, and — the property the rest now rests on — the twelve weights must sum to one, swept at 0.1° around the wheel. That last test carries a Rust mirror of `band_weight`, so a third test pins the mirror to the shader's own constants; a copy nothing checks is how the window and the spacing drifted apart in the first place. Also gone: the red branch of `rgb_to_hcl` computed its hue twice and threw the first away. This changes how existing edits render. Mixer adjustments are more selective, and where they were quietly cancelling each other they no longer are, so a saved sidecar will not come back looking the same. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-16 23:36:52 +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