Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transparency doesnt work when on webgl adreno devices #10066

Open
TotalKrill opened this issue Oct 9, 2023 · 9 comments · May be fixed by #11525
Open

Transparency doesnt work when on webgl adreno devices #10066

TotalKrill opened this issue Oct 9, 2023 · 9 comments · May be fixed by #11525
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior O-WebGL2 Specific to the WebGL2 render API

Comments

@TotalKrill
Copy link
Contributor

TotalKrill commented Oct 9, 2023

Bevy version

bevy 0.11.3, 0.10.x, and looking at the code, most likely on main as well

Relevant system information

Most adreno devices tested, currently dont have one on hand. But I assume someone can verify. I guess this might also affect android devices

`AdapterInfo { name: "Adreno 660", backend: WebGL }`

What you did

The blend_modes example shows this quite well on Adreno devices on webgl, using this wasm-web-runner branch

cargo install --git https://github.com/heisencoder/wasm-server-runner.git --branch feature/support-gzip wasm-server-runner

to host the example from another computer. Opaque flags are ignored and all balls are either entirerly invisible, or fully visible, no translucency

Investigation

Looking at the unity docs, there are references to some types being 16bit in quite a few of the mobile GPU shader hardware. And when @gnargfu investigated, he found that this commit solves it. (basically moves the bitflags to the lower 16 bits in the flags field, used to branch in the shader code, so their position in the flag field of the struct doesnt matter)

Probably, a lot of the crashes related to mobile devices, are in fact due to shader code being introduced, that assumes that all values are high precision.

Quote from unity docs

Precision, Hardware Support and Performance
One complication of float/half/fixed data type usage is that PC GPUs are always high precision. That is, for all the PC (Windows/Mac/Linux) GPUs, it does not matter whether you write float, half or fixed data types in your shaders. They always compute everything in full 32-bit floating point precision.

The half and fixed types only become relevant when targeting mobile GPUs, where these types primarily exist for power (and sometimes performance) constraints. Keep in mind that you need to test your shaders on mobile to see whether or not you are running into precision/numerical issues.

@TotalKrill TotalKrill added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Oct 9, 2023
@alice-i-cecile alice-i-cecile added O-WebGL2 Specific to the WebGL2 render API A-Rendering Drawing game state to the screen and removed S-Needs-Triage This issue needs to be labelled labels Oct 10, 2023
@oscrim
Copy link

oscrim commented Jan 22, 2024

I just tested this on 8a523de with the transparency_3d example

Transparency was broken on Adreno 660 and 619 but was working as it should on Adreno 730

@oscrim
Copy link

oscrim commented Jan 22, 2024

After further testing I can say for certain that Qualcomm Adreno 600 series (and likely 500 series as well) is unable to use the last 16 bits of the 32bit bitflags which causes transparency to not work as that is where the alpha mode bits were stored. I tested by moving the bits to all be inside the first 16 bits which make transparency function as intended.

If we look at the commit that introduced this bug 603cb43 we can see that in that commit the flags were moved inside the last 16 bits.

I was planning on splitting out the Alpha bits from the StandardMaterialFlags into another struct. Something like

    #[repr(transparent)]
    pub struct StandardMaterialFlags: u32 {
        const BASE_COLOR_TEXTURE         = 1 << 0;
        const EMISSIVE_TEXTURE           = 1 << 1;
        const METALLIC_ROUGHNESS_TEXTURE = 1 << 2;
        const OCCLUSION_TEXTURE          = 1 << 3;
        const DOUBLE_SIDED               = 1 << 4;
        const UNLIT                      = 1 << 5;
        const TWO_COMPONENT_NORMAL_MAP   = 1 << 6;
        const FLIP_NORMAL_MAP_Y          = 1 << 7;
        const FOG_ENABLED                = 1 << 8;
        const DEPTH_MAP                  = 1 << 9; // Used for parallax mapping
        const SPECULAR_TRANSMISSION_TEXTURE = 1 << 10;
        const THICKNESS_TEXTURE          = 1 << 11;
        const DIFFUSE_TRANSMISSION_TEXTURE = 1 << 12;
        const ATTENUATION_ENABLED        = 1 << 13;
        const NONE                       = 0;
        const UNINITIALIZED              = 0xFFFF;
    }

    #[repr(transparent)]
    pub struct StandardMaterialAlphaModeFlags: u32 {
        const ALPHA_MODE_OPAQUE          = 0;
        const ALPHA_MODE_MASK            = 1 << 1;
        const ALPHA_MODE_BLEND           = 1 << 2;
        const ALPHA_MODE_PREMULTIPLIED   = 1 << 3;
        const ALPHA_MODE_ADD             = 1 << 4;
        const ALPHA_MODE_MULTIPLY        = 1 << 5;
        const UNINITIALIZED              = 0xFFFF;
    }

