The extracted alpha rule disagreed with the renderer; the renderer was right

Pulling the 0x80 convention out of the renderer and into gc_formats::gs -- the
side benefit I claimed last commit rather than took -- immediately produced a
contradiction, which is the argument for having done it.

crates/gc-engine/src/level.rs already read it inline:

    let a = if c.a < 0x80 { c.a as f32 / 128.0 } else { 1.0 };

My first alpha_to_8 scaled by 127 instead of 128, which made 0x7F fully opaque
where the renderer had it at 253/255. One of them was wrong and it was not the new
one: the disc convention is a 0..0x80 range with 0x80 as the top, so 0x7F is
127/128, not 127/127.

Corrected to match, and the doc now records why -- including that the renderer was
already right, so this is a correction to code I wrote two commits ago rather than
to anyone else's.

One genuine ambiguity remains and it is stated rather than papered over: 0x40 is
exactly the midpoint, and the renderer works in floats where it is exactly 0.5.
Widening to an integer has to round, and this rounds half away from zero giving
128. There is no way to check that against the disc -- it needs a frame, which is
the tfrag morph item's unblock condition -- so it is documented as a choice.

The engine's call site keeps its inline form and now says why: it wants a float,
and the rule has one live site. Converting it would have meant changing working
code for no benefit, which is the opposite of what extracting the rule was for.

Worth noting what actually happened here. Extracting a rule to remove a magic
constant found that two expressions of it disagreed. Neither was obviously wrong
from its own code -- mine looked cleaner, and was not. That is a better argument
for centralising a rule than any amount of saying so.
This commit is contained in:
2026-10-14 04:15:11 +02:00
parent 31f4ba3f7c
commit 0b660cb0ce
2 changed files with 25 additions and 9 deletions
+2
View File
@@ -263,6 +263,8 @@ pub fn vertex_color(g: &TfragGeometry, vinfo: usize) -> [f32; 4] {
let i = g.position_index(vinfo);
match g.rgba.get(i) {
// The disc's 0x80-is-1.0 alpha convention, same as the texture CLUT.
// `gc_formats::gs::alpha_to_8` states this rule now; kept inline here
// because this path wants a float, and the rule has only one live site.
Some(c) => {
let a = if c.a < 0x80 { c.a as f32 / 128.0 } else { 1.0 };
[
+23 -9
View File
@@ -24,18 +24,24 @@ pub const ALPHA_ONE: u8 = 0x80;
///
/// `0x80` and above is the **opaque** end, and it is reached by masking off the top
/// bit -- `0x80 & 0x7F` is zero, which is precisely why `0x80` reads as "half" to
/// anything that treats the byte as 8-bit unsigned. Below `0x80` the value is 7-bit
/// and scales onto 1..255, so `0x7F` is opaque too and no non-zero alpha vanishes.
/// anything that treats the byte as 8-bit unsigned.
///
/// Below `0x80` the value scales onto `0..0x80`, so `0x7F` is 253/255 and not
/// fully opaque. That is **not** an accident of this function: `crates/gc-engine`
/// already read the same convention inline as `if c.a < 0x80 { c.a / 128.0 } else
/// { 1.0 }`, and the first version of this scaled by 127 instead, making `0x7F`
/// fully opaque where the renderer had it at 253/255. The renderer was right and
/// this was wrong.
///
/// Extracting the rule into one place is what surfaced the disagreement, which is
/// the argument for doing it even where the code already worked.
#[must_use]
pub fn alpha_to_8(v: u8) -> u8 {
if v >= ALPHA_ONE {
return 255;
}
if v == 0 {
return 0;
}
// scale 0..127 onto 1..255 so that 127 -> 255
(((v as u32) * 255 + 63) / 127) as u8
// scale 0..0x7F onto 0..255, matching the renderer's /128 normalisation
(((v as u32) * 255 + 64) / 128) as u8
}
/// A GS texture coordinate: signed 1/4096, as the hardware stores it.
@@ -163,8 +169,16 @@ mod tests {
assert_eq!(alpha_to_8(0x00), 0);
assert_eq!(
alpha_to_8(0x7F),
255,
"the top of the 7-bit range is opaque"
253,
"0x7F is 127/128, not fully opaque -- the renderer's convention"
);
// 0x40 is exactly the midpoint. The renderer works in floats and leaves
// it at 0.5; widening to an integer has to round, and this rounds half
// away from zero.
assert_eq!(
alpha_to_8(0x40),
128,
"the midpoint rounds half away from zero"
);
assert_eq!(alpha_to_8(0x01) > 0, true, "the low end must not vanish");
assert_eq!(