From eb229051ef8f12ec22a2736fb383018f26f299f6 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 22 Aug 2026 19:20:22 +0200 Subject: [PATCH] Teach the whole-chain GPU tests about neighbourhood operations Both tests composed only the fused half and rendered it through the plain path. That was correct while every operation was a point operation; with a kernel in the chain the fused pass stops short of the output transform, so the render was rejected and the operation-block count was one too high. Compose both halves and dispatch them together, and assert that each operation reaches exactly one of the two stages rather than counting blocks - so the next kernel added extends the coverage instead of breaking it. --- core/dr-gpu/src/adjust.rs | 85 ++++++++++++++++++++++++++++++++------- 1 file changed, 70 insertions(+), 15 deletions(-) diff --git a/core/dr-gpu/src/adjust.rs b/core/dr-gpu/src/adjust.rs index df7c9d1..a47e85d 100644 --- a/core/dr-gpu/src/adjust.rs +++ b/core/dr-gpu/src/adjust.rs @@ -1063,12 +1063,31 @@ mod tests { let mut g = EditGraph::default_chain(); g.set_param(cap.id, p.id, value); let shader = g.compose(); - pass.render(&img, &shader, 16, 16).unwrap_or_else(|e| { - panic!( - "{}.{} at {value} generated invalid WGSL:\n{e}", - cap.id, p.id - ) - }); + + // A neighbourhood operation compiles as a *chain*, not as + // a fragment: it contributes nothing to the fused pass, + // and the fused pass in turn stops short of the output + // transform so the last detail pass can perform it. Going + // through `render_detailed` covers both kinds with one + // loop, which is the property that makes this test extend + // itself when an operation is added — the whole reason it + // is derived from the chain rather than hand-written. + // + // Composed at the size actually being rendered, because a + // radius stated in source pixels can decide there is + // nothing to draw at sixteen pixels (FR-DSP-1); the chain + // still carries the resolve pass that finishes the render, + // and that generated source is worth compiling too. + let scale = g.render_scale(img.size(), (16, 16)); + let detail = g.compose_detail(scale); + let key = g.invalidation().through(dr_pipeline::Affects::Colour); + pass.render_detailed(&img, &shader, 16, 16, None, &detail, key) + .unwrap_or_else(|e| { + panic!( + "{}.{} at {value} generated invalid WGSL:\n{e}", + cap.id, p.id + ) + }); } } } @@ -1422,24 +1441,60 @@ mod tests { } } + // 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 + // allocation would show up. + let (w, h) = g.output_size(32, 32); let shader = g.compose(); + let scale = g.render_scale(img.size(), (w, h)); + let detail = g.compose_detail(scale); + + // Every operation has to reach the pipeline, but they do not all reach + // the same half of it, and which half is not this test's business to + // know: a point operation is a block in the fused shader, and a + // neighbourhood operation is one or more passes of the detail chain + // (`dr_pipeline::detail`) and contributes *no* fused block, because a + // fused fragment is handed a colour with no way back to a coordinate. + // + // Asserted as an exclusive or over the chain rather than as a count, + // so that adding either kind of operation extends this test on its own + // — and so that an operation which somehow managed both, or neither, + // is named rather than showing up as an arithmetic mismatch. + let mut fused_blocks = 0; + for desc in g.descriptors() { + let id = desc.id.0; + let point = shader.source.contains(&format!("---- {id} ----")); + let neighbourhood = detail + .passes + .iter() + .any(|p| p.label.starts_with(&format!("{id}/"))); + assert!( + point ^ neighbourhood, + "{id} reaches {} of the two stages; every active operation \ + belongs to exactly one", + if point { "both" } else { "neither" } + ); + fused_blocks += usize::from(point); + } + assert_eq!( shader.source.matches("---- ").count(), - // Every operation, plus framing — which emits a stage of its own - // rather than an operation block, and is not in `descriptors`. - g.descriptors().len() + 1, - "every operation and the framing should be active" + // The fused operations, plus framing — which emits a stage of its + // own rather than an operation block, and is not in `descriptors`. + fused_blocks + 1, + "the fused shader carries a block nothing in the chain asked for" ); assert!( shader.source.contains("---- framing ----"), "framing must reach the shader alongside the colour operations" ); + assert!( + !detail.is_empty(), + "with every operation active the detail stage must run" + ); - // 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 - // allocation would show up. - let (w, h) = g.output_size(32, 32); - pass.render(&img, &shader, w, h) + 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"); }