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 webp support to texture loader #2600

Closed
wants to merge 3 commits into from
Closed

Conversation

hanabi1224
Copy link

Objective

  • This PR adds webp image format support to TextureLoader behind an optional feature webp

Solution

  • Since webp support in image crate is very limited (e.g. lack of ALPH support), it uses FFI bindings to libwebp instead.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Aug 5, 2021
@DJMcNab
Copy link
Member

DJMcNab commented Aug 5, 2021

I think there are places where we document our features

We need to do this for this one (we should also mention that it uses native libraries, so will be more difficult to build for someone)

@hanabi1224
Copy link
Author

@DJMcNab Thanks! Updated changelog and doc as suggested.

CHANGELOG.md Outdated
@@ -92,6 +92,7 @@ current changes on git with [previous release tags][git_tag_comparison].
- [Added `set_minimized` and `set_position` to `Window`][1292]
- [Example for 2D Frustum Culling][1503]
- [Add remove resource to commands][1478]
- [Add webp as a supported texture format (C compiler required)][2600]
Copy link
Contributor

Choose a reason for hiding this comment

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

Thats the Changelog of the already released 0.5
0.6 does not have a Changelog yet.

Copy link
Author

Choose a reason for hiding this comment

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

fixed.

@NiklasEi NiklasEi added A-Rendering Drawing game state to the screen C-Enhancement A new feature and removed S-Needs-Triage This issue needs to be labelled labels Aug 6, 2021
@mockersf
Copy link
Member

mockersf commented Aug 9, 2021

Didn't find an easy answer with a quick read, is the link to libwebp static or dynamic?

@hanabi1224
Copy link
Author

hanabi1224 commented Aug 9, 2021

@mockersf It's statically linked, regarding license, could you please point me to where I should put up a notice, code doc or feature doc? Personally, I feel it belongs to a more detailed license section in readme.md

btw, libwebp has a github mirror repo

@mockersf
Copy link
Member

mockersf commented Aug 9, 2021

we don't have a good place yet for that, I guess for now mentioning this in #1885 when it gets merged should be enough

@inodentry
Copy link
Contributor

So, this PR is stale ... but here is what could happen given the status in current bevy.

From looking at the source code, I see that we are enumerating WebP (incl many others) in enum ImageFormat: https://github.com/bevyengine/bevy/blob/main/crates/bevy_render/src/texture/image.rs#L31

However, there is no actual asset loader enablement for it (and a few of the other formats listed there btw).

It is lacking here: https://github.com/bevyengine/bevy/blob/main/crates/bevy_render/src/texture/image_texture_loader.rs#L20
and here: https://github.com/bevyengine/bevy/blob/main/crates/bevy_render/src/texture/mod.rs#L39

For anyone wanting to pick this up, you could look at the places linked above, as well as the code that was in this PR, for reference. It seems like it should be relatively straighforward to bring this up to date to current bevy.

@james7132
Copy link
Member

This has been superceded by #8220.

@james7132 james7132 closed this Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Enhancement A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants