From 0993484164619bc64647530e46cb727d0356ed79 Mon Sep 17 00:00:00 2001
From: Thomas Weber <weber_t@mailbox.org>
Date: Fri, 24 Sep 2021 20:56:16 -0500
Subject: [PATCH] Fix color conversion bugs

---
 src/p4/color-utils.js       |  7 +++----
 test/p4/color-utils.test.js | 26 ++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/src/p4/color-utils.js b/src/p4/color-utils.js
index 8f11cd2c..12c7454c 100644
--- a/src/p4/color-utils.js
+++ b/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 {
diff --git a/test/p4/color-utils.test.js b/test/p4/color-utils.test.js
index e6dcc216..faaa4c30 100644
--- a/test/p4/color-utils.test.js
+++ b/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);
+  }
+});