twolfson / gif-encoder

Streaming GIF encoder
The Unlicense
86 stars 10 forks source link

Tentative fix for #5 #7

Closed scinos closed 7 years ago

scinos commented 7 years ago

Fixes:

This is PR is to discuss the solution. Happy to split it into smaller PRs, create individual issues, etc. before merging.

twolfson commented 7 years ago

Wow, great work on this. Solid insight on throwing out findClosest altogether :+1:

We are going to need a test but I'll take care of that

twolfson commented 7 years ago

This has been merged/released in 0.6.0. Thanks for the awesome work again on this =)

scinos commented 7 years ago

Glad to help :)

For the record, the problem was that, when a color is duplicated in the color tablet, findClosest returned the first one. I changed it to return the last one, but it was still broken for me. I didn't put a lot of energy in trying to understand what lookupRGB is returning in case of repeated colors, specially because we can just use lookupRGB and forget about findClosest alltogether.

twolfson commented 7 years ago

Ah, good research. I guess I assumed NeuQuant wouldn't have duplicates but it's not exactly something straightforward =X