Average a patch under the white balance picker, not one photosite
The probe's comment said a 192px render "averages a small neighbourhood into each of its pixels". It does not: the composed shader fetches the source at one position per output pixel - nearest for an unrotated frame, four photosites blended otherwise - so the probe was a point sample of a noisy sensor, and two painted-white air conditioners on the same wall answered +37 and -50. The tap is now narrowed to the patch of the canvas around the click, a couple of percent of its width and square on screen, and rendered at 64x64 with interpolation forced on, which puts a sample on every sensor pixel under it at any ordinary zoom. The samples are averaged, with the void and clipped ones left out rather than allowed to pull the mean, and fewer than half surviving is refused. compose_camera_probe takes the patch; the merge's compose_camera_linear keeps its nearest sampling. The readback shrinks from six megabytes to sixty-four kilobytes. A frame of alternating warm and cool columns, averaging neutral, moves the controls by at most two units; a point sample swung them to sixty.
This commit is contained in:
@@ -858,14 +858,36 @@ impl EditGraph {
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3
|
||||
/// The camera-space tap under this edit's own framing — crop, view,
|
||||
/// 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
|
||||
/// operation, no mask, no repair. See
|
||||
/// [`crate::operation::compose_camera_probe`] for why the white balance
|
||||
/// picker reads from here and not from the display.
|
||||
pub fn compose_camera_probe(&self) -> ComposedShader {
|
||||
crate::operation::compose_camera_probe(&self.warps, &self.framing)
|
||||
/// The camera-space tap over one patch of what is on the canvas.
|
||||
///
|
||||
/// `patch` is in fractions of the visible region — the coordinates a
|
||||
/// click on the canvas arrives in — and is laid over this edit's own
|
||||
/// framing: crop, view, rotation and all, so a fraction of the canvas is
|
||||
/// a fraction of the probe. Nothing else of the edit: no operation, no
|
||||
/// mask, no repair. Rendering only the patch is what lets a small target
|
||||
/// 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
|
||||
|
||||
@@ -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
|
||||
|
||||
+26
-26
File diff suppressed because one or more lines are too long
+125
-31
@@ -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
|
||||
/// averages a small neighbourhood into each of its pixels, which is what
|
||||
/// every eyedropper does deliberately: a single photosite off a noisy
|
||||
/// shadow is a worse answer than the patch around it, and the photographer
|
||||
/// is pointing at a grey card rather than at a pixel. It is also one
|
||||
/// dispatch's worth of work on a click.
|
||||
/// **A patch, not a point.** The shader fetches the source at one
|
||||
/// position per output pixel — nearest, or four photosites blended — so
|
||||
/// a probe of the whole visible region rendered at 192px was not
|
||||
/// "averaging a neighbourhood into each pixel" as its comment claimed;
|
||||
/// it was one point sample of a noisy sensor, and two painted-white air
|
||||
/// 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
|
||||
/// small rather than large.
|
||||
const PROBE_EDGE: u32 = 192;
|
||||
/// Width of the patch as a fraction of what is on the canvas.
|
||||
const PATCH: f32 = 0.015;
|
||||
/// 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 (w, h) = fit(fw, fh, PROBE_EDGE, PROBE_EDGE);
|
||||
let shader = self.graph.compose_camera_probe();
|
||||
let (cw, ch) = self.graph.output_size(sw, sh);
|
||||
let view = self.graph.framing().view();
|
||||
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 px = ((x.clamp(0.0, 1.0) * pw as f32) as usize).min(pw.saturating_sub(1));
|
||||
let py = ((y.clamp(0.0, 1.0) * ph as f32) as usize).min(ph.saturating_sub(1));
|
||||
let at = (py * pw + px) * 4;
|
||||
let pixel = rgba.get(at..at + 4)?;
|
||||
// The tap marks a pixel the lens correction pulled in from outside
|
||||
// the frame with alpha 0. There is nothing there to balance against.
|
||||
if pixel[3] < 0.5 {
|
||||
return None;
|
||||
let mut sum = [0.0f32; 3];
|
||||
let mut kept = 0usize;
|
||||
let mut seen = 0usize;
|
||||
for pixel in rgba.chunks_exact(4) {
|
||||
seen += 1;
|
||||
// The tap marks a pixel the lens correction pulled in from
|
||||
// outside the frame with alpha 0. There is nothing there to
|
||||
// balance against.
|
||||
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
|
||||
// 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) {
|
||||
if kept == 0 || kept * 2 < seen {
|
||||
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.
|
||||
///
|
||||
|
||||
Reference in New Issue
Block a user