GPU client side changes for GpuMemoryBuffers

gpu/command_buffer/client/gles2_implementation.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.
 
 // A class to emulate GLES2 over command buffers.
 
 #include "../client/gles2_implementation.h"
 
 #include <algorithm>
 #include <map>
 #include <queue>
 #include <set>
 #include <limits>
 #include <stdio.h>
 #include <string.h>
 #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/mapped_memory.h"
 #include "../client/program_info_manager.h"
 #include "../client/query_tracker.h"
 #include "../client/transfer_buffer.h"
 #include "../client/vertex_array_object_manager.h"
 #include "../common/gles2_cmd_utils.h"
 #include "../common/trace_event.h"
 
 #if defined(__native_client__) && !defined(GLES2_SUPPORT_CLIENT_SIDE_ARRAYS)
 #define GLES2_SUPPORT_CLIENT_SIDE_ARRAYS
@@ skipping 62 matching lines @@
       unpack_skip_pixels_(0),
       pack_reverse_row_order_(false),
       active_texture_unit_(0),
       bound_framebuffer_(0),
       bound_read_framebuffer_(0),
       bound_renderbuffer_(0),
       current_program_(0),
       bound_array_buffer_id_(0),
       bound_pixel_pack_transfer_buffer_id_(0),
       bound_pixel_unpack_transfer_buffer_id_(0),
+      bound_gpu_memory_buffer_id_(0),
       error_bits_(0),
       debug_(false),
       use_count_(0),
       current_query_(NULL),
       error_message_callback_(NULL) {
   GPU_DCHECK(helper);
   GPU_DCHECK(transfer_buffer);
 
   char temp[128];
   sprintf(temp, "%p", static_cast<void*>(this));
@@ skipping 55 matching lines @@
       static_state_.int_state.num_compressed_texture_formats);
   util_.set_num_shader_binary_formats(
       static_state_.int_state.num_shader_binary_formats);
 
   texture_units_.reset(
       new TextureUnit[
           static_state_.int_state.max_combined_texture_image_units]);
 
   query_tracker_.reset(new QueryTracker(mapped_memory_.get()));
   buffer_tracker_.reset(new BufferTracker(mapped_memory_.get()));
+  gpu_memory_buffer_tracker_.reset(new GpuMemoryBufferTracker());
 
 #if defined(GLES2_SUPPORT_CLIENT_SIDE_ARRAYS)
   GetIdHandler(id_namespaces::kBuffers)->MakeIds(
       this, kClientSideArrayId, arraysize(reserved_ids_), &reserved_ids_[0]);
 #endif
 
   vertex_array_object_manager_.reset(new VertexArrayObjectManager(
       static_state_.int_state.max_vertex_attribs,
       reserved_ids_[0],
       reserved_ids_[1]));
@@ skipping 437 matching lines @@
         *params = bound_read_framebuffer_;
         return true;
       }
       return false;
     case GL_RENDERBUFFER_BINDING:
       if (share_group_->bind_generates_resource()) {
         *params = bound_renderbuffer_;
         return true;
       }
       return false;
+    case GL_BOUND_GPU_MEMORY_BUFFER_CHROMIUM:

[greggman, 2013/04/25 23:45:34]

Seems like following the other ENUMs this should be

  GL_IMAGE_BUFFER_BINDING_CHROMIUM

To match the target GL_IMAGE_BUFFER_CHROMIUM

[kaanb1, 2013/04/26 03:01:21]

Done.

+      *params = bound_gpu_memory_buffer_id_;
+      return true;
     default:
       return false;
   }
 }
 
 bool GLES2Implementation::GetBooleanvHelper(GLenum pname, GLboolean* params) {
   // TODO(gman): Make this handle pnames that return more than 1 value.
   GLint value;
   if (!GetHelper(pname, &value)) {
     return false;
@@ skipping 595 matching lines @@
     }
   }
 
   GPU_DCHECK_EQ(total_size, offset);
 
   helper_->ShaderSourceBucket(shader, kResultBucketId);
   helper_->SetBucketSize(kResultBucketId, 0);
   CheckGLError();
 }
 
