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

Implement support for EXT_texture_webp #253

Merged
merged 4 commits into from
Jul 3, 2024
Merged

Conversation

bobbens
Copy link
Contributor

@bobbens bobbens commented Jun 2, 2024

@bobbens
Copy link
Contributor Author

bobbens commented Jun 2, 2024

Oops, it's EXT_texture_webp, not KHR_texture_webp.

@bobbens bobbens changed the title Implement support for KHR_texture_webp Implement support for EXT_texture_webp Jun 2, 2024
@zeux
Copy link
Contributor

zeux commented Jun 2, 2024

This needs to be structured like KHR_texture_basisu; the current implementation for example doesn’t support specifying both png and webp images. Also missing cgltf_write support.

@bobbens
Copy link
Contributor Author

bobbens commented Jun 18, 2024

This needs to be structured like KHR_texture_basisu; the current implementation for example doesn’t support specifying both png and webp images. Also missing cgltf_write support.

How does it look now?

Copy link
Contributor

@zeux zeux left a comment

Choose a reason for hiding this comment

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

Oh, one more thing - this needs a call to CGLTF_PTRFIXUP in cgltf_fixup_pointers for the newly added pointer field.

@bobbens
Copy link
Contributor Author

bobbens commented Jun 21, 2024

I added a call to CGLTF_PTRFIXUP in cgltf_fixup_pointers. Hopefully should be fine now.

@zeux
Copy link
Contributor

zeux commented Jun 21, 2024

And one more thing 😅 I think this is missing writing the appropriate extension name in cgltf_write_extensions, so a written file with webp extension will be considered invalid by glTF spec (although this would not necessarily interfere with loading it).

@bobbens
Copy link
Contributor Author

bobbens commented Jun 22, 2024

Lots of things I missed when grepping. Completed now perhaps.

Copy link
Contributor

@zeux zeux left a comment

Choose a reason for hiding this comment

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

Thanks! I've confirmed the parsing works in gltfpack (can't confirm cgltf_write changes since gltfpack doesn't use this but should be good on inspection). cc @jkuhlmann

zeux added a commit to zeux/meshoptimizer that referenced this pull request Jun 22, 2024
This is currently pending a merge to upstream:
jkuhlmann/cgltf#253
@jkuhlmann jkuhlmann merged commit fa3b80f into jkuhlmann:master Jul 3, 2024
3 checks passed
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