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

Add BGRA and BGR support. #666

Closed
wants to merge 1 commit into from

Conversation

quadrupleslap
Copy link

Also allows TGA and PPM encoders to be used with them.

Resolves #451 (I think.)

@nwin
Copy link
Contributor

nwin commented Jul 11, 2017

I think it would be a better option to convert it to RGB for now. I'm hesitant adding new values to that enumeration since it changes a lot of public API.

@quadrupleslap
Copy link
Author

Fair enough, but can you at least check over the PPM encoder? It's short, but very strangely written, e.g. let _ = try!(...); is everywhere, and I'm not sure why it's calling write_all three times with an array of one element, instead of once with an array of three elements.

@bvssvni
Copy link
Contributor

bvssvni commented Apr 17, 2018

The PPM encoder was changed in #730, so I think this should be closed. Reopen if you want to make another attempt.

@bvssvni bvssvni closed this Apr 17, 2018
@quadrupleslap
Copy link
Author

quadrupleslap commented Apr 18, 2018

That's fine, I don't use this library any more, but in any case:

  • Not many formats natively support BGRA, so a conversion has to happen somewhere.
  • Conversions probably belong in another library, not this one.

So not adding BGRA was probably the right decision.

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.

None yet

3 participants