openhab / openhab-core

Core framework of openHAB
https://www.openhab.org/
Eclipse Public License 2.0
925 stars 428 forks source link

ColorUtil proposed extensions #3540

Closed andrewfg closed 1 year ago

andrewfg commented 1 year ago

Following are proposed extensions to ColorUtil to enable higher precision conversion XY <=> HSB without intermediate transition via integer RGB values.

NOTE: this proposal is DRAFT for discussion purposes. It uses blunt clones of some methods to use double rather than int internally. So it is not code efficient. Efficiency could be improved either by using inner/outer methods, or perhaps generics.

    // convert double precision HSB to double precision XY
    public static double[] hsbToXY(HSBType hsbType, Gamut gamut) {
        PercentType[] rgb = hsbToRgbPercent(hsbType);
        double r = inverseCompand(rgb[0].doubleValue() / PercentType.HUNDRED.doubleValue());
        double g = inverseCompand(rgb[1].doubleValue() / PercentType.HUNDRED.doubleValue());
        double b = inverseCompand(rgb[2].doubleValue() / PercentType.HUNDRED.doubleValue());

        double X = r * 0.664511 + g * 0.154324 + b * 0.162028;
        double Y = r * 0.283881 + g * 0.668433 + b * 0.047685;
        double Z = r * 0.000088 + g * 0.072310 + b * 0.986039;

        double sum = X + Y + Z;
        Point p = sum == 0.0 ? new Point() : new Point(X / sum, Y / sum);
        Point q = gamut.closest(p);

        double[] xyY = new double[] { q.x, q.y, Y };

        LOGGER.trace("HSB: {} - XYZ: {} {} {} - xyY: {}", hsbType, X, Y, Z, xyY);

        return xyY;
    }

    // convert double precision XY to double precision HSB
    public static HSBType xyToHsb(double[] xy, Gamut gamut) throws IllegalArgumentException {
        if (xy.length < 2 || xy.length > 3 || !inRange(xy[0]) || !inRange(xy[1])
                || (xy.length == 3 && !inRange(xy[2]))) {
            throw new IllegalArgumentException("xy array only allowes two or three values between 0.0 and 1.0.");
        }
        Point p = gamut.closest(new Point(xy[0], xy[1]));
        double x = p.x;
        double y = p.y == 0.0 ? 0.000001 : p.y;
        double z = 1.0 - x - y;
        double Y = (xy.length == 3) ? xy[2] : 1.0;
        double X = (Y / y) * x;
        double Z = (Y / y) * z;
        double r = X * 1.656492 + Y * -0.354851 + Z * -0.255038;
        double g = X * -0.707196 + Y * 1.655397 + Z * 0.036152;
        double b = X * 0.051713 + Y * -0.121364 + Z * 1.011530;

        // Correction for negative values is missing from Philips' documentation.
        double min = Math.min(r, Math.min(g, b));
        if (min < 0.0) {
            r -= min;
            g -= min;
            b -= min;
        }

        // rescale
        double max = Math.max(r, Math.max(g, b));
        if (max > 1.0) {
            r /= max;
            g /= max;
            b /= max;
        }

        r = compand(r);
        g = compand(g);
        b = compand(b);

        // rescale
        max = Math.max(r, Math.max(g, b));
        if (max > 1.0) {
            r /= max;
            g /= max;
            b /= max;
        }

        HSBType hsb = rgbToHsb(new double[] { 255.0 * r, 255.0 * g, 255.0 * b });
        LOGGER.trace("xy: {} - XYZ: {} {} {} - RGB: {} {} {} - HSB: {} ", xy, X, Y, Z, r, g, b, hsb);

        return hsb;
    }

    // convert double precision RGB to double precision HSB
    public static HSBType rgbToHsb(double[] rgb) throws IllegalArgumentException {
        if (rgb.length != 3 || !inByteRange(rgb[0]) || !inByteRange(rgb[1]) || !inByteRange(rgb[2])) {
            throw new IllegalArgumentException("RGB array only allows values between 0 and 255");
        }
        final double r = rgb[0];
        final double g = rgb[1];
        final double b = rgb[2];

        double max = (r > g) ? r : g;
        if (b > max) {
            max = b;
        }
        double min = (r < g) ? r : g;
        if (b < min) {
            min = b;
        }
        double tmpHue;
        final double tmpBrightness = max / 2.55f;
        final double tmpSaturation = (max != 0 ? (max - min) / max : 0) * 100.0;
        // smallest possible saturation: 0 (max=0 or max-min=0), other value closest to 0 is 100/255 (max=255, min=254)
        // -> avoid float comparision to 0
        // if (tmpSaturation == 0) {
        if (max == 0 || (max - min) == 0) {
            tmpHue = 0;
        } else {
            double red = (max - r) / (max - min);
            double green = (max - g) / (max - min);
            double blue = (max - b) / (max - min);
            if (r == max) {
                tmpHue = blue - green;
            } else if (g == max) {
                tmpHue = 2.0f + red - blue;
            } else {
                tmpHue = 4.0f + green - red;
            }
            tmpHue = tmpHue / 6.0f * 360;
            if (tmpHue < 0) {
                tmpHue = tmpHue + 360.0f;
            }
        }

        return new HSBType(new DecimalType(tmpHue), new PercentType(BigDecimal.valueOf(tmpSaturation)),
                new PercentType(BigDecimal.valueOf(tmpBrightness)));
    }

    // helper method
    private static boolean inByteRange(double val) {
        return val >= 0.0 && val <= 255.0;
    }
