zschuessler / DeltaE

CIE color difference formulas in JavaScript
http://zschuessler.github.io/DeltaE
The Unlicense
260 stars 25 forks source link

“.getDeltaE94” returns a “NaN”. ……How come? #9

Closed issuefiler closed 4 years ago

issuefiler commented 4 years ago

Alright, I just stumbled upon a very specific input that caused a mysterious crash.

const ΔE = require("delta-e");
const distance = ΔE.getDeltaE94(
    {
        L: 53.23288178584245,
        A: 80.10930952982204,
        B: 67.22006831026425
    },
    {
        L: 50.9588099835815,
        A: 77.47798295202801,
        B: 65.01211079141827
    }
);
console.log(distance); // NaN

This ends up returning a NaN. I believe nothing’s special about those numbers? What makes it so special that it crashes my application?

zschuessler commented 4 years ago

I took a brief look, the A calculation specifically is causing the issue:

https://github.com/zschuessler/DeltaE/blob/master/src/dE94.js#L79

When I clamp the number of your colors by a tenths position it works correctly. I would assume a rounding problem. If you find out let me know, if not I'll come back to this when I have free time.

issuefiler commented 4 years ago

Test

I did a quick logging test on my browser.

function dE94(x1, x2, weights) {
    this.x1 = x1;
    this.x2 = x2;

    this.weights = weights || {};
    this.weights.lightness = this.weights.lightness || 1;
    this.weights.chroma = this.weights.chroma || 1;
    this.weights.hue = this.weights.hue || 1;

    if (1 === this.weights.lightness) {
        this.weights.K1 = 0.045;
        this.weights.K2 = 0.015;
    } else {
        this.weights.K1 = 0.048;
        this.weights.K2 = 0.014;
    }
}

/**
 * Returns the dE94 value.
 * @method
 * @returns {number}
 */
dE94.prototype.getDeltaE = function() {
    var x1 = this.x1;
    var x2 = this.x2;
    var sqrt = Math.sqrt;
    var pow = Math.pow;
    console.log(this.calculateL(x1, x2), this.calculateA(x1, x2), this.calculateB(x1, x2));
    return sqrt(
        pow(this.calculateL(x1, x2), 2) +
        pow(this.calculateA(x1, x2), 2) +
        pow(this.calculateB(x1, x2), 2)
    );
};

/**
 * Calculates the lightness value.
 * @method
 * @returns {number}
 */
dE94.prototype.calculateL = function(x1, x2) {
    return (x1.L - x2.L) / this.weights.lightness;
};

/**
 * Calculates the chroma value.
 * @method
 * @returns {number}
 */
dE94.prototype.calculateA = function(x1, x2) {
    var sqrt = Math.sqrt;
    var pow = Math.pow;

    //top
    var c1 = sqrt(pow(x1.A, 2) + pow(x1.B, 2));
    var c2 = sqrt(pow(x2.A, 2) + pow(x2.B, 2));
    var cab = c1 - c2;

    // bottom
    var sc = 1 + (this.weights.K1 * c1);

    return cab / (this.weights.chroma * sc);
};

/**
 * Calculates the hue value.
 * @method
 * @returns {number}
 */
dE94.prototype.calculateB = function(x1, x2) {
    var sqrt = Math.sqrt;
    var pow = Math.pow;

    // cab
    var c1 = sqrt(pow(x1.A, 2) + pow(x1.B, 2));
    var c2 = sqrt(pow(x2.A, 2) + pow(x2.B, 2));
    var cab = c1 - c2;

    // top
    var a = x1.A - x2.A;
    var b = x1.B - x2.B;
    var hab = sqrt(
        pow(a, 2) +
        pow(b, 2) -
        pow(cab, 2)
    );
console.log("HAB", pow(a, 2) +
        pow(b, 2) -
        pow(cab, 2), hab, pow(a, 2) +
        pow(b, 2),
        pow(cab, 2));

    // bottom
    var c1 = sqrt(pow(x1.A, 2) + pow(x1.B, 2));
    var sh = 1 + (this.weights.K2 * c1);
    console.log(hab, sh);
    return hab / sh;
};
(new dE94({
        L: 53.23288178584245,
        A: 80.10930952982204,
        B: 67.22006831026425
    },
    {
        L: 50.9588099835815,
        A: 77.47798295202801,
        B: 65.01211079141827
    })).getDeltaE()
HAB -6.394884621840902e-14 NaN 11.798955964033755 11.79895596403382
NaN 2.568632776599043
2.2740718022609485 0.6020017603282479 NaN
HAB -6.394884621840902e-14 NaN 11.798955964033755 11.79895596403382
NaN 2.568632776599043
NaN

Problem

And it turned out to be due to floating-point errors in the calculation. What was supposed to be 0 couldn’t but -0.00000000000006394884621840902, which caused Math.sqrt to return a NaN.


Solution

I suggest an error-tolerant square-rooting static method value => (Math.sqrt(value) || 0).

The visualized problem

zschuessler commented 4 years ago

Wow, thanks for doing that. Great work!

I'm in the process of moving across the country so I don't have much time to review this for a couple weeks. I'll take a look as soon as I can though!

zschuessler commented 4 years ago

Thanks again for the detailed explanation, that was helpful! I pushed an update to handle the sqrt issue in eE94 and also added a Mocha test to test for the issue. New release in 0.0.8 is live.

I also toyed around with using a couple workarounds to make floating-point safe calculations, but all of the options were convoluted and impacted performance. I'm not opposed to addressing that again but it seems out of scope for the common use case of this package.

Btw: the move across the country went great. The east coast is wonderful 👍