Improve BindBufferBase/BindBufferRange in GPU command buffer.

gpu/command_buffer/service/gles2_cmd_decoder.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.
 
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 
 #include <limits.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <stdio.h>
@@ skipping 711 matching lines @@
   friend class ScopedFrameBufferReadPixelHelper;
   friend class ScopedResolvedFrameBufferBinder;
   friend class BackFramebuffer;
 
   enum FramebufferOperation {
     kFramebufferDiscard,
     kFramebufferInvalidate,
     kFramebufferInvalidateSub
   };
 
+  enum BindIndexedBufferFunctionType {
+    kBindBufferBase,
+    kBindBufferRange
+  };
+
   // Initialize or re-initialize the shader translator.
   bool InitializeShaderTranslator();
 
   void UpdateCapabilities();
 
   // Helpers for the glGen and glDelete functions.
   bool GenTexturesHelper(GLsizei n, const GLuint* client_ids);
   void DeleteTexturesHelper(GLsizei n, const GLuint* client_ids);
   bool GenBuffersHelper(GLsizei n, const GLuint* client_ids);
   void DeleteBuffersHelper(GLsizei n, const GLuint* client_ids);
@@ skipping 649 matching lines @@
   // Wrapper for glBindBuffer since we need to track the current targets.
   void DoBindBuffer(GLenum target, GLuint buffer);
 
   // Wrapper for glBindBufferBase since we need to track the current targets.
   void DoBindBufferBase(GLenum target, GLuint index, GLuint buffer);
 
   // Wrapper for glBindBufferRange since we need to track the current targets.
   void DoBindBufferRange(GLenum target, GLuint index, GLuint buffer,
       GLintptr offset, GLsizeiptr size);
 
+  // Helper for DoBindBufferBase and DoBindBufferRange.
+  void BindIndexedBufferImpl(GLenum target, GLuint index, GLuint buffer,
+                             GLintptr offset, GLsizeiptr size,
+                             BindIndexedBufferFunctionType function_type,
+                             const char* function_name);
+
   // Wrapper for glBindFramebuffer since we need to track the current targets.
   void DoBindFramebuffer(GLenum target, GLuint framebuffer);
 
   // Wrapper for glBindRenderbuffer since we need to track the current targets.
   void DoBindRenderbuffer(GLenum target, GLuint renderbuffer);
 
   // Wrapper for glBindTexture since we need to track the current targets.
   void DoBindTexture(GLenum target, GLuint texture);
 
   // Wrapper for glBindSampler since we need to track the current targets.
@@ skipping 3234 matching lines @@
           GL_INVALID_OPERATION,
           "glBindBuffer", "buffer bound to more than 1 target");
       return;
     }
     service_id = buffer->service_id();
   }
   state_.SetBoundBuffer(target, buffer);
   glBindBuffer(target, service_id);
 }
 
