ash: HiDPI user avatar for SessionController

ui/gfx/image/mojo/image_skia_struct_traits.cc

Index: ui/gfx/image/mojo/image_skia_struct_traits.cc
diff --git a/ui/gfx/image/mojo/image_skia_struct_traits.cc b/ui/gfx/image/mojo/image_skia_struct_traits.cc
new file mode 100644
index 0000000000000000000000000000000000000000..6fd1dc70eebae1fb4d7d398a18a91ece238a534e
--- /dev/null
+++ b/ui/gfx/image/mojo/image_skia_struct_traits.cc
@@ -0,0 +1,140 @@
+// Copyright 2017 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/image/mojo/image_skia_struct_traits.h"
+
+#include <string.h>
+
+#include "base/logging.h"
+#include "skia/public/interfaces/bitmap_skbitmap_struct_traits.h"
+
+namespace mojo {
+
+StructTraits<gfx::mojom::ImageSkiaRepDataView,
+             gfx::ImageSkiaRep>::Context::Context() = default;
+StructTraits<gfx::mojom::ImageSkiaRepDataView,
+             gfx::ImageSkiaRep>::Context::~Context() = default;
+
+// static
+void* StructTraits<gfx::mojom::ImageSkiaRepDataView,
+                   gfx::ImageSkiaRep>::SetUpContext(const gfx::ImageSkiaRep&
+                                                        input) {
+  const std::vector<uint8_t> serialized_sk_bitmap(
+      skia::mojom::Bitmap::Serialize(&(input.sk_bitmap())));

[msw, 2017/03/23 20:47:51]

q: Do we need to make read only or make thread safe, etc. before this?

[xiyuan, 2017/03/24 05:45:20]

We dont' need to. A SkBitmap in ImageSkiaRep is immutable. [1]

[1]:
https://cs.chromium.org/chromium/src/ui/gfx/image/image_skia_rep.cc?rcl=1597c...

+
+  // Use a context to serialize bitmap to a shared buffer only once.

[msw, 2017/03/23 20:47:51]

nit: "the bitmap"

[xiyuan, 2017/03/24 05:45:20]

Done.

+  Context* context = new Context;
+  context->buffer_byte_size = serialized_sk_bitmap.size();
+  context->shared_buffer_handle =
+      mojo::SharedBufferHandle::Create(context->buffer_byte_size);
+  mojo::ScopedSharedBufferMapping mapping =
+      context->shared_buffer_handle->Map(context->buffer_byte_size);
+  memcpy(mapping.get(), serialized_sk_bitmap.data(), context->buffer_byte_size);

[msw, 2017/03/23 20:47:51]

hmm, I would have thought we could avoid a memcpy by somehow mapping the buffer
to the existing SkBitmap's underlying data, but perhaps that's not possible. In
any case, we can work to improve performance later, having a working mapping at
all is a great step forward.

[xiyuan, 2017/03/24 05:45:20]

I would love to do that as well but not sure how feasible it is either.

+  return context;
+}
+
+// static
+void StructTraits<gfx::mojom::ImageSkiaRepDataView, gfx::ImageSkiaRep>::
+    TearDownContext(const gfx::ImageSkiaRep& input, void* context) {
+  delete static_cast<Context*>(context);
+}
+
+// static
+mojo::ScopedSharedBufferHandle StructTraits<
+    gfx::mojom::ImageSkiaRepDataView,
+    gfx::ImageSkiaRep>::shared_buffer_handle(const gfx::ImageSkiaRep& input,
+                                             void* context) {
+  return (static_cast<Context*>(context))
+      ->shared_buffer_handle->Clone(
+          mojo::SharedBufferHandle::AccessMode::READ_ONLY);
+}
+
+// static
+uint32_t StructTraits<gfx::mojom::ImageSkiaRepDataView, gfx::ImageSkiaRep>::
+    buffer_byte_size(const gfx::ImageSkiaRep& input, void* context) {
+  return (static_cast<Context*>(context))->buffer_byte_size;
+}
+
+// static
+bool StructTraits<gfx::mojom::ImageSkiaRepDataView, gfx::ImageSkiaRep>::Read(
+    gfx::mojom::ImageSkiaRepDataView data,
+    gfx::ImageSkiaRep* out) {
+  mojo::ScopedSharedBufferHandle shared_buffer_handle =
+      data.TakeSharedBufferHandle();
+  if (!shared_buffer_handle.is_valid())
+    return false;
+
+  mojo::ScopedSharedBufferMapping mapping =
+      shared_buffer_handle->Map(data.buffer_byte_size());
+  if (!mapping)
+    return false;
+
+  const std::vector<uint8_t> serialized_sk_bitmap(
+      static_cast<uint8_t*>(mapping.get()),
+      static_cast<uint8_t*>(mapping.get()) + data.buffer_byte_size());
+
+  SkBitmap bitmap;
+  if (!skia::mojom::Bitmap::Deserialize(serialized_sk_bitmap, &bitmap))

[msw, 2017/03/23 20:47:52]

Again, I'm wondering if there's a more performance-focused way to do this, but
that can probably be addressed later; this is likely good enough for now.

[xiyuan, 2017/03/24 05:45:20]

Yes, I am not happy about this too. :(

Right now, we copy the pixels *4* times for the transport: serialize to a
vector, copy to shared buffer, copy from shared buffer to a vector, deserialize.
We can get rid of two (to and from vector) when merging into bitmap struct
traits. Will do that in a follow up.

+    return false;
+
+  *out = gfx::ImageSkiaRep(bitmap, data.scale());
+  return true;
+}
+
+// static
+void* StructTraits<gfx::mojom::ImageSkiaDataView, gfx::ImageSkia>::SetUpContext(
+    const gfx::ImageSkia& input) {
+  // Use a context to return a stable list of ImageSkiaRep objects. That is,
+  // multiple calls of image_reps() should return exactly the same list of
+  // ImageSkiaRep objects. So that ImageSkiaRep with the same backing pixel
+  // buffer is properly serialized and only once.
+  auto* image_reps = new std::vector<gfx::ImageSkiaRep>;
+
+  // Strip out empty image reps. Needed because ImageSkia::image_reps does not
+  // filter out empty SkBitmap.
+  for (auto& input_rep : input.image_reps()) {
+    if (!StructTraits<gfx::mojom::ImageSkiaRepDataView,
+                      gfx::ImageSkiaRep>::IsNull(input_rep)) {
+      image_reps->emplace_back(input_rep);
+    }
+  }
+  return image_reps;
+}
+
+// static
+void StructTraits<gfx::mojom::ImageSkiaDataView,
+                  gfx::ImageSkia>::TearDownContext(const gfx::ImageSkia& input,
+                                                   void* context) {
+  delete static_cast<std::vector<gfx::ImageSkiaRep>*>(context);
+}
+
+// static
+const std::vector<gfx::ImageSkiaRep>&
+StructTraits<gfx::mojom::ImageSkiaDataView, gfx::ImageSkia>::image_reps(
+    const gfx::ImageSkia& input,
+    void* context) {
+  return *(static_cast<std::vector<gfx::ImageSkiaRep>*>(context));

[msw, 2017/03/23 20:47:52]

optional nit: // See the comment in SetUpContext regarding context usage.

[xiyuan, 2017/03/24 05:45:20]

Done.

+}
+
+// static
+bool StructTraits<gfx::mojom::ImageSkiaDataView, gfx::ImageSkia>::Read(
+    gfx::mojom::ImageSkiaDataView data,
+    gfx::ImageSkia* out) {
+  DCHECK(out->isNull());
+
+  std::vector<gfx::ImageSkiaRep> image_reps;
+  if (!data.ReadImageReps(&image_reps))
+    return false;
+
+  for (const auto& image_rep : image_reps)
+    out->AddRepresentation(image_rep);
+
+  if (out->isNull())
+    return false;
+
+  out->SetReadOnly();
+  return true;
+}
+
+}  // namespace mojo