acab0d7abbb324d1bbad38a8142a1885204e88c1
13
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 dtourolle
|
af89433aee |
Offer the lens profile as a tick box, since applying it silently reads as absent
The develop panel's Optics group is three manual sliders: distortion, chromatic aberration and lens vignetting. The automatic correction was already there — the file's EXIF lens is matched against the bundled Lensfun database on open and the coefficients are fanned out to all three — but nothing in the interface said so except a line of grey text under the camera reading "· corrected", and there was no way to decline it. From the outside that is indistinguishable from the feature not existing, which is how it was read. `dr-lens` states the rule this breaks: an automatic correction that silently does nothing is worse than one the user can see is unavailable. The caption satisfied the letter of it and not the point — a photographer looking for "apply the lens profile" found three sliders and no switch. So the profile is now a control. It is a capability rather than a flag on the session, because everything a photographer sets travels one road: the capability list feeds the generated panel, `Preset` captures it, the sidecar stores it and the undo stack replays it. A bool on the side would have needed adding to each of those four by hand and would have been forgotten in at least one — which is exactly how the mask stack came to be missing from the history. It is on by default, which is what `switch_on` is for: the coefficients are a measurement of the lens that took the photograph, so accepting them is neutral and declining them is the edit. The sidecar therefore stores nothing for the ordinary case and the correction still happens. The switch appears only where a profile was matched. A tick box on a photograph whose lens the database has never heard of would be a control that looks available and does nothing, which is the failure the rule above names rather than an instance of following it — those photographs are told "· no profile" in words instead, and one whose box is unticked now says "· profile off", which is a third fact and not either of the other two. Two things had to be built underneath. `ParamKind::Bool` was in the core's closed enum and mapped to a row kind here, and had no control behind it in `adjust.slint`: a parameter declaring itself a switch was flattened into a row that drew nothing at all. Nothing shipped had one until now, so the gap cost nothing and was invisible. And `Check` self-toggled, which is right for a settings page that owns its value and wrong for a panel row that is a view of the edit graph — the click would have answered by replacing the binding with a literal, and the next undo or pasted preset would have moved the value with the tick left where the finger put it. It now takes `controlled`, and the generated row uses it. The manual sliders are unchanged and still trim whatever the profile leaves, so switching it off is "correct this by hand" rather than "stop correcting". |
||
|
dtourolle
|
901f51e6c4 |
Point at something grey and let the pipeline work out the rest
FR-DEV-3 has asked for "white balance (temperature/tint, and picker)" since it was written, and only the first half existed. `WidgetKind::WhitePoint` was in the vocabulary and `develop::supported` answered false for it, so the node degraded to two sliders — correct behaviour that had quietly become the only behaviour. Sampling a neutral is the first move of the global tonal pass and every colour judgement afterwards is measured against where the grey was put, so guessing at two sliders until a wall stops looking green is the wrong way round. The awkward part is that a picker genuinely needs to know how far a hundred units of temperature move red against blue, and that number is declared in the node's own file. So the inversion lives in `dr_pipeline::neutral` rather than in the interface: the canvas hands over a colour, the core finds the operation that asked to be driven by a pixel and bisects its declared response until the sample comes back grey. Nothing in `ui/` names white balance, and nothing holds a second copy of a response that would be wrong the first time somebody adjusted the range. A bisection rather than a closed-form inverse because only monotonicity is part of the bargain — the expression is free to become a table tomorrow. The result is rounded to the precision the control is drawn at, which is not cosmetic: unrounded, sampling something already neutral lands a ten-thousandth off zero, and the photograph comes back modified with an undo step for a correction of nothing. On the panel side this needed one distinction the generated path was missing. `is_on_canvas` was being read as "and so the panel draws nothing for it", which is right for a crop — four edge fractions are not controls anyone drags in a list — and wrong for an eyedropper, which *writes* temperature and tint and leaves them exactly the controls a photographer reaches for next. So a sampling widget keeps its sliders and puts the affordance that arms the canvas in the group's heading, built like the reset beside it. One click, one sample, one history step: `Edit::Action` never coalesces, and there is no hover preview to fill the stack with temperatures nobody chose. Declaring the presentation also groups temperature and tint under one undo step, where they were two. That follows from what `Presentation` means and reads correctly — white balance is one decision — but it is a change, and worth saying so. |
||
|
dtourolle
|
efa9d84aad |
Correct the lens first and settle the grain last
`Attribute::ALL` has claimed since it was written to be roughly the order a photographer works in, and |
||
 dtourolleandClaude Opus 5