andrewfg commented 1 year ago

Ping to @holgerfriedrich / @J-N-K ..

holgerfriedrich commented 1 year ago

@andrewfg xy was giving me a headache yesterday evening because back conversion did not work properly (even after modifying rgbtohsb to use float input properly).... I have pushed to my private repo.

Did you notice that point has a ctor which accepts only int,int?

I can take a look later.

andrewfg commented 1 year ago

point has a ctor which accepts only int,int?

Hmm. No.

       public Point(double x, double y) {
holgerfriedrich commented 1 year ago

Forget my last comment. This happens when your IDE tries to resolve it to Java standard class Point.

But nevertheless, I still have the problem when I try to convert HSB 0,100,100 to xy and back. Using floats in RGB to HSB does not help, as the computation of RGB in hsbtoxy has parameters 255.0 0.0 16.434406..... I am lacking of a deeper understanding of the hsbtoxy transform and which corner cases are hard to transform.

btw: find my implementation below, it passes the rgb to hsb and back conversion without any errors.

    public static HSBType rgbToHsb(int[] rgb) throws IllegalArgumentException {
        if (rgb.length != 3 || !inByteRange(rgb[0]) || !inByteRange(rgb[1]) || !inByteRange(rgb[2])) {
            throw new IllegalArgumentException("RGB array only allows values between 0 and 255");
        }
        return rgbToHsb(new float[] { (float) rgb[0], (float) rgb[1], (float) rgb[2] });
    }

    private static HSBType rgbToHsb(float[] rgb) {
        final float r = rgb[0];
        final float g = rgb[1];
        final float b = rgb[2];

        float max = (r > g) ? r : g;
        if (b > max) {
            max = b;
        }
        float min = (r < g) ? r : g;
        if (b < min) {
            min = b;
        }
        LOGGER.warn("RGB: {} {} {}", r, g, b);

        float tmpHue;
        final float tmpBrightness = max / 2.55f;
        final float tmpSaturation = (max >= 0.01 ? ((float) (max - min)) / ((float) max) : 0) * 100.0f;
        // smallest possible saturation: 0 (max=0 or max-min=0), other value closest to 0 is 100/255 (max=255, min=254)
        // -> avoid float comparision to 0
        // if (tmpSaturation == 0) {
        if (max < 0.01f || Math.abs(max - min) < 0.1f) {
            tmpHue = 0;
        } else {
            float red = ((float) (max - r)) / ((float) (max - min));
            float green = ((float) (max - g)) / ((float) (max - min));
            float blue = ((float) (max - b)) / ((float) (max - min));
            if (Math.abs(r - max) < 0.01) {
                tmpHue = blue - green;
            } else if (Math.abs(g - max) < 0.01) {
                tmpHue = 2.0f + red - blue;
            } else {
                tmpHue = 4.0f + green - red;
            }
            tmpHue = tmpHue / 6.0f * 360;
            if (tmpHue < 0) {
                tmpHue = tmpHue + 360.0f;
            }
        }

        // two places after the decimal seems sufficient for lossles conversions
        return new HSBType(new DecimalType(BigDecimal.valueOf(tmpHue).setScale(2, RoundingMode.HALF_UP)),
                new PercentType(BigDecimal.valueOf(tmpSaturation).setScale(2, RoundingMode.HALF_UP)),
                new PercentType(BigDecimal.valueOf(tmpBrightness).setScale(2, RoundingMode.HALF_UP)));
    }
andrewfg commented 1 year ago

Yeah. There are definitely inconsistencies in the XY -> RGB -> HSB -> RGB -> XY transform since the round trip always ends with markedly different results that what you start with. I think it is not just a matter of integer rounding, but probably something fishy in the formulae themselves. Ideally I would like to cut out the RGB stages entirely since they seem irrelevant..

J-N-K commented 1 year ago

The problem is that there is no 1:1 relationship between the different color models. Take e.g. a full brightness white, that can be expressed by any hue value, as long as saturation and brightness is 100%. So if you start with 120/0/100 as HSB, that will result in RGB 255/255/255 and if you convert it back to HSB there is no way to get the original hue value back. This is similar with xyY, no matter if you take the step with RGB or not. In addition the color space of xyY is larger than that of (s)RGB, you can't express every xyY color in RGB (or HSB), so it is a not a lossless conversion.

andrewfg commented 1 year ago

there is no 1:1 relationship between the different color models

@J-N-K your statement above is correct; but not really for the reasons that you cite :)

In fact the round trip XY -> RGB -> HSB -> RGB -> XY is accurate to within 6 or 7 decimal places .. but only if the original XY lies within the Gamut triangle .. and if that is not the case, the XY is adjusted to the closest value that does lie within the Gamut triangle .. and it is this latter correction which is indeed a one way process..