Re: [Mesa-dev] [PATCH 2/5] mesa/main: Add generic bits of ARB_clear_texture implementation
Jason Ekstrand ja...@jlekstrand.net writes: + texImages[0] = _mesa_select_tex_image(ctx, texObj, texObj-Target, level); Do you want this inside an else block? I think it's quite a common idiom in Mesa to handle the special cases as a series of if-statements with a return upfront before handling the normal case at the end without an else. But yes, in this instance it does look a bit odd because the there is only one special case and the normal case is very short, so I don't mind changing it. Regards, - Neil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/5] mesa/main: Add generic bits of ARB_clear_texture implementation
Ian Romanick i...@freedesktop.org writes: Are there potentially cases where the first clear_tex_image could succeed but the second fail? If so, then this is no good. If a GL call generates an error (other than GL_NO_MEMORY), it is supposed to be as though nothing happened. Urgh, yes, you're right because for example GL would let you make one face of a cubemap texture be compressed while the others aren't. I'll change it to have a separate loop for the checks as you suggest. Thanks for the review. - Neil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/5] mesa/main: Add generic bits of ARB_clear_texture implementation
On Tue, Jul 1, 2014 at 10:09 AM, Neil Roberts n...@linux.intel.com wrote: Jason Ekstrand ja...@jlekstrand.net writes: + texImages[0] = _mesa_select_tex_image(ctx, texObj, texObj-Target, level); Do you want this inside an else block? I think it's quite a common idiom in Mesa to handle the special cases as a series of if-statements with a return upfront before handling the normal case at the end without an else. But yes, in this instance it does look a bit odd because the there is only one special case and the normal case is very short, so I don't mind changing it. Sorry, I missed the return. In that case, I don't think it matters; feel free to leave it alone. --Jason Regards, - Neil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/5] mesa/main: Add generic bits of ARB_clear_texture implementation
On Fri, Jun 13, 2014 at 5:59 PM, Neil Roberts n...@linux.intel.com wrote: This adds the driver entry point for glClearTexSubImage and fills in the _mesa_ClearTexImage and _mesa_ClearTexSubImage functions that call it. --- src/mesa/main/dd.h | 14 +++ src/mesa/main/teximage.c | 241 ++- src/mesa/main/teximage.h | 12 +++ 3 files changed, 266 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h index 633ea2c..8976535 100644 --- a/src/mesa/main/dd.h +++ b/src/mesa/main/dd.h @@ -239,6 +239,20 @@ struct dd_function_table { struct gl_texture_image *texImage ); /** +* Called by glClearTex[Sub]Image +* +* Clears a rectangular region of the image to a given value. The +* clearValue argument is either NULL or points to a single texel to use as +* the clear value in the same internal format as the texture image. If it +* is NULL then the texture should be cleared to zeroes. +*/ + void (*ClearTexSubImage)(struct gl_context *ctx, +struct gl_texture_image *texImage, +GLint xoffset, GLint yoffset, GLint zoffset, +GLsizei width, GLsizei height, GLsizei depth, +const GLvoid *clearValue); + + /** * Called by glCopyTex[Sub]Image[123]D(). * * This function should copy a rectangular region in the rb to a single diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c index a893c70..d5baac8 100644 --- a/src/mesa/main/teximage.c +++ b/src/mesa/main/teximage.c @@ -51,6 +51,7 @@ #include textureview.h #include mtypes.h #include glformats.h +#include texstore.h /** @@ -3848,20 +3849,259 @@ _mesa_CopyTexSubImage3D( GLenum target, GLint level, x, y, width, height); } +static bool +clear_tex_image(struct gl_context *ctx, +const char *function, +struct gl_texture_image *texImage, GLint level, +GLint xoffset, GLint yoffset, GLint zoffset, +GLsizei width, GLsizei height, GLsizei depth, +GLenum format, GLenum type, +const void *data) +{ + struct gl_texture_object *texObj = texImage-TexObject; + static const GLubyte zeroData[MAX_PIXEL_BYTES]; + GLubyte clearValue[MAX_PIXEL_BYTES]; + GLubyte *clearValuePtr = clearValue; + GLenum internalFormat = texImage-InternalFormat; + GLenum err; + + if (texObj-Target == GL_TEXTURE_BUFFER) { + _mesa_error(ctx, GL_INVALID_OPERATION, + %s(buffer texture), function); + return false; + } + + if (_mesa_is_compressed_format(ctx, internalFormat)) { + _mesa_error(ctx, GL_INVALID_OPERATION, + %s(compressed texture), function); + return false; + } + + err = _mesa_error_check_format_and_type(ctx, format, type); + if (err != GL_NO_ERROR) { + _mesa_error(ctx, err, + %s(incompatible format = %s, type = %s), + function, + _mesa_lookup_enum_by_nr(format), + _mesa_lookup_enum_by_nr(type)); + return false; + } + + /* make sure internal format and format basically agree */ + if (!texture_formats_agree(internalFormat, format)) { + _mesa_error(ctx, GL_INVALID_OPERATION, + %s(incompatible internalFormat = %s, format = %s), + function, + _mesa_lookup_enum_by_nr(internalFormat), + _mesa_lookup_enum_by_nr(format)); + return false; + } + + if (ctx-Version = 30 || ctx-Extensions.EXT_texture_integer) { + /* both source and dest must be integer-valued, or neither */ + if (_mesa_is_format_integer_color(texImage-TexFormat) != + _mesa_is_enum_format_integer(format)) { + _mesa_error(ctx, GL_INVALID_OPERATION, + %s(integer/non-integer format mismatch), + function); + return false; + } + } + + if (!_mesa_texstore(ctx, + 1, /* dims */ + texImage-_BaseFormat, + texImage-TexFormat, + 0, /* dstRowStride */ + clearValuePtr, + 1, 1, 1, /* srcWidth/Height/Depth */ + format, type, + data ? data : zeroData, + ctx-DefaultPacking)) { + _mesa_error(ctx, GL_INVALID_OPERATION, %s(invalid format), function); + return false; + } + + ctx-Driver.ClearTexSubImage(ctx, +texImage, +xoffset, yoffset, zoffset, +width, height, depth, +data ? clearValue : NULL); + + return true;
Re: [Mesa-dev] [PATCH 2/5] mesa/main: Add generic bits of ARB_clear_texture implementation
On 06/13/2014 05:59 PM, Neil Roberts wrote: This adds the driver entry point for glClearTexSubImage and fills in the _mesa_ClearTexImage and _mesa_ClearTexSubImage functions that call it. --- src/mesa/main/dd.h | 14 +++ src/mesa/main/teximage.c | 241 ++- src/mesa/main/teximage.h | 12 +++ 3 files changed, 266 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h index 633ea2c..8976535 100644 --- a/src/mesa/main/dd.h +++ b/src/mesa/main/dd.h @@ -239,6 +239,20 @@ struct dd_function_table { struct gl_texture_image *texImage ); /** +* Called by glClearTex[Sub]Image +* +* Clears a rectangular region of the image to a given value. The +* clearValue argument is either NULL or points to a single texel to use as +* the clear value in the same internal format as the texture image. If it +* is NULL then the texture should be cleared to zeroes. +*/ + void (*ClearTexSubImage)(struct gl_context *ctx, +struct gl_texture_image *texImage, +GLint xoffset, GLint yoffset, GLint zoffset, +GLsizei width, GLsizei height, GLsizei depth, +const GLvoid *clearValue); + + /** * Called by glCopyTex[Sub]Image[123]D(). * * This function should copy a rectangular region in the rb to a single diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c index a893c70..d5baac8 100644 --- a/src/mesa/main/teximage.c +++ b/src/mesa/main/teximage.c @@ -51,6 +51,7 @@ #include textureview.h #include mtypes.h #include glformats.h +#include texstore.h /** @@ -3848,20 +3849,259 @@ _mesa_CopyTexSubImage3D( GLenum target, GLint level, x, y, width, height); } +static bool +clear_tex_image(struct gl_context *ctx, +const char *function, +struct gl_texture_image *texImage, GLint level, +GLint xoffset, GLint yoffset, GLint zoffset, +GLsizei width, GLsizei height, GLsizei depth, +GLenum format, GLenum type, +const void *data) +{ + struct gl_texture_object *texObj = texImage-TexObject; + static const GLubyte zeroData[MAX_PIXEL_BYTES]; + GLubyte clearValue[MAX_PIXEL_BYTES]; + GLubyte *clearValuePtr = clearValue; + GLenum internalFormat = texImage-InternalFormat; + GLenum err; + + if (texObj-Target == GL_TEXTURE_BUFFER) { + _mesa_error(ctx, GL_INVALID_OPERATION, + %s(buffer texture), function); + return false; + } + + if (_mesa_is_compressed_format(ctx, internalFormat)) { + _mesa_error(ctx, GL_INVALID_OPERATION, + %s(compressed texture), function); + return false; + } + + err = _mesa_error_check_format_and_type(ctx, format, type); + if (err != GL_NO_ERROR) { + _mesa_error(ctx, err, + %s(incompatible format = %s, type = %s), + function, + _mesa_lookup_enum_by_nr(format), + _mesa_lookup_enum_by_nr(type)); + return false; + } + + /* make sure internal format and format basically agree */ + if (!texture_formats_agree(internalFormat, format)) { + _mesa_error(ctx, GL_INVALID_OPERATION, + %s(incompatible internalFormat = %s, format = %s), + function, + _mesa_lookup_enum_by_nr(internalFormat), + _mesa_lookup_enum_by_nr(format)); + return false; + } + + if (ctx-Version = 30 || ctx-Extensions.EXT_texture_integer) { + /* both source and dest must be integer-valued, or neither */ + if (_mesa_is_format_integer_color(texImage-TexFormat) != + _mesa_is_enum_format_integer(format)) { + _mesa_error(ctx, GL_INVALID_OPERATION, + %s(integer/non-integer format mismatch), + function); + return false; + } + } + + if (!_mesa_texstore(ctx, + 1, /* dims */ + texImage-_BaseFormat, + texImage-TexFormat, + 0, /* dstRowStride */ + clearValuePtr, + 1, 1, 1, /* srcWidth/Height/Depth */ + format, type, + data ? data : zeroData, + ctx-DefaultPacking)) { + _mesa_error(ctx, GL_INVALID_OPERATION, %s(invalid format), function); + return false; + } + + ctx-Driver.ClearTexSubImage(ctx, +texImage, +xoffset, yoffset, zoffset, +width, height, depth, +data ? clearValue : NULL); + + return true; +} + +static struct
Re: [Mesa-dev] [PATCH 2/5] mesa/main: Add generic bits of ARB_clear_texture implementation
Ilia Mirkin imir...@alum.mit.edu writes: + if (ctx-Version = 30 || ctx-Extensions.EXT_texture_integer) { Just ctx-Extensions.EXT_texture_integer should be enough here, no? I'm reluctant to change this because every other place in the code that checks for integer textures does it in the same way. Perhaps if we wanted to change it then we should do it in a separate patch and change it everywhere. I think it might theoretically make sense to expose GL 3 without GL_EXT_texture_integer in some cases because the extension implies supporting luminance integer textures whereas GL 3 does not. This bit of code implies that Mesa treats the two types of support for integer textures distinctly: http://is.gd/HM3SIw Regards, - Neil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/5] mesa/main: Add generic bits of ARB_clear_texture implementation
The presence of EXT_texture_integer implies EXT_gpu_shader4, so I added ctx-Version = 30 || ctx-Extensions.EXT_texture_integer everywhere to allow drivers to disable EXT_texture_integer but not lose integer textures from GL 3.0. Marek On Mon, Jun 16, 2014 at 12:49 PM, Neil Roberts n...@linux.intel.com wrote: Ilia Mirkin imir...@alum.mit.edu writes: + if (ctx-Version = 30 || ctx-Extensions.EXT_texture_integer) { Just ctx-Extensions.EXT_texture_integer should be enough here, no? I'm reluctant to change this because every other place in the code that checks for integer textures does it in the same way. Perhaps if we wanted to change it then we should do it in a separate patch and change it everywhere. I think it might theoretically make sense to expose GL 3 without GL_EXT_texture_integer in some cases because the extension implies supporting luminance integer textures whereas GL 3 does not. This bit of code implies that Mesa treats the two types of support for integer textures distinctly: http://is.gd/HM3SIw Regards, - Neil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/5] mesa/main: Add generic bits of ARB_clear_texture implementation
On Fri, Jun 13, 2014 at 8:59 PM, Neil Roberts n...@linux.intel.com wrote: This adds the driver entry point for glClearTexSubImage and fills in the _mesa_ClearTexImage and _mesa_ClearTexSubImage functions that call it. --- src/mesa/main/dd.h | 14 +++ src/mesa/main/teximage.c | 241 ++- src/mesa/main/teximage.h | 12 +++ 3 files changed, 266 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h index 633ea2c..8976535 100644 --- a/src/mesa/main/dd.h +++ b/src/mesa/main/dd.h @@ -239,6 +239,20 @@ struct dd_function_table { struct gl_texture_image *texImage ); /** +* Called by glClearTex[Sub]Image +* +* Clears a rectangular region of the image to a given value. The +* clearValue argument is either NULL or points to a single texel to use as +* the clear value in the same internal format as the texture image. If it +* is NULL then the texture should be cleared to zeroes. +*/ + void (*ClearTexSubImage)(struct gl_context *ctx, +struct gl_texture_image *texImage, +GLint xoffset, GLint yoffset, GLint zoffset, +GLsizei width, GLsizei height, GLsizei depth, +const GLvoid *clearValue); + + /** * Called by glCopyTex[Sub]Image[123]D(). * * This function should copy a rectangular region in the rb to a single diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c index a893c70..d5baac8 100644 --- a/src/mesa/main/teximage.c +++ b/src/mesa/main/teximage.c @@ -51,6 +51,7 @@ #include textureview.h #include mtypes.h #include glformats.h +#include texstore.h /** @@ -3848,20 +3849,259 @@ _mesa_CopyTexSubImage3D( GLenum target, GLint level, x, y, width, height); } +static bool +clear_tex_image(struct gl_context *ctx, +const char *function, +struct gl_texture_image *texImage, GLint level, +GLint xoffset, GLint yoffset, GLint zoffset, +GLsizei width, GLsizei height, GLsizei depth, +GLenum format, GLenum type, +const void *data) +{ + struct gl_texture_object *texObj = texImage-TexObject; + static const GLubyte zeroData[MAX_PIXEL_BYTES]; + GLubyte clearValue[MAX_PIXEL_BYTES]; + GLubyte *clearValuePtr = clearValue; + GLenum internalFormat = texImage-InternalFormat; + GLenum err; + + if (texObj-Target == GL_TEXTURE_BUFFER) { + _mesa_error(ctx, GL_INVALID_OPERATION, + %s(buffer texture), function); + return false; + } + + if (_mesa_is_compressed_format(ctx, internalFormat)) { + _mesa_error(ctx, GL_INVALID_OPERATION, + %s(compressed texture), function); + return false; + } + + err = _mesa_error_check_format_and_type(ctx, format, type); + if (err != GL_NO_ERROR) { + _mesa_error(ctx, err, + %s(incompatible format = %s, type = %s), + function, + _mesa_lookup_enum_by_nr(format), + _mesa_lookup_enum_by_nr(type)); + return false; + } + + /* make sure internal format and format basically agree */ + if (!texture_formats_agree(internalFormat, format)) { + _mesa_error(ctx, GL_INVALID_OPERATION, + %s(incompatible internalFormat = %s, format = %s), + function, + _mesa_lookup_enum_by_nr(internalFormat), + _mesa_lookup_enum_by_nr(format)); + return false; + } + + if (ctx-Version = 30 || ctx-Extensions.EXT_texture_integer) { Just ctx-Extensions.EXT_texture_integer should be enough here, no? + /* both source and dest must be integer-valued, or neither */ + if (_mesa_is_format_integer_color(texImage-TexFormat) != + _mesa_is_enum_format_integer(format)) { + _mesa_error(ctx, GL_INVALID_OPERATION, + %s(integer/non-integer format mismatch), + function); + return false; + } + } + + if (!_mesa_texstore(ctx, + 1, /* dims */ + texImage-_BaseFormat, + texImage-TexFormat, + 0, /* dstRowStride */ + clearValuePtr, + 1, 1, 1, /* srcWidth/Height/Depth */ + format, type, + data ? data : zeroData, + ctx-DefaultPacking)) { + _mesa_error(ctx, GL_INVALID_OPERATION, %s(invalid format), function); + return false; + } + + ctx-Driver.ClearTexSubImage(ctx, +texImage, +xoffset, yoffset, zoffset, +width, height, depth, +