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

GLTFExporter: Improve alphaMode support. #12692

Merged
merged 3 commits into from
Nov 24, 2017

Conversation

donmccurdy
Copy link
Collaborator

@donmccurdy donmccurdy commented Nov 18, 2017

Handling a case for PNG textures where transparent=false but alphaTest>0. @fernandojsg does this seem like a reasonable way to guess at MASK vs BLEND modes?

Also pads the JSON chunk to 4-byte boundary to make the validator happy. (https://gltf-viewer.donmccurdy.com/ runs validation automatically now) Reverting the padding for now.

@donmccurdy donmccurdy changed the title GLTFExporter: Improve alphaMode support, align JSON chunk. GLTFExporter: Improve alphaMode support. Nov 18, 2017

if ( material.alphaTest !== 0.5 ) {
if ( material.alphaTest > 0.0 ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll keep both comparisons > 0.0 && !== 0.5. The second one just to prevent writing the alphaCutoff = 0.5 as it's already the default value on the glTF spec.


gltfMaterial.alphaMode = 'MASK'; // @FIXME We should detect MASK or BLEND
gltfMaterial.alphaMode = material.opacity < 1.0 ? 'BLEND' : 'MASK';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should work for now, although mask and blending is something that probably should be improved on the glTF spec. For example if we have a material with opacity: 1 and transparent :true , and a transparent png, and no alphatest, it will enable blending on three.js too, instead of just masking.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the purpose of opacity:1 + transparent:true? Not sure I understand this example but please weigh in on KhronosGroup/glTF#1158 if you have suggestions. 🙂

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opacity: 1 && transparent: true will still use the alpha channel of the map (if I remember it correctly), if transparent: false it won't.

@donmccurdy
Copy link
Collaborator Author

Ok, updated to avoid writing the default value.

Any changes to the glTF alpha coverage spec will take some time, so there's no need to wait for that.

@fernandojsg
Copy link
Collaborator

Yep I agree, this should work with the current spec, we could come back to it once the spec is changed.

@mrdoob mrdoob merged commit 1e1dd47 into mrdoob:dev Nov 24, 2017
@mrdoob
Copy link
Owner

mrdoob commented Nov 24, 2017

Thanks!

@donmccurdy donmccurdy deleted the feat-gltfexporter-alphamodes branch February 4, 2019 17:29
@donmccurdy
Copy link
Collaborator Author

@fernandojsg perhaps we should do this, instead?

if ( material.transparent ) {

  gltfMaterial.alphaMode = 'BLEND';

} else if ( material.alphaTest > 0.0 ) {

  gltfMaterial.alphaMode = 'MASK';

}

See https://discourse.threejs.org/t/gltfexporter-changing-alphamode/5993/2.

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