Add color correction benchmark - with comparison to qcms

bench/ColorCodecBench.cpp

+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "ColorCodecBench.h"
+#include "Resources.h"
+#include "SkCodec.h"
+#include "SkColorSpaceXform.h"
+#include "SkCommandLineFlags.h"
+
+#if !defined(GOOGLE3)
+#include "qcms.h"
+#endif
+
+#if !defined(GOOGLE3)
+DEFINE_bool(qcms, false, "Bench qcms color conversion");
+#endif
+DEFINE_bool(xform_only, false, "Only time the color xform, do not include the decode time");
+
+ColorCodecBench::ColorCodecBench(const char* name, sk_sp<SkData> encoded)
+    : fEncoded(std::move(encoded))
+{
+    fName.appendf("Color%s", FLAGS_xform_only ? "Xform" : "Codec");
+#if !defined(GOOGLE3)
+    fName.appendf("%s", FLAGS_qcms ? "QCMS" : "");
+#endif
+    fName.appendf("_%s", name);
+}
+
+const char* ColorCodecBench::onGetName() {
+    return fName.c_str();
+}
+
+bool ColorCodecBench::isSuitableFor(Backend backend) {
+    return kNonRendering_Backend == backend;
+}
+
+static void decode_and_xform(SkData* encoded, void* dst, void* srcRow, const SkImageInfo& info,
+                             void* dstProfile, SkData* /*srcData*/) {
+    SkAutoTDelete<SkCodec> codec(SkCodec::NewFromData(encoded));
+#ifdef SK_DEBUG
+    const SkCodec::Result result =
+#endif
+    codec->startScanlineDecode(info);
+    SkASSERT(SkCodec::kSuccess == result);
+
+    sk_sp<SkColorSpace> srcSpace = sk_ref_sp(codec->getColorSpace());
+    if (!srcSpace) {
+        srcSpace = SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named);
+    }
+    sk_sp<SkColorSpace> dstSpace = sk_ref_sp((SkColorSpace*) dstProfile);
+    std::unique_ptr<SkColorSpaceXform> xform = SkColorSpaceXform::New(srcSpace, dstSpace);
+    SkASSERT(xform);
+
+    for (int y = 0; y < info.height(); y++) {
+#ifdef SK_DEBUG
+        const int rows =
+#endif
+        codec->getScanlines(srcRow, 1, 0);

[scroggo, 2016/06/02 20:42:31]

This will be awfully slow on interlaced PNG, and the output will be upside
down/scrambled for bottom-up BMP/interlaced GIF. The latter may be fine for this
benchmark, since you're not looking at the output, but it does make me think
about how color transforms will interact with incrementalDecode...

One nice thing about incrementalDecode is that the client doesn't need to care
about the SkScanlineOrder - the codec knows the whole block of destination
memory and writes to it in the right place.

But if the client doesn't know what lines were written, they won't know which
lines to transform.

Some possible solutions:
- do the transformation on every row
  - inefficient when the image is partially decoded, since we'll transform
     each row on each pass
- make incrementalDecode() somehow report the rows it decoded into
  - also helps with the filling problem
  - but I don't yet know what that API would look like
- pass the destination space to the codec, so it can do the transformation as it
  decodes. (This is more or less what Chrome does now, right?)
  - this seems the cleanest to me, but I thought we had reasons not to do it
    that way?
    - one is that the same image may be drawn to different destination spaces
      - e.g. two different monitors with different profiles

Generally it's not clear to me when the transformation will be applied, and
whether or not it will be in place. Maybe I'm missing something that makes this
simpler...?

[msarett, 2016/06/02 22:17:47]

"pass the destination space to the codec, so it can do the transformation as it
decodes. (This is more or less what Chrome does now, right?)"

Yes I think this needs to happen inside the codec, the way Chrome does it right
now.  We already have a way to pass the destination space to the codec (since
there is an SkColorSpace on SkImageInfo).

Maybe I'm wrong to write benchmarks before it's integrated with our codecs. 
Because maybe we'll need new ones after...  But I'm actually thinking about
integrating with Chrome's codecs first.  I think this is a good way to measure
the impact that we'll have there (starting with jpeg in particular).

I'm really far away from thinking about BMP and GIF, I don't think anybody color
corrects those anyway.

Interlaced PNG/JPEG is an interesting thought.  It looks Chrome will redo the
correction every time a row is updated.

-------------------------------------------

I also think the color xtransform logic needs to exist outside of the codecs,
for other uses.  Which I guess is why it's floating around in src/core right
now.

[scroggo, 2016/06/03 14:25:36]

Whew! That makes the most sense to me, with the caveat of I don't know what to
do if the same image is used on multiple screens with different properties.
That's probably an uncommon use case, though (and we don't handle it today). The
simplest approach will be to decode twice, which is not perfect, but maybe it's
okay.
Of course! I had been trying to wrap my head around putting the color space in
the image info, but I think this demonstrates why it makes sense.
No, I think it does make sense to write these benchmarks now. Even if it changes
later you can find out how to improve the current code, which will likely still
apply.
I think that's the right approach. SkCodec still needs to finish incremental
decoding and to support animation. And there's still some Chromium plumbing work
to be done.
Probably not, although we'll want to do it eventually.
sgtm

