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

WebGLRenderer: Add support for GLTF opaque alpha_mode #22428

Merged
merged 3 commits into from
Aug 25, 2021
Merged

WebGLRenderer: Add support for GLTF opaque alpha_mode #22428

merged 3 commits into from
Aug 25, 2021

Conversation

mrdoob
Copy link
Owner

@mrdoob mrdoob commented Aug 25, 2021

Related issue: #22414 #22424

Description

I think this is how GLTF's OPAQUE ALPHA_MODE is supposed to behave.

(Ab)using blending doesn't feel right though.
We should be able to reuse NormalBlending, but we need a way to trigger the #define OPAQUE in the shader.

material.alpha = false
material.disableAlpha = false
???

/cc @donmccurdy @WestLangley @elalish

@mrdoob mrdoob added this to the r132 milestone Aug 25, 2021
@mrdoob
Copy link
Owner Author

mrdoob commented Aug 25, 2021

Another option that my younger-self proposed was:

material.format = THREE.RGBFormat;

@mrdoob
Copy link
Owner Author

mrdoob commented Aug 25, 2021

I think for the time being I'm going to go with material.format = THREE.RGBFormat.
I want to get this out of the way now that I've my head around it, but would love to hear any other naming ideas for it.

@mrdoob mrdoob merged commit f058376 into dev Aug 25, 2021
@mrdoob mrdoob deleted the gltf-opaque branch August 25, 2021 20:28
@mrdoob mrdoob restored the gltf-opaque branch August 25, 2021 20:37
@mrdoob mrdoob deleted the gltf-opaque branch August 25, 2021 20:39
@donmccurdy
Copy link
Collaborator

I'm OK with .format here, but perhaps .outputFormat would be a little clearer?

I do not really like the name .alpha as a boolean, too easily confused with .transparent or .opacity.

@mrdoob
Copy link
Owner Author

mrdoob commented Aug 26, 2021

I'm OK with .format here, but perhaps .outputFormat would be a little clearer?

Maybe yeah...

Although GLTF is currently confusing. Getting transmission to work in alpha: true contexts is incompatible with alpha_mode opaque...

#ifdef OPAQUE
diffuseColor.a = 1.0;
#endif
// https://github.com/mrdoob/three.js/pull/22425
#ifdef USE_TRANSMISSION
diffuseColor.a *= transmissionAlpha + 0.1;
#endif

@donmccurdy
Copy link
Collaborator

I guess I think of glTF's alphaMode as specifying how the material's alpha value (0–1) or texture values should be interpreted, and little more than that. It doesn't prohibit the snippet above, so long as alpha components in the base color texture are being ignored for opaque materials.

@mrdoob
Copy link
Owner Author

mrdoob commented Aug 26, 2021

I see. That makes sense.

@marcatec
Copy link
Contributor

marcatec commented Sep 1, 2021

I think for the time being I'm going to go with material.format = THREE.RGBFormat.
I want to get this out of the way now that I've my head around it, but would love to hear any other naming ideas for it.

Setting material.format = THREE.RGBFormat has the following side effect (e.g. in the editor):
Trying to change the opacity (and tranparent value) of an opaque material of an imported glTF file doesn't work anymore.
material.format should be set to THREE.RGBAFormat to make it work..

@mrdoob
Copy link
Owner Author

mrdoob commented Sep 2, 2021

Yeah... I'm going to continue working on this issue this month. material.format was not the right solution.

@donmccurdy
Copy link
Collaborator

/cc #22598 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants