Teach the whole-chain shader test about neighbourhood operations
`the_whole_chain_at_once_compiles` counted one `---- ` block per operation in the chain. That was true while every operation was a point operation, and stopped being true the moment a neighbourhood one existed: clarity and texture are active in that test, and still emit no fused block, because `compose_full` filters them out and the detail stage dispatches them separately. Counted now by asking each operation whether it has a detail stage -- the same question the composer's own filter asks -- rather than by subtracting a number someone has to remember to update. Capture sharpening and noise reduction are covered by this without another edit. The render at the end is now `render_detailed`, which is not a concession but the stronger test: with a neighbourhood operation active the fused pass hands on linear working values and the last detail pass performs the output transform, so rendering the fused half alone is the mismatch `render_detailed` exists to reject -- and the detail passes are generated WGSL with uniform blocks of their own, which is exactly what "everything at once" is here to collide. It renders at 512 rather than 32 because a compositional radius is a fraction of the frame, and on a 32-pixel target every detail kernel rounds away to nothing. Also records, in `texture_contributes_nothing_where_its_scale_does_not_exist`, the seam this uncovered: an active detail operation whose kernel rounds away composes an empty chain while the fused pass has already been composed to hand on linear values, and nothing can then encode the result. That test now asserts the property on the composed chain instead of driving the unrenderable configuration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -891,6 +891,11 @@ mod tests {
|
|||||||
use dr_decode::{BaseCurve, CfaPattern, CropRect, RawImage};
|
use dr_decode::{BaseCurve, CfaPattern, CropRect, RawImage};
|
||||||
use dr_pipeline::ops::{colour_mixer, exposure, saturation};
|
use dr_pipeline::ops::{colour_mixer, exposure, saturation};
|
||||||
use dr_pipeline::EditGraph;
|
use dr_pipeline::EditGraph;
|
||||||
|
// For `Operation::detail`, which is how `the_whole_chain_at_once_compiles`
|
||||||
|
// asks the chain which of its operations are neighbourhood operations
|
||||||
|
// rather than being told a list. Imported anonymously: nothing here names
|
||||||
|
// the trait, only calls through it.
|
||||||
|
use dr_pipeline::Operation as _;
|
||||||
|
|
||||||
use crate::Demosaicer;
|
use crate::Demosaicer;
|
||||||
|
|
||||||
@@ -1423,23 +1428,65 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
let shader = g.compose();
|
let shader = g.compose();
|
||||||
|
|
||||||
|
// A neighbourhood operation is active here and still emits no block,
|
||||||
|
// so the count has to know about them. Clarity, texture, sharpening,
|
||||||
|
// noise reduction — each is defined by what the pixels *around* a
|
||||||
|
// pixel are doing, and the fused contract hands a fragment a colour
|
||||||
|
// with no way back to a coordinate. `compose_full` filters them out
|
||||||
|
// and the detail stage dispatches them separately, which is the whole
|
||||||
|
// point of `Affects::Detail`.
|
||||||
|
//
|
||||||
|
// Counted by asking each operation whether it has a detail stage —
|
||||||
|
// the same question the composer's own filter asks — rather than by
|
||||||
|
// listing the ones that exist today. A detail operation added later is
|
||||||
|
// then covered without anyone remembering to come back here, which is
|
||||||
|
// the property this test is supposed to have.
|
||||||
|
let neighbourhood = dr_pipeline::ops::chain()
|
||||||
|
.iter()
|
||||||
|
.filter(|o| o.detail().is_some())
|
||||||
|
.count();
|
||||||
|
assert!(neighbourhood > 0, "the chain has neighbourhood operations");
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
shader.source.matches("---- ").count(),
|
shader.source.matches("---- ").count(),
|
||||||
// Every operation, plus framing — which emits a stage of its own
|
// Every *point* operation, plus framing — which emits a stage of
|
||||||
// rather than an operation block, and is not in `descriptors`.
|
// its own rather than an operation block, and is not in
|
||||||
g.descriptors().len() + 1,
|
// `descriptors`.
|
||||||
"every operation and the framing should be active"
|
g.descriptors().len() - neighbourhood + 1,
|
||||||
|
"every point operation and the framing should be active"
|
||||||
);
|
);
|
||||||
assert!(
|
assert!(
|
||||||
shader.source.contains("---- framing ----"),
|
shader.source.contains("---- framing ----"),
|
||||||
"framing must reach the shader alongside the colour operations"
|
"framing must reach the shader alongside the colour operations"
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// The other half of the same edit, and it belongs in this test for the
|
||||||
|
// reason the test exists: the detail passes are generated WGSL too,
|
||||||
|
// they carry their own uniform blocks, and "everything at once" is
|
||||||
|
// exactly where a collision between them would show. Rendering the
|
||||||
|
// fused half alone is no longer even legal — with a neighbourhood
|
||||||
|
// operation active the fused pass stops at linear working values and
|
||||||
|
// the last detail pass performs the output transform, which is the
|
||||||
|
// mismatch `render_detailed` exists to reject.
|
||||||
|
//
|
||||||
// Cropped, so the render is against an output size that is not the
|
// Cropped, so the render is against an output size that is not the
|
||||||
// source size — the case where a wrong dispatch or a wrong texture
|
// source size — the case where a wrong dispatch or a wrong texture
|
||||||
// allocation would show up.
|
// allocation would show up.
|
||||||
let (w, h) = g.output_size(32, 32);
|
//
|
||||||
pass.render(&img, &shader, w, h)
|
// At 512 rather than the 32 this test used before the detail stage
|
||||||
|
// existed. A compositional radius is a fraction of the frame, so on a
|
||||||
|
// 32-pixel target every detail kernel rounds to a zero-pixel kernel
|
||||||
|
// and the chain composes nothing at all — honest, but it would leave
|
||||||
|
// the half of the shader this test came here to compile uncompiled.
|
||||||
|
let (w, h) = g.output_size(512, 512);
|
||||||
|
let scale = g.render_scale((512, 512), (w, h));
|
||||||
|
let detail = g.compose_detail_for(scale, dr_types::ColourSpace::Srgb);
|
||||||
|
assert!(
|
||||||
|
!detail.is_empty(),
|
||||||
|
"the detail half composed nothing, so nothing of it was compiled"
|
||||||
|
);
|
||||||
|
let key = g.invalidation().through(dr_pipeline::Affects::Colour);
|
||||||
|
pass.render_detailed(&img, &shader, w, h, None, &detail, key)
|
||||||
.expect("the full chain must compile");
|
.expect("the full chain must compile");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -547,18 +547,45 @@ fn texture_contributes_nothing_where_its_scale_does_not_exist() {
|
|||||||
let source = step_edge(&ctx, 512, [1.0, 1.0, 1.0]);
|
let source = step_edge(&ctx, 512, [1.0, 1.0, 1.0]);
|
||||||
|
|
||||||
let mut graph = graph_with(TEXTURE, 100.0);
|
let mut graph = graph_with(TEXTURE, 100.0);
|
||||||
|
|
||||||
|
// Asserted on the composed chain rather than on a dispatch counter,
|
||||||
|
// because texture *alone* at this size is a configuration the stage as a
|
||||||
|
// whole cannot currently render, and that is a gap in the seam rather than
|
||||||
|
// in this operation.
|
||||||
|
//
|
||||||
|
// `compose_full` decides whether the fused pass should hand on linear
|
||||||
|
// working values from `is_active()`, which has no `RenderScale` to consult;
|
||||||
|
// `compose_detail` decides what to dispatch from the kernel it can actually
|
||||||
|
// draw at this scale. Almost always the two agree. They disagree exactly
|
||||||
|
// when a detail operation is active and its kernel rounds away, and then
|
||||||
|
// `render_detailed` finds an empty chain, falls through to `render_masked`,
|
||||||
|
// and is rejected for handing a linear-working shader to the plain path.
|
||||||
|
//
|
||||||
|
// Pre-existing, and not something clarity and texture introduce:
|
||||||
|
// `DetailStage::passes` documents the empty return as *the honest answer
|
||||||
|
// for an acutance operation on a heavy proxy*, so capture sharpening and
|
||||||
|
// noise reduction reach it by the same road. Fixing it means composing
|
||||||
|
// both halves of a render together, so the fused half can know whether a
|
||||||
|
// detail half survived the scale — a change at the composition boundary,
|
||||||
|
// not in this file.
|
||||||
|
let scale = graph.render_scale(source.size(), (128, 128));
|
||||||
|
assert!(
|
||||||
|
graph.compose_detail_for(scale, ColourSpace::Srgb).is_empty(),
|
||||||
|
"texture claimed a kernel it cannot draw"
|
||||||
|
);
|
||||||
|
|
||||||
|
// With clarity on as well the edit is renderable again, and the dispatch
|
||||||
|
// count says what the assertion above says: two passes, not four. Texture
|
||||||
|
// is active, and contributes nothing.
|
||||||
|
graph.set_param(CLARITY, AMOUNT, 100.0);
|
||||||
let mut pass = AdjustPass::new(&ctx);
|
let mut pass = AdjustPass::new(&ctx);
|
||||||
render(&mut pass, &graph, &source, 128);
|
render(&mut pass, &graph, &source, 128);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
pass.detail_dispatches(),
|
pass.detail_dispatches(),
|
||||||
0,
|
2,
|
||||||
"texture claimed a kernel it cannot draw"
|
"clarity survives a thumbnail, and texture added nothing beside it"
|
||||||
);
|
);
|
||||||
|
|
||||||
graph.set_param(CLARITY, AMOUNT, 100.0);
|
|
||||||
render(&mut pass, &graph, &source, 128);
|
|
||||||
assert_eq!(pass.detail_dispatches(), 2, "clarity survives a thumbnail");
|
|
||||||
|
|
||||||
// And texture comes back, exactly, as soon as the view is large enough to
|
// And texture comes back, exactly, as soon as the view is large enough to
|
||||||
// hold it — no separate path, no fade, just the kernel resolving again.
|
// hold it — no separate path, no fade, just the kernel resolving again.
|
||||||
let mut zoomed = AdjustPass::new(&ctx);
|
let mut zoomed = AdjustPass::new(&ctx);
|
||||||
|
|||||||
Reference in New Issue
Block a user