From 654c11300e621e2d2e752f77c773ab51c8de7685 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Mon, 17 Aug 2026 12:46:17 +0200 Subject: [PATCH] Pin the framing geometry on pixels, not on the generated shader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Chasing a reported shear on rotate and straighten. Two tests, and what they prove is that the pipeline is not where it comes from. A circle is the shape that makes anisotropy unmissable: any transform scaling the axes unequally returns an ellipse, and the ratio of its axes is the error. Both a quarter turn and a 20° straighten, on a 3:2 frame, return a circle within 8%. Worth recording because I had a confident and wrong hypothesis first. The quarter turn carries `* aspect.x` on one component and `/ aspect.x` on the other, which reads like an anisotropy of a-squared, and the reasoning that `p` is already isotropic is plausible enough that I changed it. The existing `a_quarter_turn_corrects_for_aspect_across_the_swap` caught that immediately, and these tests then showed the original was right all along: the crop rect is expressed in the *turned* frame and the output axes swap with it, so the factors are the conversion between those spaces rather than a mistake. Reading the shader and reasoning about which space `p` lives in is exactly how a plausible formula gets written twice. These assert on real pixels off a real adapter instead, so the next person to suspect this transform can rule it out in one command. The shear is therefore in the display path — the fit from the framed size to the viewport, or the crop overlay's uncropped render — and not in the geometry the pipeline computes. Not yet fixed. Co-Authored-By: Claude Opus 5 --- core/dr-gpu/src/adjust.rs | 110 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/core/dr-gpu/src/adjust.rs b/core/dr-gpu/src/adjust.rs index 1753f48..de75ced 100644 --- a/core/dr-gpu/src/adjust.rs +++ b/core/dr-gpu/src/adjust.rs @@ -1133,6 +1133,116 @@ 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. + #[test] + fn a_quarter_turn_keeps_a_circle_circular() { + let Some(ctx) = ctx() else { return }; + + // 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 src = DemosaicedImage::from_rgba8(&ctx, &rgba, w, h).expect("upload"); + let mut graph = dr_pipeline::EditGraph::default_chain(); + graph.rotate_quarters(1); + + let (ow, oh) = graph.output_size(w, h); + assert_eq!((ow, oh), (h, w), "a quarter turn swaps the output axes"); + + 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"); + + // Measure the disc's extent along each axis, at its centre. + 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, "the disc vanished: {across}x{down}"); + let ratio = across as f32 / down as f32; + assert!( + (ratio - 1.0).abs() < 0.08, + "a turned circle came back {across} across by {down} down \ + (ratio {ratio:.3}); anything but 1 is the frame being sheared" + ); + } + + /// The same disc, straightened by a free angle rather than turned. + /// + /// A rotation is rigid: a circle stays a circle at any angle. If the + /// straighten happens in a space whose axes carry different scales, the + /// circle comes back as an ellipse — and on a photograph that reads as the + /// frame being sheared. + #[test] + fn straightening_keeps_a_circle_circular() { + 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 src = DemosaicedImage::from_rgba8(&ctx, &rgba, w, h).expect("upload"); + let mut graph = dr_pipeline::EditGraph::default_chain(); + // A deliberate angle, not a nudge: a shear scales with the angle and + // a degree would hide inside the tolerance. + graph.set_param(dr_pipeline::framing::ID, dr_pipeline::framing::ANGLE, 20.0); + + let (ow, oh) = graph.output_size(w, h); + 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, "the disc vanished: {across}x{down}"); + let ratio = across as f32 / down as f32; + assert!( + (ratio - 1.0).abs() < 0.08, + "a straightened circle came back {across} across by {down} down \ + (ratio {ratio:.3}); a rotation is rigid, so anything but 1 is shear" + ); + } + #[test] fn the_output_is_importable_by_a_compositor() { // Every condition Slint checks before it will adopt a texture