dtourolle/DarkRoom
1
0
Fork 0
Code Issues 39 Pull Requests Actions Packages Projects Releases 20 Wiki Activity

Correct the lens first and settle the grain last

Browse Source 
`Attribute::ALL` has claimed since it was written to be roughly the order a
photographer works in, and 474dcf0 moved `Compose` to the front on exactly that
argument. Both ends went on contradicting it.

`Optics` sat fifth, so the column offered the lens corrections after the tones
they change. Removing a vignette brightens the frame; an exposure judged before
that correction has to be judged again after it, which is the definition of the
wrong order. `Detail` sat fourth, so sharpening and noise reduction — the only
work here that depends on everything above it, and the only work that cannot be
judged at fit view at all — were offered before the lens had even been put
right.

So `Optics, Compose, Tone, Colour, Effect, Detail`. `Optics` leads even
`Compose` because it is not a decision about the photograph at all: it is
undoing what the equipment did, a property of the capture rather than a choice.
`Effect` after `Colour` is a look laid over a settled picture, and is the one
slot that is genuinely arguable — a spectral film simulation declares `renders`
and replaces the base curve, which is a case for treating it as foundational
instead, and e235e99 filed film under `Effect` only days ago. The doc comment
records that tension rather than pretending to settle it; an array of six
cannot say "last, except when it is first".

`decl::Attr::ALL` moves with it. It is the second spelling of one vocabulary,
compiled by `build.rs` where `descriptor` is not visible, and the agreement
test in `declared/mod.rs` zips the two positionally — that test is what caught
the last reorder, and it would have caught this one.

Nothing persists a position in this list, which is what makes the reorder safe
rather than merely tidy. `Scope` packs one bit per attribute indexed by
`Attribute::ALL`, but `bits` is private, has no accessor and no `serde`; what
reaches a settings file is `develop.copy_attributes`, a list of names read back
through `Attribute::from_name`. A photographer's copy scope survives untouched
— only the order the names happen to be written in changes.

6a97fdf is why this is worth a commit now rather than a shrug: the
contradiction was harmless while the list only fed a row of chips nobody reads
in order, and stopped being harmless when the same list began driving a column
read top to bottom.