+bool GLES2Implementation::GetBoundGpuMemoryBuffer(

[greggman, 2013/04/25 23:45:34]

This function always returns true. Did you mean it to return false if it
generates an error?

[kaanb1, 2013/04/26 03:01:21]

GetBoundPixelTransferBuffer only returns false when called with a target it
doesn't know about. Since we don't take target as a parameter here it isn't
applicable. Are there any other error situations?

+    const char* function_name, GLuint* buffer_id) {
+  *buffer_id = bound_gpu_memory_buffer_id_;
+
+  if (*buffer_id == 0) {
+    SetGLError(GL_INVALID_OPERATION, function_name, "no buffer bound");
+  }
+
+  return true;
+}
+
 void GLES2Implementation::BufferDataHelper(
     GLenum target, GLsizeiptr size, const void* data, GLenum usage) {
   if (size < 0) {
     SetGLError(GL_INVALID_VALUE, "glBufferData", "size < 0");
     return;
   }
 
   GLuint buffer_id;
   if (GetBoundPixelTransferBuffer(target, "glBufferData", &buffer_id)) {
     if (!buffer_id) {
@@ skipping 752 matching lines @@
       precision[0] = result->precision;
       GPU_CLIENT_LOG("  min_range: " << precision[0]);
     }
   }
   CheckGLError();
 }
 
 const GLubyte* GLES2Implementation::GetStringHelper(GLenum name) {
   const char* result = NULL;
   // Clears the bucket so if the command fails nothing will be in it.
+  if (name == GL_BOUND_GPU_MEMORY_BUFFER_CHROMIUM_POINTER) {

[greggman, 2013/04/25 23:45:34]

Doesn't this seem like a kind of abuse of this function?

How about adding some other function? Maybe either
glGetMemoryBufferPointerCHROMIUM

Or

void glGetPointervCHROMIUM(GLenum pname, GLvoid** pointer);

Which you can call like this

  GLvoid* buffer = NULL;
  glGetPointervCHROMIUM(
      GL_IMAGE_BUFFER_POINTER_CHROMIUM, &buffer);

I'd prefer the second as it can be used for other things later. What do you
think?

[kaanb1, 2013/04/26 03:01:21]

Done.

+    return const_cast<const GLubyte*>(static_cast<GLubyte*>(
+        GetNativeHandleForBoundGpuMemoryBuffer()));

[jamesr, 2013/04/25 23:24:15]

this really seems like voodoo. are there no better entry points?

[apatrick_chromium, 2013/04/25 23:39:03]

Agreed. I had a "wat?" moment.

[kaanb1, 2013/04/25 23:42:19]

Would you be okay if I added a new public API that returns a GpuMemoryBuffer
pointer? 

[no sievers, 2013/04/26 00:17:43]

You don't need a full-fledged GL API. This is only used from inside WGC3DImpl
and your test. Both can get to the GLES2Implementation (the test can get it
through GLManager). So just expose it from GLES2Implementation only.

[kaanb1, 2013/04/26 03:01:21]

Looks like the test doesn't build when I just expose it from
GLES2Implementation, is there anything I'm missing? Or should I just add a new
command like I did for imageBufferDataCHROMIUM

+  }
+
   helper_->SetBucketSize(kResultBucketId, 0);
   helper_->GetString(name, kResultBucketId);
   std::string str;
   if (GetBucketAsString(kResultBucketId, &str)) {
     // Adds extensions implemented on client side only.
     switch (name) {
       case GL_EXTENSIONS:
         str += std::string(str.empty() ? "" : " ") +
             "GL_CHROMIUM_flipy "
             "GL_CHROMIUM_map_sub "

[greggman, 2013/04/25 23:45:34]

You should probably add a string here based on which GpuMemoryBufferFactory
you've attached

If a factory exists then append the string 

  "GL_CHROMIUM_gpu_memory_buffer"

[kaanb1, 2013/04/26 03:01:21]

Done.

             "GL_CHROMIUM_shallow_flush "
             "GL_EXT_unpack_subimage";
         break;
       default:
         break;
     }
 
     // Because of WebGL the extensions can change. We have to cache each unique
     // result since we don't know when the client will stop referring to a
     // previous one it queries.
@@ skipping 254 matching lines @@
       break;
     case GL_ELEMENT_ARRAY_BUFFER:
       changed = vertex_array_object_manager_->BindElementArray(buffer);
       break;
     case GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM:
       bound_pixel_pack_transfer_buffer_id_ = buffer;
       break;
     case GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM:
       bound_pixel_unpack_transfer_buffer_id_ = buffer;
       break;
+    case GL_IMAGE_BUFFER_CHROMIUM:
+      bound_gpu_memory_buffer_id_ = buffer;
+      break;
     default:
       changed = true;
       break;
   }
   // TODO(gman): There's a bug here. If the target is invalid the ID will not be
   // used even though it's marked it as used here.
   GetIdHandler(id_namespaces::kBuffers)->MarkAsUsedForBind(buffer);
   return changed;
 }
 
@@ skipping 124 matching lines @@
     BufferTracker::Buffer* buffer = buffer_tracker_->GetBuffer(buffers[ii]);
     if (buffer) {
       // Free buffer memory, pending the passage of a token.
       buffer_tracker_->FreePendingToken(buffer, helper_->InsertToken());
       // Remove buffer.
       buffer_tracker_->RemoveBuffer(buffers[ii]);
     }
     if (buffers[ii] == bound_pixel_unpack_transfer_buffer_id_) {
       bound_pixel_unpack_transfer_buffer_id_ = 0;
     }
