mdbassit / Coloris

A lightweight and elegant JavaScript color picker. Written in vanilla ES6, no dependencies. Accessible.
https://coloris.js.org/examples.html
MIT License
461 stars 58 forks source link

Alpha channel loses information #149

Closed ut4 closed 1 month ago

ut4 commented 5 months ago

When pasting a transparent color into the picker, such as #000000d7, it gets converted to #000000d6 (another example is #00000022 -> #00000021). I believe this is due to the a * 255 | 0 conversion in the RGBAToHex function. Is this conversion necessary, or could it be removed? Here is a proposed fix that addresses the issue:

@@ -922,16 +922,23 @@
     if (rgba.b < 16) {
       B = '0' + B;
     }

     if (settings.alpha && (rgba.a < 1 || settings.forceAlpha)) {
-      const alpha = rgba.a * 255 | 0;
-      A = alpha.toString(16);
-
-      if (alpha < 16) {
-        A = '0' + A;
-      }
+      const alphaHex = rgba.a.toString(16); // Examples: float (hex) -> result
+                                            //           0     (00)  -> '0'
+                                            //           0.01  (01)  -> '0.028f5c28f5c28f6'
+                                            //           0.133 (22)  -> '0.220c49ba5e354'
+                                            //           0.843 (d7)  -> '0.d7ced916872b'
+                                            //           0.98  (fa)  -> '0.fae147ae147ae'
+                                            //           0.996 (fe)  -> '0.fef9db22d0e56'
+                                            //           1     (ff)  -> '1'
+      A = (function () {
+        if (alphaHex === '0') return '00';
+        if (alphaHex === '1') return 'ff';
+        return alphaHex.slice(2, 4);
+      })();
     }

     return '#' + R + G + B + A;
   }

Alternatively:

@@ -922,16 +922,12 @@
     if (rgba.b < 16) {
       B = '0' + B;
     }

     if (settings.alpha && (rgba.a < 1 || settings.forceAlpha)) {
-      const alpha = rgba.a * 255 | 0;
-      A = alpha.toString(16);
-
-      if (alpha < 16) {
-        A = '0' + A;
-      }
+      const alphaHex = rgba.a.toString(16); // Examples: '0', '0.220c49ba5e354', '1'
+      A = alphaHex === '0' ? '00' : alphaHex === '1' ? 'ff' : alphaHex.slice(2, 4);
     }

     return '#' + R + G + B + A;
   }

Feel free to adjust the code and comments as necessary.

mdbassit commented 5 months ago

That is an effect of the rounding of the alpha value (see issue https://github.com/mdbassit/Coloris/issues/134), which means that it is not possible to get the color value #000000d7 with the UI of the color picker. I will look into preserving the original value that's entered by a user though when it's a valid color.

I believe this is due to the a * 255 | 0 conversion in the RGBAToHex function. Is this conversion necessary, or could it be removed?

The line a * 255 | 0 is equivalent to Math.floor(a * 255) in this case.

Here is a proposed fix that addresses the issue:

Your code is actually just substituting one rounding method with another, and you still won't be able to obtain the full 0 to 255 alpha values. For example, with your code, it's not possible to pick #000000d6.

ut4 commented 5 months ago

It appears that information is being lost twice: first in strToRGBA() (via ctx.fillStyle), and second in RGBAToHex(), as I reported in my initial post. For example, when pasting #000000d6 or rgba(0,0,0,0.839) (which are the same color) into the picker, ctx.fillStyle converts it to rgba(0,0,0,0.84), which then translates to #000000d7 in hex. Based on my experience, this bug requires two fixes: the first one is already provided in the initial post, and the second one needs to address strToRGBA(). Could we add a special handling for HEXA, RGBA, and maybe HSLA formats, for example, like this?

@@ -867,10 +867,13 @@
    * Parse a string to RGBA.
    * @param {string} str String representing a color.
    * @return {object} Red, green, blue and alpha values.
    */
   function strToRGBA(str) {
+    const rgba = tryToParseRGBASpecial(str);
+    if (rgba) return rgba;
+
     const regex = /^((rgba)|rgb)[\D]+([\d.]+)[\D]+([\d.]+)[\D]+([\d.]+)[\D]*?([\d.]+|$)/i;
     let match, rgba;

     // Default to black for invalid color strings
     ctx.fillStyle = '#000';
mdbassit commented 5 months ago

Could we add a special handling for HEXA, RGBA, and maybe HSLA formats, for example, like this?

That is what I meant in my previous comment when I said that I will look into preserving the original value that's entered by a user when it's a valid color.

mdbassit commented 5 months ago

Alright, I looked into this, and doing it properly will add unnecessary complexity to account for all the possible syntax variations of rgb and hsl, and when the gain is a virtually imperceivable alpha channel fidelity, I don't think it's worth the effort.

ut4 commented 5 months ago

What if the solution handled only HEXA colors? This shouldn't introduce too much complexity but would solve the core issue. Here's a proposal:

function tryToParseRGBASpecial(str) {
  const match = /^#([\dA-Fa-f]{2})([\dA-Fa-f]{2})([\dA-Fa-f]{2})([\dA-Fa-f]{2})$/i.exec(str);
  if (!match)
    return null;

  return {
    r: parseInt(match[1], 16),
    g: parseInt(match[2], 16),
    b: parseInt(match[3], 16),
    a: parseInt(match[4], 16) / 255 // 0 - 255 -> 0 - 1
  };
}