-
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.) |
Speaking from the perspective of someone new to Rust and its ecosystem (aka myself 2 years ago), one of the major advantages of With plugins, I'd have to:
For one format, this might be manageable, but this adds up quickly. Compare this to " To draw a comparison to another language: In Python, there are basically 2 python libraries that you use to decode and encode images: OpenCV and PIL. Both include support for dozens of formats, simply because it's easier for users. Note that neither supports all formats particularly well, but it's a good-enough baseline. This approach makes it very easy to load and save images in Python, because these libraries are good enough for most use cases. It's just very easy to be productive as a user of these libraries. And if you need more (e.g. better encoders, more encoder settings, more formats) you have to use a library specialized for your format. I'm not saying the situation with Python is perfect, but it has some nice properties that I think make it worth considering taking a similar approach.
I don't think this necessarily has to be the case, depending on what Basically, if format implementations, like
👍 That would like only involve decoding support, right? And even if encoding won't be part of |
|
Just finished getting through the PR backlog on My hope is that the maintenance burden should be relatively small. And if it turns out to be too much work, we always can deprecate the crate whenever. We wouldn't even have to wait for a semver breaking release of
The benefits I see are around standardization and making small changes easier. For people who need to work with these formats in the future, it is nice to have a common structure for test cases and well defined licensing status for them. And a listing of where the specifications for each format are. And when it is necessary to update the decoders (say because there's been a semver breaking release of
In most cases, I'd actually prefer the code be directly in the |
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.