quantize
quantize copied to clipboard
Correct the number of colors returned by quantize
Closes #9 The number of colors returned by quantize should match maxcolors, including for values > 7.
I've tested this fix using a test.js from a separate project that requires quantize, for many values ranging from 1 to 256, and this fix corrects the off-by-one color count.
A test.js and a Readme update would help to support this Pull Request. Let me know if you're interested.
Curious if anyone else has noticed or investigated this behavior?
I created Mocha tests for the quantize function, asserting that the format of all returned palettes is valid, for a variety of inputs.
For each of the tests below, I run it using many values of maximumColorCount between 2 and 256:
✓ works on 1-pixel 1-color images
✓ works on 5-pixel 1-color images
✓ works on 1000-pixel 1-color images
✓ works on 5-pixel 5-color images
✓ works on 1000-pixel 5-color images
✓ works on 20-pixel 20-color images
✓ works on 1000-pixel 20-color images
The most interesting results are how the returned ("Actual") count of colors lines up with the requested ("Expected") maximum color count.
Using the current quantize code: 1-pixel: always returns 1 color More than 1 pixel: Actual: 3, Expected: 2 Actual: 4, Expected: 3 Actual: 4, Expected: 4 Actual: 5, Expected: 5 Actual: 6, Expected: 6 Actual: 7, Expected: 7 Actual: 7, Expected: 8 Actual: 8, Expected: 9 Actual: 9, Expected: 10 Actual: 10, Expected: 11 Actual: 11, Expected: 12 Actual: 12, Expected: 13 Actual: 13, Expected: 14 Actual: 14, Expected: 15 Actual: 15, Expected: 16 Actual: 31, Expected: 32 Actual: 63, Expected: 64 Actual: 127, Expected: 128 Actual: 255, Expected: 256
Using my proposed change, adding "+ 1" to current quantize code line 474: 1-pixel: always returns 1 color More than 1 pixel: Actual: 3, Expected: 2 Actual: 4, Expected: 3 Actual: 4, Expected: 4 Actual: 5, Expected: 5 Actual: 6, Expected: 6 Actual: 7, Expected: 7 Actual: 8, Expected: 8 Actual: 9, Expected: 9 Actual: 10, Expected: 10 Actual: 11, Expected: 11 Actual: 12, Expected: 12 Actual: 13, Expected: 13 Actual: 14, Expected: 14 Actual: 15, Expected: 15 Actual: 16, Expected: 16 Actual: 32, Expected: 32 Actual: 64, Expected: 64 Actual: 128, Expected: 128 Actual: 256, Expected: 256
Two conclusions:
- For requested ("Expected") maximum color count of 2 and 3, the response includes one too many - exceeding the maximum - in both the original code and my change.
- For requested ("Expected") maximum color count greater than 7, my change corrects the issue and responds with the exact amount requested.
After testing AlfredJKwack's change from #6 , I've seen that it returns the exact count of colors requested for all input values between 2 and 256! I've replaced my partial correction with his superior fix.
// @olivierlesnicki
+1 for this!
and, if anyone else stumbles across this I have a fork with this pull request, which seems to fix the issue, implemented here:
https://github.com/alexharris/quantize