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 WebP extension #448

Closed
wants to merge 5 commits into from
Closed

Add support for WebP extension #448

wants to merge 5 commits into from

Conversation

OmarShehata
Copy link

@OmarShehata OmarShehata commented Jan 10, 2019

This PR adds support for the EXT_image_webp extension. It makes sure to take into account for the image defined in the extension if found when combining images into buffers.

@lilleyse
Copy link
Contributor

Make sure to add a test. Otherwise looks good.

@OmarShehata
Copy link
Author

@lilleyse just did! I didn't want to @ mention you until I did 😄

@lilleyse
Copy link
Contributor

Actually, would it make sense to wait on this until a glTF extension is in place? Only png and jpeg are officially supported.

@OmarShehata
Copy link
Author

I think that makes sense. I could start working on putting together a draft.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 11, 2019

Absolutely please write the extension concurrent to this; if the extension is held up then merge this to a non-master branch. See KhronosGroup/glTF#1503.

@lilleyse
Copy link
Contributor

lilleyse commented Jan 14, 2019

@OmarShehata Offline we talked about what changes gltf-pipeline would need once the extension is in place. Anywhere images are processed it will need to check for the WebP extension and process the WebP images too. A good way to do this is look at wherever ForEach.image is called.

@OmarShehata
Copy link
Author

I've expanded the scope of this PR. It should now correctly process glTF's with the proposed EXT_image_webp extension. I used this branch to generate the test model for the implementation in CesiumJS CesiumGS/cesium#7486

@OmarShehata OmarShehata changed the title Add WebP to image extension header check Add support for WebP extension Jan 15, 2019
@OmarShehata
Copy link
Author

Superseded by #450

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.

4 participants