Release 0.13.5

The recording sampled a red brick wall and moved the sliders by three
units, which at GIF size is a click that does nothing. The scene now
drags the frame cold first and picks a white air conditioner, so the
correction is visible and the picker's being absolute - set from the
photograph, not from where the sliders were - is what the picture
shows. The text says so, and says a blown highlight is refused.

Cargo.lock

@@ -1221,7 +1221,7 @@ checksum = "f27ae1dd37df86211c42e150270f82743308803d90a6f6e6651cd730d5e1732f"
 
 [[package]]
 name = "darkroom-android"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "android_logger",
  "dr-plat",
@@ -1234,7 +1234,7 @@ dependencies = [
 
 [[package]]
 name = "darkroom-desktop"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "anyhow",
  "dr-plat",
@@ -1408,7 +1408,7 @@ checksum = "d8b14ccef22fc6f5a8f4d7d768562a182c04ce9a3b3157b91390b52ddfdf1a76"
 
 [[package]]
 name = "dr-bench"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "anyhow",
  "dr-catalog",
@@ -1425,7 +1425,7 @@ dependencies = [
 
 [[package]]
 name = "dr-catalog"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "dr-face",
  "dr-plat",
@@ -1440,7 +1440,7 @@ dependencies = [
 
 [[package]]
 name = "dr-decode"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "dr-types",
  "env_logger",
@@ -1454,7 +1454,7 @@ dependencies = [
 
 [[package]]
 name = "dr-export"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "dr-decode",
  "dr-gpu",
@@ -1473,7 +1473,7 @@ dependencies = [
 
 [[package]]
 name = "dr-face"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "dr-inference-engine",
  "env_logger",
@@ -1486,7 +1486,7 @@ dependencies = [
 
 [[package]]
 name = "dr-film"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "log",
  "serde",
@@ -1495,7 +1495,7 @@ dependencies = [
 
 [[package]]
 name = "dr-gpu"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "bytemuck",
  "dr-decode",
@@ -1513,7 +1513,7 @@ dependencies = [
 
 [[package]]
 name = "dr-inference-engine"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "libloading",
  "log",
@@ -1527,7 +1527,7 @@ dependencies = [
 
 [[package]]
 name = "dr-ingest"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "dr-plat",
  "dr-types",
@@ -1539,7 +1539,7 @@ dependencies = [
 
 [[package]]
 name = "dr-lens"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "lensfun",
  "log",
@@ -1547,7 +1547,7 @@ dependencies = [
 
 [[package]]
 name = "dr-pano"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "dr-decode",
  "dr-inference-engine",
@@ -1561,7 +1561,7 @@ dependencies = [
 
 [[package]]
 name = "dr-pipeline"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "dr-types",
  "log",
@@ -1570,7 +1570,7 @@ dependencies = [
 
 [[package]]
 name = "dr-plat"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "android-native-keyring-store",
  "dr-types",
@@ -1586,7 +1586,7 @@ dependencies = [
 
 [[package]]
 name = "dr-preset-xmp"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "dr-pipeline",
  "log",
@@ -1596,7 +1596,7 @@ dependencies = [
 
 [[package]]
 name = "dr-segment"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "dr-inference-engine",
  "env_logger",
@@ -1609,7 +1609,7 @@ dependencies = [
 
 [[package]]
 name = "dr-sync"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "async-trait",
  "dr-plat",
@@ -1623,7 +1623,7 @@ dependencies = [
 
 [[package]]
 name = "dr-sync-folder"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "async-trait",
  "dr-sync",
@@ -1635,7 +1635,7 @@ dependencies = [
 
 [[package]]
 name = "dr-sync-nextcloud"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "async-trait",
  "dr-decode",
@@ -1657,7 +1657,7 @@ dependencies = [
 
 [[package]]
 name = "dr-thumbs"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "dr-types",
  "jpeg-encoder",
@@ -1669,7 +1669,7 @@ dependencies = [
 
 [[package]]
 name = "dr-types"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "serde",
  "serde_json",
@@ -1678,7 +1678,7 @@ dependencies = [
 
 [[package]]
 name = "dr-ui"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "anyhow",
  "async-trait",
@@ -1720,7 +1720,7 @@ dependencies = [
 
 [[package]]
 name = "dr-xmp"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "dr-types",
  "log",
@@ -7021,7 +7021,7 @@ checksum = "8df9b6e13f2d32c91b9bd719c00d1958837bc7dec474d94952798cc8e69eeec3"
 
 [[package]]
 name = "traceability"
-version = "0.13.4"
+version = "0.13.5"
 dependencies = [
  "anyhow",
  "serde",

Cargo.toml

@@ -29,7 +29,7 @@ members = [
 ]
 
 [workspace.package]
-version = "0.13.4"
+version = "0.13.5"
 edition = "2021"
 rust-version = "1.92"
 license = "GPL-3.0-or-later"

README.md

@@ -76,7 +76,7 @@ controls, its place in the chain and its tests.
 
 ## Where it stands
 
-**0.13.4**, eighteen tagged releases in. 184 numbered requirements in
+**0.13.5**, nineteen tagged releases in. 184 numbered requirements in
 scope, 84% of them claimed by code and [traced to it](docs/traceability.md);
 the rest are written down rather than merely absent.
 

core/dr-pipeline/src/graph.rs

@@ -858,14 +858,36 @@ impl EditGraph {
     }
 
     /// TRACES: FR-DEV-3
-    /// The camera-space tap under this edit's own framing — crop, view,
+    /// The camera-space tap over one patch of what is on the canvas.
-    /// rotation and all — so a fraction of what is on the canvas is a
+    ///
-    /// fraction of what this renders. Nothing else of the edit: no
+    /// `patch` is in fractions of the visible region — the coordinates a
-    /// operation, no mask, no repair. See
+    /// click on the canvas arrives in — and is laid over this edit's own
-    /// [`crate::operation::compose_camera_probe`] for why the white balance
+    /// framing: crop, view, rotation and all, so a fraction of the canvas is
-    /// picker reads from here and not from the display.
+    /// a fraction of the probe. Nothing else of the edit: no operation, no
-    pub fn compose_camera_probe(&self) -> ComposedShader {
+    /// mask, no repair. Rendering only the patch is what lets a small target
-        crate::operation::compose_camera_probe(&self.warps, &self.framing)
+    /// cover every sensor pixel under it rather than sampling one in fifty;
+    /// see [`crate::operation::compose_camera_probe`] for why the white
+    /// balance picker reads from here and not from the display.
+    ///
+    /// The patch is centred where asked and held to the view's own minimum
+    /// extent: at a deep zoom a patch a fraction of the view would be
+    /// smaller than a view may be, and letting `set_view` widen it from one
+    /// corner would move the sample off the point that was clicked.
+    pub fn compose_camera_probe(&self, patch: crate::framing::CropRect) -> ComposedShader {
+        use crate::framing::CropRect;
+        let mut framing = self.framing;
+        let view = framing.view();
+        let width = (patch.width * view.width).max(CropRect::MIN_EXTENT);
+        let height = (patch.height * view.height).max(CropRect::MIN_EXTENT);
+        let cx = view.x + (patch.x + patch.width * 0.5) * view.width;
+        let cy = view.y + (patch.y + patch.height * 0.5) * view.height;
+        framing.set_view(CropRect {
+            x: (cx - width * 0.5).clamp(0.0, 1.0 - width),
+            y: (cy - height * 0.5).clamp(0.0, 1.0 - height),
+            width,
+            height,
+        });
+        crate::operation::compose_camera_probe(&self.warps, &framing)
     }
 
     /// TRACES: FR-DEV-19c

core/dr-pipeline/src/operation.rs

@@ -604,7 +604,9 @@ pub fn compose_full_revealing(
     warps: &[Box<dyn crate::lens::Warp>],
     reveal: Option<&crate::mask::Reveal>,
 ) -> ComposedShader {
-    compose_inner(ops, framing, output, masks, spots, warps, reveal, None)
+    compose_inner(
+        ops, framing, output, masks, spots, warps, reveal, None, false,
+    )
 }
 
 /// TRACES: FR-MRG-2
@@ -629,7 +631,7 @@ pub fn compose_camera_linear(
     // upright too, and `view` is a fraction of the upright frame.
     framing.set_baseline(baseline);
     framing.set_view(view);
-    compose_camera_probe(warps, &framing)
+    compose_camera_tap(warps, &framing, false)
 }
 
 /// TRACES: FR-DEV-3
@@ -642,9 +644,26 @@ pub fn compose_camera_linear(
 /// channels, which is every body. It also has to be measured at the pixel
 /// the canvas is showing, which is why this takes the edit's own framing
 /// where a merge passes the file's orientation and a tile.
+///
+/// **Interpolated whatever the framing says.** The point of rendering a
+/// patch is to average what is under it, and the nearest sampling an
+/// unrotated frame otherwise gets is a comb: at two source pixels per
+/// probe pixel it lands on the same column of any pattern every time, and
+/// the average of a thousand samples is then the average of nothing.
 pub fn compose_camera_probe(
     warps: &[Box<dyn crate::lens::Warp>],
     framing: &Framing,
+) -> ComposedShader {
+    compose_camera_tap(warps, framing, true)
+}
+
+/// The camera-space tap proper: no operations, `rgba32float`, and the
+/// profile uniforms left for the GPU side to fill neutral. `smooth` forces
+/// the interpolating sampler; see the two callers for who wants it and why.
+fn compose_camera_tap(
+    warps: &[Box<dyn crate::lens::Warp>],
+    framing: &Framing,
+    smooth: bool,
 ) -> ComposedShader {
     compose_inner(
         &[],
@@ -655,6 +674,7 @@ pub fn compose_camera_probe(
         warps,
         None,
         Some(OutputMode::CameraLinear),
+        smooth,
     )
 }
 
@@ -668,6 +688,7 @@ fn compose_inner(
     warps: &[Box<dyn crate::lens::Warp>],
     reveal: Option<&crate::mask::Reveal>,
     forced: Option<OutputMode>,
+    smooth: bool,
 ) -> ComposedShader {
     // The lens corrections, composed into one coordinate transform. Beside
     // `framing` because they are the other half of the same stage: framing
@@ -874,7 +895,10 @@ fn compose_inner(
     // framing alone — which is what this did before the warps existed — would
     // have nearest-neighboured a distortion correction on an unstraightened
     // frame, and the aliasing would have looked like a bad profile.
-    let interpolate = framing.needs_interpolation() || warp.is_active();
+    // `smooth` is the third reason, and the only one a caller states: the
+    // white balance probe averages a patch and cannot do that through a
+    // nearest-neighbour comb (see `compose_camera_probe`).
+    let interpolate = smooth || framing.needs_interpolation() || warp.is_active();
 
     // Declared ahead of the warp block, which assigns to them. They enter
     // equal to `p` so that a chain mixing a splitting warp with a

docs/manual/README.md

@@ -99,9 +99,13 @@ wheel zooms to any amount in between.
 
 ### White balance from the photograph
 
-Press `pick` in the White Balance group, then click something neutral.
+Press `pick` in the White Balance group, then click something neutral —
+a white wall, a grey card, the air conditioner here. The picker sets the
+sliders from the photograph, not from where they were: below, the frame
+is dragged cold first and one click puts it right. A blown highlight is
+refused, since a clipped pixel has no colour left to balance.
 
-![Picking a neutral wall to set the white balance](media/develop-wb.gif)
+![Cooling the frame with the slider, then picking a white air conditioner to set the white balance](media/develop-wb.gif)
 
 ### Composing
 

packaging/PKGBUILD

@@ -4,7 +4,7 @@
 # makes `makepkg -si` in this directory install what you are actually working
 # on. Swap `source` for a tagged tarball when there is something to release.
 pkgname=darkroom
-pkgver=0.13.4
+pkgver=0.13.5
 # Back to 1 with the version: a new pkgver is a new archive name, so there is
 # nothing for makepkg to reuse and nothing for a release number to disambiguate.
 pkgrel=1

tools/manual/scenes.py

@@ -207,8 +207,9 @@ def develop_wb():
     group('colour')
     rec('develop-wb')
     pause(0.4)
+    dr.drag(1398, 826, 1330, 826, 25); pause(1.2)      # temperature cold
     dr.click(1487, 776); pause(0.8)                    # pick
-    dr.click(1000, 300); pause(1.5)                    # a neutral wall
+    dr.click(995, 635); pause(1.5)                     # the white air conditioner
     hold(*BEFORE, 1.4); pause(0.8)
     cut()
     reset_edit()

ui/dr-ui/src/develop.rs

@@ -4468,47 +4468,78 @@ impl DevelopSession {
     /// screen, and a probe rendered without the crop and the zoom would be
     /// answering about a different part of the photograph.
     ///
-    /// **Rendered small on purpose.** A 192px probe of the visible region
+    /// **A patch, not a point.** The shader fetches the source at one
-    /// averages a small neighbourhood into each of its pixels, which is what
+    /// position per output pixel — nearest, or four photosites blended — so
-    /// every eyedropper does deliberately: a single photosite off a noisy
+    /// a probe of the whole visible region rendered at 192px was not
-    /// shadow is a worse answer than the patch around it, and the photographer
+    /// "averaging a neighbourhood into each pixel" as its comment claimed;
-    /// is pointing at a grey card rather than at a pixel. It is also one
+    /// it was one point sample of a noisy sensor, and two painted-white air
-    /// dispatch's worth of work on a click.
+    /// conditioners on the same wall answered +37 and −50. Every eyedropper
+    /// averages for exactly this reason: the photographer is pointing at a
+    /// grey card, not at a photosite. So the tap is narrowed to the
+    /// [`PATCH`] of the canvas around the click — a couple of percent of
+    /// its width, square on screen — and rendered at [`PROBE_PX`] square
+    /// with interpolation on, which puts a sample on every sensor pixel
+    /// under the patch at any ordinary zoom. Those are averaged; a sample
+    /// the tap marked void (outside the frame after the lens correction) or
+    /// clipped is left out rather than allowed to pull the mean, and if
+    /// fewer than half the patch survives there was nothing there to
+    /// balance against. One small dispatch and a 64 KB readback on a click.
     fn sample_as_shot(&mut self, x: f32, y: f32) -> Option<[f32; 3]> {
-        /// Long edge of the probe render. See the note above on why it is
+        /// Width of the patch as a fraction of what is on the canvas.
-        /// small rather than large.
+        const PATCH: f32 = 0.015;
-        const PROBE_EDGE: u32 = 192;
+        /// Side of the probe render, in pixels.
+        const PROBE_PX: u32 = 64;
 
+        // Square on screen: the height fraction follows the aspect of the
+        // visible region, which is the crop's shape times the view's.
         let (sw, sh) = self.demosaiced.size();
-        let (fw, fh) = self.graph.output_size(sw, sh);
+        let (cw, ch) = self.graph.output_size(sw, sh);
-        let (w, h) = fit(fw, fh, PROBE_EDGE, PROBE_EDGE);
+        let view = self.graph.framing().view();
-        let shader = self.graph.compose_camera_probe();
+        let aspect = (cw as f32 * view.width) / (ch as f32 * view.height).max(f32::EPSILON);
+        let (pw, ph) = (PATCH, PATCH * aspect);
+        let patch = dr_pipeline::CropRect {
+            x: x.clamp(0.0, 1.0) - pw * 0.5,
+            y: y.clamp(0.0, 1.0) - ph * 0.5,
+            width: pw,
+            height: ph,
+        };
+
+        let shader = self.graph.compose_camera_probe(patch);
         let rendered = self
             .adjust
-            .render_camera_linear(&self.demosaiced, &shader, w, h)
+            .render_camera_linear(&self.demosaiced, &shader, PROBE_PX, PROBE_PX)
             .map(|_| ());
-        let (rgba, pw, ph) = rendered
+        let (rgba, _, _) = rendered
             .and_then(|()| self.adjust.read_camera_linear())
             .inspect_err(|e| log::warn!("could not read a neutral off the frame: {e}"))
             .ok()?;
 
-        let (pw, ph) = (pw as usize, ph as usize);
+        let mut sum = [0.0f32; 3];
-        let px = ((x.clamp(0.0, 1.0) * pw as f32) as usize).min(pw.saturating_sub(1));
+        let mut kept = 0usize;
-        let py = ((y.clamp(0.0, 1.0) * ph as f32) as usize).min(ph.saturating_sub(1));
+        let mut seen = 0usize;
-        let at = (py * pw + px) * 4;
+        for pixel in rgba.chunks_exact(4) {
-        let pixel = rgba.get(at..at + 4)?;
+            seen += 1;
-        // The tap marks a pixel the lens correction pulled in from outside
+            // The tap marks a pixel the lens correction pulled in from
-        // the frame with alpha 0. There is nothing there to balance against.
+            // outside the frame with alpha 0. There is nothing there to
-        if pixel[3] < 0.5 {
+            // balance against.
-            return None;
+            if pixel[3] < 0.5 {
+                continue;
+            }
+            // Nor in a clipped one. A blown sky reads as sensor white, and
+            // sensor white with the as-shot balance on is strongly magenta —
+            // a solve over it drives tint to its stop for a pixel that, on
+            // the canvas, the shader has already desaturated to neutral. The
+            // same threshold the shader fades from, so what is refused here
+            // is what it would have hidden there.
+            if pixel[..3].iter().any(|c| *c >= dr_pipeline::CLIP_ONSET) {
+                continue;
+            }
+            for (acc, c) in sum.iter_mut().zip(pixel) {
+                *acc += c;
+            }
+            kept += 1;
         }
-        // Nor in a clipped one. A blown sky reads as sensor white, and sensor
+        if kept == 0 || kept * 2 < seen {
-        // white with the as-shot balance on is strongly magenta — a solve over
-        // it drives tint to its stop for a pixel that, on the canvas, the
-        // shader has already desaturated to neutral. The same threshold the
-        // shader fades from, so what is refused here is what it would have
-        // hidden there.
-        if pixel[..3].iter().any(|c| *c >= dr_pipeline::CLIP_ONSET) {
             return None;
         }
 
@@ -4516,7 +4547,8 @@ impl DevelopSession {
         // the operation multiplies them *after* the camera's own balance, so
         // that goes on here and the solve sees what the gains will see.
         let wb = self.demosaiced.as_shot_wb();
-        Some([pixel[0] * wb[0], pixel[1] * wb[1], pixel[2] * wb[2]])
+        let n = kept as f32;
+        Some([sum[0] / n * wb[0], sum[1] / n * wb[1], sum[2] / n * wb[2]])
     }
 
     /// TRACES: FR-PLAT-AND-5 | NFR-RES-1
@@ -6754,6 +6786,68 @@ mod tests {
         );
     }
 
+    /// TRACES: FR-DEV-3
+    /// The picker reads a patch, not a photosite.
+    ///
+    /// A frame whose pixels alternate warm and cool grey, averaging to a
+    /// neutral: a point sample lands on one or the other and swings the
+    /// controls hard one way, which is what two white boxes on the same wall
+    /// answering +37 and −50 looked like. Averaged, there is nothing to
+    /// correct, and the graph says so.
+    #[test]
+    fn sampling_averages_a_patch_rather_than_reading_one_photosite() {
+        let Some(ctx) = headless() else { return };
+        // Large enough that the patch — a couple of percent of the frame —
+        // holds many sensor pixels; on a 64px frame it would hold one, and
+        // the test would be asserting about interpolation instead.
+        let size = 1536u32;
+        let warm = [0.30f32, 0.25, 0.20];
+        let cool = [0.20f32, 0.25, 0.30];
+        let mut data = Vec::with_capacity((size * size * 3) as usize);
+        for i in 0..(size * size) as usize {
+            let p = if i % 2 == 0 { warm } else { cool };
+            data.extend(p.iter().map(|c| (c * 65535.0).round() as u16));
+        }
+        let raw = RawImage {
+            width: size,
+            height: size,
+            data,
+            cfa_pattern: dr_decode::CfaPattern::Rggb,
+            black_level: [0; 4],
+            white_level: 65535,
+            wb_coeffs: [1.0, 1.0, 1.0, 0.0],
+            color_matrix: None,
+            base_curve: dr_decode::BaseCurve::IDENTITY,
+            samples_per_pixel: 3,
+            profile: None,
+            make: String::new(),
+            model: String::new(),
+            crop: dr_decode::CropRect {
+                x: 0,
+                y: 0,
+                width: size,
+                height: size,
+            },
+        };
+        let mut session =
+            DevelopSession::open(&ctx, &raw, dr_types::Orientation::NORMAL).expect("session");
+
+        assert!(
+            session.sample_neutral(0.5, 0.5),
+            "a mid-grey patch is usable"
+        );
+        let moved: Vec<_> = session
+            .rows()
+            .iter()
+            .filter(|r| r.value != r.default_value)
+            .map(|r| (r.param_label.to_string(), r.value))
+            .collect();
+        assert!(
+            moved.iter().all(|(_, v)| v.abs() <= 2.0),
+            "the patch averages neutral, so nothing should move far: {moved:?}"
+        );
+    }
+
     /// TRACES: FR-DEV-3
     /// A blown highlight is refused, the way black is.
     ///