commit da25e1a6d4122e89a93155969ec7ad648cfadbd0
author Mike Klein <mtklein@google.com> Wed Aug 29 10:20:23 2018 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Wed Aug 29 14:36:20 2018 +0000
tree 0fb4b68d01777806ad9aaee9cfab4f7e9e68ace7
parent 5f03dc9abfb1d5b0f5598a9cb551ab478bbe0945

add skcms_ApproximatelyEqualProfiles() fastpath

We're not checking exact equality, and doing so wins back a lot of the
performance lost when switching SkCodec to use skcms.

I've noted a few other places that look ripe for improvement.

Bug: skia:8278
Change-Id: Idcd17736b3928dda47ecd5e65c8ce05718974b14
Reviewed-on: https://skia-review.googlesource.com/150243
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>

skcms.cc

diff --git a/skcms.cc b/skcms.cc
index f6f7158..941dae6 100644
--- a/skcms.cc
+++ b/skcms.cc
@@ -1124,6 +1124,11 @@
 };
 
 bool skcms_ApproximatelyEqualProfiles(const skcms_ICCProfile* A, const skcms_ICCProfile* B) {
+    // Test for exactly equal profiles first.
+    if (A == B || 0 == memcmp(A,B, sizeof(skcms_ICCProfile))) {
+        return true;
+    }
+
     // For now this is the essentially the same strategy we use in test_only.c
     // for our skcms_Transform() smoke tests:
     //    1) transform A to XYZD50
@@ -1131,7 +1136,7 @@
     //    3) return true if they're similar enough
     // Our current criterion in 3) is maximum 1 bit error per XYZD50 byte.
 
-    // Here are 252 of a random shuffle of all possible bytes.
+    // skcms_252_random_bytes are 252 of a random shuffle of all possible bytes.
     // 252 is evenly divisible by 3 and 4.  Only 192, 10, 241, and 43 are missing.
 
     if (A->data_color_space != B->data_color_space) {
@@ -1139,6 +1144,7 @@
     }
 
     // Interpret as RGB_888 if data color space is RGB or GRAY, RGBA_8888 if CMYK.
+    // TODO: working with RGBA_8888 either way is probably fastest.
     skcms_PixelFormat fmt = skcms_PixelFormat_RGB_888;
     size_t npixels = 84;
     if (A->data_color_space == skcms_Signature_CMYK) {
@@ -1146,6 +1152,8 @@
         npixels = 63;
     }
 
+    // TODO: if A or B is a known profile (skcms_sRGB_profile, skcms_XYZD50_profile),
+    // use pre-canned results and skip that skcms_Transform() call?
     uint8_t dstA[252],
             dstB[252];
     if (!skcms_Transform(
@@ -1161,6 +1169,7 @@
         return false;
     }
 
+    // TODO: make sure this final check has reasonable codegen.
     for (size_t i = 0; i < 252; i++) {
         if (abs((int)dstA[i] - (int)dstB[i]) > 1) {
             return false;

tests.c

diff --git a/tests.c b/tests.c
index 4435dea..4706925 100644
--- a/tests.c
+++ b/tests.c
@@ -1093,6 +1093,16 @@
     expect(skcms_ApproximatelyEqualProfiles(&p, &srgb));
 }
 
+static void test_ExactlyEqual() {
+    const skcms_ICCProfile* srgb = skcms_sRGB_profile();
+    skcms_ICCProfile        copy = *srgb;
+
+    expect(skcms_ApproximatelyEqualProfiles( srgb,  srgb));
+    expect(skcms_ApproximatelyEqualProfiles( srgb, &copy));
+    expect(skcms_ApproximatelyEqualProfiles(&copy,  srgb));
+    expect(skcms_ApproximatelyEqualProfiles(&copy, &copy));
+}
+
 static void test_Clamp() {
     // Test that we clamp out-of-gamut values when converting to fixed point,
     // not just to byte value range but also to gamut (for compatibility with
@@ -1162,6 +1172,7 @@
     test_MakeUsableAsDestinationAdobe();
     test_PrimariesToXYZ();
     test_Programmatic_sRGB();
+    test_ExactlyEqual();
     test_Clamp();
 #if 0
     test_CLUT();