The matrix follows the two files' shifted line numbers.
This commit is contained in:
dtourolle
2026-09-05 21:17:56 +02:00
parent 59917c5183
commit efa9d84aad
3 changed files with 41 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/dr-pipeline/src/declared/decl.rs
+6 -4
View File
@@ -137,15 +137,17 @@ impl Attr {
}
}
/// Every attribute, in the order `crate::descriptor::Attribute` declares
/// them.
/// Every attribute, in the order `crate::descriptor::Attribute::ALL`
/// lists them — the reasoning for the sequence lives there, and the
/// agreement test in `declared/mod.rs` zips the two, so a reorder there
/// that is not mirrored here is a failure rather than a drift.
pub const ALL: [Attr; 6] = [
Attr::Optics,
Attr::Compose,
Attr::Tone,
Attr::Colour,
Attr::Detail,
Attr::Optics,
Attr::Effect,
Attr::Detail,
];
}
core/dr-pipeline/src/descriptor.rs
+33 -12
View File
@@ -594,26 +594,47 @@ pub enum Attribute {
}
impl Attribute {
/// Every attribute, in declaration order.
/// Every attribute, in roughly the order a photographer works through
/// them.
///
/// Declaration order is roughly the order a photographer works in, which
/// makes it a reasonable *default* for a frontend that wants one. It is
/// not a screen order: nothing here obliges a frontend to show them all,
/// show them in this sequence, or show them at all.
/// That makes it a reasonable *default* for a frontend that wants one. It
/// is not a screen order: nothing here obliges a frontend to show them
/// all, show them in this sequence, or show them at all.
///
/// `Compose` leads because framing is the first decision made about a
/// photograph and the one every later judgement is made inside — there is
/// `Optics` leads because correcting the lens is not a decision about the
/// photograph — it is undoing what the equipment did, a property of the
/// capture rather than a choice — and it moves the ground every later
/// judgement stands on. Removing a vignette *brightens the frame*, so an
/// exposure set before the correction has to be set again after it.
///
/// `Compose` follows as the first decision actually made about the
/// photograph, and the one every later judgement is made inside: there is
/// no sense in balancing tones across a frame that is about to lose a
/// third of its width. It sat fifth while it was called `Geometry`, which
/// contradicted the claim above and was noticed only when this list
/// started driving a control read top to bottom.
/// third of its width.
///
/// `Detail` trails because sharpening and noise reduction depend on
/// everything above them, and are the only ones here that cannot be judged
/// at fit view at all. Offering them fourth, before the lens has even been
/// corrected, invites the photographer to settle grain against an image
/// that is still going to move.
///
/// `Effect` after `Colour` is a look laid over a settled picture — and is
/// the one arguable slot. A spectral film simulation declares
/// [`crate::Operation::renders`] and replaces the base curve, which is an
/// argument for treating it as foundational rather than final; an array of
/// six cannot say "last, except when it is first". The tension is recorded
/// here rather than settled.
///
/// Both ends were wrong for as long as this list only fed a row of chips
/// nobody reads in order. It stopped being harmless when the same list
/// began driving a column read top to bottom.
pub const ALL: [Attribute; 6] = [
Attribute::Optics,
Attribute::Compose,
Attribute::Tone,
Attribute::Colour,
Attribute::Detail,
Attribute::Optics,
Attribute::Effect,
Attribute::Detail,
];
/// The localisation key naming this concept.
docs/traceability.md
+2 -2
View File
@@ -112,8 +112,8 @@ _None._
| FR-PLAT-AND-6 | [`apps/darkroom-android/src/lib.rs:388`](../apps/darkroom-android/src/lib.rs#L388) |
| FR-PLAT-LIN-1 | [`core/dr-types/src/settings.rs:1`](../core/dr-types/src/settings.rs#L1), [`platform/dr-plat/src/storage.rs:344`](../platform/dr-plat/src/storage.rs#L344), [`ui/dr-ui/src/lib.rs:999`](../ui/dr-ui/src/lib.rs#L999), [`ui/dr-ui/src/preset_store.rs:1`](../ui/dr-ui/src/preset_store.rs#L1), [`ui/dr-ui/src/settings_store.rs:1`](../ui/dr-ui/src/settings_store.rs#L1) |
| FR-PLAT-LIN-2 | [`platform/dr-plat/src/display.rs:1`](../platform/dr-plat/src/display.rs#L1), [`platform/dr-plat/src/display/wayland.rs:1`](../platform/dr-plat/src/display/wayland.rs#L1), [`platform/dr-plat/src/display/x11.rs:1`](../platform/dr-plat/src/display/x11.rs#L1) |
| FR-PLG-2 | [`core/dr-pipeline/src/declared/decl.rs:1`](../core/dr-pipeline/src/declared/decl.rs#L1), [`core/dr-pipeline/src/declared/expr.rs:152`](../core/dr-pipeline/src/declared/expr.rs#L152), [`core/dr-pipeline/src/declared/expr.rs:1`](../core/dr-pipeline/src/declared/expr.rs#L1), [`core/dr-pipeline/src/declared/mod.rs:1`](../core/dr-pipeline/src/declared/mod.rs#L1), [`core/dr-pipeline/src/declared/mod.rs:82`](../core/dr-pipeline/src/declared/mod.rs#L82), [`core/dr-pipeline/src/descriptor.rs:15`](../core/dr-pipeline/src/descriptor.rs#L15), [`core/dr-pipeline/src/descriptor.rs:674`](../core/dr-pipeline/src/descriptor.rs#L674), [`core/dr-pipeline/src/operation.rs:232`](../core/dr-pipeline/src/operation.rs#L232), [`core/dr-pipeline/tests/declared_parity.rs:1`](../core/dr-pipeline/tests/declared_parity.rs#L1), [`core/dr-pipeline/tests/declared_parity.rs:240`](../core/dr-pipeline/tests/declared_parity.rs#L240), [`core/dr-pipeline/tests/declared_parity.rs:305`](../core/dr-pipeline/tests/declared_parity.rs#L305), [`core/dr-pipeline/tests/declared_parity.rs:358`](../core/dr-pipeline/tests/declared_parity.rs#L358), [`core/dr-pipeline/tests/declared_parity.rs:417`](../core/dr-pipeline/tests/declared_parity.rs#L417) |
| FR-PLG-2d | [`core/dr-pipeline/src/declared/decl.rs:112`](../core/dr-pipeline/src/declared/decl.rs#L112), [`core/dr-pipeline/src/declared/decl.rs:152`](../core/dr-pipeline/src/declared/decl.rs#L152), [`core/dr-pipeline/src/declared/decl.rs:1`](../core/dr-pipeline/src/declared/decl.rs#L1), [`core/dr-pipeline/src/declared/decl.rs:420`](../core/dr-pipeline/src/declared/decl.rs#L420), [`core/dr-pipeline/src/declared/decl.rs:67`](../core/dr-pipeline/src/declared/decl.rs#L67), [`core/dr-pipeline/src/declared/mod.rs:1`](../core/dr-pipeline/src/declared/mod.rs#L1), [`core/dr-pipeline/src/declared/mod.rs:384`](../core/dr-pipeline/src/declared/mod.rs#L384), [`core/dr-pipeline/src/declared/mod.rs:403`](../core/dr-pipeline/src/declared/mod.rs#L403) |
| FR-PLG-2 | [`core/dr-pipeline/src/declared/decl.rs:1`](../core/dr-pipeline/src/declared/decl.rs#L1), [`core/dr-pipeline/src/declared/expr.rs:152`](../core/dr-pipeline/src/declared/expr.rs#L152), [`core/dr-pipeline/src/declared/expr.rs:1`](../core/dr-pipeline/src/declared/expr.rs#L1), [`core/dr-pipeline/src/declared/mod.rs:1`](../core/dr-pipeline/src/declared/mod.rs#L1), [`core/dr-pipeline/src/declared/mod.rs:82`](../core/dr-pipeline/src/declared/mod.rs#L82), [`core/dr-pipeline/src/descriptor.rs:15`](../core/dr-pipeline/src/descriptor.rs#L15), [`core/dr-pipeline/src/descriptor.rs:695`](../core/dr-pipeline/src/descriptor.rs#L695), [`core/dr-pipeline/src/operation.rs:232`](../core/dr-pipeline/src/operation.rs#L232), [`core/dr-pipeline/tests/declared_parity.rs:1`](../core/dr-pipeline/tests/declared_parity.rs#L1), [`core/dr-pipeline/tests/declared_parity.rs:240`](../core/dr-pipeline/tests/declared_parity.rs#L240), [`core/dr-pipeline/tests/declared_parity.rs:305`](../core/dr-pipeline/tests/declared_parity.rs#L305), [`core/dr-pipeline/tests/declared_parity.rs:358`](../core/dr-pipeline/tests/declared_parity.rs#L358), [`core/dr-pipeline/tests/declared_parity.rs:417`](../core/dr-pipeline/tests/declared_parity.rs#L417) |
| FR-PLG-2d | [`core/dr-pipeline/src/declared/decl.rs:112`](../core/dr-pipeline/src/declared/decl.rs#L112), [`core/dr-pipeline/src/declared/decl.rs:154`](../core/dr-pipeline/src/declared/decl.rs#L154), [`core/dr-pipeline/src/declared/decl.rs:1`](../core/dr-pipeline/src/declared/decl.rs#L1), [`core/dr-pipeline/src/declared/decl.rs:422`](../core/dr-pipeline/src/declared/decl.rs#L422), [`core/dr-pipeline/src/declared/decl.rs:67`](../core/dr-pipeline/src/declared/decl.rs#L67), [`core/dr-pipeline/src/declared/mod.rs:1`](../core/dr-pipeline/src/declared/mod.rs#L1), [`core/dr-pipeline/src/declared/mod.rs:384`](../core/dr-pipeline/src/declared/mod.rs#L384), [`core/dr-pipeline/src/declared/mod.rs:403`](../core/dr-pipeline/src/declared/mod.rs#L403) |
| FR-PLG-8 | [`core/dr-pipeline/src/sidecar.rs:1924`](../core/dr-pipeline/src/sidecar.rs#L1924), [`core/dr-pipeline/src/sidecar.rs:1957`](../core/dr-pipeline/src/sidecar.rs#L1957) |
| FR-RAW-1 | [`core/dr-decode/src/lib.rs:243`](../core/dr-decode/src/lib.rs#L243), [`core/dr-types/src/lib.rs:132`](../core/dr-types/src/lib.rs#L132), [`core/dr-types/src/lib.rs:203`](../core/dr-types/src/lib.rs#L203) |
| FR-RAW-3 | [`core/dr-decode/src/lib.rs:139`](../core/dr-decode/src/lib.rs#L139), [`core/dr-decode/src/lib.rs:506`](../core/dr-decode/src/lib.rs#L506), [`core/dr-decode/src/locate.rs:1366`](../core/dr-decode/src/locate.rs#L1366) |