An audit of every action by view against the keys the handlers bind left
develop without zoom, pan, fit or a way back to the grid, the grid without
select-none, thumbnail size or keywording, People with no key at all, and
the export and copy sheets without Enter. It also found the reverse gap
FR-UI-5 forbids: pick and reject had no route but P, X and U, and the
2026-09-19 amendment's judging in develop had not been built.
Develop: Ctrl+= and Ctrl+Plus zoom in and Ctrl+- out about the middle of the
view, Ctrl+0 fits and Ctrl+1 goes to 1:1, Shift and an arrow pan a magnified
view, G goes back to the grid, Ctrl+Y redoes, and Enter keeps a crop that hid
a mask. 0-5, P, X and U rate and flag the open photograph without moving on,
with stars and Pick/Reject in the top bar as the pointer and touch route.
= and - nudge the control last moved by a hundredth of its travel; the
framing sliders, perspective included, now count as "last moved", so R puts
them back as well. J turns the selected mask part's join chip.
Grid: Ctrl+D and Ctrl+Shift+A clear the selection, = and - resize the
thumbnails, Ctrl+K opens keywording, and Flag in the selection bar gives
pick and reject a pointer and touch route. Sidebar: Enter commits a
collection's name, and Enter or Escape hands the keyboard back to the grid,
where it used to go nowhere until something was clicked. People: Up and Down
walk the rail, F2 puts the name field under the keys, and Escape or Back now
leave the screen the way its back button does instead of doing nothing.
Sheets: Enter does what the export or copy sheet's button does.
The choices follow Lightroom where it has one. No new key steals typing: the
grid's and People's keys live on focus holders that are not ancestors of any
text field, and the sheets' Enter comes after a focused field has had it.
Every binding is tagged beside its handler, and the gate added in the
previous commit holds the two to each other.
The gesture book is generated from GESTURE tags, so it could not describe a
gesture nobody tagged, but nothing made anyone tag one. The arrow keys, Enter,
P, X, U, Delete, F1 and F2 all worked in the grid with no line in the help
sheet, and a tag could name a key whose handler had gone.
Key handlers now compare one canonical string, Keys.chord(event) == "Ctrl+Z",
instead of reading event.text and the modifiers themselves. keys.slint folds
the key and its modifiers into that spelling, so the literal in the handler is
the whole binding and the checker reads exactly what the handler dispatches
on. Each handler carries a KEYMAP comment naming the gesture-book section its
keys belong to, and a tag's keys field names its keys between backticks.
gestures-check now fails when a handler binds a key no tag in that section
names, when a tag names a key no handler there binds, when any .slint file
other than keys.slint reads event.text, when a compared literal is not
canonical, and when keys.slint's named keys drift from the Rust list.
Spellings are normalised in one place, chord.rs: Ctrl+z, Control+Z and
LeftArrow all mean what the handler's "Ctrl+Z" and "Left" mean. Shift and Alt
count only for letters and named keys, because on the French layout every
digit needs shift and a 6 has to be a 6 however it was typed.
A Rust keymap that both dispatched and was read by the generator was the
alternative. It would have moved the handlers' decisions away from the Slint
state they depend on, and a window that forgot to install it would have had
no working keys at all.
The keys that were already bound and undocumented are now tagged.
The sheet could now offer "See it", but no gesture said where to look.
Every GESTURE tag whose move the manual describes names that section:
the white-balance picker, zoom and pan, masks, undo and snapshots,
export, colour labels and ratings, selection, collections, the People
page, thumbnail size. Fifty of the fifty-one; the one left, putting a
single control back to its default, has no section and is too small to
earn one.
Three important gestures had nowhere to land, so the manual gains three
short sections, without pictures for now: Bursts (opening a folded
burst, choosing the frame it shows, and the eyes-open filter), Moving
between photographs (the roll, the arrows and A/D in develop) and
Copying settings (Copy, Paste, Paste to N, and choosing what a copy
carries). The page, the gesture book and docs/gestures.md are
regenerated from them.
The help sheet says which move does a thing, and the manual has a
picture of the thing being done, but nothing joined the two: a user
reading "Pinch it with two fingers" had no way from there to the GIF of
it.
A GESTURE tag takes an optional `manual:` field naming a heading of
docs/manual/README.md by its anchor. The scan checks every one against
the anchors the bundled page is rendered with and fails when the manual
has no such heading, so renaming a section cannot leave the sheet
linking to the top of the page; gestures-check carries the same failure
into CI. The anchor goes into gesture_book.rs as a new field, and into
docs/gestures.md as a "See it" link to manual/README.md#anchor. The help
sheet draws a "See it" button beside the title of each gesture that has
one, which opens the bundled manual at that section.
The field is additive: a tag without it is unchanged, and no gesture
carries one yet.
The packages now carry the manual, but nothing in the application opened
it: the help sheet listed gestures and stopped there.
The help sheet gains a Manual button beside Done, and Settings a Manual
row under About beside the version. Both go through dr_ui::manual, which
finds the installed page through dr_plat::system_data_dirs (the package's
share directory on Linux, the executable's directory on Windows), and a
development build also in the checkout it was compiled from. A copy with
no manual says so on the status line rather than doing nothing.
On the desktop the page goes to the system browser. A section is a URL
fragment, and xdg-open's generic mode and Windows' FileProtocolHandler
both turn a file: URL into a path and drop the fragment, so a section is
opened through a one-line redirect page written to the data directory:
the opener gets a plain path, which every opener keeps, and the browser
follows the redirect to index.html#section itself. The launcher behind
the sign-in's open_in_browser is split out so both share it; the https
check stays with the sign-in.
Android has no path to give a browser: an asset is not a file, a copy in
private storage is unreadable to other apps, a file: URI across apps is
refused, and a content: URI leaves the browser resolving every picture
against the provider. So ManualActivity, a WebView reading
file:///android_asset/manual/index.html straight out of the APK, shows
it, started by class name with the section as an extra. JavaScript is
off, links off the page go to the browser, and the theme is day-night so
the page's own light and dark follow the system. A test checks that the
manifest, the Java class and dr_ui agree on the name and the extra.
The rendered manual was in the repository and nowhere else, so an
installed application still had nothing to open.
Each packager now carries docs/manual/index.html and its pictures, to
where the application will look for them: /usr/share/darkroom/manual on
Arch, manual\ beside darkroom.exe on Windows (where the models already
are, and where dr_plat::system_data_dirs points), and assets/manual in
the APK, stored rather than deflated since a GIF or PNG is already
compressed. The manual is about 27 MB, which the APK and the installer
both grow by; the pictures are 1600x1100 screenshots and short GIFs,
and against an APK that already carries 170 MB of inference runtime and
70 MB of models they are not worth re-encoding for.
The pictures are LFS objects, so each packager refuses a pointer where a
picture should be, as it already does for the models: shipped, a pointer
is a manual of broken images that nothing reports. The Android and
Windows CI legs therefore fetch docs/manual/media, which they excluded
while nothing they built read it, and the installer smoke test checks
that the page and every picture were installed.
The manual existed only as docs/manual/README.md, which the forge renders
and nothing else does. An installed copy of the application, on a laptop
with no network or on a tablet, had no manual it could open.
`traces manual` renders the README to docs/manual/index.html with
pulldown-cmark (already in the tree as Slint's Markdown parser, so this
adds a dependency edge and no crate). The page is one file with an inline
stylesheet that follows the system's light or dark preference, a
contents list of every section and subsection, and the pictures by their
relative media/ paths. Each heading carries the id the forge gives it, so
README.md#rating-and-flagging and index.html#rating-and-flagging are the
same link. A picture alone in its paragraph becomes a figure whose alt
text is shown as the caption, and every picture reserves its 16:11 box
before it loads, so a jump into the middle of the page lands where it
aimed rather than a screenful above. Links to design documents, which the
installed page has no copy of, point at the forge.
The page is committed rather than rendered at build time, as the gesture
book is: it is user-facing text reviewed in the diff, and the three
packagers then only copy it. `traces manual-check` fails in CI when the
committed page is not the render of the README, and the pre-commit hook
regenerates it when the README is staged.
docs/gestures.md on master carried 162 lines of <<<<<<< / ======= / >>>>>>>
from 4642c77, ade627a, c3d1f83 and 46f5b95. Their branches were rebased
onto each other, the generated files conflicted, and the resolution
regenerated the requirements matrix with `traceability -- report` and then
staged gestures.md as it stood, on the assumption that the same command
writes it. It does not: the gesture book has its own `gestures` mode. The
source tags were never in conflict, so nothing is lost; this is the file
regenerated from them, and `gestures-check` passes on it.
The keystone existed in framing but nothing in develop could reach it:
framing is presented by its own Compose panel rather than generated, so
new framing parameters get no control until the panel names them.
Compose now has Vertical and Horizontal sliders under Straighten,
mirrored from the session like the angle, recorded as parameter steps
("Vertical Perspective" in the history), cleared by the Compose reset
and by opening the next photograph. Releasing either slider refits the
crop the way releasing the straighten slider does: a keystone alone
needs no crop, but it moves the empty corners of a straightened frame,
so the crop that avoided them before may not after, or may have room
to grow back.
There was no perspective transform anywhere in the pipeline: framing
offered a ±45° straighten, quarter turns and flips, and a building shot
looking up kept its leaning walls.
Framing gains a vertical and a horizontal keystone (-100..100). They are
parameters of framing rather than a new stage, so they carry its Compose
attribute, persist in the sidecar under framing, and are withheld from a
default paste exactly as the crop is. In the prologue the keystone runs
after the crop and the straightening and before the stored orientation
and the lens warp, so "vertical" is the photograph's displayed height and
the lens still sees its whole frame.
The map takes the output frame onto a trapezoid inside the source, built
as a homography from four corners and uploaded as three columns in the
framing uniform block (which grows from two vec4s to five). A keystone on
its own therefore never exposes an empty corner and leaves any crop valid.
Combined with a straightening angle the empty area is a pulled-back
quadrilateral the closed-form inscribed rectangle cannot describe, so
max_inscribed_crop searches for the largest centred rectangle whose
corners all have a source pixel behind them. source_at and output_at
apply the same map, so masks, gradients and spot handles follow it.
Issue #13 asks for a vertical and horizontal keystone, and its number was
renumbered from FR-DEV-19 when the spec gave that to mask editing. The
clause was never written into the register, so the work had nothing to
trace to.
It is written as part of framing: after the crop and the straightening,
before the stored orientation and the lens warp, carrying framing's
Compose attribute, and with the inscribed crop accounting for it.
The pre-commit hook left the matrix as it stood on two of the commits before this one, so it named neither the new test file's NFR-A11Y-3 tag nor the line numbers the label code moved. Regenerated from the tree as it now is.
The manual's library section described rating and flagging only, and
the outstanding register still said colour labels were "set and shown
nowhere" and that three NFR-A11Y-3 clauses had no test. Both are now
untrue: the manual gives the keys, the Label button and the chips, and
the register names the test file and what it can and cannot vouch for.
NFR-A11Y-3 was argued in comments beside the rating strip, the
pick/reject mark and the focus-peaking chips, and nothing would have
failed if an edit made a set star differ from an unset one only in tint.
Only the clipping readout had a test.
These read the markup, as the accessibility-name tests do, since a
rendered window cannot be asked what a colour-blind reader sees. The
star and the flag must choose their glyph from their state, and the
glyphs they choose must be different drawings in icons.slint. The peaking
chips must be distinct words that reach the screen as text. Each colour
label must carry its own letter and the name the catalog's code stands
for, the mark must draw the letter, and the grid cell, the filter chips
and develop must draw the mark or the name. Breaking any of these by
hand makes the matching test fail.
Colour labels could be read from a Lightroom sidecar and queried by the
selector, but nothing drew one or set one, so the only labels a library
held were ones another program had written.
Every mark carries its label's initial on its colour — R, Y, G, B, P —
so a label is read without telling red from green, which is what
NFR-A11Y-3 asks of colour labels by name. A grid cell shows the mark
before its filename. In the grid, 6, 7, 8 and 9 set red, yellow, green and
blue as Lightroom's keys do, on the photograph under the pointer or on
the selection by the rule the star keys follow; the same key again takes
the label off, and over a mixed selection it sets it on all. The
selection bar gains Label, which opens the six choices — each a mark and
a name — and purple, which has no key, is there. In develop the top bar
says "Label: Green" beside the mark, opens the same choices, and 6-9
label the open photograph.
Each gesture is one catalog transaction, then the grid, the counts and
both sidecars are written as a rating's are. The filter bar gains a chip
per label, its mark and its name with a count, one at a time; the filter
is one SQL term, travels in the place record, and "All" clears it.
A rating and a flag are written to DarkRoom's sidecar as well as the
catalog, because the catalog is a disposable index and the sidecar is
how a judgement reaches the photographer's other devices. A label had no
place there, so once labels could be set, one would have lived only in
the catalog of the device it was set on and gone with it.
The sidecar version now carries `label` (0 none, 1-5 as the catalog
codes it), written only when set. It merges under the rating's rule, so a
device that never labelled a frame cannot clear another device's label,
and a code this build does not know reads as none rather than as some
other colour. A judgement write carries the catalog's label with the
stars, and the scan takes a sidecar's label into the catalog when it has
one. An older build keeps the line as an unknown key and writes it back.
Colour labels reached `versions.label` only from an XMP sidecar: nothing
in the catalog could set one, clear one, or read it back alongside the
stars, so there was nothing for an interface to call.
`set_label` and `set_label_many` write it the way ratings are written,
the bulk form in one transaction so a key over a selection is one commit.
`toggled_label` holds Lightroom's rule for a label key: it clears only
when every image already carries that label, and otherwise sets it on all
of them, so a half-red selection comes out red rather than inverted.
`Judgement` carries the label, so the grid's one window query brings it
with the stars, and `label_histogram` counts each label in one grouped
statement for the filter chips. A label does not make a frame "judged":
it is a pile of the photographer's own, not a cull decision.
The doc comment for `default_version_id` had been stranded above
`label_code` when that was inserted; it is back on its function.
When a gesture stopped, the half-resolution draft was replaced by the
full-resolution frame in one step, a visible jump from soft to sharp.
FR-DSP-4 asks for a refinement that is smooth, not a jarring swap.
The canvas now keeps the last draft frame (`canvas-previous`) and draws
it over the sharp one, fading it out over 150 ms when the draft flag
clears. The fade costs no render: the draft is a refcount on the texture
it was drawn into, and the adjust pass ping-pongs between two output
targets, so the sharp frame is written into the other one. While a
gesture is drafting the layer is hidden and snapped opaque, so a new
drag shows its draft at once; past the fade it is hidden again, and a
settled canvas composites one image as before.
The histogram is measured on settled frames only, so during a drag it
describes the frame from before the gesture while the canvas shows
something newer, and nothing said so. The draft flag stopped at the
render closure.
It now reaches the interface: `canvas-draft` on the window and
`Levels.provisional` for the readouts, both set on every canvas render
from the flag that chose the frame's resolution, and cleared when a
render fails. The histogram panel dims its display reading to half
while a draft is up and brings it back when the frame settles. Dimmed
rather than captioned, because a caption appearing on every drag would
move the column; the raw reading has no frame to lag and is left alone.
During any drag longer than 120 ms the canvas rendered a full-resolution
frame every 128 ms under the finger. The settle timer was armed by the
first draft of a burst and not re-armed by later ones, so it counted from
the start of the gesture rather than from its last movement, fired
mid-drag, and the next coalesced event armed it again. Each of those
frames is the most expensive one the canvas draws, landing where the
frame budget is tightest.
The draft/sharp decision now lives in `refine::Refine`, apart from the
timers that carry it out. Every draft frame arms a settle timer carrying
a generation token and only the newest token is honoured, so the sharp
frame lands SETTLE_DELAY after the last movement. A request arriving
while a settle is still owed also counts as part of the gesture, so a
slow stretch of a drag (one event per frame, nothing to coalesce) no
longer renders sharp between drafts. A timer that fires with a render
already posted defers to it.
The tests drive the state machine through simulated timelines; the
long-drag case reproduced the four mid-drag sharp frames before the fix.
Cropping tighter past a mask layer made it invisible without a word:
the layer stayed in the panel and the sidecar, and its adjustment went
on landing on pixels nobody would see again.
When a crop is let go, develop now measures what the gesture did to the
mask stack (dr_pipeline::orphan) and, if any layer is now entirely or
mostly outside the frame, shows a notice over the photograph: how many
layers, their names, "Undo crop" and "Keep crop". The crop is already
applied and nothing waits on the answer.
The crop overlay gains a release callback carrying the rect the press
began from, so the measurement runs once per gesture and never on the
drag's per-frame changes. Choosing a ratio is measured the same way,
being a crop committed in one click.
"Undo crop" is the ordinary undo, and the notice is tied to the history
revision it was raised at: the redraw that follows any history move
clears it, so the crop and its warning go back as one step. A second
drag folded into the same step is measured from where that step began.
A crop that strands nothing shows nothing.
Mask geometry is stored in source coordinates, so re-cropping tighter
never destroys a layer. It makes it invisible: the layer stays in the
panel and the sidecar, its adjustment lands on pixels nobody will see,
and nothing says so. The spec had no clause for this; FR-DEV-17 now
states it, under the ID issue #10 reserved.
dr_pipeline::orphan samples each layer's mask on a 64x64 lattice over
the source, with the gradient, radial, brush and model-raster geometry
the mask shader uses, folds the parts by their joins and inversions, and
maps the samples through the framing to see how much of the coverage
the crop keeps. `hidden_by_crop` reports the layers whose share fell
below a tenth, and only those the change newly hid, so an already
stranded layer is not announced again on every later adjustment.
Ranges follow the picture and region selections need a label map this
crate does not hold, so a layer that adds either is never reported: a
false alarm on the common path would teach the notice to be dismissed
unread.
FR-RAW-2's "without changing callers" needs a test that would fail if a
caller named the concrete decoder; passing a real RAW through rawler
cannot tell the two apart, because both routes give the same answer.
The decoder_seam tests hand a stub decoder, for a container no real
decoder reads, to the catalog scan (read_metadata_only over a folder
backend), the preview ladder (the remote two-stage fetch, an import's
thumbnail and the viewer's no-GPU fallback) and export (open_for_export,
skipped without an adapter). Each assertion is on something only the
stub produces: its camera and date, a header fetched at its 64-byte
budget rather than HEADER_BYTES, preview and sensor sizes turned by its
orientation. Switching collect_metadata or make_thumbnail back to the
free functions fails two of the three tests.
The develop test_support module is widened to the crate so the export
test shares the one headless GPU context the other tests use. The
requirements note for FR-RAW-2 now records the trait as built and the
second decoder as not.
With the trait in place the claim still meant nothing while every caller
named dr_decode's free functions: a second decoder would have had to be
threaded through the scan, the thumbnail ladder, import, the viewer,
export, merge and repairs at the moment it arrived.
Each of those now takes a &dyn Decoder and reads headers, previews,
orientation and sensor data through it, including the header budget a
remote fetch asks for (header_bytes) and where it finds the embedded
preview (locate_preview). Only the places that start a job name
dr_decode::default(): the thumbnail, sweep and thumbnail-sweep threads,
the viewer's open handlers, and the request structs a job is handed
(BatchRequest, MergeRequest, the import Request, the repairs Toolkit),
so a caller can be given another decoder by changing what it is handed.
The default is rawler through the same free functions as before, so
nothing a user sees changes. The trait gains Debug as a supertrait so
request structs that derive Debug can carry one.
FR-RAW-2 says a second decoder may be added for broader camera coverage
without changing callers, and D2 names LibRaw as that second decoder.
Nothing tested the claim: dr_decode was one decoder reached through free
functions, so adding another would have meant editing every caller at
the moment there was most pressure not to.
Decoder is an object-safe trait over bytes: header_bytes, metadata,
orientation, locate_preview, preview and decode. Rawler implements it by
delegating to the existing free functions, so behaviour is unchanged,
and dr_decode::default() hands it out as a &'static dyn Decoder, which
is what the places that start work will name. JPEG recognition, decoding
and completeness checks stay free functions: they are not a RAW
decoder's to vary.
Nothing in the trait takes a path or a SourceRef; the decoder states how
much of a file it needs and where its preview sits, and the caller's
storage fetches that.
FR-DEV-19a said a part is added to the mask or taken out of it, and
mask-editing.md listed Intersect as an M2 item with nothing built. Both
now say what shipped: the requirement names the third join and why it is
a product rather than a minimum, and how old and new sidecars read across
it; the plan marks the blend-table row built, names the tests that hold
the GPU to the definition, and splits M2 into what is done and what is
still outstanding (joining non-painted parts from the panel, per-part
distance fields, folding two layers).
The chained lookup in the_intersect_button_joins_a_part_that_intersects was
one line past rustfmt's width, so fmt --check failed on the branch. Split
as rustfmt wants it; no behaviour changes.
The pipeline could now keep only where two selections agree, but the panel
had no way to ask for it: the part row's chip flipped between + and -, and
the buttons under the parts joined an added or a subtracted correction.
An "∩ Intersect" button joins a painted part that intersects, and the chip
on a part row cycles + -> - -> ∩ and round, so an existing part can be
turned into an intersection without being repainted. Both go through the
same session calls as before, indexing Join::ALL, whose first two entries
kept their places. The chip is now a tagged gesture, so it is in the
gesture book.
A layer's parts could be added to the mask or taken out of it, and nothing
else. The selections that need composing most are the ones that are
neither: the sky that is also bright, the subject that is also skin. With
union and subtract alone, "this and that" had to be spelled as "this minus
everything that is not that", which needs a second part that selects the
complement and rarely exists.
Join gains Intersect, stored as "intersect" in the part block of a sidecar.
It is the product of the two coverages, dst * src, which is one more
fixed-function blend state beside union's max and subtract's
dst * (1 - src) (mask-editing.md 5.2): the same scratch texture, the same
three vertices, no shader arithmetic. The product equals the minimum
wherever either side is fully in or out, and is the softer reading where
two soft edges overlap. Join::apply spells the three operations on the CPU
so the GPU tests can be held to one definition.
A layer that intersects with a part covering nothing now reports that it
covers nothing, so it is not rasterised as an empty slice. Old sidecars
never contain the word, so they read as before; a build from before this
reads "intersect" as a union, the existing unknown-join fallback, which
keeps the part visible rather than dropping it. Join::ALL keeps union and
subtract at indices 0 and 1 so a stored panel index still means the same
join.
Zoomed to 1:1 or past it, the develop canvas showed a smoothed blur
rather than the photograph's pixels, so focus and noise could not be
judged at the magnification meant for judging them.
Two things caused it. The canvas only switched to nearest-neighbour
strictly past 1:1, with a margin, so the 1:1 inspection itself stayed
smooth. And the switch mostly had nothing to act on: the pipeline
rendered a viewport-sized frame at every zoom, so past 1:1 it was the
pipeline doing the enlarging - bilinearly whenever a straightening angle
or lens correction was in the chain - and the detail stage then sharpened
and denoised those invented pixels at radii scaled up to match. The
texture reached the canvas already blurred and was presented 1:1.
Now, from 1:1 on, the visible region is rendered at the source's own
resolution (render::render_size) and the canvas enlarges it with
nearest-neighbour, so the blocks on screen are the pixels an export would
have; it is also less shading. The decision lives in two small
functions, render::magnification and render::shows_source_pixels,
measured in physical pixels like one_to_one_zoom, with a half-percent
tolerance so the inspection zoom counts as 1:1 even where fit() rounded
the other edge. Below 1:1 the render and the smooth filter are unchanged.
Before the previous commit, browser sign-in could store an account as
http://, and the client now refuses to send to one. Left alone, such a
library would fail to open with a configuration error, so its stored
endpoint is rewritten before anything reads it.
The endpoint is half of two keys, and both are handled:
- The keyring entry is filed under it. Rewriting only the record would
strand the app password under the old key and sign the user out, so
AccountStore::move_endpoint copies the secret across first, rewrites
the record in place (the last record is the one resumed), and deletes
the old entry only once nothing refers to it.
- namespace() is built from it and names the catalog directory. For an
http to https rewrite it does not change, because the namespace strips
either scheme. A move that would change it is refused, not performed,
so no later rewrite can abandon a catalog either.
The rewrite is a new BackendProvider::upgrade_endpoint hook, which does
nothing by default, and not a second call to normalise_endpoint. The
folder connector's normalise_endpoint canonicalises the path and needs
it to exist, so running it on every launch would fail a library on an
unplugged disk, or rename one whose path now resolves differently. Only
Nextcloud implements the hook.
If the move fails (for example, a locked keyring), it is logged, the
account is left as it was, and the move is tried again on the next
launch.
Closes#65.
Login Flow v2 saved the account under the `server` field of the poll
response, not under the address the person typed. That field is the
server's idea of its own URL. Behind a TLS-terminating proxy without
`overwriteprotocol` (a common setup) it says http://, and the account then
sent its app password in the clear on every request after that. The
typed address, already upgraded to https by normalise_endpoint, has just
carried the whole flow, so it is the one kept.
The flow's other two URLs come from the server as well, and are now
upgraded from http to https, and refused if they use any other scheme:
- The login URL is handed to the OS to open. On Windows that is
`rundll32 url.dll,FileProtocolHandler`, which runs a file: or UNC path
rather than showing a web page, so a hostile server could launch a
program when the user starts signing in. open_in_browser also refuses
anything that is not https, as the last check before a process starts.
- The poll endpoint is where the app password comes back from.
The host is not checked. A server reached by its LAN address can answer
with its public name, and refusing that would break a working setup
without protecting anything: the account is stored under the typed
address whatever the server says.
Part of #65.
NFR-SEC-3 held only for the address a person types: normalise_endpoint
upgrades it to https, and nothing else was checked. The login flow's poll
endpoint, an account an older build saved and a redirect all come from
somewhere else, and any of them naming http:// would send the app
password in Basic auth in the clear.
http_client now sets https_only. reqwest checks it before connecting and
again on each redirect, so a refused request never opens a socket, which
the new test checks with a listener that nothing may reach.
A refused scheme is reported as a Configuration error, not Network. The
request never left the process, and Network puts the app into offline
mode over a connection that is working. Other builder errors (a URL that
does not parse) go the same way, for the same reason.
Part of #65.
Filtering the grid by a face crawled on the reference library. The SQL
is not it — the person predicate counts in ~20 ms, the eyes-open term in
~60 — but every press on the tray ran `push_people_chips`, which read the
whole people table (26,362 rows, nearly all empty groups a regrouping
pass left behind) and then called `push_people_roster`, which read it
again and replaced the roster model. The roster is every person holding
a face, 1,581 chips, in a row Slint does not virtualise: a new model
tore down and re-created all of them and laid the row out again, to
move one tick.
A press now walks the roster model and sets `picked` on the rows whose
tick changed; the roster is built only when the tray opens. Both reads
use `people_in_use` (2,140 rows) rather than `people`. A picked person
the in-use query leaves out — emptied by a split while the filter held
them — still gets a chip, since a term with no chip cannot be removed,
and without one the in-place update would fall back to a rebuild on
every press.
bddf325 and 00c028c went in unformatted, so the Desktop job's
`cargo fmt --check` step failed on master (run 1693) and the release job
that needs it was skipped. Whitespace only.
Rating keys in the grid follow darktable's rule: with the pointer over a
photograph outside the selection, 0-5, P, X and U judge that photograph
alone; over one inside it, the whole selection, as before; off the grid,
the selection. The hover is cleared when the grid scrolls, so a key after
a wheel turn cannot judge whatever used to be under the pointer.
Holding F and tapping digits filters by stars: one digit for exactly
that many, two for everything between them, F alone to show every
rating again. The filter gains a ceiling to do it (`max_rating`, one
BETWEEN in the query). The place record carries it, and a record from an
older build reads as having none. The star chips light across a capped
range and the bar says "2-3★ only" beside them.
The grid also takes Ctrl+E and Ctrl+Shift+E for the selection, Ctrl+V to
paste onto it and Ctrl+A to select all. The "Gestures" button is now
"Help", its sheet "Controls and shortcuts", and F1 opens it.
Ctrl+E opens an export sheet: the export defaults on their own, over the
photograph, with an Export button. Ctrl+Shift+E exports straight away on
those defaults. There is no per-export copy of the settings, so what is
chosen in the sheet is saved as it is on the settings page, and the next
Ctrl+Shift+E uses it.
To make that one set of controls in two places, the export options move
out of the settings page into export.slint: an `ExportOptions` global
that Rust writes once, and two panels that read it. The window no longer
forwards forty `settings-*` properties to the page.
Ctrl+Shift+C opens a copy sheet with the edit-kind chips the preset
sheet already uses and a Copy button, which is how a paste leaves each
photograph's crop and rotation alone (Compose off). A and D step along
the roll beside the arrows. While either sheet is up the develop keys
stand down, so A cannot change the photograph behind the form, and
Escape closes it.
The arrow keys and space in develop called `next-image` and
`prev-image`, which walk the files given on the command line. A
photograph opened from a library leaves that list empty, so the keys did
nothing and only a click on the photo roll moved on.
With a library open the step now goes through the roll: it opens the
neighbouring frame exactly as clicking it would, and saves the outgoing
edit the same way.
Nothing made a release. CI built the APK and the installer on the master
push and kept them as workflow artefacts, the Linux binary was not kept
at all, and most tags went out with no downloads until they were
attached by hand.
build-and-test now also runs on v* tags. On a tag the desktop job keeps
its release binary, and a release job that needs desktop, Android and
Windows collects the three, names them with the version and runs
tools/publish-release.sh. The script titles and describes the release
from the annotated tag's message as the server holds it, writes
SHA256SUMS, and attaches what is not already there, so a re-run after
an interrupted upload finishes the job instead of duplicating it. The
same script is how a release is made or finished by hand.
Tried on v0.14.1, whose release was made by hand with the same files:
it found the release, reported all four files attached, and changed
nothing.
It was the last of a dozen buttons in a row that scrolls sideways once
it outgrows the window, which at a desktop width it does. The button
sat past the right edge and nothing says the row scrolls to a mouse,
so batch export of a selection looked like a feature the library did
not have. The rest of the row still scrolls; Export, which is also the
cancel for a running batch, now sits beside it and is always visible.
It appeared only once settings had been copied this session, so with an
empty clipboard nothing on the selection bar said pasting onto a
selection was possible. It is one of the things a selection can have
done to it, like filing it in a collection, and now sits with them
beside Presets.
They sat in the develop column under a "SETTINGS" heading, which read as
application settings, and went away with the panel toggle and in the
mask and spot modes. The strip is where undo already is for the same
reason: these act on the whole edit, not on any one panel.
The paste button still names what it would apply. The TransferPanel
component is gone; the Transfer global and its Rust wiring are
unchanged.
a_second_claim_waits_for_the_first_to_be_released failed on CI: the
second claim came back Some. The waiter thread signalled the main thread
before calling claim, so the main thread could drop the first guard
before the waiter reached the lock. The path was free by then, and the
waiter claimed it outright.
The registry now keeps a test-only count of threads parked in claim,
bumped under the lock just before the condvar wait. The test spins until
that count is one before releasing. The release needs the same lock, so
it can only reach a waiter that is already waiting. Passed 500 runs in a
row.
Nine agent branches, merged one at a time on ui-wiring and gated at each
step: develop.rs, library.rs, collections_ui.rs and library_ui.rs become
module directories; every screen's wire() and lib.rs::run() become lists
of named functions, with the develop screen's callbacks in develop_ui.rs;
the six view booleans become View and Page enums; the collections sidebar
and the library grid get their own Slint globals, taking 155 members off
AppWindow. No behaviour change: a multiset audit of code lines over every
moved region lost nothing, the workspace gate is clean, and the manual
recorded from this build and from master's from the same library snapshot
matches picture for picture, apart from a panorama stall that master
shows too when a merge starts during the engine's TensorRT compile queue.
The installer smoke test asserted seven model files, the number on the
day it was written; models/face has since gained the eye-state trio's
companions and the int8 detector forms, and the run on f71d7ba failed
with thirteen installed. The test now expects as many files as
package.sh's directories hold, so the next model needs no edit here.
package.sh also stages models/inpaint, which the APK and the Arch
package already carry and the Windows build did not: without
migan-512.onnx the panorama's border fill has no model on Windows.
xfeat needs nothing, it is embedded in the binary. windows.md §5.2
lists the result.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Moving each group of properties and callbacks out of AppWindow left
several two- and three-line gaps where a removed block's neighbours no
longer needed separating. No declarations changed.
The shard import recorded each adopted image in its own transaction:
fourteen thousand commits, and fourteen thousand turns at the write lock
that every read on the UI thread queued behind — the sync was felt as a
laggy grid and as "database is locked" from whichever writer lost the
wait. `record_detections_within` takes the caller's transaction, and the
import commits every hundred images.
The store carried every detector generation of an image — 24,123 entries
for 19,089 images on the reference library, a third of its 293 MB — when
only the strongest is ever adopted. A put now skips a pass a held one
outranks, and retires the passes it outranks from the index; sealed
shards keep their bytes, but nothing is written twice from here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The scan/opening/status/error lines, offline mode and its retry, pinning a
collection offline, the sync and thumbnail-sweep state, and the callbacks
that route the grid to a rescan, a sync, another library, a panorama merge
or an export — the last of what AppWindow still carried under the
library- prefix — move onto the `Library` global started earlier on this
branch. Rust reaches them through window.global::<Library>() rather than
window.set_/get_/on_/invoke_ on the root.
library-visible is the one name that stays: it is computed from
active-page and active-view, the shell's own routing state, which a
global cannot read. AppWindow now declares no other library- property or
callback.
The date-range fields and their band on the capture-time axis, the
timeline's bars, labels, scrub/pinch/pan/zoom callbacks and its
sweep/current-bucket/anchored state, and the filter bar's people chips,
mode, eyes-open toggle and gesture reference move from AppWindow onto the
`Library` global. Rust reaches them through window.global::<Library>()
rather than window.set_/get_/on_ on the root, as the earlier commits on
this branch did for the grid's cells, selection, ratings and keywords.
The keywording sheet's rows and its open/assign/unassign callbacks, the
star and flag callbacks a cell click or a judgement key fires, the burst
toggle and representative-chosen callbacks, the trash-selection shortcut,
and the rating/unjudged/flag filter chips with their rating-counts model
move from AppWindow onto the `Library` global started in the previous
commit. Rust reaches them through window.global::<Library>() rather than
window.set_/get_/on_ on the root.
AppWindow carried the grid's loaded window of cells, the keyboard cursor,
drag and drop, the held-row long-press state, columns and cell size, the
scroll and viewport bookkeeping, the photo roll's pick and centre-request,
and the local-only/reorder/collection-filing gestures that act on a
selection, as properties and callbacks on the root component. That state now
lives in the `Library` global declared in library.slint, next to the structs
(LibraryCell, TimelineBar, KeywordRow, PersonChip) it and the grid's other
components already share; Rust reaches it through
window.global::<Library>() instead of window.set_/get_/on_/invoke_ on the
root, the same change collections.slint's `Collections` global made for the
sidebar.
library-visible stays on AppWindow: it is computed from active-page and
active-view, the shell's own routing state, which a global cannot read.
Everything else still prefixed library- — the timeline, the filter bar,
ratings and flags, keywording, and the routes and status lines — stays on
the window for now and moves in the commits that follow.
baed1c4 landed it over rustfmt's width; `cargo fmt --check` is the
first gate the Desktop job runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
f71d7ba and 34ac2f1 added tags without re-running the report, which
the Traceability job's "is it committed" step rejects.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two CI gates have failed on every push since 0.13.4 and both are
fixed here.
The traceability gate rejected `FR-INF-1` as an orphan: settings.slint
and dr-ui tag it, but the four register entries inference.md §12 wrote
were never carried into requirements.md, which is the only file the
extractor reads. §3.12 and §4.10 now hold FR-INF-1..3 and NFR-INF-1
verbatim, with the acceptance milestones pointed back at inference.md.
The matrix is regenerated (188 defined, 155 covered) and the README's
"where it stands" line, which the 0.13.6 release commit skipped, says
0.13.6 and the new figures.
`cargo fmt --check` failed on the `fill_border` call in dr-pano's
padding test, which is the first thing the Desktop job runs after
installing the toolchain and why it failed within a minute.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adopting an image from a peer's face shard stamped its run marker as now,
and the export reads a catalog marker newer than the shard's as a
re-index. So every adopted image went straight back out under this
device's client id: 14,100 adopted, 15,457 "newly indexed" on the next
pass, twenty-two shards of a peer's faces uploaded a second time.
merge_shard now carries the peer's indexed_at into the local index, and
the import writes that marker into face_index; where an older peer's shard
carries none, the store takes the catalog's, so the two agree either way
and the export finds nothing to send.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
docs/ had 26 developer documents flat beside the manual, and the two
audiences are very differently sized: most readers want the manual and
the gesture reference, a few want the register, the designs and the
measurements. The manual and gestures.md stay at the top; everything for
someone changing the code moves to docs/dev/, and the two documents that
name their own successors — the v0.1 milestone and the UI-refinement plan
— go to docs/dev/archive/ rather than being deleted, since both are still
cited. docs/README.md is the index, users first.
Every reference follows: code comments, Cargo manifests, the workflows,
the pre-commit hook, the bench and traceability tools (which locate the
repo root by docs/dev/requirements.md now), packaging, the Docker READMEs,
CLAUDE.md, CONTRIBUTING.md and the README. The matrix links one level
deeper and is regenerated. Links out of the moved documents into the tree
gain a level; a link checker over every Markdown file finds none broken.
controller holds LibraryController and the window-sizing constants every
other module reads and writes through pub(super) fields, the same shape
collections_ui and develop already use. open is the launch-to-scan cycle and
the worker that checks the catalog file before either touches it. offline is
what of a collection is on this device and the prompt that offers to change
it. window fills the grid model from the catalog and drains the thumbnail
fetch, which is the piece the catalog-reads-are-proportional-to-what-changed
rule (docs/catalog.md §1) bears on most directly. sync is the background
passes that reach beyond the loaded window: the metadata sweep, the
whole-library thumbnail pass, and the exchange with the server.
ratings_keywords applies a judgement or a keyword to a selection and queues
the sidecar and XMP writes behind it. timeline is the capture-time sidebar
and the photographer's place together, kept in one file because a restored
place ends by moving the timeline marker and a scrub is a restore of one
instant, so most calls between the two would otherwise cross a module
boundary. grid wires the grid's own callbacks — the keyboard cursor,
cell-size zoom, the routes into and out of develop — and filter_bar wires
the rating, people, date and offline-scope filters, calling back into
whichever of the above owns the work a filter change triggers.
Extracted by item rather than by line range, so every doc comment and
TRACES/GESTURE annotation stayed attached to the code it describes; the
sorted set of TRACES/GESTURE lines in the new directory is identical to the
original file's. Tests moved with the code they exercise, including the
handful of fixtures — settle, model_of, with_catalog, zoom_cell, pinch_step
— that only one target module needed and so were not worth sharing through
a test_support module the way the other splits use one. Items that crossed
a new module boundary were widened from private to pub(super), narrower
than the whole-file access the original gave them; a few items already
pub(crate) for recovery_ui or presets stayed there rather than being
narrowed, since nothing needed them tightened further.
mod.rs re-exports the same surface library_ui:: callers used before, so
lib.rs and every other caller needed no change.
AppWindow carried the sidebar's tree, its row menu, renaming, drag and
drop between rows, the trash row, and the membership sheet as ~40
properties and callbacks on the root component, in the pattern CH-1
describes and the develop screen's globals (Adjustments, Framing, Steps,
...) already replaced. Collections.* in collections.slint now holds that
state, declared next to the MembershipRow struct it and the membership
sheet both use; Rust reaches it through window.global::<Collections>()
instead of window.set_/get_/on_/invoke_ on the root.
collection-selected, collection-select, and collection-offline-menu stay
on AppWindow: library_ui.rs invokes collection-select directly and
registers collection-offline-menu's handler, and lib.rs reads
collection-selected for back-navigation, so moving them would have meant
editing library_ui.rs, which another change on this branch is splitting
into a module directory. collections-visible stays too — it is
lib.rs's panel-layout state, seeded from the saved layout before the
sidebar exists, and collections_ui never touches it. Everything prefixed
library- (the grid's drag, selection and keyword state that the sidebar's
Rust also wires for cross-feature gestures like filing a selection into
a collection) stays on the window as well, since it belongs to the
library screen, not the sidebar.
The thumbnail stage ran first and, on a device that had just adopted its
peers' shards, spent its time re-uploading hundreds of megabytes under its
own client id while faces, people, collections and dates waited behind it.
Faces go first — the catalog merge assigns identities to faces this device
holds — then the catalog, then thumbnails.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The derived sync fired only after the metadata sweep, so a fresh device
re-derived every thumbnail it scrolled past, re-detected faces and re-read
every header for hours before adopting the shards and snapshot that held
all of it. It now fires as soon as the scan completes — the first moment
the rows the merges key on exist — and the sweep starts behind it. In
steady state that pass is one listing.
The catalog merge gains a fourth half: capture metadata (captured_at,
offset, camera, lens, ISO) for images still at metadata_state < 2, matched
by oc:fileid from a remote row at 2. A date is a fact about the file's
bytes, not local state, and the snapshot already carried it. The sweep's
per-chunk query then finds nothing left, and the timeline is whole on a
fresh device without a header fetch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
app.slint carried show-launch, show-library, show-identity, show-settings,
show-import and show-merge as separate booleans, so the root component chose
what to draw with five- and six-term conjunctions and nothing stopped two of
them being true at once. Replaced with two enums: View { develop, library,
identity, launch } for which top-level screen is showing, and Page { none,
settings, import, merge } for which page, if any, is drawn over it.
Two values rather than one, because the two questions are genuinely
different. Settings, Import and Merge are reachable from more than one View
and are drawn outermost without touching it — closing one has to return to
whichever View was already current, and today that works because the
underlying property is left alone while the page sits over it. A single
View with five or more variants would need a second field remembering what
to return to; Page needs nothing to remember, since View was never
overwritten in the first place. Identity, by contrast, genuinely replaces
the window the way Launch and Library do (see the existing "like the launch
screen" comment on its `if`), so it is a View variant, not a Page.
Every `if` chain in app.slint that used to compare four, five or six
booleans now compares active-view and active-page to at most one variant
each. library-visible collapsed from a six-term conjunction to
`active-page == Page.none && active-view == View.library`.
The Rust side follows: every set_show_*/get_show_* call in library_ui.rs,
identity_ui.rs, settings_ui.rs, merge_ui.rs, import_ui.rs, launch_ui.rs and
lib.rs now reads or writes active-view or active-page instead, including
lib.rs's startup match (View.launch vs View.develop, since a Startup that
skips the launch screen used to leave both old booleans false and fall
through the chain to develop) and identity_ui's close handler, which now
writes View.library or View.develop in one call where it used to write
show-library then show-identity separately.
back_one_step needed one deliberate adjustment beyond the mechanical
rename. Identity was never represented in NavState: back had nothing to do
when Identity was opened from the library (show-library stayed true,
unread by IdentityScreen's own condition) and could only reach ToLibrary
when opened from develop, which likewise wrote a property IdentityScreen
never read — so escaping out of Identity was invisible in both cases before
this change. With a single active-view, falling into the general case
would instead overwrite the value IdentityScreen's `if` does read and close
it as an unintended side effect. back_one_step now swallows the gesture
while View.identity is current, reproducing the same "nothing visible
happens" outcome for both origins without threading identity_ui's private
came-from-library state through lib.rs for one screen.
Verified with tools/manual/drive.py against a private Xvfb and the debug
build: launch screen to library, Settings opened and closed, Identity
opened and closed (including Escape doing nothing while it is open),
develop opened from a cell and closed both by the back button and by
Escape. Screenshots under verify/.
collections_ui.rs had grown to 4,591 lines covering the sidebar controller,
the click/drag selection policy, tree refresh, the drag gesture, the trash
worker, twelve wiring functions, and the row's rename/create/context menu,
all in one file. Split into collections_ui/ with one module per area, the
way develop/ and library/ were already split on this branch:
- controller.rs: CollectionsController and the pure drop/delete/release
decisions (decide_drop, decide_delete, decide_release, menu_detail,
delete_warning) that a test can drive without a window.
- press.rs: PressUndo and the click-and-release selection policy
(apply_press, select_row, commit_press, cancel_press).
- tree_sync.rs: rebuilding the sidebar from the catalog and pushing
catalog-derived state into the grid (refresh_tree, offline_state,
sync_lifted/sync_selection/sync_reorderable/sync_badges,
refresh_membership, direct_holdings).
- drag.rs: the cursor bitmap (compose_drag_image, blit_scaled) and the
hold/spring timers (arm_hold, arm_spring, should_spring,
collapse_spring_opened) plus their delay constants.
- trash.rs: the soft delete (start_trash, start_restore, drain_trash,
stop_trash, refresh_trash, format_bytes).
- wiring_grid.rs / wiring_tree.rs: the wire() entry point and its twelve
wire_* functions, split in two because together they were the largest
single piece (grid-facing selection/drag/trash vs. sidebar-facing
navigation/create/rename/row-drag/menu/membership).
- rename_menu.rs: creating, naming and renaming collections, and the row's
context menu (apply_rename, create_child, unique_name, open_row_menu,
close_row_menu, close_rename).
mod.rs carries the module's own top-level doc comment, the `pub use`
re-exports for the eight items the rest of the crate reaches by
`collections_ui::` path (CollectionsController, wire, refresh_tree,
sync_badges, sync_selection, select_row, commit_press, cancel_press), and a
shared `test_support` for the one fixture (`ids`) more than one file's
tests needed. Every item that only crossed a boundary within this module,
not out of it, was narrowed to `pub(super)` rather than kept at the
crate-wide `pub` a single file gave it for free.
Extracted with a brace-aware pass that kept each item's own leading doc
comment and attributes attached to it, and tests moved with the code they
exercise; every TRACES/GESTURE comment lands on the same code it did
before. No file outside the new directory changed — lib.rs's `mod
collections_ui;` resolves to the directory automatically, and every
outside caller's `collections_ui::` path still resolves through mod.rs's
re-exports.
`inference::init` listed the models from the user's shared directory
alone, while the app loads them from there or from the package's
`/usr/share/darkroom/models`. On a fresh package install the probe found
"no model to probe with", stayed on the CPU, and compiled nothing. Both
now resolve each file with the same search, `library::shared_model`.
The PKGBUILD names the ONNX Runtime packages as optional dependencies,
since the app loads one from /usr/lib if present.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The 188-line wire() registered the import page's callbacks in four
comment-delimited sections. Lift each into its own fn: wire_opening_and_closing,
wire_choosing_a_source, wire_options, wire_running. context stays generic
over C on each of the three functions that use it, matching how survey()
and start() already take it (impl Fn, implicitly Sized) rather than coercing
it to a trait object, which would have needed ?Sized added to those two
unrelated functions for no benefit.
Measured on a Radeon RX 7900 XT against Arch's onnxruntime-rocm 1.29
(docs/inference.md §1.3): MIGraphX fp16 runs the detectors at 2.4–3.4 ms
against 10–58 ms on the CPU provider, the inpainter at 8 ms against 514,
with a 15–135 s compile per graph the first time and under a second from
its cache after. A compiling rung on TensorRT's terms, wired the same way.
The ROCm execution provider is gone (removed in ONNX Runtime 1.23), so the
AMD ladder is MIGraphX then the CPU, with no non-compiling rung between.
MIGraphX is registered through the runtime's generic key/value entry
point rather than ort's builder: 1.29 reads the legacy options struct for
its precision flags only, and the compiled-program cache directory
(`migraphx_model_cache_dir`) only travels the generic way. The provider's
cache key omits the precision, so f32 and fp16 programs get their own
directories. The probe fingerprint now includes the provider libraries
beside the runtime and the ROCm version, since a distribution's CPU and
ROCm builds are the same file at the same path.
`status().failed` reports only the rungs above the selection, so an AMD
desktop's About line says why MIGraphX won rather than that the NVIDIA
providers are not in the build.
Two examples: `ep_probe` times each provider cold and from cache, and
`ladder` drives `init` as the app does to watch the first-run sequence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
run() was 2,724 lines that built every controller, owned the develop
session and its render loop, and registered every develop-screen callback
inline — the state CH-1 in docs/dev/code-health.md describes. This is the
mechanical split CH-1 calls for, done in one pass rather than section by
section since develop_ui.rs only compiles once lib.rs stops registering
those callbacks itself.
develop_ui.rs is new: a DevelopWiring struct holding the session, the rows
model, the redraw/render closures and the other controllers' handles, and
one wire_* function per section run() used to contain — presets, export,
the adjustment panel, film, undo/redo, zoom/pan/crop, rotation/flips/
straightening, navigation and peaking — called in the order run()
registered them. window travels as its own parameter throughout rather
than living on the struct, because the generated AppWindow type is not
Clone; every other field is an owned clone so each function's body could
be pasted from run() unchanged.
lib.rs::run is now under 300 lines: construction and startup only, calling
named functions for the window and its diagnostics/inference wiring, the
launch/library/collections/identity/settings screens, import and merge,
the render loop (render_now/redraw/show), the remote open path, the
window chrome (resize, layout class, panel toggles, back gesture), and
develop_ui::wire for the rest. Every TRACES and GESTURE comment moved with
the code it annotates.
Two latent type errors surfaced while restructuring rather than being
introduced by it: activity and display were being passed around as bare
ActivityLog/DisplayWatch instead of the Rc<...> their constructors
actually return, which only worked before because nothing needed to name
the type explicitly.
The 225-line wire() registered the launch screen's callbacks in nine
comment-delimited sections. Lift them into fn's, merging a few adjacent
ones that were only a handful of lines each: wire_sign_in covers both the
browser flow and the app-password fallback (the same button in two forms),
wire_choose_folder_and_open covers opening the folder browser and the
final "Open library" press, since both are short and sit back to back.
wire_use_folder, wire_sign_out, wire_formats, wire_folder_picker_navigation
and wire_copy_url stay as they were sectioned. wire() calls each in the
original order and keeps the closing render() call, which every section
relies on having run once at startup.
The 312-line wire() registered the panorama page's callbacks in three
comment-delimited sections. Lift each into its own fn: wire_start (the
"Merge to panorama" press, its fetch, and the DARKROOM_START_MERGE dev
entry point — all three only ever used together), wire_decision (confirm,
projection and border chips, the fill knobs), and wire_stop_and_leave
(abandon, close). wire() itself now just calls the three in order; S, C
and F stay generic on wire_start since sources/context/on_done are used
nowhere else.
The 837-line wire() had almost no section comments, unlike its siblings, so
the seams had to be found by reading it rather than following markers. Lift
each into its own fn: wire_dials (the two grouping sliders), wire_navigation
(open/close/switch person — needs models() too, for the missing-model
banner), wire_rename_and_merge (a rename and the namesake offer it can
raise), wire_face_actions (pick/confirm/reject/split, the grid's own
actions), wire_grouping_preview, wire_recluster, wire_indexing (the shared
launcher behind Index/Re-index plus Stop), and wire_coverage_and_ignore.
wire() keeps the generic-to-trait-object coercions and the eyes_available
closure, since most of the above need it, and calls each function in the
original order. The reload! macro moved from inside wire() to module scope,
dedented, since macro_rules is scoped textually and every extracted function
uses it.
The 1,457-line wire() registered every collections-sidebar and grid-drag
callback in one function, sectioned only by comment. Lift each section
into its own fn: wire_selection (split further into wire_selection and
wire_selection_filing, since the original section ran to 375 lines),
wire_drag, wire_trash, wire_trash_from_grid, wire_tree_navigation,
wire_create, wire_rename, wire_remove, wire_row_drag (the tree row's own
hold-drag, which the original "remove" comment's span covered but which
is really a separate feature), wire_row_menu, and wire_membership.
wire() itself now just coerces the shared closures to trait objects and
calls each in the original order. visible_ids is coerced to
Rc<dyn Fn() -> Vec<ImageId>> at the top, alongside on_scope_changed and
session, so the new functions take plain trait objects instead of
threading a generic parameter through every one of them.
library.rs was 7,729 lines wiring together everything "open a remote
library" touches: scanning, pulling other devices' judgements out of
sidecars found along the way, writing local edits back out to the
sidecar outbox, pushing/reloading XMP by hand, fetching and prefetching
thumbnails and originals, generating thumbnails locally, the metadata
and thumbnail background sweeps, on-disk paths for the catalog and
model files, and reading the grid's cells, spans and rating filter.
Same motivation as the develop.rs split (docs/dev/code-health.md CH-1):
a pure, no-behaviour-change move into one file per area, each under
about 1,500 lines.
Tracing actual call sites rather than trusting the file's physical
layout mattered here: `persist`, `load_folder_etags`, `pull_sidecars`,
`load_sidecar_etags`, `record_sidecar_read` and `apply_judgement` sit
textually beside the XMP push/reload functions but are called only
from `run_scan` (pulling a device's own past judgements out of the
sidecars a scan just walked), so they went to scan.rs and not xmp.rs.
`cells` came out at over 1,800 lines once its tests moved with it and
split further into cells.rs (windowed reads, trash, ordinals) and
spans.rs (collection scope, manual reordering, the capture-time
histogram) -- ten submodules rather than the nine first planned.
Previously-private items reached from a sibling module became
`pub(super)`, narrower than the whole-crate reachability one file gave
them. Tests moved with the code they test; the two test fixtures used
across more than one file (`scanned`, and develop.rs's
`session_with_a_left_half_subject` in the matching commit) joined the
shared `test_support` module alongside the existing `entry`/
`with_images`/`image_ids` helpers. `mod.rs` re-exports every module's
public items under `library::`, including the `pub(crate)`
`test_support` module `repairs.rs` reads its fixtures from, so no file
outside `library` needed a change.
The previous commit split develop.rs the same way; taken alone it left
dr-ui without library.rs, so that intermediate commit does not build on
its own. This one restores it.
develop.rs had grown to 9,327 lines covering everything the develop
session does: opening a photograph, the parameter-row and curve-widget
panel model, mask viewing and editing, mask creation and the rasteriser
that turns a mask stack into GPU arrays, spot repairs, scene
segmentation, framing and zoom, white-balance sampling, rendering and
film choice, and the undo/snapshot history. docs/dev/code-health.md
CH-1 names dr-ui's lack of a view layer as the reason every feature
kept landing in a handful of files; this is the first of the two pure
splits it recommends as easy, no-behaviour-change wins independent of
that larger rework.
The boundaries follow the file's own sections (several were already
marked off with comment headers) and the seams a full read turned up
underneath them -- mask storage/rasterisation turned out to be a
distinct concern from mask viewing and editing, and rows/tabs/curves
from each other, so those split further than the headers alone
suggested. Each module stays under about 1,500 lines. Struct fields
and the handful of helper methods now called from a sibling module
became `pub(super)`, which is strictly narrower than the whole-crate
reachability a single file gave them; nothing gained visibility outside
`develop`. Tests moved with the code they test, including the few
cases where a helper one file's tests needed was itself only defined
in another's -- those became shared fixtures in `mod.rs` alongside the
`headless`/`read_back`/`grey_session` helpers that already worked that
way. `mod.rs` re-exports every item `develop::` callers outside this
module used before, so lib.rs, masks_ui.rs and the rest needed no
changes.
wire() registered every grid callback in one 1,214-line function behind
four section comments, two of which were themselves far over 300 lines
with no further markers. Each fenced section becomes its own function,
called from wire() in the original order with the section's own comment
kept as its doc comment:
- "the keyboard cursor (FR-CULL-4)" (496 lines) splits at its own topic
breaks into wire_grid_cursor_and_zoom (cursor movement, cell zoom and
pinch), wire_grid_sync_and_load (explicit sync/thumbnail requests and
the reloads a changed viewport, column count or scroll position
trigger), wire_timeline (the capture-time sidebar) and wire_grid_routes
(grid/launch/develop navigation and a manual rescan).
- "ratings and flags" and "keywords" were already under 300 lines and
become one function each.
- "the filter bar" (447 lines) splits into wire_filter_ratings_and_people
(which keeps the section's own comment), wire_filter_dates and
wire_filter_scope_and_offline.
The three callbacks registered before the first section comment
(on_settings_xmp_reload, on_library_cell_clicked, on_library_roll_pick)
and the trailing crate::recovery_ui::wire call stay directly in wire(),
since neither is inside a fenced section.
wire() registered every mask-panel callback in one 778-line function
behind section comments. Each of the seven fenced sections (computing
the region map, refining a subject's mask, dragging a gradient,
selecting on the photograph, the stack, the edge treatment, adding
layers) becomes its own function, called from wire() in the original
order with the section's own comment kept as its doc comment. The
"adding layers" section was itself over 300 lines and had no further
section markers inside it, so it is split at its own natural seam
between painting/viewing a mask (wire_layers_paint) and working the
parts and add-mask buttons (wire_layers_parts); the second half gets an
introductory doc line since there was no comment of its own to reuse.
Locals declared just for one section's closures (running, refining,
dragging) move into that section's function instead of staying in
wire().
wire() registered every settings-page callback in one 416-line function,
fenced only by section comments. Each fenced section (opening and
closing, cache, faces, export, reset) is now its own private function
that wire() calls in the same order, with the section's own comment kept
as its doc comment. on_budget_changed and on_open are coerced to trait
objects at the top of wire() so the new functions take a plain
Rc<dyn Fn> rather than needing their own generic parameter, with no
change in the closures registered or the order they are registered in.
dtourolle
