WebGLRenderer: Ignore map alpha in opaque materials #18631
Conversation
|
Additions to #15483 look right to me, thanks! |
| export default /* glsl */` | ||
| #ifndef TRANSPARENT | ||
|
|
||
| diffuseColor.a = 1.0; |
There was a problem hiding this comment.
.transparent === false only disables blending if .blending is NormalBlending.
See #14171.
So with custom blending, blending is still enabled even if .transparent is false.
You are changing the user's data. I'm not sure about this change -- so, raising a red flag...
Also, if you choose to retain this, then I'd call this file opaque_fragment.glsl.js since that is what it applies to.
EDIT: I don't think this can be retained. I think you will have to implement something different -- something specific to the glTF alphaMode setting.
Maybe create alpha_mode_fragment.glsl.js.
/ping @mrdoob @donmccurdy
There was a problem hiding this comment.
Confirmed. This breaks AdditiveBlending when texture.a < 1 and material.transparent is false.
There was a problem hiding this comment.
Good points, thanks — I'd assumed .transparent = false implied that all blending was disabled.
One short-term fix would be to treat the material as if .transparent === true automatically, if any non-default blend mode is set. That sounds like it would cover the intended uses of the API, or at least I don't know why the user would intentionally use .transparent = false and a non-default blend mode? However, we are short on time to test a change like that before r114, and I'm not sure I understand the conceptual differences between:
.transparent = truevs..blending = NormalBlending.transparent = falsevs..blending = NoBlending
I think some changes to the transparency API might be worth considering later, but we certainly can't get that done in time for r114...
There was a problem hiding this comment.
What about #14171 (comment)? Was this feedback ever honored? Would it be also a solution to revert #14171?
There was a problem hiding this comment.
I'm afraid I don't understand what is meant in #14171 by "clearly not correct", or what transparency-related problems @jbaicoianu was referring to. These other blending modes seem (to me) to fit under the umbrella of semitransparency, and I would think you do want the sorting behavior that .transparent=true provides, even for other blend modes.
@jbaicoianu do you remember what problems you were referring to?
There was a problem hiding this comment.
He referred to #18579, too.
There was a problem hiding this comment.
Oh lord. Yes, just noticed...
https://github.com/mrdoob/three.js/pull/18579/files#diff-a47375d4b502ebcbbf70db4674af6724R5
There was a problem hiding this comment.
Okay, reverted #18579 too. Thanks! 🙏
Fixes #15483
/ping @donmccurdy @elalish @fms-cat