+        SkASSERT(1 == rows);
+
+        xform->xform_RGBA_8888((uint32_t*) dst, (uint32_t*) srcRow, info.width());
+        dst = SkTAddOffset<void>(dst, info.minRowBytes());
+    }
+}
+
+#if !defined(GOOGLE3)
+static void decode_and_xform_qcms(SkData* encoded, void* dst, void* srcRow, const SkImageInfo& info,
+                                  void* dstProfile, SkData* srcData) {
+    SkAutoTDelete<SkCodec> codec(SkCodec::NewFromData(encoded));
+#ifdef SK_DEBUG
+    const SkCodec::Result result =
+#endif
+    codec->startScanlineDecode(info);
+    SkASSERT(SkCodec::kSuccess == result);
+
+    SkAutoTCallVProc<qcms_profile, qcms_profile_release>
+            srcSpace(qcms_profile_from_memory(srcData->data(), srcData->size()));
+    SkASSERT(srcSpace);
+
+    qcms_profile* dstSpace = (qcms_profile*) dstProfile;
+    SkAutoTCallVProc<qcms_transform, qcms_transform_release>
+            transform (qcms_transform_create(srcSpace, QCMS_DATA_RGBA_8, dstSpace,
+                                             QCMS_DATA_RGBA_8, QCMS_INTENT_PERCEPTUAL));
+    SkASSERT(transform);
+
+#ifdef SK_PMCOLOR_IS_RGBA
+    qcms_output_type outType = QCMS_OUTPUT_RGBX;
+#else
+    qcms_output_type outType = QCMS_OUTPUT_BGRX;
+#endif
+
+    for (int y = 0; y < info.height(); y++) {
+#ifdef SK_DEBUG
+        const int rows =
+#endif
+        codec->getScanlines(srcRow, 1, 0);
+        SkASSERT(1 == rows);
+
+        qcms_transform_data_type(transform, srcRow, dst, info.width(), outType);
+        dst = SkTAddOffset<void>(dst, info.minRowBytes());
+    }
+}
+#endif
+
+static void xform_only(SkData* encoded, void* dst, void* /*srcRow*/, const SkImageInfo& info,
+                       void* dstProfile, SkData* srcData) {
+    sk_sp<SkColorSpace> srcSpace = SkColorSpace::NewICC(srcData->data(), srcData->size());
+    if (!srcSpace) {
+        srcSpace = SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named);
+    }
+    sk_sp<SkColorSpace> dstSpace = sk_ref_sp((SkColorSpace*) dstProfile);
+    std::unique_ptr<SkColorSpaceXform> xform = SkColorSpaceXform::New(srcSpace, dstSpace);
+    SkASSERT(xform);
+
+    for (int y = 0; y < info.height(); y++) {
+        // Transform in place
+        xform->xform_RGBA_8888((uint32_t*) dst, (uint32_t*) dst, info.width());
+        dst = SkTAddOffset<void>(dst, info.minRowBytes());
+    }
+}
+
+#if !defined(GOOGLE3)
+static void xform_only_qcms(SkData* encoded, void* dst, void* /*srcRow*/, const SkImageInfo& info,
+                            void* dstProfile, SkData* srcData) {
+    SkAutoTCallVProc<qcms_profile, qcms_profile_release>
+            srcSpace(qcms_profile_from_memory(srcData->data(), srcData->size()));
+    SkASSERT(srcSpace);
+
+    qcms_profile* dstSpace = (qcms_profile*) dstProfile;
+    SkAutoTCallVProc<qcms_transform, qcms_transform_release>
+            transform (qcms_transform_create(srcSpace, QCMS_DATA_RGBA_8, dstSpace,
+                                             QCMS_DATA_RGBA_8, QCMS_INTENT_PERCEPTUAL));
+    SkASSERT(transform);
+
+#ifdef SK_PMCOLOR_IS_RGBA
+    qcms_output_type outType = QCMS_OUTPUT_RGBX;
+#else
+    qcms_output_type outType = QCMS_OUTPUT_BGRX;
+#endif
+
+    for (int y = 0; y < info.height(); y++) {
+        // Transform in place
+        qcms_transform_data_type(transform, dst, dst, info.width(), outType);
+        dst = SkTAddOffset<void>(dst, info.minRowBytes());
+    }
+}
+#endif
+
+void ColorCodecBench::onDelayedSetup() {
+    SkAutoTDelete<SkCodec> codec(SkCodec::NewFromData(fEncoded.get()));
+    fInfo = codec->getInfo().makeColorType(kRGBA_8888_SkColorType);
+
+    fDst.reset(fInfo.getSafeSize(fInfo.minRowBytes()));
+    if (FLAGS_xform_only) {
+        codec->getPixels(fInfo, fDst.get(), fInfo.minRowBytes());
+    } else {
+        // Set-up a row buffer to decode into before transforming to dst.
+        fSrcRow.reset(fInfo.minRowBytes());
+    }
+
+    fDstProfile = SkData::MakeFromFileName(
+            GetResourcePath("monitor_profiles/HP_ZR30w.icc").c_str());
+    SkASSERT(fDstProfile);
+    fSrcProfile = codec->getICCData();
+
+#if !defined(GOOGLE3)
+    if (FLAGS_qcms) {
+        fProc = FLAGS_xform_only ? xform_only_qcms : decode_and_xform_qcms;
+    } else
+#endif
+    {
+        fProc = FLAGS_xform_only ? xform_only : decode_and_xform;
+    }
+}
+
+void ColorCodecBench::onDraw(int n, SkCanvas*) {
+    // Create the dst profile outside of the main loop.
+    void* dstProfile;
+    sk_sp<SkColorSpace> dstSpace = nullptr;

[scroggo, 2016/06/02 20:42:31]

Why not do all this setup in onDelayedSetup? If nanobench decides it only needs
one loop to time this benchmark, this will still be included in the time.

[msarett, 2016/06/02 22:17:47]

Even better.  Done.

+#if !defined(GOOGLE3)
+    SkAutoTCallVProc<qcms_profile, qcms_profile_release> dstProfileDeleter(nullptr);
+    if (FLAGS_qcms) {
+        dstProfileDeleter.reset(qcms_profile_from_memory(fDstProfile->data(), fDstProfile->size()));
+        dstProfile = dstProfileDeleter.get();
+        SkASSERT(dstProfile);
+    } else
+#endif
+    {
+        dstSpace = SkColorSpace::NewICC(fDstProfile->data(), fDstProfile->size());
+        dstProfile = (void*) dstSpace.get();
+    }
+
+    for (int i = 0; i < n; i++) {
+        fProc(fEncoded.get(), fDst.get(), fSrcRow.get(), fInfo, dstProfile,

[scroggo, 2016/06/02 20:42:31]

What if you made fProc call a member method? Then you wouldn't need to pass 6
parameters, and you wouldn't need to be cast dstProfile. OTOH, then fProc could
mess with all the member variables in ways you do not intend.

[msarett, 2016/06/02 22:17:47]

I've made this change, and I think it makes things a lot cleaner :)

+              fSrcProfile.get());
+    }
+}