Convert to the working space before the edits, not after

Every point operation ran in camera RGB and the camera matrix came after
them all, against the order ARCH §5.2 draws. So `luminance()` applied
Rec.709 weights to a body's own primaries, a band in the colour mixer
was a different hue on every make of sensor, and the vibrance skin guard
tested channel order in a space where skin does not have one.

Operations now declare a `Stage`. White balance is the only camera-stage
node — its multipliers scale the sensor's channels, and after a matrix
that mixes them the same numbers are a different correction — and says
so with `stage: camera` in its YAML, a key both the build-time generator
and the load-time declared op read. The composer emits the camera nodes,
then the matrix, then the rest, each group in graph order; an empty
chain still gets the matrix.

Film simulation stops converting out of camera space itself, since it is
now handed working-space colour like every other scene node. The base
curve stays where it was, after the operations, and so now acts on
working-space colour; the next commit replaces it (D19).

core/dr-pipeline/src/declared/decl.rs

@@ -271,6 +271,32 @@ pub struct Declaration {
     /// Boxed so the rare node that declares one does not widen every
     /// declaration by the size of a presentation it does not have.
     pub presentation: Option<Box<PresentationDef>>,
+    /// Whether the node runs in camera RGB, ahead of the camera matrix —
+    /// `stage: camera`. See [`read_stage`].
+    pub camera_stage: bool,
+}
+
+/// TRACES: FR-DEV-3e | FR-DEV-2
+/// Where in the chain a declared node's colour comes from: `stage: camera` or
+/// `stage: scene`, the default.
+///
+/// Camera RGB is where white balance's multipliers are defined, and it is the
+/// only thing that belongs there (D19): every other operation is handed
+/// working-space colour, so that a hue or a luminance weight means the same
+/// thing whichever body took the frame. `view` is not offered. The view
+/// transform is hand-written, and a declared node that clipped into a display
+/// range would be exactly what ARCH §6.14 forbids of every node before it.
+fn read_stage(root: &Mapping) -> Result<bool, String> {
+    match root.get("stage") {
+        None => Ok(false),
+        Some(v) => match as_str(v, "stage")? {
+            "camera" => Ok(true),
+            "scene" => Ok(false),
+            other => Err(format!(
+                "unknown stage {other:?}; expected \"camera\" or \"scene\""
+            )),
+        },
+    }
 }
 
 impl Declaration {
@@ -512,6 +538,7 @@ pub fn read_node(text: &str, ctx: &str, shared: &BTreeSet<&str>) -> Result<Node,
             "define",
             "label",
             "attributes",
+            "stage",
         ] {
             if root.contains_key(key) {
                 return Err(format!(
@@ -564,6 +591,7 @@ pub fn read_node(text: &str, ctx: &str, shared: &BTreeSet<&str>) -> Result<Node,
         .collect();
     let tests = read_tests(root, &params, &uniform_names, &helper_names)?;
     let presentation = read_presentation(root, &param_names)?;
+    let camera_stage = read_stage(root)?;
 
     Ok(Node::Declared(Box::new(Declaration {
         id,
@@ -580,6 +608,7 @@ pub fn read_node(text: &str, ctx: &str, shared: &BTreeSet<&str>) -> Result<Node,
         active,
         tests,
         presentation,
+        camera_stage,
     })))
 }
 

core/dr-pipeline/src/declared/mod.rs

@@ -107,6 +107,8 @@ pub struct DeclaredOp {
     helpers: Vec<Helper>,
     presentation: Option<Presentation>,
     order: i64,
+    /// See `decl::read_stage`.
+    camera_stage: bool,
 }
 
 /// One uniform: the name the fragment reads it by, and how to compute it.
@@ -208,6 +210,7 @@ impl DeclaredOp {
             wgsl: declaration.wgsl_body(),
             helpers,
             presentation: declaration.presentation.as_deref().map(presentation),
+            camera_stage: declaration.camera_stage,
             order: declaration.order,
         })
     }
@@ -292,6 +295,14 @@ impl Operation for DeclaredOp {
     fn presentation(&self) -> Option<Presentation> {
         self.presentation.clone()
     }
+
+    fn stage(&self) -> crate::operation::Stage {
+        if self.camera_stage {
+            crate::operation::Stage::Camera
+        } else {
+            crate::operation::Stage::Scene
+        }
+    }
 }
 
 /// A declared parameter as the descriptor the panel reads.

core/dr-pipeline/src/operation.rs

@@ -338,9 +338,10 @@ pub trait Operation: Send + Sync {
     /// one.
     ///
     /// Almost everything returns `false`. An operation that returns `true`
-    /// takes camera RGB and hands back linear sRGB, and in exchange the
-    /// composer emits neither the camera profile's base curve nor the
-    /// conversion out of camera space — because this operation has done both.
+    /// hands back display-referred linear sRGB, and in exchange the composer
+    /// does not emit the camera profile's base curve — because this operation
+    /// has done its job. It is handed working-space colour like any other
+    /// scene-stage operation (D19).
     ///
     /// The reason it is a trait method and not a flag the caller sets is the
     /// one [`compose_full`] gives for deciding the output mode the same way: a
@@ -353,6 +354,14 @@ pub trait Operation: Send + Sync {
         false
     }
 
+    /// TRACES: FR-DEV-2 | FR-DEV-3e
+    /// Which colour this operation is handed. See [`Stage`].
+    ///
+    /// Default [`Stage::Scene`], which is every operation but white balance.
+    fn stage(&self) -> Stage {
+        Stage::Scene
+    }
+
     /// TRACES: FR-DEV-3 | FR-DEV-8
     /// This operation's neighbourhood stage, if it has one.
     ///
@@ -419,6 +428,48 @@ pub trait Operation: Send + Sync {
     fn set_lens_profile(&mut self, _profile: Option<&crate::lens::LensProfile>) {}
 }
 
+/// TRACES: FR-DEV-2 | FR-DEV-3e
+/// Camera RGB to the working space, emitted between the camera-stage
+/// operations and the scene-stage ones (see [`Stage`]).
+///
+/// Identity for a non-linear source, which is already in sRGB primaries, and
+/// for the camera-space tap, whose caller fills it so (FR-MRG-2).
+const CAMERA_MATRIX: &str = "
+    // ==== camera profile: the matrix (FR-DEV-3e) ====
+    //
+    // Camera RGB -> linear sRGB primaries, unbounded. After white balance,
+    // whose multipliers are defined on the sensor's channels, and before every
+    // other operation, which is handed working-space colour so that a hue or
+    // a luminance weight means the same thing on every body (D19).
+    c = vec3<f32>(
+        dot(u.cam_to_srgb_0.rgb, c),
+        dot(u.cam_to_srgb_1.rgb, c),
+        dot(u.cam_to_srgb_2.rgb, c),
+    );
+";
+
+/// TRACES: FR-DEV-2 | FR-DEV-3e
+/// Which colour a point operation is handed (D19, ARCH §5.2).
+///
+/// The composer emits every [`Stage::Camera`] operation, then the camera
+/// matrix, then every [`Stage::Scene`] one, each group in graph order. Before
+/// D19 there was no such split: every operation ran in camera RGB and the
+/// matrix came after them all, so `luminance()`'s Rec.709 weights were
+/// applied to camera primaries and a band in the colour mixer was a
+/// different hue on each body.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum Stage {
+    /// Camera RGB, balanced as shot, ahead of the matrix.
+    ///
+    /// For white balance alone. Its multipliers scale the sensor's own
+    /// channels, and a matrix that mixes them would make the same numbers a
+    /// different correction once it had run.
+    Camera,
+    /// Working-space colour: linear Rec.709 primaries, scene-referred,
+    /// unbounded. Nothing here clamps above 1.0 or encodes (ARCH §6.14).
+    Scene,
+}
+
 /// A named WGSL helper function, deduplicated across operations.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct Helper {
@@ -897,11 +948,21 @@ fn compose_inner(
     // Every point operation that is active globally *or* in some layer. One
     // that only a layer moved still runs here, at its place in the chain,
     // with nothing on the global side of its blend.
-    for op in ops
-        .iter()
-        .map(|o| o.as_ref())
-        .filter(|o| o.detail().is_none())
-    {
+    //
+    // Camera-stage operations first, then the camera matrix, then the rest —
+    // each group in graph order (D19). Graph order still decides everything
+    // within a group; the stage only decides which colour the group is given.
+    let point = |stage: Stage| {
+        ops.iter()
+            .map(|o| o.as_ref())
+            .filter(move |o| o.detail().is_none() && o.stage() == stage)
+    };
+    let mut in_working_space = false;
+    for op in point(Stage::Camera).chain(point(Stage::Scene)) {
+        if !in_working_space && op.stage() == Stage::Scene {
+            body.push_str(CAMERA_MATRIX);
+            in_working_space = true;
+        }
         let id = op.descriptor().id.0;
         let local: Vec<&crate::mask::LocalOp> = layers.ops.iter().filter(|l| l.op == id).collect();
         if !op.is_active() && local.is_empty() {
@@ -950,6 +1011,12 @@ fn compose_inner(
         body.push_str(&local_block(&fragment, &local));
     }
 
+    // Emitted here when no scene operation was listed at all, which is what
+    // makes an empty chain still convert out of camera space.
+    if !in_working_space {
+        body.push_str(CAMERA_MATRIX);
+    }
+
     // A layer's operation the global chain does not hold at all. Not a case
     // any editor produces — both chains come from `ops::chain` — but a layer
     // must not lose an edit because a caller composed a shorter chain.
@@ -1094,14 +1161,9 @@ fn compose_inner(
         ),
     };
 
-    // The camera profile's rendering, which an operation may have taken over.
-    //
-    // Emitted as a unit because the two halves belong together: the base curve
-    // is defined in camera RGB and the matrix is what leaves it, so an
-    // operation that replaces one has necessarily replaced the other. Keeping
-    // them as one string is what makes that impossible to get half right.
+    // The base curve, which an operation may have taken over.
     let rendering_tail = if op_renders {
-        "    // The camera profile's base curve and the conversion out of camera\n    // space are both absent: an operation declaring `Operation::renders`\n    // has done both, and doing them again would render the picture twice.\n"
+        "    // The base curve is absent: an operation declaring\n    // `Operation::renders` has done its job, and doing it again would render\n    // the picture twice.\n"
             .to_string()
     } else {
         "    // ==== camera profile: the base curve (FR-DEV-3e) ====
@@ -1110,27 +1172,10 @@ fn compose_inner(
     // is not one, and the difference is what several tests count on to tell
     // an edit apart from the reading of a file.
     //
-    // The stage between demosaic and the working space that turns a correct
-    // exposure into a photograph. Sensor data is scene-referred and nearly
-    // linear; nothing anybody looks at is. Rendering it straight out is the
-    // dcraw default, and it is flat, dark through the midtones and clips its
-    // highlights instead of rolling them off.
-    //
-    // **In camera RGB, and after the adjustments**, which is a deliberate pair
-    // of choices:
-    //
-    // - Before the matrix, because that is where a base curve is defined and
-    //   where every other converter applies one. The curve was tuned against
-    //   this body's own primaries; moving it after the conversion would apply
-    //   a Canon rendering to sRGB values and change what it does.
-    // - After exposure and the tonal operations, because those are corrections
-    //   to *capture* and are only meaningful on linear values. A stop is a
-    //   doubling; run exposure after a curve and it stops being one.
-    //
-    // Per channel rather than on luminance. It desaturates the extremes
-    // slightly, and that is the point — it is what makes a blown sky roll
-    // toward white rather than toward a saturated corner of the gamut, and it
-    // is what the camera's own JPEG does.
+    // After every operation, and — since D19 moved the matrix to the front of
+    // the chain — on working-space colour rather than the camera RGB it was
+    // tuned against. An interim placement: the per-body curve is being
+    // replaced by a view transform after the detail stage (FR-DEV-3j).
     //
     // The branch is on a uniform, so the whole dispatch takes the same path.
     // It is off for a JPEG and any other already-rendered source, which must
@@ -1155,17 +1200,6 @@ fn compose_inner(
             ),
         );
     }
-
-    // Camera space -> linear sRGB. Applied after the adjustments so white
-    // balance and exposure act on sensor-native values, which is where they
-    // are physically meaningful.
-    //
-    // Identity for a non-linear source, which is already in sRGB primaries.
-    c = vec3<f32>(
-        dot(u.cam_to_srgb_0.rgb, c),
-        dot(u.cam_to_srgb_1.rgb, c),
-        dot(u.cam_to_srgb_2.rgb, c),
-    );
 "
         .to_string()
     };
@@ -2236,12 +2270,11 @@ mod tests {
     }
 
     #[test]
-    fn a_rendering_operation_takes_over_the_base_curve_and_the_camera_matrix() {
+    fn a_rendering_operation_takes_over_the_base_curve() {
         // TRACES: FR-DEV-3e | FR-DEV-3f
-        // A film stock's characteristic curve does the base curve's job, and
-        // the film node converts out of camera space itself. Emitting the
-        // profile's rendering as well would render the scene twice and convert
-        // it twice — a picture that comes out looking like neither the camera's
+        // A film stock's characteristic curve does the base curve's job.
+        // Emitting the profile's rendering as well would render the scene
+        // twice — a picture that comes out looking like neither the camera's
         // rendering nor the film's, with a colour-management bug's signature
         // and no colour-management bug to find.
         let mut film = crate::ops::FilmSim::new();
@@ -2262,26 +2295,25 @@ mod tests {
         assert!(film.is_active(), "the fixture did not load");
 
         let source = compose(&[Box::new(film) as Box<dyn Operation>]).source;
-        assert!(
-            source.contains("---- film_sim ----"),
-            "the operation itself must still be emitted"
-        );
+        let film_at = source
+            .find("---- film_sim ----")
+            .expect("the operation itself must still be emitted");
         assert!(
             !source.contains("base_curve_last.z > 0.5"),
             "the base curve is still being applied on top of the film"
         );
-        // Asserted on the composer's own comment, not on the conversion
-        // itself: the film fragment performs exactly the same three dot
-        // products, so a substring search cannot tell the composer's copy from
-        // the operation's. What must be gone is the *second* one.
-        assert!(
-            !source.contains("Camera space -> linear sRGB"),
-            "the composer converted out of camera space after the film already had"
-        );
+        // D19: the film no longer converts out of camera space itself. The
+        // composer does, once, ahead of it — the film is handed working-space
+        // colour like every other scene-stage operation.
         assert_eq!(
             source.matches("dot(u.cam_to_srgb_0.rgb, c)").count(),
             1,
-            "camera space is left exactly once, and it is the film that does it"
+            "camera space is left exactly once"
+        );
+        let matrix = source.find("camera profile: the matrix").expect("matrix");
+        assert!(
+            matrix < film_at,
+            "the film must be handed working-space colour"
         );
     }
 
@@ -2292,7 +2324,7 @@ mod tests {
         // flat and uncorrected, which reads as a broken camera profile.
         let source = compose(&[fake(DESC_A.clone(), 2.0, false)]).source;
         assert!(source.contains("base_curve_last.z > 0.5"));
-        assert!(source.contains("Camera space -> linear sRGB"));
+        assert!(source.contains("camera profile: the matrix"));
     }
 
     #[test]
@@ -2304,43 +2336,57 @@ mod tests {
         let film: Box<dyn Operation> = Box::new(crate::ops::FilmSim::new());
         let source = compose(&[film, fake(DESC_A.clone(), 2.0, false)]).source;
         assert!(source.contains("base_curve_last.z > 0.5"));
-        assert!(source.contains("Camera space -> linear sRGB"));
+        assert!(source.contains("camera profile: the matrix"));
     }
 
     #[test]
-    fn the_camera_matrix_is_applied_after_the_operations() {
-        // Adjustments are meaningful in sensor-native space, where highlight
-        // headroom still exists; converting first would clip it away.
-        let ops = vec![fake(DESC_A.clone(), 2.0, false)];
+    fn the_camera_matrix_follows_white_balance_and_precedes_the_scene() {
+        // TRACES: FR-DEV-2 | FR-DEV-3e
+        // D19. White balance's multipliers are defined on the sensor's own
+        // channels, so it is handed camera RGB. Every other operation is
+        // handed working-space colour: before this, the edits ran in camera
+        // RGB and `luminance()`'s Rec.709 weights were applied to a body's
+        // own primaries. Graph order puts the scene operation first here on
+        // purpose — the stage, not the order, decides which side of the
+        // matrix an operation lands on.
+        let mut wb = crate::ops::WhiteBalance::new();
+        wb.set_param(crate::ops::white_balance::TEMPERATURE, 30.0);
+        let ops: Vec<Box<dyn Operation>> = vec![fake(DESC_A.clone(), 2.0, false), Box::new(wb)];
         let source = compose(&ops).source;
+        let wb = source.find("---- white_balance ----").expect("wb present");
+        let matrix = source.find("camera profile: the matrix").expect("matrix");
         let op = source.find("---- op_a ----").expect("op present");
-        let matrix = source.find("u.cam_to_srgb_0").expect("matrix applied");
-        assert!(op < matrix, "the camera matrix must come after operations");
+        assert!(wb < matrix, "white balance must run in camera RGB");
+        assert!(
+            matrix < op,
+            "a scene operation must be handed working-space colour"
+        );
     }
 
     #[test]
-    fn the_base_curve_runs_after_the_operations_and_before_the_camera_matrix() {
+    fn an_empty_chain_still_leaves_camera_space() {
         // TRACES: FR-DEV-3e
-        // Both halves matter and for different reasons.
-        //
-        // After the operations: exposure and the tonal controls are
-        // corrections to capture, and they are only meaningful on linear
-        // values. A stop is a doubling; run exposure after a curve and it is
-        // not one any more, and every slider in the panel starts lying about
-        // what it does.
-        //
-        // Before the matrix: the curve was tuned against this body's own
-        // primaries. Applied after the conversion it would be a Canon
-        // rendering acting on sRGB values, which is a different curve.
+        // The matrix is emitted at the first scene operation, so a chain with
+        // none needs it emitted anyway, or an unedited RAW opens in camera
+        // primaries.
+        let source = compose(&[]).source;
+        assert_eq!(source.matches("dot(u.cam_to_srgb_0.rgb, c)").count(), 1);
+    }
+
+    #[test]
+    fn the_base_curve_runs_after_the_operations() {
+        // TRACES: FR-DEV-3e
+        // Exposure and the tonal controls are corrections to capture, and
+        // they are only meaningful on linear values. A stop is a doubling; run
+        // exposure after a curve and it is not one any more, and every slider
+        // in the panel starts lying about what it does.
         let ops = vec![fake(DESC_A.clone(), 2.0, false)];
         let source = compose(&ops).source;
         let op = source.find("---- op_a ----").expect("op present");
         let curve = source
             .find("if (u.base_curve_last.z > 0.5)")
             .expect("base curve applied");
-        let matrix = source.find("u.cam_to_srgb_0").expect("matrix applied");
         assert!(op < curve, "the base curve must come after the operations");
-        assert!(curve < matrix, "and before the camera matrix");
     }
 
     #[test]

core/dr-pipeline/src/ops/film_sim.rs

@@ -396,14 +396,10 @@ impl Operation for FilmSim {
         // sampler binding, and adding one to interpolate two lookups would
         // cost a binding in every shader whether or not a film is loaded.
         "\
-// Camera RGB to linear sRGB. The film's exposure matrix is defined against
-// sRGB primaries, and this node has taken over the conversion the composer
-// would otherwise have emitted at the end — see `Operation::renders`.
-let scene = vec3<f32>(
-    dot(u.cam_to_srgb_0.rgb, c),
-    dot(u.cam_to_srgb_1.rgb, c),
-    dot(u.cam_to_srgb_2.rgb, c),
-);
+// Working-space colour, which is linear sRGB primaries — what the film's
+// exposure matrix is defined against. The composer converted out of camera
+// RGB before any scene-stage operation ran (D19).
+let scene = c;
 
 // What each emulsion layer was exposed to. A matrix, exactly: the scene
 // spectrum reconstructed from an sRGB triple is linear in that triple, so the
@@ -727,13 +723,15 @@ mod tests {
     }
 
     #[test]
-    fn the_fragment_converts_out_of_camera_space_itself() {
-        // It has to: it has taken over the conversion the composer would
-        // otherwise emit at the end.
+    fn the_fragment_is_handed_working_space_colour() {
+        // TRACES: FR-DEV-3f
+        // D19: the composer leaves camera space before any scene-stage
+        // operation, so a film converting again would apply the camera
+        // matrix twice.
         let mut op = FilmSim::new();
         op.set_tables(Some(tables()));
         let wgsl = op.wgsl_body();
-        assert!(wgsl.contains("cam_to_srgb_0"), "{wgsl}");
+        assert!(!wgsl.contains("cam_to_srgb"), "{wgsl}");
     }
 
     #[test]