Merge branch 'histogram'
# Conflicts: # ui/dr-ui/src/lib.rs
This commit is contained in:
+119
-1
@@ -10,7 +10,7 @@
|
||||
//! new operation appears in the panel with no change here (FR-DEV-3c).
|
||||
|
||||
use dr_decode::RawImage;
|
||||
use dr_gpu::{AdjustPass, DemosaicedImage, Demosaicer, GpuContext};
|
||||
use dr_gpu::{AdjustPass, DemosaicedImage, Demosaicer, GpuContext, Histogram, HistogramPass};
|
||||
use dr_pipeline::ops::curve;
|
||||
use dr_pipeline::{
|
||||
CropRect, Edit, EditGraph, History, OpCapability, OpId, ParamId, ParamKind, Presentation,
|
||||
@@ -34,6 +34,12 @@ pub struct DevelopSession {
|
||||
history: History,
|
||||
demosaiced: DemosaicedImage,
|
||||
adjust: AdjustPass,
|
||||
/// TRACES: FR-DSP-7
|
||||
/// Optional, because a session that cannot count its frames is still a
|
||||
/// session that can develop them. If the reduction fails to build — an
|
||||
/// old driver, a device without the storage-buffer atomics it needs — the
|
||||
/// photographer loses the histogram and keeps the photograph.
|
||||
histogram: Option<HistogramPass>,
|
||||
}
|
||||
|
||||
impl DevelopSession {
|
||||
@@ -88,6 +94,9 @@ impl DevelopSession {
|
||||
history,
|
||||
demosaiced,
|
||||
adjust: AdjustPass::new(ctx),
|
||||
histogram: HistogramPass::new(ctx)
|
||||
.inspect_err(|e| log::warn!("no histogram on this device: {e}"))
|
||||
.ok(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -551,6 +560,34 @@ impl DevelopSession {
|
||||
Ok(slint::Image::from_rgba8(buffer))
|
||||
}
|
||||
|
||||
/// TRACES: FR-DSP-7
|
||||
/// Count the frame that is currently on the canvas.
|
||||
///
|
||||
/// **Reads the frame [`Self::render`] last produced rather than rendering
|
||||
/// its own.** The histogram has to describe what the photographer is
|
||||
/// looking at, and rendering a second time to count it would both cost a
|
||||
/// second pass and open the possibility of the two disagreeing.
|
||||
///
|
||||
/// That the frame is the *displayed* one has two consequences worth being
|
||||
/// explicit about. It is in the output colour space, which is what
|
||||
/// FR-DSP-7 asks for — the levels counted are the levels the display will
|
||||
/// show, so a clipped bin means a highlight that is actually gone rather
|
||||
/// than one the transform might still recover. And when the view is zoomed
|
||||
/// or cropped it describes the visible region, not the whole file: a
|
||||
/// photographer inspecting a highlight at 4× is asking about *that*
|
||||
/// highlight, and a histogram of the parts of the frame off screen would
|
||||
/// be answering a question nobody asked.
|
||||
///
|
||||
/// `None` where nothing has been rendered yet, or where the device could
|
||||
/// not build the reduction.
|
||||
pub fn histogram(&self) -> Option<Histogram> {
|
||||
let pass = self.histogram.as_ref()?;
|
||||
let frame = self.adjust.output()?;
|
||||
pass.compute(frame)
|
||||
.inspect_err(|e| log::warn!("histogram failed: {e}"))
|
||||
.ok()
|
||||
}
|
||||
|
||||
/// Render the *whole* frame for the crop overlay to be drawn over.
|
||||
///
|
||||
/// Crop mode cannot use [`Self::render`]: that applies the crop, so the
|
||||
@@ -1944,6 +1981,87 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// A frame black on the left half and white on the right, at `size`
|
||||
/// square. Both ends of the histogram are occupied and both clipping
|
||||
/// counters are non-zero, and cropping to one half leaves exactly one of
|
||||
/// them so.
|
||||
fn split_frame(size: u32) -> Vec<u8> {
|
||||
let mut rgba = Vec::with_capacity((size * size * 4) as usize);
|
||||
for _ in 0..size {
|
||||
for x in 0..size {
|
||||
let v = if x < size / 2 { 0u8 } else { 255 };
|
||||
rgba.extend_from_slice(&[v, v, v, 255]);
|
||||
}
|
||||
}
|
||||
rgba
|
||||
}
|
||||
|
||||
/// TRACES: FR-DSP-7
|
||||
#[test]
|
||||
fn the_histogram_counts_the_frame_that_is_actually_on_the_canvas() {
|
||||
// The wiring, end to end and against exact numbers: a 64x64 frame that
|
||||
// is half black and half white must come back as 2048 pixels at level
|
||||
// 0, 2048 at 255, and both clipping counters at 2048.
|
||||
//
|
||||
// Asserted at the session rather than at the pass because the mistake
|
||||
// this catches is not arithmetic — `dr_gpu` has its own tests for that
|
||||
// — it is counting the *wrong texture*. Reading a stale target, or the
|
||||
// demosaiced source instead of the adjusted output, produces a
|
||||
// perfectly well-formed histogram of an image the photographer is not
|
||||
// looking at, which is the one failure mode that cannot be seen.
|
||||
let Ok(ctx) = pollster::block_on(dr_gpu::GpuContext::new_headless()) else {
|
||||
log::warn!("no GPU adapter; skipping");
|
||||
return;
|
||||
};
|
||||
|
||||
let rgba = split_frame(64);
|
||||
let mut session =
|
||||
DevelopSession::open_rgb(&ctx, &rgba, 64, 64, dr_types::Orientation::NORMAL)
|
||||
.expect("session");
|
||||
session.render(64, 64).expect("render");
|
||||
|
||||
let hist = session.histogram().expect("a rendered session must count");
|
||||
assert_eq!(hist.pixels(), 64 * 64);
|
||||
assert_eq!(hist.red()[0], 2048, "the black half");
|
||||
assert_eq!(hist.red()[255], 2048, "the white half");
|
||||
assert_eq!(hist.clipped_shadows(), 2048);
|
||||
assert_eq!(hist.clipped_highlights(), 2048);
|
||||
}
|
||||
|
||||
/// TRACES: FR-DSP-7
|
||||
#[test]
|
||||
fn the_histogram_follows_the_edit_rather_than_the_file() {
|
||||
// The property that makes it *live*. A histogram computed once from the
|
||||
// source would pass the test above and be useless — the whole reason
|
||||
// FR-DSP-7 exists is to show what an adjustment is doing, so cropping
|
||||
// away the white half must leave a histogram with no white in it and
|
||||
// no highlight clipping to report.
|
||||
let Ok(ctx) = pollster::block_on(dr_gpu::GpuContext::new_headless()) else {
|
||||
log::warn!("no GPU adapter; skipping");
|
||||
return;
|
||||
};
|
||||
|
||||
let rgba = split_frame(64);
|
||||
let mut session =
|
||||
DevelopSession::open_rgb(&ctx, &rgba, 64, 64, dr_types::Orientation::NORMAL)
|
||||
.expect("session");
|
||||
|
||||
session.set_crop(CropRect {
|
||||
x: 0.0,
|
||||
y: 0.0,
|
||||
width: 0.5,
|
||||
height: 1.0,
|
||||
});
|
||||
session.render(64, 64).expect("render");
|
||||
|
||||
let hist = session.histogram().expect("histogram");
|
||||
assert_eq!(hist.pixels(), 32 * 64, "the crop halved the frame");
|
||||
assert_eq!(hist.red()[0], 32 * 64);
|
||||
assert_eq!(hist.red()[255], 0, "the white half was cropped away");
|
||||
assert_eq!(hist.clipped_highlights(), 0);
|
||||
assert_eq!(hist.clipped_shadows(), 32 * 64);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn routing_indices_map_back_to_the_right_parameter() {
|
||||
// A wrong index would silently move the wrong slider's value, which
|
||||
|
||||
Reference in New Issue
Block a user