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

GLTFScene: basic support for KHR_materials_pbrSpecularGlossiness extension #625

Closed
wants to merge 1 commit into from
Closed

GLTFScene: basic support for KHR_materials_pbrSpecularGlossiness extension #625

wants to merge 1 commit into from

Conversation

jtbandes
Copy link
Contributor

Handle material.extensions.KHR_materials_pbrSpecularGlossiness by using the diffuseTexture in place of the pbrMetallicRoughness.baseColorTexture. This is not a perfect/complete implementation, however, it's better than having no support at all for pbrSpecularGlossiness!

Other small fixes:

Example model from: SpecGlossVsMetalRough

The two bottles are supposed to look identical, so obviously this is not a complete rendering implementation.

image

Copy link
Collaborator

@hhsaez hhsaez left a comment

Choose a reason for hiding this comment

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

These changes look good to me. Although, like you mentioned in the description, the end result might not look correct, since we don't really have support for any lighting model.

Thanks for adding the extra fixes as well!

I'm not sure how to fix the CI errors, though. I'll ask around.

jtbandes added a commit to foxglove/regl-worldview that referenced this pull request Oct 15, 2021
Make it possible to import GLText in tests (but not use it) by adding a `typeof self` check before accessing `self`.

Also import a couple of small fixes to parseGLB and GLTFScene from cruise-automation/webviz#625 that were never merged.
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants