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

Add support for EXT_texture_webp v2 #425

Merged
merged 13 commits into from
Jul 3, 2024
Merged

Add support for EXT_texture_webp v2 #425

merged 13 commits into from
Jul 3, 2024

Conversation

marstaik
Copy link
Contributor

@marstaik marstaik commented Jun 19, 2024

I was recently working in three_d which uses gltf in it's asset parser. I realized that neither three_d nor gltf actually had support for WebP.

I noticed some unmerged PR's and decided to update them in hopes of getting this in (and a dependent PR in three_d merged as well).

Going down the rabbit hole, I noticed some gremlin interaction with the allow_empty_textures feature.

  • Validate needs to also acknowledge webp's potential existence
  • Don't want to hide things from the gltf_json::Texture as it is the source of truth (ie make source private and provide a helper source())

I took the approach of making the source() function in texture use a primary_source() function, which prefers the webp and falls back to whatever was there originally.

However, I believe the most accurate solution is to permanently enable allow_empty_textures, and remove that flag, and make applications handle webp images (if they have the EXT_texture_webp feature enabled).
Perhaps allow_empty_texture in its off-state is not gltf compliant anyways?

Original PR: #365
Related: #413

@marstaik marstaik changed the title Add support for WebP Add support for EXT_texture_webp v2 Jun 19, 2024
@alteous
Copy link
Member

alteous commented Jul 3, 2024

Thanks!

@alteous alteous merged commit a29fb75 into gltf-rs:main Jul 3, 2024
1 check 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