-
Notifications
You must be signed in to change notification settings - Fork 674
New DDS decoder and encoder #2461
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
base: main
Are you sure you want to change the base?
Conversation
|
I fixed the error for |
|
Nice, |
|
I tested this branch and am very pleased. The current dds decoder is very limiting and confusing (returning errors like "The image format Excited to see this land. |
|
AFAIK the DDS format is outside the expertise of all of the current maintainers, and the in-tree code was mostly inherited and poorly maintained. I am excited to see it removed and replaced by something better. Now that So I believe the format decoding plugin route is the best option for everyone involved. You can find example plugin integration for a real-world image format here: https://github.com/tirr-c/jxl-oxide/blob/main/crates/jxl-oxide/src/integration/image.rs |
|
Thanks for the update! Okay, let's see if I understand this correctly: So the basic idea is to invert the dependency. Instead of Assuming that I understood this correctly, then this should be pretty easy to do. I should be able to just move over most of the implementation in this PR to On that note: Is there any documentation how this hook registration function should be presented to consumers of my library? Since plugins need to be explicitly registered by users by calling a function exported by my library, this function needs a name. Different libraries may choose different names, so it would be nice to have some guidance to ensure uniformity across the whole (currently-establishing) ecosystem. Else we risk that users that need support for lots of formats will have to write code like this: fn main() {
format_crate1::register_image_detect_hooks();
format_crate2::register_image();
format_crate3::register();
format_crate4::load_hooks();
}I believe it would be nice if the documentation of the
Of course, different libraries have different needs, but some general guidance and recommendations to keep things consistent between libraries would be beneficial IMO. Lastly, are there any plans for |
Yes, that is correct.
Indeed. I think it's actually a better idea to make it a separate crate, like
That's a good point. In jxl-oxide it's
That is all correct.
I agree discoverability is a concern. Maybe we can put a list of known crates using the plugin API in the |
I'm not sure whether a separate crate is ideal here. I agree with your semver argument and think that a separate crate would be ideal for pure decoders, but I don't like it for encoders. For pure decoders, users basically just have to call the
However, there's an argument to be made that plugins are all about decoding and nothing more (at least right now). So why should the user need a plugin crate to encode images? Honestly, I'm torn about what the best approach would be, since they all seem to come with their own ups and downs. And ideally, the approach would work not just for
In addition to such a list, an official/recommended keyword like |
|
I'm in favor of including DDS in |
|
I've been wondering, is There doesn't seem to be a whole lot of benefits to bundling all these different formats together in a single crate. However, the drawbacks are very real. First of all, them all sharing a single semver is a major drawback. Maybe I only ever want to use a single format from image-extras, but semver breaks in any single format affect users of all formats. Second, it puts additional maintenance burden on the image-rs team. I see the plugin interface as a way to lift the burden of dealing with the myriad of obscure formats from the already busy image-rs team, but image-extras puts it right back: https://github.com/image-rs/image-extras/pulls And some of those PRs date back to February; whoever contributed it is still waiting on their format to be supported. Wouldn't it be better for everyone not to group all these obscure formats under a single crate, and instead just have a prominent list of plugins for discoverability and otherwise let them evolve independently from each other and from us? |
I wouldn't mind reviewing PRs and bug reports for any obscure formats in image-extras, if other people find any that they want to add :-).
I do not think that splitting the overall task of decoding less common image formats into many small projects produces a better overall solution than centralized libraries would. Tiny independently evolving crates tend to get abandoned. It is also harder to make systematic improvements (like (In my case, while I can find the time to make and review large and tricky changes every now and then, I am not available consistently enough to effectively maintain and publish yet another project for its decade+ expected useful lifespan.) |
Resolves #2435
Resolves #2093
In this PR, I removed the old DDS decoder implementation and added a new decoder and encoder based on the
ddscrate.Decoder: The decoder now supports a bunch more formats, cube maps, rectangle decoding, and the ability to decode as a color format of the user's choosing.
Encoder: The encoder supports a selection of DDS formats, automatic mipmap generation, dithering, compression quality, and both header formats. If the user does specify a format, the encoder will automatically pick an uncompressed that can losslessly represent the image.
Only encoding single images (with optional mipmaps) is supported. Cube maps, texture arrays, and volumes are not. They could be in the future, but I don't see the need right now.
Changes:
ddsas a (private) dependency.ddsto power a new DDS de/encoder API forimage. I made all new types, so nothing fromddsis re-exported/used publicly.TODO:
DdsDecoderandDdsEncoderstructs should have a lengthy doc comment explaining what they can do and how they are supposed to be used.