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

Document unsupported formats #6

Merged
merged 4 commits into from Apr 25, 2017
Merged

Conversation

gianluca-nitti
Copy link
Contributor

@gianluca-nitti gianluca-nitti commented Sep 11, 2016

After failing to convert a png to ico (I was getting an ico with pixels of random colors), I did some research and I found out that my image was in the indexed colormap format (8 bits per pixel), while pngjs only supports 8 bits per channel depth. I suggest adding this to the readme so users can find this out without needing to digg in the documentation of the underlying modules.

@jdalton
Copy link

jdalton commented Sep 13, 2016

I hit this and was wondering why I had a corrupt ico. This explains it.

@sindresorhus sindresorhus changed the title Documentation about unsupported formats. Documentation about unsupported formats Apr 25, 2017
@sindresorhus sindresorhus changed the title Documentation about unsupported formats Document unsupported formats Apr 25, 2017
@sindresorhus sindresorhus merged commit a19752a into kevva:master Apr 25, 2017
@sindresorhus
Copy link

Merging this, but we should really find a better PNG encoder/decoder: #16

@kevva
Copy link
Owner

kevva commented May 19, 2017

Some of this is incorrect. First and foremost, we're not using the linked module, but https://github.com/lukeapage/pngjs which does support interlacing. Indexed color is still unsupported though.

@gianluca-nitti
Copy link
Contributor Author

First and foremost, we're not using the linked module, but https://github.com/lukeapage/pngjs

Oh I see now, sorry for the misunderstanding. Most likely I got confused by the two modules having very similar names on npm (pngjs and node-pngjs).

@kevva
Copy link
Owner

kevva commented May 19, 2017

No worries, it's understandable since there are a lot of forks of it with similar names :).

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

4 participants