3b5d56449500e7c88673d85914e8dc825e2900cb
77
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  dtourolleandClaude Opus 5
|
31e20399c8 |
Read the true white level, and clamp the sensor stage at both ends
Two corrections to the sensor stage, found while chasing magenta highlights. Neither is the cause of that — see below — but both are wrong on their own terms. `white_level` took the *first* of rawler's per-channel saturation points. On a Canon 6D that reports 15070 while the data reaches 16383, so every sample above it was treated as brighter than white. It takes the maximum now. The normalisation clamped its floor and not its ceiling, so those over-white samples passed through as values above 1.0. Clamped at both ends. **This does not fix the pink.** Measured on _MG_8596.CR2, exported and looked at: the subject renders correctly and only the blown sky is magenta. A fully clipped pixel is (1,1,1) in raw, the as-shot balance multiplies it to (1.93, 1.00, 1.68), and the camera matrix turns that into R 2.88, G 0.51, B 2.03 — red and blue clip at one, green does not, and the result is magenta. It is correct white balance applied to already-saturated data, which is the classic highlight-clipping cast and needs highlight desaturation to fix: a pixel at saturation carries no colour information and must be rendered neutral, not balanced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
7d0fb710a6 |
Bound a readback by time, so a full-resolution export can finish
Exporting a real 20 MP CR2 failed every time with "readback did not complete", while the copy itself was perfectly healthy. The bound was 100,000 non-blocking polls. That sounds generous and is not: a `Poll` that finds nothing returns immediately, so the loop spent its entire budget in a few milliseconds. Small transfers — the histogram's 4 KB, a viewport-sized frame — happened to land inside it. An 80 MB frame never could. It is a deadline now, thirty seconds, which is the only thing the bound was ever for: catching a lost device that will never deliver the callback. A one-millisecond pause after the first sixty-four spins stops the loop saturating a core for the length of the copy, while keeping a small transfer as immediate as it was. Found by developing /home/dtourolle/Downloads/_MG_8596.CR2 through the export example: 5472×3648 renders in 127 ms and writes all five formats. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
654c11300e |
Pin the framing geometry on pixels, not on the generated shader
Chasing a reported shear on rotate and straighten. Two tests, and what they prove is that the pipeline is not where it comes from. A circle is the shape that makes anisotropy unmissable: any transform scaling the axes unequally returns an ellipse, and the ratio of its axes is the error. Both a quarter turn and a 20° straighten, on a 3:2 frame, return a circle within 8%. Worth recording because I had a confident and wrong hypothesis first. The quarter turn carries `* aspect.x` on one component and `/ aspect.x` on the other, which reads like an anisotropy of a-squared, and the reasoning that `p` is already isotropic is plausible enough that I changed it. The existing `a_quarter_turn_corrects_for_aspect_across_the_swap` caught that immediately, and these tests then showed the original was right all along: the crop rect is expressed in the *turned* frame and the output axes swap with it, so the factors are the conversion between those spaces rather than a mistake. Reading the shader and reasoning about which space `p` lives in is exactly how a plausible formula gets written twice. These assert on real pixels off a real adapter instead, so the next person to suspect this transform can rule it out in one command. The shear is therefore in the display path — the fit from the framed size to the viewport, or the crop overlay's uncropped render — and not in the geometry the pipeline computes. Not yet fixed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
02ae92ba0d |
Tidy what the seven-branch merge left behind
Build and test / Desktop (Linux) (push) Failing after 57m16s
Build and test / Layer separation (push) Successful in 34s
🐳 Android image / Build and push (push) Successful in 3s
Build and test / android-image (push) Successful in 3s
Traceability / Requirement traces (push) Successful in 1m7s
Build and test / Android (aarch64) (push) Failing after 9m41s
Three lints, all from merged work rather than from any one branch: `terrace` and `disc` were steps on the way to the ramp the plateau test now uses, and the reasoning that discarded them lives in docs/segmentation.md §12 rather than needing the code; two mechanical clippy suggestions in segment and cache. 1176 tests pass, clippy and fmt clean, traceability regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolle
|
76bb6b2847 | Merge branch 'worktree-watershed-plateaux' | ||
|
dtourolleandClaude Opus 5
|
0b20436445 |
Prove the plateau pass does nothing, and stop paying for it
Picks up the lower-completion work a crashed session left mid-debug, with one failing test and no diagnosis. The diagnosis is that the pass is a no-op. Not "does not reduce basin count" — it changes *no pixel's basin at all*, zero of 9216, comparing one plateau iteration against sixty-four. That assertion is the substance of this commit: the original test asserted a consequence (fewer basins) which a working pass need not produce, so it could have been satisfied by weakening it. A no-op check cannot pass vacuously, and it is what turned an opinion into a fact. Three candidate causes were tried and none was it. Exact float equality is genuinely wrong and is fixed regardless — a gradient computed from 8-bit samples is never exactly equal across a region the eye calls flat, so `==` never fires and `<` fires everywhere; `LEVEL_EPS` now sits behind all three comparisons. The test image is not it either: a flat disc, a terraced disc and a constant-slope ramp all behave the same. The finding worth keeping is about the domain rather than the code. On a gradient-magnitude watershed every flat region of the picture is at gradient zero, the global minimum, and a plateau with no descending exit is a minimum — one basin already, nothing to resolve. The plateaux lower-completion is defined for are regions of constant non-zero gradient, which are rarer in a photograph than F1's phrasing implies. That may be the whole answer, or it may be hiding a fourth cause; I could not close it. So `plateau_iterations` defaults to 0. The implementation stays, correct as far as it goes and costing nothing until someone finishes it; the test stays, ignored with its reason; docs/segmentation.md §12 records what was ruled out so the next attempt starts further along than this one did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
a8b28136a6 |
Merge the batch export, and settle the seven-branch merge
Resolves the last of the parallel work. Two conflicts worth recording, because both were semantic rather than textual: `render_for_export` gained a colour space on master while the batch branch was rewriting the single-image export path around it. Kept both: the batch request supersedes the synchronous path, and the space still has to be chosen at render time because the conversion happens in the shader before the clip to 0..1. `render_open_frame` takes it as an argument rather than reaching for a controller it does not hold. The map-wait moved into `readback::await_mapping` on one branch while another was editing the constant it used, so `READBACK_POLL_LIMIT` survived the merge with no callers. Removed rather than left for clippy to find later. 1164 tests pass, clippy clean, fmt clean. Traceability 53.0% -> 54.3%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolle
|
cfff6a3302 |
Merge branch 'zero-copy-display'
# Conflicts: # core/dr-gpu/src/adjust.rs # ui/dr-ui/src/develop.rs |
||
|
dtourolle
|
9fc8721fa8 |
Merge branch 'histogram'
# Conflicts: # ui/dr-ui/src/lib.rs |
||
|
dtourolleandClaude Opus 5
|
0233df4bf2 |
See what the highlights are doing: a live histogram (FR-DSP-7)
Exposure, blacks and whites were set by eye. Nothing said a highlight had blown — the canvas shows white where a channel is at 250 and white where it is at 255, and the difference is the whole question. **Counted on the GPU, not on the readback.** There is a full frame sitting in CPU memory on every canvas update right now — `AdjustPass::read_output`, the bridge spike S1 removes — and walking it would have been thirty lines and no shader. FR-DSP-7 states the mechanism and not just the feature: "these derive from a GPU-side reduction into a small buffer. Per-frame CPU readback of image data is prohibited." A histogram founded on the bridge would be correct today and deleted by S1, and would meanwhile be the reason the bridge could not go. What crosses the bus here is 4104 bytes whatever the image size. The reduction tallies into workgroup memory first and merges once per workgroup. A photograph is not noise: a clear sky puts tens of thousands of adjacent pixels in one bin, and contending for that single global atomic serialises the dispatch. **On the settled frame only.** `render_now` already knows whether a gesture is still moving — `draft` is the flag `redraw` derives from `was_coalesced` — so the dispatch and its transfer happen once when the slider stops rather than on each of the forty frames a drag emits. Nothing is lost: a histogram flickering past under a finger is not a reading anyone takes. FR-DSP-7 requires exactly this, that it not extend the FR-DSP-3 frame budget. Luma is weighted in 8.8 fixed point — 54, 183, 19, summing to 256 exactly — rather than in floats. Not thrift: it makes the shader's arithmetic reproducible bit for bit, which is what lets the test below be an `assert_eq` against a CPU count rather than a tolerance. ARCH §6.13's line about integer state, applied where it happens to also be free. **What the numbers were checked against.** A flat frame must put all 4096 pixels in one bin and one only. A 256-wide ramp must occupy every level with exactly the same count, which is what catches an off-by-one in the quantisation — a `floor` where a rounding was needed shifts the whole photograph one bin left and looks like nothing at all. And a 101x37 frame of seeded pseudo-random pixels — deliberately not a multiple of the 16x16 workgroup, so the edge tiles run off the image — is compared slot for slot against a second, obvious CPU implementation. Exact equality, no tolerance. The CPU version is a deliberate reimplementation rather than shared code: the bugs worth catching here are ones shared code would commit identically on both sides. Above that, the presentation arithmetic is unit-tested headless, because it is where a wrong answer is invisible. A histogram of the wrong shape looks exactly as plausible as one of the right shape. So: 64 columns because it divides 256 and an uneven fold draws an even ramp as a comb; the peak excludes the end columns, or a night scene scaled against its own black spike is a flat line with no information in it; heights are clamped into the plot; and "0%" is kept distinct from "<0.1%" and from "—", since an indicator reading "clipped" over a figure reading "none" is a panel contradicting itself. Clipping counts a *pixel* with any channel at an extreme, not a channel. Any, because a blown red has no gradation left in it however much green and blue still hold — and it is the saturated highlight, the sunset and the red jersey, that clips first and recovers worst. Per pixel, because counting channels can report 200% of a frame clipped, and a percentage above 100 is a readout nobody trusts again. Two affordances for it, which NFR-A11Y-3 asks for: a bar standing at the end of the plot the tones are piling against, and a figure saying how much. Either alone reads. The panel sits directly under the capture metadata and above every control, because it is what the controls are judged against. It is hand-built rather than generated, and ARCH §4.3a is untroubled: a histogram is not an operation — no parameters, changes nothing, answers a question rather than asking one — and nothing in it reads a parameter out of a descriptor. Three plot colours and a neutral luma trace join the palette. That is the swatch's exception rather than a second one: a per-channel histogram has to say which channel, and no achromatic treatment distinguishes red from blue, so the hue is data exactly as the image beside it is. Held well back from full strength for the reason the theme preamble gives. The bounded, non-parking map wait moves out of `AdjustPass` into `readback::await_mapping`, shared with the histogram's transfer. Thirty lines of load-bearing reasoning about frozen interfaces and lost devices, and two copies of it would have drifted. The histogram describes the frame on the canvas, so it is in the output colour space FR-DSP-7 asks for, and when zoomed it describes the visible region — a photographer inspecting a highlight at 4x is asking about that highlight. A device that cannot build the reduction loses the histogram and keeps the photograph. Still to do for FR-DSP-7: the pixel colour readout under the cursor. 324 tests pass, clippy and fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
cf8f5b632f |
Show the develop frame itself, instead of a photocopy of it
The oldest open item in the project (ARCH §6.1, spike S1, AC-8). Every frame
in develop was read off the GPU into a `SharedPixelBuffer` and handed back to
Slint to upload again: ~7 ms at 4K against a 0.28 ms compute pass, 96% of the
frame spent carrying pixels to the CPU and back so they could be drawn where
they already were.
Slint 1.17 will adopt a `wgpu::Texture` directly, and the whole of what that
needs is arrangement rather than code.
**One device, made before the window.** A texture belongs to the device that
allocated it, so the compute passes and the compositor cannot each open their
own. `GpuContext::new_shared` opens one and hands back the instance and
adapter alongside it; `dr_ui::shared_gpu` gives all four to
`BackendSelector::require_wgpu_29(WGPUConfiguration::Manual { .. })`. That
call has to come before the first window, because creating one selects a
backend for you — which is why the GPU is now opened at the top of `run`
rather than two hundred lines down beside the other controllers.
dr-gpu still names no UI type. It hands out raw wgpu and does not ask who is
compositing (ARCH §6.5a).
**Vulkan only on the shared path**, where headless keeps its GL fallback.
wgpu's GL backend reaches its display through EGL at instance creation, and
before a window exists there is no display handle to give it — so a GL
instance cannot later produce the window surface Slint needs from it. A
machine with no Vulkan gets no shared device and browses without develop,
which is the same degradation as no adapter at all.
**`renderer-femtovg` becomes `renderer-femtovg-wgpu`.** The old one is FemtoVG
over OpenGL and cannot be handed a wgpu texture at all. It is not kept
alongside as a fallback: FemtoVG-over-GL has no branch for an imported
texture, falls through to "render this image into a buffer", gets nothing, and
draws nothing — a blank canvas with no error, which is worse than the failure
it would be papering over. The consequence is stated plainly in the manifest:
the desktop app now needs a working wgpu adapter to open a window.
**Two output textures, not one, and this is the part that is not obvious.**
Slint repaints when the image property *changes*, and it decides that with
`PartialEq` — which for two images over the same `wgpu::Texture` says
"unchanged". A pass that reused a single target would have rendered every
slider move correctly on the GPU and shown none of them: right, and invisible.
`AdjustPass` alternates between two targets, so consecutive frames are
genuinely different values. It also settles the read-while-write question that
one queue was already answering.
`RENDER_ATTACHMENT` is added to both render targets. Neither pass uses it;
Slint rejects an imported texture without it, on the reasoning that a
compositor handed a texture may need to draw into it.
**`AdjustPass::read_output` is deleted rather than gated.** It and
`export_pixels` were the same transfer under two names, and the comments
explaining why they were separate are the point of the whole criterion:
reading pixels back to *display* them is the defect, reading them back to
*encode a file* is the only way a file is made. The display twin is now gone
outright, which is stronger than a feature flag — it cannot be turned back on.
`export_pixels` is untouched and still ungated. The `readback` feature comes
off dr-ui, darkroom-desktop and darkroom-android; it stays in dr-gpu, where it
still gates `RenderTarget::read_pixels` and the segmentation field readback.
`examples/develop` moves to `export_pixels`, which is honest — it writes a
PPM — and so no longer needs the feature.
Four tests, each named for what it protects and each of which fails without a
screen if the property it guards breaks:
- the adjust target satisfies every condition Slint's import checks, asserted
in the crate that owns the descriptor, because a descriptor that drifts
fails at runtime on a real display and nothing else would notice;
- consecutive renders are different textures, and the third is the first
again, so the alternation is a rotation and not an allocation per frame;
- the develop canvas has no CPU pixel buffer and does have a wgpu texture —
AC-8 itself, in the terms Slint uses;
- consecutive frames compare unequal as `slint::Image`, which is the property
the repaint actually depends on.
The zoom test's readback moves into the test module. It has to: there is no
library function that copies a displayed frame to the CPU any more, and that
is the point — the round-trip now exists in the test binary and nowhere a
shipping build can reach.
**What is not proven.** No GUI was run. What is verified is that the texture
satisfies the import contract, that the import succeeds, that the canvas is a
texture rather than a buffer, and that consecutive frames are distinguishable.
What is unverified is everything that needs a display: that Slint's FemtoVG
wgpu renderer adopts the Manual configuration on a real surface, that the
picture appears the right way up and the right colour, and the frame timing
that motivated the whole exercise. Android is untouched by testing — the
android backend routes a WGPU29 request to Skia, whose wgpu surface does
handle imported textures, but that is read from the source, not observed.
56 dr-gpu tests and 255 dr-ui tests pass, clippy clean under `-D warnings`,
fmt clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
dtourolle
|
1c16ca3d27 | Merge branch 'colour-managed-export' | ||
|
dtourolleandClaude Opus 5
|
914d14ec0d |
Tell the shader which colour space it is encoding for
The generated shader ended with `encode_srgb` and a clamp, so every photograph leaving DarkRoom had been through sRGB's gamut whatever the settings page said. Export refused the other three spaces rather than tag clipped pixels with a gamut they did not contain — correct, and not something an encoder could fix. So the output space becomes a parameter of composition. `compose_for` emits a constant primaries matrix after the camera matrix and before the clip, and generates the transfer function to match: the sRGB curve for sRGB and Display P3, a pure 2.199 gamma for Adobe RGB, 1.8 with a linear toe for ProPhoto. The ordering the camera matrix depends on is untouched — operations still run in camera space — and sRGB emits no conversion at all, so the shader compiled on nearly every frame is byte-for-byte what it was. The numbers live in dr-types, derived from four chromaticity pairs per space rather than tabulated. That is not tidiness: the shader encodes the pixels and the ICC profile describes them, and a file whose profile disagrees with its own contents is worse than one with no profile. One derivation makes them agree by construction, and can be checked against the values the specifications publish. Profiles are generated here too — minimal v2 matrix/TRC, about 2 KB, pure Rust, no lcms to satisfy under the NDK. A JPEG carries it in APP2, a PNG in iCCP, a TIFF in tag 34675. sRGB gets one as well, because untagged does not mean sRGB, it means guess. The refusal survives in a sharper form. A `Frame` now carries the space it was rendered in, and export refuses to label it anything else. The develop session still composes for sRGB, so a P3 export from the interface fails with an accurate error instead of producing a file that lies — the frontend half is a separate change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
1c0994c807 |
Demosaic a Fujifilm sensor instead of refusing it
Every RAF stopped at the embedded preview, because the demosaicer had one kernel and it was a Bayer kernel. D11 makes Fujifilm first-class and FR-RAW-5 asks for it by name, so a hard error there was a promise we had not kept. X-Trans is a 6x6 tile, and nothing in the Bayer path survives that: the missing channels sit at different offsets at all 36 positions, so there is no fixed kernel to write. The new shader fits a weighted plane through each channel's samples in a 5x5 window and carries the other two channels across as the difference between those planes, keeping the pixel's own measured value untouched. A plane rather than a mean because the three channels are sampled at different places in the tile: a mean compares a red taken slightly left of the pixel with a green taken slightly right of it, and that offset is a colour cast that follows every gradient in the frame. The fit is done in white-balanced space, where the constant-colour-difference model it rests on is actually true of a neutral subject; that alone halves the error at a luminance edge. Two compromises, both deliberate. It is not Markesteijn. There are no directional hypotheses and no homogeneity map, so it does not resolve detail finer than the CFA period and a hard edge arrives about two pixels wide. It cannot ring — the output is bounded by the local sample range — so it does not produce the worms FR-RAW-5 exists to avoid, but the quality that requirement asks for is still owed. The tile's phase is guessed rather than known. rawler has each body's pattern exactly, as a 36-character string, but CfaPattern::XTrans throws it away before dr-gpu sees the file, and it is not a constant to hard-code: the bodies in that database start the tile at four different origins. So the phase is read back out of the pixels, by grouping the 36 per-position means and taking the grouping with the least spread. That part needs nothing from the scene. Telling red from blue does — shifting the tile by half a tile turns it into itself with red and blue swapped, so no geometry can decide it — and the as-shot white balance is what breaks the tie. A frame that is almost entirely one colour can defeat that; widening dr-decode to carry the pattern string would retire the guess altogether. The tests assert reconstruction, not success: a flat patch comes back exactly at all six phases tested, and a linear ramp comes back exactly too, which is the property the plane fit exists for and the one a mean would fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
cb1d2be240 |
Choose the export folder by walking the server, not by typing it
The destination for a Nextcloud export was a text field. Nobody recalls the exact spelling of a path three levels down, and getting it wrong does not fail — `create_dir` makes whatever was typed, so a misremembered folder becomes a new one at the root and the exports are somewhere nobody looks. So it is picked the way the library root is picked, using the same `FolderBrowser` model the launch screen drives: up, into, and "use this folder", confirming the folder currently *shown* rather than one selected in the list. Same rule in both places, so the phrase means one thing. The model is shared; the worker is not. `settings_ui::spawn_folder_list` is a near-twin of the launch screen's, because that one reaches into the `LaunchController` for its session and reports onto the launch screen's error line, while this one is handed credentials and writes to the settings page. Factoring them together needs a function taking both controllers or a trait implemented twice to abstract two call sites — more machinery than the twenty lines it saves. What matters is shared already: navigation behaves identically because both drive the same model. The callbacks are wired in `lib.rs` rather than in `settings_ui::wire`, because listing a remote folder needs credentials and the settings page holds no session on purpose — it is reachable before a library is opened and must not depend on one existing. With no account the picker says to sign in first, rather than showing an empty list that reads as a server with no folders. Details that are decisions rather than accidents: the picker opens at the library root rather than at whatever half-typed path is in the field, which would list nothing and look broken. The listing area is a fixed 180px, since a folder with sixty children would otherwise push the rest of the settings page off the bottom. "Up" is disabled at the root rather than hidden, so the row does not jump as the user navigates. A failed listing leaves the picker open on the folder it was showing — where the user had got to is not something to discard over a dropped request. And the chosen folder saves immediately like every other setting on a page that has no Save button. The poll timer lives on the controller for the reason `LaunchController` keeps its own there: a `slint::Timer` stops when dropped, so one local to the function that starts it would be collected before the listing arrived. Carries in-flight work from a parallel session — a segmentation pass in dr-gpu, a sidecar cache, and the develop panel's continuing changes. 1020 tests pass, fmt clean. One clippy warning remains and is not mine: `sidecar_cache::dir` is unused while that work is in progress. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
151dcc3c02 |
Make a file out of a photograph
Export existed as a settings page and nothing else: format, quality, colour
space, five sizing modes, a filename template and a metadata switch, all
configurable in detail, and no way to produce a single file. dr-export is the
other half.
**It returns bytes and a name, and writes nothing.** An export has three
destinations with nothing in common — a path on Linux, a SAF document on
Android where there is no path at all (ARCH §6.9), and a PUT to a Nextcloud
folder — so a crate that opened the file itself would serve one of them and be
rewritten for the other two. The caller places the bytes.
Resize, then sharpen, then encode, in that order and for a reason: output
sharpening compensates for the softening the resample introduced, so its
strength scales with how much scaling actually happened, and sharpening before
shrinking would throw the result away. Lanczos-3, separable, with weights
computed once per output row — FR-EXP-4 asks for Lanczos or better because a
box filter turns a distant fence into moiré.
Collision handling takes the "is this name taken" test as a closure rather
than looking at a directory, because there is no directory it could look at
that works everywhere. That shape is not politeness toward Linux: Android's
createDocument renames on collision by itself and cannot overwrite at all, so
all three CollisionPolicy settings need the answer *before* anything is
created. Overwrite, Skip and Increment are each tested, and Increment gives up
after ten thousand rather than spinning against a destination that reports
everything as taken.
Three things are honest rather than done:
- **Colour space.** sRGB only. The shader encodes and clips to sRGB before
this crate sees a pixel, so tagging a file Display P3 would claim a gamut
it does not contain. Refused with a typed error instead of mislabelled;
honouring it is a pipeline change (FR-EXP-2).
- **AVIF and JPEG XL.** No encoder. libaom and libjxl are C, ravif is slow
enough to change what a batch feels like, and the settings page offers
both because FR-EXP-1 lists them — so asking for one says so rather than
writing a JPEG under a .avif name.
- **16-bit TIFF** is a real 16-bit file carrying eight bits of information,
because AdjustPass renders to Rgba8Unorm. Widened by *257, not <<8, so
white lands on 65535 rather than a quarter-percent grey. Making it mean
what it says needs the composer told what format to write.
Metadata is not written at all, which satisfies the half of FR-EXP-8 that
matters most: strip_location defaults to on, and a file with no EXIF block has
no GPS tag. Retaining camera and copyright when asked is not implemented and
cannot be faked by omission.
Also here:
- `AdjustPass::export_pixels`, ungated where `read_output` is behind a
feature. The two are the same transfer and opposites in intent: reading
pixels back to *display* them is what ARCH §6.1 forbids and AC-8 asserts
against, while reading them back to encode a JPEG is the only way a file
has ever been made. Separate methods so the instrumentation can count one
without counting the other.
- `ExportTarget`, so a destination can be a folder on the server. On Android
that is the only destination needing no platform work whatsoever — a PUT
against create_dir, already on the RemoteBackend trait, behaving
identically on both platforms. Switching target clears the destination,
since a path is not a remote folder and carrying one across would offer to
create a folder called `home` at the library root.
Verified end to end rather than by unit test alone: `cargo run -p dr-export
--example export` decodes a frame, runs the develop chain on the GPU at full
resolution, reads it back, and writes all five formats — 27 ms for a
full-size JPEG, 165 ms with a Lanczos reduction to 1200px. ImageMagick agrees
the 16-bit TIFF is 16-bit. dr-export cross-compiles clean for
aarch64-linux-android; all three encoders are pure Rust, which is why they
were chosen. 944 tests pass, clippy and fmt clean.
Not yet wired to a button. The develop view has no export action, so nothing
in the running app can reach any of this yet.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
dtourolleandClaude Opus 5
|
d7aeafaf84 |
Move to wgpu 29, the version Slint can share a device with
Build and test / Desktop (Linux) (push) Failing after 38s
Build and test / Layer separation (push) Successful in 24s
Traceability / Requirement traces (push) Successful in 1m3s
🐳 Android image / Build and push (push) Successful in 3s
Build and test / android-image (push) Successful in 3s
Build and test / Android (aarch64) (push) Failing after 9m54s
Groundwork for spike S1. Importing a texture into a Slint scene requires it
to come from the *same* `wgpu::Device` Slint renders with, and Slint hands
out a device of the version it was compiled against. Slint 1.17 offers
`unstable-wgpu-28` and `unstable-wgpu-29` and nothing older, so wgpu 23 could
never have met it: two semver-incompatible wgpu crates in one tree are two
distinct types, and the device would not typecheck across the gap.
The version is therefore not a free choice, and the manifest now says so —
Slint and wgpu move together or not at all. The Slint requirement is also
corrected from "1.9" to the 1.17 it has actually been resolving to.
Nothing about the render path changes here. The readback bridge is still in
place and still the display path, so this is verified by the tests that
already existed rather than by anything new: 39 dr-gpu tests, which compare
real pixels off a real device, and 888 across the workspace, all passing.
Zero-copy lands separately and small.
What the six releases cost, in full:
- `ImageCopyTexture`/`ImageCopyBuffer`/`ImageDataLayout` became the
`TexelCopy*` names (24).
- `Instance::new` takes the descriptor by value, and `InstanceDescriptor`
lost its `Default` — it carries a boxed display handle now, so a headless
context says `new_without_display_handle` and means it.
- `request_adapter` returns `Result` rather than `Option` (24).
- `DeviceDescriptor` absorbed the API trace from `request_device`'s second
argument and gained `experimental_features` (25).
- `PipelineLayoutDescriptor` takes `Option<&BindGroupLayout>` per slot, and
`push_constant_ranges` became `immediate_size`.
- `Maintain` became `PollType`, and `poll` is fallible.
Two of those are improvements worth having rather than churn. The error scope
is a guard whose `pop` runs on drop, so an early return from the pipeline
compiler no longer leaves a scope open on the device for whatever ran next to
fall into. And a fallible `poll` reports a lost device (NFR-R7) at the point
it happens, where before the map callback simply never arrived and the
failure surfaced later as a readback that spun out its poll limit.
Still to do for S1: dr-ui renders through `renderer-femtovg`, which is
OpenGL. Texture import needs Slint itself rendering on wgpu.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
dtourolleandClaude Opus 5
|
7c57f490fe |
Declare a develop operation in YAML, and generate the rest
An operation was, in the overwhelming majority of cases, four facts: what its parameters are, what uniforms they compute, what WGSL those uniforms drive, and where it sits in the chain. Written in Rust those four facts arrived wrapped in ninety lines of trait implementation — a match on parameter id to a struct field, another match back, an is_active comparing each field to its default, a Vec<Uniform> built by hand. All mechanical, and each one a place to make a silent mistake: a param() arm returning the wrong field reads perfectly and breaks the sidecar round-trip. So the four facts are the file now. core/dr-pipeline/ops/<id>.yaml is a node, build.rs compiles it into the same Operation impl as before, and the result lands in OUT_DIR — the same reasoning as style.yaml -> theme.slint, including why it does not land beside the sources it would look exactly like. Nothing downstream can tell a declared node from a hand-written one: same &'static OpDescriptor, same fused-shader composition, same sidecar. Nine nodes moved: exposure, white_balance, contrast, highlights_shadows, blacks_whites, brilliance, vibrance, saturation, and the shared WGSL helper registry. Their prose came with them, and so did their tests — set/expect/expect_active/expect_wgsl in the declaration compile to real #[test]s, so a node file carries its own proof rather than leaving it behind in a file that no longer exists. Two stayed in Rust and say so with `rust:`. The tone curve's neutral is a relationship between five interpolated points rather than a set of values; the colour mixer generates thirty-six faceted parameters from twelve computed hue bands. A schema stretched to cover either would be a worse language than Rust aimed at one caller. They still declare their position here, because the chain's *order* is the one thing a reader comes to this directory to learn, and an order written half in YAML and half in Rust would be worse than either alone. default_chain() is generated from it. Uniforms are derived by a small expression language — exp2(exposure), blacks / 100 * 0.02 — compiled to Rust rather than interpreted, so an unknown name or a wrong arity is a build error naming the file and the key and the arithmetic costs nothing at runtime. The build script refuses a duplicate order, a filename disagreeing with its id, a default outside its own range, a test value the graph would clamp before the node saw it, a helper that does not define the function it names, and a declared node colliding with a file in src/ops. Verified by adding a scratch node and removing it again: one file, no other edit, and it joined the chain at its declared order with its test running. 237 tests pass in dr-pipeline, clippy and fmt clean. .yaml joins the traceability tool's scanned suffixes, because a node's Rust now lives in OUT_DIR where a tag could never be linked from the report. Coverage 47.7% -> 48.3%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolleandClaude Opus 5
|
cd75e5a4c6 |
Run the library from local data when the server is unreachable
Also carries in-flight work that shared these files: the zoom structure-key fix in the adjust pipeline, nearest-neighbour filtering past 1:1, the timeline scrub marker correction, the 423-Locked retry in the metadata sweep, and the thumbnail size-class migration. # Offline mode (FR-CAT-9) The app previously assumed the server was reachable and treated its absence as a series of unrelated per-operation failures. A launch without a connection produced an empty grid, even with a complete catalog on disk and every thumbnail already in the shards. Reachability is now inferred from traffic the app was already making, rather than probed for. `RemoteError::indicates_offline` draws the line that makes this possible: a dead connection is offline, a 403 or a 500 is not — the server answered, so blanking the library over one forbidden file would be a worse error than the one being reported. `Reachability` turns those outcomes into a state, so a library browsing happily never issues a probe at all. Going offline takes one failure, because the user is already experiencing it. Coming back requires evidence — a completed scan or a fetched thumbnail — with a capped exponential backoff behind the manual retry, so twelve sweep lanes failing together do not schedule twelve immediate probes. What keeps working: the catalog opens even when the scan that normally provides it failed, so the grid fills from the last successful scan. Thumbnails come from the shards. Rating, flagging and collecting are catalog writes that never touched the network. What stops is opening an original that was never stored locally, and it now says so in those words instead of reporting "network error: connection refused" over a photograph. Work that is pure network is refused rather than left to fail slowly: the metadata sweep, derived sync, and sidecar writes. The sweep would otherwise spend a timeout per image across the whole library while the progress bar implied something was happening. Deferring sidecars is a real gap rather than a hidden one — a rating made offline reaches its sidecar only when that image is judged again while connected — and it is recorded as such at the call site. # The "On this device" filter A chip beside the rating filters, narrowing the grid to images whose original is held locally. It composes with the rating terms rather than replacing them, so "five-star frames I can actually edit on this train" is one filter. The predicate is SQL, like the rating terms and for the same reason: the count in the header has to agree with the cells drawn. It reads `image_cache.tier_actual`, which nothing writes yet — the next commit fills it. Until then the chip honestly reports zero. `Tier` gains an explicit on-disk encoding. The variants are ordered by generosity and the derived `Ord` invites reordering them, which would silently reinterpret every cached row; the round-trip test is what holds the two in agreement. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
dtourolle
|
250ff1e327 |
Poll the adjust readback instead of parking the UI thread
`read_output` waited on the copy with `Maintain::Wait`, which parks the calling thread until the GPU is done. That call is made from the UI thread, so the interface was frozen for the length of the copy — the note on this function measures it at ~7 ms at 4K against a 0.28 ms compute pass, so nearly all of it was the wait. `Maintain::Poll` drives the same callbacks without sleeping. The mapping still completes and the pixels are identical; the thread simply is not parked while it happens. The poll loop is bounded. A lost device never delivers the map callback, and spinning forever on that would hang the app rather than report the error the caller already handles. This does not remove the round-trip itself, which ARCH §6.1 forbids and spike S1 replaces by importing the texture into Slint directly. It stops the round-trip from blocking input until then. dr-gpu tests pass, including those comparing readback pixels. |
||
|
dtourolle
|
5786977a51 |
Develop a JPEG through the same pipeline as a RAW
DemosaicedImage gains a second producer, from_rgba8, alongside the CFA path. Nothing about the type is CFA-specific — it is "an image on the GPU, ready to adjust" — which is what lets develop mode work on a JPEG without the edit graph or any operation knowing the source was not a RAW file. The one real difference is the transfer function: sensor data is linear, a JPEG is gamma-encoded. Every operation assumes linear scene-referred colour (exposure is a multiply, and doubling a gamma-encoded value is not a stop), so the shader prologue linearises once, at the only point where the two source kinds still differ. The flag rides in as_shot_wb.w, which was padding. For a JPEG the white balance uniform is neutral and the colour matrix is identity, so both stay unconditional multiplies rather than becoming branches. max_dimension is exposed because it is a hardware limit the caller must plan around, not a failure to report afterwards: a 13728x8928 film scan exceeds the common 8192 texture limit, and fitting it first is the only way to develop it at all. Assisted-by: LLM |
||
|
dtourolle
|
2ca1716a29 |
Make "Choose folder" an actual folder picker
It previously fetched the folder list and threw it away into a status line — a button that looked like it worked and did not. Now it opens a browsable picker: click a folder to descend, ".." to go back, "Use this folder" to select, "Cancel" to leave the root unchanged. Descends one level per click because that is what the backend supports: Depth: infinity is frequently disabled server-side and prohibitively expensive where it is not (ARCH §8.4). The chosen root persists immediately on confirm, so it survives a crash before the library is opened. Confirming at the account root is allowed — a user may legitimately keep everything at the top level — and cancelling leaves any previous selection untouched, which a test asserts. Verified against nextcloud.tourolle.paris at both depths: 30 folders at the root, 21 year-folders inside PhotosRaw. 19 launch tests, 38 in dr-ui. |
||
|
dtourolle
|
09e3043f4c |
Add secure credential storage, sessions, and a launch screen
Login now persists properly rather than through the JSON file the test
harness was using.
dr-plat SecretStore trait plus a Secret Service backend.
Verified against the live GNOME Keyring: store,
retrieve, delete, confirm-gone all round-trip.
Session/SessionStore splits credentials from settings — the app
password goes to the keyring (FR-NC-2), while
server, login, chosen root and format selection are
ordinary config. A test asserts the credential never
appears in the config file.
LaunchModel the launch-screen state machine, testable without a
display server: sign in, approve in browser, choose
folder, tick formats, sign out.
launch.slint the screen itself, in its own file.
Absence of a secrets daemon is an explicit degraded mode, not a silent
fallback to plaintext — the screen says sign-in will not persist rather
than letting the user find out next launch. Android's Keystore backend
fails loudly for the same reason: a no-op store would look like it
worked and then lose the credential.
Two bugs caught by tests rather than by running it:
- fail() after busy() signed the user out, because busy() had already
discarded the session. A failed *scan* would have logged you out.
Busy now carries the session.
- normalise_server upgrades http:// to https:// rather than accepting
it. NFR-SEC-3 requires TLS, and silently sending a credential in the
clear is not a decision to make on the user's behalf.
launch.slint is not yet wired into app.slint. Calling slint_build::compile
twice replaces the generated module rather than adding to it, which broke
the other in-flight work on dr-ui; I reverted that immediately. Wiring it
needs an import inside app.slint, which is that work's file to change.
419 tests passing across ten crates.
|
||
|
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> |
||
|
dtourolle
|
0f202fd3f9 |
Add requirements traceability gate and Gitea pipelines
Ports JellyTau's traceability tooling to Rust, carrying across the bug it was repaired for. That gate divided a traced count by frozen literal denominators; the requirements file outgrew them and it reported 158% coverage, so it could never fail its own threshold. Two rules, both enforced by the extractor's own tests: - denominators parsed from docs/requirements.md at run time - coverage is |traced ∩ defined| / |defined|, never a raw traced count The gate additionally fails hard on a misconfigured run — zero requirements parsed or zero files scanned — rather than reporting a plausible 0%, and on any orphan tag naming a requirement that does not exist. Adapted for DarkRoom: IDs are FR-CAT-1 / NFR-P13 / FR-DEV-3a shapes rather than JellyTau's fixed three digits, and decisions (D), spikes (S), milestone items (M) and test ids remain taggable while being excluded from the denominator — counting them inflated it by 25. Also adds dr-sync: the RemoteBackend trait and capability model, so the Nextcloud connector is one implementation rather than the only shape the engine understands. No mature Nextcloud crate exists (reqwest_dav is too thin), so the connector will be hand-rolled over reqwest per D7. Gitea workflows follow the same style: containerised, commented with the reasoning, desktop and Android on every push, plus a CI check that no core/ crate depends on the UI toolkit (ARCH §6.5a). Coverage today: 13.3% (19/143). 50 tests passing. |
||
|
dtourolle
|
82a5e21ec6 |
Initial workspace: GPU context, compute pass, adaptive Slint shell
Establishes the v0.1 foundations on both platforms: - dr-types: SourceRef (never a filesystem path — Android SAF has none), Format, Availability, Validator with ETag quote normalisation - dr-gpu: wgpu device, compute pass writing a storage texture, resize - dr-ui: Slint shell with FR-UI-1 adaptive layout, computed in Rust to avoid a binding loop - docker/android: pinned toolchain, verified producing API 28 ARM binaries Measured the cost of the temporary CPU readback path (dr-gpu bench): compute is 0.06-0.28ms across sizes while readback is 0.63-7.43ms, so readback is 90-96% of frame time and scales with area. Recorded in ARCH §6.1 — this is why spike S1 is the priority. Mitigations pending S1: reuse the staging buffer, apply at most one resize per frame, and cap render resolution at 2048 on the long edge. 10 tests passing; core crates cross-compile for aarch64-linux-android. |