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

@RunDevelopment
Copy link
Contributor Author

RunDevelopment commented Nov 22, 2025

There doesn't seem to be a whole lot of benefits to bundling all these different formats together in a single crate.

Speaking from the perspective of someone new to Rust and its ecosystem (aka myself 2 years ago), one of the major advantages of image is its completeness and ease of use. I can just throw an image at it, and it will probably be able to read it and even save with the same format. That's very convenient.

With plugins, I'd have to:

  1. Search whether a plugin for the format I want exists.
  2. Decide whether I trust the author of the plugin.
  3. Select one crate if several competing crates exist. (And there probably will be several, all with varying levels of quality, correctness, and completeness.)

For one format, this might be manageable, but this adds up quickly. Compare this to "image or image-extras supports it." It's just easier for users.

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.

Second, it puts additional maintenance burden on the image-rs team.

I don't think this necessarily has to be the case, depending on what image-extras becomes. I agree with you if image-extras primarily contains novel implementations of image decoders. But if it primarily acts as a compatibility layer between image and third-party libraries, then I don't see this being too big of an issue. (Anything in between is also possible of course.)

Basically, if format implementations, like dds, are independent crates and image-extras just uses those crates to implement a plugin, then the maintenance burden could be minimized. Of course, this then comes with the downside of having to trust crates that the image-rs org does not control. (Not that this is a concern for dds.)

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.

👍

That would like only involve decoding support, right? And even if encoding won't be part of image-extras, I think this would be a win.

@fintelia
Copy link
Contributor

Just finished getting through the PR backlog on image-extras. Reviewing changes to decoders used by image tends to be a very slow process because I have to read over the changes carefully to verify correctness (sometimes even cross-checking against the spec) and ensure the code will be sufficiently performant/maintainable going forward. On image-extras I just skimmed enough to make sure it wasn't doing anything too unreasonable.

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 image.

There doesn't seem to be a whole lot of benefits to bundling all these different formats together in a single crate.

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 image), it is nice to have a single repository rather than having to track down a whole bunch of crate authors who may no longer be interested in working on their crates.

Basically, if format implementations, like dds, are independent crates and image-extras just uses those crates to implement a plugin, then the maintenance burden could be minimized. Of course, this then comes with the downside of having to trust crates that the image-rs org does not control.

In most cases, I'd actually prefer the code be directly in the image-extras crate. Even if a decoder needs several thousand lines of code, most of the formats are simple enough that I don't expect much ongoing development. And then we don't have to worry about the supply chain implications of having lots of small dependencies not under our control.

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