Give every colour band its own compiled shader
🐳 Android image / Build and push (push) Successful in 5s
Build and test / android-image (push) Successful in 5s
Build and test / Desktop (Linux) (push) Failing after 57m33s
Build and test / Layer separation (push) Successful in 35s
Traceability / Requirement traces (push) Failing after 35s
Build and test / Android (aarch64) (push) Failing after 9m42s
🐳 Android image / Build and push (push) Successful in 5s
Build and test / android-image (push) Successful in 5s
Build and test / Desktop (Linux) (push) Failing after 57m33s
Build and test / Layer separation (push) Successful in 35s
Traceability / Requirement traces (push) Failing after 35s
Build and test / Android (aarch64) (push) Failing after 9m42s
The colour mixer emits a code block and a uniform only for the bands that are set, so which bands are adjusted is part of the shader's structure. The pipeline cache key was not: it hashed the set of *active operations*, which is "colour_mixer" whichever band that is. So a red adjustment and a blue one hashed alike. The second render was handed the first's compiled pipeline while its uniform was uploaded into a slot that shader had assigned to another band — whichever band compiled first kept acting on every subsequent move, and every other slider did nothing at all. Red is the first band declared, and the one reported as the only one working. The hash is now taken over the generated WGSL, because the source is what gets compiled and therefore is the structure. A summary of what went into it has to be kept in step with every operation's code generation by hand, and this one had fallen out of step. Values still do not enter it: no operation writes a parameter value into its source, so a slider drag regenerates identical text and reuses the pipeline, and one that did inline a value would have to recompile to be correct anyway. `each_colour_band_gets_its_own_pipeline` in dr-gpu renders a blue pixel through one pass with red set first and then blue, and fails on the old hash with the reported symptom — the blue slider returning the pixel unchanged to the byte. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -687,6 +687,64 @@ mod tests {
|
||||
assert!(body.contains("w_total > 0.0001"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn two_bands_are_two_shaders() {
|
||||
// The bug this closes: the shader cache was keyed on the set of
|
||||
// active operations, and this operation is active whichever band is
|
||||
// set. A red adjustment and a blue one therefore shared a compiled
|
||||
// pipeline — the first one to compile — and the second was rendered
|
||||
// with the first's code while its uniform was uploaded into the
|
||||
// first's slot. Every band but the one compiled first appeared to do
|
||||
// nothing, and moving its slider moved the other band's colour.
|
||||
//
|
||||
// Asserted here rather than only in `operation.rs` because this is
|
||||
// the operation that generates per-value code, and so the one whose
|
||||
// shaders must not collide.
|
||||
let compose_with = |id, v| {
|
||||
let mut m = ColourMixer::new();
|
||||
m.set_param(ParamId(id), v);
|
||||
let ops: Vec<Box<dyn Operation>> = vec![Box::new(m)];
|
||||
crate::operation::compose(&ops)
|
||||
};
|
||||
|
||||
let red = compose_with("red_sat", 60.0);
|
||||
let blue = compose_with("blue_sat", 60.0);
|
||||
assert_ne!(
|
||||
red.source, blue.source,
|
||||
"two bands emit different code, which is the premise"
|
||||
);
|
||||
assert_ne!(
|
||||
red.structure_hash, blue.structure_hash,
|
||||
"two bands must not share a compiled pipeline"
|
||||
);
|
||||
|
||||
// The same band at a different setting is the same shader, which is
|
||||
// what keeps a slider drag from recompiling once per frame.
|
||||
let red_harder = compose_with("red_sat", 90.0);
|
||||
assert_eq!(red.structure_hash, red_harder.structure_hash);
|
||||
assert_ne!(red.uniforms, red_harder.uniforms);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn every_band_and_channel_is_its_own_shader() {
|
||||
// All thirty-six, because the collision above was not special to red
|
||||
// and blue: any two settings that generate different code and hash
|
||||
// alike put one of them on the other's pipeline.
|
||||
let mut seen = std::collections::HashMap::new();
|
||||
for band in BANDS.iter() {
|
||||
for ch in Channel::ALL {
|
||||
let id = format!("{}_{}", band.key, ch.suffix());
|
||||
let mut m = ColourMixer::new();
|
||||
m.set_param(ParamId(Box::leak(id.clone().into_boxed_str())), 50.0);
|
||||
let ops: Vec<Box<dyn Operation>> = vec![Box::new(m)];
|
||||
let hash = crate::operation::compose(&ops).structure_hash;
|
||||
if let Some(other) = seen.insert(hash, id.clone()) {
|
||||
panic!("{id} and {other} share a pipeline");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn band_weights_sum_to_one_at_every_hue() {
|
||||
// The property the shader relies on to add band deltas unscaled, and
|
||||
|
||||
Reference in New Issue
Block a user