GPU client side changes for GpuMemoryBuffers

gpu/command_buffer/client/gles2_implementation.cc

Index: gpu/command_buffer/client/gles2_implementation.cc
diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc
index 84baef0dda8187fbbe694b90d794eb02ceb49b13..0bc0f094cfa78fe78fda7a8e4fe7495df4ad81a1 100644
--- a/gpu/command_buffer/client/gles2_implementation.cc
+++ b/gpu/command_buffer/client/gles2_implementation.cc
@@ -16,6 +16,9 @@
 #include <GLES2/gl2ext.h>
 #include <GLES2/gl2extchromium.h>
 #include "../client/buffer_tracker.h"
+#include "../client/gpu_memory_buffer.h"
+#include "../client/gpu_memory_buffer_factory.h"
+#include "../client/gpu_memory_buffer_tracker_in_process.h"
 #include "../client/mapped_memory.h"
 #include "../client/program_info_manager.h"
 #include "../client/query_tracker.h"
@@ -102,7 +105,8 @@ GLES2Implementation::GLES2Implementation(
       debug_(false),
       use_count_(0),
       current_query_(NULL),
-      error_message_callback_(NULL) {
+      error_message_callback_(NULL),
+      image_factory_() {

[no sievers, 2013/05/10 01:21:51]

nit: remove this

[kaanb, 2013/05/13 23:00:36]

We're now passing the image_factory via the constructor.

   GPU_DCHECK(helper);
   GPU_DCHECK(transfer_buffer);
 
@@ -173,6 +177,9 @@ bool GLES2Implementation::Initialize(
 
   query_tracker_.reset(new QueryTracker(mapped_memory_.get()));
   buffer_tracker_.reset(new BufferTracker(mapped_memory_.get()));
+  if (gpu_memory_buffer_tracker_.get() == NULL && image_factory_.get() != NULL)
+      gpu_memory_buffer_tracker_.reset(
+         new GpuMemoryBufferTrackerInProcess(image_factory_.get(), this));

[reveman, 2013/05/10 02:06:16]

You can't make assumptions about single/multi-process here. I think the
GpuMemoryBufferTracker should be generic. You already have the ImageFactory for
single-process specific code.

[kaanb, 2013/05/13 23:00:36]

Done.

 
 #if defined(GLES2_SUPPORT_CLIENT_SIDE_ARRAYS)
   GetIdHandler(id_namespaces::kBuffers)->MakeIds(
@@ -2029,6 +2036,10 @@ const GLubyte* GLES2Implementation::GetStringHelper(GLenum name) {
             "GL_CHROMIUM_map_sub "
             "GL_CHROMIUM_shallow_flush "
             "GL_EXT_unpack_subimage";
+        if (image_factory_.get() != NULL) {
+          str += " ";

[no sievers, 2013/05/10 01:21:51]

str += std::string(str.empty() ? "" : " ") + "GL_CHROMIUM_map_image";

[kaanb, 2013/05/13 23:00:36]

str.empty() is guaranteed to be false here, so I simplified your suggestion a
bit.

+          str += "GL_CHROMIUM_map_image";
+        }
         break;
       default:
         break;
@@ -3438,11 +3449,11 @@ GLboolean GLES2Implementation::UnmapBufferCHROMIUM(GLuint target) {
   }
   BufferTracker::Buffer* buffer = buffer_tracker_->GetBuffer(buffer_id);
   if (!buffer) {
-    SetGLError(GL_INVALID_OPERATION, "glMapBufferCHROMIUM", "invalid buffer");
+    SetGLError(GL_INVALID_OPERATION, "glUnmapBufferCHROMIUM", "invalid buffer");
     return false;
   }
   if (!buffer->mapped()) {
-    SetGLError(GL_INVALID_OPERATION, "glMapBufferCHROMIUM", "not mapped");
+    SetGLError(GL_INVALID_OPERATION, "glUnmapBufferCHROMIUM", "not mapped");
     return false;
   }
   buffer->set_mapped(false);
@@ -3558,6 +3569,158 @@ GLuint GLES2Implementation::InsertSyncPointCHROMIUM() {
   return helper_->InsertSyncPointCHROMIUM();
 }
 
+GLuint GLES2Implementation::CreateImageCHROMIUMHelper(GLsizei width,
+                                                      GLsizei height) {
+  if (width <= 0) {
+    SetGLError(GL_INVALID_VALUE, "glCreateImageCHROMIUM", "width <= 0");
+    return 0;
+  }
+
+  if (height <= 0) {
+    SetGLError(GL_INVALID_VALUE, "glCreateImageCHROMIUM", "height <= 0");
+    return 0;
+  }
+
+  // Create new buffer.
+  return gpu_memory_buffer_tracker_->CreateBuffer(width, height);

[no sievers, 2013/05/10 01:21:51]

Could that fail? Should we set OUT_OF_MEMORY or something in that case?

[kaanb, 2013/05/13 23:00:36]

Yes, it could fail, right now we DCHECK() in
android_webview::GpuMemoryBufferImpl() if that happens. I can put a TODO to
plumb it here.

+}
+
+GLuint GLES2Implementation::CreateImageCHROMIUM(GLsizei width, GLsizei height) {
+  GPU_CLIENT_SINGLE_THREAD_CHECK();
+  GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glCreateImageCHROMIUM("
+      << width << ", "
+      << height << ")");
+  GLuint image_id = CreateImageCHROMIUMHelper(width, height);
+  CheckGLError();
+  return image_id;
+}
+
+void GLES2Implementation::DestroyImageCHROMIUMHelper(GLuint image_id) {
+  GpuMemoryBuffer* gpu_buffer =
+       gpu_memory_buffer_tracker_->GetBuffer(image_id);
+   if (!gpu_buffer) {
+     SetGLError(GL_INVALID_OPERATION, "glDestroyImageImageCHROMIUM",
+                "invalid GPU memory buffer");

[no sievers, 2013/05/10 01:21:51]

nit: "invalid image" here and below

[kaanb, 2013/05/13 23:00:36]

Done.

+     return;
+   }
+
+   gpu_memory_buffer_tracker_->RemoveBuffer(image_id);
+}
+
+void GLES2Implementation::DestroyImageCHROMIUM(GLuint image_id) {
+  GPU_CLIENT_SINGLE_THREAD_CHECK();
+  GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glDestroyImageCHROMIUM("
+      << image_id << ")");
+  DestroyImageCHROMIUMHelper(image_id);
+  CheckGLError();
+}
+
+GLboolean GLES2Implementation::UnmapImageCHROMIUMHelper(GLuint image_id) {
+   GpuMemoryBuffer* gpu_buffer =
+       gpu_memory_buffer_tracker_->GetBuffer(image_id);
+   if (!gpu_buffer) {
+     SetGLError(GL_INVALID_OPERATION, "glUnmapImageCHROMIUM",
+                "invalid GPU memory buffer");
+     return false;
+   }
+
+   if (!gpu_buffer->IsMapped()) {
+     SetGLError(GL_INVALID_OPERATION, "glUnmapImageCHROMIUM",
+                "not mapped");
+     return false;
+   }
+   gpu_buffer->Unmap();
+   return true;
+}
+
+GLboolean GLES2Implementation::UnmapImageCHROMIUM(GLuint image_id) {
+  GPU_CLIENT_SINGLE_THREAD_CHECK();
+  GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glUnmapImageCHROMIUM("
+       << image_id << ")");
+
+  bool success = UnmapImageCHROMIUMHelper(image_id);
+  CheckGLError();
+  return success;
+}
+
+void* GLES2Implementation::MapImageCHROMIUMHelper(GLuint image_id,
+                                                  GLenum access) {
+   GpuMemoryBuffer* gpu_buffer =
+       gpu_memory_buffer_tracker_->GetBuffer(image_id);
+   if (!gpu_buffer) {
+     SetGLError(GL_INVALID_OPERATION, "glMapImageCHROMIUM",
+                "invalid GPU memory buffer");
+     return NULL;
+   }
+   GpuMemoryBuffer::AccessMode mode = GpuMemoryBuffer::READ_ONLY;

[no sievers, 2013/05/10 01:21:51]

nit: no need to initialize

[kaanb, 2013/05/13 23:00:36]

Done.

+   switch(access) {
+     case GL_WRITE_ONLY:
+       mode = GpuMemoryBuffer::WRITE_ONLY;
+       break;
+     case GL_READ_ONLY:
+       mode = GpuMemoryBuffer::READ_ONLY;
+       break;
+       // TODO(kaanb): should we add GL_READ_WRITE to gl2ext.h?

[no sievers, 2013/05/10 01:21:51]

Yes, isn't readwrite what we need to raster into this for tiles?

[kaanb, 2013/05/13 23:00:36]

Done.

+     default:
+       SetGLError(GL_INVALID_ENUM, "glMapImageCHROMIUM",
+                  "invalid GPU access mode");
+       return NULL;
+   }
+
+   if (gpu_buffer->IsMapped()) {
+     SetGLError(GL_INVALID_OPERATION, "glMapImageCHROMIUM",
+                "already mapped");
+     return NULL;
+   }
+
+   void* mapped_buffer = NULL;
+   gpu_buffer->Map(mode, &mapped_buffer);

[no sievers, 2013/05/10 01:21:51]

Can this fail and should we set a GL error if so?

[kaanb, 2013/05/13 23:00:36]

From what I understand the documentation, the only failure mode is if the flags
used to create the buffer are inconsistent with the map access mode. Since it
indicated a setup failure, currently I DCHECK in android_webview::Map() if Map()
(i.e. Lock()) fails.

[no sievers, 2013/05/14 21:10:03]

If it's something the client can do wrong, then it should not be a DCHECK() but
a properly handled error.

+   return mapped_buffer;
+}
+
+void* GLES2Implementation::MapImageCHROMIUM(
+    GLuint image_id, GLenum access) {
+  GPU_CLIENT_SINGLE_THREAD_CHECK();
+  GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glMapImageCHROMIUM("
+      << image_id << ", "
+      << GLES2Util::GetStringEnum(access) << ")");
+
+  void* mapped = MapImageCHROMIUMHelper(image_id, access);
+  CheckGLError();
+  return mapped;
+}
+
+void GLES2Implementation::GetImageParameterivCHROMIUMHelper(
+    GLuint image_id, GLenum pname, GLint* params) {
+  if (pname != GL_IMAGE_ROWBYTES) {
+    SetGLError(GL_INVALID_ENUM, "glGetImageParameterivCHROMIUM",
+               "invalid target");

[no sievers, 2013/05/10 01:21:51]

nit: invalid parameter

[kaanb, 2013/05/13 23:00:36]

Done.

+    return;
+  }
+
+  GpuMemoryBuffer* gpu_buffer =
+      gpu_memory_buffer_tracker_->GetBuffer(image_id);
+  if (!gpu_buffer) {
+    SetGLError(GL_INVALID_OPERATION, "glGetImageParameterivCHROMIUM",
+               "invalid GPU memory buffer");
+    return;
+  }
+
+  *params = gpu_buffer->GetStride();
+}
+
+void GLES2Implementation::GetImageParameterivCHROMIUM(
+    GLuint image_id, GLenum pname, GLint* params) {
+  GPU_CLIENT_SINGLE_THREAD_CHECK();
+  GPU_CLIENT_VALIDATE_DESTINATION_INITALIZATION(GLint, params);
+  GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glMapImageCHROMIUM("
+      << image_id << ", "
+      << GLES2Util::GetStringBufferParameter(pname) << ", "
+      << static_cast<const void*>(params) << ")");
+  GetImageParameterivCHROMIUM(image_id, pname, params);
+  CheckGLError();
+}
+
 // Include the auto-generated part of this file. We split this because it means
 // we can easily edit the non-auto generated parts right here in this file
 // instead of having to edit some template or the code generator.