+    if (buffers[ii] == bound_gpu_memory_buffer_id_) {
+      bound_gpu_memory_buffer_id_ = 0;
+    }
+    GpuMemoryBuffer* gpu_buffer =
+        gpu_memory_buffer_tracker_->GetBuffer(buffers[ii]);
+    if (gpu_buffer) {
+      gpu_memory_buffer_tracker_->RemoveBuffer(buffers[ii]);
+    }
   }
 }
 
 void GLES2Implementation::DeleteBuffersStub(
     GLsizei n, const GLuint* buffers) {
   helper_->DeleteBuffersImmediate(n, buffers);
 }
 
 
 void GLES2Implementation::DeleteFramebuffersHelper(
@@ skipping 925 matching lines @@
         SetGLError(GL_INVALID_ENUM, "glMapBufferCHROMIUM", "bad access mode");
         return NULL;
       }
       break;
     case GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM:
       if (access != GL_WRITE_ONLY) {
         SetGLError(GL_INVALID_ENUM, "glMapBufferCHROMIUM", "bad access mode");
         return NULL;
       }
       break;
+    case GL_IMAGE_BUFFER_CHROMIUM:
+      break;
     default:
       SetGLError(
           GL_INVALID_ENUM, "glMapBufferCHROMIUM", "invalid target");
       return NULL;
   }
+
   GLuint buffer_id;
+  if (target == GL_IMAGE_BUFFER_CHROMIUM &&
+      GetBoundGpuMemoryBuffer("glMapBufferCHROMIUM", &buffer_id)) {

[greggman, 2013/04/25 23:45:34]

If GetBoundGPUMemoryBuffer returns false (assuming you meant it to return falls
on error), then it just falls through to GetBoundPixelBuffer. Is that correct?

[kaanb1, 2013/04/26 03:01:21]

Converted this function to use a switch statement.

+    if (buffer_id == 0) {
+      return NULL;
+    }
+
+    GpuMemoryBuffer* gpu_buffer =
+        gpu_memory_buffer_tracker_->GetBuffer(buffer_id);
+    if (!gpu_buffer) {
+      SetGLError(GL_INVALID_OPERATION,
+                 "glMapBufferCHROMIUM",
+                 "invalid GPU memory buffer");
+      return NULL;
+    }
+    GpuMemoryBuffer::AccessMode mode = GpuMemoryBuffer::READ_ONLY;
+    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?
+      default:
+        SetGLError(
+            GL_INVALID_ENUM, "glMapBufferCHROMIUM", "invalid GPU access mode");
+        return NULL;
+    }
+
+    void* mapped_buffer = NULL;
+    gpu_buffer->Map(mode, &mapped_buffer);

[greggman, 2013/04/25 23:45:34]

The semantics of map buffer require that you can only map once, otherwise
generate GL_INVALID_OPERATION

In other words calling glMapBuffer twice for the same buffer should fail. The
buffer must be unmapped before mapping it again.

[kaanb1, 2013/04/26 03:01:21]

Done.