|
474dcf0bf6 |
Let Compose lead the attributes, as their own doc always said
`Attribute::ALL` claims to be "roughly the order a photographer works in" and then listed framing fifth, behind tone, colour and detail. Framing is the first decision made about a photograph and the one every later judgement is made inside — there is no sense balancing tones across a frame about to lose a third of its width. The contradiction was harmless while the list only fed a row of chips nobody reads in order. It stops being harmless now that the same list drives a column read top to bottom. `declared::Attr::ALL` moves with it. The two are separate spellings of one vocabulary and a test asserts they agree, which is what caught this rather than the order silently disagreeing between the YAML front end and the crate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
8e7b1350bf |
Name the frame's category for the decision, not the maths
`Attribute::Geometry` becomes `Attribute::Compose`, and `film_sim` moves from `[tone, colour]` to `[effect]`. Two categories were doing the wrong job. "Geometry" describes what crop, straighten and the quarter turns do to coordinates — but it describes lens distortion correction exactly as well, and that is not a compositional choice at all. Naming the attribute for the photographer's decision is what separates it from `Optics`: one is what the lens did, the other is what they chose. The maths the two have in common is not the thing worth filing them under. A film stock declared both `tone` and `colour`, so "Kodachrome" appeared in the Light group beside exposure and again in Colour beside white balance — two places, neither of which is where anyone looks for it. It is neither: `Effect` is defined in this same file as "applied rather than corrected — a look, not a fix", which is what a stock is. That it moves tone and colour is true of every look, and is not what the attribute is for. `from_name` still accepts "geometry" on the way in. That string is persisted in `develop.copy_attributes`, and an entry it fails to parse is not an error — `presets::scope_for` logs it and drops it — so without the alias an existing settings file would have quietly narrowed what a paste carries. `name` writes the current spelling, so the file migrates itself the first time it is saved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
bb35665bd2 |
Let a paste carry some kinds of edit and not others
FR-DEV-6 asks for presets "covering a subset of the edit graph". What landed with the named presets covered two subsets: everything, and everything but the crop. "Match the colour but not the sharpening" had no way to be said. `Scope` is now a set of `Attribute` — the same six kinds every operation already declares and the develop panel already builds its tabs from. The photographer ticking "tone and colour" is naming the groups they navigate by, and neither this module nor the interface has to name an operation to do it (FR-DEV-3c). The pleasing part is what left. Framing used to be excluded by an explicit test against one operation's id; it is now excluded because Geometry is not in the default set. The special case dissolved into the general rule, and the argument for it — a crop is a decision about *this* photograph, and carrying it across forty destroys forty compositions — is now a statement about a kind of edit rather than about a node. All thirty-three existing preset tests pass unchanged, which is the evidence that the generalisation kept its promises. One decision that is a field rather than a rule, because the two cases genuinely differ. An operation this build cannot classify — from a newer version, arriving over sync — travels under "everything" and "everything but the crop", because those are claims about the whole edit and an unrecognised operation is part of it (FR-NC-8). It does not travel under a hand-picked set, because that is a claim about kinds, and an unknown kind is not one of the kinds that were ticked. The settings page's "Copy crop and rotation" checkbox is gone, replaced by the same chips the preset sheet draws. It asked the right first question — geometry is the kind whose accidental travel destroys work — but it was the only question a boolean could ask. The field stays in `Settings`, read exactly once to seed the new set, so anyone who had ticked it keeps their behaviour. The chips are deliberately not in the develop column. Six of them there would set the width of the whole sidebar, which is the bug `ChipGrid`'s comment records at length. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
0c3b8cb1c4 |
Hand out descriptors a declaration could produce
`Operation::descriptor()` returned `&'static OpDescriptor`, and that lifetime
is the whole reason a build-time node is free and a run-time node is
impossible: only a compile-time literal can satisfy it, so no amount of
reading `ops/*.yaml` at startup could ever produce a descriptor the rest of
the application would accept. FR-PLG-2 says a bundled operation and a
third-party plugin are the same kind of thing, differing only in where the
file was found — and a lifetime outsiders cannot meet is exactly the second,
weaker format that requirement forbids.
So a descriptor is now owned and handed out as `Arc<OpDescriptor>`, with `Vec`
where it held `&'static` slices. `Arc` rather than a `&self`-borrowed
reference because the callers want to *keep* it: the develop panel collects
descriptors and then mutates the graph, and a borrow would tie the
descriptor's lifetime to a borrow of the operation it came from, which is the
one thing `&'static` was doing right.
The identifier newtypes deliberately did not follow. `ParamId` is `Copy`, is
compared in `match` arms against generated constants, is a map key in the
sidecar and history, and reaches Slint model rows; an `Arc<str>` there would
cost a refcount on every one of those and would take `match id { EXPOSURE =>
.. }` away from the generated code. They gain an interner instead, which is
honest about its lifetime rather than pretending to one — the set of ids is
bounded by deduplication and is process-lifetime by construction, because the
sidecar on disk names its parameters and an id has to stay resolvable for as
long as any edit naming it can be opened.
No behaviour changes. Every descriptor that was a `static` is a `LazyLock`
initialiser now, `Operation::helpers` borrows from `self` instead of being
`'static` so a future run-time node can own its list, and `Warp` and `Framing`
follow `Operation` so there is one shape rather than two.
The one place a descriptor is read per frame is `compose_full`, which takes
`descriptor().id` to prefix each active operation's uniforms, and `dr-ui`
composes on every frame it draws. That is a dozen atomic increments beside a
composition that is already building several kilobytes of WGSL on the same
call; it is noted at the trait method rather than left for a profiler to find.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
dtourolle
|
7421837c8a |
Let an operation say what it is about, so the panel can group without naming
Tool tabs need a taxonomy, and the taxonomy was the problem: a table in `ui/` mapping operation to tab breaks FR-DEV-3a, and a `group:` field risks what `ui-refinement.md` condemned `starts-group` for — the core deciding where the panel draws things. `Attribute` threads the needle. It says what an operation *is* — tone, colour, detail, optics, geometry, effect — which is the same category as `ParamKind` and squarely on the core's side of ARCH §4.3a's line. What is drawn, where it sits and whether it is visible stay the frontend's. There is no attribute for "the third tab", the enum's order is declaration order rather than screen order, and a frontend may render these as tabs, as headings, or ignore them. The payoff is that a tab strip can be *derived*: the groups are the attributes present in the capability list, so the interface names no operation and needs no table to keep in step. An operation joins the right group by declaring what it is, which is the one thing its author is well placed to say. Plural, because the tone curve is genuinely both — an RGB curve is tonal and the per-channel curves are chromatic, and filing it under one would hide it from half the people looking for it. Required and non-empty, enforced in `build.rs`, and the failure was checked by removing the line rather than assumed. An operation with no attribute is invisible to a panel that groups by them; a build that stops costs ten seconds, a control nobody can find costs more. The vocabulary is closed for the same reason: a typo would otherwise invent a category holding exactly one operation, which looks like a deliberate one until somebody counts. Six tests over the real chain, including the hand-written operations that `build.rs` never sees and so cannot check. |
||
|
dtourolleandClaude Opus 5
|
0a331c717e |
Give the controls a vocabulary, and let a node ask for one
widgets.slint set the rule — screens consume components, and a bare `Theme.*` at a call site means a component is missing — and it set it for chrome only. The controls never got the same treatment, so they were written wherever they were first needed and copied from there. **The slider was private to the develop panel.** `SliderTrack`, with the fifty-line preamble explaining how it wrests a drag away from a Flickable, lived inside adjust.slint and no other screen could reach it. It shows: export quality is a 1-to-100 value, and the settings page offered a free-text box for it, with the range written in a hint and enforced nowhere. `to-float()` answers 0 for anything it cannot parse, so a typo saved a quality of 0 and the page displayed the 0 back as though it had been asked for. The tick-box was written twice, in launch.slint and settings.slint, from the same 18px box and the same handler; the second carried a comment deferring the lift until a third caller appeared. The label-and-hint header was written three times inside settings.slint alone. controls.slint is the input layer beside widgets.slint's chrome layer, and the constraint that makes it reusable is that **nothing in it knows about `ParamRow`** — that struct is the develop panel's flattening of the capability model, and a control that imported it could only ever be used by the develop panel. The primitives take plain numbers; the ParamRow-shaped wrappers stay in the panel that owns the model. 658 lines came out of the three screens. `SliderRow` is the slider-plus-number-box ARCH §4.3 names as the pointer presentation of a bounded scalar, and quality is its first adopter. It commits on gesture end rather than on every movement, because the settings page saves to disk on change and a two-second drag is a couple of hundred writes where a text field committed once. The develop panel keeps the live stream — that is what its pipeline is for — so `SliderTrack` now reports both. **The other half is the descriptor.** FR-DEV-3a and ARCH §4.3a already specify more than was built: an ordered preference list of widgets rather than one, the demands a widget makes, and kinds beyond scalar and bool. - `Presentation.widgets` is now a list, walked by `choose`, falling back to plain sliders. Falling off the end is not an error, and there is a test asserting an operation asking only for an unimplemented widget still yields one control per parameter. - `WidgetDemand` carries what a widget inherently needs — two-dimensional dragging, precise pointing — and no pixels, breakpoints or platform names. - `WidgetKind` grows to the specified set. There is deliberately no `Colour` *kind*: a colour is three numbers, and a value type that is not an `f32` would reach through the graph, the uniform block and the sidecar format to buy what `ColourWheel` over three scalars already describes. Every widget here is a hint over ordinary scalars, which is what keeps the fallback honest. - `ParamKind::Enum` is the one new shape, and it fits because a variant index is exact in binary32. `kind: enum` with a `variants:` list works in `ops/*.yaml`, so a node declaring one gets a segmented control with no UI file edited — which is the promise ops/mod.rs already makes. The panel's dispatch was duplicated: a lone parameter and a grouped one each wrote out their own list of kinds, so `enum` would have had to be added twice and a kind added to one would appear or vanish depending on how many parameters its operation happened to declare. `ParamControl` is now the only such chain. `rows_from` is free-standing rather than a method, which is what lets the FR-DEV-3c acceptance test requirements.md asks for actually be written: an operation the frontend has never heard of, appearing in a generated panel, with no GPU in sight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
9b2ee0d0eb |
Show the colour mixer as three runs of twelve, each row a colour
Build and test / Desktop (Linux) (push) Successful in 17m20s
Build and test / Layer separation (push) Successful in 33s
🐳 Android image / Build and push (push) Successful in 2s
Build and test / android-image (push) Successful in 2s
Traceability / Requirement traces (push) Failing after 26s
Build and test / Android (aarch64) (push) Failing after 8m59s
The mixer was thirty-six sliders reading "Hue / Sat / Lum" twelve times over with nothing saying which band any row belonged to. The identity was there all along — the descriptor declares param.mixer.orange.sat and BANDS carries orange at 30° — and was discarded on the way out: labels.rs had no mixer entries, so every key fell through to a derived label that yields the bare channel name. A parameter can now say which aspect it adjusts and which subject it adjusts it on, with the subject's hue where the subject is a colour (descriptor::Facet). That is data about what the operation does, not a layout: the mixer genuinely weights pixels around 30°. What to draw from 30°, and in what order to stack the runs, stay in dr-ui (ARCH §4.3a) — develop.rs brings rows sharing an aspect together and marks the first of each, and adjust.slint names the run once and draws a swatch, a track and a readout on one line. Grouped by channel rather than by band because an edit is almost never "everything about orange"; it is the saturation of the greens, made by comparing one channel across neighbouring bands. Twelve band sections put those twelve rows in twelve different places. The swatch is the label, which is what makes twelve rows fit where four did. The band name is not lost: it is the row's accessible label, so the control is not colour-only, and labels.rs is where the mapping is written down — including chartreuse as "Yellow-Green" and spring as "Blue-Green", since nobody hunting foliage scans a list for "Spring". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolle
|
f630a3ff81 |
Wire the launch screen into the app
The app now opens on the login screen when there is nothing else to show
— no local paths and no configured library — and goes straight to the
images otherwise. Making someone click past a login they already
completed is pure friction.
launch.slint imported by app.slint, replacing the window rather
than overlaying it: there is no library to look at
until an account is configured
launch_ui.rs the Slint wiring, kept out of lib.rs so the launch
flow can change without touching the develop window
Login runs on a worker thread and posts results back through a channel,
since Slint's event loop is single-threaded and a 20-minute browser wait
cannot block it. The system browser is opened via xdg-open, never an
embedded webview (FR-NC-1).
Sign-out deletes the local credential even if server-side revocation
fails: a network error must not leave a usable secret on the machine.
Format tick-boxes persist on each toggle, so a selection survives a
crash before the library is opened.
Two things deliberately incomplete rather than faked:
- "Choose folder" lists the account's folders and reports them, but
there is no picker widget yet, so selection still happens via the
connect example.
- "Open library" logs the request. Opening a remote library needs the
scan-and-cache path, which belongs with the catalog work in flight.
Earlier I broke the other in-flight dr-ui work by calling
slint_build::compile twice, which replaces the generated module. The
correct wiring is an import inside app.slint, which is what this does.
30 dr-ui tests passing; both launch paths verified by running the app.
|
||
|
dtourolle
|
c8bb08e661 |
Add folder scan with format selection; validate A3 on a real library
Library setup as the user described it: pick a folder, choose which RAW
types to look for, scan recursively.
dr-types::FormatFilter the tick-box selection, seeing through VFS
placeholder suffixes so a dehydrated CR2 still
matches as a CR2
dr-sync::scan recursive walk, Depth:1 per directory, pruning
unchanged subtrees where the backend propagates
directory ETags
Verified against nextcloud.tourolle.paris (34.0.2) on a real library:
browse root 32 entries, 98ms
scan PhotosRaw 17,185 RAW files in 334 directories, 34.1s
(7,836 CR2 + 9,349 DNG)
range read 262KB of a 21.5MB DNG in 119ms — 1.22% of the file,
and enough to read "Canon EOS 6D | ISO 100"
That last line is assumption A3 validated on real data. Cataloguing this
library by whole-file fetch would move roughly 370GB; the range path
moves a few MB.
Pruning is capability-gated rather than assumed: with per-entry ETags a
probe costs a request and proves nothing about children, so it is skipped
entirely. A test asserts zero probes in that case.
Still unresolved: /core/preview returns 400 for every parameter
combination tried, including on a JPEG the server reports as having a
preview. Not a request-shape bug — it fails identically bare. Recorded
rather than worked around; ARCH §6.7 already treats server previews as
opportunistic, so nothing depends on it.
|
||
|
dtourolleandClaude Opus 5
|
78e3e6b846 |
Add the develop pipeline: demosaic and seven raw adjustments
Decode through display, on the GPU: black/white normalisation, Bayer demosaic, camera colour transform, and the first seven adjustment operations — white balance, exposure, highlights/shadows, blacks/whites, brilliance, vibrance, saturation. Composable shaders. Each operation contributes a WGSL fragment rather than owning a pass, and dr-pipeline fuses the *active* ones into a single compute shader. One texture read and one write per frame regardless of how many adjustments are in play, while the operations stay independent in Rust — adding one is a new file, with no central shader to edit. An operation at neutral settings contributes no code, no uniform and no branch. Uniforms are prefixed per operation so two may both declare `amount`; helpers dedupe by name from a single source of truth. Pipelines cache on a structure hash covering the op-set and its order but not the values, so dragging a slider uploads uniforms and reuses the compiled pipeline. Measured on a 24 MP CR2: 0.60 ms re-render, one pipeline compiled across ten slider positions. The UI is generated, not written. EditGraph::capabilities() reports parameters with their kinds, ranges, defaults and current values; the panel builds one control per entry chosen by ParamKind. No file in ui/ names an operation, and dr-pipeline has no wgpu dependency, so codegen is testable without a device (ARCH §6.5a). Three defects found against real files, each silent: - rawler 0.7.2's `xyz_to_cam` is all zeros — deprecated and no longer populated. The live matrices are in `color_matrix`, keyed by illuminant. Reading the old field yields no colour transform at all. - `cam_to_xyz_normalized()` returns all NaN on any Bayer sensor: it divides each of four rows by its own sum, and the unused fourth (emerald) row sums to zero. Inverting the 3x3 ourselves avoids it. `wb_coeffs[3]` is NaN for the same reason and is normalised at decode. - As-shot white balance reached the uniform block but no shader read it, so the first render of a real CR2 came out violently green. Green photosites collect roughly twice the signal of red and blue. Now applied unconditionally before any operation, with tests on ordering. Demosaic is Malvar-He-Cutler rather than bilinear: gradient-corrected interpolation at one 5x5 neighbourhood per pixel, where bilinear leaves visible zippering on any high-contrast edge at 1:1. Two of the four packed CFA constants were wrong on the first attempt, so all four layouts are asserted to reconstruct the same colour. Crop origins at odd coordinates re-phase the pattern; without that, red and blue swap. X-Trans reports GpuError::UnsupportedCfa rather than approximating with the Bayer path, which would look like a corrupt file. 206 tests, including GPU tests proving every operation and the full seven-operation chain generate compilable WGSL. Known gaps: the display path still reads back to the CPU each frame, which ARCH §6.1 forbids and AC-8 asserts against — it is gated behind the `readback` feature and waits on spike S1 wiring Slint's texture import. Curve shapes are a first draft and want tuning against real photographs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |