[Mesa-dev] [PATCH] mesa/main: add ARB_clear_texture entrypoints
Ian Romanick
idr at freedesktop.org
Mon Mar 3 23:14:28 PST 2014
On 03/02/2014 12:09 AM, Ilia Mirkin wrote:
> This adds enough code to let drivers implement texture clearing, but
> doesn't actually do that for any of them.
There's always the usual... this should be split into two patches. :)
Generally the changes in gl_API, dispatch_sanity, and "stub" functions
is the first patch. Changes to dd.h is a patch. Implementing the
functions is a third.
>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>
> Thought I'd give this a shot. Am I on the right track here? Is the dd API
> reasonable? I haven't written a piglit test for all the "generic" error cases
> yet, but I'll get to that in a bit. I'm a little confused between all the
> different formats on a teximage... BaseFormat appears to be some
> generalization of the format, but what's the difference between InternalFormat
> and TexFormat? Just the type, but otherwise equivalent? Or are they
> representing different things?
BaseFormat is the generic format. So, GL_RGB or GL_RED are the base
formats that go with GL_RGB8 or GL_RED16. internalFormat is usually the
GL format (e.g., GL_RGBA32F), and TexFormat is usually the Mesa format
(e.g., MESA_FORMAT_RGBA_FLOAT32).
> BTW, this is the first time I'm adding stuff to mesa/main, so forgive me if I
> left out a step.
>
> src/mapi/glapi/gen/ARB_clear_texture.xml | 34 ++++++++
> src/mapi/glapi/gen/gl_API.xml | 2 +
> src/mesa/main/dd.h | 11 +++
> src/mesa/main/formatquery.c | 9 +++
> src/mesa/main/mtypes.h | 1 +
> src/mesa/main/tests/dispatch_sanity.cpp | 4 +
> src/mesa/main/teximage.c | 131 +++++++++++++++++++++++++++++++
> src/mesa/main/teximage.h | 10 +++
> 8 files changed, 202 insertions(+)
> create mode 100644 src/mapi/glapi/gen/ARB_clear_texture.xml
>
> diff --git a/src/mapi/glapi/gen/ARB_clear_texture.xml b/src/mapi/glapi/gen/ARB_clear_texture.xml
> new file mode 100644
> index 0000000..bd9116f
> --- /dev/null
> +++ b/src/mapi/glapi/gen/ARB_clear_texture.xml
> @@ -0,0 +1,34 @@
> +<?xml version="1.0"?>
> +<!DOCTYPE OpenGLAPI SYSTEM "gl_API.dtd">
> +
> +<OpenGLAPI>
> +
> +<category name="GL_ARB_clear_texture" number="145">
> +
> + <enum name="CLEAR_TEXTURE" value="0x9365"/>
> +
> + <function name ="ClearTexImage" offset="assign">
> + <param name="texture" type="GLuint"/>
> + <param name="level" type="GLint"/>
> + <param name="format" type="GLenum"/>
> + <param name="type" type="GLenum"/>
> + <param name="data" type="const GLvoid *"/>
> + </function>
> +
> + <function name ="ClearTexSubImage" offset="assign">
> + <param name="texture" type="GLuint"/>
> + <param name="level" type="GLint"/>
> + <param name="xoffset" type="GLint"/>
> + <param name="yoffset" type="GLint"/>
> + <param name="zoffset" type="GLint"/>
> + <param name="width" type="GLsizei"/>
> + <param name="height" type="GLsizei"/>
> + <param name="depth" type="GLsizei"/>
> + <param name="format" type="GLenum"/>
> + <param name="type" type="GLenum"/>
> + <param name="data" type="const GLvoid *"/>
> + </function>
> +
> +</category>
> +
> +</OpenGLAPI>
> diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
> index 7e1946e..15f8e32 100644
> --- a/src/mapi/glapi/gen/gl_API.xml
> +++ b/src/mapi/glapi/gen/gl_API.xml
> @@ -8515,6 +8515,8 @@
> </function>
> </category>
>
> +<xi:include href="ARB_clear_texture.xml" xmlns:xi="http://www.w3.org/2001/XInclude"/>
> +
> <!-- Non-ARB extensions sorted by extension number. -->
>
> <category name="GL_EXT_blend_color" number="2">
> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> index 9715241..d824bca 100644
> --- a/src/mesa/main/dd.h
> +++ b/src/mesa/main/dd.h
> @@ -275,6 +275,17 @@ struct dd_function_table {
> GLint level, mesa_format format,
> GLint width, GLint height,
> GLint depth, GLint border);
> +
> +
> + void (*ClearTexSubImage)(struct gl_context *ctx,
> + struct gl_texture_image *texImage,
> + GLint xoffset, GLint yoffset, GLint zoffset,
> + GLsizei width, GLsizei height, GLsizei depth,
> + GLenum format, GLenum type, const GLvoid *data);
> +
> + GLenum (*QuerySupportForClearTex)(struct gl_context *ctx,
> + GLenum internalFormat);
I think I'd rather add a generic query function.
GL_ARB_query_internalformt2 add a large number of
target/internalFormat/pname queries. I'm having difficulty deciding
whether it's better to just pipe this interface directly into the driver
or use something slightly more abstract. Maybe add a function like
GLenum (*QueryFormatSupport)(struct gl_context *ctx,
GLenum internalFormat,
GLenum pname);
> +
> /*@}*/
>
>
> diff --git a/src/mesa/main/formatquery.c b/src/mesa/main/formatquery.c
> index 40eca87..7997cb4 100644
> --- a/src/mesa/main/formatquery.c
> +++ b/src/mesa/main/formatquery.c
> @@ -140,6 +140,15 @@ _mesa_GetInternalformativ(GLenum target, GLenum internalformat, GLenum pname,
> count = 1;
> break;
> }
> + case GL_CLEAR_TEXTURE:
> + if (ctx->Extensions.ARB_clear_texture) {
> + const GLenum support = ctx->Driver.QuerySupportForClearTex(
> + ctx, internalformat);
> + buffer[0] = (GLint) support;
> + count = 1;
> + break;
> + }
> + /* fallthrough */
> default:
> _mesa_error(ctx, GL_INVALID_ENUM,
> "glGetInternalformativ(pname=%s)",
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 7246b1e..e4a3837 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -3490,6 +3490,7 @@ struct gl_extensions
> GLboolean ARB_base_instance;
> GLboolean ARB_blend_func_extended;
> GLboolean ARB_buffer_storage;
> + GLboolean ARB_clear_texture;
> GLboolean ARB_color_buffer_float;
> GLboolean ARB_compute_shader;
> GLboolean ARB_conservative_depth;
> diff --git a/src/mesa/main/tests/dispatch_sanity.cpp b/src/mesa/main/tests/dispatch_sanity.cpp
> index d1b0011..8690b5d 100644
> --- a/src/mesa/main/tests/dispatch_sanity.cpp
> +++ b/src/mesa/main/tests/dispatch_sanity.cpp
> @@ -936,6 +936,10 @@ const struct function gl_core_functions_possible[] = {
> /* GL_ARB_buffer_storage */
> { "glBufferStorage", 43, -1 },
>
> + /* GL_ARB_clear_texture */
> + { "glClearTexImage", 13, -1 },
> + { "glClearTexSubImage", 13, -1 },
> +
> { NULL, 0, -1 }
> };
>
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index 0519d22..9171187 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -3768,6 +3768,137 @@ _mesa_CopyTexSubImage3D( GLenum target, GLint level,
> x, y, width, height);
> }
>
> +static struct gl_texture_image *
> +get_tex_image(struct gl_context *ctx, GLuint texture, GLint level, GLint zoffset)
> +{
> + GLenum target = 0;
> + struct gl_texture_object *texObj = _mesa_lookup_texture(ctx, texture);
> + struct gl_texture_image *texImage;
> +
> + if (!texture || !texObj) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glClearTexSubImage(invalid texture)");
> + return NULL;
> + }
> +
> + switch (texObj->Target) {
> + case GL_TEXTURE_BUFFER:
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glClearTexSubImage(texture can't be buffer)");
> + return NULL;
> + case GL_TEXTURE_CUBE_MAP_ARB:
> + case GL_TEXTURE_CUBE_MAP_ARRAY:
> + target = GL_TEXTURE_CUBE_MAP_POSITIVE_X + zoffset;
> + break;
> + default:
> + break;
> + }
> +
> + texImage = _mesa_select_tex_image(ctx, texObj, target, level);
> + if (!texImage) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glClearTexSubImage(missing image)");
> + return NULL;
> + }
> +
> + return texImage;
> +}
> +
> +static void
> +clear_tex_sub_image(struct gl_context *ctx, struct gl_texture_image *texImage,
> + GLint xoffset, GLint yoffset, GLint zoffset,
> + GLsizei width, GLsizei height, GLsizei depth,
> + GLenum format, GLenum type, const void *data)
> +{
> + if (!texImage)
> + return;
> +
> + if (_mesa_is_compressed_format(ctx, texImage->InternalFormat)) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glClearTexSubImage(compressed format)");
This will cause the wrong function name to get logged for
glClearTexImage. Similar code in other parts of Mesa will pass a 'const
char *caller' to the utility function.
> + return;
> + }
> +
> + if (texImage->_BaseFormat == GL_DEPTH_COMPONENT &&
> + format != GL_DEPTH_COMPONENT) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glClearTexSubImage(invalid format)");
> + return;
> + }
> + if (texImage->_BaseFormat == GL_DEPTH_STENCIL &&
> + format != GL_DEPTH_STENCIL) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glClearTexSubImage(invalid format)");
> + return;
> + }
> + if (texImage->_BaseFormat == GL_STENCIL_INDEX &&
> + format != GL_STENCIL_INDEX) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glClearTexSubImage(invalid format)");
> + return;
> + }
> + if (texImage->_BaseFormat == GL_RGBA &&
> + (format == GL_DEPTH_COMPONENT ||
> + format == GL_DEPTH_STENCIL ||
> + format == GL_STENCIL_INDEX)) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glClearTexSubImage(invalid format)");
> + return;
> + }
> + if (_mesa_is_format_integer_color(texImage->InternalFormat) !=
> + _mesa_is_format_integer_color(format)) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glClearTexSubImage(invalid format)");
> + return;
> + }
I think the format validation is missing some bits. What will happen if
the application passes a completely garbage value for format,
texImage->_BaseFormat is GL_RGBA and texImage->InternalFormat is not an
integer color format?
> +
> + if (xoffset < -texImage->Border ||
> + xoffset + width > texImage->Width - texImage->Border ||
> + yoffset < -texImage->Border ||
> + yoffset + height > texImage->Height - texImage->Border ||
> + zoffset < -texImage->Border ||
> + zoffset + depth > texImage->Depth - texImage->Border) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glClearTexSubImage(invalid bounds)");
> + }
> +
> + ctx->Driver.ClearTexSubImage(ctx, texImage, xoffset, yoffset, zoffset,
> + width, height, depth, format, type, data);
> +}
> +
> +void GLAPIENTRY
> +_mesa_ClearTexSubImage( GLuint texture, GLint level,
> + GLint xoffset, GLint yoffset, GLint zoffset,
> + GLsizei width, GLsizei height, GLsizei depth,
> + GLenum format, GLenum type, const void *data )
> +{
> + GET_CURRENT_CONTEXT(ctx);
> + struct gl_texture_image *texImage =
> + get_tex_image(ctx, texture, level, zoffset);
> +
> + clear_tex_sub_image(ctx, texImage, xoffset, yoffset, zoffset,
> + width, height, depth, format, type, data);
> +}
> +
> +void GLAPIENTRY
> +_mesa_ClearTexImage( GLuint texture, GLint level,
> + GLenum format, GLenum type, const void *data )
> +{
> + GET_CURRENT_CONTEXT(ctx);
> +
> + struct gl_texture_image *texImage
> + = get_tex_image(ctx, texture, level, 0);
> + if (!texImage)
> + return;
> +
> + /* XXX figure out which dimensions the texImage has, and pass in 0's for
> + * the border.
> + */
> + clear_tex_sub_image(ctx, texImage,
> + -texImage->Border, -texImage->Border, -texImage->Border,
> + texImage->Width, texImage->Height, texImage->Depth,
> + format, type, data);
I would just pass 0s. Mesa doesn't support textures with borders.
> +}
>
>
>
> diff --git a/src/mesa/main/teximage.h b/src/mesa/main/teximage.h
> index 5f8a477..4d01ff9 100644
> --- a/src/mesa/main/teximage.h
> +++ b/src/mesa/main/teximage.h
> @@ -261,6 +261,16 @@ _mesa_CopyTexSubImage3D( GLenum target, GLint level,
>
>
> extern void GLAPIENTRY
> +_mesa_ClearTexSubImage( GLuint texture, GLint level,
> + GLint xoffset, GLint yoffset, GLint zoffset,
> + GLsizei width, GLsizei height, GLsizei depth,
> + GLenum format, GLenum type, const void *data );
> +
> +extern void GLAPIENTRY
> +_mesa_ClearTexImage( GLuint texture, GLint level,
> + GLenum format, GLenum type, const void *data );
> +
> +extern void GLAPIENTRY
> _mesa_CompressedTexImage1D(GLenum target, GLint level,
> GLenum internalformat, GLsizei width,
> GLint border, GLsizei imageSize,
>
More information about the mesa-dev
mailing list