Re: [Mesa-dev] [PATCH 10/10] mesa: add KHR_no_error support for glBindBufferRange()

Timothy Arceri Tue, 23 May 2017 02:31:28 -0700

On 23/05/17 18:48, Iago Toral wrote:

On Mon, 2017-05-22 at 15:47 +1000, Timothy Arceri wrote:

---
  src/mapi/glapi/gen/GL3x.xml |   2 +-
  src/mesa/main/bufferobj.c   | 103 ++++++++++++++++++++++++++++----
------------
  src/mesa/main/bufferobj.h   |   3 ++
  3 files changed, 70 insertions(+), 38 deletions(-)


diff --git a/src/mapi/glapi/gen/GL3x.xml
b/src/mapi/glapi/gen/GL3x.xml
index 10c157e..f488ba3 100644
--- a/src/mapi/glapi/gen/GL3x.xml
+++ b/src/mapi/glapi/gen/GL3x.xml
@@ -206,21 +206,21 @@
      <param name="name" type="const GLchar *"/>
    </function>

<function name="BeginTransformFeedback" es2="3.0">

      <param name="mode" type="GLenum"/>
    </function>

<function name="EndTransformFeedback" es2="3.0">

    </function>

- <function name="BindBufferRange" es2="3.0">

+  <function name="BindBufferRange" es2="3.0" no_error="true">
      <param name="target" type="GLenum"/>
      <param name="index" type="GLuint"/>
      <param name="buffer" type="GLuint"/>
      <param name="offset" type="GLintptr"/>
      <param name="size" type="GLsizeiptr"/>
    </function>

<function name="BindBufferBase" es2="3.0">

      <param name="target" type="GLenum"/>
      <param name="index" type="GLuint"/>
diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
index 5aae579..9e656a4 100644
--- a/src/mesa/main/bufferobj.c
+++ b/src/mesa/main/bufferobj.c
@@ -3986,87 +3986,116 @@ bind_atomic_buffers(struct gl_context *ctx,

if (bufObj)

           set_atomic_buffer_binding(ctx, binding, bufObj, offset,
size);
     }

_mesa_HashUnlockMutex(ctx->Shared->BufferObjects);

