Fix color conversion bugs

src/p4/color-utils.js

@@ -20,7 +20,7 @@ export const hexToRgb = (hex) => {
 };
 
 export const rgbToHex = (r, g, b) => {
- return '#' + Math.round(r).toString(16).padStart(2, '0') + Math.round(b).toString(16).padStart(2, '0') + Math.round(g).toString(16).padStart(2, '0');
+ return '#' + Math.round(r).toString(16).padStart(2, '0') + Math.round(g).toString(16).padStart(2, '0') + Math.round(b).toString(16).padStart(2, '0');
 };
 
 export const rgbToHsv = (r, g, b) => {
@@ -33,9 +33,8 @@ export const rgbToHsv = (r, g, b) => {
  s = max == 0 ? 0 : d / max;
  if (max == min) {
  h = 0;
- if (min === 0 || min === 1) {
- // Saturation does not matter in the case of pure white or black
- // In these cases we'll set saturation 1 to provide a better editing experience
+ if (min === 0) {
+ // Saturation does not matter in the case of pure black, so just set it to 1 instead so the editor makes more sense
  s = 1;
  }
  } else {

test/p4/color-utils.test.js

@@ -14,14 +14,40 @@ test('rgbToHex', () => {
  expect(rgbToHex(0, 0, 0)).toEqual('#000000');
  expect(rgbToHex(255, 255, 255)).toEqual('#ffffff');
  expect(rgbToHex(255, 0, 0)).toEqual('#ff0000');
+ expect(rgbToHex(0, 255, 0)).toEqual('#00ff00');
+ expect(rgbToHex(0, 0, 255)).toEqual('#0000ff');
+ expect(rgbToHex(0xab, 0xcd, 0x12)).toEqual('#abcd12');
+ expect(rgbToHex(1, 2, 255)).toEqual('#0102ff');
 });
 
 test('rgbToHsv', () => {
  expect(rgbToHsv(0, 0, 0)[0]).toEqual(0);
+ // (saturation does not matter in pure black)
  expect(rgbToHsv(0, 0, 0)[2]).toEqual(0);
+ expect(rgbToHsv(255, 255, 255)).toEqual([0, 0, 1]);
  expect(rgbToHsv(255, 0, 0)).toEqual([0, 1, 1]);
 });
 
 test('hsvToRgb', () => {
  expect(hsvToRgb(0, 0, 0)).toEqual([0, 0, 0]);
 });
+
+test('round-trip accuracy', () => {
+ for (const color of [
+ '#4c5aff',
+ '5a4cff',
+ '000000',
+ 'ffffff',
+ 'abcdef',
+ '123456',
+ '002348',
+ 'acbd38'
+ ]) {
+ const rgb = hexToRgb(color);
+ const hsv = rgbToHsv(...rgb);
+ const rgb2 = hsvToRgb(...hsv);
+ expect(Math.abs(rgb2[0] - rgb[0])).toBeLessThan(0.0001);
+ expect(Math.abs(rgb2[1] - rgb[1])).toBeLessThan(0.0001);
+ expect(Math.abs(rgb2[2] - rgb[2])).toBeLessThan(0.0001);
+ }
+});