Re: [Mesa-dev] [PATCH 02/23] main: Add entry point for TextureBufferRange.
On 18/02/2015 20:10, Ian Romanick wrote:
On 02/17/2015 07:59 AM, Martin Peres wrote:
Hey Laura,
Thanks for this code. I'll be sending reviews throughout the week :)
On 12/02/15 04:05, Laura Ekstrand wrote:
diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index 336feff..ce6f446 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -5197,6 +5197,96 @@ _mesa_TextureBuffer(GLuint texture, GLenum
internalFormat, GLuint buffer)
bufObj, 0, buffer ? -1 : 0, false, true);
}
+void GLAPIENTRY
+_mesa_TextureBufferRange(GLuint texture, GLenum internalFormat,
GLuint buffer,
+ GLintptr offset, GLsizeiptr size)
+{
+ struct gl_texture_object *texObj;
+ struct gl_buffer_object *bufObj;
+
+ GET_CURRENT_CONTEXT(ctx);
+
+ /* NOTE: ARB_texture_buffer_object has interactions with
+* the compatibility profile that are not implemented.
+*/
+ if (!(ctx->API == API_OPENGL_CORE &&
+ ctx->Extensions.ARB_texture_buffer_object)) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ "glTextureBufferRange(ARB_texture_buffer_object is
not"
+ " implemented for the compatibility profile)");
+ return;
+ }
Can you point me to the relevant part of the spec that explains the
interaction?
In compatibility profile, TexBos can support luminance, luminance-alpha,
and intensity textures. Intel hardware doesn't support that, and none
of the TexBo common code supports it either.
But... at least for glTexBuffer and glTexBufferRange there is (almost) a
better way to do this. We should change the GL API generator code so
that we can omit functions from compatibility profile. We can already
do this for core profile (look for the deprecated='3.1' bits in
src/mapi/glapi/gen).
Due to previously mentioned issues with libraries like GLEW, we may not
be able to do this with DSA functions that are core-only if we're going
to expose DSA on compatibility profile.
Thanks Ian. How should I check for interactions with the compatibility mode
on the entry points I added? I guess I need to check Opengl 4.5 compat,
right?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 02/23] main: Add entry point for TextureBufferRange.
On 02/11/2015 06:05 PM, Laura Ekstrand wrote:
> ---
> src/mapi/glapi/gen/ARB_direct_state_access.xml | 8 +++
> src/mesa/main/tests/dispatch_sanity.cpp| 1 +
> src/mesa/main/teximage.c | 90
> ++
> src/mesa/main/teximage.h | 4 ++
> 4 files changed, 103 insertions(+)
>
> diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml
> b/src/mapi/glapi/gen/ARB_direct_state_access.xml
> index 2fe1638..86c00f9 100644
> --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml
> +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml
> @@ -21,6 +21,14 @@
>
>
>
> +
> +
> +
> +
> +
> +
> +
> +
>
>
>
> diff --git a/src/mesa/main/tests/dispatch_sanity.cpp
> b/src/mesa/main/tests/dispatch_sanity.cpp
> index 1f1a3a8..b78c1ce 100644
> --- a/src/mesa/main/tests/dispatch_sanity.cpp
> +++ b/src/mesa/main/tests/dispatch_sanity.cpp
> @@ -987,6 +987,7 @@ const struct function gl_core_functions_possible[] = {
> { "glTextureStorage2DMultisample", 45, -1 },
> { "glTextureStorage3DMultisample", 45, -1 },
> { "glTextureBuffer", 45, -1 },
> + { "glTextureBufferRange", 45, -1 },
>
> /* GL_EXT_polygon_offset_clamp */
> { "glPolygonOffsetClampEXT", 11, -1 },
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index 336feff..ce6f446 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -5197,6 +5197,96 @@ _mesa_TextureBuffer(GLuint texture, GLenum
> internalFormat, GLuint buffer)
>bufObj, 0, buffer ? -1 : 0, false, true);
> }
>
> +void GLAPIENTRY
> +_mesa_TextureBufferRange(GLuint texture, GLenum internalFormat, GLuint
> buffer,
> + GLintptr offset, GLsizeiptr size)
> +{
> + struct gl_texture_object *texObj;
> + struct gl_buffer_object *bufObj;
> +
> + GET_CURRENT_CONTEXT(ctx);
> +
> + /* NOTE: ARB_texture_buffer_object has interactions with
> +* the compatibility profile that are not implemented.
> +*/
> + if (!(ctx->API == API_OPENGL_CORE &&
> + ctx->Extensions.ARB_texture_buffer_object)) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glTextureBufferRange(ARB_texture_buffer_object is not"
> + " implemented for the compatibility profile)");
> + return;
> + }
> +
> + bufObj = _mesa_lookup_bufferobj(ctx, buffer);
> + if (bufObj) {
> + /* OpenGL 4.5 core spec (30.10.2014) says in Section 8.9 Buffer
> + * Textures:
> + *"An INVALID_VALUE error is generated if offset is negative, if
> + *size is less than or equal to zero, or if offset + size is
> greater
> + *than the value of BUFFER_SIZE for the buffer bound to target."
> + */
> + if (offset < 0) {
> + _mesa_error(ctx, GL_INVALID_VALUE,
> + "glTextureBufferRange(offset %d < 0)", (int) offset);
> + return;
> + }
> +
> + if (size <= 0) {
> + _mesa_error(ctx, GL_INVALID_VALUE,
> + "glTextureBufferRange(size %d <= 0)", (int) size);
> + return;
> + }
> +
> + if (offset + size > bufObj->Size) {
> + _mesa_error(ctx, GL_INVALID_VALUE,
> + "glTextureBufferRange("
> + "offset %d + size %d > buffer_size %d)", (int) offset,
> + (int) size, (int) bufObj->Size);
> + return;
> + }
> +
> + /* OpenGL 4.5 core spec (30.10.2014) says in Section 8.9 Buffer
> + * Textures:
> + *"An INVALID_VALUE error is generated if offset is not an integer
> + *multiple of the value of TEXTURE_BUFFER_OFFSET_ALIGNMENT."
> + */
> + if (offset % ctx->Const.TextureBufferOffsetAlignment) {
> + _mesa_error(ctx, GL_INVALID_VALUE,
> + "glTextureBufferRange(invalid offset alignment)");
> + return;
> + }
> + } else if (buffer) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glTextureBufferRange(unrecognized buffer %u)", buffer);
> + return;
> + } else {
> +
> + /* OpenGL 4.5 core spec (30.10.2014) says in Section 8.9 Buffer
> + * Textures:
> + *"If buffer is zero, then any buffer object attached to the buffer
> + *texture is detached, the values offset and size are ignored and
> + *the state for offset and size for the buffer texture are reset to
> + *zero."
> + */
> + offset = 0;
> + size = 0;
> + }
We usually try to keep flow control for error checking more flat. It
makes it easier to follow. For example, the above block would be much
better as
struct gl_buffer_object *bufObj = NULL;
...
if (buffer == 0) {
/* OpenGL 4.5 core spec (30.10.2014) says in Section 8.9 Buffer
* Textures:
*"If buffer is zero, then any buffer object attached to the buf
Re: [Mesa-dev] [PATCH 02/23] main: Add entry point for TextureBufferRange.
On 02/17/2015 07:59 AM, Martin Peres wrote:
> Hey Laura,
>
> Thanks for this code. I'll be sending reviews throughout the week :)
>
> On 12/02/15 04:05, Laura Ekstrand wrote:
>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>> index 336feff..ce6f446 100644
>> --- a/src/mesa/main/teximage.c
>> +++ b/src/mesa/main/teximage.c
>> @@ -5197,6 +5197,96 @@ _mesa_TextureBuffer(GLuint texture, GLenum
>> internalFormat, GLuint buffer)
>> bufObj, 0, buffer ? -1 : 0, false, true);
>> }
>> +void GLAPIENTRY
>> +_mesa_TextureBufferRange(GLuint texture, GLenum internalFormat,
>> GLuint buffer,
>> + GLintptr offset, GLsizeiptr size)
>> +{
>> + struct gl_texture_object *texObj;
>> + struct gl_buffer_object *bufObj;
>> +
>> + GET_CURRENT_CONTEXT(ctx);
>> +
>> + /* NOTE: ARB_texture_buffer_object has interactions with
>> +* the compatibility profile that are not implemented.
>> +*/
>> + if (!(ctx->API == API_OPENGL_CORE &&
>> + ctx->Extensions.ARB_texture_buffer_object)) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> + "glTextureBufferRange(ARB_texture_buffer_object is
>> not"
>> + " implemented for the compatibility profile)");
>> + return;
>> + }
>
> Can you point me to the relevant part of the spec that explains the
> interaction?
In compatibility profile, TexBos can support luminance, luminance-alpha,
and intensity textures. Intel hardware doesn't support that, and none
of the TexBo common code supports it either.
But... at least for glTexBuffer and glTexBufferRange there is (almost) a
better way to do this. We should change the GL API generator code so
that we can omit functions from compatibility profile. We can already
do this for core profile (look for the deprecated='3.1' bits in
src/mapi/glapi/gen).
Due to previously mentioned issues with libraries like GLEW, we may not
be able to do this with DSA functions that are core-only if we're going
to expose DSA on compatibility profile.
> Did you simply took this code out of _mesa_TexBufferRange?
>> +
>> + bufObj = _mesa_lookup_bufferobj(ctx, buffer);
>> + if (bufObj) {
>> + /* OpenGL 4.5 core spec (30.10.2014) says in Section 8.9 Buffer
>> + * Textures:
>> + *"An INVALID_VALUE error is generated if offset is
>> negative, if
>> + *size is less than or equal to zero, or if offset + size
>> is greater
>> + *than the value of BUFFER_SIZE for the buffer bound to
>> target."
>> + */
>> + if (offset < 0) {
>> + _mesa_error(ctx, GL_INVALID_VALUE,
>> + "glTextureBufferRange(offset %d < 0)", (int)
>> offset);
>> + return;
>> + }
>> +
>> + if (size <= 0) {
>> + _mesa_error(ctx, GL_INVALID_VALUE,
>> + "glTextureBufferRange(size %d <= 0)", (int) size);
>> + return;
>> + }
>> +
>> + if (offset + size > bufObj->Size) {
>> + _mesa_error(ctx, GL_INVALID_VALUE,
>> + "glTextureBufferRange("
>> + "offset %d + size %d > buffer_size %d)", (int)
>> offset,
>> + (int) size, (int) bufObj->Size);
>> + return;
>> + }
>> +
>> + /* OpenGL 4.5 core spec (30.10.2014) says in Section 8.9 Buffer
>> + * Textures:
>> + *"An INVALID_VALUE error is generated if offset is not an
>> integer
>> + *multiple of the value of TEXTURE_BUFFER_OFFSET_ALIGNMENT."
>> + */
>> + if (offset % ctx->Const.TextureBufferOffsetAlignment) {
>> + _mesa_error(ctx, GL_INVALID_VALUE,
>> + "glTextureBufferRange(invalid offset alignment)");
>> + return;
>> + }
>> + } else if (buffer) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> + "glTextureBufferRange(unrecognized buffer %u)",
>> buffer);
>> + return;
>> + } else {
>> +
>> + /* OpenGL 4.5 core spec (30.10.2014) says in Section 8.9 Buffer
>> + * Textures:
>> + *"If buffer is zero, then any buffer object attached to
>> the buffer
>> + *texture is detached, the values offset and size are
>> ignored and
>> + *the state for offset and size for the buffer texture are
>> reset to
>> + *zero."
>> + */
>> + offset = 0;
>> + size = 0;
>> + }
>> +
>> + /* Get the texture object by Name. */
>> + texObj = _mesa_lookup_texture_err(ctx, texture,
>> "glTextureBufferRange");
>> + if (!texObj)
>> + return;
>
> This should return INVALID_OPERATION:
>
> "An INVALID_OPERATION error is generated by TextureBufferRange if
> texture is not the name of an existing texture object."
>> +
>> + if (texObj->Target != GL_TEXTURE_BUFFER_ARB) {
>> + _mesa_error(ctx, GL_INVALID_ENUM, "glTextureBufferRange(texture
>> target"
>> +" is not GL_TEXTURE_BUFFER)");
>> + return;
>> + }
>
> Wha
Re: [Mesa-dev] [PATCH 02/23] main: Add entry point for TextureBufferRange.
Hey Laura,
Thanks for this code. I'll be sending reviews throughout the week :)
On 12/02/15 04:05, Laura Ekstrand wrote:
diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index 336feff..ce6f446 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -5197,6 +5197,96 @@ _mesa_TextureBuffer(GLuint texture, GLenum
internalFormat, GLuint buffer)
bufObj, 0, buffer ? -1 : 0, false, true);
}
+void GLAPIENTRY
+_mesa_TextureBufferRange(GLuint texture, GLenum internalFormat, GLuint buffer,
+ GLintptr offset, GLsizeiptr size)
+{
+ struct gl_texture_object *texObj;
+ struct gl_buffer_object *bufObj;
+
+ GET_CURRENT_CONTEXT(ctx);
+
+ /* NOTE: ARB_texture_buffer_object has interactions with
+* the compatibility profile that are not implemented.
+*/
+ if (!(ctx->API == API_OPENGL_CORE &&
+ ctx->Extensions.ARB_texture_buffer_object)) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ "glTextureBufferRange(ARB_texture_buffer_object is not"
+ " implemented for the compatibility profile)");
+ return;
+ }
Can you point me to the relevant part of the spec that explains the
interaction?
Did you simply took this code out of _mesa_TexBufferRange?
+
+ bufObj = _mesa_lookup_bufferobj(ctx, buffer);
+ if (bufObj) {
+ /* OpenGL 4.5 core spec (30.10.2014) says in Section 8.9 Buffer
+ * Textures:
+ *"An INVALID_VALUE error is generated if offset is negative, if
+ *size is less than or equal to zero, or if offset + size is greater
+ *than the value of BUFFER_SIZE for the buffer bound to target."
+ */
+ if (offset < 0) {
+ _mesa_error(ctx, GL_INVALID_VALUE,
+ "glTextureBufferRange(offset %d < 0)", (int) offset);
+ return;
+ }
+
+ if (size <= 0) {
+ _mesa_error(ctx, GL_INVALID_VALUE,
+ "glTextureBufferRange(size %d <= 0)", (int) size);
+ return;
+ }
+
+ if (offset + size > bufObj->Size) {
+ _mesa_error(ctx, GL_INVALID_VALUE,
+ "glTextureBufferRange("
+ "offset %d + size %d > buffer_size %d)", (int) offset,
+ (int) size, (int) bufObj->Size);
+ return;
+ }
+
+ /* OpenGL 4.5 core spec (30.10.2014) says in Section 8.9 Buffer
+ * Textures:
+ *"An INVALID_VALUE error is generated if offset is not an integer
+ *multiple of the value of TEXTURE_BUFFER_OFFSET_ALIGNMENT."
+ */
+ if (offset % ctx->Const.TextureBufferOffsetAlignment) {
+ _mesa_error(ctx, GL_INVALID_VALUE,
+ "glTextureBufferRange(invalid offset alignment)");
+ return;
+ }
+ } else if (buffer) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ "glTextureBufferRange(unrecognized buffer %u)", buffer);
+ return;
+ } else {
+
+ /* OpenGL 4.5 core spec (30.10.2014) says in Section 8.9 Buffer
+ * Textures:
+ *"If buffer is zero, then any buffer object attached to the buffer
+ *texture is detached, the values offset and size are ignored and
+ *the state for offset and size for the buffer texture are reset to
+ *zero."
+ */
+ offset = 0;
+ size = 0;
+ }
+
+ /* Get the texture object by Name. */
+ texObj = _mesa_lookup_texture_err(ctx, texture, "glTextureBufferRange");
+ if (!texObj)
+ return;
This should return INVALID_OPERATION:
"An INVALID_OPERATION error is generated by TextureBufferRange if
texture is not the name of an existing texture object."
+
+ if (texObj->Target != GL_TEXTURE_BUFFER_ARB) {
+ _mesa_error(ctx, GL_INVALID_ENUM, "glTextureBufferRange(texture target"
+" is not GL_TEXTURE_BUFFER)");
+ return;
+ }
What are you trying to do here? First, I see no mention of this in the
spec (but I am quite possibly wrong).
Secondly, you are checking the target before calling
_mesa_texture_buffer_range which does not care about the target.
Is it a copy/paste from _mesa_TextureBuffer? Please explain why this is
needed since I surely have not read the spec
as closely as you did on this matter.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
