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)

[scroggo, 2016/06/02 18:51:50]

nit: My general preference would be to have a separate flag for QCMS, which gets
set to false/undefined when GOOGLE3 is defined.

[msarett, 2016/06/02 19:34:39]

I'll follow up with this change - so I can fix it here and elsewhere.

+#include "qcms.h"
+#endif
+
+#if !defined(GOOGLE3)
+DEFINE_bool(qcms, false, "Bench qcms color conversion");
+#endif
+DEFINE_bool(xform_only, false, "Only bench the color xform, do not include the decode time");

[scroggo, 2016/06/02 18:51:50]

nit: "bench" -> "time"

[msarett, 2016/06/02 19:34:39]

Done.

+
+ColorCodecBench::ColorCodecBench(const char* name, sk_sp<SkData> encoded)
+    : fEncoded(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,
+                             SkData* dstProfile, SkData* srcProfile) {
+    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) {

[scroggo, 2016/06/02 18:51:50]

Should this be if (!srcSpace)?

[msarett, 2016/06/02 19:34:39]

Yes!  Good catch.

+        srcSpace = SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named);
+    }
+    sk_sp<SkColorSpace> dstSpace = SkColorSpace::NewICC(dstProfile->data(), dstProfile->size());

[scroggo, 2016/06/02 18:51:50]

Do you want this to be part of the timing? Otherwise you could create this once
outside the loop. (I assume in practice it doesn't need to be done for every
decode?)

[msarett, 2016/06/02 19:34:39]

Good point, let's create this outside the loop.

+    SkASSERT(dstSpace);
+    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);
+        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,
+                                  SkData* dstProfile, SkData* srcProfile) {
+    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(srcProfile->data(), srcProfile->size()));
+    SkASSERT(srcSpace);
+
+    SkAutoTCallVProc<qcms_profile, qcms_profile_release>
+            dstSpace(qcms_profile_from_memory(dstProfile->data(), dstProfile->size()));
+    SkASSERT(dstSpace);
+    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,

[scroggo, 2016/06/02 18:51:49]

nit: Maybe comment out srcRow, since you're not using it?

[msarett, 2016/06/02 19:34:39]

Done.

+                       SkData* dstProfile, SkData* srcProfile) {
+    sk_sp<SkColorSpace> srcSpace = SkColorSpace::NewICC(srcProfile->data(), srcProfile->size());
+    if (srcSpace) {

[scroggo, 2016/06/02 18:51:50]

!srcSpace

[msarett, 2016/06/02 19:34:39]

Done.

+        srcSpace = SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named);
+    }
+    sk_sp<SkColorSpace> dstSpace = SkColorSpace::NewICC(dstProfile->data(), dstProfile->size());
+    SkASSERT(dstSpace);
+    std::unique_ptr<SkColorSpaceXform> xform = SkColorSpaceXform::New(srcSpace, dstSpace);
+    SkASSERT(xform);
+
+    for (int y = 0; y < info.height(); y++) {
+        // Transform in place

[scroggo, 2016/06/02 18:51:50]

Why is this in place, but not some others?

[msarett, 2016/06/02 19:34:39]

I decided to time the "xform only" benches in place.  I think this makes more
sense than keeping an entire src buffer and dst buffer.  I'm not sure if the
extra memory pressure would impact the timing (probably not), but it doesn't
match Chrome's use case (where they only have a single dst buffer).

When we time the decode and the xform together, we can do exactly what chrome
does.  First decode into a row buffer, then xform to the dst buffer.

If this is too strange, I think we can make both src->dst without worrying too
much about it changing anything.

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

Looking at this a little harder, won't the second iteration transform the output
from the first? And the third one will transform the output from the second?
Don't you want to transform the same/original data each time?

FWIW, I don't think you ever need an entire dst buffer - you only need one row,
right? We're never going to look at the output of this test, so can you just
write over it? Or might the transformation do some blending?

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

You're right, I didn't think past the first loop iteration...  I'll decode into
a srcBuffer and then xform into the dstBuffer.
You're right.  I think I'll keep the dstBuffer though, just to match the actual
use case as close as we can.

+        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,
+                            SkData* dstProfile, SkData* srcProfile) {
+    SkAutoTCallVProc<qcms_profile, qcms_profile_release>
+            srcSpace(qcms_profile_from_memory(srcProfile->data(), srcProfile->size()));
+    SkASSERT(srcSpace);
+
+    SkAutoTCallVProc<qcms_profile, qcms_profile_release>
+            dstSpace(qcms_profile_from_memory(dstProfile->data(), dstProfile->size()));
+    SkASSERT(dstSpace);
+    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* canvas) {

[scroggo, 2016/06/02 18:51:50]

nit: no need for the name "canvas" here, which is unused.

[msarett, 2016/06/02 19:34:39]

Done.

+    for (int i = 0; i < n; i++) {
+        fProc(fEncoded.get(), fDst.get(), fSrcRow.get(), fInfo, fDstProfile.get(),
+              fSrcProfile.get());
+    }
+}