Give a gradient the angle it was asked for

A linear mask at 45° was not at 45°, and a radial with equal radii was an
ellipse. Both on every photograph that is not square, which is all of them.

The geometry is stored in normalised coordinates so that a mask survives a
crop, a zoom and an export at another size — that part was right. What was
wrong is that a *distance* was being measured in those coordinates too, and a
fraction of the width is not the same length as a fraction of the height. So
`dot(uv - centre, axis)` measured the ramp in a space one of whose axes is
squashed against the other by the aspect ratio, and the iso-lines came out
sheared: on a 3:2 frame a ramp asked for at 45° arrives at about 34°.

Nothing announces it. The stored numbers are exactly what was written, the
shader is doing exactly what it says, and the only place the fault exists is
between the photographer's intent and the picture. It has been invisible so far
because there is no way yet to place a gradient by eye — the handles that make
it visible are what turned it up.

So distances and angles move into the frame's own isotropic units: y spans
`0..1` and x spans `0..aspect`, which makes a circle round and 45° a real
diagonal. The centre stays a plain fraction of each axis, because it is a point
and a point has no such problem — and because that is the space a click arrives
in. `frame_delta` is the one conversion and must stay the only one; the mask
array's own dimensions carry the aspect, so it costs no uniform.

The sidecar format does not change. What changes is what the numbers mean, and
the only geometry in the wild is a default that has never been movable.

The two tests are at 96×64 rather than square, which is the whole point: on a
square target this bug cannot be reproduced, and every existing mask test was
square. Both fail without the conversion — the radial reaching 28px sideways
where it reaches 19px down, and the diagonal landing on the wrong side of the
line it is supposed to lie along.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

core/dr-gpu/src/shaders/mask.wgsl

