The alpha bug had a test named after the bug
Fixes a broken build left by777a6a8, and the test it breaks is the interesting part. texture::tests::alpha_is_scaled_from_eight_bit_halves assert_eq!(scale_alpha(0x7f), 0xfe); That is not a test that happened to pin the wrong value. The test's NAME asserts the wrong rule -- "scaled from eight bit halves" is the `a * 2` implementation written down as intent, with 0xFE as its proof. Someone looked at this, thought "alpha is a half-scale byte, double it", and then locked the conclusion into both the identifier and the assertion. So the previous commit's claim that `a * 2` was an implementation detail was wrong in a way that matters: it had a name, a doc comment and a test defending it. That is not an accident that escaped review, it is a convention that was adopted. Two things follow. The rule is now /128 * 255, and the test says so in its name. It also pins 0x7E -> 0xFB, a value the doubling cannot produce and which the disc does contain. And777a6a8was pushed with a failing test in it. I only caught it because the count came back as 100 rather than 278 and the discrepancy was worth one more command -- `cargo test -q` plus a grep for `^test result: ok` sums only the binaries that passed, so a failure is invisible in the total unless you go and look at the raw lines. I had been using that filtered sum to report "278 tests" for several commits without knowing it could hide a red build. That is worth more than the bug it found. The measurement I had been reporting as evidence of health was itself an instance of the defect this whole stretch has been about: a quantity that should have had one definition, expressed as a filtered sum, agreeing with the wrong answer and nobody comparing it against anything. 278 passing with the disc present, clippy clean.
This commit is contained in:
@@ -273,12 +273,18 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn alpha_is_scaled_from_eight_bit_halves() {
|
||||
fn alpha_runs_zero_to_0x80_and_scales_to_0xff() {
|
||||
assert_eq!(scale_alpha(0), 0);
|
||||
assert_eq!(scale_alpha(0x40), 0x80);
|
||||
assert_eq!(scale_alpha(0x7f), 0xfe);
|
||||
// 0x7F is 127/128, which is 253 and not 0xFE. This test was called
|
||||
// `alpha_is_scaled_from_eight_bit_halves` and asserted 0xfe, pinning the
|
||||
// `a * 2` implementation -- it had the bug in its name as well as its
|
||||
// assertion. The rule is /128 * 255, not a doubling.
|
||||
assert_eq!(scale_alpha(0x7f), 0xfd);
|
||||
assert_eq!(scale_alpha(0x80), 0xff);
|
||||
assert_eq!(scale_alpha(0xff), 0xff);
|
||||
// A doubling cannot produce this, and the disc contains it.
|
||||
assert_eq!(scale_alpha(0x7e), 0xfb);
|
||||
}
|
||||
|
||||
/// A 2x1 image whose two texels select distinct CLUT entries.
|
||||
|
||||
Reference in New Issue
Block a user