search

lokesh / color-thief

Grab the color palette from an image using just Javascript. Works in the browser and in Node.
https://lokeshdhakar.com/projects/color-thief/
MIT License
12.46k stars 1.31k forks source link

Error thrown when sampled image is white #40

Open dg-robinson opened 10 years ago

dg-robinson commented 10 years ago

I've just set up Color Thief for a project I'm working on, and during my testing it threw an error when the uploaded image was a white logo on a transparent background. I tested again with a completely white image, and the same problem occurred.

Looking at the code, this section checks if the colour value is greater than rgb(250, 250, 250). I tried taking that if statement out and it adversely affected performance. Is there a better way to handle this? Perhaps a more graceful method of failure?

hitchcockwill commented 10 years ago

Also running into this issue right now. I commented out the same line. What happened to performance for you once you took it out?

gkoberger commented 10 years ago

Also ran into this same problem. I personally fixed it by returning ['#fff'] right above this line if pixelArray.length === 0.

 var cmap    = MMCQ.quantize(pixelArray, colorCount);
clarkwinkelmann commented 3 years ago

I see there are multiple issues opened about this. I'll comment on the oldest one since I'm unsure if/how duplicate issues are handled. (is this still actively maintained btw?)

We are using this project as part of https://github.com/flarum/core and automatically call getColor on user avatars to establish the profile page background. The consequence is that setting a white image as avatar crashes the single page application.

Reading existing issues, I understand it cannot handle white backgrounds by design. So we will probably adapt to ignore white backgrounds.

Even if white images are technically unsupported, it would be nice to have better error handling. In the browser, it results in this.getPalette(...) is null error when calling getColor, so there's no way of catching the error cleanly since it happens inside of a ColorThief method.

Ideally getColor should return null itself, throw a dedicated error message if the underlying call to getPalette fails, or even better getPalette() should throw a custom error for this exact use case (something like "no valid pixels to sample" ?).

Possible errors should also ideally be described in the API documentation https://lokeshdhakar.com/projects/color-thief/#api There's no mention of getPalette() able to return null, and no mention of getColor() throwing errors.

I'm happy to provide a PR if there's interest in changing the way the error is handled.