Skip to content

Conversation

@RunDevelopment
Copy link
Contributor

@RunDevelopment RunDevelopment commented May 1, 2025

Resolves #2435
Resolves #2093

In this PR, I removed the old DDS decoder implementation and added a new decoder and encoder based on the dds crate.

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:

  • Add dds as a (private) dependency.
  • Use dds to power a new DDS de/encoder API for image. I made all new types, so nothing from dds is re-exported/used publicly.
  • Removed old DXT implementation. This was already non-public, so I don't think this will cause any issues.

TODO:

  • Add more documentation. The DdsDecoder and DdsEncoder structs should have a lengthy doc comment explaining what they can do and how they are supposed to be used.
  • Add tests.

@RunDevelopment RunDevelopment changed the title Dds New DDS decoder and encoder May 1, 2025
@RunDevelopment RunDevelopment mentioned this pull request May 1, 2025
@RunDevelopment
Copy link
Contributor Author

I fixed the error for test_toolchains (1.70.0) in image-rs/image-dds#59, so just cargo-deny remains. It doesn't like that dds uses a newer version of zerocopy than some other dependency. I'm not sure what the best solution for that is. Should I downgrade the version to v0.7.x, or should I try to make dds work with both v0.8.x and v0.7.x? dds doesn't use a lot of zerocopy's APIs, so this might be possible.

@RunDevelopment
Copy link
Contributor Author

Nice, cargo-deny solved itself. I haven't looked into it, but I think the other dep got updated and now uses zerocopy v0.8.x.

@nickbabcock
Copy link

I tested this branch and am very pleased. The current dds decoder is very limiting and confusing (returning errors like "The image format DDS is not supported") that it should be called out in the docs. This branch fixed all those issues.

Excited to see this land.

@Shnatsel
Copy link
Member

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 image has a plugin API, I think it's better to have this as a format plugin rather than in-tree in image. That will prevent your work from getting blocked on upstream review (like it happened here) or the image release schedule (even if this were merged today, the next version is 1.0 and it will likely take months to ship).

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
Encoding is easier, here's an example: https://docs.rs/webp/latest/webp/struct.Encoder.html#method.from_image

@RunDevelopment
Copy link
Contributor Author

Thanks for the update!

Okay, let's see if I understand this correctly: So the basic idea is to invert the dependency. Instead of image depending on dds, image will be completely independent with dds depending on image and its plugin API.

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 dds and just add a pub fn register_image_detection_hook behind a feature.

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 hooks module or registration-related functions could provide recommendation for plugins authors that answer the following questions:

  1. What's the preferred ways to make their users register hooks? (I think the answer here is: with a public function.)
  2. Assuming the answer to (1) is a pub fn, what should the name of that function be?
  3. Should the function be allowed to be called multiple times? (I think the answer here is: yes)
  4. If image support is behind a feature:
    1. What should the feature be called? (webp called it img, jxl doesn't have one, I'm tending towards image).
    2. Should the register function be behind the feature? If not, should it do nothing?

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 image-extras to include DDS support? image previously supported DDS files, but since 1.0 will get rid of that, I believe it would be nice if image could provide a more first-party way to restore old functionality via image-extras. At the very least, this will make it easier for users to discover that DDS support exists.

@Shnatsel
Copy link
Member

So the basic idea is to invert the dependency. Instead of image depending on dds, image will be completely independent with dds depending on image and its plugin API.

Yes, that is correct.

I should be able to just move over most of the implementation in this PR to dds and just add a pub fn register_image_detection_hook behind a feature.

Indeed. I think it's actually a better idea to make it a separate crate, like dds-image-plugin or something, so that the dds crate doesn't have to break semver every time image does. It only happens every ~2 years, so it's not a big deal either way. There is no established name for this sort of crate yet, so you get to set the trend!

Different libraries may choose different names, so it would be nice to have some guidance to ensure uniformity across the whole (currently-establishing) ecosystem.

That's a good point. In jxl-oxide it's integration::register_image_decoding_hook and in image-extras it is simply register().

What's the preferred ways to make their users register hooks? (I think the answer here is: with a public function.)
Should the function be allowed to be called multiple times? (I think the answer here is: yes)

That is all correct.

At the very least, this will make it easier for users to discover that DDS support exists.

I agree discoverability is a concern. Maybe we can put a list of known crates using the plugin API in the image README.

@RunDevelopment
Copy link
Contributor Author

RunDevelopment commented Nov 22, 2025

I think it's actually a better idea to make it a separate crate, like dds-image-plugin or something, so that the dds crate doesn't have to break semver every time image does.

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 register_hooks functions of the plugin crate to get the decoder functionality, but this is not the case for encoder functionality. Users have to directly use the underlying encoder crate with image types in some form (see the webp example you linked).

dds is both decoder and encoder, so the encoder part has to interface with image's image types on some level. Unlike webp however, dds has abstractions over image slices (ImageView and ImageViewMut), so it's a bit easier. It's just a question of how to convert image types to dds types. This could be done in a separate crate via e.g. extension traits to get an ergonomic API.

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 dds, but also most of the ecosystem, so third-party libraries are a little consistent.

I agree discoverability is a concern. Maybe we can put a list of known crates using the plugin API in the image README.

In addition to such a list, an official/recommended keyword like image-plugin would probably also be a good idea. That way, this sentence in the README could link to a keyword search on crates.io that directly lists available plugins.

@fintelia
Copy link
Contributor

I'm in favor of including DDS in image-extras. I'll be spending some time this weekend trying to get that crate ready for a first release. Would be nice if we could announce that any formats dropped from image in the 1.0 release were just a cargo add away via image-extras.

@Shnatsel
Copy link
Member

I've been wondering, is image-extras even needed now that the plugin interface exists?

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?

@mstoeckl
Copy link
Contributor

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.

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 :-).

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 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 no_std support, fuzzing, truncation tests, panic-free code style, memory limits, documentation, code deduplication) because the implementations are harder to find, and the cost of developing and upstreaming changes isn't worth it for any individual format.

(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.)

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.

DDS support Encoding DDS images?

5 participants