diff --git a/core/dr-gpu/src/adjust.rs b/core/dr-gpu/src/adjust.rs index df7c9d1..1043ad9 100644 --- a/core/dr-gpu/src/adjust.rs +++ b/core/dr-gpu/src/adjust.rs @@ -891,6 +891,11 @@ mod tests { use dr_decode::{BaseCurve, CfaPattern, CropRect, RawImage}; use dr_pipeline::ops::{colour_mixer, exposure, saturation}; 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; @@ -1423,23 +1428,65 @@ mod tests { } 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!( 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" + // Every *point* operation, plus framing — which emits a stage of + // its own rather than an operation block, and is not in + // `descriptors`. + g.descriptors().len() - neighbourhood + 1, + "every point operation and the framing should be active" ); assert!( shader.source.contains("---- framing ----"), "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 // 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) + // + // 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"); } diff --git a/core/dr-gpu/tests/local_contrast.rs b/core/dr-gpu/tests/local_contrast.rs index f93e328..9611940 100644 --- a/core/dr-gpu/tests/local_contrast.rs +++ b/core/dr-gpu/tests/local_contrast.rs @@ -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 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); render(&mut pass, &graph, &source, 128); assert_eq!( pass.detail_dispatches(), - 0, - "texture claimed a kernel it cannot draw" + 2, + "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 // hold it — no separate path, no fade, just the kernel resolving again. let mut zoomed = AdjustPass::new(&ctx);