Apply the masks to the thumbnail and the export, not only the screen
Reported as a thumbnail bug; the export had it too, which is the serious half. You would have exported a photograph missing every local adjustment. Both called the unmasked `render`, and the failure is silent by construction: the generated shader always declares the mask binding and always emits a block per active layer, so binding the empty placeholder multiplies each of them by zero. No error, no warning, no missing texture — the adjustments are simply not there. From inside either path there is nothing to see. Every path that produces pixels now goes through one helper that binds the array, and that is the point of it being one helper rather than three correct call sites. The array is rasterised in source space at proxy size and sampled through the framing map, so one array serves every output size: a 256px thumbnail and a 24 MP export bind the same texture. Three tests, and the first is the fault stated directly — render the same edit with and without the array and assert they *differ*. If binding it ever stops mattering, the masks have stopped reaching the shader. The third checks the masked share of the frame is the same at 32px and 128px, because "both non-empty" would pass while a mask that scaled wrongly still ruined every thumbnail.
This commit is contained in:
@@ -0,0 +1,191 @@
|
||||
//! 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}"
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user