Split `StandardMaterialFlags` in two for compatibility with low end GPUs
Objective
Fixes #10066
Solution
Added StandardMaterialAlphaModeFlags containing the alpha mode bitflags previously in StandardMaterialFlags. This removes all the bits previously in the last 16-bits of the StandardMaterialFlags u32 which makes transparency function as intended again.
Changelog
This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.
- What changed as a result of this PR?
- If applicable, organize changes under "Added", "Changed", or "Fixed" sub-headings
- Stick to one or two sentences. If more detail is needed for a particular change, consider adding it to the "Solution" section
- If you can't summarize the work, your change may be unreasonably large / unrelated. Consider splitting your PR to make it easier to review and merge!
Migration Guide
This section is optional. If there are no breaking changes, you can delete this section.
- If this PR is a breaking change (relative to the last release of Bevy), describe how a user might need to migrate their code to support these changes
- Simply adding new functionality is not a breaking change.
- Fixing behavior that was definitely a bug, rather than a questionable design choice is not a breaking change.
Welcome, new contributor!
Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨
Ive tested this fix on my phone which uses an Adreno 660 which previously couldnt render transparency but with this fix it can.
could you add comments in the code explaining why it's needed and we can't have everything in a single u32?
could you add comments in the code explaining why it's needed and we can't have everything in a single u32?
Absolutely, would something like this suffice?
// NOTE: These bitflags are separate from StandardMaterialFlags due to a bug on some older GPUs
// that are unable to use the last 16 bits of a u32
bitflags::bitflags! {
/// Bitflags info about the alpha mode of the material a shader is currently rendering.
/// This is accessible in the shader in the [`StandardMaterialUniform`]
#[repr(transparent)]
pub struct StandardMaterialAlphaModeFlags: u32 {
const ALPHA_MODE_OPAQUE = 0;
const ALPHA_MODE_MASK = 1;
const ALPHA_MODE_BLEND = 2;
const ALPHA_MODE_PREMULTIPLIED = 3;
const ALPHA_MODE_ADD = 4;
const ALPHA_MODE_MULTIPLY = 5;
}
}
Also this in pbr_types.wgsl
// 'flags' is a bit field indicating various options.
// u32 is 32 bits so we would have up to 32 options. Some lower-end mobile GPUs only support 16-bits
// so we can only use half of the u32
flags: u32,
alpha_mode_flags: u32,
@alice-i-cecile Can I ask why this is considered controversial? And is there anything I can do to where its not controversial?
Twiddling with bitflags for hardware-specific support is always a bit scary. It looks like this is a good approach, but it requires more testing and expert opinion than the average rendering PR as a result.
Hmm, actually I think Complex is a better tag here. Going to wait for rendering expertise and testing before merging :)
Might be worth creating an issue in wgpu (as it could be poly-filled/worked around there)
Might be worth creating an issue in wgpu (as it could be poly-filled/worked around there)
I created an issue https://github.com/gfx-rs/wgpu/issues/5318
Going to wait for rendering expertise and testing before merging
Is there anything I can do to help out with the testing?