image-rs / image

Encoding and decoding images in Rust
Apache License 2.0
4.98k stars 618 forks source link

Support for 15-bit and 16-bit TGA file formats #559

Open HarryLovesCode opened 8 years ago

HarryLovesCode commented 8 years ago

Hello, I noticed recently that the TGA decoder didn't support 15-bit and 16-bit formats. This led me to ask the question, does this crate handle non-8-bit components? From what I understand, it doesn't (see dynimage.rs).

So, if I were to implement support for TGA file formats that use 5-bits per component, how would I need to represent them? Should we add support in the DynamicImage file I linked, or should they be scaled and cast to u8s?

nwin commented 8 years ago

There is limited support for 16-bit formats. DecodingResult::U16 exists but it is not used in dynimage.rs#L450. The most sensible approach would be to just scale it down. :)

When implementing reading uN, (N%8 != 0) image types you have two options. Scale it to the next bigger uint and lie about the ColorType. Or just read it into an u8 and modify dynimage.rs#L450 in order to scale it, which would be the better approach. Have a look at dynimage.rs#L473 on how 1, 2 and 4 bit grayscale images are handled.

Contribution are very welcome. :)

philipc commented 8 years ago

The BMP decoder is currently scaling to u8 and lying about the ColorType. It can handle any non-8-bit components. (It's also scaling down in some cases which isn't ideal, but those are unusual BMP formats.) @nwin Is it worth moving that scaling out of the BMP decoder? My main concern with that is it might be slower, but not sure how much that matters.

nwin commented 8 years ago

You’re right it’s not worth it.