On Thursday 31 October 2013, Eric Anholt wrote:
> There was some spec text, and what there isn't text for we have obvious
> intended behavior from other buffer object bindings.
The first revision of the specification didn't say anything about it,
but the intent is indeed pretty clear now.
I changed this patch slightly by adding a caller parameter to
_mesa_handle_bind_buffer_gen() so the right entry point gets reported
when there is an error. I also moved the bufferobj.c/.h changes into a
separate commit. I think this commit might be a candidate for the
stable branches since this was already wrong for BindBufferBase()
and BindBufferRange().
See the attachment.
> ---
> src/mesa/main/bufferobj.c | 16 ++++++++--------
> src/mesa/main/bufferobj.h | 8 +++++++-
> src/mesa/main/varray.c | 19 ++++++++++---------
> 3 files changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
> index 2d57cab..ef5fbce 100644
> --- a/src/mesa/main/bufferobj.c
> +++ b/src/mesa/main/bufferobj.c
> @@ -655,11 +655,11 @@ _mesa_free_buffer_objects( struct gl_context *ctx )
> }
> }
>
> -static bool
> -handle_bind_buffer_gen(struct gl_context *ctx,
> - GLenum target,
> - GLuint buffer,
> - struct gl_buffer_object **buf_handle)
> +bool
> +_mesa_handle_bind_buffer_gen(struct gl_context *ctx,
> + GLenum target,
> + GLuint buffer,
> + struct gl_buffer_object **buf_handle)
> {
> struct gl_buffer_object *buf = *buf_handle;
>
> @@ -719,7 +719,7 @@ bind_buffer_object(struct gl_context *ctx, GLenum target,
> GLuint buffer)
> else {
> /* non-default buffer object */
> newBufObj = _mesa_lookup_bufferobj(ctx, buffer);
> - if (!handle_bind_buffer_gen(ctx, target, buffer, &newBufObj))
> + if (!_mesa_handle_bind_buffer_gen(ctx, target, buffer, &newBufObj))
> return;
> }
>
> @@ -2181,7 +2181,7 @@ _mesa_BindBufferRange(GLenum target, GLuint index,
> } else {
> bufObj = _mesa_lookup_bufferobj(ctx, buffer);
> }
> - if (!handle_bind_buffer_gen(ctx, target, buffer, &bufObj))
> + if (!_mesa_handle_bind_buffer_gen(ctx, target, buffer, &bufObj))
> return;
>
> if (!bufObj) {
> @@ -2227,7 +2227,7 @@ _mesa_BindBufferBase(GLenum target, GLuint index,
> GLuint buffer)
> } else {
> bufObj = _mesa_lookup_bufferobj(ctx, buffer);
> }
> - if (!handle_bind_buffer_gen(ctx, target, buffer, &bufObj))
> + if (!_mesa_handle_bind_buffer_gen(ctx, target, buffer, &bufObj))
> return;
>
> if (!bufObj) {
> diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h
> index 9b582f8c..503223d 100644
> --- a/src/mesa/main/bufferobj.h
> +++ b/src/mesa/main/bufferobj.h
> @@ -28,7 +28,7 @@
> #ifndef BUFFEROBJ_H
> #define BUFFEROBJ_H
>
> -
> +#include <stdbool.h>
> #include "mtypes.h"
>
>
> @@ -88,6 +88,12 @@ _mesa_reference_buffer_object(struct gl_context *ctx,
> _mesa_reference_buffer_object_(ctx, ptr, bufObj);
> }
>
> +bool
> +_mesa_handle_bind_buffer_gen(struct gl_context *ctx,
> + GLenum target,
> + GLuint buffer,
> + struct gl_buffer_object **buf_handle);
> +
> extern GLuint
> _mesa_total_buffer_object_memory(struct gl_context *ctx);
>
> diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
> index ed3d047..71d13a7 100644
> --- a/src/mesa/main/varray.c
> +++ b/src/mesa/main/varray.c
> @@ -1400,17 +1400,18 @@ _mesa_BindVertexBuffer(GLuint bindingIndex, GLuint
> buffer, GLintptr offset,
> } else if (buffer != 0) {
> vbo = _mesa_lookup_bufferobj(ctx, buffer);
>
> - /* The ARB_vertex_attrib_binding spec doesn't specify that an error
> - * should be generated when <buffer> doesn't refer to a valid buffer
> - * object, but we assume that this is an oversight.
> + /* From the GL_ARB_vertex_attrib_array spec:
> + *
> + * "[Core profile only:]
> + * An INVALID_OPERATION error is generated if buffer is not zero or
> a
> + * name returned from a previous call to GenBuffers, or if such a
> name
> + * has since been deleted with DeleteBuffers.
> + *
> + * Otherwise, we fall back to the same compat profile behavior as other
> + * object references (automatically gen it).
> */
> - if (!vbo) {
> - _mesa_error(ctx, GL_INVALID_OPERATION,
> - "glBindVertexBuffer(buffer=%u is not a valid "
> - "buffer object)",
> - buffer);
> + if (!_mesa_handle_bind_buffer_gen(ctx, GL_ARRAY_BUFFER, buffer, &vbo))
> return;
> - }
> } else {
> /* The ARB_vertex_attrib_binding spec says:
> *
>
From e691fc5b3d23deb101bf9be26f7ffee4b0b118f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= <fred...@kde.org>
Date: Fri, 1 Nov 2013 19:09:58 +0100
Subject: [PATCH 1/9] mesa: Make handle_bind_buffer_gen() non-static
...and rename it to _mesa_bind_buffer_gen().
This is so the function can be called from _mesa_BindVertexBuffer().
This patch also adds a caller parameter so we can report the right
entry point in error messages.
Based on a patch by Eric Anholt
---
src/mesa/main/bufferobj.c | 24 ++++++++++++++----------
src/mesa/main/bufferobj.h | 9 ++++++++-
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
index 54bba1a..15ec40c 100644
--- a/src/mesa/main/bufferobj.c
+++ b/src/mesa/main/bufferobj.c
@@ -655,16 +655,17 @@ _mesa_free_buffer_objects( struct gl_context *ctx )
}
}
-static bool
-handle_bind_buffer_gen(struct gl_context *ctx,
- GLenum target,
- GLuint buffer,
- struct gl_buffer_object **buf_handle)
+bool
+_mesa_handle_bind_buffer_gen(struct gl_context *ctx,
+ GLenum target,
+ GLuint buffer,
+ struct gl_buffer_object **buf_handle,
+ const char *caller)
{
struct gl_buffer_object *buf = *buf_handle;
if (!buf && ctx->API == API_OPENGL_CORE) {
- _mesa_error(ctx, GL_INVALID_OPERATION, "glBindBuffer(non-gen name)");
+ _mesa_error(ctx, GL_INVALID_OPERATION, "%s(non-gen name)", caller);
return false;
}
@@ -675,7 +676,7 @@ handle_bind_buffer_gen(struct gl_context *ctx,
ASSERT(ctx->Driver.NewBufferObject);
buf = ctx->Driver.NewBufferObject(ctx, buffer, target);
if (!buf) {
- _mesa_error(ctx, GL_OUT_OF_MEMORY, "glBindBufferARB");
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s", caller);
return false;
}
_mesa_HashInsert(ctx->Shared->BufferObjects, buffer, buf);
@@ -719,7 +720,8 @@ bind_buffer_object(struct gl_context *ctx, GLenum target, GLuint buffer)
else {
/* non-default buffer object */
newBufObj = _mesa_lookup_bufferobj(ctx, buffer);
- if (!handle_bind_buffer_gen(ctx, target, buffer, &newBufObj))
+ if (!_mesa_handle_bind_buffer_gen(ctx, target, buffer,
+ &newBufObj, "glBindBuffer"))
return;
}
@@ -2181,7 +2183,8 @@ _mesa_BindBufferRange(GLenum target, GLuint index,
} else {
bufObj = _mesa_lookup_bufferobj(ctx, buffer);
}
- if (!handle_bind_buffer_gen(ctx, target, buffer, &bufObj))
+ if (!_mesa_handle_bind_buffer_gen(ctx, target, buffer,
+ &bufObj, "glBindBufferRange"))
return;
if (!bufObj) {
@@ -2227,7 +2230,8 @@ _mesa_BindBufferBase(GLenum target, GLuint index, GLuint buffer)
} else {
bufObj = _mesa_lookup_bufferobj(ctx, buffer);
}
- if (!handle_bind_buffer_gen(ctx, target, buffer, &bufObj))
+ if (!_mesa_handle_bind_buffer_gen(ctx, target, buffer,
+ &bufObj, "glBindBufferBase"))
return;
if (!bufObj) {
diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h
index 9b582f8c..0b898a2 100644
--- a/src/mesa/main/bufferobj.h
+++ b/src/mesa/main/bufferobj.h
@@ -28,7 +28,7 @@
#ifndef BUFFEROBJ_H
#define BUFFEROBJ_H
-
+#include <stdbool.h>
#include "mtypes.h"
@@ -62,6 +62,13 @@ _mesa_init_buffer_objects( struct gl_context *ctx );
extern void
_mesa_free_buffer_objects( struct gl_context *ctx );
+extern bool
+_mesa_handle_bind_buffer_gen(struct gl_context *ctx,
+ GLenum target,
+ GLuint buffer,
+ struct gl_buffer_object **buf_handle,
+ const char *caller);
+
extern void
_mesa_update_default_objects_buffer_objects(struct gl_context *ctx);
--
1.7.10.4
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
