reduce alpha's precision

[DnecLv]

I've test this on Sponza and found it has white patches. This mistake mainly appears in textures of blue basecolor.

I think the process of packing basecolor's' alpha component leads to this. the operation of float on gpu is not precise and the second element {blue} suffered the greatest impact.

after:

[Patryk27]

Hi, thanks for the pull request!

I'm still wondering why wrong alpha calculations would affect the blue channel the most, but I certainly see the mistake in doing gamma-correction (.powf(1.0 / 2.2)) to the alpha channel now - would you mind checking if something like this solves the issue as well?

(on my machine, M1 Mac, the original issue does not occur, so this might be something GPU-specific)

diff --git a/strolle-gpu/src/gbuffer.rs b/strolle-gpu/src/gbuffer.rs
index 011ba5e..af0993d 100644
--- a/strolle-gpu/src/gbuffer.rs
+++ b/strolle-gpu/src/gbuffer.rs
@@ -1,4 +1,4 @@
-use glam::{vec4, Vec2, Vec3, Vec4, Vec4Swizzles};
+use glam::{vec3, vec4, Vec2, Vec3, Vec4, Vec4Swizzles};
 #[cfg(target_arch = "spirv")]
 use spirv_std::num_traits::Float;

@@ -36,13 +36,13 @@ impl GBufferEntry {
         let base_color = {
             let [x, y, z, w] = d1.w.to_bits().to_bytes();

-            vec4(
-                x as f32 / 255.0,
-                y as f32 / 255.0,
-                z as f32 / 255.0,
-                w as f32 / 63.0,
-            )
-            .powf(2.2)
+            let base_color =
+                vec3(x as f32 / 255.0, y as f32 / 255.0, z as f32 / 255.0)
+                    .powf(2.2);
+
+            let base_color_alpha = w as f32 / 255.0;
+
+            base_color.extend(base_color_alpha)
         };

         Self {
@@ -84,11 +84,16 @@ impl GBufferEntry {
             let z = self.emissive.z;

             let w = {
-                let base_color = self
+                let base_color_gamma_corrected = self
                     .base_color
+                    .xyz()
                     .powf(1.0 / 2.2)
-                    .clamp(Vec4::ZERO, Vec4::ONE);
-                let base_color = (vec4(base_color.x*255.0,base_color.y*255.0,base_color.z*255.0,base_color.w*63.0)).as_uvec4();
+                    .clamp(Vec3::ZERO, Vec3::ONE);
+
+                let base_color =
+                    base_color_gamma_corrected.extend(self.base_color.w);
+
+                let base_color = (base_color * 255.0).as_uvec4();

                 f32::from_bits(u32::from_bytes([
                     base_color.x,

[DnecLv]

I noticed the gamma issue when I first saw this code, but after testing I found that it was not the main problem. I test again with the code you provide and get the same conclusion - Not necessarily of alpha's gamma correction, but it doesn't lead to bug. This is my environment: { os: "Windows 10 Home China", kernel: "19045", cpu: "AMD Ryzen 7 5800H with Radeon Graphics", core_count: "8", memory: "15.9 GiB" }

[Patryk27]

I see, would you mind committing that version of the fix, then? (assuming I understood you correctly that it does fix the issue) -- feels less hacky to me 😇

[DnecLv]

Maybe I should apologize for my poor English expression, gamma-correction for alpha is unnecessary, I agree. But that doesn't lead to the white patches mistake. Another experiment I did just now: I fixed the 'base_color_alpha' = 1.0 in unpack function, and fixed the pack input here. 255 lead to the white patches but 254 not.

let w = {
    let base_color_gamma_corrected = self.base_color.xyz().powf(1.0 / 2.2).clamp(Vec3::ZERO, Vec3::ONE);
    let base_color = base_color_gamma_corrected.extend(self.base_color.w);
    let base_color = (base_color * 255.0).as_uvec4();
    f32::from_bits(u32::from_bytes([
        base_color.x,
        base_color.y,
        base_color.z,
        255 as u32, // -> white patches error
        254 as u32, // -> not error
    ]))
};

In the packing method of u32::from_bytes d(alpha/w) occupied the highest position, and c(blue/z) next, so I guess that when blue component is large, it might have overflow issues in Windows OS (you said there's no problem with M1). Reducing the precise of alpha (maybe a little hacky) could temporarily solve this problem.

impl U32Ext for u32 {
    fn from_bytes([a, b, c, d]: [u32; 4]) -> Self {
        a | (b << 8) | (c << 16) | (d << 24)
    }

    fn to_bytes(mut self) -> [u32; 4] {
        let a = self & 0xff;
        self >>= 8;
        let b = self & 0xff;
        self >>= 8;
        let c = self & 0xff;
        self >>= 8;
        let d = self & 0xff;

        [a, b, c, d]
    }
}

Another test when I fixed all objects' blue to 0.3

// prim_raster.rs
...
let material = MaterialsView::new(materials)
    .get(MaterialId::new(params.material_id()));

let mut base_color = material.base_color(atlas_tex, atlas_sampler, uv);
base_color.z = 0.3; // fixed to 0.3
// If our material is transparent and doesn't rely on refraction, kill the
// current fragment to re-use GPU in finding the next triangle
if base_color.w < 0.01 && material.ior == 1.0 {
    arch::kill();
}
...

[Patryk27]

Alright, I see - this looks a bit suspicious, because the alpha value is not actually read anywhere, so this nudges me a bit towards a miscompilation happening somewhere.

Would you mind checking out the fix-base-colors branch here and telling me whether it solves the issue as well? (I avoid storing and reading the alpha value there whatsoever, 'cause currently light transmission is not supported anyway)

[DnecLv]

result of 'fix-base-colors' ↓

but fix 'base_color.z = 0.3' it works well in this branch

[Patryk27]

Thanks for checking, this is pretty interesting! - I think your fix is alright, then (it's a bit worrying I can't reproduce it on my side, but the fix doesn't break anything either, so there's that).

One thing, would you mind running cargo fmt and committing again?

[Patryk27]

Thanks!