Add support for DDS decoding

[RunDevelopment]

This PR takes the decoder part (and ONLY the decoder part) of image-rs/image#2461 and adds it to image-extras. As such, the implementation is relatively simple, since the dds crate does the actual decoding work.

Q: Should I also include the encoder part of image-rs/image#2461? If so, the testing infra should probably be extended to test encoders.

---

The only thing which confused me is that this works at all. The image crate currently still supports DDS images, so I don't really understand why it lets me add support for DDS as a plugin. With both the image/dds and image-extras/dds features enable, image will use the plugin instead of its own DDS implementation. I'm not sure if this behavior is intentional, but it is useful in this case, because image's DDS implementation can't decode any of my test images.

[RunDevelopment]

Tests currently don't pass, because the testing infrastructure doesn't support Rgba32f and Rgb32f images. I'm currently working on a PR for that.

[197g]

I'm not sure if this behavior is intentional, but it is useful in this case, because image's DDS implementation can't decode any of my test images.

Yes, that's precisely the use-case for which the override was designed. Good to see it working correctly in action.

[RunDevelopment]

I just updated to use TIFF reference images for float32 images, but they are a bit weird. I used image to create them via DynamicImage::save, so this might be a problem with the TIFF encoder. Anyway, the issue is that most image editing programs I opened the TIFF images with display the colors incorrectly.

This is what it's supposed to look like:

This is how Paint.net, Photoshop, and Windows Fotos (on Windows 10) show it:

It kind of looks like these programs interpret the image data as linear and convert it to sRGB. From all programs I tested, only gimp and image open the TIFF images with the correct colors.

So I suspect that something is going wrong with the way image creates float32 TIFF images. Note that if I convert the image data to u16 or u8 before saving as TIFF, all programs I tested opening the TIFF files generated by image correctly.

(The image data is also completely uncompressed. I thought TIFF supports lossless compression.)

[fintelia]

The tiff crate supports compression but I don't think the option is wired up to the image crate at the moment. Probably wouldn't be too hard to switch it to always write LZW compressed TIFF files

[RunDevelopment]

Good to know. I might make a PR for that later, since it's really unexpected that image produces these huge files by default. (Edit: Just found the discussion in image-rs/image#2251. I ain't touching TIFF 😄)

In any case, I'm not sure if the TIFF color issue should block this PR. I mean, if different decoders cannot agree on how to interpret a reference file, is it a good reference?

---

Also, are there any plans to extend image's hooks API to include image encoding as well? Would be nice if both loading and saving "just works". E.g. dyn_image.save("my file.dds").

[fintelia]

There's no immediate plans to add hooks for image encoding, though you can already use DynamicImage::write_with_encoder to write images with encoders implemented outside of the image crate.

And yeah, the TIFF issue shouldn't be blocking. I'm pretty sure that whatever color differences are happening with the viewers you're using, they're being applied after reading the raw pixel data. And since the reference tests don't attempt to apply tone mapping those difference shouldn't matter

[RunDevelopment]

There's no immediate plans to add hooks for image encoding

That's unfortunate.

And yeah, the TIFF issue shouldn't be blocking.

Then it's ready to review.

I have the DDS encoder ready as well, but I'll make that a separate PR to not hold off this one or release (in case you're waiting on DDS support).

[RunDevelopment]

Question: What's the policy for trait bounds in structs? All encoders and decoders are generic over the writer/reader. Should the struct contain that trait bound? So E.g. struct DdsEncoder<W: Write> { w: W }.

[197g]

The struct itself probably should not contain trait bounds. There's a bunch of structs that have been declared a long time ago, and not touched up since.

[197g]

@fintelia: In my opinion, the constructor should have a bound even if it does not read itself. But we never made an explicit decision on that policy iirc.

[RunDevelopment]

I agree with that. On a similar note: what should the trait bound on the reader for decoders be?

When implementing ImageDecoder, image use BufRead + Seek (bmp, gif, ico, jpeg, exr, png, tiff, webp) and plain Read (avif, old dds, farbfeld, hdr, pnm, qoi, tga). I used Read for the DDS decoder in this PR, because it doesn't need more than that, but all other decoders in this repo use BufRead + Seek (probably because that's what the plugin API gives us).

[fintelia]

Whether to have a Read or BufRead + Seek bound doesn't matter too much. We can be more flexible with semver breaking changes in this crate, so changing later is less of a problem.

What sources/licensing status for the test images?

[RunDevelopment]

What sources/licensing status for the test images?

The cubemap is Pretoria Gardens, which is CC0. The other images I created myself. If you need a license for those too, then they are hereby CC0.