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

Name the two spaces a photograph lives in, so a turn cannot go the wrong way

Browse Source 
Every orientation bug this codebase has had has been the same bug: a turn of
the right size applied in the wrong direction. That failure is worth naming
precisely, because it does not look like one — a quarter turn applied
backwards lands 180 degrees from right, so the result is a plausible
transform of the picture rather than anything obviously broken, and on
landscape frames it is not wrong at all. It was the straighten shear, and it
was the segmentation overlay, and each time it was found by eye rather than
by a test.

The reason it keeps happening is that "rotate 90 degrees clockwise" cannot be
checked by reading it. The reader has to hold in their head which of the two
images is being rotated and which way the y axis runs, and there were four
hand-written copies of the permutation to hold it for: the shader prologue,
its CPU twin, the thumbnail path, and the segmentation.

So nothing added here says clockwise, anticlockwise, horizontal or vertical.
The functions say *which space they take and which space they return* —
`into_shown` and `into_stored`, `source_pixel` and `shown_pixel`,
`into_shown_rect` and `into_stored_rect` — and each takes the dimensions of
the space it reads from, so no caller has to work out which pair it is
holding. `StoredRect` and `ShownRect` are separate types because they are the
same four numbers meaning different things, which is exactly the case where a
mistake is silent: a shown rect measured against stored dimensions produces a
rectangle in the wrong place, not an error.

Underneath there is one permutation. `source_pixel` was already shared by the
prologue and the thumbnails; `source_point` is its normalised twin, written
beside it so the two cannot drift, and everything else is those two read
forwards or backwards. `Orientation::inverse` is the group inverse rather
than `4 - turns`: mirrors apply after the turn, so undoing means undoing them
first, and a mirror seen from the far side of an odd turn is about the other
axis. That is the diagonal-mirror case, tags 5 and 7, and getting it wrong
renders as — again — 180 degrees.

Three call sites lose their own copy: the thumbnail path, `dr-ui`'s
segmentation, and `dr-gpu`'s `local` example. "Upright" now means one thing
across the application rather than one thing per caller.

The gate that matters most is `the_render_and_the_orientation_map_agree`. The
shader prologue and `Orientation` answer the same question by different
routes, and until now nothing checked that they answered it the same way. It
now checks every EXIF tag against every user rotation and mirror on top of
it, because the composition is where the two could agree singly and disagree
together.

The rest earn their place by having caught something. Writing these found two
real errors in this commit's own new code before it ran anywhere: `shown_pixel`
was handed the dimensions of the wrong space and overflowed, and the rect map
turned the wrong way for the diagonal mirrors. A round trip that returns what
went in is the only check worth having here, since every wrong answer is
still a picture.

