PPAPI/NaCl: Make ImageData for NaCl just use shared memory

webkit/plugins/ppapi/ppb_image_data_impl.cc

Index: webkit/plugins/ppapi/ppb_image_data_impl.cc
diff --git a/webkit/plugins/ppapi/ppb_image_data_impl.cc b/webkit/plugins/ppapi/ppb_image_data_impl.cc
index ea5f2c663d540b299033f9916a06dc2e469d907f..e268f2aaedc67bb4071cbdd6d9ed8cc6386e2a92 100644
--- a/webkit/plugins/ppapi/ppb_image_data_impl.cc
+++ b/webkit/plugins/ppapi/ppb_image_data_impl.cc
@@ -25,36 +25,31 @@ using ::ppapi::thunk::PPB_ImageData_API;
 namespace webkit {
 namespace ppapi {
 
-PPB_ImageData_Impl::PPB_ImageData_Impl(PP_Instance instance)
+PPB_ImageData_Impl::PPB_ImageData_Impl(PP_Instance instance,
+                                       ImageDataType type)
     : Resource(::ppapi::OBJECT_IS_IMPL, instance),
       format_(PP_IMAGEDATAFORMAT_BGRA_PREMUL),
       width_(0),
       height_(0) {
+  switch (type) {
+    case PLATFORM:
+      delegate_.reset(new ImageDataPlatformDelegate);
+      return;
+    case NACL:
+      delegate_.reset(new ImageDataNaClDelegate);
+      return;
+    // No default: so that we get a compiler warning if any types are added.
+  }
+  NOTREACHED();
 }
 
 PPB_ImageData_Impl::~PPB_ImageData_Impl() {
 }
 
-// static
-PP_Resource PPB_ImageData_Impl::Create(PP_Instance instance,
-                                       PP_ImageDataFormat format,
-                                       const PP_Size& size,
-                                       PP_Bool init_to_zero) {
-  scoped_refptr<PPB_ImageData_Impl> data(new PPB_ImageData_Impl(instance));
-  if (!data->Init(format, size.width, size.height, !!init_to_zero))
-    return 0;
-  return data->GetReference();
-}
-
-PPB_ImageData_API* PPB_ImageData_Impl::AsPPB_ImageData_API() {
-  return this;
-}
-
 bool PPB_ImageData_Impl::Init(PP_ImageDataFormat format,
                               int width, int height,
                               bool init_to_zero) {
   // TODO(brettw) this should be called only on the main thread!
-  // TODO(brettw) use init_to_zero when we implement caching.
   if (!IsImageDataFormatSupported(format))
     return false;  // Only support this one format for now.
   if (width <= 0 || height <= 0)
@@ -63,15 +58,46 @@ bool PPB_ImageData_Impl::Init(PP_ImageDataFormat format,
       std::numeric_limits<int32>::max())
     return false;  // Prevent overflow of signed 32-bit ints.
 
-  PluginDelegate* plugin_delegate = ResourceHelper::GetPluginDelegate(this);
-  if (!plugin_delegate)
-    return false;
-
-  platform_image_.reset(plugin_delegate->CreateImage2D(width, height));
   format_ = format;
   width_ = width;
   height_ = height;
-  return !!platform_image_.get();
+  return delegate_->Init(this, format, width, height, init_to_zero);
+}
+
+// static
+PP_Resource PPB_ImageData_Impl::CreatePlatform(PP_Instance instance,
+                                               PP_ImageDataFormat format,
+                                               const PP_Size& size,
+                                               PP_Bool init_to_zero) {
+  scoped_refptr<PPB_ImageData_Impl>
+      data(new PPB_ImageData_Impl(instance, PLATFORM));
+  if (!data->Init(format, size.width, size.height, !!init_to_zero))
+    return 0;
+  return data->GetReference();
+}
+
+// static
+PP_Resource PPB_ImageData_Impl::CreateNaCl(PP_Instance instance,
+                                               PP_ImageDataFormat format,
+                                               const PP_Size& size,
+                                               PP_Bool init_to_zero) {
+  scoped_refptr<PPB_ImageData_Impl>
+      data(new PPB_ImageData_Impl(instance, NACL));
+  if (!data->Init(format, size.width, size.height, !!init_to_zero))
+    return 0;
+  return data->GetReference();
+}
+
+PPB_ImageData_API* PPB_ImageData_Impl::AsPPB_ImageData_API() {
+  return this;
+}
+
+bool PPB_ImageData_Impl::IsMapped() const {
+  return delegate_->IsMapped();
+}
+
+PluginDelegate::PlatformImage2D* PPB_ImageData_Impl::PlatformImage() const {
+  return delegate_->PlatformImage();
 }
 
 PP_Bool PPB_ImageData_Impl::Describe(PP_ImageDataDesc* desc) {
@@ -83,6 +109,66 @@ PP_Bool PPB_ImageData_Impl::Describe(PP_ImageDataDesc* desc) {
 }
 
 void* PPB_ImageData_Impl::Map() {
+  return delegate_->Map();
+}
+
+void PPB_ImageData_Impl::Unmap() {
+  delegate_->Unmap();
+}
+
+int32_t PPB_ImageData_Impl::GetSharedMemory(int* handle, uint32_t* byte_count) {
+  return delegate_->GetSharedMemory(handle, byte_count);
+}
+
+skia::PlatformCanvas* PPB_ImageData_Impl::GetPlatformCanvas() {
+  return delegate_->GetPlatformCanvas();
+}
+
+SkCanvas* PPB_ImageData_Impl::GetCanvas() {
+  return delegate_->GetCanvas();
+}
+
+const SkBitmap* PPB_ImageData_Impl::GetMappedBitmap() const {
+  return delegate_->GetMappedBitmap();
+}
+
+void PPB_ImageData_Impl::Swap(PPB_ImageData_Impl* other) {
+  delegate_.swap(other->delegate_);
+  std::swap(other->format_, format_);
+  std::swap(other->width_, width_);
+  std::swap(other->height_, height_);
+}
+
+// ImageDataPlatformDelegate --------------------------------------------------

[brettw, 2012/07/20 21:57:04]

I'd do a blank line after this.

[dmichael (off chromium), 2012/07/20 22:32:40]

Done.

+ImageDataPlatformDelegate::ImageDataPlatformDelegate() {
+}
+
+ImageDataPlatformDelegate::~ImageDataPlatformDelegate() {
+}
+
+bool ImageDataPlatformDelegate::Init(PPB_ImageData_Impl* impl,
+                                     PP_ImageDataFormat format,
+                                     int width, int height,
+                                     bool init_to_zero) {
+  PluginDelegate* plugin_delegate = ResourceHelper::GetPluginDelegate(impl);
+  if (!plugin_delegate)
+    return false;
+
+  // TODO(brettw) use init_to_zero when we implement caching.
+  platform_image_.reset(plugin_delegate->CreateImage2D(width, height));
+  return !!platform_image_.get();
+}
+
+bool ImageDataPlatformDelegate::IsMapped() const {
+  return !!mapped_canvas_.get();
+}
+
+PluginDelegate::PlatformImage2D*
+ImageDataPlatformDelegate::PlatformImage() const {
+  return platform_image_.get();
+}
+
+void* ImageDataPlatformDelegate::Map() {
   if (!mapped_canvas_.get()) {
     mapped_canvas_.reset(platform_image_->Map());
     if (!mapped_canvas_.get())
@@ -98,35 +184,112 @@ void* PPB_ImageData_Impl::Map() {
   return bitmap.getAddr32(0, 0);
 }
 
-void PPB_ImageData_Impl::Unmap() {
+void ImageDataPlatformDelegate::Unmap() {
   // This is currently unimplemented, which is OK. The data will just always
   // be around once it's mapped. Chrome's TransportDIB isn't currently
   // unmappable without freeing it, but this may be something we want to support
   // in the future to save some memory.
 }
 
-int32_t PPB_ImageData_Impl::GetSharedMemory(int* handle,
-                                            uint32_t* byte_count) {
+int32_t ImageDataPlatformDelegate::GetSharedMemory(int* handle,
+                                                   uint32_t* byte_count) {
   *handle = platform_image_->GetSharedMemoryHandle(byte_count);
   return PP_OK;
 }
 
-skia::PlatformCanvas* PPB_ImageData_Impl::GetPlatformCanvas() {
+skia::PlatformCanvas* ImageDataPlatformDelegate::GetPlatformCanvas() {
   return mapped_canvas_.get();
 }
 
-const SkBitmap* PPB_ImageData_Impl::GetMappedBitmap() const {
+SkCanvas* ImageDataPlatformDelegate::GetCanvas() {
+  return mapped_canvas_.get();
+}
+
+const SkBitmap* ImageDataPlatformDelegate::GetMappedBitmap() const {
   if (!mapped_canvas_.get())
     return NULL;
   return &skia::GetTopDevice(*mapped_canvas_)->accessBitmap(false);
 }
 
-void PPB_ImageData_Impl::Swap(PPB_ImageData_Impl* other) {
-  swap(other->platform_image_, platform_image_);
-  swap(other->mapped_canvas_, mapped_canvas_);
-  std::swap(other->format_, format_);
-  std::swap(other->width_, width_);
-  std::swap(other->height_, height_);
+// ImageDataNaClDelegate ------------------------------------------------------
+ImageDataNaClDelegate::ImageDataNaClDelegate()
+    : map_count_(0) {
+}
+
+ImageDataNaClDelegate::~ImageDataNaClDelegate() {
+}
+
+bool ImageDataNaClDelegate::Init(PPB_ImageData_Impl* impl,
+                                 PP_ImageDataFormat format,
+                                 int width, int height,
+                                 bool init_to_zero) {
+  skia_bitmap_.setConfig(SkBitmap::kARGB_8888_Config,
+                         impl->width(), impl->height());
+  PluginDelegate* plugin_delegate = ResourceHelper::GetPluginDelegate(impl);
+  if (!plugin_delegate)
+    return false;
+  shared_memory_.reset(
+      plugin_delegate->CreateAnonymousSharedMemory(skia_bitmap_.getSize()));
+  return !!shared_memory_.get();
+}
+
+bool ImageDataNaClDelegate::IsMapped() const {
+  return map_count_ > 0;
+}
+
+PluginDelegate::PlatformImage2D* ImageDataNaClDelegate::PlatformImage() const {
+  return NULL;
+}
+
+void* ImageDataNaClDelegate::Map() {
+  DCHECK(shared_memory_.get());
+  if (map_count_++ == 0) {
+    shared_memory_->Map(skia_bitmap_.getSize());
+    skia_bitmap_.setPixels(shared_memory_->memory());
+    // Our platform bitmaps are set to opaque by default, which we don't want.
+    skia_bitmap_.setIsOpaque(false);
+    // TODO(dmichael): Is lockPixels necessary? We own the pixels:

[brettw, 2012/07/20 21:57:04]

I'm pretty sure it isn't necessary. I think assigning the pixels gives you an
implicit lock.

[dmichael (off chromium), 2012/07/20 22:32:40]

Okay, I'll take the lock and unlock out until we hear back from an expert.

By my reading, it's irrelevant; I think when you provide the backing memory (via
setPixels), that there's no SkPixelRef, so locking is a no-op. But I would love
to have someone confirm or deny :-)

+    skia_bitmap_.lockPixels();
+    skia_canvas_.setBitmapDevice(skia_bitmap_);
+    return skia_bitmap_.getAddr32(0, 0);
+  }
+  return shared_memory_->memory();
+}
+
+void ImageDataNaClDelegate::Unmap() {
+  if (--map_count_ == 0) {
+    shared_memory_->Unmap();
+    skia_bitmap_.unlockPixels();

[brettw, 2012/07/20 21:57:04]

I'm going to add Mike Reed to check if this is correct.

+  }
+}
+
+int32_t ImageDataNaClDelegate::GetSharedMemory(int* handle,
+                                                uint32_t* byte_count) {
+  *byte_count = skia_bitmap_.getSize();
+#if defined(OS_POSIX)
+  *handle = shared_memory_->handle().fd;
+#elif defined(OS_WIN)
+  *handle = reinterpret_cast<int>(shared_memory_->handle());
+#else
+#error "Platform not supported."
+#endif
+  return PP_OK;
+}
+
+skia::PlatformCanvas* ImageDataNaClDelegate::GetPlatformCanvas() {
+  return NULL;
+}
+
+SkCanvas* ImageDataNaClDelegate::GetCanvas() {
+  if (!IsMapped())
+    return NULL;
+  return &skia_canvas_;
+}
+
+const SkBitmap* ImageDataNaClDelegate::GetMappedBitmap() const {
+  if (!IsMapped())
+    return NULL;
+  return &skia_bitmap_;
 }
 
 }  // namespace ppapi