DX: Allow using a prepared palette for performance, enable receiving/returning hex colors

[benjie]

This PR builds on #14 (and #13) and allows the user to reap the majority of the performance benefits of #14 without having to work in the Lab-colorspace assuming they're using the same palette over and over (as I am).

Further, this PR allows colors to be input in #rrggbb format. It does NOT support #rgb, #rgba, or #rrggbbaa formats at this time (though we could easily do that by switching on the string length).

rnd.gen 2.6455512711991234 ± 0.12131436269185036
colorDiff.closest 160.00713217160924 ± 7.918198857054954
colorDiff.closest [predef] 147.80799770923468 ± 4.87628525715313
colorDiff.closest [predef, hex] 165.18798979520074 ± 6.254912585153722
colorDiff.closest [prepared, predef] 40.89235910779613 ± 1.2220039687832747
colorDiff.closest [prepared, predef, hex] 41.27558060008121 ± 1.6421383218742023
lib.palette_lab [predef] 0 ± 0

Benchmark code

```js const Benchmark = require('benchmark'); const colorDiff = require('./lib'); const suite = new Benchmark.Suite; const rnd = function () { return { R: Math.floor(Math.random() * 256), G: Math.floor(Math.random() * 256), B: Math.floor(Math.random() * 256), }; }; const rndA = function () { return [ rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), rnd(), ]; }; const pad = s => s.length === 1 ? "0" + s : s; const hex = n => pad(n.toString(16)); const rgb_to_hex = c => `#${hex(c.R)}${hex(c.G)}${hex(c.B)}`; const predefTarget = rnd(); const predefPalette = rndA(); const predefTargetHex = rgb_to_hex(predefTarget); const predefPaletteHex = predefPalette.map(rgb_to_hex); const predefTargetLab = colorDiff.rgb_to_lab(predefTarget); const predefPaletteLab = predefPalette.map(colorDiff.rgb_to_lab); const predefPalettePrepared = colorDiff.prepare_palette(predefPalette); const predefPalettePreparedHex = colorDiff.prepare_palette(predefPaletteHex); suite .add('rnd.gen', function () { return [rnd(), rndA()]; }) .add('colorDiff.closest', function () { return colorDiff.closest(rnd(), rndA()); }) .add('colorDiff.closest [predef]', function () { return colorDiff.closest(predefTarget, predefPalette); }) .add('colorDiff.closest [predef, hex]', function () { return colorDiff.closest(predefTargetHex, predefPaletteHex); }) .add('colorDiff.closest [prepared, predef]', function () { return colorDiff.closest(predefTarget, predefPalettePrepared); }) .add('colorDiff.closest [prepared, predef, hex]', function () { return colorDiff.closest(predefTargetHex, predefPalettePreparedHex); }) .add('lib.palette_lab [predef]', function () { return colorDiff.palette_lab(predefTargetLab, predefPaletteLab, true); }) .on('complete', function () { for (let i = 0; i < 7; i += 1) { console.log(this[i].name, this[i].stats.mean * 1000000, '±', this[i].stats.deviation * 1000000); } }) .run({}); ```

[coveralls]

Coverage decreased (-1.7%) to 97.633% when pulling b7951d851de84c71c14f9c537c025bc4a3fc2eb2 on benjie:dx into ad7d2da284546078abeee52dbf5fe3fc421f31ac on markusn:master.

[coveralls]

Coverage decreased (-1.8%) to 97.537% when pulling cf770968fb8841eb3b276ddf79208854d3d66864 on benjie:dx into ad7d2da284546078abeee52dbf5fe3fc421f31ac on markusn:master.

[coveralls]

Coverage decreased (-1.3%) to 98.03% when pulling 39644a52d87b5580b0241b1ae4e30a1220bf9c89 on benjie:dx into ad7d2da284546078abeee52dbf5fe3fc421f31ac on markusn:master.

[benjie]

Sorry the commits in this one aren't as tidy as the others, I was in a rush.

[coveralls]

Coverage increased (+0.2%) to 99.507% when pulling 650f5d00c03047d498443830dabd01106f1b06dd on benjie:dx into ad7d2da284546078abeee52dbf5fe3fc421f31ac on markusn:master.

[coveralls]

Coverage increased (+0.2%) to 99.507% when pulling 527ea113c1192f43da01b357eeed835817aba541 on benjie:dx into ad7d2da284546078abeee52dbf5fe3fc421f31ac on markusn:master.

[coveralls]

Coverage increased (+0.2%) to 99.507% when pulling aceba80d937227ecade4e8ae333b8e3d1098c658 on benjie:dx into 098b7b8da9011ec64e0d15c4574bd65a598d2c3c on markusn:master.

