gpu: Add support for GLX_EXT_texture_from_pixmap extension.

ui/gl/gl_image_glx.cc

+// Copyright (c) 2012 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.
+
+extern "C" {
+#include <X11/extensions/Xcomposite.h>
+}
+
+#include "ui/gl/gl_image_glx.h"
+
+#include "base/basictypes.h"
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
+#include "ui/gl/gl_surface_glx.h"
+
+namespace gfx {
+
+namespace {
+
+// scoped_ptr functor for XFree(). Use as follows:
+//   scoped_ptr_malloc<XVisualInfo, ScopedPtrXFree> foo(...);
+// where "XVisualInfo" is any X type that is freed with XFree.
+class ScopedPtrXFree {
+ public:
+  void operator()(void* x) const {
+    ::XFree(x);
+  }
+};
+
+}  // namespace anonymous
+
+GLImageGLX::GLImageGLX(gfx::PluginWindowHandle window)
+  : display_(base::MessagePumpForUI::GetDefaultXDisplay()),
+    pixmap_(XCompositeNameWindowPixmap(display_, window)),
+    glx_pixmap_(0) {
+}
+
+bool GLImageGLX::Initialize() {
+  if (!GLSurfaceGLX::IsTextureFromPixmapSupported()) {
+    LOG(ERROR) << "GLX_EXT_texture_from_pixmap not supported.";
+    return false;
+  }
+
+  XID root;
+  int x, y;
+  unsigned int width, height, border_width, depth;

[piman, 2012/10/12 22:05:11]

nit: please initialize all local variables.

[reveman, 2012/10/13 05:25:53]

Done.

+  if (!XGetGeometry(display_, pixmap_, &root, &x, &y, &width, &height,
+                    &border_width, &depth)) {
+    LOG(ERROR) << "XGetGeometry failed for pixmap " << pixmap_ << ".";
+    return false;
+  }
+
+  int num_elements = 0;
+  scoped_ptr_malloc<GLXFBConfig, ScopedPtrXFree> configs(
+      glXGetFBConfigs(display_,
+                      DefaultScreen(display_),
+                      &num_elements));
+  if (!configs.get()) {
+    LOG(ERROR) << "glXGetFBConfigs failed.";
+    return false;
+  }
+  if (!num_elements) {
+    LOG(ERROR) << "glXGetFBConfigs returned 0 elements.";
+    return false;
+  }
+  bool found = false;
+  int i;

[piman, 2012/10/12 22:05:11]

nit move the = 0 initialization here.

[reveman, 2012/10/13 05:25:53]

Done.

+  for (i = 0; i < num_elements; ++i) {
+    int value;

[piman, 2012/10/12 22:05:11]

nit: initialize

[reveman, 2012/10/13 05:25:53]

Done.

+    if (glXGetFBConfigAttrib(
+            display_, configs.get()[i], GLX_DRAWABLE_TYPE, &value)) {
+      LOG(ERROR) << "glXGetFBConfigAttrib failed.";
+      return false;
+    }
+    if (!(value & GLX_PIXMAP_BIT))
+      continue;
+    if (glXGetFBConfigAttrib(
+            display_, configs.get()[i], GLX_BIND_TO_TEXTURE_TARGETS_EXT,
+            &value)) {
+      LOG(ERROR) << "glXGetFBConfigAttrib failed.";
+      return false;
+    }
+    if (!(value & GLX_TEXTURE_2D_BIT_EXT))
+      continue;
+    if (glXGetFBConfigAttrib(
+            display_, configs.get()[i], (depth == 32) ?
+            GLX_BIND_TO_TEXTURE_RGBA_EXT : GLX_BIND_TO_TEXTURE_RGB_EXT,

[piman, 2012/10/12 22:05:11]

so, techincally, we can have RGB windows in a 32 bit visual, in which case alpha
may be garbage. Should we inspect the window's visual instead of the resulting
pixmap's depth?

[reveman, 2012/10/13 05:25:53]

Done.

+            &value)) {
+      LOG(ERROR) << "glXGetFBConfigAttrib failed.";
+      return false;
+    }
+    if (value == GL_FALSE)
+      continue;
+    if (glXGetFBConfigAttrib(
+            display_, configs.get()[i], GLX_DOUBLEBUFFER, &value)) {
+      LOG(ERROR) << "glXGetFBConfigAttrib failed.";
+      return false;
+    }
+    // Avoid double buffered config.
+    if (value == GL_TRUE)
+      continue;
+    if (glXGetFBConfigAttrib(
+            display_, configs.get()[i], GLX_BUFFER_SIZE, &value)) {
+      LOG(ERROR) << "glXGetFBConfigAttrib failed.";
+      return false;
+    }
+    if (value < static_cast<int>(depth))
+      continue;
+
+    found = true;
+    break;
+  }

[piman, 2012/10/12 22:05:11]

Can we replace all this by glXChooseFBConfig?

[reveman, 2012/10/13 05:25:53]

Done.

+
+  if (!found) {
+    LOG(ERROR) << "Failed to find valid FBConfig for pixmap.";
+    return false;
+  }
+
+  std::vector<int> attribs;
+  attribs.push_back(GLX_TEXTURE_TARGET_EXT);
+  attribs.push_back(GLX_TEXTURE_2D_EXT);
+  attribs.push_back(GLX_TEXTURE_FORMAT_EXT);
+  if (depth == 32)
+    attribs.push_back(GLX_TEXTURE_FORMAT_RGBA_EXT);
+  else
+    attribs.push_back(GLX_TEXTURE_FORMAT_RGB_EXT);
+  attribs.push_back(0);

[piman, 2012/10/12 22:05:11]

Since the array is fixed size, can we just make this an array? It'll be both
more readable and more efficient, while avoiding the ugliness of passing a
pointer to the underlying storage ( &attribs.front() ).

[reveman, 2012/10/13 05:25:53]

Done.

+
+  glx_pixmap_ = glXCreatePixmap(
+      display_,
+      configs.get()[i],
+      pixmap_,
+      &attribs.front());
+  if (!glx_pixmap_) {
+    LOG(ERROR) << "glXCreatePixmap failed.";
+    return false;
+  }
+
+  size_ = gfx::Size(width, height);
+  return true;
+}
+
+void GLImageGLX::Destroy() {
+  if (glx_pixmap_) {
+    glXDestroyGLXPixmap(display_, glx_pixmap_);
+    glx_pixmap_ = 0;
+  }
+  if (pixmap_) {
+    XFreePixmap(display_, pixmap_);
+    pixmap_ = 0;
+  }
+}
+
+gfx::Size GLImageGLX::GetSize() {
+  return size_;
+}
+
+bool GLImageGLX::BindTexImage() {
+  if (!glx_pixmap_)
+    return false;
+
+  glXBindTexImageEXT(display_, glx_pixmap_, GLX_FRONT_LEFT_EXT, 0);
+  return true;
+}
+
+void GLImageGLX::ReleaseTexImage() {
+  if (!glx_pixmap_)
+    return;
+
+  glXReleaseTexImageEXT(display_, glx_pixmap_, GLX_FRONT_LEFT_EXT);
+}
+
+GLImageGLX::~GLImageGLX() {
+  Destroy();
+}
+
+}  // namespace gfx