ozone: unify GpuMemoryBufferFactoryOzoneNativePixmap in content/

ui/gl/gl_image_ozone_native_pixmap.cc

+// Copyright 2015 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/gl/gl_image_ozone_native_pixmap.h"
+
+#include "ui/gl/gl_image_linux_dma_buffer.h"
+
+namespace gfx {
+namespace {
+
+class GLImageOzoneNativePixmap : public GLImageEGL {
+ public:
+  explicit GLImageOzoneNativePixmap(const Size& size) : GLImageEGL(size) {}
+
+  void Destroy(bool have_context) override {
+    GLImageEGL::Destroy(have_context);
+    pixmap_ = nullptr;
+  }
+
+  bool Initialize(ui::NativePixmap* pixmap) {

[piman, 2015/07/24 18:41:25]

Can you avoid overloading? It makes the code confusing, and is discouraged by
the style guide

My suggestion is to provide a factory function instead:
static scoped_ptr<GLImageOzoneNativePixmap> Create(const Size& size,
ui::NativePixmap* pixmap), which does both the allocation and initialization.

[dshwang, 2015/07/24 19:57:20]

Good suggestion! Done.

+    EGLint attrs[] = {EGL_IMAGE_PRESERVED_KHR, EGL_TRUE, EGL_NONE};
+    if (!Initialize(EGL_NATIVE_PIXMAP_KHR, pixmap->GetEGLClientBuffer(), attrs))
+      return false;
+    pixmap_ = pixmap;
+    return true;
+  }
+
+  bool ScheduleOverlayPlane(AcceleratedWidget widget,
+                            int z_order,
+                            OverlayTransform transform,
+                            const Rect& bounds_rect,
+                            const RectF& crop_rect) override {
+    return pixmap_ &&
+           pixmap_->ScheduleOverlayPlane(widget, z_order, transform,
+                                         bounds_rect, crop_rect);
+  }
+
+ protected:
+  ~GLImageOzoneNativePixmap() override {}
+
+ private:
+  using GLImageEGL::Initialize;
+  scoped_refptr<ui::NativePixmap> pixmap_;
+};
+
+class GLImageOzoneNativePixmapDmaBuf : public GLImageLinuxDMABuffer {
+ public:
+  GLImageOzoneNativePixmapDmaBuf(const Size& size, unsigned internalformat)
+      : GLImageLinuxDMABuffer(size, internalformat) {}
+
+  void Destroy(bool have_context) override {
+    GLImageLinuxDMABuffer::Destroy(have_context);
+    pixmap_ = nullptr;
+  }
+
+  bool Initialize(ui::NativePixmap* pixmap, GpuMemoryBuffer::Format format) {

[piman, 2015/07/24 18:41:25]

Same here, no overloading please.

[dshwang, 2015/07/24 19:57:20]

Done.

+    base::FileDescriptor handle(pixmap->GetDmaBufFd(), false);
+    if (!GLImageLinuxDMABuffer::Initialize(handle, format,
+                                           pixmap->GetDmaBufPitch()))
+      return false;
+    pixmap_ = pixmap;
+    return true;
+  }
+
+  bool ScheduleOverlayPlane(AcceleratedWidget widget,
+                            int z_order,
+                            OverlayTransform transform,
+                            const Rect& bounds_rect,
+                            const RectF& crop_rect) override {
+    return pixmap_ &&
+           pixmap_->ScheduleOverlayPlane(widget, z_order, transform,
+                                         bounds_rect, crop_rect);
+  }
+
+ protected:
+  ~GLImageOzoneNativePixmapDmaBuf() override {}
+
+ private:
+  scoped_refptr<ui::NativePixmap> pixmap_;
+};
+
+class GLImageOzoneOverlayOnlyPassThrough : public GLImage {
+ public:
+  GLImageOzoneOverlayOnlyPassThrough(scoped_refptr<ui::NativePixmap> pixmap,
+                                     const Size& size,
+                                     unsigned internalformat)
+      : pixmap_(pixmap), size_(size), internalformat_(internalformat) {}
+
+  void Destroy(bool have_context) override { pixmap_ = nullptr; }
+  Size GetSize() override { return size_; }
+  unsigned GetInternalFormat() override { return internalformat_; }
+  bool BindTexImage(unsigned target) override { return true; }
+  void ReleaseTexImage(unsigned target) override {}
+  bool CopyTexSubImage(unsigned target,
+                       const Point& offset,
+                       const Rect& rect) override {
+    return false;
+  }
+  void WillUseTexImage() override {}
+  void DidUseTexImage() override {}
+  void WillModifyTexImage() override {}
+  void DidModifyTexImage() override {}
+  bool ScheduleOverlayPlane(AcceleratedWidget widget,
+                            int z_order,
+                            OverlayTransform transform,
+                            const Rect& bounds_rect,
+                            const RectF& crop_rect) override {
+    return pixmap_ &&
+           pixmap_->ScheduleOverlayPlane(widget, z_order, transform,
+                                         bounds_rect, crop_rect);
+  }
+
+ protected:
+  ~GLImageOzoneOverlayOnlyPassThrough() override {}
+
+ private:
+  scoped_refptr<ui::NativePixmap> pixmap_;
+  const Size size_;
+  unsigned internalformat_;
+};
+
+}  // namespace
+
+scoped_refptr<GLImage> CreateImageForNativePixmap(
+    scoped_refptr<ui::NativePixmap> pixmap,
+    const Size& size,
+    GpuMemoryBuffer::Format format,
+    unsigned internalformat) {
+  if (!pixmap.get()) {
+    NOTREACHED();

[piman, 2015/07/24 18:41:25]

If NOTREACHED(), can you replace this by a DCHECK(!pixmap) ?

[dshwang, 2015/07/24 19:57:20]

Yes, DCHECK is better. Done.

+    return scoped_refptr<GLImage>();

[piman, 2015/07/24 18:41:25]

nit: return nullptr should work

+  }
+
+  if (pixmap->GetEGLClientBuffer()) {
+    scoped_refptr<GLImageOzoneNativePixmap> image =
+        new GLImageOzoneNativePixmap(size);
+    if (!image->Initialize(pixmap.get())) {
+      return scoped_refptr<GLImage>();

[piman, 2015/07/24 18:41:25]

nit: here too.

+    }
+    return image;
+  }
+  if (pixmap->GetDmaBufFd() > 0) {

[piman, 2015/07/24 18:41:25]

0 is a valid fd.

[dshwang, 2015/07/24 19:57:20]

change to "pixmap->GetDmaBufFd() >= 0"

+    scoped_refptr<GLImageOzoneNativePixmapDmaBuf> image =
+        new GLImageOzoneNativePixmapDmaBuf(size, internalformat);
+    if (!image->Initialize(pixmap.get(), format)) {
+      return scoped_refptr<GLImage>();

[piman, 2015/07/24 18:41:25]

nit: return nullptr should work

+    }
+    return image;
+  }
+  return new GLImageOzoneOverlayOnlyPassThrough(pixmap, size, internalformat);
+}
+
+}  // namespace gfx