Implement SkEncodeImage()

ui/gfx/codec/install_skia_codec.cc

+// Copyright (c) 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/gfx/codec/install_skia_codec.h"
+
+#include "base/synchronization/lock.h"
+#include "skia/ext/skia_encode_image.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/skia/include/core/SkEncodedImageFormat.h"
+#include "third_party/skia/include/core/SkStream.h"
+#include "ui/gfx/codec/jpeg_codec.h"
+#include "ui/gfx/codec/png_codec.h"
+#include "ui/gfx/geometry/size.h"
+
+static bool Encode(SkWStream* dst,
+                   const SkPixmap& pixmap,
+                   SkEncodedImageFormat format,
+                   int quality) {
+  if (kN32_SkColorType != pixmap.colorType() ||
+      (kPremul_SkAlphaType != pixmap.alphaType() &&
+       kOpaque_SkAlphaType != pixmap.alphaType())) {

[scroggo, 2016/11/29 20:15:24]

Why not support unpremul? It looks like PNGCodec::Encode expects
premultiplication (it goes through this unpremultiply step [1]), but we *could*
support it. I guess none of your input data is unpremul? (Or is there a TODO?)

[1]
https://cs.chromium.org/chromium/src/ui/gfx/skia_util.cc?cl=GROK&gsn=ConvertS...

[hal.canary, 2016/11/29 20:27:04]

Fixing gfx::PNGCodec::Encode() to allow unpremuliplied input is outside of the
scope of what I am doing.

+    return false;
+  }
+  std::vector<unsigned char> buffer;
+  switch (format) {
+    case SkEncodedImageFormat::kPNG:
+      return gfx::PNGCodec::Encode(
+                 reinterpret_cast<const unsigned char*>(pixmap.addr()),
+                 gfx::PNGCodec::FORMAT_SkBitmap,
+                 gfx::Size(pixmap.width(), pixmap.height()),
+                 static_cast<int>(pixmap.rowBytes()), false,
+                 std::vector<gfx::PNGCodec::Comment>(), &buffer) &&
+             dst->write(&buffer[0], buffer.size());

[dcheng, 2016/11/29 23:37:43]

Is it guaranteed that |buffer.size()| > 0 here and below?

(It's not clear to me; it seems like the answer is probably "Yes" but it's also
not guaranteed)

[hal.canary, 2016/11/30 18:24:06]

Done.

[dcheng, 2016/12/01 06:22:38]

Rather than the conditional, I think it's probably better to just
write(buffer.data(), buffer.size()) - C++11 guarantee that data() will always
return *something*: it's just not dereferencable if the container is empty.

+    case SkEncodedImageFormat::kJPEG:
+      return gfx::JPEGCodec::Encode(
+                 reinterpret_cast<const unsigned char*>(pixmap.addr()),
+                 gfx::JPEGCodec::FORMAT_SkBitmap, pixmap.width(),
+                 pixmap.height(), static_cast<int>(pixmap.rowBytes()), quality,
+                 &buffer) &&
+             dst->write(&buffer[0], buffer.size());
+    default:
+      return false;
+  }
+}
+
+void gfx::InstallCodecsIntoSkia() {
+  // FIXME:  What is the chromium version of SkOnce?

[dcheng, 2016/11/29 23:37:43]

There is no Chromium version: the base::Lock here doesn't do anything, since
it's a stack variable.

How is the GPU benchmarking extension installed? Is this something we expect to
be more widely used in the future? Usually, writing code like this in Chromium
would be a bit of an anti-pattern, since initialization is usually supposed to
be better coordinated.

[hal.canary, 2016/11/30 18:24:06]

d'oh!  That's what I meant to do.
I am using the benchmarking extension to test printing to XPS. In the future, we
will need to both initialize image encoding and set up SkXPS to work inside the
sandbox when the printing process is started.

+  base::Lock lock;
+  base::AutoLock autoLock(lock);
+  static bool g_done = false;
+  if (!g_done) {
+    skia::SetImageEncoder(&Encode);
+    g_done = true;
+  }
+}