static ALWAYS_INLINE void

  bind_buffer_range(GLenum target, GLuint index, GLuint buffer,
GLintptr offset,
-                  GLsizeiptr size)
+                  GLsizeiptr size, bool no_error)
  {
     GET_CURRENT_CONTEXT(ctx);
     struct gl_buffer_object *bufObj;

if (MESA_VERBOSE & VERBOSE_API) {

        _mesa_debug(ctx, "glBindBufferRange(%s, %u, %u, %lu, %lu)\n",
                    _mesa_enum_to_string(target), index, buffer,
                    (unsigned long) offset, (unsigned long) size);
     }

if (buffer == 0) {

        bufObj = ctx->Shared->NullBufferObj;
     } else {
        bufObj = _mesa_lookup_bufferobj(ctx, buffer);
     }
     if (!_mesa_handle_bind_buffer_gen(ctx, buffer,
                                       &bufObj, "glBindBufferRange"))


It seems that _mesa_handle_bind_buffer_gen() can still generate
INVALID_OPERATION but I guess we seem to avoid that scenario herewith
the if (buffer == 0) above...

Anyway, would it make sense to add a no_error parameter to that
function too? Otherwise it might be possible to mess things up in the
future if people move or refactor code around I guess.

Hi, firstly thanks for the review. The problem is that here all theno_error checks are optimised away because we set a compiler flag toforce inlining. Adding a no_error check to_mesa_handle_bind_buffer_gen() will add an extra if to avoid an if soits not worth doing.

The idea is to lower overhead by removing error checks, in most casesI've been able to do so fairly cleanly but this was one check that was abit tricker so I left as is. It's possible we could do some inlining butwe need to be careful with that as it can bloat things for not muchbenefit. I'd rather leave this one as is, does that seem reasonable?

        return;

- if (!bufObj) {

-      _mesa_error(ctx, GL_INVALID_OPERATION,
-                  "glBindBufferRange(invalid buffer=%u)", buffer);
-      return;
-   }
-
-   if (buffer != 0) {
-      if (size <= 0) {
-         _mesa_error(ctx, GL_INVALID_VALUE,
"glBindBufferRange(size=%d)",
-                     (int) size);
+   if (no_error) {
+      switch (target) {
+      case GL_TRANSFORM_FEEDBACK_BUFFER:
+         _mesa_bind_buffer_range_xfb(ctx, ctx-

TransformFeedback.CurrentObject,

+                                     index, bufObj, offset, size);
           return;
+      case GL_UNIFORM_BUFFER:
+         bind_buffer_range_uniform_buffer(ctx, index, bufObj,
offset, size);
+         return;
+      case GL_SHADER_STORAGE_BUFFER:
+         bind_buffer_range_shader_storage_buffer(ctx, index, bufObj,
offset,
+                                                 size);
+         return;
+      case GL_ATOMIC_COUNTER_BUFFER:
+         bind_atomic_buffer(ctx, index, bufObj, offset, size);
+         return;
+      default:
+         unreachable("invalid BindBufferRange target with
KHR_no_error");
        }
-   }
-
-   switch (target) {
-   case GL_TRANSFORM_FEEDBACK_BUFFER:
-      if (!_mesa_validate_buffer_range_xfb(ctx,
-                                           ctx-

TransformFeedback.CurrentObject,

-                                           index, bufObj, offset,
size,
-                                           false))
+   } else {
+      if (!bufObj) {
+         _mesa_error(ctx, GL_INVALID_OPERATION,
+                     "glBindBufferRange(invalid buffer=%u)",
buffer);
           return;
+      }

- _mesa_bind_buffer_range_xfb(ctx, ctx-

TransformFeedback.CurrentObject,

-                                  index, bufObj, offset, size);
-      return;
-   case GL_UNIFORM_BUFFER:
-      bind_buffer_range_uniform_buffer_err(ctx, index, bufObj,
offset, size);
-      return;
-   case GL_SHADER_STORAGE_BUFFER:
-      bind_buffer_range_shader_storage_buffer_err(ctx, index,
bufObj, offset,
-                                                  size);
-      return;
-   case GL_ATOMIC_COUNTER_BUFFER:
-      bind_atomic_buffer_err(ctx, index, bufObj, offset, size,
-                             "glBindBufferRange");
-      return;
-   default:
-      _mesa_error(ctx, GL_INVALID_ENUM,
"glBindBufferRange(target)");
-      return;
+      if (buffer != 0) {
+         if (size <= 0) {
+            _mesa_error(ctx, GL_INVALID_VALUE,
"glBindBufferRange(size=%d)",
+                        (int) size);
+            return;
+         }
+      }
+
+      switch (target) {
+      case GL_TRANSFORM_FEEDBACK_BUFFER:
+         if (!_mesa_validate_buffer_range_xfb(ctx,
+                                              ctx-

TransformFeedback.CurrentObject,

+                                              index, bufObj, offset,
size,
+                                              false))
+            return;
+
+         _mesa_bind_buffer_range_xfb(ctx, ctx-

TransformFeedback.CurrentObject,

+                                     index, bufObj, offset, size);
+         return;
+      case GL_UNIFORM_BUFFER:
+         bind_buffer_range_uniform_buffer_err(ctx, index, bufObj,
offset,
+                                              size);
+         return;
+      case GL_SHADER_STORAGE_BUFFER:
+         bind_buffer_range_shader_storage_buffer_err(ctx, index,
bufObj,
+                                                     offset, size);
+         return;
+      case GL_ATOMIC_COUNTER_BUFFER:
+         bind_atomic_buffer_err(ctx, index, bufObj, offset, size,
+                                "glBindBufferRange");
+         return;
+      default:
+         _mesa_error(ctx, GL_INVALID_ENUM,
"glBindBufferRange(target)");
+         return;
+      }
     }
  }

void GLAPIENTRY

+_mesa_BindBufferRange_no_error(GLenum target, GLuint index, GLuint
buffer,
+                               GLintptr offset, GLsizeiptr size)
+{
+   bind_buffer_range(target, index, buffer, offset, size, true);
+}
+
+void GLAPIENTRY
  _mesa_BindBufferRange(GLenum target, GLuint index,
                        GLuint buffer, GLintptr offset, GLsizeiptr
size)
  {
-   bind_buffer_range(target, index, buffer, offset, size);
+   bind_buffer_range(target, index, buffer, offset, size, false);
  }

void GLAPIENTRY

  _mesa_BindBufferBase(GLenum target, GLuint index, GLuint buffer)
  {
     GET_CURRENT_CONTEXT(ctx);
     struct gl_buffer_object *bufObj;

if (MESA_VERBOSE & VERBOSE_API) {

        _mesa_debug(ctx, "glBindBufferBase(%s, %u, %u)\n",
diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h
index f652ec3..ff44fed 100644
--- a/src/mesa/main/bufferobj.h
+++ b/src/mesa/main/bufferobj.h
@@ -315,20 +315,23 @@ _mesa_FlushMappedBufferRange(GLenum target,
                               GLintptr offset, GLsizeiptr length);

void GLAPIENTRY

  _mesa_FlushMappedNamedBufferRange_no_error(GLuint buffer, GLintptr
offset,
                                             GLsizeiptr length);
  void GLAPIENTRY
  _mesa_FlushMappedNamedBufferRange(GLuint buffer, GLintptr offset,
                                    GLsizeiptr length);

void GLAPIENTRY

+_mesa_BindBufferRange_no_error(GLenum target, GLuint index, GLuint
buffer,
+                               GLintptr offset, GLsizeiptr size);
+void GLAPIENTRY
  _mesa_BindBufferRange(GLenum target, GLuint index,
                        GLuint buffer, GLintptr offset, GLsizeiptr
size);

void GLAPIENTRY

  _mesa_BindBufferBase(GLenum target, GLuint index, GLuint buffer);

void GLAPIENTRY

  _mesa_BindBuffersRange(GLenum target, GLuint first, GLsizei count,
                         const GLuint *buffers,
                         const GLintptr *offsets, const GLsizeiptr
*sizes);

_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/10] mesa: add KHR_no_error support for glBindBufferRange()

Reply via email to