No behaviour changes. The permutations are the ones that were already being
applied; they are simply applied from one place now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-27 09:46:24 +02:00
co-authored by Claude Opus 5
parent d7a81375ee
commit ce201c7dd6
6 changed files with 473 additions and 170 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/segmentation.rs
+39 -93
View File
@@ -333,35 +333,18 @@ pub fn compute(
/// 0.36` and nothing else, against `dog 0.82, person 0.61, person 0.49` for
/// the same pixels stood up.
///
/// The turn is [`Orientation::source_pixel`], which is the function the grid's
/// thumbnails already go through (`dr_decode::Preview::apply_orientation`) —
/// so the detector now reads exactly the kind of image the thumbnailer makes,
/// rather than a second opinion about what "upright" means.
///
/// A quarter turn and its mirrors are a permutation of the pixel grid, so this
/// is exact: no filter, no resampling, and no edge softened on the way in that
/// would have to be judged on the way out.
/// One line, because the permutation belongs to
/// [`dr_types::Orientation`] and every other consumer goes through the same
/// one — the grid's thumbnails included, which is what makes "upright" mean
/// one thing across the application rather than one thing per caller.
pub(crate) fn upright(
rgb: &[f32],
width: usize,
height: usize,
orientation: Orientation,
) -> (Vec<f32>, usize, usize) {
if orientation.is_normal() || width == 0 || height == 0 {
return (rgb.to_vec(), width, height);
}
let (dw, dh) = oriented(width, height, orientation);
let mut out = vec![0.0f32; dw * dh * 3];
for y in 0..dh {
for x in 0..dw {
let (sx, sy) = orientation.source_pixel(x as u32, y as u32, dw as u32, dh as u32);
let s = (sy as usize * width + sx as usize) * 3;
let d = (y * dw + x) * 3;
out[d..d + 3].copy_from_slice(&rgb[s..s + 3]);
}
}
(out, dw, dh)
let (out, w, h) = orientation.into_shown(rgb, width as u32, height as u32, 3);
(out, w as usize, h as usize)
}
/// TRACES: FR-DEV-3
@@ -373,7 +356,7 @@ pub(crate) fn upright(
/// one and forgetting the other is silent — the mask lands a quarter turn off
/// the subject, which reads as a bad detection rather than as a bug.
///
/// `dw`/`dh` are the *upright* dimensions, as [`upright`] returned them.
/// `dw`/`dh` are the *shown* dimensions, as [`upright`] returned them.
pub(crate) fn lay_down(
mask: &[f32],
bbox: (f32, f32, f32, f32),
@@ -381,80 +364,42 @@ pub(crate) fn lay_down(
dh: usize,
orientation: Orientation,
) -> (Vec<f32>, (f32, f32, f32, f32)) {
if orientation.is_normal() {
return (mask.to_vec(), bbox);
}
(
lay_down_mask(mask, dw, dh, orientation),
lay_down_bbox(bbox, dw, dh, orientation),
)
let (out, sw, sh) = orientation.into_stored(mask, dw as u32, dh as u32, 1);
(out, lay_down_bbox(bbox, dw, dh, sw, sh, orientation))
}
/// The same permutation as [`upright`], read as a scatter rather than a
/// gather: a quarter turn is a bijection of the grid, so writing every upright
/// pixel to where it came from fills the sensor-space mask exactly once and
/// leaves no hole. Running the one function in the one direction is the point
/// — an inverse written out by hand is a second thing to keep in step, and its
/// way of being wrong is a mask mirrored about the wrong axis, which still
/// looks like a mask.
fn lay_down_mask(mask: &[f32], dw: usize, dh: usize, orientation: Orientation) -> Vec<f32> {
let (sw, sh) = oriented(dw, dh, orientation);
let mut out = vec![0.0f32; sw * sh];
for y in 0..dh {
for x in 0..dw {
let (sx, sy) = orientation.source_pixel(x as u32, y as u32, dw as u32, dh as u32);
out[sy as usize * sw + sx as usize] = mask[y * dw + x];
}
}
out
}
/// [`lay_down_mask`] for a box.
/// [`lay_down`] for a box.
///
/// The corners go through the same permutation in *continuous* coordinates —
/// `dw - x` where the pixel map says `dw - 1 - x`, because a pixel centre at
/// `x + 0.5` has to land at `dw - x - 0.5`. Then the extremes, since a turn
/// exchanges which corner is which and a box written `(x0, y0, x1, y1)` has to
/// keep `x0 <= x1`.
/// Normalised on the way in and scaled on the way out, so the turn itself is
/// `Orientation::into_stored_rect` rather than a fourth copy of the corner
/// arithmetic. A box is the one place a permutation can be *nearly* right —
/// the corners land correctly and `x0 > x1` — so the shared map takes the
/// extremes and this only has to say what space it is in.
fn lay_down_bbox(
bbox: (f32, f32, f32, f32),
dw: usize,
dh: usize,
sw: u32,
sh: u32,
orientation: Orientation,
) -> (f32, f32, f32, f32) {
let (sw, sh) = oriented(dw, dh, orientation);
let (dw, dh) = (dw as f32, dh as f32);
let (sw, sh) = (sw as f32, sh as f32);
let corner = |x: f32, y: f32| {
let (mut sx, mut sy) = match orientation.quarter_turns {
1 => (y, dw - x),
2 => (dw - x, dh - y),
3 => (dh - y, x),
_ => (x, y),
};
if orientation.flip_h {
sx = sw - sx;
}
if orientation.flip_v {
sy = sh - sy;
}
(sx, sy)
};
let (ax, ay) = corner(bbox.0, bbox.1);
let (bx, by) = corner(bbox.2, bbox.3);
(ax.min(bx), ay.min(by), ax.max(bx), ay.max(by))
}
/// The size those `width x height` pixels have once turned.
fn oriented(width: usize, height: usize, orientation: Orientation) -> (usize, usize) {
if orientation.swaps_axes() {
(height, width)
} else {
(width, height)
if dw == 0 || dh == 0 {
return bbox;
}
let (fw, fh) = (dw as f32, dh as f32);
let shown = dr_types::ShownRect {
x: bbox.0 / fw,
y: bbox.1 / fh,
width: (bbox.2 - bbox.0) / fw,
height: (bbox.3 - bbox.1) / fh,
};
let stored = orientation.into_stored_rect(shown);
(
stored.x * sw as f32,
stored.y * sh as f32,
(stored.x + stored.width) * sw as f32,
(stored.y + stored.height) * sh as f32,
)
}
/// One of eight transforms, as bits a signature can carry.
@@ -664,7 +609,7 @@ mod tests {
"tag {tag}: the upright size is the oriented one"
);
let back = lay_down_mask(&red(&up), uw, uh, o);
let (back, _) = lay_down(&red(&up), (0.0, 0.0, 1.0, 1.0), uw, uh, o);
assert_eq!(back, red(&source), "tag {tag} did not come back");
}
}
@@ -703,8 +648,8 @@ mod tests {
#[test]
fn a_box_comes_back_in_sensor_pixels() {
let o = Orientation::from_exif(6);
// Upright 4x6; the sensor it came from is 6x4.
let bbox = lay_down_bbox((0.0, 0.0, 2.0, 3.0), 4, 6, o);
// Shown 4x6; the sensor it came from is 6x4.
let bbox = lay_down_bbox((0.0, 0.0, 2.0, 3.0), 4, 6, 6, 4, o);
assert_eq!(bbox, (0.0, 2.0, 3.0, 4.0));
}
@@ -715,11 +660,12 @@ mod tests {
fn a_restored_box_keeps_its_corners_in_order() {
for tag in 1..=8u16 {
let o = Orientation::from_exif(tag);
let (x0, y0, x1, y1) = lay_down_bbox((1.0, 2.0, 7.0, 5.0), 9, 6, o);
let (sw, sh) = o.oriented_size(9, 6);
let (x0, y0, x1, y1) = lay_down_bbox((1.0, 2.0, 7.0, 5.0), 9, 6, sw, sh, o);
assert!(x0 <= x1, "tag {tag}: x runs backwards");
assert!(y0 <= y1, "tag {tag}: y runs backwards");
// A permutation moves a box; it does not resize one.
let (sw, sh) = oriented(9, 6, o);
let (sw, sh) = o.oriented_size(9, 6);
assert!(x1 <= sw as f32 && y1 <= sh as f32, "tag {tag}: box escaped");
assert!((((x1 - x0) * (y1 - y0)) - 18.0).abs() < 1e-3, "tag {tag}");
}