-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[Merged by Bors] - Standard Material Blend Modes #6644
Changes from all commits
7141282
bbb3420
e2a0c46
e1b54dd
63b7844
4efa604
0562f46
92ca5bd
b34341f
fc33691
295d8d2
4187b15
c8f914c
95aad7e
2c9e22a
7d9989e
a3810fd
d538109
fe052e7
b2196b1
e035576
071a4c6
1ee663a
54591c8
28bfb45
5a14a7e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -298,16 +298,25 @@ bitflags::bitflags! { | |
const OCCLUSION_TEXTURE = (1 << 3); | ||
const DOUBLE_SIDED = (1 << 4); | ||
const UNLIT = (1 << 5); | ||
const ALPHA_MODE_OPAQUE = (1 << 6); | ||
const ALPHA_MODE_MASK = (1 << 7); | ||
const ALPHA_MODE_BLEND = (1 << 8); | ||
const TWO_COMPONENT_NORMAL_MAP = (1 << 9); | ||
const FLIP_NORMAL_MAP_Y = (1 << 10); | ||
const TWO_COMPONENT_NORMAL_MAP = (1 << 6); | ||
const FLIP_NORMAL_MAP_Y = (1 << 7); | ||
const ALPHA_MODE_RESERVED_BITS = (Self::ALPHA_MODE_MASK_BITS << Self::ALPHA_MODE_SHIFT_BITS); // ← Bitmask reserving bits for the `AlphaMode` | ||
const ALPHA_MODE_OPAQUE = (0 << Self::ALPHA_MODE_SHIFT_BITS); // ← Values are just sequential values bitshifted into | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 0 << n won’t work. |
||
const ALPHA_MODE_MASK = (1 << Self::ALPHA_MODE_SHIFT_BITS); // the bitmask, and can range from 0 to 7. | ||
const ALPHA_MODE_BLEND = (2 << Self::ALPHA_MODE_SHIFT_BITS); // | ||
const ALPHA_MODE_PREMULTIPLIED = (3 << Self::ALPHA_MODE_SHIFT_BITS); // | ||
const ALPHA_MODE_ADD = (4 << Self::ALPHA_MODE_SHIFT_BITS); // Right now only values 0–5 are used, which still gives | ||
const ALPHA_MODE_MULTIPLY = (5 << Self::ALPHA_MODE_SHIFT_BITS); // ← us "room" for two more modes without adding more bits | ||
const NONE = 0; | ||
const UNINITIALIZED = 0xFFFF; | ||
} | ||
} | ||
|
||
impl StandardMaterialFlags { | ||
const ALPHA_MODE_MASK_BITS: u32 = 0b111; | ||
const ALPHA_MODE_SHIFT_BITS: u32 = 32 - Self::ALPHA_MODE_MASK_BITS.count_ones(); | ||
} | ||
|
||
/// The GPU representation of the uniform data of a [`StandardMaterial`]. | ||
#[derive(Clone, Default, ShaderType)] | ||
pub struct StandardMaterialUniform { | ||
|
@@ -380,6 +389,9 @@ impl AsBindGroupShaderType<StandardMaterialUniform> for StandardMaterial { | |
flags |= StandardMaterialFlags::ALPHA_MODE_MASK; | ||
} | ||
AlphaMode::Blend => flags |= StandardMaterialFlags::ALPHA_MODE_BLEND, | ||
AlphaMode::Premultiplied => flags |= StandardMaterialFlags::ALPHA_MODE_PREMULTIPLIED, | ||
AlphaMode::Add => flags |= StandardMaterialFlags::ALPHA_MODE_ADD, | ||
AlphaMode::Multiply => flags |= StandardMaterialFlags::ALPHA_MODE_MULTIPLY, | ||
}; | ||
|
||
StandardMaterialUniform { | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -557,13 +557,16 @@ bitflags::bitflags! { | |||||
/// MSAA uses the highest 3 bits for the MSAA log2(sample count) to support up to 128x MSAA. | ||||||
pub struct MeshPipelineKey: u32 { | ||||||
const NONE = 0; | ||||||
const TRANSPARENT_MAIN_PASS = (1 << 0); | ||||||
const HDR = (1 << 1); | ||||||
const TONEMAP_IN_SHADER = (1 << 2); | ||||||
const DEBAND_DITHER = (1 << 3); | ||||||
const DEPTH_PREPASS = (1 << 4); | ||||||
const NORMAL_PREPASS = (1 << 5); | ||||||
const ALPHA_MASK = (1 << 6); | ||||||
const HDR = (1 << 0); | ||||||
const TONEMAP_IN_SHADER = (1 << 1); | ||||||
const DEBAND_DITHER = (1 << 2); | ||||||
const DEPTH_PREPASS = (1 << 3); | ||||||
const NORMAL_PREPASS = (1 << 4); | ||||||
const ALPHA_MASK = (1 << 5); | ||||||
const BLEND_RESERVED_BITS = Self::BLEND_MASK_BITS << Self::BLEND_SHIFT_BITS; // ← Bitmask reserving bits for the blend state | ||||||
const BLEND_OPAQUE = (0 << Self::BLEND_SHIFT_BITS); // ← Values are just sequential within the mask, and can range from 0 to 3 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here |
||||||
const BLEND_PREMULTIPLIED_ALPHA = (1 << Self::BLEND_SHIFT_BITS); // | ||||||
const BLEND_MULTIPLY = (2 << Self::BLEND_SHIFT_BITS); // ← We still have room for one more value without adding more bits | ||||||
const MSAA_RESERVED_BITS = Self::MSAA_MASK_BITS << Self::MSAA_SHIFT_BITS; | ||||||
const PRIMITIVE_TOPOLOGY_RESERVED_BITS = Self::PRIMITIVE_TOPOLOGY_MASK_BITS << Self::PRIMITIVE_TOPOLOGY_SHIFT_BITS; | ||||||
} | ||||||
|
@@ -573,7 +576,11 @@ impl MeshPipelineKey { | |||||
const MSAA_MASK_BITS: u32 = 0b111; | ||||||
const MSAA_SHIFT_BITS: u32 = 32 - Self::MSAA_MASK_BITS.count_ones(); | ||||||
const PRIMITIVE_TOPOLOGY_MASK_BITS: u32 = 0b111; | ||||||
const PRIMITIVE_TOPOLOGY_SHIFT_BITS: u32 = Self::MSAA_SHIFT_BITS - 3; | ||||||
const PRIMITIVE_TOPOLOGY_SHIFT_BITS: u32 = | ||||||
Self::MSAA_SHIFT_BITS - Self::PRIMITIVE_TOPOLOGY_MASK_BITS.count_ones(); | ||||||
const BLEND_MASK_BITS: u32 = 0b11; | ||||||
const BLEND_SHIFT_BITS: u32 = | ||||||
Self::PRIMITIVE_TOPOLOGY_SHIFT_BITS - Self::BLEND_MASK_BITS.count_ones(); | ||||||
|
||||||
pub fn from_msaa_samples(msaa_samples: u32) -> Self { | ||||||
let msaa_bits = | ||||||
|
@@ -677,12 +684,30 @@ impl SpecializedMeshPipeline for MeshPipeline { | |||||
let vertex_buffer_layout = layout.get_layout(&vertex_attributes)?; | ||||||
|
||||||
let (label, blend, depth_write_enabled); | ||||||
if key.contains(MeshPipelineKey::TRANSPARENT_MAIN_PASS) { | ||||||
label = "transparent_mesh_pipeline".into(); | ||||||
blend = Some(BlendState::ALPHA_BLENDING); | ||||||
let pass = key.intersection(MeshPipelineKey::BLEND_RESERVED_BITS); | ||||||
if pass == MeshPipelineKey::BLEND_PREMULTIPLIED_ALPHA { | ||||||
label = "premultiplied_alpha_mesh_pipeline".into(); | ||||||
blend = Some(BlendState::PREMULTIPLIED_ALPHA_BLENDING); | ||||||
shader_defs.push("PREMULTIPLY_ALPHA".into()); | ||||||
shader_defs.push("BLEND_PREMULTIPLIED_ALPHA".into()); | ||||||
// For the transparent pass, fragments that are closer will be alpha blended | ||||||
// but their depth is not written to the depth buffer | ||||||
depth_write_enabled = false; | ||||||
} else if pass == MeshPipelineKey::BLEND_MULTIPLY { | ||||||
label = "multiply_mesh_pipeline".into(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can change this, but it will be slightly inconsistent with the other two, that do match 1:1 with the key name, e.g.
Let me know what you prefer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
EDIT: No, that’s something else. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Stick with multiply. |
||||||
blend = Some(BlendState { | ||||||
color: BlendComponent { | ||||||
src_factor: BlendFactor::Dst, | ||||||
dst_factor: BlendFactor::OneMinusSrcAlpha, | ||||||
coreh marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
operation: BlendOperation::Add, | ||||||
}, | ||||||
alpha: BlendComponent::OVER, | ||||||
}); | ||||||
shader_defs.push("PREMULTIPLY_ALPHA".into()); | ||||||
shader_defs.push("BLEND_MULTIPLY".into()); | ||||||
// For the multiply pass, fragments that are closer will be alpha blended | ||||||
// but their depth is not written to the depth buffer | ||||||
depth_write_enabled = false; | ||||||
} else { | ||||||
label = "opaque_mesh_pipeline".into(); | ||||||
blend = Some(BlendState::REPLACE); | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,10 +7,11 @@ | |
|
||
fn alpha_discard(material: StandardMaterial, output_color: vec4<f32>) -> vec4<f32> { | ||
var color = output_color; | ||
if (material.flags & STANDARD_MATERIAL_FLAGS_ALPHA_MODE_OPAQUE) != 0u { | ||
let alpha_mode = material.flags & STANDARD_MATERIAL_FLAGS_ALPHA_MODE_RESERVED_BITS; | ||
if alpha_mode == STANDARD_MATERIAL_FLAGS_ALPHA_MODE_OPAQUE { | ||
// NOTE: If rendering as opaque, alpha should be ignored so set to 1.0 | ||
color.a = 1.0; | ||
} else if (material.flags & STANDARD_MATERIAL_FLAGS_ALPHA_MODE_MASK) != 0u { | ||
} else if alpha_mode == STANDARD_MATERIAL_FLAGS_ALPHA_MODE_MASK { | ||
if color.a >= material.alpha_cutoff { | ||
// NOTE: If rendering as masked alpha and >= the cutoff, render as fully opaque | ||
color.a = 1.0; | ||
|
@@ -268,3 +269,66 @@ fn dither(color: vec4<f32>, pos: vec2<f32>) -> vec4<f32> { | |
} | ||
#endif // DEBAND_DITHER | ||
|
||
#ifdef PREMULTIPLY_ALPHA | ||
fn premultiply_alpha(standard_material_flags: u32, color: vec4<f32>) -> vec4<f32> { | ||
coreh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// `Blend`, `Premultiplied` and `Alpha` all share the same `BlendState`. Depending | ||
// on the alpha mode, we premultiply the color channels by the alpha channel value, | ||
// (and also optionally replace the alpha value with 0.0) so that the result produces | ||
// the desired blend mode when sent to the blending operation. | ||
#ifdef BLEND_PREMULTIPLIED_ALPHA | ||
// For `BlendState::PREMULTIPLIED_ALPHA_BLENDING` the blend function is: | ||
// | ||
// result = 1 * src_color + (1 - src_alpha) * dst_color | ||
let alpha_mode = standard_material_flags & STANDARD_MATERIAL_FLAGS_ALPHA_MODE_RESERVED_BITS; | ||
if (alpha_mode == STANDARD_MATERIAL_FLAGS_ALPHA_MODE_BLEND) { | ||
coreh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Here, we premultiply `src_color` by `src_alpha` (ahead of time, here in the shader) | ||
// | ||
// src_color *= src_alpha | ||
// | ||
// We end up with: | ||
// | ||
// result = 1 * (src_alpha * src_color) + (1 - src_alpha) * dst_color | ||
// result = src_alpha * src_color + (1 - src_alpha) * dst_color | ||
// | ||
// Which is the blend operation for regular alpha blending `BlendState::ALPHA_BLENDING` | ||
return vec4<f32>(color.rgb * color.a, color.a); | ||
coreh marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we risk losing precision here, compared to returning unmultiplied and using I don't really see a reason to change this path though, we could just not push There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the nvidia paper https://developer.nvidia.com/content/alpha-blending-pre-or-not-pre from comments above gives a good reason for the change: it behaves much better with mipmapping. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's been a while since I wrote this PR; Consolidating additive, premultiplied and regular alpha blending into a single pass was based off some early feedback. I don't recall the exact reasoning, but looking at the code, and if I understand it correctly, the benefits I see are:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That was entirely coincidental, at least on my part! 😆 I'm trying to find who originally suggested consolidating, maybe they had that in mind as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Being able to use the same pipeline for more things is good as it allows us to batch multiple draw commands into one. |
||
} else if (alpha_mode == STANDARD_MATERIAL_FLAGS_ALPHA_MODE_ADD) { | ||
// Here, we premultiply `src_color` by `src_alpha`, and replace `src_alpha` with 0.0: | ||
// | ||
// src_color *= src_alpha | ||
// src_alpha = 0.0 | ||
// | ||
// We end up with: | ||
// | ||
// result = 1 * (src_alpha * src_color) + (1 - 0) * dst_color | ||
// result = src_alpha * src_color + 1 * dst_color | ||
// | ||
// Which is the blend operation for additive blending | ||
return vec4<f32>(color.rgb * color.a, 0.0); | ||
} else { | ||
// Here, we don't do anything, so that we get premultiplied alpha blending. (As expected) | ||
return color.rgba; | ||
} | ||
#endif | ||
// `Multiply` uses its own `BlendState`, but we still need to premultiply here in the | ||
// shader so that we get correct results as we tweak the alpha channel | ||
#ifdef BLEND_MULTIPLY | ||
// The blend function is: | ||
// | ||
// result = dst_color * src_color + (1 - src_alpha) * dst_color | ||
// | ||
// We premultiply `src_color` by `src_alpha`: | ||
// | ||
// src_color *= src_alpha | ||
// | ||
// We end up with: | ||
// | ||
// result = dst_color * (src_color * src_alpha) + (1 - src_alpha) * dst_color | ||
// result = src_alpha * (src_color * dst_color) + (1 - src_alpha) * dst_color | ||
// | ||
// Which is the blend operation for multiplicative blending with arbitrary mixing | ||
// controlled by the source alpha channel | ||
return vec4<f32>(color.rgb * color.a, color.a); | ||
#endif | ||
} | ||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,11 +17,17 @@ let STANDARD_MATERIAL_FLAGS_METALLIC_ROUGHNESS_TEXTURE_BIT: u32 = 4u; | |
let STANDARD_MATERIAL_FLAGS_OCCLUSION_TEXTURE_BIT: u32 = 8u; | ||
let STANDARD_MATERIAL_FLAGS_DOUBLE_SIDED_BIT: u32 = 16u; | ||
let STANDARD_MATERIAL_FLAGS_UNLIT_BIT: u32 = 32u; | ||
let STANDARD_MATERIAL_FLAGS_ALPHA_MODE_OPAQUE: u32 = 64u; | ||
let STANDARD_MATERIAL_FLAGS_ALPHA_MODE_MASK: u32 = 128u; | ||
let STANDARD_MATERIAL_FLAGS_ALPHA_MODE_BLEND: u32 = 256u; | ||
let STANDARD_MATERIAL_FLAGS_TWO_COMPONENT_NORMAL_MAP: u32 = 512u; | ||
let STANDARD_MATERIAL_FLAGS_FLIP_NORMAL_MAP_Y: u32 = 1024u; | ||
let STANDARD_MATERIAL_FLAGS_TWO_COMPONENT_NORMAL_MAP: u32 = 64u; | ||
let STANDARD_MATERIAL_FLAGS_FLIP_NORMAL_MAP_Y: u32 = 128u; | ||
let STANDARD_MATERIAL_FLAGS_ALPHA_MODE_RESERVED_BITS: u32 = 3758096384u; // (0b111u32 << 29) | ||
let STANDARD_MATERIAL_FLAGS_ALPHA_MODE_OPAQUE: u32 = 0u; // (0u32 << 29) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 0 means no flags are set, you’ll have to start from 1 << 29. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh… I see. You mean that none of the 3 bits are set. Ok. |
||
let STANDARD_MATERIAL_FLAGS_ALPHA_MODE_MASK: u32 = 536870912u; // (1u32 << 29) | ||
let STANDARD_MATERIAL_FLAGS_ALPHA_MODE_BLEND: u32 = 1073741824u; // (2u32 << 29) | ||
let STANDARD_MATERIAL_FLAGS_ALPHA_MODE_PREMULTIPLIED: u32 = 1610612736u; // (3u32 << 29) | ||
let STANDARD_MATERIAL_FLAGS_ALPHA_MODE_ADD: u32 = 2147483648u; // (4u32 << 29) | ||
let STANDARD_MATERIAL_FLAGS_ALPHA_MODE_MULTIPLY: u32 = 2684354560u; // (5u32 << 29) | ||
// ↑ To calculate/verify the values above, use the following playground: | ||
// https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=7792f8dd6fc6a8d4d0b6b1776898a7f4 | ||
|
||
// Creates a StandardMaterial with default values | ||
fn standard_material_new() -> StandardMaterial { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This syntax is... interesting. I guess we are e certain it works? As in, this will match if alpha mode is one of blend, premultiplied, or add.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, just double checked it, and it does work as expected
Playground example
I didn't think much of it as I was making this change, since I just reused the existing
if let
statement, but looking back it does look kinda cleaner thanif alpha_mode == AlphaMode::Blend || alpha_mode == AlphaMode::Premultiplied || alpha_mode == AlphaMode::Add
😄There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it’s more concise. I think the mental hiccup for me is because the flags can be combined using the | or operator. But I don’t mind either way. This is fine.