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

Straighten a portrait frame in the frame the user is looking at

Browse Source 
The framing prologue built the centred position `p` by scaling with the
source's aspect, then straightened, then permuted the quarter turns. On a
landscape frame those are one space and it worked. Once a turn has swapped
the axes — the rotate button, or a file whose EXIF tag says the camera was
held sideways — they are not: `p` was measured with the source's ruler on a
frame that is no longer that shape, stretching one axis against the other by
(w/h)², which is 2.25 on a 3:2 photograph.

The quarter-turn permutation happened to undo that stretch, so rotation alone
looked right, which is how this survived. The straighten in between did not,
and a rotation in a space whose axes carry different scales is a shear.

`p` is now built in `frame_aspect` — the frame as the user sees it — and the
permutation becomes the one place the two rulers meet: each axis divided by
the aspect it is read from, multiplied by the aspect it is written to.

Asserted on pixels rather than on the generated WGSL, because reading the
shader and reasoning about which space `p` lives in is how the wrong formula
got written in the first place: a disc, straightened by 20° on a turned 3:2
frame, must come back circular by every route to a swapped frame — the
button, the tag, and the two composed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-18 15:17:54 +02:00
co-authored by Claude Opus 5
parent f1cd6ed5b3
commit f76e024f41
2 changed files with 244 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/dr-gpu/src/adjust.rs
+170 -40
View File
@@ -448,7 +448,7 @@ fn numbered(src: &str) -> String {
mod tests {
use super::*;
use dr_decode::{CfaPattern, CropRect, RawImage};
use dr_pipeline::ops::{exposure, saturation};
use dr_pipeline::ops::{colour_mixer, exposure, saturation};
use dr_pipeline::EditGraph;
use crate::Demosaicer;
@@ -494,6 +494,32 @@ mod tests {
.expect("demosaic")
}
/// A white disc on black, centred in a `w`x`h` frame.
///
/// The one shape that makes anisotropy unmissable: any transform that
/// scales the axes unequally returns it as an ellipse, and the ratio of
/// the ellipse's axes *is* the error.
fn disc_rgba(w: u32, h: u32, radius: f32) -> Vec<u8> {
let mut rgba = vec![0u8; (w * h * 4) as usize];
for y in 0..h {
for x in 0..w {
let dx = x as f32 - w as f32 / 2.0;
let dy = y as f32 - h as f32 / 2.0;
let v = if (dx * dx + dy * dy).sqrt() < radius {
255
} else {
0
};
let i = ((y * w + x) * 4) as usize;
rgba[i] = v;
rgba[i + 1] = v;
rgba[i + 2] = v;
rgba[i + 3] = 255;
}
}
rgba
}
fn read_centre(ctx: &GpuContext, tex: &wgpu::Texture) -> [u8; 4] {
let (w, h) = (tex.width(), tex.height());
read_pixel(ctx, tex, w / 2, h / 2)
@@ -1077,6 +1103,91 @@ mod tests {
);
}
#[test]
fn each_colour_band_gets_its_own_pipeline() {
// The bug this closes, end to end and through one cache: the mixer
// emits code only for the bands that are set, but the cache key was
// the set of *active operations*, which is "colour_mixer" whichever
// band that is. A red adjustment and a blue one hashed alike, so the
// second render reused the first's compiled pipeline and uploaded its
// uniform into the first band's slot — whichever band compiled first
// kept acting and every other slider did nothing.
//
// Ordered red first deliberately: red is the first band declared, and
// is the one users reported as the only one that worked.
let Some(ctx) = ctx() else { return };
let mut pass = AdjustPass::new(&ctx);
let img = blue_image(&ctx);
// `None` renders the chain untouched, which is the baseline the two
// band settings are measured against.
fn band(
ctx: &GpuContext,
pass: &mut AdjustPass,
img: &DemosaicedImage,
set: Option<(&'static str, f32)>,
) -> [u8; 4] {
let mut g = EditGraph::default_chain();
if let Some((id, v)) = set {
g.set_param(colour_mixer::ID, dr_pipeline::ParamId(id), v);
}
let shader = g.compose();
let t = pass.render(img, &shader, 16, 16).expect("render");
read_centre(ctx, t)
}
let neutral = band(&ctx, &mut pass, &img, None);
// Red first, so its pipeline is the one in the cache when blue asks.
let reds_turn = band(&ctx, &mut pass, &img, Some(("red_sat", 100.0)));
let blues_turn = band(&ctx, &mut pass, &img, Some(("blue_sat", -100.0)));
let spread = |p: [u8; 4]| p[2].abs_diff(p[0]);
assert_eq!(
spread(reds_turn),
spread(neutral),
"a blue pixel is outside the red band, so red must leave it alone"
);
assert!(
spread(blues_turn) + 8 < spread(neutral),
"blue at -100 must desaturate a blue pixel: {neutral:?} -> {blues_turn:?}"
);
}
/// A demosaiced image whose pixels sit at the centre of the blue band.
///
/// Red and green equal, blue well above them, which `rgb_to_hcl` reads as
/// exactly 240 degrees — full weight to blue, none to any other band.
fn blue_image(ctx: &GpuContext) -> DemosaicedImage {
let size = 16u32;
let mut data = vec![0u16; (size * size) as usize];
for y in 0..size {
for x in 0..size {
let c = CfaPattern::Rggb.colour_at(x, y);
data[(y * size + x) as usize] = if c == 2 { 12000 } else { 3000 };
}
}
let raw = RawImage {
width: size,
height: size,
data,
cfa_pattern: CfaPattern::Rggb,
black_level: [0; 4],
white_level: 16383,
wb_coeffs: [1.0, 1.0, 1.0, 1.0],
color_matrix: Some([1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0]),
crop: CropRect {
x: 0,
y: 0,
width: size,
height: size,
},
};
Demosaicer::new(ctx)
.expect("demosaicer")
.run(&raw)
.expect("demosaic")
}
#[test]
fn moving_a_slider_does_not_recompile() {
// The property the pipeline cache exists for. Recompiling per frame
@@ -1135,12 +1246,9 @@ mod tests {
/// TRACES: FR-DSP-1 | AC-8
/// A disc on a non-square frame, rendered through a quarter turn.
///
/// Geometry, asserted on real pixels rather than on the generated WGSL.
/// A circle is the one shape that makes anisotropy unmissable: any
/// transform that scales the axes unequally turns it into an ellipse, and
/// the ratio of the ellipse's axes *is* the error. Reading the shader and
/// reasoning about which space `p` lives in is exactly how a plausible
/// formula gets written twice.
/// Geometry, asserted on real pixels rather than on the generated WGSL —
/// reading the shader and reasoning about which space `p` lives in is
/// exactly how a plausible formula gets written twice.
#[test]
fn a_quarter_turn_keeps_a_circle_circular() {
let Some(ctx) = ctx() else { return };
@@ -1148,21 +1256,7 @@ mod tests {
// 3:2, so an aspect mistake is a 2.25x distortion rather than a
// subtlety. The disc is centred and comfortably inside the frame.
let (w, h) = (180u32, 120u32);
let radius = 40.0f32;
let mut rgba = vec![0u8; (w * h * 4) as usize];
for y in 0..h {
for x in 0..w {
let dx = x as f32 - w as f32 / 2.0;
let dy = y as f32 - h as f32 / 2.0;
let inside = (dx * dx + dy * dy).sqrt() < radius;
let i = ((y * w + x) * 4) as usize;
let v = if inside { 255 } else { 0 };
rgba[i] = v;
rgba[i + 1] = v;
rgba[i + 2] = v;
rgba[i + 3] = 255;
}
}
let rgba = disc_rgba(w, h, 40.0);
let src = DemosaicedImage::from_rgba8(&ctx, &rgba, w, h).expect("upload");
let mut graph = dr_pipeline::EditGraph::default_chain();
@@ -1200,24 +1294,7 @@ mod tests {
let Some(ctx) = ctx() else { return };
let (w, h) = (180u32, 120u32);
let radius = 34.0f32;
let mut rgba = vec![0u8; (w * h * 4) as usize];
for y in 0..h {
for x in 0..w {
let dx = x as f32 - w as f32 / 2.0;
let dy = y as f32 - h as f32 / 2.0;
let v = if (dx * dx + dy * dy).sqrt() < radius {
255
} else {
0
};
let i = ((y * w + x) * 4) as usize;
rgba[i] = v;
rgba[i + 1] = v;
rgba[i + 2] = v;
rgba[i + 3] = 255;
}
}
let rgba = disc_rgba(w, h, 34.0);
let src = DemosaicedImage::from_rgba8(&ctx, &rgba, w, h).expect("upload");
let mut graph = dr_pipeline::EditGraph::default_chain();
@@ -1243,6 +1320,59 @@ mod tests {
);
}
/// TRACES: FR-DEV-3 | FR-DSP-1
/// The same disc again, straightened *and* turned.
///
/// The case neither test above reaches, and the one a portrait photograph
/// hits every time. A quarter turn — the user's or the file's EXIF tag —
/// swaps the frame's axes, so the space the straightening happens in is no
/// longer the source's: measuring a 2:3 frame with a 3:2 aspect stretches
/// one axis against the other by 2.25, and the rotation that follows comes
/// out as a shear. Each transform alone looks right, which is exactly why
/// it survived: only the pair is wrong.
#[test]
fn straightening_a_turned_frame_keeps_a_circle_circular() {
let Some(ctx) = ctx() else { return };
let (w, h) = (180u32, 120u32);
let rgba = disc_rgba(w, h, 34.0);
let src = DemosaicedImage::from_rgba8(&ctx, &rgba, w, h).expect("upload");
// Every route to a swapped frame: the button, the file's tag, and the
// two composed. All three reach the shader as one permutation, and a
// fix that only covers one of them is not a fix.
for (name, turns, tag) in [
("a user quarter turn", 1, 1u16),
("an EXIF-portrait file", 0, 6),
("both, composed", 2, 6),
] {
let mut graph = dr_pipeline::EditGraph::default_chain();
graph.set_orientation(dr_types::Orientation::from_exif(tag));
graph.rotate_quarters(turns);
graph.set_param(dr_pipeline::framing::ID, dr_pipeline::framing::ANGLE, 20.0);
let (ow, oh) = graph.output_size(w, h);
assert_eq!((ow, oh), (h, w), "{name}: the frame should be portrait");
let mut pass = AdjustPass::new(&ctx);
pass.render(&src, &graph.compose(), ow, oh).expect("render");
let (pixels, rw, rh) = pass.export_pixels().expect("read back");
let lit = |x: u32, y: u32| pixels[((y * rw + x) * 4) as usize] > 128;
let across = (0..rw).filter(|&x| lit(x, rh / 2)).count();
let down = (0..rh).filter(|&y| lit(rw / 2, y)).count();
assert!(across > 0 && down > 0, "{name}: the disc vanished");
let ratio = across as f32 / down as f32;
assert!(
(ratio - 1.0).abs() < 0.08,
"{name}: a straightened circle came back {across} across by \
{down} down (ratio {ratio:.3}); the turn and the angle are \
disagreeing about which frame they act in"
);
}
}
#[test]
fn the_output_is_importable_by_a_compositor() {
// Every condition Slint checks before it will adopt a texture