Finish the render when a kernel is too small to draw
An active detail operation may emit no pass at a given render scale - the honest answer for a sensor-sized radius on a heavy proxy. The fused composer cannot see that, having no resolution to consult, so it had already stopped short of the output transform and the frame died on a storage-format mismatch. Compose a bodyless resolve pass in that case so the output transform still happens exactly once.
This commit is contained in:
@@ -449,6 +449,48 @@ pub fn compose_detail(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// An active detail operation that emitted nothing at this scale.
|
||||||
|
//
|
||||||
|
// Legal, and the honest answer for an acutance operation on a heavy proxy
|
||||||
|
// — a one-source-pixel radius is a third of a render pixel there and no
|
||||||
|
// kernel represents a third of a pixel (see [`RenderScale`]). But it opens
|
||||||
|
// a hole between the two halves of the composition: [`compose_full`]
|
||||||
|
// decides to hand on linear working values from the *operations*, which it
|
||||||
|
// must, having no scale to consult, so the fused pass has already stopped
|
||||||
|
// short of the output transform. Returning an empty chain here would leave
|
||||||
|
// that transform undone and bind an `rgba16float` shader to an
|
||||||
|
// `rgba8unorm` target, which surfaces as a wgpu validation failure a long
|
||||||
|
// way from the cause.
|
||||||
|
//
|
||||||
|
// So the chain is never empty when the fused pass is expecting one: a
|
||||||
|
// single pass with no body, which reads the intermediate and performs the
|
||||||
|
// output transform the fused pass skipped. One dispatch, in the uncommon
|
||||||
|
// case where a photographer has a kernel switched on at a scale that
|
||||||
|
// cannot draw it — against the alternative of the preview failing outright
|
||||||
|
// or `compose_full` growing a resolution argument it has no other use for.
|
||||||
|
if planned.is_empty()
|
||||||
|
&& ops
|
||||||
|
.iter()
|
||||||
|
.any(|o| o.is_active() && o.detail().is_some())
|
||||||
|
{
|
||||||
|
return ComposedDetail {
|
||||||
|
passes: vec![compose_one(
|
||||||
|
RESOLVE_ID,
|
||||||
|
&[],
|
||||||
|
&DetailPass {
|
||||||
|
label: "resolve",
|
||||||
|
radius: 0,
|
||||||
|
wgsl: String::new(),
|
||||||
|
uniforms: Vec::new(),
|
||||||
|
},
|
||||||
|
0,
|
||||||
|
scale,
|
||||||
|
output,
|
||||||
|
true,
|
||||||
|
)],
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
let last = planned.len().saturating_sub(1);
|
let last = planned.len().saturating_sub(1);
|
||||||
let passes = planned
|
let passes = planned
|
||||||
.into_iter()
|
.into_iter()
|
||||||
@@ -461,6 +503,14 @@ pub fn compose_detail(
|
|||||||
ComposedDetail { passes }
|
ComposedDetail { passes }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The operation id the resolve pass is labelled with.
|
||||||
|
///
|
||||||
|
/// Not an operation: no `ops/*.yaml` declares it and nothing in the chain
|
||||||
|
/// answers to it. It exists so the generated label reads `detail/resolve`
|
||||||
|
/// rather than borrowing the id of whichever operation happened to fall
|
||||||
|
/// through, which would send a reader looking for a bug in that operation.
|
||||||
|
const RESOLVE_ID: &str = "detail";
|
||||||
|
|
||||||
#[allow(clippy::too_many_arguments)]
|
#[allow(clippy::too_many_arguments)]
|
||||||
fn compose_one(
|
fn compose_one(
|
||||||
id: &str,
|
id: &str,
|
||||||
@@ -880,6 +930,49 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn an_active_operation_that_draws_nothing_still_finishes_the_render() {
|
||||||
|
// The seam between the two composers, and the one case where they
|
||||||
|
// cannot see each other. `compose_full` decides to hand on linear
|
||||||
|
// working values from the *operations* — it has no resolution to
|
||||||
|
// consult — while this composer converts a radius and can legitimately
|
||||||
|
// decide there is nothing to draw at this size. An empty chain would
|
||||||
|
// then leave the output transform undone: the fused pass writes
|
||||||
|
// `rgba16float` and the frontend binds an `rgba8unorm` target to it.
|
||||||
|
//
|
||||||
|
// A photographer meets this by turning on capture sharpening or
|
||||||
|
// luminance noise reduction while the develop view is fitted to a
|
||||||
|
// large file, which is the normal way to work, so it is not an edge
|
||||||
|
// case that can be left to fail.
|
||||||
|
let ops = with_blur(0.001);
|
||||||
|
let scale = RenderScale::full((400, 400));
|
||||||
|
assert!(ops.last().expect("the blur").is_active());
|
||||||
|
assert_eq!(
|
||||||
|
BoxBlur::with_radius(0.001).passes(scale).len(),
|
||||||
|
0,
|
||||||
|
"the premise: a radius too small to draw emits no pass"
|
||||||
|
);
|
||||||
|
|
||||||
|
let composed = compose_detail(&ops, scale, dr_types::ColourSpace::Srgb);
|
||||||
|
assert_eq!(composed.len(), 1, "the chain must not be empty here");
|
||||||
|
assert_eq!(composed.radius(), 0, "it reads only the pixel it writes");
|
||||||
|
|
||||||
|
let resolve = &composed.passes[0];
|
||||||
|
assert_eq!(resolve.label, "detail/resolve");
|
||||||
|
assert!(resolve.writes_output);
|
||||||
|
assert!(resolve.source.contains("texture_storage_2d<rgba8unorm"));
|
||||||
|
assert!(resolve.source.contains("fn encode_output"));
|
||||||
|
// Exactly the fixed base block and no more: a pass with no body has
|
||||||
|
// nothing of its own to upload, and the block still has to be a
|
||||||
|
// multiple of sixteen bytes.
|
||||||
|
assert_eq!(resolve.uniforms.len(), DETAIL_BASE_UNIFORM_FIELDS);
|
||||||
|
assert_eq!(resolve.uniforms.len() % 4, 0);
|
||||||
|
|
||||||
|
// And it really is a copy: the fused pass composed alongside it is the
|
||||||
|
// one that stopped short, so the two agree about who encodes.
|
||||||
|
assert_eq!(fused(&ops).output_mode, OutputMode::LinearWorking);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn an_edit_with_no_detail_operation_composes_no_passes() {
|
fn an_edit_with_no_detail_operation_composes_no_passes() {
|
||||||
// The property that keeps the cost of this stage at zero for the
|
// The property that keeps the cost of this stage at zero for the
|
||||||
|
|||||||
Reference in New Issue
Block a user