Skip to content
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

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

Open
HarryLovesCode opened this issue Jul 12, 2016 · 3 comments
Open

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

HarryLovesCode opened this issue Jul 12, 2016 · 3 comments

Comments

@HarryLovesCode
Copy link
Contributor

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
Copy link
Contributor

nwin commented Jul 12, 2016

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
Copy link
Contributor

philipc commented Jul 12, 2016

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
Copy link
Contributor

nwin commented Jul 16, 2016

You’re right it’s not worth it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Version 0.23
  
New Features
Development

No branches or pull requests

4 participants