gpu: Mailbox emulation with EGLImage

gpu/command_buffer/service/texture_definition.cc

Index: gpu/command_buffer/service/texture_definition.cc
diff --git a/gpu/command_buffer/service/texture_definition.cc b/gpu/command_buffer/service/texture_definition.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a7a2956b22bf4ac2f3f9e2aa05e78d5c01e24a65
--- /dev/null
+++ b/gpu/command_buffer/service/texture_definition.cc
@@ -0,0 +1,438 @@
+// Copyright (c) 2014 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 "gpu/command_buffer/service/texture_definition.h"
+
+#include "gpu/command_buffer/service/texture_manager.h"
+#include "ui/gl/gl_image.h"
+#include "ui/gl/gl_implementation.h"
+#include "ui/gl/scoped_binders.h"
+
+#if !defined(OS_MACOSX)
+#include "ui/gl/gl_surface_egl.h"
+#endif
+
+namespace gpu {
+namespace gles2 {
+
+namespace {
+
+class GLImageSync : public gfx::GLImage {
+ public:
+  explicit GLImageSync(
+      const scoped_refptr<NativeImageBuffer>& buffer);
+
+  // Implement GLImage.
+  virtual void Destroy() OVERRIDE;
+  virtual gfx::Size GetSize() OVERRIDE;
+  virtual bool BindTexImage(unsigned target) OVERRIDE;
+  virtual void ReleaseTexImage(unsigned target) OVERRIDE;
+  virtual void WillUseTexImage() OVERRIDE;
+  virtual void WillModifyTexImage() OVERRIDE;
+  virtual void DidModifyTexImage() OVERRIDE;
+
+  virtual void DidUseTexImage() OVERRIDE;
+  virtual void SetReleaseAfterUse() OVERRIDE;
+
+ protected:
+  virtual ~GLImageSync();
+
+ private:
+  scoped_refptr<NativeImageBuffer> buffer_;
+
+  DISALLOW_COPY_AND_ASSIGN(GLImageSync);
+};
+
+GLImageSync::GLImageSync(const scoped_refptr<NativeImageBuffer>& buffer)
+    : buffer_(buffer) {
+  if (buffer)
+    buffer->AddClient(this);
+}
+
+GLImageSync::~GLImageSync() {
+  if (buffer_)
+    buffer_->RemoveClient(this);
+}
+
+void GLImageSync::Destroy() {}
+
+gfx::Size GLImageSync::GetSize() {
+  NOTREACHED();
+  return gfx::Size();
+}
+
+bool GLImageSync::BindTexImage(unsigned target) {
+  NOTREACHED();
+  return false;
+}
+
+void GLImageSync::ReleaseTexImage(unsigned target) {
+  NOTREACHED();
+}
+
+void GLImageSync::WillUseTexImage() {
+  if (buffer_)
+    buffer_->WillRead(this);
+}
+
+void GLImageSync::DidUseTexImage() {
+  if (buffer_)
+    buffer_->DidRead(this);
+}
+
+void GLImageSync::WillModifyTexImage() {
+  if (buffer_)
+    buffer_->WillWrite(this);
+}
+
+void GLImageSync::DidModifyTexImage() {
+  if (buffer_)
+    buffer_->DidWrite(this);
+}
+
+void GLImageSync::SetReleaseAfterUse() {
+  NOTREACHED();
+}
+
+#if !defined(OS_MACOSX)
+class NativeImageBufferEGL : public NativeImageBuffer {
+ public:
+  static NativeImageBufferEGL* Create(GLuint texture_id);
+
+ private:
+  explicit NativeImageBufferEGL(EGLDisplay display, EGLImageKHR image);
+  virtual ~NativeImageBufferEGL();
+  virtual void BindToTexture(GLenum target) OVERRIDE;
+
+  EGLDisplay egl_display_;
+  EGLImageKHR egl_image_;
+
+  DISALLOW_COPY_AND_ASSIGN(NativeImageBufferEGL);
+};
+
+NativeImageBufferEGL* NativeImageBufferEGL::Create(GLuint texture_id) {

[piman, 2014/03/13 04:41:58]

nit: return a scoped_refptr<NativeImageBufferEGL>

[no sievers, 2014/03/13 20:50:15]

Done.

+  EGLDisplay egl_display = gfx::GLSurfaceEGL::GetHardwareDisplay();
+  EGLContext egl_context = eglGetCurrentContext();
+
+  if (egl_context == EGL_NO_CONTEXT || egl_display == EGL_NO_DISPLAY ||
+      !glIsTexture(texture_id)) {
+    return NULL;
+  }

[piman, 2014/03/13 04:41:58]

Can these conditions happen? Should they just be DCHECKs?

[no sievers, 2014/03/13 20:50:15]

You mean no context or display, right?

[piman, 2014/03/13 23:29:06]

Yes, thanks.

+
+  // TODO: Need to generate and check EGL_KHR_gl_texture_2D_image
+  if (!gfx::g_driver_egl.ext.b_EGL_KHR_image_base ||
+      !gfx::g_driver_gl.ext.b_GL_OES_EGL_image) {
+    return NULL;
+  }
+
+  const EGLint egl_attrib_list[] = {
+      EGL_GL_TEXTURE_LEVEL_KHR, 0, EGL_IMAGE_PRESERVED_KHR, EGL_TRUE, EGL_NONE};
+  EGLClientBuffer egl_buffer = reinterpret_cast<EGLClientBuffer>(texture_id);
+  EGLenum egl_target = EGL_GL_TEXTURE_2D_KHR; // TODO
+
+  EGLImageKHR egl_image = eglCreateImageKHR(
+      egl_display, egl_context, egl_target, egl_buffer, egl_attrib_list);
+
+  if (egl_image == EGL_NO_IMAGE_KHR)
+    return NULL;
+
+  return new NativeImageBufferEGL(egl_display, egl_image);
+}
+
+NativeImageBufferEGL::NativeImageBufferEGL(EGLDisplay display,
+                                           EGLImageKHR image)
+    : egl_display_(display), egl_image_(image) {
+  DCHECK(egl_display_ != EGL_NO_DISPLAY);
+  DCHECK(egl_image_ != EGL_NO_IMAGE_KHR);
+}
+
+NativeImageBufferEGL::~NativeImageBufferEGL() {
+  if (egl_image_ != EGL_NO_IMAGE_KHR)
+    eglDestroyImageKHR(egl_display_, egl_image_);
+}
+
+void NativeImageBufferEGL::BindToTexture(GLenum target) {
+  DCHECK(egl_image_ != EGL_NO_IMAGE_KHR);
+  glEGLImageTargetTexture2DOES(target, egl_image_);
+  DCHECK_EQ(EGL_SUCCESS, (EGLint)eglGetError());
+  DCHECK_EQ(GL_NO_ERROR, (GLint)glGetError());

[piman, 2014/03/13 04:41:58]

nit: no c-style casts. Prefer static_cast or reinterpret_cast as appropriate.

[no sievers, 2014/03/13 20:50:15]

Done.

+}
+#endif
+
+class NativeImageBufferStub : public NativeImageBuffer {
+ public:
+  NativeImageBufferStub() {}
+
+ private:
+  virtual ~NativeImageBufferStub() {}
+  virtual void BindToTexture(GLenum target) OVERRIDE {}
+
+  DISALLOW_COPY_AND_ASSIGN(NativeImageBufferStub);
+};
+
+}  // anonymous namespace
+
+// static
+NativeImageBuffer* NativeImageBuffer::Create(GLuint texture_id) {
+  switch (gfx::GetGLImplementation()) {
+#if !defined(OS_MACOSX)
+    case gfx::kGLImplementationEGLGLES2:
+      return NativeImageBufferEGL::Create(texture_id);
+#endif
+    case gfx::kGLImplementationMockGL:
+      return new NativeImageBufferStub;
+    default:
+      NOTREACHED();
+      return NULL;
+  }
+}
+
+NativeImageBuffer::ClientInfo::ClientInfo(gfx::GLImage* client)
+    : client(client), needs_wait_before_read(true) {}
+
+NativeImageBuffer::ClientInfo::~ClientInfo() {}
+
+NativeImageBuffer::NativeImageBuffer() : write_client_(NULL) {
+  write_fence_.reset(gfx::GLFence::Create());
+}
+
+NativeImageBuffer::~NativeImageBuffer() {
+  DCHECK(client_infos_.empty());
+}
+
+void NativeImageBuffer::AddClient(gfx::GLImage* client) {
+  base::AutoLock lock(lock_);
+  client_infos_.push_back(ClientInfo(client));
+}
+
+void NativeImageBuffer::RemoveClient(gfx::GLImage* client) {
+  base::AutoLock lock(lock_);
+  if (write_client_ == client)
+    write_client_ = NULL;
+  for (std::list<ClientInfo>::iterator it = client_infos_.begin();
+       it != client_infos_.end();
+       it++) {
+    if (it->client == client) {
+      client_infos_.erase(it);
+      return;
+    }
+  }
+  NOTREACHED();
+}
+
+bool NativeImageBuffer::IsClient(gfx::GLImage* client) {
+  base::AutoLock lock(lock_);
+  for (std::list<ClientInfo>::iterator it = client_infos_.begin();
+       it != client_infos_.end();
+       it++) {
+    if (it->client == client)
+      return true;
+  }
+  return false;
+}
+
+void NativeImageBuffer::WillRead(gfx::GLImage* client) {
+  base::AutoLock lock(lock_);
+  if (!write_fence_.get() || write_client_ == client)
+    return;
+
+  for (std::list<ClientInfo>::iterator it = client_infos_.begin();
+       it != client_infos_.end();
+       it++) {
+    if (it->client == client) {
+      if (it->needs_wait_before_read) {
+        it->needs_wait_before_read = false;
+        write_fence_->ServerWait();
+      }
+      return;
+    }
+  }
+  NOTREACHED();
+}
+
+void NativeImageBuffer::WillWrite(gfx::GLImage* client) {
+  base::AutoLock lock(lock_);
+  if (write_client_ != client)
+    write_fence_->ServerWait();
+
+  for (std::list<ClientInfo>::iterator it = client_infos_.begin();
+       it != client_infos_.end();
+       it++) {
+    if (it->read_fence.get() && it->client != client)
+      it->read_fence->ServerWait();
+  }
+}
+
+void NativeImageBuffer::DidRead(gfx::GLImage* client) {
+  base::AutoLock lock(lock_);
+  for (std::list<ClientInfo>::iterator it = client_infos_.begin();
+       it != client_infos_.end();
+       it++) {
+    if (it->client == client) {
+      it->read_fence = make_linked_ptr(gfx::GLFence::Create());
+      return;
+    }
+  }
+  NOTREACHED();
+}
+
+void NativeImageBuffer::DidWrite(gfx::GLImage* client) {
+  base::AutoLock lock(lock_);
+  // TODO(sievers): This is super-risky. We need to somehow find out
+  // about when the current context gets flushed, so that we will only
+  // ever wait on the write fence (esp. from another context) if it was
+  // flushed and is guaranteed to clear.
+  // On the other hand, proactively flushing here is not feasible in terms
+  // of perf when there are multiple draw calls per frame.

[piman, 2014/03/13 04:41:58]

So, the risk is that if we're missing a sync point somewhere, we might deadlock
the GPU if we have one write and one read, and no glFlush in the middle.

Would it be possible to, say, not "publish" the write fence until we push? I.e.
make the write_fence a property of the TextureDefinition? We might even be able
to delay the inserting of the fence until the Push rather than the draw, since
otherwise you'll insert one fence for every draw, which seems heavy.

We would need to tag the NativeImageBuffer as "dirty" in DidWrite, then on Push,
look for dirty NativeImageBuffers (and treat them as a version update), and if
any, lazily insert a single fence. Then on Pull, give the fence (if any) to the
GLImageSync (we may need refcounting...), which will then wait on it before read
or write.

WDYT?

[no sievers, 2014/03/13 20:50:15]

Can I owe this as an immediate follow-up?
I was also considering to expose this functionality in GLFence. Maybe the fence
could be notified about flushes by the GL api?

[piman, 2014/03/13 23:29:06]

Maybe file a bug and reference here... It's not something I would typically want
to have in the GPU process, depending on client-side behavior for correctness. I
guess this would only happen on WebView for now where the security aspects are
different, but I don't want us to start relying on this in other cases.

[no sievers, 2014/03/13 23:51:13]

Done.

+  write_fence_.reset(gfx::GLFence::CreateWithoutFlush());
+  write_client_ = client;
+  for (std::list<ClientInfo>::iterator it = client_infos_.begin();
+       it != client_infos_.end();
+       it++) {
+    it->needs_wait_before_read = true;
+  }
+}
+
+TextureDefinition::LevelInfo::LevelInfo(GLenum target,
+                                        GLenum internal_format,
+                                        GLsizei width,
+                                        GLsizei height,
+                                        GLsizei depth,
+                                        GLint border,
+                                        GLenum format,
+                                        GLenum type,
+                                        bool cleared)
+    : target(target),
+      internal_format(internal_format),
+      width(width),
+      height(height),
+      depth(depth),
+      border(border),
+      format(format),
+      type(type),
+      cleared(cleared) {}
+
+TextureDefinition::LevelInfo::~LevelInfo() {}
+
+TextureDefinition::TextureDefinition(GLenum target,
+                                     Texture* texture,
+                                     unsigned int version,
+                                     NativeImageBuffer* image)
+    : version_(version),
+      target_(target),
+      image_(image ? image : NativeImageBuffer::Create(texture->service_id())),
+      min_filter_(texture->min_filter()),
+      mag_filter_(texture->mag_filter()),
+      wrap_s_(texture->wrap_s()),
+      wrap_t_(texture->wrap_t()),
+      usage_(texture->usage()),
+      immutable_(texture->IsImmutable()) {
+
+  // TODO
+  DCHECK(!texture->level_infos_.empty());
+  DCHECK(!texture->level_infos_[0].empty());
+  DCHECK(!texture->NeedsMips());
+  DCHECK(texture->level_infos_[0][0].width);
+  DCHECK(texture->level_infos_[0][0].height);
+
+  scoped_refptr<gfx::GLImage> gl_image(new GLImageSync(image_));
+  texture->SetLevelImage(NULL, target, 0, gl_image);
+
+  // TODO: all levels
+  level_infos_.clear();
+  const Texture::LevelInfo& level = texture->level_infos_[0][0];
+  LevelInfo info(level.target,
+                 level.internal_format,
+                 level.width,
+                 level.height,
+                 level.depth,
+                 level.border,
+                 level.format,
+                 level.type,
+                 level.cleared);
+  std::vector<LevelInfo> infos;
+  infos.push_back(info);
+  level_infos_.push_back(infos);
+
+}
+
+TextureDefinition::~TextureDefinition() {
+}
+
+Texture* TextureDefinition::CreateTexture() const {
+  if (!image_)
+    return NULL;
+
+  GLuint texture_id;
+  glGenTextures(1, &texture_id);
+
+  Texture* texture(new Texture(texture_id));
+  UpdateTexture(texture);
+
+  return texture;
+}
+
+void TextureDefinition::UpdateTexture(Texture* texture) const {
+  gfx::ScopedTextureBinder texture_binder(target_, texture->service_id());
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, min_filter_);
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, mag_filter_);
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, wrap_s_);
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, wrap_t_);
+  DCHECK(image_);
+  if (image_)