and in the StandardMaterialUniform add

pub alpha_mode_flags: u32,

Just to reiterate, my two reasons for doing it like this are:

  • Qualcomm Adreno 600 series (and likely 500 series as well) is unable to use the 32bit bitflags which breaks transparency as the alpha mode bits were stored on the upper half of the bits
  • The total number of bits used in the original bitmask is 17 including the bits needed for the alpha mode so simply moving how much the alpha mode bits are shifted wont work

Is this an acceptable solution that could be merged?

@oscrim
Copy link

oscrim commented Jan 24, 2024

@alice-i-cecile @mockersf
do any of you have an opinion on my proposed fix? I would very much like to upstream a fix for this to avoid having to use a patch in my cargo file

@alice-i-cecile
Copy link
Member

I'm in favor of fixing this, but I definitely don't have the rendering expertise to comment further. Let me ask around.

@kdashg
Copy link

kdashg commented Jan 24, 2024

This sounds like it might be this issue: KhronosGroup/WebGL#3351

@kdashg
Copy link

kdashg commented Jan 24, 2024

Specifically, GLSL loads and stores to structs seemingly truncate precision to mediump on adreno 600s, for both ints and floats.
Threejs worked around this by decomposing the problematic struct to its fields. (e.g. s/geometry.normal/geometryNormal/)

@coreh
Copy link
Contributor

coreh commented Jan 25, 2024

IMO, splitting the actual StandardMaterialFlags bitflags structure adds complexity to the Rust side of things that isn't really required for the GPU-side workaround.

Also a minor naming-related issue, but if we name the split struct StandardMaterialAlphaModeFlags we'll probably also have to rename it soon, since when we add two more flags to the first structure we'll have to spill any new flags over to it.

What if we used a vec2<u32>/UVec2 just on the uniform instead of two separate fields for the flags? I did an experiment, and minus having to make some changes from 0u to vec2(0u), and adding a couple any() and all() calls, it allows us to keep most of the existing code structure intact:

main...coreh:bevy:split-u32-flags

☝️ Can you confirm if this fixes it for the Adreno 600 series GPUs? If it works and other people are okay with the suggestion, I can also open a PR with it

@oscrim
Copy link

oscrim commented Jan 25, 2024

There still seems to be some bug when running the transparency_3d example, but whats odd is that the blend_modes example it seems to work

Screenshot_2024-01-25-08-45-16-74_e4424258c8b8649f6e67d283a50a2cbc
Screenshot_2024-01-25-08-45-11-02_e4424258c8b8649f6e67d283a50a2cbc
Screenshot_2024-01-25-08-45-21-49_e4424258c8b8649f6e67d283a50a2cbc
Screenshot_2024-01-25-08-44-03-08_e4424258c8b8649f6e67d283a50a2cbc

The issue in transparency_3d also shows up in the browser on my computer

2024-01-25.08-48-38.mp4

@oscrim
Copy link

oscrim commented Jan 25, 2024

Splitting it in two does fix the transparency issue. I opened a Pull Request (#11525) with the fix but if you prefer fixing it with vec2<u32>/UVec2 I can close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior O-WebGL2 Specific to the WebGL2 render API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants