skcms_TransferFunction_invert() for PQ/HLG
Absolutely nothing tricky.
This is enough to clean up tf_is_valid().
Bug: chromium:960620
Change-Id: I56c5017fddf167f576879a7d1041455023108c0c
Reviewed-on: https://skia-review.googlesource.com/c/skcms/+/247729
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/skcms.cc b/skcms.cc
index 4a4fc47..85282c0 100644
--- a/skcms.cc
+++ b/skcms.cc
@@ -160,12 +160,6 @@
return Bad;
}
-// TODO: temporary shim for old call sites
-static bool tf_is_valid(const skcms_TransferFunction* tf) {
- return classify(*tf) == sRGBish;
-}
-
-
bool skcms_TransferFunction_makePQish(skcms_TransferFunction* tf,
float A, float B, float C,
float D, float E, float F) {
@@ -489,7 +483,7 @@
curve->parametric.f = read_big_fixed(paraTag->variable + 24);
break;
}
- return tf_is_valid(&curve->parametric);
+ return classify(curve->parametric) == sRGBish;
}
typedef struct {
@@ -1511,13 +1505,33 @@
}
#if defined(__clang__)
- [[clang::no_sanitize("float-divide-by-zero")]] // Checked for by tf_is_valid() on the way out.
+ [[clang::no_sanitize("float-divide-by-zero")]] // Checked for by classify() on the way out.
#endif
bool skcms_TransferFunction_invert(const skcms_TransferFunction* src, skcms_TransferFunction* dst) {
- if (!tf_is_valid(src)) {
- return false;
+ TF_PQish pq;
+ TF_HLGish hlg;
+ switch (classify(*src, &pq, &hlg)) {
+ case Bad: return false;
+ case sRGBish: break; // handled below
+
+ case PQish:
+ *dst = { TFKind_marker(PQish), -pq.A, pq.D, 1.0f/pq.F
+ , pq.B, -pq.E, 1.0f/pq.C};
+ return true;
+
+ case HLGish:
+ *dst = { TFKind_marker(HLGinvish), 1.0f/hlg.R, 1.0f/hlg.G
+ , 1.0f/hlg.a, hlg.b, hlg.c, 0 };
+ return true;
+
+ case HLGinvish:
+ *dst = { TFKind_marker(HLGish), 1.0f/hlg.R, 1.0f/hlg.G
+ , 1.0f/hlg.a, hlg.b, hlg.c, 0 };
+ return true;
}
+ assert (classify(*src) == sRGBish);
+
// We're inverting this function, solving for x in terms of y.
// y = (cx + f) x < d
// (ax + b)^g + e x ≥ d
@@ -1563,7 +1577,7 @@
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
+ // a >= 0 and ad+b >= 0. These constraints are checked by classify(), so they're true
// of the source function if we're here.
// Just like when fitting the curve, there's really no way to rescue a < 0.
@@ -1575,9 +1589,9 @@
inv.b = -inv.a * inv.d;
}
- // That should usually make tf_is_valid(&inv) true, but there are a couple situations
+ // That should usually make classify(inv) == sRGBish true, but there are a couple situations
// where we might still fail here, like non-finite parameter values.
- if (!tf_is_valid(&inv)) {
+ if (classify(inv) != sRGBish) {
return false;
}
@@ -1601,7 +1615,7 @@
}
*dst = inv;
- return tf_is_valid(dst);
+ return classify(*dst) == sRGBish;
}
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ //
diff --git a/tests.c b/tests.c
index b7370a0..026f669 100644
--- a/tests.c
+++ b/tests.c
@@ -1458,6 +1458,79 @@
expect(12.00001f > skcms_TransferFunction_eval(&dec, 1.0f));
}
+static void test_PQ_invert() {
+ skcms_TransferFunction pqA, invA, invB;
+
+ expect(skcms_TransferFunction_makePQ (&pqA) );
+ expect(skcms_TransferFunction_makePQinv(&invA));
+ expect(skcms_TransferFunction_invert(&pqA, &invB));
+
+ // a,b,d,e really just negate and swap around,
+ // so those should be exact. c and f will 1.0f/x
+ // each other, so they might not be exactly perfect,
+ // but it turns out we do get lucky here.
+
+ expect(invA.g == invB.g); // I.e. are we still PQ?
+ expect(invA.a == invB.a);
+ expect(invA.b == invB.b);
+ expect(invA.c == invB.c); // We got lucky here.
+ expect(invA.d == invB.d);
+ expect(invA.e == invB.e);
+ expect(invA.f == invB.f); // And here.
+
+ // Just for fun, invert back to PQ.
+ // This just tests the same code path twice.
+ skcms_TransferFunction pqB;
+ expect(skcms_TransferFunction_invert(&invA, &pqB));
+
+ expect(pqA.g == pqB.g);
+ expect(pqA.a == pqB.a);
+ expect(pqA.b == pqB.b);
+ expect(pqA.c == pqB.c);
+ expect(pqA.d == pqB.d);
+ expect(pqA.e == pqB.e);
+ expect(pqA.f == pqB.f);
+
+ // PQ functions invert to the same form.
+ expect(pqA.g == invA.g);
+}
+
+static void test_HLG_invert() {
+ skcms_TransferFunction hlgA, invA, invB;
+
+ expect(skcms_TransferFunction_makeHLG (&hlgA) );
+ expect(skcms_TransferFunction_makeHLGinv(&invA));
+ expect(skcms_TransferFunction_invert(&hlgA, &invB));
+
+ // Like PQ above, some of these values are expected
+ // to be exact, and some of them we're just getting
+ // lucky with that they happen to round trip exactly.
+
+ expect(invA.g == invB.g); // Is this still HLGinvish?
+ expect(invA.a == invB.a); // Lucky.
+ expect(invA.b == invB.b); // Lucky.
+ expect(invA.c == invB.c); // Lucky.
+ expect(invA.d == invB.d); // Exact.
+ expect(invA.e == invB.e); // Exact.
+ expect(invA.f == invB.f); // Exact (zero).
+
+ // ... and invert back to HLG.
+ // This tests a slightly different code path (really very similar).
+ skcms_TransferFunction hlgB;
+ expect(skcms_TransferFunction_invert(&invB, &hlgB));
+
+ expect(hlgA.g == hlgB.g);
+ expect(hlgA.a == hlgB.a);
+ expect(hlgA.b == hlgB.b);
+ expect(hlgA.c == hlgB.c);
+ expect(hlgA.d == hlgB.d);
+ expect(hlgA.e == hlgB.e);
+ expect(hlgA.f == hlgB.f);
+
+ // HLG functions invert between two different forms.
+ expect(hlgA.g != invA.g);
+}
+
int main(int argc, char** argv) {
bool regenTestData = false;
for (int i = 1; i < argc; ++i) {
@@ -1494,6 +1567,8 @@
test_Premul();
test_PQ();
test_HLG();
+ test_PQ_invert();
+ test_HLG_invert();
// Temporarily disable some tests while getting FP16 compute working.
if (!kFP16) {