[piman, 2014/03/13 04:41:58]

DCHECK, or if? Chose one.

[no sievers, 2014/03/13 20:50:15]

Done. Should eventually handle incomplete textures, that later create an image
when defined.

+    image_->BindToTexture(target_);
+  glFlush();

[piman, 2014/03/13 04:41:58]

DO you need this glFlush?

[no sievers, 2014/03/13 20:50:15]

Most of the time probably not. There is one case I'm not sure about wrt
compatibility to share group mailbox semantics:

ctx1 is in driver share group A
ctx2a and ctx2b are in driver share group B

ctx1 defines the texture and flushes to make changes visible, then produces
ctx2a consumes and then shares with ctx2b

With existing mailbox semantics ctx2a does not need to flush because it's not
making changes to the texture before sharing it with ctx2b.
But with this we defined a new texture in ctx2a and it wouldn't be necessarily
visible to ctx2b, right?

[piman, 2014/03/13 23:29:06]

I see... arguably, ctx2a needs to flush after the Consume to share with ctx2b.
But in any case, that shouldn't necessarily be required for subsequent updates,
so I could see this.
Maybe add a comment?

[no sievers, 2014/03/13 23:51:13]

Done. Maybe we could be smarter about when the flush is needed.

+
+  texture->level_infos_.resize(1);
+  for (size_t i = 0; i < level_infos_.size(); i++) {
+    const LevelInfo& base_info = level_infos_[i][0];
+    const size_t levels_needed = TextureManager::ComputeMipMapCount(
+        base_info.target, base_info.width, base_info.height, base_info.depth);
+    DCHECK(level_infos_.size() <= levels_needed);
+    texture->level_infos_[0].resize(levels_needed);
+    for (size_t n = 0; n < level_infos_.size(); n++) {
+      const LevelInfo& info = level_infos_[i][n];
+      texture->SetLevelInfo(NULL,
+                            info.target,
+                            i,
+                            info.internal_format,
+                            info.width,
+                            info.height,
+                            info.depth,
+                            info.border,
+                            info.format,
+                            info.type,
+                            info.cleared);
+    }
+  }
+  if (image_)
+    texture->SetLevelImage(NULL, target_, 0, new GLImageSync(image_));
+
+  texture->target_ = target_;
+  texture->SetImmutable(immutable_);
+  texture->min_filter_ = min_filter_;
+  texture->mag_filter_ = mag_filter_;
+  texture->wrap_s_ = wrap_s_;
+  texture->wrap_t_ = wrap_t_;
+  texture->usage_ = usage_;
+}
+
+bool TextureDefinition::Matches(const Texture* texture) const {
+  DCHECK(target_ == texture->target());
+  if (texture->min_filter_ != min_filter_ ||
+      texture->mag_filter_ != mag_filter_ ||
+      texture->wrap_s_ != wrap_s_ ||
+      texture->wrap_t_ != wrap_t_) {
+    return false;
+  }
+
+  // All structural changes should have orphaned the texture.
+  if (image_ && !texture->GetLevelImage(texture->target(), 0))
+    return false;
+
+  return true;
+}
+
+}  // namespace gles2
+}  // namespace gpu