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>
324 lines
11 KiB
Rust
324 lines
11 KiB
Rust
//! Every path that produces pixels must apply the masks.
|
|
//!
|
|
//! The display, the export and the thumbnail run the same generated shader,
|
|
//! and that shader always emits a block per active mask layer. Bind the empty
|
|
//! placeholder instead of the real array and every one of those blocks
|
|
//! multiplies by zero: the local adjustments are simply absent, with no error
|
|
//! and no warning.
|
|
//!
|
|
//! That is what happened — exports and thumbnails both took the unmasked
|
|
//! path — and it is invisible from inside either one. The only way to see it
|
|
//! is to render the same edit twice and compare, which is what this does.
|
|
|
|
use dr_gpu::{AdjustPass, DemosaicedImage, GpuContext, MaskPass, SubjectMasks};
|
|
use dr_pipeline::descriptor::ParamId;
|
|
use dr_pipeline::mask::{MaskLayer, MaskSource, MaskStack};
|
|
use dr_pipeline::operation::compose_full;
|
|
use dr_pipeline::{ops, Framing};
|
|
use dr_segment::Shaped;
|
|
use dr_types::ColourSpace;
|
|
|
|
const PROXY: u32 = 24;
|
|
|
|
fn ctx() -> Option<GpuContext> {
|
|
pollster::block_on(GpuContext::new_headless()).ok()
|
|
}
|
|
|
|
fn grey(ctx: &GpuContext, size: u32) -> DemosaicedImage {
|
|
let data: Vec<u8> = (0..size * size).flat_map(|_| [128, 128, 128, 255]).collect();
|
|
DemosaicedImage::from_rgba8(ctx, &data, size, size).expect("upload")
|
|
}
|
|
|
|
/// Coverage over the left half, as a detection would produce.
|
|
fn left_half() -> Vec<u8> {
|
|
(0..PROXY * PROXY)
|
|
.map(|i| if i % PROXY < PROXY / 2 { 255 } else { 0 })
|
|
.collect()
|
|
}
|
|
|
|
fn brightening_stack() -> MaskStack {
|
|
let mut stack = MaskStack::new();
|
|
let mut layer = MaskLayer::new(
|
|
"m1",
|
|
MaskSource::Subject {
|
|
signature: 1,
|
|
index: 0,
|
|
class: "person".into(),
|
|
score: 0.9,
|
|
},
|
|
);
|
|
layer.set_param("exposure", ParamId("exposure"), 2.0);
|
|
layer.feather = 0.0;
|
|
stack.push(layer);
|
|
stack
|
|
}
|
|
|
|
/// Render `stack` at `out` pixels, exactly as the session's shared helper
|
|
/// does: fields, array, then a masked render.
|
|
fn render_at(ctx: &GpuContext, stack: &MaskStack, out: u32) -> Vec<u8> {
|
|
let source = grey(ctx, 64);
|
|
|
|
let coverage = left_half();
|
|
let fields: Vec<Vec<f32>> = stack
|
|
.active()
|
|
.map(|_| {
|
|
Shaped::build(
|
|
&coverage,
|
|
PROXY as usize,
|
|
PROXY as usize,
|
|
128,
|
|
dr_segment::Morphology::None,
|
|
0.0,
|
|
)
|
|
.distance
|
|
})
|
|
.collect();
|
|
let refs: Vec<&[f32]> = fields.iter().map(|f| f.as_slice()).collect();
|
|
let subjects = SubjectMasks::upload(ctx, &refs, PROXY, PROXY).expect("upload");
|
|
|
|
let mut masks = MaskPass::new(ctx).expect("mask pass");
|
|
let array = masks
|
|
.render(stack, None, Some(&subjects), PROXY, PROXY)
|
|
.expect("rasterise");
|
|
|
|
let shader = compose_full(&ops::chain(), &Framing::new(), ColourSpace::Srgb, stack);
|
|
let mut adjust = AdjustPass::new(ctx);
|
|
adjust
|
|
.render_masked(&source, &shader, out, out, Some(array))
|
|
.expect("render");
|
|
adjust.export_pixels().expect("readback").0
|
|
}
|
|
|
|
/// The same edit rendered *without* the array bound — what export and
|
|
/// thumbnail were doing.
|
|
fn render_unmasked(ctx: &GpuContext, stack: &MaskStack, out: u32) -> Vec<u8> {
|
|
let source = grey(ctx, 64);
|
|
let shader = compose_full(&ops::chain(), &Framing::new(), ColourSpace::Srgb, stack);
|
|
let mut adjust = AdjustPass::new(ctx);
|
|
adjust.render(&source, &shader, out, out).expect("render");
|
|
adjust.export_pixels().expect("readback").0
|
|
}
|
|
|
|
fn luma(pixels: &[u8], size: u32, x: u32, y: u32) -> u8 {
|
|
pixels[((y * size + x) * 4) as usize]
|
|
}
|
|
|
|
/// The fault itself, stated as a test: the two paths must not agree.
|
|
///
|
|
/// If binding the array made no difference, the masks would not be reaching
|
|
/// the shader at all — which is precisely the bug this file exists for.
|
|
#[test]
|
|
fn binding_the_masks_changes_the_result() {
|
|
let Some(ctx) = ctx() else {
|
|
eprintln!("no adapter; skipping");
|
|
return;
|
|
};
|
|
let stack = brightening_stack();
|
|
const OUT: u32 = 64;
|
|
|
|
let masked = render_at(&ctx, &stack, OUT);
|
|
let unmasked = render_unmasked(&ctx, &stack, OUT);
|
|
|
|
let left_masked = luma(&masked, OUT, 8, 32);
|
|
let left_unmasked = luma(&unmasked, OUT, 8, 32);
|
|
|
|
assert!(
|
|
left_masked > left_unmasked + 40,
|
|
"with the array bound the masked half must brighten: {left_masked} \
|
|
against {left_unmasked}"
|
|
);
|
|
assert!(
|
|
(120..=136).contains(&left_unmasked),
|
|
"and without it the layer contributes nothing at all, silently: \
|
|
{left_unmasked}"
|
|
);
|
|
}
|
|
|
|
/// A thumbnail is the same edit at a small size, so it must carry the same
|
|
/// local adjustments. This is the reported bug.
|
|
#[test]
|
|
fn a_thumbnail_sized_render_still_carries_its_masks() {
|
|
let Some(ctx) = ctx() else {
|
|
eprintln!("no adapter; skipping");
|
|
return;
|
|
};
|
|
let stack = brightening_stack();
|
|
|
|
for out in [16u32, 32, 64, 128] {
|
|
let pixels = render_at(&ctx, &stack, out);
|
|
let inside = luma(&pixels, out, out / 8, out / 2);
|
|
let outside = luma(&pixels, out, out - out / 8 - 1, out / 2);
|
|
|
|
assert!(
|
|
inside > outside + 40,
|
|
"at {out}px the masked half should be brighter: {inside} against \
|
|
{outside}"
|
|
);
|
|
}
|
|
}
|
|
|
|
/// One mask array, every output size. The array is rasterised in source space
|
|
/// and sampled through the framing map, so a thumbnail and a full-size export
|
|
/// must reach the same *proportion* of the frame — not merely both be
|
|
/// non-empty.
|
|
#[test]
|
|
fn the_mask_covers_the_same_fraction_at_every_size() {
|
|
let Some(ctx) = ctx() else {
|
|
eprintln!("no adapter; skipping");
|
|
return;
|
|
};
|
|
let stack = brightening_stack();
|
|
|
|
let fraction = |out: u32| {
|
|
let pixels = render_at(&ctx, &stack, out);
|
|
let lit = (0..out * out)
|
|
.filter(|i| pixels[(*i as usize) * 4] > 150)
|
|
.count();
|
|
lit as f32 / (out * out) as f32
|
|
};
|
|
|
|
let small = fraction(32);
|
|
let large = fraction(128);
|
|
assert!(
|
|
(small - large).abs() < 0.05,
|
|
"the same edit should mask the same share of the frame at any size: \
|
|
{small:.3} at 32px against {large:.3} at 128px"
|
|
);
|
|
assert!(
|
|
(0.4..0.6).contains(&small),
|
|
"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"
|
|
);
|
|
}
|