# Contributing to DarkRoom There is a lot of documentation here — twenty-odd documents and 192 numbered requirements — and almost all of it is written for someone who has already decided to work on this. This file is the other thing: how to get a first change landed without reading any of it. ## The shortest useful contribution **A develop operation is one file.** Not one file plus a registration, plus a shader edit, plus a control in the UI — one file: ``` core/dr-pipeline/ops/split_toning.yaml ``` `build.rs` finds it with `read_dir`, compiles it into Rust implementing `Operation`, and from there it is indistinguishable from a hand-written node. It arrives with controls built from its declared parameter kinds, a place in the chain from `order:`, a place in the panel from `attributes:`, sidecar persistence, and its own tests — which are declared in the same file and run under `cargo test`. Read [`core/dr-pipeline/ops/README.md`](core/dr-pipeline/ops/README.md) and copy [`exposure.yaml`](core/dr-pipeline/ops/exposure.yaml). Split toning, colour zones, selective colour and channel-mixer variants are all pure point operations, which means all of them are declarations rather than code. If you want to understand one thing about the architecture before starting, make it this: **the core describes its capabilities and the interface composes them.** No code in `ui/` names an operation, and a test enforces that (`ui/dr-ui/tests/ui_names_no_operation.rs`). It is why your node needs no UI change. ## Getting it to build **Git LFS is required.** Model weights are stored in LFS, and a clone made without it leaves a ~130-byte text pointer where an 11 MB model should be: ```bash git lfs install && git lfs pull ``` Forget this and `dr-segment`'s build script stops with an instruction rather than embedding the pointer and failing at inference time — but it is easier to run the two commands now. **The toolchain pins itself.** `rust-toolchain.toml` selects 1.92.0 and rustup fetches it on first use. Do not override it; `cargo fmt` and `clippy` are both version-sensitive and CI runs exactly this version. **System packages.** Slint and winit need these at build time. On Debian or Ubuntu: ```bash sudo apt-get install pkg-config libfontconfig1-dev libxkbcommon-dev ``` **Then:** ```bash cargo run -p darkroom-desktop ``` The first build resolves some 850 crates and takes a while — on a laptop, long enough to look like a hang. It is not one. Android is a containerised toolchain and is not needed for most work; see [`docker/android/README.md`](docker/android/README.md) if you get there. ## What CI will check All four of these run on every push, so run them before you send anything: ```bash cargo fmt --all -- --check cargo clippy --workspace --all-targets -- -D warnings cargo test --workspace cargo build --workspace --release ``` GPU tests skip themselves where there is no adapter rather than failing — a test that cannot run is not evidence either way — so a green run on a machine without a GPU is expected, and does not mean the GPU paths were exercised. There is a fifth check, and it is not in that list because you are unlikely to break it by accident: ```bash cargo run --release -p dr-bench -- check ``` That is the benchmark suite (`docs/dev/requirements.md` §8), which builds a synthetic 50,000-image catalog and fails the build if a performance target is missed or a measurement has drifted past its tolerance. It runs on every push in its own workflow. [`docs/dev/benchmarks.md`](docs/dev/benchmarks.md) says what it measures, what it deliberately does not, and how to read a failure. If you have touched the catalog, the decoder, the thumbnail store or the exporter, run it before you send. ## Requirements and traceability [`requirements.md`](docs/dev/requirements.md) is the register of record. [`traceability.md`](docs/dev/traceability.md) is generated from `TRACES:` tags in the source and must never be hand-edited: ```rust // TRACES: FR-DEV-3a | FR-DEV-3c ``` Tags are read from `.rs`, `.slint`, `.wgsl` and `.yaml` — the last so a declared operation can record the requirement it satisfies, since the Rust it generates lands in `OUT_DIR` and is not scanned. A pre-commit hook regenerates the matrix and stages it whenever you touch something that can carry a tag, so you should not have to think about it. If you do need to run it by hand: ```bash cargo run -p traceability -- report ``` Note that it tracks line numbers, so a change that only moves code still moves the matrix. Never regenerate it with a stale prebuilt binary. **One convention that the tooling cannot enforce.** A tag proves that a tag exists, not that the code under it does the thing — `docs/dev/code-health.md` CH-4 has the details, and two requirements currently read as covered on the strength of plumbing a future feature would use. So: **close a requirement with a test that would fail if the behaviour were removed.** Coverage that moves slowly and means something beats coverage that moves quickly. **Keys and gestures are held the same way.** A key handler in Slint compares one canonical chord, `Keys.chord(event) == "Ctrl+Z"`, under a `// KEYMAP:` comment naming its section of the gesture book, and every key it binds must be named by a `GESTURE:` block beside it. `cargo run -p traceability -- gestures` regenerates [`docs/gestures.md`](docs/gestures.md) and the in-app help sheet from those blocks; `-- gestures-check` fails when a handler binds a key no tag names, or a tag names a key no handler binds. A `manual:` field in a block links the gesture to a section of the manual, and a heading that is not there fails the scan. **The manual is checked too.** `docs/manual/index.html` is rendered from `docs/manual/README.md` by `-- manual` and `-- manual-check` fails when they differ; `tools/manual/record.sh --check` fails when the manual shows a picture no scene in `tools/manual/scenes.py` makes. If you change what a pictured screen looks like, [`tools/manual`](tools/manual/README.md) says how to record it again. The pre-commit hook regenerates the matrix, the gesture book and the page; CI runs all three checks. ## Three invariants the build defends Worth knowing before you trip one, because each failure names a requirement rather than a line: - **No operation may be named in `ui/`** (FR-DEV-3a). Special-casing one operation in the panel to fix a layout problem is how a generated interface stops being generated. If a node needs presentation the panel cannot give it, the answer is a `presentation:` hint in the declaration and a `WidgetKind`, not a branch in `develop.rs`. - **The operation schema rejects ambiguity at build time**: a duplicate `order:`, a filename disagreeing with its `id:`, a default outside its own range, an expression naming something that is not a parameter. Each error names the key you got wrong and exits rather than panicking. - **No verdict is written without a user action** (FR-CULL-13). A rating, flag, colour label or trash membership is the photographer's to set, never a signal's. `tools/traceability/src/verdicts.rs` finds every write of one in the shipped code — the catalog setters, SQL that assigns those columns, the sidecar's judgement amendment — and holds each to a reviewed list with its reason: inside a Slint `on_*` callback, writing for callers that are checked in turn, or carrying a verdict made elsewhere, such as a sidecar pull or the sync merge. A new write fails `cargo test` (the `traceability` crate's tests, part of the workspace run) until it is listed, and so does a listed one that has gone; `cargo run -p traceability -- verdicts` prints the list. ## Commit messages Imperative subject describing the change from the reader's side — "Offer the merge when two people turn out to share a name", not "fix: merge dialog". No conventional-commits prefixes. The body is where the reasoning goes, and it is expected to be substantial when the change is. This codebase records *why* far more than most, in commits and in comments alike, and that is the single habit most worth adopting: the constraint you worked around is invisible to whoever reads the diff next. One commit per change. If you fixed two things, that is two commits. ## Where to read next, in order | Document | Read it when | |---|---| | [`core/dr-pipeline/ops/README.md`](core/dr-pipeline/ops/README.md) | Adding or changing a develop operation — start here regardless | | [`docs/dev/architecture.md`](docs/dev/architecture.md) | Anything touching the render path, catalog or sync | | [`docs/dev/code-health.md`](docs/dev/code-health.md) | Deciding what to work on; grades each seam by what it costs | | [`docs/dev/benchmarks.md`](docs/dev/benchmarks.md) | A change that could plausibly cost time or memory | | [`docs/dev/technical-debt.md`](docs/dev/technical-debt.md) | Something looks wrong — check it was not chosen | | [`docs/dev/distribution.md`](docs/dev/distribution.md) | Packaging a build, or adding a permission to one | | [`docs/dev/requirements.md`](docs/dev/requirements.md) | Reference, not reading | `technical-debt.md` is the one to check before "fixing" anything surprising. It records compromises that were deliberate, each with the reasoning and a falsifiable condition for when it stops being one — the point being that you can tell a constraint from an accident without asking. ## Licence GPL-3.0-or-later. By contributing you agree your work is licensed the same way.