[Merged by Bors] - use blendstate blend for alphamode::blend #7899
Conversation
There was a problem hiding this comment.
Ouch, yeah, I hadn't considered this would subtly break existing custom shaders. 😕
Was going to also remind to update the release notes as part of this review, but I see that @cart has already removed the section about the shared blend state 👍
| #endif // ALPHA_MASK | ||
|
|
||
| #ifdef BLEND_PREMULTIPLIED_ALPHA | ||
| #else // BLEND_PREMULTIPLIED_ALPHA || BLEND_ALPHA | ||
| let alpha_mode = material.flags & STANDARD_MATERIAL_FLAGS_ALPHA_MODE_RESERVED_BITS; | ||
| if (alpha_mode == STANDARD_MATERIAL_FLAGS_ALPHA_MODE_BLEND || alpha_mode == STANDARD_MATERIAL_FLAGS_ALPHA_MODE_ADD) |
There was a problem hiding this comment.
Hmm I think this will needleslly run in BLEND_MULTIPLY, maybe we can guard it with a #ifndef BLEND_MULTIPLY?
There was a problem hiding this comment.
i think it's ok as is, since if we have BLEND_MULTIPLY, which implies we have !BLEND_PREMULTIPLIED_ALPHA && !BLEND_ALPHA && !ALPHA_MASK, then EMPTY_PREPASS_ALPHA_DISCARD gets defined and this whole containing block is skipped.
i think this set of defs could use a bit of a rationalization at some point though, it seems complex for what it's actually doing.
There was a problem hiding this comment.
Yeah. Probably better to do that after 0.10 is out to avoid subtly breaking anything.
I wanna try adding a dither-based alpha mode that renders “transparency” in the opaque pass (would go well with TAA) so maybe that could be a good time to do that
alice-i-cecile
added
C-Bug
# Objective revert combining pipelines for AlphaMode::Blend and AlphaMode::Premultiplied & Add the recent blend state pr changed `AlphaMode::Blend` to use a blend state of `Blend::PREMULTIPLIED_ALPHA_BLENDING`, and recovered the original behaviour by multiplying colour by alpha in the standard material's fragment shader. this had some advantages (specifically it means more material instances can be batched together in future), but this also means that custom materials that specify `AlphaMode::Blend` now get a premultiplied blend state, so they must also multiply colour by alpha. ## Solution revert that combination to preserve 0.9 behaviour for custom materials with AlphaMode::Blend.
bors
bot
changed the title
# Objective revert combining pipelines for AlphaMode::Blend and AlphaMode::Premultiplied & Add the recent blend state pr changed `AlphaMode::Blend` to use a blend state of `Blend::PREMULTIPLIED_ALPHA_BLENDING`, and recovered the original behaviour by multiplying colour by alpha in the standard material's fragment shader. this had some advantages (specifically it means more material instances can be batched together in future), but this also means that custom materials that specify `AlphaMode::Blend` now get a premultiplied blend state, so they must also multiply colour by alpha. ## Solution revert that combination to preserve 0.9 behaviour for custom materials with AlphaMode::Blend.
Objective
revert combining pipelines for AlphaMode::Blend and AlphaMode::Premultiplied & Add
the recent blend state pr changed
AlphaMode::Blendto use a blend state ofBlend::PREMULTIPLIED_ALPHA_BLENDING, and recovered the original behaviour by multiplying colour by alpha in the standard material's fragment shader.this had some advantages (specifically it means more material instances can be batched together in future), but this also means that custom materials that specify
AlphaMode::Blendnow get a premultiplied blend state, so they must also multiply colour by alpha.Solution
revert that combination to preserve 0.9 behaviour for custom materials with AlphaMode::Blend.