655 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
dtourolleandClaude Opus 5	75fd5619ca	Refuse a scan whose root has gone, instead of reporting it empty ... FR-PLAT-AND-2, and a silent failure on both platforms. `dr_sync::scan` stepped over a NotFound or PermissionDenied the way it does for a child that vanished mid-walk -- correct for a child, wrong for the root, where it ended the walk, returned Ok with nothing in it, and reported a successful scan of a library that was no longer there. A lost root is now its own error. The images under it are marked Availability::Offline per FR-CAT-9 and no catalog row is deleted; `library::persist` clears the mark per file as each one is listed again, so a root that comes back needs no repair step. Partly satisfied rather than closed, and the gap is worth stating. The recovery half is real and reachable on Android today, because `map_status` turns Nextcloud's 403 and 404 into it and Nextcloud is how a phone actually gets a library in this build. The causes the requirement names -- revocation, reinstall, a removed card -- are properties of a persisted tree permission, and there is none: SAF does not exist here, `SourceRef::Document` is constructed only in test modules, and `LocalStorage` rejects the variant outright. When SAF lands it becomes a third producer of this error and nothing above it changes, which is why the discovery belongs in the connector. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 22:19:38 +02:00
dtourolleandClaude Opus 5	2b812ebe21	Give memory back in the order the user will miss it least ... FR-PLAT-AND-5. Android asks for memory back through onTrimMemory and kills the process if it is not given; until now nothing listened, so the answer was always "no". A tiered registry answers instead: GPU caches first, then proxies, then thumbnails, driven from android_main on MainEvent::LowMemory and MainEvent::Stop. The order is the argument. A backgrounded app has no window to draw and therefore no use for a render pipeline, while its thumbnails are exactly what the user will be looking at half a second after they come back -- so going into the background frees only the GPU tier, and only being measured against death frees everything. Sinks register beside the cache they free and hold weak handles, so the registry cannot keep a controller -- and every decoded portrait in it -- alive past the interface it belonged to. `try_borrow_mut` and skip: a warning can land mid-render, freeing textures under the code drawing with them is worse than missing one, and a warning not acted on is always followed by another. The GPU test is the one that matters: an eviction must change no pixel. A freed intermediate pool whose `colour_key` promise still stands renders an empty texture, and nothing else would have caught it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 22:19:29 +02:00
dtourolleandClaude Opus 5	6246a2e346	Merge: group the frames of one moment, and let a burst fold away ... FR-CULL-5. Frames join a burst when they are adjacent in time and look like the frame before them -- both, because time alone groups a whole ceremony and similarity alone groups a studio setup across two days. Adjacent pairs only, chained; there is no all-pairs step and there must never be one. No selection of any kind. The representative is the earliest frame, a fact about the clock rather than a judgement about the photograph, and a newly found burst arrives open, so the pass never takes a row off the screen. Verified: fmt, clippy --workspace --all-targets -D warnings, 346 dr-catalog tests, 511 dr-ui tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 22:07:05 +02:00
dtourolleandClaude Opus 5	fc4157a1e0	Keep the test scene's own arithmetic from overflowing a u64 ... The first compile this branch ever had. `cargo fmt` reflowed four files and clippy passed at -D warnings untouched, but one test panicked: `the_signature_does_not_change_with_scale`, on "attempt to multiply with overflow". It is the fixture, not the feature. `scene()`'s little LCG multiplied the block's y by the golden-ratio constant with a plain `*` while the term beside it already used `wrapping_mul`, so any scene taller than about 104 pixels overflowed in debug. Only the scale test builds one that large, which is why 345 of 346 passed around it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 22:07:05 +02:00
dtourolle	91fe6cf300	Merge master into partial-preset-scope ... # Conflicts: # docs/traceability.md # ui/dr-ui/ui/app.slint	2026-08-29 22:02:03 +02:00
dtourolleandClaude Opus 5	754ab91347	Bring a Lightroom library across, and start with something in the list ... Two halves of the same complaint: a preset sheet that opens on "No presets yet" is homework, and a photographer with ten years of presets in Lightroom has no way to bring them. `dr-preset-xmp` reads Camera Raw `.xmp`. The mapping turned out to be mostly a rename rather than a conversion, because Adobe and this pipeline already agree: exposure is in stops in both, and contrast, the four recovery controls, clarity, texture, vibrance and saturation are all ±100 in both. That is not imitation, it is the convention raw developers converged on — `highlights_shadows.yaml` cites it in as many words. Only sharpening needed arithmetic, Adobe's 0…150 against our 0…100. The white balance does not come across, and says so rather than guessing. Adobe writes absolute Kelvin for a raw file where ours is a relative nudge from what the camera recorded, so converting needs the *target image's* as-shot white balance — exactly what a preset cannot carry, since the same preset lands on a frame shot at 3200K and one shot at 7000K. A guess would be wrong on most images and invisibly so. A folder is read as readily as a file, nested, because that is the shape an exported preset folder is in and importing ninety files one at a time is asking someone not to bother. `dr_pipeline::starter` is six presets a first run begins with, written against this pipeline in its units and deliberately mild — a starting point, not a caricature. They are seeded when the library *file* does not exist rather than when the library is empty, so deleting all six does not hand them back on the next launch. Both of these name operations, and `ui_names_no_operation` was right to stop them living in `ui/`. That test exists because the failure is silent and cumulative, and it caught exactly what it was written for: a preset called "Punch" is a statement about contrast, clarity and vibrance, and a table mapping Adobe's vocabulary to ours is a statement about the pipeline. Neither is a fact about an interface. So the starter set went into `dr-pipeline`, and the importer into its own crate — between two walls, since `dr-pipeline` depends on nothing on purpose and XMP is real XML not worth hand-rolling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-29 22:00:53 +02:00
dtourolleandClaude Opus 5	36ea53cceb	Merge: stop a subject's name from setting the width of the develop column ... Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 21:49:40 +02:00
dtourolleandClaude Opus 5	3e19628324	Stop a subject's name from setting the width of the develop column ... The column moved sideways the moment segmentation finished. It takes the widest minimum any panel declares, each panel publishes `layout.preferred-width` as that minimum, and `MaskPanel` gained rows whose width came from the model's output -- so a photograph the user was looking at jumped because a label said "traffic light". `overflow: elide` did not prevent it and was never going to. Eliding is what a `Text` does when it draws; at layout time it still asks for the width of its whole string, and it is the asking that reaches the column. Both the subject rows and the mask entries are bounded, because a mask is itself a segmentation result -- without the second half the column moved when a subject was clicked instead of when one was found. The bound is stated for the reason `ChipGrid` declares its width from its column count rather than from its options (ff6c313): what a panel asks for must follow from its structure, never from its data. 160px is the same judgement as that commit's 88px chip, and it is a policy rather than a measurement -- there is no harness in the tree that measures a panel's width, and the 482-to-351 figure in ff6c313 was read off the running app. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 21:49:33 +02:00
dtourolle	4a04496c78	Merge: focus peaking, so a frame can be judged without zooming to 100% ... FR-CULL-3's peaking half. The raw histogram and raw clipping indicators remain unbuilt -- what exists is a display histogram tagged FR-DSP-7, counting AdjustPass's 8-bit output, which reports a highlight as gone precisely where FR-CULL-3 needs it to report the highlight recoverable. Verified before merge: fmt clean, clippy --workspace --all-targets -D warnings green, 11 focus GPU tests, 79 baseline dr-gpu tests, 511 dr-ui tests. The cfg(target_os = "android") arm is unverified -- the host-target clippy never compiled it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> # Conflicts: # ui/dr-ui/src/lib.rs # ui/dr-ui/ui/app.slint	2026-08-29 21:43:35 +02:00
dtourolleandClaude Opus 5	2168cdd1c4	Mark what is in focus, so a frame can be judged without zooming to 100% ... FR-CULL-3's focus peaking. One compute dispatch measures local contrast in WGSL and writes an overlay texture; on desktop it reaches Slint through the same zero-copy wgpu import the canvas uses, so nothing per-pixel touches the CPU on the frame path. With peaking off the cost is zero and structurally so: focus_overlay opens with `let settings = self.peaking?;` before the frame is touched, and clearing drops both overlay textures, so no VRAM is held either. NFR-P14 is met by construction rather than by measurement -- one dispatch, no second render, no pipeline compile after session open, and a test asserting allocations stay at 2 over eight frames. The budget test asserts 50ms at 4K rather than a tight bound, deliberately: a tight bound fails on a loaded machine and gets deleted, which is worse than a loose one that still catches the regression that matters. TD-1 is amended rather than joined by a TD-6: on Android the overlay rides the readback that already exists there, roughly doubling that transfer while peaking is on, and TD-1's own "Done when" removes both because both are the same missing capability. Verified: cargo fmt clean; clippy --workspace --all-targets -D warnings green, which also compiles peaking.slint through dr-ui's build.rs; 11 focus GPU tests and 79 baseline dr-gpu tests pass; 511 dr-ui tests pass. Not verified: the cfg(target_os = "android") arm, which the host-target clippy never compiled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 21:43:08 +02:00
dtourolleandClaude Opus 5	7ad75dd905	Let a manual collection be put in the order the photographer wants ... `collections::set_order` and `Sort::CollectionPosition` have been in the catalog since collections were, and nothing above dr-catalog has ever called either. A manual order existed, could not be seen, and could not be set. This is the half that was missing. Three pieces, because it needed all three to be visible at all. The catalog gains `orders_manually` and `members_in_order`. The first is the rule about *when* a manual order means anything, kept in one place with one name: a collection must be manual, and it must have no children. A set shows its descendants' images, and positions are only ever assigned within one collection — so two children's positions are unrelated integers, and ordering by them would sort the grid by a coincidence. The existing comment on `read_cells_scoped` already argued this; now something enforces it. `members_in_order` returns the *whole* membership rather than the filtered view, because `set_order` renumbers exactly what it is handed. Reordering a filtered list would renumber those and leave every hidden image on a stale position — two images sharing one, and a grid that rearranges itself the moment the filter comes off. The grid reads position where the scope qualifies and capture time everywhere else. Manual order joins the member row rather than testing membership with `IN`, which is safe from fanning out rows *because* that branch is a single collection. The gesture is a DropArea over the viewport, drawn only where a reorder means something, with a caret in the gap the photographs would go into — a line between two images rather than a highlight on one, because lighting up a cell would say the drop replaces it. The trap worth naming: `DropEvent.position` is in **window** coordinates. Slint maps it through `map_to_window` when the drag begins and hands every target the same event untranslated, so a target inside a Flickable has to subtract its own `absolute-position`. Getting that wrong is invisible until the grid is scrolled, because at the top the two frames coincide. `reordered` is pure and names its destination by the image it goes before rather than by an index, because the grid can only name a gap in what it is showing and the ids are what survive a window swap. A drop that changes nothing returns the order untouched: that counter is what a cross-device merge resolves by, and spending a revision on a no-op makes this device win an argument it did not have. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-29 20:49:00 +02:00
dtourolleandClaude Opus 5	b52be080ff	Merge: say which requirements the code already satisfied, and which it does not ... Eight requirements were implemented and untagged; tagging them takes coverage from 59.8% to 64.2% without a line of feature code. Five more were refused rather than tagged -- R2's own acceptance criterion still reads '(figure TBD)', R5 has one of three clauses built, FR-RAW-2 meets the requirement's purpose but not its stated mechanism. docs/outstanding.md records what is genuinely unbuilt, so the gap reads as a decision rather than an oversight. It also names four requirements the matrix reports as covered that are not, including two covered only by string literals inside the traceability tool's own test fixtures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 20:46:08 +02:00
dtourolleandClaude Opus 5	4d0ad0601c	Merge: a Flatpak that asks for no filesystem, and the channels v1 ships through ... FR-PLAT-LIN-3 and NFR-COMPAT-2. The manifest grants no filesystem permission of any kind, which is not an oversight: a folder library is chosen by typing an absolute path, nothing in the tree calls the FileChooser portal, and a static --filesystem= grant would have made that design appear to work by not testing it. docs/distribution.md records what a portal-based picker would take. No Flatpak has been built -- flatpak-builder is not installed here -- so the finish-args set is reasoned from what the binary links, not observed to be sufficient. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 20:46:02 +02:00
dtourolleandClaude Opus 5	115653a262	Mark a burst in the grid, and let it be folded away ... The counterpart to the grouping: where the signatures come from, and how a group reaches a cell. Signatures are computed from the 256px thumbnails dr-thumbs already holds -- vastly more resolution than a 9x8 reduction can use -- so a library that has been browsed, or that has synced somebody else's shards, has already paid for them and no RAW is decoded for this. The consequence is stated rather than hidden: an image with no thumbnail gets no signature and never joins a burst. That is self-correcting, and it is why the pass runs when the thumbnail sweep finishes rather than on a timer. Nothing happens at import and nothing happens at query time. The mark is drawn as a child of the cell's TouchArea, for the same reason the star strip is: a click on it must not also reach `cell-clicked` and throw the user into develop, and children are hit-tested before the element they sit in. It is never hidden on hover the way the stars are -- a collapsed burst stands in for frames that are not on screen, and something has to say so whether or not a pointer is nearby. Folding changes what the grid's *query* returns rather than what its cells draw, because the grid is a window over an ordered query and the frames a fold hides are mostly not loaded. So the predicate joins VISIBLE in every query that lists or counts cells -- the window, the header's count, the run a shift-click resolves, and the ordinal a scrub lands on -- under the discipline VISIBLE's own comment sets out: present in four places of five is worse than absent, because the counts disagree with the cells and neither looks wrong on its own. There is a test for exactly that. `the_window_read_walks_the_ordering_index` now includes the burst clause. It asserts on the query plan while holding its own copy of the query, so left alone it would have gone on reporting green against a query the grid no longer runs. If the clause costs `images_grid_order` and puts the sort back, that fails here rather than becoming jitter someone measures in six months. The pass keeps its own drain timer in a thread-local instead of taking fields on the library controller, so everything the feature needs to run lives in one file and the screen that starts it holds nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 20:39:51 +02:00
dtourolleandClaude Opus 5	bb35665bd2	Let a paste carry some kinds of edit and not others ... FR-DEV-6 asks for presets "covering a subset of the edit graph". What landed with the named presets covered two subsets: everything, and everything but the crop. "Match the colour but not the sharpening" had no way to be said. `Scope` is now a set of `Attribute` — the same six kinds every operation already declares and the develop panel already builds its tabs from. The photographer ticking "tone and colour" is naming the groups they navigate by, and neither this module nor the interface has to name an operation to do it (FR-DEV-3c). The pleasing part is what left. Framing used to be excluded by an explicit test against one operation's id; it is now excluded because Geometry is not in the default set. The special case dissolved into the general rule, and the argument for it — a crop is a decision about *this* photograph, and carrying it across forty destroys forty compositions — is now a statement about a kind of edit rather than about a node. All thirty-three existing preset tests pass unchanged, which is the evidence that the generalisation kept its promises. One decision that is a field rather than a rule, because the two cases genuinely differ. An operation this build cannot classify — from a newer version, arriving over sync — travels under "everything" and "everything but the crop", because those are claims about the whole edit and an unrecognised operation is part of it (FR-NC-8). It does not travel under a hand-picked set, because that is a claim about kinds, and an unknown kind is not one of the kinds that were ticked. The settings page's "Copy crop and rotation" checkbox is gone, replaced by the same chips the preset sheet draws. It asked the right first question — geometry is the kind whose accidental travel destroys work — but it was the only question a boolean could ask. The field stays in `Settings`, read exactly once to seed the new set, so anyone who had ticked it keeps their behaviour. The chips are deliberately not in the develop column. Six of them there would set the width of the whole sidebar, which is the bug `ChipGrid`'s comment records at length. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-29 20:38:19 +02:00
dtourolleandClaude Opus 5	d3dadbd725	Group the frames of one moment, by when they were taken and what they look like ... A burst is the commonest thing in a cull and the least interesting: twelve frames of the same gull at 10 fps occupy twelve cells, are scrolled past twelve times, and end with the photographer keeping one. FR-CULL-5 asks for them to collapse to one representative and be judged as a unit. Two signals, because neither alone survives a real library. Time alone groups a whole wedding ceremony -- a photographer working steadily never leaves the gap that would end the run. Similarity alone groups a studio setup shot across two days, which is a project rather than a moment. Together they are specific: adjacent in time *and* looks like the frame before it. Two seconds is the time bound, and the reason is worth recording because the figure looks absurd next to a 10 fps camera. `images.captured_at` is whole seconds -- EXIF's DateTimeOriginal has no sub-second field and SubSecTimeOriginal is optional and widely omitted -- so a burst arrives in the catalog as ten frames sharing one timestamp. Any threshold finer than a second is a threshold on information that is not there. Where the pace really is faster, the similarity bound is what separates the frames. Similarity is a 64-bit difference hash over a 9x8 box-averaged reduction, compared between *adjacent* frames only. Chained rather than anchored on the first frame, because by frame twenty a camera following a bird has nothing in common with frame one while no two neighbours differ by much; the time bound is what stops the chain running away. There is no all-pairs step and there must never be one -- that is what turns a grouping pass into something nobody can afford to run over 50k images. Nothing here ranks a frame. FR-CULL-5 names the failure it is avoiding, which is rejecting the only frame of an important moment because somebody blinked, so there is no sharpness score and no best-of-burst. The representative is the earliest frame -- a fact about the clock, not a judgement about the photograph -- and the user's own choice lives in its own table so that rebuilding the grouping cannot erase it. Same argument `people.ignored` makes one subsystem over: nothing short of remembering a decision survives re-clustering. A newly found burst is recorded *open*. Collapsing on discovery would be tidier, and would also mean a background pass taking photographs off the screen part way through a cull. The pass marks; the user folds. It is a pass rather than a job kind for the reason catalog.md 10.2 gives for face clustering: a burst is a property of a run of frames and has no natural subject_id, so a per-image job would rebuild the world once per photograph. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 20:37:54 +02:00
dtourolleandClaude Opus 5	8d5dc423ea	Say that all three of FR-CULL-3's bullets are unbuilt, not one ... The first draft got this half right and half wrong. It correctly said the existing histogram and clipping indicators are not FR-CULL-3's, but it described them as adjacent — as though the work left were mostly focus peaking and a relocation. They are not adjacent. The histogram reads AdjustPass's 8-bit output and counts clipping as r == 255, so it describes the frame the display is about to show, after the entire develop chain. FR-CULL-3 asks for the histogram of the sensor data, and gives its reason in the requirement itself: a rendered image "systematically lies about what is recoverable in the raw". A readout taken from the render cannot answer that question however it is presented, which means two of the three bullets need a new measurement rather than a new placement. Found by the agent building focus peaking, who had to go looking at the counters to find out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 20:36:51 +02:00
dtourolleandClaude Opus 5	1cd58bcaaa	Say where a range ends in words, instead of hiding it in a double tap ... Touch has had a range gesture for as long as selection mode has: hold to enter it, then double-tap the far cell. Nobody finds it. It is invisible, it is unreliable on a grid that scrolls under the second tap, and it extends from the anchor as it was *before* the two taps moved it — a rule subtle enough to need two paragraphs of Rust to explain itself. The deeper problem is the timer. A double tap is bounded by the double-tap interval, so the two ends have to be on screen together. The ranges that actually hurt on a tablet are longer than a screenful, and those are exactly the ones it cannot describe — which is also why a drag-to-select sweep would not have fixed this, and why it is not what went in. So: a "Select to…" button arms the range, the strip stops reporting and starts instructing ("Tap the last photograph — scroll first if you need to"), and a Cancel puts it down. Nothing is timing the two taps, so the user may scroll as far as they like between them, and the run is resolved by the catalog rather than by what happens to be loaded. An armed range reaches Rust as `cell-pressed`'s shift argument, because that is what it is: `apply_press` already reads ctrl+shift as "add the run from the anchor to here", and in selection mode ctrl is already set. So this needs no Rust state and no third selection policy — one copy of the rules, in the function that had them. "Select all" goes in beside it, asked of the catalog for the same reason: the window is a hundred cells over a library of thousands, and a select-all that quietly meant "the hundred that are loaded" is a lie the user cannot see until the export runs. It sets the anchor to the first frame, or a "Select to…" straight afterwards would reach `apply_press` with no anchor, take its shift branch, and clear everything it had just taken. The strip is at four buttons and a count now, so "New collection from selection" loses its tail — the sheet it opens already says "New collection holding 12 photographs" across the top. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-29 20:31:38 +02:00
dtourolleandClaude Opus 5	ab49280614	Ask for the collection's name before there is a collection to name ... "New collection from selection" created the collection under a placeholder name and then opened the sidebar tree's rename field to correct it. On a tablet that field is not on screen. The collections panel is always instantiated — app.slint collapses it to zero width and `visible: false` rather than using an `if`, because an `if` there is a layout loop Slint panics on — so the rename TextInput was created all the same, its `init` called `self.focus()`, and Android raised the on-screen keyboard for a box nobody could see. Nothing else on the screen is focusable, so the keyboard had nowhere to go back to: it stayed up, the name could not be typed, and the collection was already written under the name the user did not want. Asked in a sheet instead, on the same card, scrim and dismissal the filing and keywording sheets use. The field takes the keyboard as the sheet appears — over a field that is actually drawn, which is the whole difference — and the card sits a third of the way down rather than centred, because on a tablet the keyboard is the bottom half of the window. Nothing reaches the catalog until Create. That also ends a second bug the old order could not avoid: an abandoned rename used to leave a collection called "New collection" behind, because creating came first. `Field` grows a `take-focus()` for this. A function rather than a property: focus is an event, and bound to a property it would fight whatever took focus next and re-take it on every unrelated re-evaluation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-29 20:31:02 +02:00
dtourolleandClaude Opus 5	0d9f802f88	Regenerate the matrix that this branch is about ... Eight requirements gained a tag in this branch's first commit, so the generated matrix is stale until it is rerun. Coverage moves from 59.8% (107/179) to 64.2% (115/179), and the untagged list drops from 72 entries to 64. Regenerating here rather than leaving it to the pre-commit hook, because this branch's subject *is* the matrix: a reader comparing the two commits should see the number move for a stated reason. The eight are R3, R6, FR-DEV-1, FR-UI-6, FR-NC-6d, NFR-OPS-3, NFR-PORT-2 and NFR-SEC-3, and none of them is new work — every one was already satisfied by code that had simply never said so. Six of the thirteen tag sites were new `TRACES:` lines rather than additions to existing ones, so the tag count moves 896 → 902 while the covered count moves by eight. Orphan tags remain at zero. Run from source with `cargo run -p traceability -- report`, never from a prebuilt binary, and note that the tool tracks line numbers — the six prepended module tags shift every subsequent line in their own files, which accounts for the churn in this diff that is not a coverage change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 20:26:07 +02:00
dtourolleandClaude Opus 5	88ef7148d6	Write down what is not built, so the gap reads as a decision ... The traceability matrix reports one number and cannot say what it means. An untagged requirement is either one nobody built or one somebody built and did not label, and both look identical in the summary table. Eight of the second kind were tagged in the previous commit. This is the first kind, written out with the reasoning, so that the distance between the register and the binary is something a reader can see rather than reconstruct from a percentage. Eleven clusters, each verified against the tree rather than inherited from a survey. Several turned out to be more interesting than "not done": Plugins are 21 untagged requirements — nearly a third of the shortfall — and §7 already lists the Plugin API as out of scope for v1 while §3.10 spends 280 lines specifying it. The two that *are* built, FR-PLG-2 and FR-PLG-2d, are the declarative operation format, which is a plugin system that resolves at build time. The fix here is an edit to requirements.md, not code, and D16 has to be answered before any format is called stable. FR-DSP-2 is not merely unbuilt; it is under challenge. frame_budget.rs and TD-4 independently argue that tiling costs more than it saves on the interactive path, so the open question is whether the requirement should survive, not when it will be met — and the spike that would settle it, S6, has not run. NFR-A11Y-1's hard half is done and its easy half is not. LocalizedKey already keeps display strings out of core/ and labels::resolve is a single resolution point; what that point does is a hardcoded English match, so a translation needs a recompile, which is the one thing the requirement forbids. The §4.1 performance targets are unverified rather than unmet. §8 requires a per-commit benchmark suite whose regressions fail the build; there is no benches/ directory, no criterion, and no benchmark step in any of the three CI workflows. The one guard that does live in CI skips itself without a GPU adapter and asserts its CPU half only in release, while CI tests in dev. Nothing says the targets are missed. It says nobody would find out. And three requirements are reported as covered while not being met: R1 and NFR-OPS-1 are tagged only by string literals inside the traceability tool's own tests, which it scans along with everything else, and FR-CAT-13's single tag sits on keyword storage while no XMP is parsed or written anywhere. CONTRIBUTING already warns that a tag proves a tag exists; these are the specific ones. Focus peaking, burst grouping, Flatpak packaging and the Android platform integration are being built in parallel and are marked in progress rather than listed as absent, so those lines can be struck as they land. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 20:24:26 +02:00
dtourolleandClaude Opus 5	8165619065	Describe the project the README is actually in front of ... It said the status was "early", that v0.1 is a remote library viewer, and that the zero-copy display path was "not yet working" and its replacement was the highest priority. All three were true when they were written and none of them has been true for eight releases. The zero-copy line was the damaging one, because it is the project's central architectural constraint and the README stated the opposite of what happened. Desktop keeps the zero-copy path — the compute pass writes a texture Slint composites directly — and the readback survives in exactly one place, the Android develop view, because wgpu's Vulkan swapchain tears a portrait window on a tablet whose panel is mounted landscape. That is TD-1, with the on-device measurements and the three things any one of which would remove it. A reader who took the old text at face value would have gone looking for a bug that was fixed, and missed a compromise that was chosen. "Current state" now says what runs, checked item by item against the tree rather than from memory: the first draft claimed AVIF and JPEG XL export, which the settings page offers and dr-export refuses on purpose, and sixteen declared operations where there are fifteen. Both are the kind of error this commit exists to remove. The requirement count moves from 122 to 179, the documentation table gains the five documents somebody would actually want next, and the "Not built" paragraph points at docs/outstanding.md rather than leaving the reader to infer the gap from silence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 20:23:35 +02:00
dtourolleandClaude Opus 5	ab0ef6a26d	Say which requirements the code was already satisfying ... Thirteen requirements were surveyed as built but untagged. Eight of them were: R3, R6, FR-DEV-1, FR-UI-6, FR-NC-6d, NFR-OPS-3, NFR-PORT-2 and NFR-SEC-3. Each was read against its full text in requirements.md and against the code before the tag was added, because a tag that is wrong is worse than an absent one — it turns a visible gap into an invisible one. The five that were refused, and why, because the reasoning is the part worth keeping: R2 carries "(figure TBD)" in its own acceptance criterion and asks for a stated prefetch margin and cache-hit rate; neither figure exists anywhere in the tree and neither quantity is measured, while TD-2 and TD-3 both describe the thumbnail path falling short of it. R5 asks for three things and the code does one. The display pipeline does run at viewport resolution, but "only visible tiles are computed" and "panning recomputes only newly exposed tiles" need a tile scheduler that does not exist — and frame_budget.rs currently argues for striking tiled computation from the interactive path rather than building it. FR-RAW-2 asks for a trait taking a SourceRef, so that a second decoder can be added without changing callers. What exists is free functions over &[u8]. That meets the requirement's stated *purpose* — the same decoder serves a local file, a SAF document and a byte range, which is exactly why it takes bytes — but there is no trait and no second implementation seam, so the requirement should probably be amended rather than tagged. NFR-ARCH-1 asks for named executors with stated thread counts. architecture.md §7.1 states the table; nothing implements it. Workers are twenty-odd ad-hoc std::thread::spawn sites, each building its own one-worker tokio runtime, with no decode pool, no GPU-submit executor and no I/O pool. The requirement's own text says R4 and NFR-P9 "assert an outcome with no stated means", and that is still true. NFR-SEC-4 is satisfied by absence — there is no telemetry — and absence has no module to tag. A tag would point at nothing. NFR-OPS-3 was the closest call of the eight taken. The store is single, separate from the catalog, survives a catalog rebuild and does not sync between devices; it has no version *field*, deliberately, and settings.rs argues why and names the condition that would need one. The substance is met and the reasoning is recorded where it belongs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 20:23:16 +02:00
dtourolleandClaude Opus 5	a56ac87e2c	Build a Flatpak that asks for no filesystem at all ... FR-PLAT-LIN-3 says that where DarkRoom is distributed as a Flatpak, filesystem access uses portals. There was no manifest, so the sandboxed path had never been exercised and the requirement had never been tested against the code. The manifest is YAML rather than JSON because it has more to explain than to declare, and every permission in finish-args carries the argument for itself. The two that are not obvious: - --device=dri is not an optimisation. The develop pipeline is compute shaders through wgpu with nothing behind it while NFR-R8 is open, so without the render node the application starts and cannot develop. - --talk-name=org.freedesktop.secrets rather than the Secret portal. These are different things and the requirement's wording invites the wrong one: the portal hands an app a master key for a store it keeps itself, whereas the session's secret daemon is what keeps the Nextcloud app password visible to secret-tool and Seahorse, and therefore individually revocable by the user. FR-NC-2's degraded mode is what happens if nothing answers, inside the sandbox exactly as outside it. There is no --filesystem= line, and that absence is the substance rather than an oversight. It leaves the document-portal path working — a photograph opened from a file manager arrives in argv under /run/user/$UID/doc and opens with no code change — and leaves library selection broken, because dr-sync-folder takes a typed absolute path and nothing in the tree calls the FileChooser portal. --filesystem=host would fix that and is precisely what the requirement forbids; --filesystem=xdg-pictures would fix it by not testing the design, in a smaller directory. docs/distribution.md §4 records what actually closes the gap and the flatpak override to use in the meantime. The runtime version is chosen from what the binary needs rather than from what is newest. ldd on a release build names fontconfig, freetype, expat, libpng, zlib, brotli and bzip2 and nothing more: wgpu dlopens libvulkan.so.1, and x11rb and wayland-client speak the wire protocols in Rust rather than binding libxcb or libwayland. So what the runtime must supply at runtime is a Vulkan loader and an ICD, which is the GL extension's job, and freedesktop 25.08 carries a rust-stable extension at 1.98.0 — comfortably above the workspace's 1.92 minimum. That extension is not rustup, so rust-toolchain.toml's pin is ignored here; the manifest says why that is correct rather than a violation of CONTRIBUTING.md, since the pin exists to make fmt and clippy agree and neither runs in a packaging build. Like the PKGBUILD, it builds from the local checkout, so a build is of what you are working on. That needs network for cargo, which Flathub forbids — the comment says what a submission there would need instead and why generating 30,000 lines of vendored sources buys nothing yet. The LFS pointer check is carried over from the PKGBUILD for the same reason it exists there: a dir source copies a 130-byte pointer in without complaint, and the failure would land on a user's machine rather than the packager's. Not verified: nothing has been built. flatpak-builder is not installed here and the machine is under a build embargo. The manifest parses, its keys are the ones flatpak-builder reads, and the desktop entry and metainfo it installs both validate — but no Flatpak has been produced from it and no permission has been observed to be sufficient. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 20:18:56 +02:00
dtourolleandClaude Opus 5	47a40d1afa	Describe the application once, in a file every channel installs ... A desktop entry gives a software centre a name and a one-line Comment, and nothing else — no description, no licence, no age rating, no statement of what hardware the interface was laid out for. GNOME Software and Discover both show an application with no metainfo as an unexplained icon, and both packages this repository produces were in that position. packaging/paris.tourolle.darkroom.metainfo.xml is the AppStream component, and it is installed by the PKGBUILD as well as by the Flatpak manifest because the description, the licence fields and the OARS rating are facts about the application rather than about how it was packaged. Writing it twice is how the two packages start disagreeing. Two things in it are easy to get wrong and are commented in place. The two licence fields differ on purpose: metadata_license covers the file itself and has to permit the unconditional redistribution and reformatting a catalogue does, which GPLv3 does not, so it is CC0-1.0; project_license is the application's own and reads GPL-3.0-or-later to match D8. And the component id is not a fifth name but the same string as the desktop basename, the Flatpak application id, and the app_id dr_ui::run sets — a rename that misses one costs the icon or the association, and neither failure announces itself. D15's decision that the target devices are a tablet and a desktop is stated as a display_length requirement rather than left implicit, so a software centre does not offer this on hardware where the photograph and the parameter panel cannot both be on screen. Validates clean under appstream-util validate-relax; appstreamcli --pedantic reports only that the gitea URLs are unreachable from a machine that cannot see that host. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 20:18:35 +02:00
dtourolleandClaude Opus 5	95847e3a31	Say which channels v1 ships through, and that the Flatpak cannot reach a library ... NFR-COMPAT-2 asks for the v1 channels to be stated, and says why in its own second sentence: the channel decision and the storage design are coupled. There was nowhere that statement lived. packaging/ held a PKGBUILD and a desktop entry, which is a recipe rather than a decision, and the coupling the requirement points at was therefore invisible. docs/distribution.md states five channels and, more usefully, which two of them exist only as promises. It also records what every channel has to get right independently of format — the one identifier that appears in four places, the metainfo, Vulkan being a requirement rather than a preference while NFR-R8 is open, a secrets daemon being optional rather than required, and the LFS pointer check that stops a package shipping 130 bytes where an 11 MB model should be. §4 is the part worth reading. Preparing a Flatpak is what surfaced that FR-PLAT-LIN-3 is not satisfied and cannot be satisfied by packaging alone: a folder library is chosen by typing an absolute path into an EndpointOnly field that checks it with std::fs, and nothing in the tree calls the FileChooser portal. Inside a sandbox that path does not exist, so the launch screen refuses it. Import fails one step earlier, because a sandboxed process reads its own mount namespace and a card mounted on the host is not in it. That is written down rather than fixed with --filesystem=host, and the argument for not fixing it that way is §2: the Arch package and an AppImage both hand the application the same unrestricted process the developer runs it in, so Flatpak is the only Linux channel that tests whether a design assumed unrestricted access. Granting the permission removes the only reason to ship it. The reverse coupling on Android is recorded too. NFR-COMPAT-2 says Play distribution is what makes ARCH §6.9 binding; §6.9 is verified rather than assumed, so SAF is already unconditional and a sideloaded build would gain nothing by asking for more. Play is deferred over the GPLv3 question, which is a licence-reading exercise and blocks nothing in the storage design. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 20:18:20 +02:00
dtourolle	eb39599f12	Merge: named presets (FR-DEV-6)	2026-08-29 20:07:58 +02:00
dtourolleandClaude Opus 5	5a8327824f	Keep an edit under a name, not just on the clipboard ... FR-DEV-6 asks for three things — named presets, copy/paste between images, and batch-apply to a selection. The last two have been here for a while; this is the first. The format is the sidecar's, deliberately. A preset *is* the non-default half of a version, so the lines are the same lines keyed the same way, which makes the two files diffable against each other and lets someone debugging an edit paste a block from one into the other. One file rather than one per preset: a preset per file makes the name a path, and every name then has to survive a filesystem — a `/` becomes a directory, a name differing only in case collides on one platform and not another, and renaming becomes two operations that can half-fail. As a key in a document it is none of those. Unknown *parameters* needed no machinery. `Preset` already holds whatever keys it is given and resolves them against the descriptors only at apply time, so one written by a newer build survives by being stored. Only lines that are not `op.param = float` at all are preserved verbatim, which is the sidecar's version-skew promise made here too. Applying is the paste path with a different source, so a preset reaches a selection through the sidecar read-modify-write that was already there: no graph, no decode, no GPU, forty files or one. Two smaller decisions worth the record. A library that fails to parse is held empty in memory and *not* written back over — settings regenerate themselves and this is work, so a parse failure must not be the moment it is destroyed. And every save persists immediately and rolls the in-memory copy back if the write fails, so the sheet never lists a preset the file does not have. The grid's "Presets" button is gated on the selection alone, unlike the "Paste to 40" beside it. That button needs a clipboard armed this session; the preset list is whatever was saved last month, and hiding it behind an unrelated action is what makes a feature only its author knows about. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-29 20:07:14 +02:00
dtourolleandClaude Opus 5	574107bc39	Let a manual collection be put in the order it is meant to be seen in ... `collection_members.position` and `Sort::CollectionPosition` have been in the catalog since collections were, and nothing above dr-catalog has ever written or read either: `collections::set_order` had no callers, and the grid ordered everything by capture time whatever it was scoped to — dr-ui does not construct a `Query` at all, it has its own `GRID_ORDER` constant. So a manual collection was a set with an order nobody could see or change. Three pieces, because it could not be fewer: `grid_order_for` decides the ordering from the scope, and both readers take it from there. That is the load-bearing part. An ordinal only names a photograph relative to an ordering, so the window read and the span read have to agree — a shift-click resolved through a different ORDER BY than the cells were drawn with selects a different run than the one on screen, and the user finds out when the export runs. `read_ids_span` already stated that invariant about `GRID_ORDER`; this widens it to an ordering that depends on the scope. Only a single manual collection has one. A set draws its descendants' images too, and two children's positions are unrelated integers that interleave arbitrarily; a smart collection has no member rows to carry a position at all. Both fall back to capture time and refuse the drop rather than pretending. The drop is on the cell, on whichever half of it the finger landed — the trailing edge is the only way to name the last place in a collection, since there is no cell beyond the last one to drop in front of. `reordered` is pure and the membership is rewritten whole. `set_order` sets the positions it is given and leaves the rest, so a partial write would interleave the moved run with rows nobody touched; and it is read unfiltered, so what the filter is hiding keeps its place relative to what the user can see. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 20:04:17 +02:00
dtourolleandClaude Opus 5	6d25d85f18	Make the range gesture visible, and offer the whole grid at once ... Touch has had a range gesture for as long as selection mode has: double-tap the far end. It is invisible, it is unreliable on a grid that scrolls under the second tap, and it extends from the anchor *before* the two taps moved it — a rule subtle enough that the code needs two paragraphs to explain it to itself. Nobody who was not told about it has ever used it. "Select to…" is the same operation with state you can see. Press it, the strip stops reporting and says "Tap the last photograph", and the next cell taken is the far end. It reaches Rust as shift on `cell-pressed`, so it lands in `apply_press` as the ctrl+shift it already is, and there is no third selection policy to keep in step with the other two. This is deliberately not the sweep gesture. A drag that paints cells can only reach what is on screen, and the ranges that hurt on a tablet are longer than a screenful — between the two taps here the user may scroll as far as they like, and the run is resolved by the catalog rather than by what happened to be loaded. A sweep is still worth having for short runs; it is not what this should have rested on. "Select all" beside it, asked of the catalog for the same reason: a select-all that quietly meant "the hundred cells that happen to be loaded" is a lie the user cannot see until the export runs. The double-tap stays. It is tested, and an accelerator that costs nothing is worth keeping for whoever has already learnt it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 19:46:44 +02:00
dtourolleandClaude Opus 5	9ac1447139	Ask for the collection's name where the keyboard can reach it ... "New collection from selection" created the collection under a placeholder name and then opened the rename field in the sidebar tree. On a tablet the sidebar is not on screen. It is instantiated all the same — app.slint collapses it to zero width and `visible: false` rather than using an `if`, because an `if` there is a layout loop Slint panics on — so the rename field was created, its `init` took focus, and Android raised the on-screen keyboard over a box nobody could see. Nothing else on the screen is focusable, so the keyboard had nowhere to go: it stayed, the name could not be typed, and the collection was already written under the name the user did not want. Asked in a sheet instead, on the same card as the filing and keywording sheets, before anything is written. That also fixes what was hiding behind it: an abandoned rename used to leave a "New collection" in the tree, because the collection existed before the name did. `Field` gains `take-focus()` so a sheet whose field is the only thing to do in it can answer the keyboard for the user — a function rather than a property, because focus is an event and a bound property would re-take it on every unrelated re-evaluation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 19:42:33 +02:00
dtourolleandClaude Opus 5	5133e53bc8	Give back what a straighten took, when the angle comes back ... The auto-crop only ever shrank. Straighten to 20 degrees and the corners are cropped away correctly; come back to 3, or all the way to zero, and the crop stays at the size 20 degrees demanded. Nothing on screen explains why the photograph is still small, and the only way back was undo. The cause was that each correction was computed from the previous correction's output, so it accumulated: every angle the slider rested at took its cut and none was ever returned. The fix is to stop accumulating and recompute. The applied crop is now always the user's own rectangle fitted into the current angle's safe area, so as the angle falls and that area opens up the crop grows back — and stops, exactly, at the rectangle they chose. At zero the safe area is the whole frame and the fit is the identity, which is what carries it the last of the way home. There is deliberately no early exit for the upright case now: that exit is precisely what would strand the crop small. **The intent is remembered as a pair, so it repairs itself.** The session keeps `(applied, intended)` — what the correction wrote, and what it was derived from — and trusts the remembered intent only while the graph still holds `applied`. Every other route to the crop leaves something else there: a handle dragged, a ratio chosen, a sidecar loaded, a paste, an undo. That mismatch is the signal the memory is stale, and the current rectangle becomes the new intent. The alternative was a write into this field from each of those paths, which is the kind of bookkeeping that is correct until someone adds a seventh path. Dragging a handle therefore *is* the user choosing, including at a non-zero angle: the correction will not later grow the crop past what they dragged it to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 19:16:43 +02:00
dtourolleandClaude Opus 5	ff6c313bab	Wrap the chip rows, so one six-choice parameter stops sizing the sidebar ... The develop column asked for 482px. Every comment in it, a dozen of them, describes it as a 280px column — and on a tablet it was taking 40% of the screen from the photograph it exists to serve. Measured rather than guessed, because none of it is visible in the source: the column takes the widest width any panel declares, `AdjustPanel` wanted 482 of it, its rows wanted 458, and after the 44px scroll gutter the widest single row was 414. That row is `film_sim`'s `format` — six film formats from 35mm to 8x10, laid out by `Segmented` as six 64px chips in a `HorizontalLayout` that cannot wrap. 6 x 64 + 5 x 6 = 414, exactly. Nothing about that is the film simulation's fault. An operation declares its parameters and the panel decides how to draw them (FR-DEV-3a), so a node is entitled to offer six choices; it is the drawing that has to cope. Any future operation with a five-choice enum would have done the same thing, silently, to every screen in the application. `ChipGrid` is the general answer: chips placed by index arithmetic inside a plain `Rectangle`, wrapping at a column count, declaring a width that depends on the columns rather than on the number of choices. `Segmented` takes a `columns` property and uses it when asked — zero, one row however many chips, stays the settings page's behaviour, where the page is full-width and reading the alternatives side by side is the whole argument for chips over a dropdown. The generated enum rows and the curve-channel picker now wrap at three, and the crop ratio chips use the shared grid instead of the private copy of it they shipped with last week. The column measures 351 now, down from 482, and what sets it is the mode strip rather than a parameter — which is a control the user chose to have on screen rather than an accident of one node's variant list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 19:16:43 +02:00
dtourolleandClaude Opus 5	bbd26f05f9	Release 0.9.0 ... Thirty-five commits since 0.8.0, and two of them are the reason this is a minor rather than a patch. **Storage became pluggable.** A folder backend now sits beside the Nextcloud one behind the same seam, proved by a test rather than by a trait, and the launch screen offers three routes into a library instead of one. **Faces gained a confidence that means something.** A suggestion is scored against the people the user has actually named, the curve it came from is stated rather than implied, and a regroup runs roughly three times faster — the similarity scan and the merge engine both use the machine's own SIMD kernel now, measured on the tablet where the NEON path is the one that runs. Alongside those, the framing tools got the quality-of-life pass this release is named for: a crop can be held to a ratio, a straighten crops away the corners it exposed, an export can be pinned to an exact resolution with the common panel sizes offered as buttons, and dragging the crop rectangle no longer tracks the pointer at half speed. `pkgrel` returns to 1: a new `pkgver` is a new archive name, so there is nothing left for a release number to disambiguate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:53:19 +02:00
dtourolleandClaude Opus 5	bd33487054	Say which axis each export dimension is, in the one place that renders ... The box modes put two numeric fields one above the other, and on screen they are two anonymous numbers: `TextRow` draws its label behind the field rather than above it, so "Width" and "Height" never appear. That fault is older than this feature — the storage panel's cache sizes have the same missing labels, and the single "Size value" row always did — and it belongs in its own change rather than being fixed under cover of this one. But one unlabelled number is survivable and two are not, so the axis goes where the page does render it: the unit. It reads "3840 px wide" above "2160 px high", which is the sentence the user is trying to write anyway. The `label` bindings stay correct and stay where they are, so this becomes redundant rather than wrong the day `TextRow` is fixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:49:21 +02:00
dtourolleandClaude Opus 5	c38df01bf7	Hang the auto-crop off the slider's commit, not off the pointer passing over ... The straighten auto-crop worked once and then stopped. It was keyed on `PlainSlider::drag-changed`, whose name is a lie inherited from what it forwards: `SliderTrack` defines `engaged` as `has-hover || claimed`, and it has to — a `Flickable` withholds the press for 100ms, so hover is the only signal that arrives in time to stand the scrolling ancestor down. That is the right definition for the job it was written for and the wrong one for this. Keyed on hover, the correction fires when the pointer first crosses the track — before anything has been dragged — and then does not fire again for as long as the pointer stays on it, however many times the angle is changed. Which is exactly what "it only works once" looks like. `SliderTrack` already publishes the signal this wants. `committed` fires on release, once per gesture, after the final `changed`, and its doc comment says so in as many words. It was simply not forwarded through `PlainSlider`, so it now is, and the geometry panel's callback is a `committed(float)` rather than a `drag-changed(bool)`. The angle needs no re-applying here: the track emits its last `changed` before it commits, so the value is already in the graph by the time this runs. What is left is the correction that has to happen exactly once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:49:21 +02:00
dtourolleandClaude Opus 5	2c8768ac29	Lay the ratio chips out by hand, because Slint will not lay them out ... The ratio chips shipped inside a `GridLayout` with `row` and `col` computed from the repeater's index. It compiles. On screen every chip piles into a single row that runs off the panel, and the terminal fills with Internal error in Slint: RepeatedItemTree::grid_layout_input_data() not implemented once per frame. A `for` inside a `GridLayout` is not supported, and nothing says so until it is running. A `HorizontalLayout` is not the answer either: six chips side by side need over 400px, and this column's width is the largest width any panel declares — so one row would widen every other panel in the application to fit a control that is on screen only while cropping. So the grid is arithmetic over the index inside a plain `Rectangle`. It costs the layout engine nothing, it wraps a seventh ratio onto a third row by itself, and — because a bare `Rectangle` declares no preferred width — it takes the width the column already has instead of setting it. The Portrait chip gets a container of its own for the same reason: a `ChoiceChip` dropped straight into a `VerticalLayout` is stretched the full width of the panel and reads as a button for the section rather than as one more chip. The three conditional pieces are also now individually-conditional children of the one layout rather than a nested layout under a single `if`, which is the convention `app.slint` and `masks.slint` already carry notes about: a conditional nested layout under-reports its height here and the panels below it draw on top of one another. All of this was invisible in the source and obvious in a screenshot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:49:20 +02:00
dtourolleandClaude Opus 5	52c655e6cf	Turn the ratio lock with the photograph when it is turned ... A quarter turn carries the crop with it — that is what makes turning a photograph keep its composition rather than sliding the selection onto a different part of the picture. So a rect locked to 16:9 comes out of the turn at 9:16, of a frame whose axes have also swapped, and the lock was left claiming landscape over a portrait rect. The next drag would then snap it back upright and undo what the turn had just done. The orientation switch now turns with it, on odd numbers of quarters. `Original` is deliberately excluded, and getting that wrong flips it twice: it is resolved against the framed size every time it is asked for, and the turn has already swapped that frame's axes — so it has turned by the time anything asks. `turns_with_the_frame` is the one predicate that separates the two cases, with a test that pins both. `CropAspect` arrived without tests of its own; it has them now, including the round trip this fixes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:49:20 +02:00
dtourolleandClaude Opus 5	3bb68cff69	Export at an exact resolution, and offer the panels worth naming ... Export sizing could bound an image but not fix it. Long edge, short edge and percentage all preserve the aspect ratio by letting one dimension fall where it may, which is right for most work and useless against a display that accepts one resolution and rejects everything else — a television's art mode, a digital frame, a wallpaper slot. FR-EXP-3 has always listed both halves of the answer, and this adds them. **Fit box** scales to fit inside a width and height, so nothing is thrown away and the result is smaller than the box on one axis unless the crop already matches it. **Fill box** scales to cover the box and cuts the overhang off the middle, so the file is exactly the pixels asked for. Fill is the only mode in the file that discards image data, so two things about it are worth stating. The overhang comes off symmetrically: the crop tool is where a photographer decides which part of a frame survives, and this stage having an opinion of its own would fight it. And locking the crop to the same ratio leaves nothing here to cut, which is the workflow the two features are meant to be used in. With upscaling off and a source too small to cover, a fill box keeps its *shape* rather than falling back to the source's: exporting a 3:2 file where 16:9 was asked for is silently wrong in exactly the way the mode exists to prevent, so the box shrinks instead. The existing rule — clamp, never fail — is otherwise unchanged. Four panel sizes are offered as buttons beside the fields. Getting 3840 x 2160 by typing four digits twice is a step at which the mistake is discovered after the upload rather than before it. They fill in the numbers and nothing else, in particular not the fit/fill choice: both are legitimate against a screen, and guessing would discard the edges of a photograph for a user who wanted them. The list is panels rather than platforms, because a screen has one exact pixel count for ever where "what a photo site wants" would rot in the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:49:20 +02:00
dtourolleandClaude Opus 5	d9eb8faffd	Crop away the corners a straighten exposed, once the slider is let go ... Turning a rectangle inside its own bounds exposes its corners: there is no source pixel out there, and the shader renders it black. Nothing in the render prevents that, deliberately — a free angle does not change the output size, which is what leaves the frame where the user put it while the slider moves. Correct during the drag; four black wedges on the finished photograph. Letting go of the slider now pulls the crop inside the area the angle leaves defined. `Framing::max_inscribed_crop` already computed that bound and had no caller; this is the caller its doc comment described. **Once, at the end of the gesture.** Applied per frame it would shrink the crop on every step of the slider and never grow it back, so a user who overshot to 20° and came back to 3° would be left with a crop ratcheted down by the excursion rather than by the angle they settled on. Per gesture it is bounded by the angles actually rested at, and undo steps back through them. **The crop is fitted into the bound, not replaced by it.** A crop placed deliberately off-centre is a decision, and an automatic correction that recentred it would undo the user's work to fix a problem they did not have. `CropRect::fitted_into` scales only as far as the bound demands and then slides the rect the shortest distance needed to be inside — so a ratio locked in the crop panel survives the straighten too, since the shape is never touched. It returns the rect unchanged, bit for bit, when nothing needed to move. That matters more than it looks: this runs on every release of the slider, including releases at zero, and a rect that drifted by a rounding error each time would be an edit recorded for no reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:49:19 +02:00
dtourolleandClaude Opus 5	e6ad906bc1	Let the crop be held to a ratio while it is dragged ... A photographer cropping for a print, a phone wallpaper or a 16:9 frame is not choosing four edges — they are choosing one edge and a known shape. Free-dragging every corner made them do that arithmetic by eye on every drag, and get it slightly wrong. The panel now offers Free, Original, 1:1, 3:2, 4:3 and 16:9, with a Portrait switch for the ones that have two orientations. Original follows the frame rather than naming a number, so it stays right on the next photograph from another body and after a quarter turn. **The ratio is of output pixels, and the rect is not.** `CropRect` is stored in fractions of a frame that is not itself square, so holding a shape needs the frame's size — `ratio * height / width` of the frame. Skipping that gives a "1:1" crop that is square only on a square photograph, which is the one case nobody would test on, so the conversion lives in `CropRect::with_aspect` where it is explained and pinned by a test that asserts the fractions are *not* equal. Two decisions worth recording: The reshaped rect **grows** onto the ratio rather than shrinking onto it, then scales down only as far as the frame's edge demands. Fitting inside instead makes a one-axis drag do nothing at all — the other axis clamps the first straight back, and the handle simply refuses to move. The overlay now reports **which corner the drag is holding**, because reshaping onto a ratio has to know which corner is nailed down and only the handle that took the press knows that. A move reports no corner and keeps its shape: reshaping about a centre would pull an over-moved rect smaller instead of sliding it along the edge. The lock lives with the window rather than the session. A `DevelopSession` is per image, and cropping a set of frames to one shape is exactly when the lock earns its place. It is not an edit and reaches no sidecar — what is saved is the rectangle it produced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:48:51 +02:00
dtourolleandClaude Opus 5	dbaf5358d1	Measure a crop drag against the frame, not against the rect it is moving ... Dragging the crop rectangle tracked the pointer at half speed: the rect slid out from under the cursor, and the handle being held stopped being the one under the finger. On a photograph where the whole point is to place an edge by eye, that made the tool close to unusable. The cause is that a `TouchArea` reports `mouse-x` relative to itself, and every area in the overlay is positioned *by the very rect the drag is editing*. Rust echoes the applied rect back on each step, so the area moves under the pointer and the reported position falls by exactly the amount it had just risen. `mouse-x - pressed-x` therefore subtracts the drag from itself, and the fixed point of that feedback is a rect that moves half as far as the pointer does — which is why it looked like sluggish tracking rather than like a coordinate bug. Both terms are now taken in the frame's own coordinates: `parent.x + mouse-x`, where `parent.x` tracks precisely the movement `mouse-x` lost, with the press captured in those same coordinates on the way down. The two movements cancel and the rect follows the pointer exactly. `GradientHandles` in masks.slint was already written this way, and for the same reason — its handles are placed by the mask they drag. The header note here now says why the shape matters, since the wrong version compiles, looks plausible, and is only wrong once the rect starts moving. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:48:44 +02:00
dtourolleandClaude Opus 5	480c46c41a	Quote the percentile the measurement can actually support ... The before/after published a p99 ratio of 6.0x at 4K. It is not supported and the correction is worth more than the number was. The baseline run's `fit` rows spread 2.3x between median and 99th percentile while every row of the after run spreads about 1.1x. A stage costing `radius x pixels` has no reason to be bimodal, and `fit` is the memory-bound configuration — it walks the whole 482 MB source on a stride where `1:1` reads a contiguous window. Something else had the machine. The merge brings in the cross-check that settles it: "Try every GPU, not only the fastest one" measured the same baseline code on the same card and reports 4.67 ms p99 for M3 clarity fit at 2560x1600, against 10.40 ms here. Two measurements of one thing differing by 2.2x mean the noisier one is wrong. So both percentiles are now published and the p50 column is the claim: 2.8x at 4K rather than 6.0x. The p99 improvement is real and larger; this run cannot say by how much, and says so. What the caveat does not touch: every after figure is inside the 16 ms budget with a p99 within 26% of its median at every size and both views, and clarity against all-four is a within-run comparison. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:28:41 +02:00
dtourolle	30162e80df	Merge branch 'master' into clarity-reduced-base ... # Conflicts: # docs/traceability.md	2026-08-29 13:27:04 +02:00
dtourolleandClaude Opus 5	8b251b84a0	Record what the reduced base actually bought ... TD-4 asked for the measurement as well as the change, and this is it: 25.05 ms to 4.17 ms at 3840 x 2160, six times faster, with clarity no longer dominating the neighbourhood stage it used to be 97% of. Measured before and after on the same machine and the same adapter minutes apart, baseline at the branch's merge-base, so the only variable is the change. That adapter is not the RTX 3050 the rest of this document was measured on, so the new table says to read it on its own rather than against the ones above — the before/after is comparable, the absolute figures are not, and quietly replacing the existing tables would have changed the instrument. Also recorded: the declared halo is now quantised to multiples of the output scale, because the 2-sigma truncation rounds on the reduced grid. 29 px becomes 28 at 1920x1200 and 38 becomes 40 at 2560x1600. It is inside what the cross-form test holds — 0.03 stops of peak, 2% of reach — but it is a change in reach and not only in cost, and a tile scheduler would be handed it. Better written down now than found later as a seam. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:18:37 +02:00
dtourolleandClaude Opus 5	bff95e25ad	Let clarity's base be computed where it is still fully determined ... Clarity's Gaussian sigma is 1.2% of the frame's shorter edge, so its radius is a property of the viewport: 52 render pixels at 4K, two separable passes of 105 taps each over 8.3 M pixels. That measured 33.9 ms — seven times the entire fused point chain, for one slider — and is docs/technical-debt.md TD-4. A detail pass may now declare `output_scale`, and clarity's base is computed on a grid a quarter the size on each axis. The pass that combines needs the blur *and* the full-resolution colour, and a colour that has been through a quarter-scale target is no longer full resolution. So a scaled pass cannot simply join the ping-pong: there are two chains now. The full-resolution one carries the colour and no scaled pass touches it; the reduced one carries the base and reaches the combining pass through a second binding as `reduced_at()`. The reduce is a dispatch of its own rather than something the first blur half does on the way past, and that is the whole difference between this and the strided kernel the module documentation rules out. A stride samples an image that is not band-limited and aliases high-frequency content down into the base, which is then subtracted, and arrives in the output as mottling across smooth gradients. This band-limits first and samples after. What is discarded is content the base could not represent at any resolution, because a Gaussian at sigma = 26 px holds nothing above one cycle per 26 px and the quarter-scale grid carries one per 8 — so the reduced base is not an approximation of the full-resolution one, it is the same function sampled where it is still determined. Which is also why the scale belongs to the band rather than to the stage. Texture's sigma is a decade finer, so the reduce pass's own box would be wider than the Gaussian it was prefiltering; texture never reduces. And clarity steps 4 -> 2 -> 1 as sigma falls, because a quarter of a small sigma is not a Gaussian either — the case that gives up is the one that was already cheap. `radius` stays in each pass's own pixels and `ComposedDetail::radius` multiplies it back up, so 13 reduced pixels at scale 4 still report the 52 render pixels a tile would have to be grown by. The halo a scheduler sees does not move. The halo tests pass unchanged, which was TD-4's stated bar; they render at 1024 px and so exercise the reduced path rather than stepping around it. Added `crossing_the_reduction_threshold_does_not_change_the_picture`, because nothing yet compared the reduced form against a *less* reduced one — every other test measures one form against itself. It renders the same edit either side of the 4 -> 2 step-down and holds the peak excursion to 0.03 stops and the reach to 2% of the frame. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:12:29 +02:00
dtourolle	3b5d564495	Try every GPU, not only the fastest one ... `request_adapter` with `HighPerformance` returns one adapter and no second chance. That is right on a healthy machine and wrong on one with a sick GPU, which is not rare: observed 2026-08-29 on a laptop whose discrete card had hit an NVRM assertion failure and a fullchip reset. The driver still advertised it, wgpu dutifully picked it as the highest performing, and the process died on it — while a working integrated GPU and a working external card sat unused in the same enumeration. A photo editor that will not start because the *fastest* GPU is broken, on a machine holding two that are not, is worse than a slow one. So: enumerate, order by preference, take the first that yields a device. The ordering reproduces what `HighPerformance` meant, so a healthy machine picks what it always picked and pays one enumeration for it. A CPU adapter sorts last rather than being excluded — software rendering is a poor experience and a working one. Which GPU to prefer is now a policy rather than an assumption, because the fastest is not obviously the right one. A 24 MP frame is ~96 MB of RGBA and every upload and export readback crosses PCIe on a discrete card, where an integrated GPU shares memory and crosses nothing — and does not empty a battery. Measured before choosing a default, on this machine's Iris Xe against its RX 5700 XT. The fused colour pass is within 1.5x, which is the shape shared memory suits. The neighbourhood stage is 5-8x slower, and that decides it: clarity at 1920x1200 costs 20 ms on the iGPU, over the budget on its own at the smallest size tested. So `Performance` stays the default and `Efficiency` is offered rather than chosen (`DARKROOM_GPU=integrated`). docs/frame-budget.md carries the table, and says what it does *not* show: the harness renders from a resident texture and never uploads or reads back, so the transfer cost an iGPU avoids appears in none of it. Import, export and the thumbnail sweeps may well go the other way. What this cannot fix: a GPU sick enough to accept `request_device` and segfault afterwards, which arrives as a driver crash rather than an error. It moves the boundary from "the preferred adapter is unusable" to "unusable and dishonest about it".	2026-08-29 12:43:10 +02:00
dtourolleandClaude Opus 5	0407fb8d2d	Format the two new examples ... They were written after the last fmt run and CI gates on --check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 12:33:23 +02:00
dtourolleandClaude Opus 5	3944e17aa5	Merge: face confidences that mean something, and a regroup three times faster ... Two things the People screen was getting wrong, and then the arithmetic underneath it. It refused to show a confidence at all until the library had fitted its own calibration, which needs 200 confirmed positive pairs — made on the screen that was withholding the number used to rank them. The default curve is the reference implementation's fitted one, a published operating point rather than an invention, so the percentage is shown and the screen says which curve it came from. The number itself was the mean similarity between a face and every other member of its group, which punished well-photographed people and never asked who else the face might be. It is now the mean of the best ten matches into the identity, times that identity's share of the evidence against every other identity the user has ruled on. Leave-one-out over this library's 2,702 confirmations across 54 named people: 99.33% placed on the right person, and the number shown for the right person moves from a median of 90.4% to 99.3%. Both of those were measured rather than argued, on a copy of a real 18,143-face library, and the instrument is committed with them. Measuring is also what found the rest. A regroup went from 10.0s to 3.1s: the similarity scan was walking the whole embedding array once per row and not vectorising at all, and the agglomeration was spending 3.06s having every component scan the entire pair list for its own pairs. The scan now picks its kernel per machine — AVX2 where the CPU has it, NEON on the tablet, where the whole suite and a full regroup have both now been run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 12:32:54 +02:00
dtourolleandClaude Opus 5	39c34d4e44	Say what actually counts as a rival, now that a name anchors too ... assign's denominator is the identities the user has ruled on, and it reads Cluster::person to find them. Master's "Let a name hold a group together" widened what sets that field: a confirmation, a name, or an ignore, where before it was a confirmation alone. The behaviour is right either way — a named person is exactly the identity a suggestion should be discounted against — but the module note and faces.md §9.1 both said "a confirmation", which is now too narrow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 12:32:31 +02:00