-void GLES2DecoderImpl::DoBindBufferBase(GLenum target, GLuint index,
-                                        GLuint client_id) {
-  Buffer* buffer = NULL;
+void GLES2DecoderImpl::BindIndexedBufferImpl(
+    GLenum target, GLuint index, GLuint client_id,
+    GLintptr offset, GLsizeiptr size,
+    BindIndexedBufferFunctionType function_type, const char* function_name) {
+  switch (target) {
+    case GL_TRANSFORM_FEEDBACK_BUFFER: {
+      if (index >= group_->max_transform_feedback_separate_attribs()) {
+        LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name,
+                           "index out of range");
+        return;
+      }
+      // TODO(zmo): Check transform feedback isn't currently active.
+      break;
+    }
+    case GL_UNIFORM_BUFFER: {
+      if (index >= group_->max_uniform_buffer_bindings()) {
+        LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name,
+                           "index out of range");
+        return;
+      }
+      break;
+    }
+    default:
+      LOCAL_SET_GL_ERROR_INVALID_ENUM(function_name, target, "invalid target");
+      return;
+  }
+
+  if (function_type == kBindBufferRange) {
+    switch (target) {
+      case GL_TRANSFORM_FEEDBACK_BUFFER:
+        if ((size % 4 != 0) || (offset % 4 != 0)) {
+          LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name,
+                             "size or offset are not multiples of 4");
+          return;
+        }
+        break;
+      case GL_UNIFORM_BUFFER: {
+        if (offset % group_->uniform_buffer_offset_alignment() != 0) {
+          LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name,
+              "offset is not a multiple of UNIFORM_BUFFER_OFFSET_ALIGNMENT");
+          return;
+        }
+        break;
+      }
+      default:
+        NOTREACHED();
+        break;
+    }
+
+    if (client_id != 0) {
+      if (size <= 0) {
+        LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "size <= 0");
+        return;
+      }
+      if (offset < 0) {
+        LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "offset < 0");
+        return;
+      }
+    }
+  }
+
+  Buffer* buffer = nullptr;
   GLuint service_id = 0;
   if (client_id != 0) {
     buffer = GetBuffer(client_id);
     if (!buffer) {
       if (!group_->bind_generates_resource()) {
-        LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION,
-                           "glBindBufferBase",
+        LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, function_name,
                            "id not generated by glGenBuffers");
         return;
       }
-
-      // It's a new id so make a buffer for it.
-      glGenBuffersARB(1, &service_id);
-      CreateBuffer(client_id, service_id);
-      buffer = GetBuffer(client_id);
-    }
-  }
-  LogClientServiceForInfo(buffer, client_id, "glBindBufferBase");
-  if (buffer) {
-    // TODO(kbr): track indexed bound buffers.
-    service_id = buffer->service_id();
-  }
-  switch (target) {
-    case GL_TRANSFORM_FEEDBACK_BUFFER: {
-      GLint max_transform_feedback_separate_attribs = 0;
-      DoGetIntegerv(GL_MAX_TRANSFORM_FEEDBACK_SEPARATE_ATTRIBS,
-                    &max_transform_feedback_separate_attribs);
-      if (index >=
-          static_cast<GLuint>(max_transform_feedback_separate_attribs)) {
-        LOCAL_SET_GL_ERROR(GL_INVALID_VALUE,
-                           "glBindBufferBase", "index out of range");
-        return;
-      }
-      break;
-    }
-    case GL_UNIFORM_BUFFER: {
-      GLint max_uniform_buffer_bindings = 0;
-      DoGetIntegerv(GL_MAX_UNIFORM_BUFFER_BINDINGS,
-                    &max_uniform_buffer_bindings);
-      if (index >= static_cast<GLuint>(max_uniform_buffer_bindings)) {
-        LOCAL_SET_GL_ERROR(GL_INVALID_VALUE,
-                           "glBindBufferBase", "index out of range");
-        return;
-      }
-      break;
-    }
-    default:
-      LOCAL_SET_GL_ERROR_INVALID_ENUM(
-          "glBindBufferBase", target, "invalid target");
-      return;
-  }
-  state_.SetBoundBuffer(target, buffer);
-  glBindBufferBase(target, index, service_id);
-}
-
-void GLES2DecoderImpl::DoBindBufferRange(GLenum target, GLuint index,
-                                         GLuint client_id,
-                                         GLintptr offset,
-                                         GLsizeiptr size) {
-  Buffer* buffer = NULL;
-  GLuint service_id = 0;
-  if (client_id != 0) {
-    buffer = GetBuffer(client_id);
-    if (!buffer) {
-      if (!group_->bind_generates_resource()) {
-        LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION,
-                           "glBindBufferRange",
-                           "id not generated by glGenBuffers");
-        return;
-      }
 
       // It's a new id so make a buffer for it.
       glGenBuffersARB(1, &service_id);
       CreateBuffer(client_id, service_id);
       buffer = GetBuffer(client_id);
+      DCHECK(buffer);
     }
-  }
-  LogClientServiceForInfo(buffer, client_id, "glBindBufferRange");
-  if (buffer) {
-    // TODO(kbr): track indexed bound buffers.
     service_id = buffer->service_id();
   }
-  glBindBufferRange(target, index, service_id, offset, size);
+  LogClientServiceForInfo(buffer, client_id, function_name);
+
+  switch (function_type) {
+    case kBindBufferBase:
+      glBindBufferBase(target, index, service_id);
+      break;
+    case kBindBufferRange:
+      // On Desktop GL 4.1 or lower, clamp the offset/size not to exceed the

[piman, 2016/04/20 21:14:35]

nit: is that a TODO? Mark it so (this reads as if we *are* doing it, but we
aren't).

[Zhenyao Mo, 2016/04/20 22:08:50]

Done.

+      // size of the buffer. crbug.com/604436.
+      glBindBufferRange(target, index, service_id, offset, size);
+      break;
+  }
+  // TODO(kbr): track indexed bound buffers.
+}
+
+void GLES2DecoderImpl::DoBindBufferBase(GLenum target, GLuint index,
+                                        GLuint client_id) {
+  BindIndexedBufferImpl(target, index, client_id, 0, 0,
+                        kBindBufferBase, "glBindBufferBase");
+}
+
+void GLES2DecoderImpl::DoBindBufferRange(GLenum target, GLuint index,
+                                         GLuint client_id,
+                                         GLintptr offset,
+                                         GLsizeiptr size) {
+  BindIndexedBufferImpl(target, index, client_id, offset, size,
+                        kBindBufferRange, "glBindBufferRange");
 }
 
 bool GLES2DecoderImpl::BoundFramebufferHasColorAttachmentWithAlpha() {
   Framebuffer* framebuffer =
       GetFramebufferInfoForTarget(GL_DRAW_FRAMEBUFFER_EXT);
   if (framebuffer)
     return framebuffer->HasAlphaMRT();
   return BackBufferHasAlpha();
 }
 
@@ skipping 11761 matching lines @@
 }
 
 // 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 "base/macros.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder_autogen.h"
 
 }  // namespace gles2
 }  // namespace gpu