Merge branch 'master' into feat/library-toolbar
# Conflicts: # docs/gestures.md # docs/traceability.md
This commit is contained in:
@@ -0,0 +1,113 @@
|
||||
{
|
||||
"_readme": [
|
||||
"The committed numbers for DarkRoom's benchmark suite (docs/requirements.md §8).",
|
||||
"Produced and checked by `cargo run --release -p dr-bench`; docs/benchmarks.md explains each metric.",
|
||||
"",
|
||||
"Two gates, and they are not the same gate. `budget` is the requirement's own threshold and never moves.",
|
||||
"`recorded` is what the reference desktop last measured, and a run that drifts past `tolerance` beyond it",
|
||||
"fails the build even while still inside the budget — which is how most performance rot actually arrives.",
|
||||
"",
|
||||
"`recorded` is null on every metric because nobody has run the suite yet. That is deliberate: writing",
|
||||
"plausible-looking figures here would make every later comparison a comparison against a guess. Run",
|
||||
"`cargo run --release -p dr-bench -- record --reference` on the reference desktop and commit the diff.",
|
||||
"Until then the budget gate works and the regression gate says, in the report, that it cannot.",
|
||||
"",
|
||||
"`machine_sensitive` says whether a budget is a statement about a machine as much as about the code.",
|
||||
"Those budgets are asserted only under --reference: §8 names the reference desktop, and a two-core CI",
|
||||
"container cannot speak to a target written for twenty-four threads. Asserting one there would produce a",
|
||||
"red gate everybody learns to ignore, which is the trap core/dr-gpu/tests/frame_budget.rs already avoids.",
|
||||
"",
|
||||
"Several metrics carry a requirement ID with a qualifier. Read those literally. NFR-P7's budget here is",
|
||||
"checked against the encode half of an export only — no GPU render is in the figure — so it can fail the",
|
||||
"requirement and cannot pass it, and no TRACES tag claims otherwise. NFR-P8 has no budget at all yet,",
|
||||
"because nobody has decided how much of its 500 MB belongs to the catalog layer; this records the number",
|
||||
"that decision needs."
|
||||
],
|
||||
"tolerance": 0.15,
|
||||
"recorded_on": null,
|
||||
"recorded_at_unix": null,
|
||||
"fixture": null,
|
||||
"metrics": {
|
||||
"catalog_filtered_ms": {
|
||||
"requirement": "FR-CAT-6",
|
||||
"what": "Count plus first window under a rating filter, which compiles to a correlated subquery over versions.",
|
||||
"unit": "ms",
|
||||
"direction": "lower_is_better",
|
||||
"machine_sensitive": true,
|
||||
"budget": null,
|
||||
"recorded": null
|
||||
},
|
||||
"catalog_idle_rss_mb": {
|
||||
"requirement": "NFR-P8 (the catalog layer's share only — no toolkit, no adapter, no decode cache)",
|
||||
"what": "Resident memory of a process that opened the 50k catalog and scrolled ten thousand rows.",
|
||||
"unit": "MB",
|
||||
"direction": "lower_is_better",
|
||||
"machine_sensitive": false,
|
||||
"budget": null,
|
||||
"recorded": null
|
||||
},
|
||||
"catalog_open_ms": {
|
||||
"requirement": "NFR-P1",
|
||||
"what": "Catalog::open plus the count, first window and timeline the grid cannot paint without.",
|
||||
"unit": "ms",
|
||||
"direction": "lower_is_better",
|
||||
"machine_sensitive": false,
|
||||
"budget": 2000.0,
|
||||
"recorded": null
|
||||
},
|
||||
"catalog_open_warm_ms": {
|
||||
"requirement": "NFR-P1",
|
||||
"what": "The same four calls on a second connection, with SQLite's page cache already warm.",
|
||||
"unit": "ms",
|
||||
"direction": "lower_is_better",
|
||||
"machine_sensitive": false,
|
||||
"budget": 2000.0,
|
||||
"recorded": null
|
||||
},
|
||||
"catalog_window_p99_ms": {
|
||||
"requirement": "FR-CAT-4",
|
||||
"what": "One 400-row grid window at a random offset, p99 of one hundred.",
|
||||
"unit": "ms",
|
||||
"direction": "lower_is_better",
|
||||
"machine_sensitive": true,
|
||||
"budget": null,
|
||||
"recorded": null
|
||||
},
|
||||
"export_24mp_long_edge_2048_ms": {
|
||||
"requirement": "FR-EXP-3",
|
||||
"what": "The web export: resample a 24 MP frame to a 2048 px long edge, sharpen, encode. p99 of five.",
|
||||
"unit": "ms",
|
||||
"direction": "lower_is_better",
|
||||
"machine_sensitive": true,
|
||||
"budget": null,
|
||||
"recorded": null
|
||||
},
|
||||
"export_24mp_original_ms": {
|
||||
"requirement": "NFR-P7 (the encode half only — the GPU render is not in this figure)",
|
||||
"what": "Resample, output-sharpen and JPEG-encode a 24 MP frame at source size. p99 of five.",
|
||||
"unit": "ms",
|
||||
"direction": "lower_is_better",
|
||||
"machine_sensitive": true,
|
||||
"budget": 2000.0,
|
||||
"recorded": null
|
||||
},
|
||||
"thumbnail_per_image_p99_ms": {
|
||||
"requirement": "NFR-P3",
|
||||
"what": "One thumbnail on its own lane: decode the preview, downscale, orient, encode. p99.",
|
||||
"unit": "ms",
|
||||
"direction": "lower_is_better",
|
||||
"machine_sensitive": true,
|
||||
"budget": null,
|
||||
"recorded": null
|
||||
},
|
||||
"thumbnail_throughput_ips": {
|
||||
"requirement": "NFR-P3",
|
||||
"what": "Whole-sweep throughput: 1200 thumbnails through the sweep's chunk-and-lane shape, wall clock.",
|
||||
"unit": "img/s",
|
||||
"direction": "higher_is_better",
|
||||
"machine_sensitive": true,
|
||||
"budget": 100.0,
|
||||
"recorded": null
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,243 @@
|
||||
# The benchmark suite
|
||||
|
||||
**Status:** Built, not yet recorded · 2026-08-30
|
||||
**Companion to:** [requirements.md](requirements.md) §4.1 (performance targets) · §8 (verification)
|
||||
**Instrument:** [`tools/bench`](../tools/bench) — `cargo run --release -p dr-bench -- check`
|
||||
**Committed numbers:** [`bench-baseline.json`](bench-baseline.json)
|
||||
**GPU half:** [`core/dr-gpu/tests/frame_budget.rs`](../core/dr-gpu/tests/frame_budget.rs) ·
|
||||
[frame-budget.md](frame-budget.md)
|
||||
|
||||
§8 has said since it was written that performance is verified by *"an automated
|
||||
benchmark suite against a synthetic 50k catalog, run per-commit … A regression
|
||||
beyond stated tolerance fails the build."* Until this suite there was none. No
|
||||
`benches/`, no `[[bench]]`, no criterion, no fixture — and ten performance
|
||||
requirements that could therefore be neither passed nor failed, five of them
|
||||
carrying a `TRACES:` tag regardless.
|
||||
|
||||
This file is what the suite covers, what it deliberately does not, and how to
|
||||
read a failure.
|
||||
|
||||
---
|
||||
|
||||
## The state of it, first
|
||||
|
||||
**No numbers have been recorded yet.** Every `recorded` field in
|
||||
[`bench-baseline.json`](bench-baseline.json) is `null`, on purpose: writing
|
||||
plausible-looking figures into a baseline would make every later comparison a
|
||||
comparison against a guess, and the first real regression would be invisible.
|
||||
|
||||
To record them, on the reference desktop:
|
||||
|
||||
```sh
|
||||
cargo run --release -p dr-bench -- record --reference
|
||||
```
|
||||
|
||||
and commit the diff. Until that happens the **budget** gate works — a catalog
|
||||
that takes three seconds to open fails the build today — and the **regression**
|
||||
gate reports that it has nothing to compare against, rather than pretending.
|
||||
|
||||
---
|
||||
|
||||
## What it measures
|
||||
|
||||
| Metric | Requirement | Gated? |
|
||||
|---|---|---|
|
||||
| `catalog_open_ms` | **NFR-P1**, and R2's second sentence | Yes, everywhere — budget 2000 ms |
|
||||
| `catalog_open_warm_ms` | NFR-P1, page cache warm | Yes, everywhere — budget 2000 ms |
|
||||
| `catalog_window_p99_ms` | FR-CAT-4 | Regression only |
|
||||
| `catalog_filtered_ms` | FR-CAT-6 | Regression only |
|
||||
| `thumbnail_throughput_ips` | **NFR-P3** | Budget 100 img/s, on the reference desktop |
|
||||
| `thumbnail_per_image_p99_ms` | NFR-P3 | Regression only |
|
||||
| `export_24mp_original_ms` | NFR-P7, **encode half only** | One-sided: can fail it, cannot pass it |
|
||||
| `export_24mp_long_edge_2048_ms` | FR-EXP-3 | Regression only |
|
||||
| `catalog_idle_rss_mb` | NFR-P8, **catalog layer only** | Regression only — see below |
|
||||
|
||||
Two of those rows carry a qualifier, and the qualifiers are the point.
|
||||
|
||||
### Requirements this can now pass *or* fail
|
||||
|
||||
**NFR-P1 — catalog open under 2 s.** The measured span is the four things the
|
||||
library view cannot paint without: `Catalog::open` (which connects, migrates and
|
||||
**backfills**, and the backfill is three passes over the images table on every
|
||||
open), `count`, the first 400-row `window`, and the monthly `timeline`. Tagged
|
||||
`TRACES: NFR-P1` in [`tools/bench/src/catalog_open.rs`](../tools/bench/src/catalog_open.rs),
|
||||
because a build that breaks it fails this gate.
|
||||
|
||||
**NFR-P3 — ≥ 100 images per second on the embedded preview path.** The
|
||||
per-image work is exactly what `spawn_thumbnail_sweep` does — `decode_jpeg`,
|
||||
`Preview::downscale_to`, `Preview::apply_orientation`, `encode_rgba`,
|
||||
`ThumbStore::put` — arranged in the same shape: chunks of 96, lanes owning
|
||||
disjoint slices, and the single thread that owns the store writing the finished
|
||||
chunk. Tagged `TRACES: NFR-P3` in
|
||||
[`tools/bench/src/thumbnails.rs`](../tools/bench/src/thumbnails.rs).
|
||||
|
||||
### Requirements this can only half-answer, and is not tagged for
|
||||
|
||||
**NFR-P7 — 24 MP export under 2 s, full chain.** The full chain is decode,
|
||||
demosaic, a full-resolution GPU render, a read-back, then resize, sharpen and
|
||||
encode. Only the last three run without an adapter. So the figure here is a
|
||||
**lower bound** on the requirement: exceeding 2 s in the encode alone violates
|
||||
NFR-P7 no matter how fast the render is, and coming in under it proves nothing.
|
||||
The budget is gated on that basis and there is no `TRACES: NFR-P7` anywhere in
|
||||
`tools/bench`.
|
||||
|
||||
**NFR-P8 — idle memory under 500 MB.** The probe is a fresh process holding the
|
||||
catalog and nothing else: no Slint, no wgpu device, no font stack, no decode
|
||||
cache. Its RSS is the catalog layer's *share* of that 500 MB, not the figure the
|
||||
requirement is about. It carries no budget for a reason given below.
|
||||
|
||||
### Requirements out of scope, listed so their absence reads as a decision
|
||||
|
||||
NFR-P2 (grid scroll at 60 fps), P4 (open in develop), P5 (slider to visible),
|
||||
P6 (pan/zoom), P9 (UI-executor blocking), P10 (touch response), P11 (layout
|
||||
transition), P12 (warm shader setup), P13 (next image in culling), P14 (focus
|
||||
peaking), P15 (drawn mask stroke). Every one of them needs a frame-timing probe
|
||||
inside a running Slint application, a GPU adapter, or both. None is faked here.
|
||||
|
||||
The GPU half of the story that *does* exist is
|
||||
[frame-budget.md](frame-budget.md) and its guard test, which asserts FR-DSP-3
|
||||
and skips itself where there is no adapter. `.gitea/workflows/benchmark.yml`
|
||||
runs it as its own job for exactly that reason.
|
||||
|
||||
---
|
||||
|
||||
## The fixture
|
||||
|
||||
Fifty thousand rows over a pool of twelve real image files. Rows are cheap and
|
||||
pixels are not: everything the catalog half touches is rows and is therefore
|
||||
exact at full scale, and everything the pixel half touches is one file at a time
|
||||
and does not care how many rows point at it. The result is ~14 MB on disk
|
||||
instead of ~2 TB, and neither half is flattered by that.
|
||||
|
||||
| | |
|
||||
|---|---|
|
||||
| Rows | 50,000 images, 50,000 default versions, 400 folders, one root |
|
||||
| Capture times | Twelve years from a fixed epoch, so the timeline has ~144 monthly buckets |
|
||||
| Sources | 12 synthesised JPEGs at 1620 × 1080 — the size `dr-decode` records a CR2 carrying in IFD2 |
|
||||
| Seed | 20260829, in [`tools/bench/src/main.rs`](../tools/bench/src/main.rs) |
|
||||
| Location | `$DR_BENCH_DIR`, else the system temporary directory |
|
||||
|
||||
It is reproducible from the seed, and a `stamp.json` beside it records what it
|
||||
was built from — seed, row count, source count, preview size, and `dr-catalog`'s
|
||||
schema version. A mismatch rebuilds rather than silently measuring a different
|
||||
workload than the baseline describes.
|
||||
|
||||
Two honest limits on it:
|
||||
|
||||
- **The page cache is warm.** The fixture was written by this suite or by an
|
||||
earlier run of it, so neither the catalog open nor the thumbnail sweep pays
|
||||
for a cold disk. On the reference desktop's NVMe a genuinely cold read of a
|
||||
14 MB catalog is tens of milliseconds; on spinning rust it is not.
|
||||
- **The sources are synthetic.** A coarse gradient with a fine dither, which is
|
||||
what `frame_budget.rs` synthesises for the same reason — a flat frame lets the
|
||||
memory system serve every sample from one cache line and flatters a box
|
||||
filter, and pure noise defeats the entropy coder in the other direction.
|
||||
|
||||
---
|
||||
|
||||
## Two gates, and how to read a failure
|
||||
|
||||
**Budget.** The requirement's own threshold. It does not move. Failing it means
|
||||
a requirement is violated.
|
||||
|
||||
**Regression.** More than 15% worse than the last recorded figure *on the same
|
||||
machine, against the same fixture*. Failing it means the code got slower while
|
||||
still inside the requirement — which is how most performance rot actually
|
||||
arrives, never over the line, always a little worse, until one day the line is
|
||||
crossed by a change that was not the cause.
|
||||
|
||||
A metric declares whether its budget is `machine_sensitive`. Those are asserted
|
||||
only under `--reference`, and reported everywhere else. §8 names *"the reference
|
||||
desktop"*, not CI, and it is right to: a container with two cores cannot speak
|
||||
to a throughput target written for twenty-four threads, and asserting one there
|
||||
would produce exactly what `core/dr-gpu/tests/frame_budget.rs` refused to
|
||||
produce — *"a red suite that everyone learns to ignore"*. Catalog open is not
|
||||
machine-sensitive: 2 s against an expected figure two orders of magnitude
|
||||
smaller is a threshold any machine can be held to.
|
||||
|
||||
Exit codes: `0` everything passed, `1` a gate failed, `2` the harness itself
|
||||
could not run. Distinguished so a CI log that says "failed" does not leave
|
||||
anyone guessing whether the code got slower or the fixture would not build.
|
||||
|
||||
**Release, always.** The workspace builds its own crates at `opt-level = 0` in
|
||||
dev, and every figure here is dominated by this workspace's own code — the JPEG
|
||||
decode, the box filter, the resample, the sharpen. A debug run measures rustc's
|
||||
shadow. The report says which profile it was built in on its second line.
|
||||
|
||||
---
|
||||
|
||||
## NFR-P8, and the question §4.1 asks
|
||||
|
||||
§4.1 says NFR-P8 *"must state whether it measures RSS inclusive or exclusive of
|
||||
GPU allocations, and whether it holds after SQLite's page cache warms on a 50k
|
||||
catalog."* Both halves have an answer.
|
||||
|
||||
**On the page cache: warm.** The probe runs the count, the timeline and
|
||||
twenty-five windows before it reads its counters, so SQLite's cache holds the
|
||||
b-tree pages a scroll touches. That is the right side to err on — a figure taken
|
||||
before the cache warms would understate a steady-state library.
|
||||
|
||||
**On GPU memory: RSS is exclusive of device-local allocations, and cannot be
|
||||
made otherwise.** A Vulkan allocation in a device-local heap never enters the
|
||||
process's address space, so nothing under `/proc/self/status` can see it. What
|
||||
*does* land in RSS is the host-visible side — staging buffers, mapped upload
|
||||
rings, the read-back `AdjustPass` performs on export — plus the driver's own
|
||||
resident pages.
|
||||
|
||||
So "idle memory < 500 MB" is two questions wearing one number, and a build
|
||||
holding 400 MB of RSS and 3 GB of textures would pass it.
|
||||
|
||||
**Recommendation: NFR-P8 should be restated as two figures** — host RSS
|
||||
exclusive of device-local memory, and a separate VRAM ceiling read from the
|
||||
adapter — because the second is the one that decides whether the application
|
||||
survives beside a browser on an 8 GB card, and nothing in this repository
|
||||
measures it today.
|
||||
|
||||
**And a decision is outstanding.** `catalog_idle_rss_mb` carries no budget
|
||||
because nobody has decided how much of the 500 MB belongs to the catalog layer
|
||||
and how much to everything above it. The suite records the number so that
|
||||
decision can be taken against a measurement rather than an estimate. When it is
|
||||
taken, put the figure in `budget` and the metric becomes a gate.
|
||||
|
||||
---
|
||||
|
||||
## What is not measured, and would be worth adding
|
||||
|
||||
- **The UI's own open.** `ui/dr-ui/src/library.rs` does not call
|
||||
`Catalog::count` or `Catalog::window`; it issues its own SQL against the same
|
||||
tables, with a `VISIBLE` predicate and a burst-folding clause. `dr-bench`
|
||||
cannot see those without depending on `dr-ui`, which would drag Slint into a
|
||||
job that has no display. **Falsifiable end:** when the grid's queries move
|
||||
down into `dr-catalog` — which is where SQL over catalog tables belongs —
|
||||
`catalog_open_ms` becomes the whole of the application's open and this caveat
|
||||
can be deleted rather than argued about.
|
||||
- **The remote sweep.** `spawn_thumbnail_sweep`'s wall clock against a real
|
||||
server is latency, not CPU, and is what FR-NC-3's design is judged by. It
|
||||
needs a server and belongs in a different kind of test.
|
||||
- **A cold disk.** See the fixture's limits above.
|
||||
- **Android.** §4.1 states a second column of targets and §8 asks for
|
||||
"periodically on the named reference Android devices". Nothing here runs on a
|
||||
device. Spike S10 is the piece of work that would start it.
|
||||
- **Everything with a frame in it.** See the out-of-scope list above.
|
||||
|
||||
---
|
||||
|
||||
## Running it
|
||||
|
||||
```sh
|
||||
# Measure and print. Judges nothing.
|
||||
cargo run --release -p dr-bench -- run
|
||||
|
||||
# Measure and gate. What CI runs.
|
||||
cargo run --release -p dr-bench -- check
|
||||
|
||||
# The same, with machine-sensitive budgets asserted too.
|
||||
cargo run --release -p dr-bench -- check --reference
|
||||
|
||||
# Rewrite bench-baseline.json from this run, and commit the diff.
|
||||
cargo run --release -p dr-bench -- record --reference
|
||||
```
|
||||
|
||||
Useful flags: `--fixture <dir>` (or `$DR_BENCH_DIR`) to put the synthetic
|
||||
catalog somewhere specific, `--lanes <n>` to pin the sweep's parallelism, and
|
||||
`--thumbnails <n>` to lengthen or shorten the throughput row.
|
||||
+24
-15
@@ -5,7 +5,7 @@
|
||||
|
||||
Every entry here is extracted from the comment beside the code that implements it, so this file cannot describe a gesture the application does not have. Add one by writing a `GESTURE:` block next to the implementation; there is nowhere else to write it.
|
||||
|
||||
15 gestures, in 2 places.
|
||||
16 gestures, in 2 places.
|
||||
|
||||
## People
|
||||
|
||||
@@ -23,7 +23,7 @@ Grouping over-merges on siblings, on parents and children, and on the same perso
|
||||
- **Touch** — Tick to confirm it, cross to reject it
|
||||
- **Pointer** — Tick to confirm it, cross to reject it
|
||||
|
||||
A face is either the system's guess or the user's judgement, and the two are never conflated. A rejection is remembered, so the face is not suggested for that person again.
|
||||
A face is either the system's guess or the user's judgement, and the two are never conflated. A rejection is remembered, so the face is not suggested for that person again. The gesture note above is the whole label: a tick and a cross are only "confirm" and "reject" to someone who can see the suggestion they sit beside, and `IconButton`'s fallback would announce them as "check" and "cross" — two icon names that say nothing about which person is being ruled on.
|
||||
|
||||
<sub>`ui/dr-ui/ui/identity.slint:150`</sub>
|
||||
|
||||
@@ -34,7 +34,7 @@ A face is either the system's guess or the user's judgement, and the two are nev
|
||||
|
||||
This is the point of having identified anybody. Without it the screen is a filing cabinet with no drawer handles.
|
||||
|
||||
<sub>`ui/dr-ui/ui/identity.slint:545`</sub>
|
||||
<sub>`ui/dr-ui/ui/identity.slint:552`</sub>
|
||||
|
||||
### Change how faces are grouped
|
||||
|
||||
@@ -43,7 +43,7 @@ This is the point of having identified anybody. Without it the screen is a filin
|
||||
|
||||
The right match confidence is a property of your library, not of the model. "What would this do?" answers for this library without writing anything; names, confirmations and the groups you have set aside are kept whatever the dials say.
|
||||
|
||||
<sub>`ui/dr-ui/ui/identity.slint:582`</sub>
|
||||
<sub>`ui/dr-ui/ui/identity.slint:589`</sub>
|
||||
|
||||
## Library grid
|
||||
|
||||
@@ -54,7 +54,7 @@ The right match confidence is a property of your library, not of the model. "Wha
|
||||
|
||||
Touch has no ctrl, so without a mode there is no way to select a second photograph — the first tap would open it. The hold is the fast way in and the button is the one that can be found.
|
||||
|
||||
<sub>`ui/dr-ui/ui/library.slint:1182`</sub>
|
||||
<sub>`ui/dr-ui/ui/library.slint:1184`</sub>
|
||||
|
||||
### Add or remove one photograph
|
||||
|
||||
@@ -63,7 +63,7 @@ Touch has no ctrl, so without a mode there is no way to select a second photogra
|
||||
|
||||
While selecting, a tap never opens. That is the whole point of the mode: one meaning per gesture at a time. Press Done to get tap-to-open back.
|
||||
|
||||
<sub>`ui/dr-ui/ui/library.slint:1191`</sub>
|
||||
<sub>`ui/dr-ui/ui/library.slint:1193`</sub>
|
||||
|
||||
### Leave selecting
|
||||
|
||||
@@ -71,7 +71,16 @@ While selecting, a tap never opens. That is the whole point of the mode: one mea
|
||||
- **Pointer** — Press Done in the header
|
||||
- **Keyboard** — Escape
|
||||
|
||||
<sub>`ui/dr-ui/ui/library.slint:1199`</sub>
|
||||
<sub>`ui/dr-ui/ui/library.slint:1201`</sub>
|
||||
|
||||
### Pick a photograph up to drag it
|
||||
|
||||
- **Touch** — Press and hold it until a ring opens around it, then drag
|
||||
- **Pointer** — Drag it
|
||||
|
||||
A finger on a photograph might be starting a scroll, and for the first half-second the grid assumes it is. Holding says otherwise, and the ring is the grid saying it heard — from there the drag cannot be lost to a scroll. A mouse never waits: the cursor is precise enough that a sideways drag is unambiguous from the first pixel.
|
||||
|
||||
<sub>`ui/dr-ui/ui/library.slint:1231`</sub>
|
||||
|
||||
### Select a range
|
||||
|
||||
@@ -80,7 +89,7 @@ While selecting, a tap never opens. That is the whole point of the mode: one mea
|
||||
|
||||
This replaced a double tap, which had no visible state and could take forty photographs by accident. The run is resolved by the catalog rather than by what is on screen, so the grid can scroll between the two taps — the ranges that hurt on a tablet are longer than a screenful, which is exactly where a finger sweep runs out.
|
||||
|
||||
<sub>`ui/dr-ui/ui/library.slint:1260`</sub>
|
||||
<sub>`ui/dr-ui/ui/library.slint:1296`</sub>
|
||||
|
||||
### Find photographs with two people in them
|
||||
|
||||
@@ -89,7 +98,7 @@ This replaced a double tap, which had no visible state and could take forty phot
|
||||
|
||||
"Any of them" is a union and "all of them" is an intersection. The tray is where both terms and the choice between them live, because a filter belongs on the filter bar.
|
||||
|
||||
<sub>`ui/dr-ui/ui/library.slint:1928`</sub>
|
||||
<sub>`ui/dr-ui/ui/library.slint:1978`</sub>
|
||||
|
||||
### Resize the thumbnails
|
||||
|
||||
@@ -98,7 +107,7 @@ This replaced a double tap, which had no visible state and could take forty phot
|
||||
|
||||
There is no wheel on a tablet, so without the pinch the cell size could only be changed by a control a finger cannot reach.
|
||||
|
||||
<sub>`ui/dr-ui/ui/library.slint:2554`</sub>
|
||||
<sub>`ui/dr-ui/ui/library.slint:2620`</sub>
|
||||
|
||||
### File photographs in a collection
|
||||
|
||||
@@ -107,7 +116,7 @@ There is no wheel on a tablet, so without the pinch the cell size could only be
|
||||
|
||||
The selection is what the drag carries, which is why selecting several is worth the mode: forty photographs file in one gesture.
|
||||
|
||||
<sub>`ui/dr-ui/ui/library.slint:2717`</sub>
|
||||
<sub>`ui/dr-ui/ui/library.slint:2783`</sub>
|
||||
|
||||
### Open a photograph
|
||||
|
||||
@@ -116,7 +125,7 @@ The selection is what the drag carries, which is why selecting several is worth
|
||||
|
||||
A tap opens; a tap that *moved* does not. Travel is what separates a deliberate tap from a hand brushing past, and it is the only thing that does: the two are the same length. An earlier version required the finger to dwell 120 ms instead, and that rejected ordinary taps — a real tap is often quicker than a brush.
|
||||
|
||||
<sub>`ui/dr-ui/ui/library.slint:2972`</sub>
|
||||
<sub>`ui/dr-ui/ui/library.slint:3051`</sub>
|
||||
|
||||
### Rate a photograph without opening it
|
||||
|
||||
@@ -126,7 +135,7 @@ A tap opens; a tap that *moved* does not. Travel is what separates a deliberate
|
||||
|
||||
A star has to take the press without it also reaching the cell, or every rating throws the user into develop.
|
||||
|
||||
<sub>`ui/dr-ui/ui/library.slint:3084`</sub>
|
||||
<sub>`ui/dr-ui/ui/library.slint:3171`</sub>
|
||||
|
||||
### Drop the selection but keep selecting
|
||||
|
||||
@@ -135,7 +144,7 @@ A star has to take the press without it also reaching the cell, or every rating
|
||||
|
||||
Distinct from Done, which leaves the mode entirely. Clearing keeps it, so the next selection can start straight away.
|
||||
|
||||
<sub>`ui/dr-ui/ui/library.slint:3747`</sub>
|
||||
<sub>`ui/dr-ui/ui/library.slint:3879`</sub>
|
||||
|
||||
### Select everything the grid is showing
|
||||
|
||||
@@ -144,4 +153,4 @@ Distinct from Done, which leaves the mode entirely. Clearing keeps it, so the ne
|
||||
|
||||
A scoped grid of two hundred frames is two hundred taps otherwise, and "all of them, except those three" is a far more common shape than the taps it took to say it.
|
||||
|
||||
<sub>`ui/dr-ui/ui/library.slint:3764`</sub>
|
||||
<sub>`ui/dr-ui/ui/library.slint:3896`</sub>
|
||||
|
||||
+96
-46
@@ -177,13 +177,18 @@ The Android app is not a stub — it builds an APK, runs the whole application,
|
||||
models, and has been measured on a tablet ([faces.md §12.1](faces.md),
|
||||
[technical-debt.md TD-1](technical-debt.md)). What is missing is the platform contract around it.
|
||||
|
||||
**FR-PLAT-AND-1 is tagged and should not be relied on.** The requirement demands that library access
|
||||
be obtained *exclusively* through the Storage Access Framework. There is no SAF code: no
|
||||
`ACTION_OPEN_DOCUMENT_TREE`, no `takePersistableUriPermission`, no `DocumentsContract`. The two tags
|
||||
rest on a `SourceRef::Document` variant that nothing constructs and a volumes helper, which is the
|
||||
"plumbing a future feature would use" case [CONTRIBUTING.md](../CONTRIBUTING.md) and
|
||||
[code-health.md CH-4](code-health.md) both warn about. Android reaches a library through a Nextcloud
|
||||
account or a folder, over paths, like the desktop.
|
||||
**FR-PLAT-AND-1 is untagged, and what it was tagged for was intent rather than code.**
|
||||
The requirement demands that library access be obtained *exclusively* through the Storage Access
|
||||
Framework. There is no SAF code: no `ACTION_OPEN_DOCUMENT_TREE`, no `takePersistableUriPermission`,
|
||||
no `DocumentsContract`. Its two tags rested on a `SourceRef::Document` variant constructed only
|
||||
inside `#[cfg(test)]` — `LocalStorage::open` refuses it, and the test that proves so is named
|
||||
`a_reference_of_the_wrong_kind_is_refused_rather_than_guessed_at` — and on
|
||||
`dr_plat::imports_supported`, which *returns false on Android* and whose own documentation says it
|
||||
"stops being false when a SAF implementation lands". The second tag documented the absence of the
|
||||
thing it was counted as evidence for. Both have been removed; this is the "plumbing a future feature
|
||||
would use" case [CONTRIBUTING.md](../CONTRIBUTING.md) and [code-health.md CH-4](code-health.md) both
|
||||
warn about. Android reaches a library through a Nextcloud account or a folder, over paths, like the
|
||||
desktop.
|
||||
|
||||
That has a consequence for the rest of the cluster: **FR-PLAT-AND-2** — detecting the loss of a
|
||||
granted tree permission and marking images offline rather than deleting rows — cannot be built until
|
||||
@@ -220,10 +225,20 @@ the host is invisible to a sandboxed process as well. The fix is an `ashpd` dire
|
||||
`LocalStorage::grant`, not a change to the manifest. No Flatpak has been built here —
|
||||
`flatpak-builder` is not installed — so the permission set is reasoned, not observed.
|
||||
|
||||
**NFR-COMPAT-2 — distribution channels.** Unstated, and this is the requirement that makes the
|
||||
others binding: §4.8 observes that the decision to publish on Play is what turns SAF from a
|
||||
preference into a constraint. Spike S11, the Play permissions dry-run that would settle it, has not
|
||||
run. Related, NFR-COMPAT-1's baseline is real but scattered — API 28/36 live in the Android
|
||||
**NFR-COMPAT-2 — distribution channels. Stated, which is all this requirement asks.** The paragraph
|
||||
above cites [distribution.md](distribution.md) and it is the same document that answers this: §1
|
||||
names five channels and their state — Arch source package and Flatpak in tree, AppImage a v1 channel
|
||||
whose recipe is not written, F-Droid a v1 channel not yet submitted, and Play explicitly **not** v1.
|
||||
The requirement is to *state* the channels, and they are stated, including the deferral.
|
||||
|
||||
What remains is the coupling the requirement points at rather than the statement it demands. §4.8
|
||||
observes that publishing on Play is what turns SAF from a preference into a constraint, and
|
||||
distribution.md §6 argues the coupling runs the other way for this project — F-Droid asks nothing
|
||||
that ARCH §6.9 does not already require. Spike S11, the Play permissions dry-run, has not run, and
|
||||
until it does that argument is reasoned rather than confirmed. Two of the five channels also exist
|
||||
as decisions rather than as recipes, and no Flatpak has been built here at all.
|
||||
|
||||
Related, NFR-COMPAT-1's baseline is real but scattered — API 28/36 live in the Android
|
||||
Dockerfile and are checked in CI against the built ELF, which is good — while the items the
|
||||
requirement singles out are missing: whether `shaderFloat16` and 16-bit storage are required (the
|
||||
one it flags as jeopardising R1), minimum RAM, minimum desktop Mesa, and a named reference device
|
||||
@@ -255,9 +270,22 @@ on one control — the parameter slider in `adjust.slint` — and nothing is set
|
||||
all. Everything else in eighteen Slint files is unnamed to AT-SPI and TalkBack. The requirement's own
|
||||
caveat, that Slint's Android accessibility needs verifying, is spike S13, which has not run.
|
||||
|
||||
**NFR-A11Y-3 — Colour-independent status.** No compliance work found. This is cheap to satisfy while
|
||||
a control is being written and expensive to retrofit across forty of them, which is an argument for
|
||||
doing it as part of the NFR-A11Y-2 pass rather than after it.
|
||||
**NFR-A11Y-3 — Colour-independent status.** Built where a control exists, and now tagged: the
|
||||
clipping readout pairs a marker that appears or disappears with a figure in words, the rating strip
|
||||
is a solid star against an outline in an achromatic palette, the pick/reject mark is a tick against
|
||||
a cross, and the focus-peaking colour chips say "Red" and "Cyan" rather than showing swatches. Each
|
||||
of those already carried the reasoning in a comment naming this requirement and simply had no
|
||||
`TRACES` line.
|
||||
|
||||
Two caveats, because the tag now says more than the evidence does. **Only the clipping clause has a
|
||||
test** — `a_clipping_figure_distinguishes_none_from_nearly_none`, which pins `<0.1%` apart from `0%`
|
||||
so the figure cannot contradict the lit marker beside it. The three Slint components are
|
||||
inspected-and-argued, not asserted, and nothing would fail if a future edit made a star differ only
|
||||
in tint. **And the requirement's first named example has no interface at all**: catalog colour
|
||||
labels are a nullable `label INTEGER` column on the versions table and are set and shown nowhere, so
|
||||
the clause about them is untestable rather than satisfied. That clause closes when the label UI is
|
||||
built, not before, and it should be built with a shape from the outset — which is the same argument
|
||||
as below, for doing this alongside NFR-A11Y-2 rather than after it.
|
||||
|
||||
---
|
||||
|
||||
@@ -277,39 +305,54 @@ well optimised — ETag pruning under FR-NC-4 turns an unchanged 50k library int
|
||||
gap is narrower than it reads. It is the *first* build against a large remote library that pays, and
|
||||
that is the moment a new user meets.
|
||||
|
||||
**FR-CAT-13 — XMP interoperability, tagged and not met.** Read and write standard XMP sidecars. The
|
||||
single tag sits on `keywords.rs`, which stores keywords; no XMP is parsed or written anywhere in the
|
||||
tree, and `dr-export`'s metadata module says so about its own half ("neither is read by `dr-decode`
|
||||
today"). Listed here rather than silently, because a tag makes a gap invisible and this one is
|
||||
load-bearing for interoperating with the editors FR-CAT-14 imports from.
|
||||
**FR-CAT-13 — XMP interoperability, untagged and not met.** Read and write standard XMP sidecars.
|
||||
Its single tag sat on `keywords.rs`, which stores keywords in the catalog and mentions `dc:subject`
|
||||
in a comment about what a keyword's text is *for*; no XMP is parsed or written anywhere in the tree,
|
||||
and `dr-export`'s metadata module says so about its own half ("neither is read by `dr-decode`
|
||||
today"). The tag has been removed — `dr-preset-xmp` is not the counter-example it looks like, being
|
||||
a reader of Lightroom *presets* under FR-DEV-6, which is a different file and a different purpose.
|
||||
Listed here rather than silently, because a tag makes a gap invisible and this one is load-bearing
|
||||
for interoperating with the editors FR-CAT-14 imports from.
|
||||
|
||||
---
|
||||
|
||||
## 8. The performance targets are unverified, not unmet
|
||||
## 8. The performance targets are half-verified, and the half that is left is the hard one
|
||||
|
||||
Eleven of the fifteen §4.1 targets carry no tag: NFR-P2, -P3, -P4, -P6, -P7, -P8, -P10, -P11, -P12,
|
||||
-P14, -P15. That is the uninteresting part of this section.
|
||||
§8 and §4.1 both require the same thing in the same words: an automated benchmark suite against a
|
||||
synthetic 50k catalog, run per commit, where **"a regression beyond a stated tolerance is a build
|
||||
failure, not a notification."** For most of this project's life it did not exist — no `benches/`, no
|
||||
criterion, no synthetic catalog, and three CI workflows that between them measured nothing.
|
||||
|
||||
The interesting part is that §8 and §4.1 both require the same thing, in the same words, and it does
|
||||
not exist: an automated benchmark suite against a synthetic 50k catalog, run per commit, where **"a
|
||||
regression beyond a stated tolerance is a build failure, not a notification."** There is no
|
||||
`benches/` directory in the workspace, no criterion dependency, and no synthetic catalog. The three
|
||||
CI workflows run `cargo fmt --check`, clippy, `cargo test --workspace`, a release build, an Android
|
||||
cross-build and a layering check. None of them measures anything, so there is no baseline to
|
||||
regress against and no tolerance to exceed.
|
||||
**It exists now, for everything that does not need a frame.** [`tools/bench`](../tools/bench) builds
|
||||
a deterministic 50,000-row catalog over a pool of a dozen real files, measures against it, and fails
|
||||
the build on a violated budget or a drift past tolerance;
|
||||
[`.gitea/workflows/benchmark.yml`](../.gitea/workflows/benchmark.yml) runs it on every push, and
|
||||
[benchmarks.md](benchmarks.md) is the account of what it does and does not cover. **NFR-P1** and
|
||||
**NFR-P3** are now genuinely gated, and R2's "catalog opens in under 2s" clause with them.
|
||||
|
||||
What does exist is narrower and genuinely good: `dr-gpu/examples/frame_budget` is a real instrument,
|
||||
its results are committed in [frame-budget.md](frame-budget.md) with the machine and profile named,
|
||||
and TD-4's before-and-after was measured with it. But it is run by hand — frame-budget.md's own
|
||||
instruction is "rerun and diff this file" — and the guard version that does live in CI skips itself
|
||||
where there is no GPU adapter, which the workflow notes is the normal case on a runner, while
|
||||
asserting its CPU half only when `debug_assertions` is off, which a dev-profile `cargo test` is not.
|
||||
In CI it therefore asserts approximately nothing.
|
||||
Three qualifications, all of them stated in the harness itself rather than only here:
|
||||
|
||||
**The claim to take from this is precise.** Nothing here says the performance targets are missed.
|
||||
Several are plausibly met. It says that if one were broken tomorrow, nobody would find out — which
|
||||
is the failure mode §8 was written to prevent, and the reason it belongs in this document rather
|
||||
than in a backlog.
|
||||
- **The numbers have not been recorded yet.** Every `recorded` field in
|
||||
[bench-baseline.json](bench-baseline.json) is `null`, deliberately: a fabricated baseline is worse
|
||||
than none. Until `dr-bench record --reference` is run on the reference desktop and committed, the
|
||||
budget gate works and the regression gate does not.
|
||||
- **NFR-P7 and NFR-P8 are half-measured and are not tagged.** The export row covers the encode half
|
||||
of the chain and no GPU render, so it can fail the requirement and cannot pass it. The memory row
|
||||
covers a process holding the catalog and nothing else — no toolkit, no adapter — so it is the
|
||||
catalog layer's share of the 500 MB rather than the figure NFR-P8 is about. Neither carries a
|
||||
`TRACES:` tag, which is the point.
|
||||
- **NFR-P8 needs a decision, not more code.** How much of its 500 MB belongs below the UI is
|
||||
unstated, and until somebody says, the metric can record but not judge. [benchmarks.md](benchmarks.md)
|
||||
also answers the question §4.1 raises about GPU memory — RSS cannot see device-local allocations
|
||||
at all — and recommends restating the requirement as two figures.
|
||||
|
||||
**What is left is the frame-timing half, and it is the hard one.** NFR-P2, -P4, -P5, -P6, -P9, -P10,
|
||||
-P11, -P12, -P13, -P14 and -P15 all need a probe inside a running Slint application, a GPU adapter,
|
||||
or both. `dr-gpu/examples/frame_budget` is a real instrument for the GPU part and its results are
|
||||
committed in [frame-budget.md](frame-budget.md) with the machine and profile named — but it is run
|
||||
by hand, and the guard version in CI skips itself where there is no adapter, which is the normal
|
||||
case on a runner. So the claim to take from this section is now narrower than it was, and still
|
||||
true: **a scroll that dropped to 30 fps tomorrow would reach a user before it reached CI.**
|
||||
|
||||
---
|
||||
|
||||
@@ -320,12 +363,19 @@ fixed before spike S9", because S9 both validates R1 and calibrates what toleran
|
||||
The threshold was never fixed and S9 has not run, so R1 currently has no acceptance criterion at
|
||||
all — there is nothing a test could assert.
|
||||
|
||||
Worse, the matrix reports R1 as *covered*. Both of its tags are string literals inside the
|
||||
traceability tool's own unit tests (`tools/traceability/src/lib.rs`), which the tool scans along with
|
||||
everything else, because a fixture demonstrating tag extraction is indistinguishable from a tag.
|
||||
NFR-OPS-1 is covered the same way, from a tag on `compute_coverage` — and no rotating, size-capped
|
||||
on-disk log exists; logging goes to stderr and logcat. These are two of the cases
|
||||
[CONTRIBUTING.md](../CONTRIBUTING.md) already warns about, now named.
|
||||
The matrix used to report R1 as *covered*, and what covered it was two string literals: fixtures
|
||||
inside the traceability tool's own unit tests, which the tool scans along with everything else,
|
||||
because a fixture demonstrating tag extraction was indistinguishable from a tag. The extractor now
|
||||
asks where the tag sits — a tag is the first word of a comment, not a string appearing anywhere on a
|
||||
line — and R1 is untagged again, which is the honest reading while it has no acceptance criterion to
|
||||
tag anything against.
|
||||
NFR-OPS-1 was covered by tags that were real rather than fixtures, which is the worse case of the
|
||||
two: one on `compute_coverage` and one on the gesture extractor, both on the traceability tool. A
|
||||
coverage calculation and a documentation generator are not diagnostics under any reading, and the
|
||||
requirement asks for a rotating, size-capped on-disk log in the XDG state directory, credential
|
||||
redaction, and a consented diagnostics bundle. None of that exists — logging goes to stderr and
|
||||
logcat — so both tags have been removed and NFR-OPS-1 is untagged. It is the case
|
||||
[CONTRIBUTING.md](../CONTRIBUTING.md) warns about in its own words: a tag proves a tag exists.
|
||||
|
||||
**R2 — Efficient display of huge RAW libraries.** Its acceptance criterion contains "*(figure
|
||||
TBD)*" — the scroll velocity below which no cell may render as a placeholder — and asks for a stated
|
||||
|
||||
+85
-8
@@ -55,7 +55,7 @@ These are the user's stated requirements, restated as testable criteria.
|
||||
| **R2** | Efficient display of huge RAW libraries | A 50,000-image catalog scrolls at 60fps sustained, with a stated prefetch margin and cache-hit rate sufficient that no cell renders as a placeholder at a scroll velocity of *(figure TBD)* rows/second. Catalog opens in under 2s. |
|
||||
| **R3** | Make a RAW beautiful at 8-bit output | Full non-destructive develop chain at high internal precision, with camera input profiles (FR-DEV-3e) and a colour-managed path to 8/16-bit export. |
|
||||
| **R4** | HW acceleration and parallelism | All per-pixel work runs on GPU compute. CPU work (decode, I/O) is parallelised across cores. The UI executor never blocks on image work (NFR-ARCH-1). |
|
||||
| **R5** | Work on downscaled proxies for display | Display pipeline operates at viewport resolution, not source resolution. Only visible tiles are computed; panning recomputes only newly exposed tiles. |
|
||||
| **R5** | Work on downscaled proxies for display | Display pipeline operates at viewport resolution, not source resolution. The tiling clauses this criterion used to carry have been struck — see below. |
|
||||
| **R6** | Nextcloud integration | Browse, download, and upload images and edit metadata against a Nextcloud instance, offline-capable. |
|
||||
|
||||
**On R1's tolerance.** An earlier draft required output to be *bit-identical* across platforms.
|
||||
@@ -72,6 +72,31 @@ Where genuine bit-identity is required — cache keys, edit-graph hashing (§5.2
|
||||
applies to *integer* operations on CPU-side state, which are deterministic, never to GPU float
|
||||
results.
|
||||
|
||||
**On R5's tiling.** An earlier draft added two clauses to R5's criterion: *"only visible tiles are
|
||||
computed; panning recomputes only newly exposed tiles"*. They have been struck, and the reason is
|
||||
the same one FR-DSP-2 was rewritten for rather than implemented:
|
||||
[frame-budget.md](frame-budget.md) measured it.
|
||||
|
||||
R5's actual demand is met and tested. The display pipeline works at viewport resolution:
|
||||
`Framing::view` shrinks the sampled region while the render target keeps its size, so zooming raises
|
||||
the resolution the pipeline works at rather than magnifying pixels already drawn, and
|
||||
`core/dr-gpu/tests/zoom_resolution.rs` establishes it as a pixel equality rather than an impression
|
||||
of sharpness.
|
||||
|
||||
Tiling is a different claim, and it was written in as though it were the mechanism by which the
|
||||
first one is achieved. It is not. Recomputing the *entire* 4K viewport costs 4.5 ms of a 16 ms
|
||||
budget, so a perfect tile cache saves at most that, in exchange for a cache keyed by
|
||||
`(VersionId, tile, zoom, graph_hash_prefix)` that has to stay correct across every parameter change
|
||||
in the graph — a large correctness surface bought with a small number. And for the one stage that
|
||||
does miss the budget, tiling makes it worse: that stage is a convolution, and a tiled convolution
|
||||
reads a halo per tile, so at the 52 px radius measured at 4K a 256 px tile would read (256+104)²
|
||||
taps instead of 256², very nearly twice the work.
|
||||
|
||||
The intent behind the struck clauses — that the display path must not do work proportional to the
|
||||
source image — survives in the clause that remains, which is the honest statement of it. Tiling
|
||||
stays where FR-DSP-2 puts it: a scheduling concern for export and thumbnailing, both of which
|
||||
already run off the frame path.
|
||||
|
||||
---
|
||||
|
||||
## 3. Functional requirements
|
||||
@@ -181,10 +206,34 @@ destructive action, since that figure is what tells the user whether they meant
|
||||
CR3), Nikon (NEF), Sony (ARW), Fujifilm (RAF, including X-Trans), Panasonic (RW2), Olympus (ORF),
|
||||
Adobe DNG. Additional formats are a coverage goal, not a launch blocker.
|
||||
|
||||
**FR-RAW-2 — Decoder abstraction.** RAW decoding sits behind a trait taking a `SourceRef`
|
||||
(FR-CAT-1a), not a filesystem path, so the same decoder works over a local file, an Android SAF
|
||||
document, or a byte range fetched from Nextcloud. A second implementation may be added for broader
|
||||
camera coverage without changing callers (D2).
|
||||
**FR-RAW-2 — Decoder abstraction.** RAW decoding takes **bytes**, never a filesystem path and never
|
||||
a reference it would have to resolve. Resolving a `SourceRef` (FR-CAT-1a) to bytes is
|
||||
`Storage::open`'s job and happens at the caller, so the same decoder works over a local file, an
|
||||
Android SAF document, or a byte range fetched from Nextcloud. A second implementation may be added
|
||||
for broader camera coverage without changing callers (D2).
|
||||
|
||||
*On the change of mechanism.* This clause used to require "a trait taking a `SourceRef`". The
|
||||
purpose — that no decoder API takes a path, so nothing in the decode path assumes a filesystem —
|
||||
is met and is not in question: `dr_decode::decode` takes `&[u8]`, and there is no path-based entry
|
||||
point in the crate. The mechanism was wrong, and stating it that way would have made the design
|
||||
worse.
|
||||
|
||||
A `SourceRef` is opaque by construction; the only thing that turns one into readable bytes is
|
||||
`Storage`, in `platform/dr-plat`. A decoder taking a `SourceRef` would therefore have to take a
|
||||
`Storage` alongside it, which moves retry, permission loss and remote fetching inside the decoder
|
||||
and leaves it constructible only where a `Storage` exists. Bytes in, image out, is both narrower and
|
||||
more portable: the decoder has no idea where its input came from, which is the property this
|
||||
requirement is actually asking for.
|
||||
|
||||
It also serves the Nextcloud case better rather than worse, which is the one a byte-oriented API
|
||||
looks like it would lose. `dr_decode::HEADER_BYTES` declares how much of a file the decoder needs to
|
||||
read metadata, and `import.rs` fetches exactly that range through `Storage::read_range` before
|
||||
calling `dr_decode::metadata`. The decoder states its requirement and the storage layer satisfies
|
||||
it; a decoder holding its own `SourceRef` would have had to implement the range policy itself.
|
||||
|
||||
What is genuinely not built is the trait. There is one decoder, reached through free functions, so
|
||||
"without changing callers" is a claim nothing yet tests. The clause stands as written and is
|
||||
outstanding work, not a satisfied one.
|
||||
|
||||
**FR-RAW-3 — Sensor data handling.** Correctly apply per-camera black/white levels, CFA pattern
|
||||
identification, and camera-native colour matrices. Demosaic quality shall be selectable, with at
|
||||
@@ -493,8 +542,24 @@ region.
|
||||
|
||||
### 3.6 Export
|
||||
|
||||
**FR-EXP-1 — Formats.** Export to JPEG, PNG, TIFF (8 and 16-bit), and AVIF or JPEG XL. Quality,
|
||||
chroma subsampling, and bit depth are configurable.
|
||||
**FR-EXP-1 — Formats.** Export to JPEG, PNG, and TIFF (8 and 16-bit). Quality, chroma subsampling,
|
||||
and bit depth are configurable.
|
||||
|
||||
**AVIF and JPEG XL are post-v1** and are not part of this requirement's acceptance. Either may still
|
||||
be listed in the settings page before its encoder exists, on one condition: choosing it shall fail
|
||||
with a typed error naming the format, never with a file. The test
|
||||
`every_offered_format_either_encodes_or_explains_itself` walks every format the page offers and
|
||||
enforces exactly that, so a format cannot be added to the picker and quietly reach an encoder that
|
||||
does not handle it.
|
||||
|
||||
*On splitting this requirement.* It read as one undifferentiated list — "JPEG, PNG, TIFF (8 and
|
||||
16-bit), and AVIF or JPEG XL" — which left it neither met nor unmet. Three formats, both TIFF
|
||||
depths, and the configurability clause are built, encode, embed their profile, and are tested; the
|
||||
fourth item is a deliberate deferral, and the encoders for it are the two with the least settled
|
||||
library support. Fused into one sentence, the only choices were a tag asserting something untrue or
|
||||
no tag at all, and the second is the worse of the two: it would have removed the register's record
|
||||
of four-fifths of a requirement that is finished. The deferral is now stated where it can be read as
|
||||
a decision rather than inferred from an error variant.
|
||||
|
||||
**FR-EXP-2 — Colour space.** Export in a selectable output colour space (sRGB, Display P3,
|
||||
Adobe RGB, ProPhoto), with the correct ICC profile embedded.
|
||||
@@ -1305,13 +1370,25 @@ These are targets to design against and measure, on the reference desktop
|
||||
| **NFR-P14** | Focus peaking overlay ready | < 100 ms after preview | < 150 ms |
|
||||
| **NFR-P15** | Drawn mask stroke → visible (ARCH §6.11) | < 16 ms, no cursor lag | < 16 ms |
|
||||
| **NFR-P10** | Touch gesture → visual response | < 16 ms | < 16 ms |
|
||||
| **NFR-P11** | Layout class transition (window resize) | No dropped frames, no state loss | n/a |
|
||||
| **NFR-P11** | Layout class transition (window resize) | No dropped frames. No loss of *photographic* state: the open image and version, the selection, scroll position, the in-progress edit and its undo history, and the current mode. Panel disclosure is explicitly exempt — see below | n/a |
|
||||
| **NFR-P12** | Warm-start shader pipeline setup (cached) | < 100 ms | < 100 ms |
|
||||
|
||||
Every target above requires a stated measurement method, workload, and pass threshold before it is
|
||||
testable. NFR-P8 in particular must state whether it measures RSS inclusive or exclusive of GPU
|
||||
allocations, and whether it holds after SQLite's page cache warms on a 50k catalog.
|
||||
|
||||
**On what NFR-P11 means by state.** It said "no state loss", which the implementation contradicts on
|
||||
purpose, so the requirement has been made specific rather than left to be read as forbidding
|
||||
something it should not. `apply_layout_class` discards the user's panel open/closed choices when the
|
||||
class changes, and the argument for that is sound: a choice made in landscape answers a different
|
||||
question from the one portrait asks, and carrying it across is how a photographer ends up with a
|
||||
232 px sidebar on a screen with no room for it and no memory of having asked for it. Panel
|
||||
disclosure is a *default*, re-derived per class, with the user's disagreement remembered only within
|
||||
the class where it was expressed.
|
||||
|
||||
Everything the photographer produced or navigated to is a different matter, and none of it may be
|
||||
touched by a resize. That is the list in the criterion, and it is the testable half.
|
||||
|
||||
**Performance regressions fail the build.** §9's benchmark suite runs per-commit; a regression
|
||||
beyond a stated tolerance is a build failure, not a notification. Performance work rots otherwise.
|
||||
|
||||
|
||||
@@ -297,6 +297,152 @@ millisecond for the `point` and `all` chains, and the codegen tests still pass b
|
||||
|
||||
---
|
||||
|
||||
## TD-6 — The quietest ink does not reach WCAG AA, and the rule does not reach 3:1
|
||||
|
||||
**Breaks:** [requirements.md](requirements.md) NFR-A11Y-2 — "non-canvas UI meets WCAG AA contrast".
|
||||
|
||||
### What it does
|
||||
|
||||
`style.yaml` sets three inks and four surfaces. Measured as WCAG 2 contrast ratios (sRGB relative
|
||||
luminance, the standard formula), against the surfaces each ink is actually drawn on:
|
||||
|
||||
| ink | on `ground` | on `surface` | on `surface-raised` | on `hover` | on `selected` |
|
||||
|---|---|---|---|---|---|
|
||||
| `ink` #EDEEF0 | 16.02 | 14.69 | 13.03 | 11.39 | 9.68 |
|
||||
| `ink-dim` #9EA1A6 | 7.18 | 6.58 | 5.84 | 5.10 | **4.34** |
|
||||
| `ink-faint` #71747A | **3.97** | **3.64** | **3.23** | **2.82** | **2.40** |
|
||||
| `warn-ink` #C9A05A | 7.67 | 7.03 | 6.24 | 5.45 | 4.64 |
|
||||
| `rule` #323438 | **1.49** | **1.37** | **1.21** | **1.06** | **1.11** |
|
||||
|
||||
Every text size in the application is 11px, 13px, 17px or 24px, and WCAG's "large text" relief
|
||||
begins at 18.66px bold or 24px regular — so all four of those thresholds are the normal-text one,
|
||||
**4.5:1**, except the masthead. Bold entries fail it.
|
||||
|
||||
The inverted cases pass and are worth stating so nobody re-measures them: `ground` on `active`
|
||||
(#FFFFFF) is 18.60, on `active-dim` 11.20, on `active-pressed` 5.95, on `selected-ring` 13.02. The
|
||||
near-white fills that `Button.primary`, `FilterChip.active` and the held tool-rail entry use are
|
||||
the *best*-contrasting text in the interface, not the worst.
|
||||
|
||||
So the failures are exactly two, and neither is where one would guess:
|
||||
|
||||
- **`ink-faint` reaches 4.5:1 nowhere at all.** It is the ink for `Caption`, `PanelHeading`,
|
||||
`Disclosure`, `Value`'s placeholder state and `FilterChip`'s count — every hint, every section
|
||||
name, every "3 photographs" under a title.
|
||||
- **`rule` reaches 3:1 nowhere.** WCAG 1.4.11 asks 3:1 of the boundary of a control the user must
|
||||
perceive, and `rule` is the border of every `Button`, `Field`, `Panel`, `ChoiceChip` and
|
||||
`IconButton`. An unfilled secondary button is a 1.4:1 outline on a 1.2:1 background.
|
||||
|
||||
`ink-dim` on `selected` at 4.34 is a third case, marginal enough that a two-point lift fixes it.
|
||||
|
||||
### Why
|
||||
|
||||
Not an oversight — the direct consequence of the palette's own argument, which `style.yaml`'s
|
||||
preamble makes at length and correctly. The chrome is deliberately quiet because a bright surround
|
||||
biases how a photograph is judged, and hue is banned outright because an accent beside the image
|
||||
shifts the perception of nearby colours. What is left to signal with is luminance, and the palette
|
||||
spends its luminance range on the *photograph*, keeping the chrome inside a narrow band above the
|
||||
ground.
|
||||
|
||||
A narrow band is precisely what a contrast ratio measures. `ink-faint` exists to be skipped by the
|
||||
reader who did not stop to look; that is a real design intent, and "text you are meant to skip"
|
||||
and "text everyone can read" are in genuine tension rather than one being a mistake.
|
||||
|
||||
### What it costs
|
||||
|
||||
The photographer who cannot read a caption cannot read *any* caption, on any screen — this is one
|
||||
token, so it fails everywhere at once. The hints under the settings switches say what a setting
|
||||
costs, the section names say what a panel is, and the counts say how big a filter is. None of it is
|
||||
decorative.
|
||||
|
||||
### Paying it off
|
||||
|
||||
Two token changes, and the second is the awkward one.
|
||||
|
||||
`ink-faint` needs roughly #8A8D93 to clear 4.5:1 against `surface-raised`, the darkest surface it
|
||||
is drawn on that matters — which puts it about where `ink-dim` sits today and collapses the
|
||||
three-ink scale to two. So the real fix is to re-derive all three inks against the surfaces rather
|
||||
than to nudge one: the scale wants to start higher and keep its steps, not compress.
|
||||
|
||||
`rule` needs about #4A4D52 for 3:1 against `surface`. That is a visibly stronger line, and the
|
||||
preamble's "instrument rather than absence" reasoning applies to it as much as to the greys — this
|
||||
is a look change, not a number change, and it should be looked at rather than computed.
|
||||
|
||||
Both are decisions about how the application appears next to a photograph, which is the one thing
|
||||
this palette was designed around. They want a screenshot and an opinion, not a patch.
|
||||
|
||||
**Done when:** every `Theme` ink reaches 4.5:1 against every surface it is drawn on, `rule` reaches
|
||||
3:1 against `surface` and `surface-raised`, and a test recomputes those ratios from `style.yaml` so
|
||||
the next palette edit cannot quietly undo it. The table above is the baseline to compare against.
|
||||
|
||||
### Not in scope
|
||||
|
||||
The histogram's `plot-*` inks (2.36 for `plot-luma` on `ground`) are drawn *on* the canvas, and
|
||||
NFR-A11Y-2 scopes contrast to non-canvas UI. NFR-A11Y-3 covers what those need instead, and is
|
||||
already met — the readouts name the channel in words.
|
||||
|
||||
---
|
||||
|
||||
## TD-7 — Platform font scaling is not honoured
|
||||
|
||||
**Breaks:** [requirements.md](requirements.md) NFR-A11Y-2 — "platform font scaling is honoured
|
||||
without clipping".
|
||||
|
||||
### What it does
|
||||
|
||||
Nothing at all, which is the entry. Every type size is a constant in `style.yaml` — 11, 13, 17, 24
|
||||
— read as `Theme.text-sm` and friends at 65 call sites, and there is no multiplier anywhere between
|
||||
the platform's font-size preference and those numbers. `scale_factor()` is read in `display_ui.rs`
|
||||
and in `lib.rs`, but only to size the canvas in physical pixels for the render; it is display DPI,
|
||||
which Slint already applies to logical lengths, and it is not the user's text-size setting. A
|
||||
photographer who sets 130% text on GNOME or Android gets an application that ignores it.
|
||||
|
||||
### Why
|
||||
|
||||
Because honouring it is not a multiplier, and pretending it is would be worse than not doing it.
|
||||
|
||||
The layout is built on constants that are not derived from the type size: `control-height` 28,
|
||||
`touch-target` 44, `row-height` 26, `rail-entry-height` 54, `panel-width` 360, and a dozen fixed
|
||||
heights written at their call sites — `ParamSlider`'s 46px, the folder picker's 220px box, the
|
||||
readout column's 30px. Scaling the type alone clips against every one of them, silently, because
|
||||
Slint elides rather than errors. `SwatchSlider` is the sharpest case: 12 hue bands × 3 channels in
|
||||
a 360px column, sized so that a track and a swatch and a three-character readout fit on one line.
|
||||
|
||||
And the failure is invisible to the person shipping it. The requirement's own phrase is "without
|
||||
clipping", and clipping is exactly what a screenshot at 100% cannot show — the memory note on
|
||||
verifying Slint changes exists because these files have a history of compiling, rendering and being
|
||||
wrong.
|
||||
|
||||
### What it costs
|
||||
|
||||
The user for whom this matters most is not the screen-reader user the rest of this branch serves —
|
||||
it is the one with usable but poor sight, who reads the interface and needs it larger. The
|
||||
application is unusable to them at any setting, and there is no partial credit: text scaling is a
|
||||
system-wide preference, so an app that ignores it is the one thing on the desktop that did.
|
||||
|
||||
### Paying it off
|
||||
|
||||
In the order the pieces depend on each other:
|
||||
|
||||
1. **A scale token.** `Theme.text-sm` and the rest become `base × Theme.type-scale`, with the
|
||||
scale an `in-out` property Rust writes at startup from the platform. `build.rs` already emits
|
||||
`in-out` tokens under `live-style`, so the codegen half of this exists and is proven — that
|
||||
feature is the mechanism, one line from being general.
|
||||
2. **A source for the number.** GNOME publishes `text-scaling-factor` over the settings portal;
|
||||
Android has `Configuration.fontScale` through JNI, beside the calls `lib.rs` already makes for
|
||||
`ACTION_VIEW`. Both want a default of 1.0 and a sane clamp — 0.8 to 2.0 — because a user who has
|
||||
set 300% for a phone launcher has not asked for a 72px slider readout.
|
||||
3. **The constants that are not type.** Every fixed height a *label* sits inside has to follow the
|
||||
scale; every touch target must not shrink and need not grow. That is the work, and it is where
|
||||
the 46px and 220px literals get read one at a time.
|
||||
4. **Evidence.** Screenshots at 1.0, 1.3 and 2.0 of the develop column, the settings page and the
|
||||
colour mixer — the three densest layouts — because "without clipping" is a claim about the
|
||||
worst case and nothing else will show it.
|
||||
|
||||
**Done when:** the develop column, the settings page and the colour mixer render at a 2.0 scale
|
||||
with no elided label and no touch target under 44 logical pixels.
|
||||
|
||||
---
|
||||
|
||||
## Related, and deliberately not here
|
||||
|
||||
The window-move rule, the grid's ordering index and the whole-library readout cache were *fixed*
|
||||
|
||||
+74
-74
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user