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

gltfio: add support for KHR_materials_variants #5361

Merged
merged 3 commits into from
Mar 25, 2022
Merged

gltfio: add support for KHR_materials_variants #5361

merged 3 commits into from
Mar 25, 2022

Conversation

prideout
Copy link
Contributor

Tested the Java API by hacking ModelViewer.
Tested the JavaScript API by adding to filament-viewer-test.

Tested the Java API by hacking ModelViewer.
Tested the JavaScript API by adding to filament-viewer-test.
Copy link
Collaborator

@romainguy romainguy left a comment

Choose a reason for hiding this comment

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

Looks great to me. Would it make sense to have an API to get the material/materialinstances associated with a given variant?

@@ -83,7 +83,8 @@ <h3>Skybox, fixed size</h3>
<h3>No skybox, no IBL, fixed size, round border</h3>
<p>
<filament-viewer
src="https://prideout.net/models/dino/trex.gltf"
src="https://prideout.net/models/MaterialsVariantsShoe.glb"
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should prob not rely on your domain name btw.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we should make an assets repo at some point. I tried replacing these with blob URL's from the Khronos repo and ran into fun CORS errors:

Access to fetch at 'https://github.com/KhronosGroup/glTF-Sample-Models/raw/master/2.0/MaterialsVariantsShoe/glTF-Binary/MaterialsVariantsShoe.glb' from origin 'http://127.0.0.1:8080' has been blocked by CORS policy: The 'Access-Control-Allow-Origin' header has a value 'https://render.githubusercontent.com' that is not equal to the supplied origin. Have the server send the header with a valid value, or, if an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.

@prideout
Copy link
Contributor Author

In a subsequent PR, I would like to add an applyMaterialVariant method to FilamentInstance.

@prideout
Copy link
Contributor Author

Looks great to me. Would it make sense to have an API to get the material/materialinstances associated with a given variant?

Yes it might make sense to expose the MaterialInstance objects associated with a given variant, I might do that in my follow up PR. (Makes less sense to expose Material, those are owned by the MaterialProvider and are widely shared.)

@prideout prideout force-pushed the pr/variants branch 2 times, most recently from 61cabf0 to e0a1e27 Compare March 24, 2022 22:57
@prideout prideout merged commit ba042db into main Mar 25, 2022
@prideout prideout deleted the pr/variants branch March 25, 2022 02:23
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