Files
DarkRoom/CONTRIBUTING.md
T
dtourolleandClaude Opus 5 95847e3a31 Say which channels v1 ships through, and that the Flatpak cannot reach a library
NFR-COMPAT-2 asks for the v1 channels to be stated, and says why in its own
second sentence: the channel decision and the storage design are coupled. There
was nowhere that statement lived. packaging/ held a PKGBUILD and a desktop
entry, which is a recipe rather than a decision, and the coupling the
requirement points at was therefore invisible.

docs/distribution.md states five channels and, more usefully, which two of them
exist only as promises. It also records what every channel has to get right
independently of format — the one identifier that appears in four places, the
metainfo, Vulkan being a requirement rather than a preference while NFR-R8 is
open, a secrets daemon being optional rather than required, and the LFS pointer
check that stops a package shipping 130 bytes where an 11 MB model should be.

§4 is the part worth reading. Preparing a Flatpak is what surfaced that
FR-PLAT-LIN-3 is not satisfied and cannot be satisfied by packaging alone: a
folder library is chosen by typing an absolute path into an EndpointOnly field
that checks it with std::fs, and nothing in the tree calls the FileChooser
portal. Inside a sandbox that path does not exist, so the launch screen refuses
it. Import fails one step earlier, because a sandboxed process reads its own
mount namespace and a card mounted on the host is not in it.

That is written down rather than fixed with --filesystem=host, and the argument
for not fixing it that way is §2: the Arch package and an AppImage both hand the
application the same unrestricted process the developer runs it in, so Flatpak
is the only Linux channel that tests whether a design assumed unrestricted
access. Granting the permission removes the only reason to ship it.

The reverse coupling on Android is recorded too. NFR-COMPAT-2 says Play
distribution is what makes ARCH §6.9 binding; §6.9 is verified rather than
assumed, so SAF is already unconditional and a sideloaded build would gain
nothing by asking for more. Play is deferred over the GPLv3 question, which is
a licence-reading exercise and blocks nothing in the storage design.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 20:18:20 +02:00

6.7 KiB

Contributing to DarkRoom

There is a lot of documentation here — 14 documents and 177 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 and copy 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:

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:

sudo apt-get install pkg-config libfontconfig1-dev libxkbcommon-dev

Then:

cargo run -p darkroom-desktop

The first build resolves 826 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 if you get there.

What CI will check

All four of these run on every push, so run them before you send anything:

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.

Requirements and traceability

requirements.md is the register of record. traceability.md is generated from TRACES: tags in the source and must never be hand-edited:

// 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:

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/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.

Two invariants the build defends

Worth knowing before you trip one, because both failures name 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.

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.

Document Read it when
core/dr-pipeline/ops/README.md Adding or changing a develop operation — start here regardless
docs/architecture.md Anything touching the render path, catalog or sync
docs/code-health.md Deciding what to work on; grades each seam by what it costs
docs/technical-debt.md Something looks wrong — check it was not chosen
docs/distribution.md Packaging a build, or adding a permission to one
docs/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.