@@ -39,7 +39,9 @@ struct MaskParams {
     // `falloff_code` on the Rust side.
     falloff: u32,
 
-    // Geometry, in normalised output coordinates. Meaning depends on `mode`.
+    // Geometry. Meaning depends on `mode`. The centre is in normalised 0..1
+    // coordinates; every distance below it is in the isotropic frame units
+    // `frame_delta` establishes.
     centre: vec2<f32>,
     // Linear: (cos, sin) of the ramp direction. Radial: semi-axes.
     axis: vec2<f32>,
@@ -124,9 +126,23 @@ fn region_mask(px: vec2<i32>) -> f32 {
     return total / n;
 }
 
+// Offset from a gradient's centre, in the frame's own **isotropic** units:
+// y spans 0..1 and x spans 0..aspect, so a step of the same length means the
+// same distance whichever way it points.
+//
+// Without this the geometry lives in raw 0..1, where one axis is compressed
+// against the other by the aspect ratio — so a 45° ramp is not at 45° on
+// anything but a square frame, and a radial with equal radii draws an ellipse.
+// Both faults are invisible in the stored numbers and obvious the moment a
+// handle is dragged on a photograph, which is what this exists for.
+fn frame_delta(uv: vec2<f32>) -> vec2<f32> {
+    let aspect = vec2<f32>(f32(p.width) / f32(max(p.height, 1u)), 1.0);
+    return (uv - p.centre) * aspect;
+}
+
 fn linear_mask(uv: vec2<f32>) -> f32 {
     // Signed distance along the ramp direction, from the centre.
-    let d = dot(uv - p.centre, p.axis);
+    let d = dot(frame_delta(uv), p.axis);
     if (p.softness <= 0.0) {
         return select(0.0, 1.0, d >= 0.0);
     }
@@ -136,7 +152,7 @@ fn linear_mask(uv: vec2<f32>) -> f32 {
 fn radial_mask(uv: vec2<f32>) -> f32 {
     let ca = cos(-p.angle);
     let sa = sin(-p.angle);
-    let d = uv - p.centre;
+    let d = frame_delta(uv);
     // Into the ellipse's own frame, then normalised by its semi-axes so the
     // problem becomes a unit circle.
     let local = vec2<f32>(d.x * ca - d.y * sa, d.x * sa + d.y * ca);

core/dr-gpu/tests/masked_outputs.rs

@@ -189,3 +189,135 @@ fn the_mask_covers_the_same_fraction_at_every_size() {
         "and that share is the left half: {small:.3}"
     );
 }
+
+// ---------------------------------------------------------------------------
+// Gradient geometry
+//
+// The mask is rasterised over the source frame, whose two axes are not the
+// same length. A gradient measured in raw 0..1 fractions therefore means a
+// different distance horizontally than vertically — so a circle comes out an
+// ellipse and an angle is not the angle asked for. Neither shows in the stored
+// numbers, and both are what a photographer is looking straight at while
+// dragging a handle.
+// ---------------------------------------------------------------------------
+
+/// A flat grey frame at an arbitrary shape, brightened wherever `stack` covers.
+fn render_gradient(ctx: &GpuContext, stack: &MaskStack, w: u32, h: u32) -> Vec<u8> {
+    let data: Vec<u8> = (0..w * h).flat_map(|_| [128u8, 128, 128, 255]).collect();
+    let source = DemosaicedImage::from_rgba8(ctx, &data, w, h).expect("upload");
+
+    let mut masks = MaskPass::new(ctx).expect("mask pass");
+    let array = masks.render(stack, None, None, w, h).expect("rasterise");
+
+    let shader = compose_full(&ops::chain(), &Framing::new(), ColourSpace::Srgb, stack);
+    let mut adjust = AdjustPass::new(ctx);
+    adjust
+        .render_masked(&source, &shader, w, h, Some(array))
+        .expect("render");
+    adjust.export_pixels().expect("readback").0
+}
+
+fn brightened(pixels: &[u8], w: u32, x: u32, y: u32) -> bool {
+    pixels[((y * w + x) * 4) as usize] > 160
+}
+
+fn gradient(source: MaskSource) -> MaskStack {
+    let mut stack = MaskStack::new();
+    let mut layer = MaskLayer::new("g1", source);
+    layer.set_param("exposure", ParamId("exposure"), 2.0);
+    stack.push(layer);
+    stack
+}
+
+/// A radial with equal radii must be round on the screen, not on the numbers.
+#[test]
+fn a_radial_with_equal_radii_is_a_circle_on_a_wide_frame() {
+    let Some(ctx) = ctx() else {
+        eprintln!("no adapter; skipping");
+        return;
+    };
+    // 3:2. On a square frame this test cannot fail, which is why it is not one.
+    const W: u32 = 96;
+    const H: u32 = 64;
+
+    // 0.3 of the frame's height. In pixels that is 19 either way — the x axis
+    // spans 0..1.5 in the same units, so the fraction is smaller and the
+    // distance is the same.
+    let pixels = render_gradient(
+        &ctx,
+        &gradient(MaskSource::Radial {
+            centre: (0.5, 0.5),
+            radii: (0.3, 0.3),
+            angle: 0.0,
+            // Hard, so "covered" is a question with an answer rather than a
+            // ramp to pick a threshold out of.
+            feather: 0.0,
+        }),
+        W,
+        H,
+    );
+
+    let (cx, cy) = (W / 2, H / 2);
+    assert!(brightened(&pixels, W, cx, cy), "the centre must be covered");
+
+    for (dx, dy, what) in [(15u32, 0u32, "right"), (0, 15, "down")] {
+        assert!(
+            brightened(&pixels, W, cx + dx, cy + dy),
+            "15px {what} of centre is inside a 19px radius"
+        );
+    }
+    for (dx, dy, what) in [(24u32, 0u32, "right"), (0, 24, "down")] {
+        assert!(
+            !brightened(&pixels, W, cx + dx, cy + dy),
+            "24px {what} of centre is outside it — before the aspect \
+             correction the horizontal reach was 28px and this passed only \
+             downwards"
+        );
+    }
+}
+
+/// And a ramp at 45° must be at 45° where the photographer sees it.
+#[test]
+fn a_diagonal_ramp_runs_at_the_angle_it_was_given() {
+    let Some(ctx) = ctx() else {
+        eprintln!("no adapter; skipping");
+        return;
+    };
+    const W: u32 = 96;
+    const H: u32 = 64;
+
+    let pixels = render_gradient(
+        &ctx,
+        &gradient(MaskSource::Linear {
+            centre: (0.5, 0.5),
+            angle: std::f32::consts::FRAC_PI_4,
+            width: 0.0,
+        }),
+        W,
+        H,
+    );
+
+    // Coverage increases along (cos 45°, sin 45°), so the half-way line runs
+    // from lower-left to upper-right through the centre: equal steps right and
+    // down stay on the covered side, and the two sides of it disagree.
+    let (cx, cy) = (W / 2, H / 2);
+    assert!(
+        brightened(&pixels, W, cx + 16, cy + 16),
+        "down and to the right of the line is inside"
+    );
+    assert!(
+        !brightened(&pixels, W, cx - 16, cy - 16),
+        "up and to the left of it is outside"
+    );
+    // The diagonal itself, sampled a few pixels either side. Without the
+    // aspect correction the line comes out at 34 degrees and both of these
+    // land on the same side of it.
+    assert!(
+        brightened(&pixels, W, cx + 18, cy - 14),
+        "just below the 45 degree line is inside"
+    );
+    assert!(
+        !brightened(&pixels, W, cx + 14, cy - 18),
+        "just above it is not"
+    );
+}

core/dr-pipeline/src/mask.rs

@@ -283,25 +283,44 @@ pub enum MaskSource {
 
     /// A linear gradient — the graduated-filter mask.
     ///
-    /// Geometry is in **normalised output coordinates**, so it survives a crop
-    /// or an export at another size. Storing pixels would make a mask that
-    /// silently moves when the frame changes.
+    /// Geometry is in **normalised source coordinates**, so it survives a crop,
+    /// a zoom or an export at another size: the mask is rasterised in source
+    /// space and sampled through the framing map, exactly as a subject mask is.
+    /// Storing pixels would make a mask that silently moves when the frame
+    /// changes.
+    ///
+    /// # Two spaces, and why both are here
+    ///
+    /// The **centre** is a point, so it is a plain `0.0..=1.0` fraction of each
+    /// axis — the same coordinates a click carries.
+    ///
+    /// The **angle and the width are measurements**, and a fraction of the
+    /// width is not the same length as a fraction of the height on any frame
+    /// that is not square. So they are in the frame's *isotropic* units: y
+    /// spans `0..1` and x spans `0..aspect`, which is what makes 45° actually
+    /// 45° and a circle actually round. `frame_delta` in `mask.wgsl` is the
+    /// one place that conversion happens, and it must stay the only one.
     Linear {
         /// Midpoint of the ramp, `0.0..=1.0` in each axis.
         centre: (f32, f32),
-        /// Radians, measured from the +x axis.
+        /// Radians, measured clockwise from the +x axis in frame units.
+        /// Coverage increases in this direction.
         angle: f32,
-        /// Distance from full effect to none, in normalised units. Zero is a
-        /// hard edge.
+        /// Distance from full effect to none, in frame units. Zero is a hard
+        /// edge.
         width: f32,
     },
 
     /// A radial gradient — the classic vignette-shaped local adjustment.
+    ///
+    /// The same two spaces as [`Self::Linear`]: a `0..1` centre, and semi-axes
+    /// measured in the frame's isotropic units so equal radii draw a circle.
     Radial {
         centre: (f32, f32),
-        /// Semi-axes, normalised. Two of them, because a face is an ellipse
-        /// and forcing a circle makes the user compensate with a crop.
+        /// Semi-axes, in frame units. Two of them, because a face is an
+        /// ellipse and forcing a circle makes the user compensate with a crop.
         radii: (f32, f32),
+        /// Rotation of the ellipse, radians.
         angle: f32,
         /// Fraction of the radius over which the edge falls off.
         feather: f32,

core/dr-pipeline/tests/mask_sidecar.rs

@@ -421,7 +421,10 @@ fn show_a_sidecar() {
         "m2",
         MaskSource::Linear {
             centre: (0.5, 0.25),
-            angle: 1.5708,
+            // The constant rather than four digits of it: clippy rejects the
+            // literal, and a quarter turn written as a number is a quarter
+            // turn nobody can see at a glance.
+            angle: std::f32::consts::FRAC_PI_2,
             width: 0.4,
         },
     );