Add missing b-tweak to skcms_TF_Invert()
First I added an assert (x >= 0) to powf_().
That failing helped me notice we're not
checking ad+b >=0 in tf_is_valid().
That failing helped me notice skcms_TF_Invert() can
take ad+b >= 0 inputs and create ad + b < 0 outputs.
To fix that, apply the same tweak to b we do when fitting.
To follow the story in smaller steps,
1) https://skia-review.googlesource.com/c/skcms/+/235899
2) https://skia-review.googlesource.com/c/skcms/+/235909
3) https://skia-review.googlesource.com/c/skcms/+/235912
Bug: chromium:976551
Change-Id: If281a2151f41b6dd5bcc86992d2104576f7c985f
Reviewed-on: https://skia-review.googlesource.com/c/skcms/+/235913
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
diff --git a/profiles/fuzz/polytf_big_float_to_int_cast.icc.txt b/profiles/fuzz/polytf_big_float_to_int_cast.icc.txt
index ceb639a..979baac 100644
--- a/profiles/fuzz/polytf_big_float_to_int_cast.icc.txt
+++ b/profiles/fuzz/polytf_big_float_to_int_cast.icc.txt
@@ -1,48 +1 @@
- Size : 0x0000020C : 524
- Data color space : 0x20202020 : ' '
- PCS : 0x58595A20 : 'XYZ '
- Tag count : 0x0000000A : 10
-
- Tag : Type : Size : Offset
- ------ : ------ : ------ : --------
- ' ' : ' ' : 32 : 32
- ' ' : ' ' : 32 : 288
- ' ' : ' ' : 32 : 288
- ' ' : ' ' : 32 : 288
- 'rXYZ' : 'XYZ ' : 32 : 400
- 'gXYZ' : 'XYZ ' : 32 : 420
- 'bXYZ' : 'XYZ ' : 32 : 440
- 'rTRC' : 'para' : 32 : 456
- 'gTRC' : 'para' : 32 : 456
- 'bTRC' : 'curv' : 32 : 460
-
-rTRC : 1.525879e-05, 0.003890991, -32768, 8224.125, 0, 0, 0
-gTRC : 1.525879e-05, 0.003890991, -32768, 8224.125, 0, 0, 0
-bTRC : 0, 1, 0, 0, 0, 0, 0
- XYZ : | 8224.125 8224.125 8224.125 |
- | 8224.125 8224.125 8224.125 |
- | 8224.125 -223.87451 28769.447 |
- !!! This does not appear to use a D50 whitepoint, rather [24672.4 24672.4 36769.7]
-252 random bytes transformed to linear XYZD50 bytes:
- ffffff ffffff ffffff ffffff ffffff ffffff ffffff
- ffffff ffffff ffffff ffffff ffffff ffffff ffffff
- ffffff ffffff ffffff ffffff ffffff ffffff ffffff
- ffffff ffffff ffffff ffffff ffffff ffffff ffffff
- ffffff ffffff ffffff ffffff ffffff ffffff ffffff
- ffffff ffffff ffffff ffffff ffffff ffffff ffffff
- ffffff ffffff ffffff ffffff ffffff ffffff ffffff
- ffffff ffffff ffffff ffffff ffffff ffffff ffffff
- ffffff ffffff ffffff ffffff ffffff ffffff ffffff
- ffffff ffffff ffffff ffffff ffffff ffffff ffffff
- ffffff ffffff ffffff ffffff ffffff ffffff ffffff
- ffffff ffffff ffffff ffffff ffffff ffffff ffffff
-81 edge-case pixels transformed to sRGB 8888 (unpremul):
- 00ffffff 00ffffff 00ffffff 00ffffff 00ffffff 00ffffff 00ffffff 00ffffff 00ffffff
- 00ffffff 00ffffff 00ffffff 00ffffff 00ffffff 00ffffff 00ffffff 00ffffff 00ffffff
- 00ffffff 00ffffff 00ffffff 00ffffff 00ffffff 00ffffff 00ffffff 00ffffff 00ffffff
- 7fffffff 7fffffff 7fffffff 7fffffff 7fffffff 7fffffff 7fffffff 7fffffff 7fffffff
- 7fffffff 7fffffff 7fffffff 7fffffff 7fffffff 7fffffff 7fffffff 7fffffff 7fffffff
- 7fffffff 7fffffff 7fffffff 7fffffff 7fffffff 7fffffff 7fffffff 7fffffff 7fffffff
- ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff
- ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff
- ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff
+Unable to parse ICC profile
diff --git a/profiles/misc/Coated_FOGRA39_CMYK.icc.txt b/profiles/misc/Coated_FOGRA39_CMYK.icc.txt
index 4a0e2fe..cae8402 100644
--- a/profiles/misc/Coated_FOGRA39_CMYK.icc.txt
+++ b/profiles/misc/Coated_FOGRA39_CMYK.icc.txt
@@ -23,7 +23,7 @@
A2B : "A", CLUT, "B"
"A" : 4 inputs
A0 : 16-bit table with 256 entries
- ~= : 1.05448, 1.018715, -0.007989922, 0.8365758, 0.007843138, 0.0066185, 0 (Max error: 0.890015) (D-gap: 5.71203e-05)
+ ~= : 1.05448, 1.018715, -0.007989922, 0.8365758, 0.007843138, 0.0066185, 0 (Max error: 0.0167046) (D-gap: 5.71203e-05)
A1 : 16-bit table with 256 entries
~= : 1.117216, 1.017609, 0.0126555, 0.7665369, 0.007843138, -0.007049561, 0 (Max error: 0.0234528) (D-gap: 3.23332e-05)
A2 : 16-bit table with 256 entries
diff --git a/profiles/misc/DisplayCal_ASUS_NonMonotonic.icc.txt b/profiles/misc/DisplayCal_ASUS_NonMonotonic.icc.txt
index 0556959..5bbce8a 100644
--- a/profiles/misc/DisplayCal_ASUS_NonMonotonic.icc.txt
+++ b/profiles/misc/DisplayCal_ASUS_NonMonotonic.icc.txt
@@ -31,7 +31,7 @@
'meta' : 'dict' : 2312 : 738448
rTRC : 16-bit table with 256 entries
- ~= : 1.929522, 1.131104, -0.133071, 0.03579767, 0.1176471, 0.004390717, 0 (Max error: 1.34651e+35) (D-gap: 0.000179225)
+ ~= : 1.929522, 1.131104, -0.133071, 0.03579767, 0.1176471, 0.004390717, 0 (Max error: 0.0831201) (D-gap: 0.000179225)
gTRC : 16-bit table with 256 entries
~= : 2.287451, 0.9762917, 0.03631328, 0.03112842, 0.1921569, -0.02641296, 0 (Max error: 0.0558824) (D-gap: 0.000206613)
bTRC : 16-bit table with 256 entries
diff --git a/profiles/misc/Japan_Color_2001_Coated.icc.txt b/profiles/misc/Japan_Color_2001_Coated.icc.txt
index a8f9508..df4905b 100644
--- a/profiles/misc/Japan_Color_2001_Coated.icc.txt
+++ b/profiles/misc/Japan_Color_2001_Coated.icc.txt
@@ -19,9 +19,9 @@
A2B : "A", CLUT, "B"
"A" : 4 inputs
A0 : 16-bit table with 256 entries
- ~= : 1.010957, 1.063629, -0.03336876, 0.7801556, 0.03137255, 0.02462769, 0 (Max error: 0.0513268) (D-gap: 0.000152213)
+ ~= : 1.010957, 1.063629, -0.03336876, 0.7801556, 0.03137255, 0.02462769, 0 (Max error: 0.0513306) (D-gap: 0.000152213)
A1 : 16-bit table with 256 entries
- ~= : 1.127547, 1.01329, -0.000483401, 0.3501945, 0.003921568, -0.000289917, 0 (Max error: 0.0123787) (D-gap: 3.26649e-05)
+ ~= : 1.117948, 1.022087, -0.0280573, 0.6270149, 0.02745098, 0.01732635, 0 (Max error: 0.00934219) (D-gap: 0.000114178)
A2 : 16-bit table with 256 entries
~= : 1.158917, 0.990242, 0.04165715, 0.6193256, 0.02352941, -0.02733612, 0 (Max error: 0.00832367) (D-gap: 0.000154509)
A3 : 16-bit table with 256 entries
diff --git a/profiles/misc/Phase_One_P25.icc.txt b/profiles/misc/Phase_One_P25.icc.txt
index 7f6fd47..cbf18f2 100644
--- a/profiles/misc/Phase_One_P25.icc.txt
+++ b/profiles/misc/Phase_One_P25.icc.txt
@@ -22,7 +22,7 @@
~= : 0.3802269, 1.743515, 0.01529081, 3.844358, 0.007843138, -0.2299347, 0 (Max error: 0.0201225) (D-gap: 9.07481e-06)
gTRC : 16-bit table with 256 entries
bTRC : 16-bit table with 256 entries
- ~= : 0.7390234, 0.8481178, -0.009977858, 2.721141, 0.01176471, 0.03213882, 0 (Max error: 0.139809) (D-gap: 0.000125397)
+ ~= : 0.7390234, 0.8481178, -0.009977858, 2.721141, 0.01176471, 0.03213882, 0 (Max error: 0.139824) (D-gap: 0.000125397)
XYZ : | 0.64790344 0.35736084 0.15641785 |
| 0.3829193 1.109726 0 |
| 0.08326721 0.6792755 0.52342224 |
diff --git a/profiles/misc/US_Web_Coated_SWOP_CMYK.icc.txt b/profiles/misc/US_Web_Coated_SWOP_CMYK.icc.txt
index bcd76ba..35f97bf 100644
--- a/profiles/misc/US_Web_Coated_SWOP_CMYK.icc.txt
+++ b/profiles/misc/US_Web_Coated_SWOP_CMYK.icc.txt
@@ -21,9 +21,9 @@
A0 : 16-bit table with 256 entries
~= : 0.7457523, 1.273102, 0.2824915, 2.132296, 0.003921569, -0.3862457, 0 (Max error: 0.00514561) (D-gap: 5.51855e-05)
A1 : 16-bit table with 256 entries
- ~= : 1.001741, 0.9817963, -0.01540073, 1.622568, 0.01568628, 0.02549744, 0 (Max error: 0.00973922) (D-gap: 4.53889e-05)
+ ~= : 1.001741, 0.9817963, -0.01540073, 1.622568, 0.01568628, 0.02549744, 0 (Max error: 0.00973541) (D-gap: 4.53889e-05)
A2 : 16-bit table with 256 entries
- ~= : 1.041717, 0.9965515, -0.003908046, 2.011673, 0.003921569, 0.007892609, 0 (Max error: 0.022772) (D-gap: 3.69456e-06)
+ ~= : 1.043713, 0.9970474, -0.01563996, 1.68677, 0.01568628, 0.02657318, 0 (Max error: 0.0163442) (D-gap: 0.000114037)
A3 : 16-bit table with 256 entries
CLUT : 9 x 9 x 9 x 9 (16 bpp)
"B" : 3 outputs
diff --git a/profiles/misc/crbug_976551.icc.txt b/profiles/misc/crbug_976551.icc.txt
index d36933d..e614b09 100644
--- a/profiles/misc/crbug_976551.icc.txt
+++ b/profiles/misc/crbug_976551.icc.txt
@@ -21,13 +21,13 @@
'mmod' : 'mmod' : 40 : 13080
rTRC : 16-bit table with 1024 entries
- ~= : 1.348469, 1.17157, -0.009161836, 0.009756237, 0.007820137, 0.0001716614, 0 (Max error: 7.31654e+16) (D-gap: 9.53663e-05)
+ ~= : 1.408492, 1.078773, -0.08119801, 0.1001471, 0.07526882, 0.007678986, 0 (Max error: 0.0193188) (D-gap: 0.000141029)
gTRC : 16-bit table with 1024 entries
- ~= : 1.378535, 1.13559, -0.007770408, 0.008919988, 0.00684262, 0.0001716614, 0 (Max error: 1.72219e+18) (D-gap: 0.000110625)
+ ~= : 1.430862, 1.067261, -0.07720166, 0.1031524, 0.07233626, 0.007591248, 0 (Max error: 0.0184169) (D-gap: 0.000129586)
bTRC : 16-bit table with 1024 entries
- ~= : 1.326482, 1.226715, -0.00959308, 0.009756237, 0.007820137, 0.0001296997, 0 (Max error: 4.54796e+15) (D-gap: 5.34046e-05)
-Best : 1.326482, 1.226715, -0.00959308, 0.009756237, 0.007820137, 0.0001296997, 0 (D-gap: 5.34046e-05)
-Inv : 0.7538736, 0.7626305, -9.891295e-05, 102.4985, 7.629511e-05, 0.007846832, -0 (D-gap: -4.54796e+15)
+ ~= : 1.335367, 1.135376, -0.0876781, 0.09840215, 0.07722384, 0.007732391, 0 (Max error: 0.0542429) (D-gap: 0.000133399)
+Best : 1.408492, 1.078773, -0.08119801, 0.1001471, 0.07526882, 0.007678986, 0 (D-gap: 0.000141029)
+Inv : 0.7099792, 0.8987885, -0.006775029, 9.985308, 0.007537957, 0.0752182, -0 (D-gap: -5.06192e-05)
XYZ : | 0.512558 0.29548645 0.15614319 |
| 0.24029541 0.7005768 0.059127808 |
| -0.0010375977 0.042297363 0.7836609 |
diff --git a/skcms.cc b/skcms.cc
index a27d025..65188ca 100644
--- a/skcms.cc
+++ b/skcms.cc
@@ -266,6 +266,11 @@
return false;
}
+ // It's rather _complex_ to raise a negative number to a fractional power tf->g.
+ if (tf->a * tf->d + tf->b < 0) {
+ return false;
+ }
+
return true;
}
@@ -1404,6 +1409,7 @@
}
float powf_(float x, float y) {
+ assert (x >= 0);
return (x == 0) || (x == 1) ? x
: exp2f_(log2f_(x) * y);
}
@@ -1468,6 +1474,17 @@
inv.b = -k * src->e;
inv.e = -src->b / src->a;
+ // We need to enforce the same constraints here that we do when fitting a curve,
+ // a >= 0 and ad+b >= 0. These constraints are checked by tf_is_valid(), so they're true
+ // of the source function if we're here. And if the source a >= 0, inv.a definitely is too.
+ assert (inv.a >= 0);
+
+ // On the other hand our ad+b could have gone slightly negative here. Tweak inv.b if needed.
+ if (inv.a * inv.d + inv.b < 0) {
+ inv.b = -inv.a * inv.d;
+ }
+ assert (inv.a * inv.d + inv.b >= 0);
+
// Now in principle we're done.
// But to preserve the valuable invariant inv(src(1.0f)) == 1.0f,
// we'll tweak e. These two values should be close to each other,
@@ -1663,6 +1680,9 @@
P[2] = -P[1] * tf->d;
}
+ assert (P[1] >= 0 &&
+ P[1] * tf->d + P[2] >= 0);
+
tf->g = P[0];
tf->a = P[1];
tf->b = P[2];