[coveralls]

Coverage increased (+0.2%) to 99.507% when pulling aceba80d937227ecade4e8ae333b8e3d1098c658 on benjie:dx into 098b7b8da9011ec64e0d15c4574bd65a598d2c3c on markusn:master.

[coveralls]

Coverage increased (+0.2%) to 99.507% when pulling aceba80d937227ecade4e8ae333b8e3d1098c658 on benjie:dx into 098b7b8da9011ec64e0d15c4574bd65a598d2c3c on markusn:master.

[coveralls]

Coverage increased (+0.2%) to 99.507% when pulling aceba80d937227ecade4e8ae333b8e3d1098c658 on benjie:dx into 098b7b8da9011ec64e0d15c4574bd65a598d2c3c on markusn:master.

[markusn]

Hi! I will postpone reviewing this as the previous PR is pending.

[markusn]

Post-poning review until the conflicts with master have been resolved.

[benjie]

Yeah; I've a fair bit of re-writing ahead of me. If you don't receive an update from me within a week feel free (if not encouraged!) to give me a nudge 👍

[markusn]

Ok, I will post-pone tagging and releasing a 1.1 as well unless you need it earlier.

[coveralls]

Coverage decreased (-0.8%) to 98.63% when pulling 60139172873c77943720b97c4a34276a2e19d076 on benjie:dx into 237a313b9a7e60bb8ec28ff4fdc8b0470905003c on markusn:master.

[coveralls]

Coverage decreased (-0.8%) to 98.618% when pulling 9bb485ec6cbc42248d2e4c4b79958aa885be7a52 on benjie:dx into 237a313b9a7e60bb8ec28ff4fdc8b0470905003c on markusn:master.

[benjie]

@markusn I think this is ready for review now. Coveralls is upset because I've added tests for index which has thus dropped our coverage by 1% because I don't think it's worth testing these functions because the underlying functions already have tests:

color.closest_lab = function(target, relative) {
  return color.match_palette_lab(target, relative, false);
};

color.furthest_lab = function(target, relative) {
  return color.match_palette_lab(target, relative, true);
};

[markusn]

Ok, I will try to have a look this weekend!

[markusn]

Sorry for the delay, I got caught up with other things during the weekend.

I've had a look at the patch and to be honest I'm do not think the new functionality is a good match for this library. The added functionality makes the library code complicated (functions that take either a RGB-object or its' string representation, an array of colors or a dict with a couple of fields of which one is an array of colors) and can be implemented outside the library when needed using match_palette_lab and the appropriate X_to_lab conversion function (rgb_hex_to_lab would be a good match for this library thought).

[benjie]

I hear what you're saying - I suspected you'd say something along these lines hence why I split my work into 3 pull requests (each one less likely to be accepted than the previous one!).

It's worth noting that the dict you mention (the result of `prepare_palette`) is intended to be an opaque value, like that returned by `setTimeout`, not one that users actually inspect or generate for themselves.

_I could have made that more obvious by slapping a few more underscores around the place, and maybe using `Object.defineProperty` to make the attributes non-enumerable, but I felt that would only serve to make the code uglier!_

Just to give some background I figured that hex is a very common (and concise!) format for dealing with colours, and is much more standard than an {R,G,B} object, so would give developers wishing to use this library a more familiar interface they could use with very little effort - I implemented this whilst maintaining full backwards compatibility, which is where some of the complexity comes from.

On top of that, I wanted to allow people to reap the performance benefits of pre-calculating the L,a,b transforms when doing many comparisons against the same palette without even having to understand the existence of the Lab colourspace (they simply feed in hex, get an opaque object back which they can then feed into the regular functions to get the same results but in lower time).

A new user with this code can get really good performance by simply doing something like:

const palette = colorDiff.prepare_palette(['#000000', '#ff0000', '#00ff00', '#0000ff', '#ffffff']);
for (const row of image) {
  for (const colorHex of row) {
    console.log(colorHex, colorDiff.closest(colorHex, palette));
  }
}

which I felt was a much more familiar interface for most developers. However, I totally agree that it complicates the code; I tried to keep this to a minimum but it's your project to maintain and if you don't want these changes that's definitely your prerogative. No hard feelings whatsoever! As I mentioned at the start, I was never expecting this pull request to get merged in the first place, but felt I would be remiss to not explain my reasoning for doing the work.

Thanks for your excellent work writing out the ciede function! 👍 Feel free to take what you want from this pull request (if anything) and leave the rest.

[markusn]

Thanks for your contribution, I will consider merging the hex part in the future, in the meantime I will tag and release a 1.1 based on your previous PRs!

[benjie]

Thanks @markusn 👍