Implement client side PBOs for glReadPixel

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>
@@ skipping 78 matching lines @@
       unpack_row_length_(0),
       unpack_skip_rows_(0),
       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),
       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];
@@ skipping 473 matching lines @@
         return true;
       }
       return false;
     case GL_ELEMENT_ARRAY_BUFFER_BINDING:
       if (share_group_->bind_generates_resource()) {
         *params =
             vertex_array_object_manager_->bound_element_array_buffer();
         return true;
       }
       return false;
+    case GL_PIXEL_PACK_TRANSFER_BUFFER_BINDING_CHROMIUM:
+      *params = bound_pixel_pack_transfer_buffer_id_;
+      return true;
     case GL_PIXEL_UNPACK_TRANSFER_BUFFER_BINDING_CHROMIUM:
       *params = bound_pixel_unpack_transfer_buffer_id_;
       return true;
     case GL_ACTIVE_TEXTURE:
       *params = active_texture_unit_ + GL_TEXTURE0;
       return true;
     case GL_TEXTURE_BINDING_2D:
       if (share_group_->bind_generates_resource()) {
         *params = texture_units_[active_texture_unit_].bound_texture_2d;
         return true;
@@ skipping 639 matching lines @@
   CheckGLError();
 }
 
 void GLES2Implementation::BufferDataHelper(
     GLenum target, GLsizeiptr size, const void* data, GLenum usage) {
   if (size < 0) {
     SetGLError(GL_INVALID_VALUE, "glBufferData", "size < 0");
     return;
   }
 
-  if (target == GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM) {
-    GLuint buffer_id = bound_pixel_unpack_transfer_buffer_id_;
-    if (!buffer_id) {
-      SetGLError(GL_INVALID_VALUE, "glBufferData", "unknown buffer");
-      return;
-    }
+  GLuint buffer_id;
+  if (GetBoundPixelTransferBuffer(target, "glBufferData", &buffer_id)) {
+    if (!buffer_id) return;
 
     BufferTracker::Buffer* buffer = buffer_tracker_->GetBuffer(buffer_id);
     if (buffer) {
       // Free buffer memory, pending the passage of a token.
       buffer_tracker_->FreePendingToken(buffer, helper_->InsertToken());
 
       // Remove old buffer.
       buffer_tracker_->RemoveBuffer(buffer_id);
     }
 
@@ skipping 54 matching lines @@
     GLenum target, GLintptr offset, GLsizeiptr size, const void* data) {
   if (size == 0) {
     return;
   }
 
   if (size < 0) {
     SetGLError(GL_INVALID_VALUE, "glBufferSubData", "size < 0");
     return;
   }
 
-  if (target == GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM) {
-    BufferTracker::Buffer* buffer = buffer_tracker_->GetBuffer(
-        bound_pixel_unpack_transfer_buffer_id_);
+  GLuint buffer_id;
+  if (GetBoundPixelTransferBuffer(target, "glBufferSubData", &buffer_id)) {
+    if (!buffer_id) return;
+    BufferTracker::Buffer* buffer = buffer_tracker_->GetBuffer(buffer_id);
     if (!buffer) {
       SetGLError(GL_INVALID_VALUE, "glBufferSubData", "unknown buffer");
       return;
     }
 
     int32 end = 0;
     int32 buffer_size = buffer->size();
     if (!SafeAddInt32(offset, size, &end) || end > buffer_size) {
       SetGLError(GL_INVALID_VALUE, "glBufferSubData", "out of range");
       return;
@@ skipping 36 matching lines @@
     GLenum target, GLintptr offset, GLsizeiptr size, const void* data) {
   GPU_CLIENT_SINGLE_THREAD_CHECK();
   GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glBufferSubData("
       << GLES2Util::GetStringBufferTarget(target) << ", "
       << offset << ", " << size << ", "
       << static_cast<const void*>(data) << ")");
   BufferSubDataHelper(target, offset, size, data);
   CheckGLError();
 }
 
+bool GLES2Implementation::GetBoundPixelTransferBuffer(
+    GLenum target,
+    const char* function_name,
+    GLuint* buffer_id) {
+  *buffer_id = 0;
+
+  switch (target) {
+    case GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM:
+      *buffer_id = bound_pixel_pack_transfer_buffer_id_;
+      break;
+    case GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM:
+      *buffer_id = bound_pixel_pack_transfer_buffer_id_;
+      break;
+    default:
+      // Unknown target
+      return false;
+  }
+  if (!*buffer_id) {
+    SetGLError(GL_INVALID_VALUE, function_name, "no buffer bound");
+  }
+  return true;
+}
+
 BufferTracker::Buffer*
 GLES2Implementation::GetBoundPixelUnpackTransferBufferIfValid(
-    const char* function_name, GLuint offset, GLsizei size)
+    GLuint buffer_id,
+    const char* function_name,
+    GLuint offset, GLsizei size)
 {
-  BufferTracker::Buffer* buffer = buffer_tracker_->GetBuffer(
-      bound_pixel_unpack_transfer_buffer_id_);
-  if (!buffer) {
-    SetGLError(GL_INVALID_OPERATION, function_name, "invalid buffer");
-    return NULL;
-  }
+  DCHECK(buffer_id);
+  BufferTracker::Buffer* buffer = buffer_tracker_->GetBuffer(buffer_id);
   if (buffer->mapped()) {

[greggman, 2013/03/20 00:42:17]

You removed the check for buffer being NULL but you're dereferencing it here?

[hubbe, 2013/03/20 22:03:47]

Done.

     SetGLError(GL_INVALID_OPERATION, function_name, "buffer mapped");
     return NULL;
   }
   if ((buffer->size() - offset) < static_cast<GLuint>(size)) {
     SetGLError(GL_INVALID_VALUE, function_name, "unpack size to large");
     return NULL;
   }
   return buffer;
 }
 
@@ skipping 13 matching lines @@
     return;
   }
   if (height == 0 || width == 0) {
     return;
   }
   // If there's a pixel unpack buffer bound use it when issuing
   // CompressedTexImage2D.
   if (bound_pixel_unpack_transfer_buffer_id_) {
     GLuint offset = ToGLuint(data);
     BufferTracker::Buffer* buffer = GetBoundPixelUnpackTransferBufferIfValid(
+        bound_pixel_unpack_transfer_buffer_id_,
         "glCompressedTexImage2D", offset, image_size);
-    if (buffer)
+    if (buffer) {
       helper_->CompressedTexImage2D(
           target, level, internalformat, width, height, border, image_size,
           buffer->shm_id(), buffer->shm_offset() + offset);
+      buffer->set_transfer_ready_token(helper_->InsertToken());
+    } else {
+      SetGLError(GL_INVALID_VALUE, "glCompressedTexImage2D", "invalid buffer");
+    }
     return;
   }
   SetBucketContents(kResultBucketId, data, image_size);
   helper_->CompressedTexImage2DBucket(
       target, level, internalformat, width, height, border, kResultBucketId);
   // Free the bucket. This is not required but it does free up the memory.
   // and we don't have to wait for the result so from the client's perspective
   // it's cheap.
   helper_->SetBucketSize(kResultBucketId, 0);
   CheckGLError();
@@ skipping 13 matching lines @@
       << static_cast<const void*>(data) << ")");
   if (width < 0 || height < 0 || level < 0) {
     SetGLError(GL_INVALID_VALUE, "glCompressedTexSubImage2D", "dimension < 0");
     return;
   }
   // If there's a pixel unpack buffer bound use it when issuing
   // CompressedTexSubImage2D.
   if (bound_pixel_unpack_transfer_buffer_id_) {
     GLuint offset = ToGLuint(data);
     BufferTracker::Buffer* buffer = GetBoundPixelUnpackTransferBufferIfValid(
+        bound_pixel_unpack_transfer_buffer_id_,
         "glCompressedTexSubImage2D", offset, image_size);
-    if (buffer)
+    if (buffer) {
       helper_->CompressedTexSubImage2D(
           target, level, xoffset, yoffset, width, height, format, image_size,
           buffer->shm_id(), buffer->shm_offset() + offset);
+      buffer->set_transfer_ready_token(helper_->InsertToken());

[greggman, 2013/03/20 00:42:17]

Add a CheckGLError here (was missing from before)

[hubbe, 2013/03/20 22:03:47]

Done.

+    } else {
+      SetGLError(GL_INVALID_VALUE,
+                 "glCompressedTexSubImage2D", "invalid buffer");
+    }
     return;
   }
   SetBucketContents(kResultBucketId, data, image_size);
   helper_->CompressedTexSubImage2DBucket(
       target, level, xoffset, yoffset, width, height, format, kResultBucketId);
   // Free the bucket. This is not required but it does free up the memory.
   // and we don't have to wait for the result so from the client's perspective
   // it's cheap.
   helper_->SetBucketSize(kResultBucketId, 0);
   CheckGLError();
@@ skipping 58 matching lines @@
           width, height, format, type, unpack_alignment_, &size,
           &unpadded_row_size, &padded_row_size)) {
     SetGLError(GL_INVALID_VALUE, "glTexImage2D", "image size too large");
     return;
   }
 
   // If there's a pixel unpack buffer bound use it when issuing TexImage2D.
   if (bound_pixel_unpack_transfer_buffer_id_) {
     GLuint offset = ToGLuint(pixels);
     BufferTracker::Buffer* buffer = GetBoundPixelUnpackTransferBufferIfValid(
+        bound_pixel_unpack_transfer_buffer_id_,
         "glTexImage2D", offset, size);
-    if (buffer)
+    if (buffer) {
       helper_->TexImage2D(
           target, level, internalformat, width, height, border, format, type,
           buffer->shm_id(), buffer->shm_offset() + offset);
-    CheckGLError();
+      buffer->set_transfer_ready_token(helper_->InsertToken());
+      CheckGLError();
+    } else {
+      SetGLError(GL_INVALID_VALUE, "glTexImage2D", "invalid buffer");
+    }
     return;
   }
 
   // If there's no data just issue TexImage2D
   if (!pixels) {
     helper_->TexImage2D(
        target, level, internalformat, width, height, border, format, type,
        0, 0);
     CheckGLError();
     return;
@@ skipping 77 matching lines @@
         width, height, format, type, unpack_alignment_, &temp_size,
         &unpadded_row_size, &padded_row_size)) {
     SetGLError(GL_INVALID_VALUE, "glTexSubImage2D", "size to large");
     return;
   }
 
   // If there's a pixel unpack buffer bound use it when issuing TexSubImage2D.
   if (bound_pixel_unpack_transfer_buffer_id_) {
     GLuint offset = ToGLuint(pixels);
     BufferTracker::Buffer* buffer = GetBoundPixelUnpackTransferBufferIfValid(
+        bound_pixel_unpack_transfer_buffer_id_,
         "glTexSubImage2D", offset, temp_size);
-    if (buffer)
+    if (buffer) {
       helper_->TexSubImage2D(
           target, level, xoffset, yoffset, width, height, format, type,
           buffer->shm_id(), buffer->shm_offset() + offset, false);
-    CheckGLError();
+      buffer->set_transfer_ready_token(helper_->InsertToken());
+      CheckGLError();
+    } else {
+      SetGLError(GL_INVALID_VALUE, "glTexSubImage2D", "invalid buffer");
+    }
     return;
   }
 
   // compute the advance bytes per row for the src pixels
   uint32 src_padded_row_size;
   if (unpack_row_length_ > 0) {
     if (!GLES2Util::ComputeImagePaddedRowSize(
         unpack_row_length_, format, type, unpack_alignment_,
         &src_padded_row_size)) {
       SetGLError(
@@ skipping 422 matching lines @@
   int8* dest = reinterpret_cast<int8*>(pixels);
   uint32 temp_size;
   uint32 unpadded_row_size;
   uint32 padded_row_size;
   if (!GLES2Util::ComputeImageDataSizes(
       width, 2, format, type, pack_alignment_, &temp_size, &unpadded_row_size,
       &padded_row_size)) {
     SetGLError(GL_INVALID_VALUE, "glReadPixels", "size too large.");
     return;
   }
+
+  if (bound_pixel_pack_transfer_buffer_id_) {
+    GLuint offset = ToGLuint(pixels);
+    BufferTracker::Buffer* buffer = GetBoundPixelUnpackTransferBufferIfValid(
+        bound_pixel_pack_transfer_buffer_id_,
+        "glReadPixels", offset, temp_size);
+    if (buffer) {
+      helper_->ReadPixels(xoffset, yoffset, width, height, format, type,
+                          buffer->shm_id(), buffer->shm_offset(),
+                          0, 0);
+      buffer->set_transfer_ready_token(helper_->InsertToken());
+      CheckGLError();
+    } else {
+      SetGLError(GL_INVALID_VALUE, "glReadPixels", "invalid readback buffer");
+    }
+    return;
+  }
+
+  if (!pixels) {
+    SetGLError(GL_INVALID_VALUE, "glReadPixels", "pixels = NULL");
+    return;
+  }
+
   // Transfer by rows.
   // The max rows we can transfer.
   while (height) {
     GLsizei desired_size = padded_row_size * height - 1 + unpadded_row_size;
     ScopedTransferBufferPtr buffer(desired_size, helper_, transfer_buffer_);
     if (!buffer.valid()) {
       return;
     }
     GLint num_rows = ComputeNumRowsThatFitInBuffer(
         padded_row_size, unpadded_row_size, buffer.size());
@@ skipping 97 matching lines @@
   switch (target) {
     case GL_ARRAY_BUFFER:
       if (bound_array_buffer_id_ != buffer) {
         bound_array_buffer_id_ = buffer;
         changed = true;
       }
       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;
     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);
@@ skipping 1059 matching lines @@
   }
   helper_->TraceEndCHROMIUM();
   TRACE_EVENT_COPY_ASYNC_END0("gpu", current_trace_name_->c_str(), this);
   current_trace_name_.reset();
 }
 
 void* GLES2Implementation::MapBufferCHROMIUM(GLuint target, GLenum access) {
   GPU_CLIENT_SINGLE_THREAD_CHECK();
   GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glMapBufferCHROMIUM("
       << target << ", " << GLES2Util::GetStringEnum(access) << ")");
-  if (target != GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM) {
-    SetGLError(
-        GL_INVALID_ENUM, "glMapBufferCHROMIUM", "invalid target");
-    return NULL;
+  switch (target)  {
+    case GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM:
+      if (access != GL_READ_ONLY) {
+        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;
+    default:
+      SetGLError(
+          GL_INVALID_ENUM, "glMapBufferCHROMIUM", "invalid target");
+      return NULL;
   }
-  if (access != GL_WRITE_ONLY) {
-    SetGLError(GL_INVALID_ENUM, "glMapBufferCHROMIUM", "bad access mode");
-    return NULL;
-  }
-  BufferTracker::Buffer* buffer = buffer_tracker_->GetBuffer(
-        bound_pixel_unpack_transfer_buffer_id_);
+  GLuint buffer_id;
+  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()) {
     SetGLError(GL_INVALID_OPERATION, "glMapBufferCHROMIUM", "already mapped");
     return NULL;
   }
+  if (buffer->transfer_ready_token()) {
+    helper_->WaitForToken(buffer->transfer_ready_token());
+    buffer->set_transfer_ready_token(0);
+  }
   buffer->set_mapped(true);
 
   GPU_DCHECK(buffer->address());
   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 << ")");
-  if (target != GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM) {
+  GLuint buffer_id;
+  if (!GetBoundPixelTransferBuffer(target, "glMapBufferCHROMIUM", &buffer_id)) {
     SetGLError(GL_INVALID_ENUM, "glUnmapBufferCHROMIUM", "invalid target");
-    return false;
   }
+  if (!buffer_id) return false;
   BufferTracker::Buffer* buffer = buffer_tracker_->GetBuffer(
       bound_pixel_unpack_transfer_buffer_id_);
   if (!buffer) {
     SetGLError(GL_INVALID_OPERATION, "glMapBufferCHROMIUM", "invalid buffer");
     return false;
   }
   if (!buffer->mapped()) {
     SetGLError(GL_INVALID_OPERATION, "glMapBufferCHROMIUM", "not mapped");
     return false;
   }
@@ skipping 22 matching lines @@
   uint32 size;
   uint32 unpadded_row_size;
   uint32 padded_row_size;
   if (!GLES2Util::ComputeImageDataSizes(
           width, height, format, type, unpack_alignment_, &size,
           &unpadded_row_size, &padded_row_size)) {
     SetGLError(GL_INVALID_VALUE, "glTexImage2D", "image size too large");
     return;
   }
 
   // If there's no data/buffer just issue the AsyncTexImage2D

[greggman, 2013/03/20 00:42:17]

Probably need at  least a TODO that async uploads need to block on MapBuffer if
they are still pending for the same buffer.

[hubbe, 2013/03/20 22:03:47]

What buffer is that?

[greggman, 2013/03/21 23:18:13]

The buffer referenced by bound_pixel_unpack_transfer_buffer_id_.
If someone calls AsyncTexImage2DCHROMIUM and then calls MapBufferCHROMIUM
there's no token inserted and it won't block so MapBufferCHROMIUM will not wait
like you've made it do for other use-cases. You can't use a token in this case
either so in order to make MapBufferCHROMIUM block until the async upload is
finished someone needs to add code. That's probably a lot of work so it would be
good to add a TODO somewhere, possibly in 3 places (1) here, (2) in
AsyncTexSubImage2D and (3) in MapBufferCHROMIUM that waiting for a token is not
sufficient to make sure the buffer is not in use.

[hubbe, 2013/03/21 23:36:51]

Done.

   if (!pixels && !bound_pixel_unpack_transfer_buffer_id_) {
     helper_->AsyncTexImage2DCHROMIUM(
        target, level, internalformat, width, height, border, format, type,
        0, 0);
     return;
   }
 
   // Otherwise, async uploads require a transfer buffer to be bound.
   GLuint offset = ToGLuint(pixels);
   BufferTracker::Buffer* buffer = GetBoundPixelUnpackTransferBufferIfValid(
+      bound_pixel_unpack_transfer_buffer_id_,
       "glAsyncTexImage2DCHROMIUM", offset, size);
   if (!buffer)
     return;
 
   helper_->AsyncTexImage2DCHROMIUM(
       target, level, internalformat, width, height, border, format, type,
       buffer->shm_id(), buffer->shm_offset() + offset);
   return;
 }
 
@@ skipping 22 matching lines @@
         width, height, format, type, unpack_alignment_, &size,
         &unpadded_row_size, &padded_row_size)) {
     SetGLError(
         GL_INVALID_VALUE, "glAsyncTexSubImage2DCHROMIUM", "size to large");
     return;
   }
 
   // Async uploads require a transfer buffer to be bound.
   GLuint offset = ToGLuint(pixels);
   BufferTracker::Buffer* buffer = GetBoundPixelUnpackTransferBufferIfValid(
+      bound_pixel_unpack_transfer_buffer_id_,
       "glAsyncTexSubImage2DCHROMIUM", offset, size);
   if (!buffer)
     return;
 
   helper_->AsyncTexSubImage2DCHROMIUM(
       target, level, xoffset, yoffset, width, height, format, type,
       buffer->shm_id(), buffer->shm_offset() + offset);
   return;
 }
 
@@ skipping 11 matching lines @@
   return helper_->InsertSyncPointCHROMIUM();
 }
 
 // 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