You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/12/25 23:29:12 UTC
[GitHub] [commons-imaging] Brixomatic opened a new pull request #114: Util color extension
Brixomatic opened a new pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: imaging.color extension. More precision and DIN99b + DIN99o added to ColorConversion.
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r548983923
##########
File path: src/test/java/org/apache/commons/imaging/color/ColorConversionsTest.java
##########
@@ -39,7 +40,7 @@ public void testRGBtoCMYK() {
Debug.debug("cmyk_cmy: " + cmyk_cmy);
Debug.debug("cmyk_cmy_rgb: " + cmyk_cmy_rgb + " (" + Integer.toHexString(cmyk_cmy_rgb) + ")");
- assertEquals((0xffffff & cmyk_cmy_rgb), (0xffffff & rgb));
+ assertEquals(toHexString(0xffffff & rgb), toHexString(0xffffff & cmyk_cmy_rgb));
Review comment:
"expected" and "actual" values where in the wrong order. Also, comparing the hex strings is much more comprehensible in test failure messages.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls commented on pull request #114: imaging.color extension. More precision and DIN99b + DIN99o added to ColorConversion.
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-751301554
[](https://coveralls.io/builds/35946757)
Coverage increased (+0.03%) to 76.459% when pulling **b471486578d463872d933bfb6052608fe8157478 on Brixomatic:util_color_extension** into **2b48e795b58f02108ab262988b673756801de87b on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r588951722
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -747,4 +679,147 @@ public static ColorXyz convertCIELuvtoXYZ(final double L, final double u, final
return new ColorXyz(X, Y, Z);
}
+
+ public static ColorDIN99Lab convertCIELabToDIN99bLab(final ColorCieLab cie) {
+ return convertCIELabToDIN99bLab(cie.L, cie.a, cie.b);
+ }
+
+ public static ColorDIN99Lab convertCIELabToDIN99bLab(final double L, final double a, final double b) {
+ final double FAC_1 = 100.0 / Math.log(129.0 / 50.0); // = 105.51
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double ang = Math.toRadians(16.0);
+
+ final double L99 = kE * FAC_1 * Math.log(1. + 0.0158 * L);
+ double a99 = 0.0;
+ double b99 = 0.0;
+ if (a != 0.0 || b != 0.0) {
+ final double e = a * Math.cos(ang) + b * Math.sin(ang);
+ final double f = 0.7 * (b * Math.cos(ang) - a * Math.sin(ang));
+ final double G = Math.sqrt(e * e + f * f);
+ if (G != 0.) {
+ final double k = Math.log(1. + 0.045 * G) / (0.045 * kCH * kE * G);
+ a99 = k * e;
+ b99 = k * f;
+ }
+ }
+ return new ColorDIN99Lab(L99, a99, b99);
+ }
+
+ public static ColorCieLab convertDIN99bLabToCIELab(final ColorDIN99Lab dinb) {
+ return convertDIN99bLabToCIELab(dinb.L99, dinb.a99, dinb.b99);
+ }
+
+ public static ColorCieLab convertDIN99bLabToCIELab(final double L99b, final double a99b, final double b99b) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(129.0 / 50.0); // L99 scaling factor = 105.50867113783109
+ final double ang = Math.toRadians(16.0);
+
+ final double hef = Math.atan2(b99b, a99b);
+ final double C = Math.sqrt(a99b * a99b + b99b * b99b);
+ final double G = (Math.exp(0.045 * C * kCH * kE) - 1.0) / 0.045;
+ final double e = G * Math.cos(hef);
+ final double f = G * Math.sin(hef) / 0.7;
+
+ final double L = (Math.exp(L99b * kE / FAC_1) - 1.) / 0.0158;
+ final double a = e * Math.cos(ang) - f * Math.sin(ang);
+ final double b = e * Math.sin(ang) + f * Math.cos(ang);
+ return new ColorCieLab(L, a, b);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorDIN99Lab convertCIELabToDIN99oLab(final ColorCieLab cie) {
+ return convertCIELabToDIN99oLab(cie.L, cie.a, cie.b);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorDIN99Lab convertCIELabToDIN99oLab(final double L, final double a, final double b) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(139.0 / 100.0); // L99 scaling factor = 303.67100547050995
+ final double ang = Math.toRadians(26.0);
+
+ final double L99o = FAC_1 / kE * Math.log(1 + 0.0039 * L); // Lightness correction kE
+ double a99o = 0.0;
+ double b99o = 0.0;
+ if (a != 0.0 || b != 0.0) {
+ final double eo = a * Math.cos(ang) + b * Math.sin(ang); // a stretching
+ final double fo = 0.83 * (b * Math.cos(ang) - a * Math.sin(ang)); // b rotation/stretching
+ final double Go = Math.sqrt(eo * eo + fo * fo); // chroma
+ final double C99o = Math.log(1.0 + 0.075 * Go) / (0.0435 * kCH * kE); // factor for chroma compression and viewing conditions
+ final double heofo = Math.atan2(fo, eo); // arctan in four quadrants
+ final double h99o = heofo + ang; // hue rotation
+ a99o = C99o * Math.cos(h99o);
+ b99o = C99o * Math.sin(h99o);
+ }
+ return new ColorDIN99Lab(L99o, a99o, b99o);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorCieLab convertDIN99oLabToCIELab(final ColorDIN99Lab dino) {
+ return convertDIN99oLabToCIELab(dino.L99, dino.a99, dino.b99);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorCieLab convertDIN99oLabToCIELab(final double L99o, final double a99o, final double b99o) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(139.0 / 100.0); // L99 scaling factor = 303.67100547050995
+ final double ang = Math.toRadians(26.0);
+
+ final double L = (Math.exp(L99o * kE / FAC_1) - 1.0) / 0.0039;
+
+ final double h99ef = Math.atan2(b99o, a99o); // arctan in four quadrants
+
+ final double heofo = h99ef - ang; // backwards hue rotation
+
+ final double C99 = Math.sqrt(a99o * a99o + b99o * b99o); // DIN99 chroma
+ final double G = (Math.exp(0.0435 * kE * kCH * C99) - 1.0) / 0.075; // factor for chroma decompression and viewing conditions
+ final double e = G * Math.cos(heofo);
+ final double f = G * Math.sin(heofo);
+
+ final double a = e * Math.cos(ang) - f / 0.83 * Math.sin(ang); // rotation by 26 degrees
+ final double b = e * Math.sin(ang) + f / 0.83 * Math.cos(ang); // rotation by 26 degrees
+
+ return new ColorCieLab(L, a, b);
+ }
+
+ private static double pivotRGB(double n) {
+ if (n > 0.0031308) {
+ n = 1.055 * Math.pow(n, 1 / 2.4) - 0.055;
+ } else {
+ n = 12.92 * n;
+ }
+ return n;
+ }
+
+ private static double unPivotRGB(double n) {
+ if (n > 0.04045) {
+ n = Math.pow((n + 0.055) / 1.055, 2.4);
+ } else {
+ n = n / 12.92;
+ }
+ return n;
+ }
+
+ private static double pivotXYZ(double n) {
+ if (n > XYZ_t0) {
+ n = Math.pow(n, 1 / 3.0);
+ } else {
+ n = XYZ_m * n + 16 / 116.0;
+ }
+ return n;
+ }
+
+ private static double unPivotXYZ(double n) {
+ final double nCube = Math.pow(n, 3);
+ if (nCube > XYZ_t0) {
+ n = nCube;
+ } else {
+ n = (n - 16 / 116.0) / XYZ_m;
+ }
+ return n;
Review comment:
These functions here are removing **a lot** of duplicate code. Thanks for tidying it up.
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -99,8 +89,8 @@ public static ColorHunterLab convertXYZtoHunterLab(final ColorXyz xyz) {
public static ColorHunterLab convertXYZtoHunterLab(final double X,
final double Y, final double Z) {
final double L = 10 * Math.sqrt(Y);
- final double a = 17.5 * (((1.02 * X) - Y) / Math.sqrt(Y));
- final double b = 7 * ((Y - (0.847 * Z)) / Math.sqrt(Y));
+ final double a = Y == 0.0 ? 0.0 : 17.5 * (((1.02 * X) - Y) / Math.sqrt(Y));
+ final double b = Y == 0.0 ? 0.0 : 7 * ((Y - (0.847 * Z)) / Math.sqrt(Y));
Review comment:
Oh, good catch! :clap:
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -747,4 +679,147 @@ public static ColorXyz convertCIELuvtoXYZ(final double L, final double u, final
return new ColorXyz(X, Y, Z);
}
+
+ public static ColorDIN99Lab convertCIELabToDIN99bLab(final ColorCieLab cie) {
+ return convertCIELabToDIN99bLab(cie.L, cie.a, cie.b);
+ }
+
+ public static ColorDIN99Lab convertCIELabToDIN99bLab(final double L, final double a, final double b) {
+ final double FAC_1 = 100.0 / Math.log(129.0 / 50.0); // = 105.51
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double ang = Math.toRadians(16.0);
+
+ final double L99 = kE * FAC_1 * Math.log(1. + 0.0158 * L);
+ double a99 = 0.0;
+ double b99 = 0.0;
+ if (a != 0.0 || b != 0.0) {
+ final double e = a * Math.cos(ang) + b * Math.sin(ang);
+ final double f = 0.7 * (b * Math.cos(ang) - a * Math.sin(ang));
+ final double G = Math.sqrt(e * e + f * f);
+ if (G != 0.) {
+ final double k = Math.log(1. + 0.045 * G) / (0.045 * kCH * kE * G);
+ a99 = k * e;
+ b99 = k * f;
+ }
+ }
+ return new ColorDIN99Lab(L99, a99, b99);
+ }
+
+ public static ColorCieLab convertDIN99bLabToCIELab(final ColorDIN99Lab dinb) {
+ return convertDIN99bLabToCIELab(dinb.L99, dinb.a99, dinb.b99);
+ }
+
+ public static ColorCieLab convertDIN99bLabToCIELab(final double L99b, final double a99b, final double b99b) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(129.0 / 50.0); // L99 scaling factor = 105.50867113783109
+ final double ang = Math.toRadians(16.0);
+
+ final double hef = Math.atan2(b99b, a99b);
+ final double C = Math.sqrt(a99b * a99b + b99b * b99b);
+ final double G = (Math.exp(0.045 * C * kCH * kE) - 1.0) / 0.045;
+ final double e = G * Math.cos(hef);
+ final double f = G * Math.sin(hef) / 0.7;
+
+ final double L = (Math.exp(L99b * kE / FAC_1) - 1.) / 0.0158;
+ final double a = e * Math.cos(ang) - f * Math.sin(ang);
+ final double b = e * Math.sin(ang) + f * Math.cos(ang);
+ return new ColorCieLab(L, a, b);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
Review comment:
:+1:
Thanks for keeping variables similar to the original text. With some machine translation from German to English, and following your code, I think I've managed to review these functions :-)
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -168,30 +152,24 @@ public static ColorXyz convertRGBtoXYZ(final int rgb) {
double var_G = g / 255.0; // Where G = 0 ÷ 255
double var_B = b / 255.0; // Where B = 0 ÷ 255
- if (var_R > 0.04045) {
- var_R = Math.pow((var_R + 0.055) / 1.055, 2.4);
- } else {
- var_R = var_R / 12.92;
- }
- if (var_G > 0.04045) {
- var_G = Math.pow((var_G + 0.055) / 1.055, 2.4);
- } else {
- var_G = var_G / 12.92;
- }
- if (var_B > 0.04045) {
- var_B = Math.pow((var_B + 0.055) / 1.055, 2.4);
- } else {
- var_B = var_B / 12.92;
- }
+ // Pivot RGB:
+ var_R = unPivotRGB(var_R);
+ var_G = unPivotRGB(var_G);
+ var_B = unPivotRGB(var_B);
- var_R = var_R * 100;
- var_G = var_G * 100;
- var_B = var_B * 100;
+ var_R *= 100;
+ var_G *= 100;
+ var_B *= 100;
// Observer. = 2°, Illuminant = D65
- final double X = var_R * 0.4124 + var_G * 0.3576 + var_B * 0.1805;
- final double Y = var_R * 0.2126 + var_G * 0.7152 + var_B * 0.0722;
- final double Z = var_R * 0.0193 + var_G * 0.1192 + var_B * 0.9505;
+ // see: https://github.com/StanfordHCI/c3/blob/master/java/src/edu/stanford/vis/color/LAB.java
+ final double X = var_R * 0.4124564 + var_G * 0.3575761 + var_B * 0.1804375;
+ final double Y = var_R * 0.2126729 + var_G * 0.7151522 + var_B * 0.0721750;
+ final double Z = var_R * 0.0193339 + var_G * 0.1191920 + var_B * 0.9503041;
+
+ // final double X = var_R * 0.4124 + var_G * 0.3576 + var_B * 0.1805;
+ // final double Y = var_R * 0.2126 + var_G * 0.7152 + var_B * 0.0722;
+ // final double Z = var_R * 0.0193 + var_G * 0.1192 + var_B * 0.9505;
Review comment:
Was it intentionally commented, or left-overs from the development/testing?
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -747,4 +700,147 @@ public static ColorXyz convertCIELuvtoXYZ(final double L, final double u, final
return new ColorXyz(X, Y, Z);
}
+
+ public static ColorDIN99Lab convertCIELabToDIN99bLab(final ColorCieLab cie) {
+ return convertCIELabToDIN99bLab(cie.L, cie.a, cie.b);
+ }
+
+ public static ColorDIN99Lab convertCIELabToDIN99bLab(final double L, final double a, final double b) {
+ final double FAC_1 = 100.0 / Math.log(129.0 / 50.0); // = 105.51
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double ang = Math.toRadians(16.0);
+
+ final double L99 = kE * FAC_1 * Math.log(1. + 0.0158 * L);
+ double a99 = 0.0;
+ double b99 = 0.0;
+ if (a != 0.0 || b != 0.0) {
+ final double e = a * Math.cos(ang) + b * Math.sin(ang);
+ final double f = 0.7 * (b * Math.cos(ang) - a * Math.sin(ang));
+ final double G = Math.sqrt(e * e + f * f);
+ if (G != 0.) {
+ final double k = Math.log(1. + 0.045 * G) / (0.045 * kCH * kE * G);
+ a99 = k * e;
+ b99 = k * f;
+ }
+ }
+ return new ColorDIN99Lab(L99, a99, b99);
+ }
+
+ public static ColorCieLab convertDIN99bLabToCIELab(final ColorDIN99Lab dinb) {
+ return convertDIN99bLabToCIELab(dinb.L99, dinb.a99, dinb.b99);
+ }
+
+ public static ColorCieLab convertDIN99bLabToCIELab(final double L99b, final double a99b, final double b99b) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(129.0 / 50.0); // L99 scaling factor = 105.50867113783109
+ final double ang = Math.toRadians(16.0);
+
+ final double hef = Math.atan2(b99b, a99b);
+ final double C = Math.sqrt(a99b * a99b + b99b * b99b);
+ final double G = (Math.exp(0.045 * C * kCH * kE) - 1.0) / 0.045;
+ final double e = G * Math.cos(hef);
+ final double f = G * Math.sin(hef) / 0.7;
+
+ final double L = (Math.exp(L99b * kE / FAC_1) - 1.) / 0.0158;
+ final double a = e * Math.cos(ang) - f * Math.sin(ang);
+ final double b = e * Math.sin(ang) + f * Math.cos(ang);
+ return new ColorCieLab(L, a, b);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorDIN99Lab convertCIELabToDIN99oLab(final ColorCieLab cie) {
+ return convertCIELabToDIN99oLab(cie.L, cie.a, cie.b);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorDIN99Lab convertCIELabToDIN99oLab(final double L, final double a, final double b) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(139.0 / 100.0); // L99 scaling factor = 303.67100547050995
+ final double ang = Math.toRadians(26.0);
+
+ final double L99o = FAC_1 / kE * Math.log(1 + 0.0039 * L); // Lightness correction kE
+ double a99o = 0.0;
+ double b99o = 0.0;
+ if (a != 0.0 || b != 0.0) {
+ final double eo = a * Math.cos(ang) + b * Math.sin(ang); // a stretching
+ final double fo = 0.83 * (b * Math.cos(ang) - a * Math.sin(ang)); // b rotation/stretching
+ final double Go = Math.sqrt(eo * eo + fo * fo); // chroma
+ final double C99o = Math.log(1.0 + 0.075 * Go) / (0.0435 * kCH * kE); // factor for chroma compression and viewing conditions
+ final double heofo = Math.atan2(fo, eo); // arctan in four quadrants
+ final double h99o = heofo + ang; // hue rotation
+ a99o = C99o * Math.cos(h99o);
+ b99o = C99o * Math.sin(h99o);
+ }
+ return new ColorDIN99Lab(L99o, a99o, b99o);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorCieLab convertDIN99oLabToCIELab(final ColorDIN99Lab dino) {
+ return convertDIN99oLabToCIELab(dino.L99, dino.a99, dino.b99);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorCieLab convertDIN99oLabToCIELab(final double L99o, final double a99o, final double b99o) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(139.0 / 100.0); // L99 scaling factor = 303.67100547050995
+ final double ang = Math.toRadians(26.0);
+
+ final double L = (Math.exp(L99o * kE / FAC_1) - 1.0) / 0.0039;
+
+ final double h99ef = Math.atan2(b99o, a99o); // arctan in four quadrants
+
+ final double heofo = h99ef - ang; // backwards hue rotation
+
+ final double C99 = Math.sqrt(a99o * a99o + b99o * b99o); // DIN99 chroma
+ final double G = (Math.exp(0.0435 * kE * kCH * C99) - 1.0) / 0.075; // factor for chroma decompression and viewing conditions
+ final double e = G * Math.cos(heofo);
+ final double f = G * Math.sin(heofo);
+
+ final double a = e * Math.cos(ang) - f / 0.83 * Math.sin(ang); // rotation by 26 degrees
+ final double b = e * Math.sin(ang) + f / 0.83 * Math.cos(ang); // rotation by 26 degrees
+
+ return new ColorCieLab(L, a, b);
+ }
Review comment:
Even though the sources are in German, you've done an excellent job maintaining this new code as similar as possible to the Wikipedia text, and also added some comments :+1: Looks good to me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r560532422
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorCieLch.java
##########
@@ -19,10 +19,14 @@
/**
* Represents a color in the CIELCH color space.
*
- * <p>Contains the constant values for black, white, red,
- * green, and blue.</p>
+ * <p>
+ * Contains the constant values for black, white, red,
+ * green, and blue.
+ * </p>
+ * Changes: H renamed to h.
Review comment:
Sure, will create it later and link it here :-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #114: [IMAGING-283] Add CIELAB and DIN99 conversion, reduce code duplication, and issues related to zero-division and precision
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-751301554
[](https://coveralls.io/builds/37714393)
Coverage increased (+0.07%) to 76.503% when pulling **d94468844ba455e54a40c431f9f1a2f651a4f97b on Brixomatic:util_color_extension** into **2b48e795b58f02108ab262988b673756801de87b on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on pull request #114: [IMAGING-283] Add CIELAB and DIN99 conversion, reduce code duplication, and issues related to zero-division and precision
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-792240503
Made requested changes.
Thanks for creating https://issues.apache.org/jira/browse/IMAGING-283
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: imaging.color extension. More precision and DIN99b + DIN99o added to ColorConversion.
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r548922875
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -643,28 +602,30 @@ private static int convertRGBtoRGB(int red, int green, int blue) {
return rgb;
}
+ private static int convertRGBtoRGB(final Color color) {
+ return color.getRGB() | 0xff << 24; // alpha channel always opaque
+ }
+
public static ColorCieLch convertCIELabtoCIELCH(final ColorCieLab cielab) {
return convertCIELabtoCIELCH(cielab.L, cielab.a, cielab.b);
}
public static ColorCieLch convertCIELabtoCIELCH(final double L, final double a, final double b) {
- double var_H = Math.atan2(b, a); // Quadrant by signs
+ // atan2(y,x) returns atan(y/x)
+ final double atanba = Math.atan2(b, a); // Quadrant by signs
- if (var_H > 0) {
- var_H = (var_H / Math.PI) * 180.0;
- } else {
- var_H = 360 - radian_2_degree(Math.abs(var_H));
- }
+ final double h = atanba > 0 //
+ ? Math.toDegrees(atanba) //
+ : Math.toDegrees(atanba) + 360;
Review comment:
We can very well use java.lang.Math, instead of doing it step by step. Also reads better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: Util color extension
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r548922591
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -68,24 +70,12 @@ public static ColorXyz convertCIELabtoXYZ(final double L, final double a, final
double var_X = a / 500 + var_Y;
double var_Z = var_Y - b / 200.0;
- if (Math.pow(var_Y, 3) > 0.008856) {
- var_Y = Math.pow(var_Y, 3);
- } else {
- var_Y = (var_Y - 16 / 116.0) / 7.787;
- }
- if (Math.pow(var_X, 3) > 0.008856) {
- var_X = Math.pow(var_X, 3);
- } else {
- var_X = (var_X - 16 / 116.0) / 7.787;
- }
- if (Math.pow(var_Z, 3) > 0.008856) {
- var_Z = Math.pow(var_Z, 3);
- } else {
- var_Z = (var_Z - 16 / 116.0) / 7.787;
- }
+ var_Y = unPivotXYZ(var_Y);
+ var_X = unPivotXYZ(var_X);
+ var_Z = unPivotXYZ(var_Z);
Review comment:
again removing some copy/paste
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-751301554
[](https://coveralls.io/builds/35995840)
Coverage increased (+0.03%) to 76.459% when pulling **8850b15d2c863f682123c6dc9d9c92fda7fc741d on Brixomatic:util_color_extension** into **2b48e795b58f02108ab262988b673756801de87b on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: [IMAGING-283] Add CIELAB and DIN99 conversion, reduce code duplication, and issues related to zero-division and precision
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r588994104
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -168,30 +152,24 @@ public static ColorXyz convertRGBtoXYZ(final int rgb) {
double var_G = g / 255.0; // Where G = 0 ÷ 255
double var_B = b / 255.0; // Where B = 0 ÷ 255
- if (var_R > 0.04045) {
- var_R = Math.pow((var_R + 0.055) / 1.055, 2.4);
- } else {
- var_R = var_R / 12.92;
- }
- if (var_G > 0.04045) {
- var_G = Math.pow((var_G + 0.055) / 1.055, 2.4);
- } else {
- var_G = var_G / 12.92;
- }
- if (var_B > 0.04045) {
- var_B = Math.pow((var_B + 0.055) / 1.055, 2.4);
- } else {
- var_B = var_B / 12.92;
- }
+ // Pivot RGB:
+ var_R = unPivotRGB(var_R);
+ var_G = unPivotRGB(var_G);
+ var_B = unPivotRGB(var_B);
- var_R = var_R * 100;
- var_G = var_G * 100;
- var_B = var_B * 100;
+ var_R *= 100;
+ var_G *= 100;
+ var_B *= 100;
// Observer. = 2°, Illuminant = D65
- final double X = var_R * 0.4124 + var_G * 0.3576 + var_B * 0.1805;
- final double Y = var_R * 0.2126 + var_G * 0.7152 + var_B * 0.0722;
- final double Z = var_R * 0.0193 + var_G * 0.1192 + var_B * 0.9505;
+ // see: https://github.com/StanfordHCI/c3/blob/master/java/src/edu/stanford/vis/color/LAB.java
+ final double X = var_R * 0.4124564 + var_G * 0.3575761 + var_B * 0.1804375;
+ final double Y = var_R * 0.2126729 + var_G * 0.7151522 + var_B * 0.0721750;
+ final double Z = var_R * 0.0193339 + var_G * 0.1191920 + var_B * 0.9503041;
+
+ // final double X = var_R * 0.4124 + var_G * 0.3576 + var_B * 0.1805;
+ // final double Y = var_R * 0.2126 + var_G * 0.7152 + var_B * 0.0722;
+ // final double Z = var_R * 0.0193 + var_G * 0.1192 + var_B * 0.9505;
Review comment:
I left them over to make the change more apparent, since most sources just list the shorter value. If you like I will remove them. For the time being, I'll add a line of explanation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-763176214
> Also, there are several commits. Would it be possible to rebase, and squash the commits? Unless you think it would be better to leave this way for now to edit commits if necessary during the review.
At my workplace on the Github-Enterprise we have the option to squash-merge in Github itself when merging PRs (we even made that mandatory in the settings). Doesn't this method exist here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow closed pull request #114: [IMAGING-283] Add CIELAB and DIN99 conversion, reduce code duplication, and issues related to zero-division and precision
Posted by GitBox <gi...@apache.org>.
kinow closed pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: imaging.color extension. More precision and DIN99b + DIN99o added to ColorConversion.
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r548983060
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -99,8 +89,8 @@ public static ColorHunterLab convertXYZtoHunterLab(final ColorXyz xyz) {
public static ColorHunterLab convertXYZtoHunterLab(final double X,
final double Y, final double Z) {
final double L = 10 * Math.sqrt(Y);
- final double a = 17.5 * (((1.02 * X) - Y) / Math.sqrt(Y));
- final double b = 7 * ((Y - (0.847 * Z)) / Math.sqrt(Y));
+ final double a = Y == 0.0 ? 0.0 : 17.5 * (((1.02 * X) - Y) / Math.sqrt(Y));
+ final double b = Y == 0.0 ? 0.0 : 7 * ((Y - (0.847 * Z)) / Math.sqrt(Y));
Review comment:
If Y == 0.0 (as with black color) "a" and "b" would become "NaN", this the same fix that colormine uses (see [HunterLabConverter.cs](https://github.com/muak/ColorMine/blob/master/ColorMine/ColorSpaces/Conversions/HunterLabConverter.cs) )
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: [IMAGING-283] Add CIELAB and DIN99 conversion, reduce code duplication, and issues related to zero-division and precision
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r588994104
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -168,30 +152,24 @@ public static ColorXyz convertRGBtoXYZ(final int rgb) {
double var_G = g / 255.0; // Where G = 0 ÷ 255
double var_B = b / 255.0; // Where B = 0 ÷ 255
- if (var_R > 0.04045) {
- var_R = Math.pow((var_R + 0.055) / 1.055, 2.4);
- } else {
- var_R = var_R / 12.92;
- }
- if (var_G > 0.04045) {
- var_G = Math.pow((var_G + 0.055) / 1.055, 2.4);
- } else {
- var_G = var_G / 12.92;
- }
- if (var_B > 0.04045) {
- var_B = Math.pow((var_B + 0.055) / 1.055, 2.4);
- } else {
- var_B = var_B / 12.92;
- }
+ // Pivot RGB:
+ var_R = unPivotRGB(var_R);
+ var_G = unPivotRGB(var_G);
+ var_B = unPivotRGB(var_B);
- var_R = var_R * 100;
- var_G = var_G * 100;
- var_B = var_B * 100;
+ var_R *= 100;
+ var_G *= 100;
+ var_B *= 100;
// Observer. = 2°, Illuminant = D65
- final double X = var_R * 0.4124 + var_G * 0.3576 + var_B * 0.1805;
- final double Y = var_R * 0.2126 + var_G * 0.7152 + var_B * 0.0722;
- final double Z = var_R * 0.0193 + var_G * 0.1192 + var_B * 0.9505;
+ // see: https://github.com/StanfordHCI/c3/blob/master/java/src/edu/stanford/vis/color/LAB.java
+ final double X = var_R * 0.4124564 + var_G * 0.3575761 + var_B * 0.1804375;
+ final double Y = var_R * 0.2126729 + var_G * 0.7151522 + var_B * 0.0721750;
+ final double Z = var_R * 0.0193339 + var_G * 0.1191920 + var_B * 0.9503041;
+
+ // final double X = var_R * 0.4124 + var_G * 0.3576 + var_B * 0.1805;
+ // final double Y = var_R * 0.2126 + var_G * 0.7152 + var_B * 0.0722;
+ // final double Z = var_R * 0.0193 + var_G * 0.1192 + var_B * 0.9505;
Review comment:
I left them over to make the change more apparent, since most sources just list the shorter value. If you like I will remove them.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r559898056
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorCieLch.java
##########
@@ -19,10 +19,14 @@
/**
* Represents a color in the CIELCH color space.
*
- * <p>Contains the constant values for black, white, red,
- * green, and blue.</p>
+ * <p>
+ * Contains the constant values for black, white, red,
+ * green, and blue.
+ * </p>
+ * Changes: H renamed to h.
Review comment:
The changes must be documented in a JIRA issue and linked to the changelog (i.e. the JIRA number is added to src/changes/changes.xml later, used to generate the changes report from the project site).
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -643,28 +602,30 @@ private static int convertRGBtoRGB(int red, int green, int blue) {
return rgb;
}
+ private static int convertRGBtoRGB(final Color color) {
+ return color.getRGB() | 0xff << 24; // alpha channel always opaque
+ }
Review comment:
Ideally we would limit awt use, or remove it, so that users can use Imaging for Android apps too (we have one user with a workaround on his code at the moment). Unless that makes maintaining the code extremely difficult, in which case we can just try to figure out another way to provide a version compatible with Android...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-763184894
> One issue with commiters merging (and not the contributor) is that there are some complex issues where it might be better to have more than 1 commit too. But in those cases, the committer might not be aware, and squash everything down to a single commit.
I am totally fine with having everything squashed to a single commit, I haven't seen a PR that collides.
And if there is a collision, I'm happy to solve it until it's mergeable.
I'm just reluctant to mess up the history with a squash on a public branch. That should have been made when the branch was still local.
By the way, is there a file that contains the formatting rules for use in Eclipse or another IDE? I had to have the build done on Github to see whether the lint check was passed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: Util color extension
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r548922651
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -132,33 +123,31 @@ public static int convertXYZtoRGB(final double X, final double Y, final double Z
final double var_Y = Y / 100.0; // Where Y = 0 ÷ 100.000
final double var_Z = Z / 100.0; // Where Z = 0 ÷ 108.883
- double var_R = var_X * 3.2406 + var_Y * -1.5372 + var_Z * -0.4986;
- double var_G = var_X * -0.9689 + var_Y * 1.8758 + var_Z * 0.0415;
- double var_B = var_X * 0.0557 + var_Y * -0.2040 + var_Z * 1.0570;
+ // see: https://github.com/StanfordHCI/c3/blob/master/java/src/edu/stanford/vis/color/LAB.java
+ double var_R = var_X * 3.2404542 + var_Y * -1.5371385 + var_Z * -0.4985314;
+ double var_G = var_X * -0.9692660 + var_Y * 1.8760108 + var_Z * 0.0415560;
+ double var_B = var_X * 0.0556434 + var_Y * -0.2040259 + var_Z * 1.0572252;
Review comment:
Using more precise numbers. This made some small, but noticeable difference in tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: Util color extension
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r548922568
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -16,44 +16,46 @@
*/
package org.apache.commons.imaging.color;
+import java.awt.Color;
public final class ColorConversions {
- private static final double REF_X = 95.047; // Observer= 2°, Illuminant= D65
+
+ // White reference
+ /** see: https://en.wikipedia.org/wiki/CIELAB_color_space#From_CIEXYZ_to_CIELAB[10] */
+ private static final double REF_X = 95.047; // Observer= 2°, Illuminant= D65
+
+ /** see: https://en.wikipedia.org/wiki/CIELAB_color_space#From_CIEXYZ_to_CIELAB[10] */
private static final double REF_Y = 100.000;
+
+ /** see: https://en.wikipedia.org/wiki/CIELAB_color_space#From_CIEXYZ_to_CIELAB[10] */
private static final double REF_Z = 108.883;
+ /** see: https://en.wikipedia.org/wiki/CIELAB_color_space#From_CIEXYZ_to_CIELAB[10] */
+ private static final double XYZ_m = 7.787037; // match in slope. Note commonly seen 7.787 gives worse results
+
+ /** see: https://en.wikipedia.org/wiki/CIELAB_color_space#From_CIEXYZ_to_CIELAB[10] */
+ private static final double XYZ_t0 = 0.008856;
+
private ColorConversions() {
}
public static ColorCieLab convertXYZtoCIELab(final ColorXyz xyz) {
return convertXYZtoCIELab(xyz.X, xyz.Y, xyz.Z);
}
- public static ColorCieLab convertXYZtoCIELab(final double X, final double Y,
- final double Z) {
+ public static ColorCieLab convertXYZtoCIELab(final double X, final double Y, final double Z) {
- double var_X = X / REF_X; // REF_X = 95.047 Observer= 2°, Illuminant=
- // D65
+ double var_X = X / REF_X; // REF_X = 95.047 Observer= 2°, Illuminant= D65
double var_Y = Y / REF_Y; // REF_Y = 100.000
double var_Z = Z / REF_Z; // REF_Z = 108.883
- if (var_X > 0.008856) {
- var_X = Math.pow(var_X, (1 / 3.0));
- } else {
- var_X = (7.787 * var_X) + (16 / 116.0);
- }
- if (var_Y > 0.008856) {
- var_Y = Math.pow(var_Y, 1 / 3.0);
- } else {
- var_Y = (7.787 * var_Y) + (16 / 116.0);
- }
- if (var_Z > 0.008856) {
- var_Z = Math.pow(var_Z, 1 / 3.0);
- } else {
- var_Z = (7.787 * var_Z) + (16 / 116.0);
- }
+ // Pivot XÝZ:
+ var_X = pivotXYZ(var_X);
+ var_Y = pivotXYZ(var_Y);
+ var_Z = pivotXYZ(var_Z);
Review comment:
avoids some copy/paste errors as well, in case there are changes
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: Util color extension
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r548923359
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -16,44 +16,46 @@
*/
package org.apache.commons.imaging.color;
+import java.awt.Color;
public final class ColorConversions {
- private static final double REF_X = 95.047; // Observer= 2°, Illuminant= D65
+
+ // White reference
+ /** see: https://en.wikipedia.org/wiki/CIELAB_color_space#From_CIEXYZ_to_CIELAB[10] */
+ private static final double REF_X = 95.047; // Observer= 2°, Illuminant= D65
+
+ /** see: https://en.wikipedia.org/wiki/CIELAB_color_space#From_CIEXYZ_to_CIELAB[10] */
private static final double REF_Y = 100.000;
+
+ /** see: https://en.wikipedia.org/wiki/CIELAB_color_space#From_CIEXYZ_to_CIELAB[10] */
private static final double REF_Z = 108.883;
+ /** see: https://en.wikipedia.org/wiki/CIELAB_color_space#From_CIEXYZ_to_CIELAB[10] */
+ private static final double XYZ_m = 7.787037; // match in slope. Note commonly seen 7.787 gives worse results
+
+ /** see: https://en.wikipedia.org/wiki/CIELAB_color_space#From_CIEXYZ_to_CIELAB[10] */
+ private static final double XYZ_t0 = 0.008856;
Review comment:
Extracted these constants to give them a name that matches the published formulas
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic edited a comment on pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
Brixomatic edited a comment on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-763176214
> Also, there are several commits. Would it be possible to rebase, and squash the commits? Unless you think it would be better to leave this way for now to edit commits if necessary during the review.
It is not advisable to squash/rebase a pushed (i.e. public) branch.
At my workplace on the Github-Enterprise we have the option to squash-merge in Github itself when merging PRs (we even made that mandatory in the settings). It's hidden in the drop down merge-button. Doesn't this method exist here?
See (further down at "Using squash when merging the branch" and "Squashing commits in Github"):
[https://gitbetter.substack.com/p/how-to-squash-git-commits](https://gitbetter.substack.com/p/how-to-squash-git-commits)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-770251882
I removed all the awt connections.
Do you still need a change on this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r560529826
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -643,28 +602,30 @@ private static int convertRGBtoRGB(int red, int green, int blue) {
return rgb;
}
+ private static int convertRGBtoRGB(final Color color) {
+ return color.getRGB() | 0xff << 24; // alpha channel always opaque
+ }
Review comment:
Point taken. I have removed the java.awt reference.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: Util color extension
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r548922724
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -643,28 +602,30 @@ private static int convertRGBtoRGB(int red, int green, int blue) {
return rgb;
}
+ private static int convertRGBtoRGB(final Color color) {
+ return color.getRGB() | 0xff << 24; // alpha channel always opaque
+ }
Review comment:
Adding the direct use of java.awt.Color.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-751301554
[](https://coveralls.io/builds/36450265)
Coverage increased (+0.07%) to 76.497% when pulling **1584ba91343569a1cf5ba16d26ce34a8656ce674 on Brixomatic:util_color_extension** into **2b48e795b58f02108ab262988b673756801de87b on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic edited a comment on pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
Brixomatic edited a comment on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-763176214
> Also, there are several commits. Would it be possible to rebase, and squash the commits? Unless you think it would be better to leave this way for now to edit commits if necessary during the review.
At my workplace on the Github-Enterprise we have the option to squash-merge in Github itself when merging PRs (we even made that mandatory in the settings). It's hidden in the drop down merge-button. Doesn't this method exist here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-792015865
Last chance to ask for any change.
All I can do is offer my contribution, but I won't run after it anymore.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r560529826
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -643,28 +602,30 @@ private static int convertRGBtoRGB(int red, int green, int blue) {
return rgb;
}
+ private static int convertRGBtoRGB(final Color color) {
+ return color.getRGB() | 0xff << 24; // alpha channel always opaque
+ }
Review comment:
Reason taken. I will remove the java.awt reference.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: Util color extension
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r548923245
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -747,4 +700,147 @@ public static ColorXyz convertCIELuvtoXYZ(final double L, final double u, final
return new ColorXyz(X, Y, Z);
}
+
+ public static ColorDIN99Lab convertCIELabToDIN99bLab(final ColorCieLab cie) {
+ return convertCIELabToDIN99bLab(cie.L, cie.a, cie.b);
+ }
+
+ public static ColorDIN99Lab convertCIELabToDIN99bLab(final double L, final double a, final double b) {
+ final double FAC_1 = 100.0 / Math.log(129.0 / 50.0); // = 105.51
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double ang = Math.toRadians(16.0);
+
+ final double L99 = kE * FAC_1 * Math.log(1. + 0.0158 * L);
+ double a99 = 0.0;
+ double b99 = 0.0;
+ if (a != 0.0 || b != 0.0) {
+ final double e = a * Math.cos(ang) + b * Math.sin(ang);
+ final double f = 0.7 * (b * Math.cos(ang) - a * Math.sin(ang));
+ final double G = Math.sqrt(e * e + f * f);
+ if (G != 0.) {
+ final double k = Math.log(1. + 0.045 * G) / (0.045 * kCH * kE * G);
+ a99 = k * e;
+ b99 = k * f;
+ }
+ }
+ return new ColorDIN99Lab(L99, a99, b99);
+ }
+
+ public static ColorCieLab convertDIN99bLabToCIELab(final ColorDIN99Lab dinb) {
+ return convertDIN99bLabToCIELab(dinb.L99, dinb.a99, dinb.b99);
+ }
+
+ public static ColorCieLab convertDIN99bLabToCIELab(final double L99b, final double a99b, final double b99b) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(129.0 / 50.0); // L99 scaling factor = 105.50867113783109
+ final double ang = Math.toRadians(16.0);
+
+ final double hef = Math.atan2(b99b, a99b);
+ final double C = Math.sqrt(a99b * a99b + b99b * b99b);
+ final double G = (Math.exp(0.045 * C * kCH * kE) - 1.0) / 0.045;
+ final double e = G * Math.cos(hef);
+ final double f = G * Math.sin(hef) / 0.7;
+
+ final double L = (Math.exp(L99b * kE / FAC_1) - 1.) / 0.0158;
+ final double a = e * Math.cos(ang) - f * Math.sin(ang);
+ final double b = e * Math.sin(ang) + f * Math.cos(ang);
+ return new ColorCieLab(L, a, b);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorDIN99Lab convertCIELabToDIN99oLab(final ColorCieLab cie) {
+ return convertCIELabToDIN99oLab(cie.L, cie.a, cie.b);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorDIN99Lab convertCIELabToDIN99oLab(final double L, final double a, final double b) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(139.0 / 100.0); // L99 scaling factor = 303.67100547050995
+ final double ang = Math.toRadians(26.0);
+
+ final double L99o = FAC_1 / kE * Math.log(1 + 0.0039 * L); // Lightness correction kE
+ double a99o = 0.0;
+ double b99o = 0.0;
+ if (a != 0.0 || b != 0.0) {
+ final double eo = a * Math.cos(ang) + b * Math.sin(ang); // a stretching
+ final double fo = 0.83 * (b * Math.cos(ang) - a * Math.sin(ang)); // b rotation/stretching
+ final double Go = Math.sqrt(eo * eo + fo * fo); // chroma
+ final double C99o = Math.log(1.0 + 0.075 * Go) / (0.0435 * kCH * kE); // factor for chroma compression and viewing conditions
+ final double heofo = Math.atan2(fo, eo); // arctan in four quadrants
+ final double h99o = heofo + ang; // hue rotation
+ a99o = C99o * Math.cos(h99o);
+ b99o = C99o * Math.sin(h99o);
+ }
+ return new ColorDIN99Lab(L99o, a99o, b99o);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorCieLab convertDIN99oLabToCIELab(final ColorDIN99Lab dino) {
+ return convertDIN99oLabToCIELab(dino.L99, dino.a99, dino.b99);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorCieLab convertDIN99oLabToCIELab(final double L99o, final double a99o, final double b99o) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(139.0 / 100.0); // L99 scaling factor = 303.67100547050995
+ final double ang = Math.toRadians(26.0);
+
+ final double L = (Math.exp(L99o * kE / FAC_1) - 1.0) / 0.0039;
+
+ final double h99ef = Math.atan2(b99o, a99o); // arctan in four quadrants
+
+ final double heofo = h99ef - ang; // backwards hue rotation
+
+ final double C99 = Math.sqrt(a99o * a99o + b99o * b99o); // DIN99 chroma
+ final double G = (Math.exp(0.0435 * kE * kCH * C99) - 1.0) / 0.075; // factor for chroma decompression and viewing conditions
+ final double e = G * Math.cos(heofo);
+ final double f = G * Math.sin(heofo);
+
+ final double a = e * Math.cos(ang) - f / 0.83 * Math.sin(ang); // rotation by 26 degrees
+ final double b = e * Math.sin(ang) + f / 0.83 * Math.cos(ang); // rotation by 26 degrees
+
+ return new ColorCieLab(L, a, b);
+ }
Review comment:
Note that the DIN99 standard has several iterations. These here deal with DIN99b (important milestone) and DIN99o (latest to date, AFAIK). Note I'm using the new class ColorDIN99Lab for both of these. Not sure we need to have separate classes, since both denote a Lab coordinate in the DIN99 color space.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r560530621
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorCieLch.java
##########
@@ -19,10 +19,14 @@
/**
* Represents a color in the CIELCH color space.
*
- * <p>Contains the constant values for black, white, red,
- * green, and blue.</p>
+ * <p>
+ * Contains the constant values for black, white, red,
+ * green, and blue.
+ * </p>
+ * Changes: H renamed to h.
Review comment:
> The changes must be documented in a JIRA issue and linked to the changelog (i.e. the JIRA number is added to src/changes/changes.xml later, used to generate the changes report from the project site).
I don't have access to the JIRA. Would you mind to do that for me, given my comments in this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #114: [IMAGING-283] Add CIELAB and DIN99 conversion, reduce code duplication, and issues related to zero-division and precision
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-751301554
[](https://coveralls.io/builds/37716659)
Coverage increased (+0.07%) to 76.503% when pulling **8d90269a607c77581d147d5b426589984d5d4eeb on Brixomatic:util_color_extension** into **2b48e795b58f02108ab262988b673756801de87b on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #114: [IMAGING-283] Add CIELAB and DIN99 conversion, reduce code duplication, and issues related to zero-division and precision
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-792135471
I've also created the JIRA that will be included in our changelog, along with the credit to you @Brixomatic . Take a look at the issue & updated pull request title, and let me know if there's anything that's missing/incorrect and I'll fix it later.
Cheers
Bruno
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #114: [IMAGING-283] Add CIELAB and DIN99 conversion, reduce code duplication, and issues related to zero-division and precision
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-751301554
[](https://coveralls.io/builds/37714465)
Coverage increased (+0.07%) to 76.503% when pulling **de780beec3b3fb52002db2e3e09fe826cb970915 on Brixomatic:util_color_extension** into **2b48e795b58f02108ab262988b673756801de87b on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: Util color extension
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r548922875
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -643,28 +602,30 @@ private static int convertRGBtoRGB(int red, int green, int blue) {
return rgb;
}
+ private static int convertRGBtoRGB(final Color color) {
+ return color.getRGB() | 0xff << 24; // alpha channel always opaque
+ }
+
public static ColorCieLch convertCIELabtoCIELCH(final ColorCieLab cielab) {
return convertCIELabtoCIELCH(cielab.L, cielab.a, cielab.b);
}
public static ColorCieLch convertCIELabtoCIELCH(final double L, final double a, final double b) {
- double var_H = Math.atan2(b, a); // Quadrant by signs
+ // atan2(y,x) returns atan(y/x)
+ final double atanba = Math.atan2(b, a); // Quadrant by signs
- if (var_H > 0) {
- var_H = (var_H / Math.PI) * 180.0;
- } else {
- var_H = 360 - radian_2_degree(Math.abs(var_H));
- }
+ final double h = atanba > 0 //
+ ? Math.toDegrees(atanba) //
+ : Math.toDegrees(atanba) + 360;
Review comment:
We can very well use java.lang.Math, instead of doing it steo by step. Also reads better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: Util color extension
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r548922678
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -168,34 +157,32 @@ public static ColorXyz convertRGBtoXYZ(final int rgb) {
double var_G = g / 255.0; // Where G = 0 ÷ 255
double var_B = b / 255.0; // Where B = 0 ÷ 255
- if (var_R > 0.04045) {
- var_R = Math.pow((var_R + 0.055) / 1.055, 2.4);
- } else {
- var_R = var_R / 12.92;
- }
- if (var_G > 0.04045) {
- var_G = Math.pow((var_G + 0.055) / 1.055, 2.4);
- } else {
- var_G = var_G / 12.92;
- }
- if (var_B > 0.04045) {
- var_B = Math.pow((var_B + 0.055) / 1.055, 2.4);
- } else {
- var_B = var_B / 12.92;
- }
+ // Pivot RGB:
+ var_R = unPivotRGB(var_R);
+ var_G = unPivotRGB(var_G);
+ var_B = unPivotRGB(var_B);
- var_R = var_R * 100;
- var_G = var_G * 100;
- var_B = var_B * 100;
+ var_R *= 100;
+ var_G *= 100;
+ var_B *= 100;
// Observer. = 2°, Illuminant = D65
- final double X = var_R * 0.4124 + var_G * 0.3576 + var_B * 0.1805;
- final double Y = var_R * 0.2126 + var_G * 0.7152 + var_B * 0.0722;
- final double Z = var_R * 0.0193 + var_G * 0.1192 + var_B * 0.9505;
+ // see: https://github.com/StanfordHCI/c3/blob/master/java/src/edu/stanford/vis/color/LAB.java
+ final double X = var_R * 0.4124564 + var_G * 0.3575761 + var_B * 0.1804375;
+ final double Y = var_R * 0.2126729 + var_G * 0.7151522 + var_B * 0.0721750;
+ final double Z = var_R * 0.0193339 + var_G * 0.1191920 + var_B * 0.9503041;
Review comment:
more precise numbers for better conversion
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #114: [IMAGING-283] Add CIELAB and DIN99 conversion, reduce code duplication, and issues related to zero-division and precision
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-792321946
Squashed commits, re-worded to include JIRA number, added two new commits. The first renaming `ColorDIN99Lab` to `ColorDin99Lab` to match other classes (Jpeg, Lzw, Cie, etc), and last commit adding entry to change log with credit to contributor.
Thank you @Brixomatic for your contribution :tada:
Bruno
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: [IMAGING-283] Add CIELAB and DIN99 conversion, reduce code duplication, and issues related to zero-division and precision
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r588993913
##########
File path: src/test/java/org/apache/commons/imaging/color/ColorConversionsTest.java
##########
@@ -116,4 +117,36 @@ public void testXYZ() {
Debug.debug("cieluv_xyz", cieluv_xyz);
}
}
+
+ @Test
+ public void testRGBtoDin99b() {
+ for (final int rgb : SAMPLE_RGBS) {
+
+ final ColorXyz xyz = ColorConversions.convertRGBtoXYZ(rgb);
Review comment:
will do
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: imaging.color extension. More precision and DIN99b + DIN99o added to ColorConversion.
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r548923245
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -747,4 +700,147 @@ public static ColorXyz convertCIELuvtoXYZ(final double L, final double u, final
return new ColorXyz(X, Y, Z);
}
+
+ public static ColorDIN99Lab convertCIELabToDIN99bLab(final ColorCieLab cie) {
+ return convertCIELabToDIN99bLab(cie.L, cie.a, cie.b);
+ }
+
+ public static ColorDIN99Lab convertCIELabToDIN99bLab(final double L, final double a, final double b) {
+ final double FAC_1 = 100.0 / Math.log(129.0 / 50.0); // = 105.51
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double ang = Math.toRadians(16.0);
+
+ final double L99 = kE * FAC_1 * Math.log(1. + 0.0158 * L);
+ double a99 = 0.0;
+ double b99 = 0.0;
+ if (a != 0.0 || b != 0.0) {
+ final double e = a * Math.cos(ang) + b * Math.sin(ang);
+ final double f = 0.7 * (b * Math.cos(ang) - a * Math.sin(ang));
+ final double G = Math.sqrt(e * e + f * f);
+ if (G != 0.) {
+ final double k = Math.log(1. + 0.045 * G) / (0.045 * kCH * kE * G);
+ a99 = k * e;
+ b99 = k * f;
+ }
+ }
+ return new ColorDIN99Lab(L99, a99, b99);
+ }
+
+ public static ColorCieLab convertDIN99bLabToCIELab(final ColorDIN99Lab dinb) {
+ return convertDIN99bLabToCIELab(dinb.L99, dinb.a99, dinb.b99);
+ }
+
+ public static ColorCieLab convertDIN99bLabToCIELab(final double L99b, final double a99b, final double b99b) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(129.0 / 50.0); // L99 scaling factor = 105.50867113783109
+ final double ang = Math.toRadians(16.0);
+
+ final double hef = Math.atan2(b99b, a99b);
+ final double C = Math.sqrt(a99b * a99b + b99b * b99b);
+ final double G = (Math.exp(0.045 * C * kCH * kE) - 1.0) / 0.045;
+ final double e = G * Math.cos(hef);
+ final double f = G * Math.sin(hef) / 0.7;
+
+ final double L = (Math.exp(L99b * kE / FAC_1) - 1.) / 0.0158;
+ final double a = e * Math.cos(ang) - f * Math.sin(ang);
+ final double b = e * Math.sin(ang) + f * Math.cos(ang);
+ return new ColorCieLab(L, a, b);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorDIN99Lab convertCIELabToDIN99oLab(final ColorCieLab cie) {
+ return convertCIELabToDIN99oLab(cie.L, cie.a, cie.b);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorDIN99Lab convertCIELabToDIN99oLab(final double L, final double a, final double b) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(139.0 / 100.0); // L99 scaling factor = 303.67100547050995
+ final double ang = Math.toRadians(26.0);
+
+ final double L99o = FAC_1 / kE * Math.log(1 + 0.0039 * L); // Lightness correction kE
+ double a99o = 0.0;
+ double b99o = 0.0;
+ if (a != 0.0 || b != 0.0) {
+ final double eo = a * Math.cos(ang) + b * Math.sin(ang); // a stretching
+ final double fo = 0.83 * (b * Math.cos(ang) - a * Math.sin(ang)); // b rotation/stretching
+ final double Go = Math.sqrt(eo * eo + fo * fo); // chroma
+ final double C99o = Math.log(1.0 + 0.075 * Go) / (0.0435 * kCH * kE); // factor for chroma compression and viewing conditions
+ final double heofo = Math.atan2(fo, eo); // arctan in four quadrants
+ final double h99o = heofo + ang; // hue rotation
+ a99o = C99o * Math.cos(h99o);
+ b99o = C99o * Math.sin(h99o);
+ }
+ return new ColorDIN99Lab(L99o, a99o, b99o);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorCieLab convertDIN99oLabToCIELab(final ColorDIN99Lab dino) {
+ return convertDIN99oLabToCIELab(dino.L99, dino.a99, dino.b99);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorCieLab convertDIN99oLabToCIELab(final double L99o, final double a99o, final double b99o) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(139.0 / 100.0); // L99 scaling factor = 303.67100547050995
+ final double ang = Math.toRadians(26.0);
+
+ final double L = (Math.exp(L99o * kE / FAC_1) - 1.0) / 0.0039;
+
+ final double h99ef = Math.atan2(b99o, a99o); // arctan in four quadrants
+
+ final double heofo = h99ef - ang; // backwards hue rotation
+
+ final double C99 = Math.sqrt(a99o * a99o + b99o * b99o); // DIN99 chroma
+ final double G = (Math.exp(0.0435 * kE * kCH * C99) - 1.0) / 0.075; // factor for chroma decompression and viewing conditions
+ final double e = G * Math.cos(heofo);
+ final double f = G * Math.sin(heofo);
+
+ final double a = e * Math.cos(ang) - f / 0.83 * Math.sin(ang); // rotation by 26 degrees
+ final double b = e * Math.sin(ang) + f / 0.83 * Math.cos(ang); // rotation by 26 degrees
+
+ return new ColorCieLab(L, a, b);
+ }
Review comment:
Note that the DIN99 standard has several iterations. These here deal with DIN99b (important milestone) and DIN99o (latest to date, AFAIK). Note I'm using the new class ColorDIN99Lab for both of these. Not sure we need to have separate classes, since both denote a Lab coordinate in the DIN99 color space.
Note that DIN99 is a German norm (DIN stands or German Institute for Standardization), most comprehensive documents and discussions it are in the German language, hence some links to it point to the original sources consulted for the implementation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-763219586
>By the way, is there a file that contains the formatting rules for use in Eclipse or another IDE? I had to have the build done on Github to see whether the lint check was passed.
Unfortunately not. I normally just follow the code style more or less, though I am used to the code base. Then I use `mvn` locally before pushing. The default build target runs checkstyle as well as other plug-ins.
Perhaps Eclipse or IntelliJ would be able to import the settings from Checkstyle? https://github.com/apache/commons-imaging/blob/1b83183805d6034fd9c4fc0aa126ef89abef1a64/checkstyle.xml
Not sure if there's a plug-in for that, or some existing integration.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: Util color extension
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r548923411
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -16,44 +16,46 @@
*/
package org.apache.commons.imaging.color;
+import java.awt.Color;
public final class ColorConversions {
- private static final double REF_X = 95.047; // Observer= 2°, Illuminant= D65
+
+ // White reference
+ /** see: https://en.wikipedia.org/wiki/CIELAB_color_space#From_CIEXYZ_to_CIELAB[10] */
+ private static final double REF_X = 95.047; // Observer= 2°, Illuminant= D65
+
+ /** see: https://en.wikipedia.org/wiki/CIELAB_color_space#From_CIEXYZ_to_CIELAB[10] */
private static final double REF_Y = 100.000;
+
+ /** see: https://en.wikipedia.org/wiki/CIELAB_color_space#From_CIEXYZ_to_CIELAB[10] */
private static final double REF_Z = 108.883;
+ /** see: https://en.wikipedia.org/wiki/CIELAB_color_space#From_CIEXYZ_to_CIELAB[10] */
+ private static final double XYZ_m = 7.787037; // match in slope. Note commonly seen 7.787 gives worse results
+
+ /** see: https://en.wikipedia.org/wiki/CIELAB_color_space#From_CIEXYZ_to_CIELAB[10] */
+ private static final double XYZ_t0 = 0.008856;
Review comment:
Extracted these constants to give them a name that matches the published formulas
Note that for XYZ_m, I'm using a more precise value.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-792027295
Sorry for the delay @Brixomatic , I understand the frustration, but there are only volunteers reviewing the project, and life sometimes gets in the way :-) any time an issue takes too long to be reviewed, merged, or have some feedback, just send something like a "ping" message and a committer normally checks why it's taking so long.
Thanks
Bruno
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r560543860
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorCieLch.java
##########
@@ -19,10 +19,14 @@
/**
* Represents a color in the CIELCH color space.
*
- * <p>Contains the constant values for black, white, red,
- * green, and blue.</p>
+ * <p>
+ * Contains the constant values for black, white, red,
+ * green, and blue.
+ * </p>
+ * Changes: H renamed to h.
Review comment:
> Sure, will create it later and link it here :-)
Splendid, thank you!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: imaging.color extension. More precision and DIN99b + DIN99o added to ColorConversion.
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r548923245
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -747,4 +700,147 @@ public static ColorXyz convertCIELuvtoXYZ(final double L, final double u, final
return new ColorXyz(X, Y, Z);
}
+
+ public static ColorDIN99Lab convertCIELabToDIN99bLab(final ColorCieLab cie) {
+ return convertCIELabToDIN99bLab(cie.L, cie.a, cie.b);
+ }
+
+ public static ColorDIN99Lab convertCIELabToDIN99bLab(final double L, final double a, final double b) {
+ final double FAC_1 = 100.0 / Math.log(129.0 / 50.0); // = 105.51
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double ang = Math.toRadians(16.0);
+
+ final double L99 = kE * FAC_1 * Math.log(1. + 0.0158 * L);
+ double a99 = 0.0;
+ double b99 = 0.0;
+ if (a != 0.0 || b != 0.0) {
+ final double e = a * Math.cos(ang) + b * Math.sin(ang);
+ final double f = 0.7 * (b * Math.cos(ang) - a * Math.sin(ang));
+ final double G = Math.sqrt(e * e + f * f);
+ if (G != 0.) {
+ final double k = Math.log(1. + 0.045 * G) / (0.045 * kCH * kE * G);
+ a99 = k * e;
+ b99 = k * f;
+ }
+ }
+ return new ColorDIN99Lab(L99, a99, b99);
+ }
+
+ public static ColorCieLab convertDIN99bLabToCIELab(final ColorDIN99Lab dinb) {
+ return convertDIN99bLabToCIELab(dinb.L99, dinb.a99, dinb.b99);
+ }
+
+ public static ColorCieLab convertDIN99bLabToCIELab(final double L99b, final double a99b, final double b99b) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(129.0 / 50.0); // L99 scaling factor = 105.50867113783109
+ final double ang = Math.toRadians(16.0);
+
+ final double hef = Math.atan2(b99b, a99b);
+ final double C = Math.sqrt(a99b * a99b + b99b * b99b);
+ final double G = (Math.exp(0.045 * C * kCH * kE) - 1.0) / 0.045;
+ final double e = G * Math.cos(hef);
+ final double f = G * Math.sin(hef) / 0.7;
+
+ final double L = (Math.exp(L99b * kE / FAC_1) - 1.) / 0.0158;
+ final double a = e * Math.cos(ang) - f * Math.sin(ang);
+ final double b = e * Math.sin(ang) + f * Math.cos(ang);
+ return new ColorCieLab(L, a, b);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorDIN99Lab convertCIELabToDIN99oLab(final ColorCieLab cie) {
+ return convertCIELabToDIN99oLab(cie.L, cie.a, cie.b);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorDIN99Lab convertCIELabToDIN99oLab(final double L, final double a, final double b) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(139.0 / 100.0); // L99 scaling factor = 303.67100547050995
+ final double ang = Math.toRadians(26.0);
+
+ final double L99o = FAC_1 / kE * Math.log(1 + 0.0039 * L); // Lightness correction kE
+ double a99o = 0.0;
+ double b99o = 0.0;
+ if (a != 0.0 || b != 0.0) {
+ final double eo = a * Math.cos(ang) + b * Math.sin(ang); // a stretching
+ final double fo = 0.83 * (b * Math.cos(ang) - a * Math.sin(ang)); // b rotation/stretching
+ final double Go = Math.sqrt(eo * eo + fo * fo); // chroma
+ final double C99o = Math.log(1.0 + 0.075 * Go) / (0.0435 * kCH * kE); // factor for chroma compression and viewing conditions
+ final double heofo = Math.atan2(fo, eo); // arctan in four quadrants
+ final double h99o = heofo + ang; // hue rotation
+ a99o = C99o * Math.cos(h99o);
+ b99o = C99o * Math.sin(h99o);
+ }
+ return new ColorDIN99Lab(L99o, a99o, b99o);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorCieLab convertDIN99oLabToCIELab(final ColorDIN99Lab dino) {
+ return convertDIN99oLabToCIELab(dino.L99, dino.a99, dino.b99);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorCieLab convertDIN99oLabToCIELab(final double L99o, final double a99o, final double b99o) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(139.0 / 100.0); // L99 scaling factor = 303.67100547050995
+ final double ang = Math.toRadians(26.0);
+
+ final double L = (Math.exp(L99o * kE / FAC_1) - 1.0) / 0.0039;
+
+ final double h99ef = Math.atan2(b99o, a99o); // arctan in four quadrants
+
+ final double heofo = h99ef - ang; // backwards hue rotation
+
+ final double C99 = Math.sqrt(a99o * a99o + b99o * b99o); // DIN99 chroma
+ final double G = (Math.exp(0.0435 * kE * kCH * C99) - 1.0) / 0.075; // factor for chroma decompression and viewing conditions
+ final double e = G * Math.cos(heofo);
+ final double f = G * Math.sin(heofo);
+
+ final double a = e * Math.cos(ang) - f / 0.83 * Math.sin(ang); // rotation by 26 degrees
+ final double b = e * Math.sin(ang) + f / 0.83 * Math.cos(ang); // rotation by 26 degrees
+
+ return new ColorCieLab(L, a, b);
+ }
Review comment:
Note that the DIN99 standard has several iterations. These here deal with DIN99b (important milestone) and DIN99o (latest to date, AFAIK). Note I'm using the new class ColorDIN99Lab for both of these. Not sure we need to have separate classes, since both denote a Lab coordinate in the DIN99 color space.
Note that DIN99 is a German norm (DIN stands or German Institute for Standardization), most comprehensive documents and discussions it are in the German language, hence links to it point to the source documents consulted for the implementation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-763179912
> > Also, there are several commits. Would it be possible to rebase, and squash the commits? Unless you think it would be better to leave this way for now to edit commits if necessary during the review.
>
> At my workplace on the Github-Enterprise we have the option to squash-merge in Github itself when merging PRs (we even made that mandatory in the settings). Doesn't this method exist here?
Not sure. A committer when merging (when s/he remembers) will squash the commits, normally manually (that's how I do it, with `git` command line locally).
One issue with commiters merging (and not the contributor) is that there are some complex issues where it might be better to have more than 1 commit too. But in those cases, the committer might not be aware, and squash everything down to a single commit.
Another issue is when the PR takes a while to be merged. If there's any large-ish PR merged first, fixing conflicts in multiple commits may be complicated (or it may be helpful too). In that case, the contributor's code could have an issue that happened during the merge process, not from the contributor code.
We ask contributors to squash the commits down to the minimum or a reasonable number of commits, but in cases where it's hard (we know and appreciate the contribution, and understand some times it's hard to find spare time to rebase/squash or do other chores for the ASF review process :-) we can take care of that too :+1:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Brixomatic commented on a change in pull request #114: imaging.color extension. More precision and DIN99b + DIN99o added to ColorConversion.
Posted by GitBox <gi...@apache.org>.
Brixomatic commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r548923245
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorConversions.java
##########
@@ -747,4 +700,147 @@ public static ColorXyz convertCIELuvtoXYZ(final double L, final double u, final
return new ColorXyz(X, Y, Z);
}
+
+ public static ColorDIN99Lab convertCIELabToDIN99bLab(final ColorCieLab cie) {
+ return convertCIELabToDIN99bLab(cie.L, cie.a, cie.b);
+ }
+
+ public static ColorDIN99Lab convertCIELabToDIN99bLab(final double L, final double a, final double b) {
+ final double FAC_1 = 100.0 / Math.log(129.0 / 50.0); // = 105.51
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double ang = Math.toRadians(16.0);
+
+ final double L99 = kE * FAC_1 * Math.log(1. + 0.0158 * L);
+ double a99 = 0.0;
+ double b99 = 0.0;
+ if (a != 0.0 || b != 0.0) {
+ final double e = a * Math.cos(ang) + b * Math.sin(ang);
+ final double f = 0.7 * (b * Math.cos(ang) - a * Math.sin(ang));
+ final double G = Math.sqrt(e * e + f * f);
+ if (G != 0.) {
+ final double k = Math.log(1. + 0.045 * G) / (0.045 * kCH * kE * G);
+ a99 = k * e;
+ b99 = k * f;
+ }
+ }
+ return new ColorDIN99Lab(L99, a99, b99);
+ }
+
+ public static ColorCieLab convertDIN99bLabToCIELab(final ColorDIN99Lab dinb) {
+ return convertDIN99bLabToCIELab(dinb.L99, dinb.a99, dinb.b99);
+ }
+
+ public static ColorCieLab convertDIN99bLabToCIELab(final double L99b, final double a99b, final double b99b) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(129.0 / 50.0); // L99 scaling factor = 105.50867113783109
+ final double ang = Math.toRadians(16.0);
+
+ final double hef = Math.atan2(b99b, a99b);
+ final double C = Math.sqrt(a99b * a99b + b99b * b99b);
+ final double G = (Math.exp(0.045 * C * kCH * kE) - 1.0) / 0.045;
+ final double e = G * Math.cos(hef);
+ final double f = G * Math.sin(hef) / 0.7;
+
+ final double L = (Math.exp(L99b * kE / FAC_1) - 1.) / 0.0158;
+ final double a = e * Math.cos(ang) - f * Math.sin(ang);
+ final double b = e * Math.sin(ang) + f * Math.cos(ang);
+ return new ColorCieLab(L, a, b);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorDIN99Lab convertCIELabToDIN99oLab(final ColorCieLab cie) {
+ return convertCIELabToDIN99oLab(cie.L, cie.a, cie.b);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorDIN99Lab convertCIELabToDIN99oLab(final double L, final double a, final double b) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(139.0 / 100.0); // L99 scaling factor = 303.67100547050995
+ final double ang = Math.toRadians(26.0);
+
+ final double L99o = FAC_1 / kE * Math.log(1 + 0.0039 * L); // Lightness correction kE
+ double a99o = 0.0;
+ double b99o = 0.0;
+ if (a != 0.0 || b != 0.0) {
+ final double eo = a * Math.cos(ang) + b * Math.sin(ang); // a stretching
+ final double fo = 0.83 * (b * Math.cos(ang) - a * Math.sin(ang)); // b rotation/stretching
+ final double Go = Math.sqrt(eo * eo + fo * fo); // chroma
+ final double C99o = Math.log(1.0 + 0.075 * Go) / (0.0435 * kCH * kE); // factor for chroma compression and viewing conditions
+ final double heofo = Math.atan2(fo, eo); // arctan in four quadrants
+ final double h99o = heofo + ang; // hue rotation
+ a99o = C99o * Math.cos(h99o);
+ b99o = C99o * Math.sin(h99o);
+ }
+ return new ColorDIN99Lab(L99o, a99o, b99o);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorCieLab convertDIN99oLabToCIELab(final ColorDIN99Lab dino) {
+ return convertDIN99oLabToCIELab(dino.L99, dino.a99, dino.b99);
+ }
+
+ /** DIN99o, see: https://de.wikipedia.org/w/index.php?title=Diskussion:DIN99-Farbraum */
+ public static ColorCieLab convertDIN99oLabToCIELab(final double L99o, final double a99o, final double b99o) {
+ final double kE = 1.0; // brightness factor, 1.0 for CIE reference conditions
+ final double kCH = 1.0; // chroma and hue factor, 1.0 for CIE reference conditions
+ final double FAC_1 = 100.0 / Math.log(139.0 / 100.0); // L99 scaling factor = 303.67100547050995
+ final double ang = Math.toRadians(26.0);
+
+ final double L = (Math.exp(L99o * kE / FAC_1) - 1.0) / 0.0039;
+
+ final double h99ef = Math.atan2(b99o, a99o); // arctan in four quadrants
+
+ final double heofo = h99ef - ang; // backwards hue rotation
+
+ final double C99 = Math.sqrt(a99o * a99o + b99o * b99o); // DIN99 chroma
+ final double G = (Math.exp(0.0435 * kE * kCH * C99) - 1.0) / 0.075; // factor for chroma decompression and viewing conditions
+ final double e = G * Math.cos(heofo);
+ final double f = G * Math.sin(heofo);
+
+ final double a = e * Math.cos(ang) - f / 0.83 * Math.sin(ang); // rotation by 26 degrees
+ final double b = e * Math.sin(ang) + f / 0.83 * Math.cos(ang); // rotation by 26 degrees
+
+ return new ColorCieLab(L, a, b);
+ }
Review comment:
Note that the DIN99 standard has several iterations. These here deal with DIN99b (important milestone) and DIN99o (latest to date, AFAIK). Note I'm using the new class ColorDIN99Lab for both of these. Not sure we need to have separate classes, since both denote a Lab coordinate in the DIN99 color space.
Note that DIN99 is a German norm (DIN stands or German Institute for Standardization), most comprehensive documents and discussions it are in the German language, hence some links to it point to the source documents consulted for the implementation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r588928755
##########
File path: src/test/java/org/apache/commons/imaging/color/ColorConversionsTest.java
##########
@@ -116,4 +117,36 @@ public void testXYZ() {
Debug.debug("cieluv_xyz", cieluv_xyz);
}
}
+
+ @Test
+ public void testRGBtoDin99b() {
+ for (final int rgb : SAMPLE_RGBS) {
+
+ final ColorXyz xyz = ColorConversions.convertRGBtoXYZ(rgb);
Review comment:
@Brixomatic it looks like there are tabs in this file. When you update this PR, could you check if there are any tabs and, please, replace by spaces?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #114: imaging.color extension. More precision and DIN99b + DIN99o added to ColorConversion.
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#issuecomment-751301554
[](https://coveralls.io/builds/35949436)
Coverage increased (+0.03%) to 76.459% when pulling **8850b15d2c863f682123c6dc9d9c92fda7fc741d on Brixomatic:util_color_extension** into **2b48e795b58f02108ab262988b673756801de87b on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #114: imaging.color extension/fix. More precision, DIN99b + DIN99o added to ColorConversion, division by zero error fixed.
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #114:
URL: https://github.com/apache/commons-imaging/pull/114#discussion_r588928623
##########
File path: src/main/java/org/apache/commons/imaging/color/ColorCieLch.java
##########
@@ -19,10 +19,14 @@
/**
* Represents a color in the CIELCH color space.
*
- * <p>Contains the constant values for black, white, red,
- * green, and blue.</p>
+ * <p>
+ * Contains the constant values for black, white, red,
+ * green, and blue.
+ * </p>
+ * Changes: H renamed to h.
Review comment:
Not a problem. But this needs to be removed from this file anyway in the meantime :+1: I will create the issue later once I think of a good title/description.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org