+    CheckGLError();
+    return mapped_buffer;
+  }
+
   GetBoundPixelTransferBuffer(target, "glMapBufferCHROMIUM", &buffer_id);
   if (!buffer_id) {
     return NULL;
   }
   BufferTracker::Buffer* buffer = buffer_tracker_->GetBuffer(buffer_id);
   if (!buffer) {
     SetGLError(GL_INVALID_OPERATION, "glMapBufferCHROMIUM", "invalid buffer");
     return NULL;
   }
   if (buffer->mapped()) {
@@ skipping 15 matching lines @@
   GPU_CLIENT_LOG("  returned " << buffer->address());
   CheckGLError();
   return buffer->address();
 }
 
 GLboolean GLES2Implementation::UnmapBufferCHROMIUM(GLuint target) {
   GPU_CLIENT_SINGLE_THREAD_CHECK();
   GPU_CLIENT_LOG(
       "[" << GetLogPrefix() << "] glUnmapBufferCHROMIUM(" << target << ")");
   GLuint buffer_id;
+  if (target == GL_IMAGE_BUFFER_CHROMIUM &&
+      GetBoundGpuMemoryBuffer("glUnmapBufferCHROMIUM", &buffer_id)) {

[greggman, 2013/04/25 23:45:34]

Same here. If GetBoundGpuMemroyBuffer returns false (assuming it's supposed to
return false) then it doesn't seem like you want it to fall through?

Actually, looking at the fall through works but it seems fragile to me. Maybe
I'm being dumb.

I'd think it might be clearer if the code here at in MapBuffer was more like
either

   switch (target) {
     case GL_IMAGE_BUFFER_CHROMIUM:
       ...
       break;
     case GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM:
     case GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM:
       ...
       break;
     default:
       SetGLError(..., INVALID_ENUM, ...)

Or at least

   if (target == GL_IMAGE_BUFFER_CHROMIUM) {
     ...

     return;  // don't let the code follow through since it can't
              // possibly be interested in anything since we 
              // know the target is incorrect for any code below.
   }

What do you think? I think I'd prefer the switch as far as readability goes.

[kaanb1, 2013/04/26 03:01:21]

Done.

+    if (buffer_id == 0) {
+      return false;
+    }
+
+    GpuMemoryBuffer* gpu_buffer =
+        gpu_memory_buffer_tracker_->GetBuffer(buffer_id);
+
+    gpu_buffer->Unmap();

[greggman, 2013/04/25 23:45:34]

The semantics of map buffer require if the buffer is not mapped this should
generate GL_INVALID_OPERATION.

[kaanb1, 2013/04/26 03:01:21]

Done.

+    return true;
+  }
+
   if (!GetBoundPixelTransferBuffer(target, "glMapBufferCHROMIUM", &buffer_id)) {
     SetGLError(GL_INVALID_ENUM, "glUnmapBufferCHROMIUM", "invalid target");
   }
   if (!buffer_id) {
     return false;
   }
   BufferTracker::Buffer* buffer = buffer_tracker_->GetBuffer(buffer_id);
   if (!buffer) {
     SetGLError(GL_INVALID_OPERATION, "glMapBufferCHROMIUM", "invalid buffer");
     return false;
@@ skipping 112 matching lines @@
   helper_->WaitAsyncTexImage2DCHROMIUM(target);
   CheckGLError();
 }
 
 GLuint GLES2Implementation::InsertSyncPointCHROMIUM() {
   GPU_CLIENT_SINGLE_THREAD_CHECK();
   GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glInsertSyncPointCHROMIUM");
   return helper_->InsertSyncPointCHROMIUM();
 }
 
+void* GLES2Implementation::GetNativeHandleForBoundGpuMemoryBuffer() {
+  if (bound_gpu_memory_buffer_id_ == 0) {
+    return NULL;
+  }
+
+  GpuMemoryBuffer* buffer =
+      gpu_memory_buffer_tracker_->GetBuffer(bound_gpu_memory_buffer_id_);
+
+  return (buffer != NULL) ? buffer->GetNativeBuffer() : NULL;
+}
+
+void GLES2Implementation::ImageBufferDataHelperCHROMIUM(
+    GLenum target, GLsizei width, GLsizei height) {
+  if (width < 0) {
+    SetGLError(GL_INVALID_VALUE, "glImageBufferDataCHROMIUM", "width < 0");
+    return;
+  }
+
+  if (height < 0) {
+    SetGLError(GL_INVALID_VALUE, "glImageBufferDataCHROMIUM", "height < 0");
+    return;
+  }
+
+  if (target != GL_IMAGE_BUFFER_CHROMIUM) {
+    SetGLErrorInvalidEnum("glImageBufferDataCHROMIUM", target, "target");
+    return;
+  }
+
+  GLuint buffer_id;
+  bool found_buffer = GetBoundGpuMemoryBuffer(
+      "glImageBufferDataCHROMIUM", &buffer_id);
+
+  if (!found_buffer || buffer_id == 0) {
+    return;
+  }
+
+  GpuMemoryBuffer* buffer =
+      gpu_memory_buffer_tracker_->GetBuffer(buffer_id);
+  if (buffer) {
+    // Remove old buffer, deletes the buffer pointer.
+    gpu_memory_buffer_tracker_->RemoveBuffer(buffer_id);
+  }
+
+  if (width != 0 && height != 0) {
+    // Create new buffer.
+    buffer = gpu_memory_buffer_tracker_->CreateBuffer(buffer_id, width, height);
+    GPU_DCHECK(buffer);
+  }
+
+  return;
+}
+
+void GLES2Implementation::ImageBufferDataCHROMIUM(
+    GLenum target, GLsizei width, GLsizei height) {
+  GPU_CLIENT_SINGLE_THREAD_CHECK();
+  GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glImageBufferDataCHROMIUM("
+      << GLES2Util::GetStringBufferTarget(target) << ", "
+      << width << ", "
+      << height << ")");
+  ImageBufferDataHelperCHROMIUM(target, width, height);
+  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.
 #include "../client/gles2_implementation_impl_autogen.h"
 
 }  // namespace gles2
 }  // namespace gpu