[Mesa-dev] [PATCH] mesa/main: add ARB_clear_texture entrypoints
Ilia Mirkin
imirkin at alum.mit.edu
Tue Mar 4 10:31:50 PST 2014
On Tue, Mar 4, 2014 at 2:14 AM, Ian Romanick <idr at freedesktop.org> wrote:
> 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.
OK, will do.
>
>
>>
>> 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).
So TexFormat == InternalFormat, right? (Just different types)
>
>
>> 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);
Others have also pointed out to me that query2 isn't supported, so I'm
just going to drop it for now.
>
>
>
>> +
>> /*@}*/
>>
>>
>> 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.
OK, didn't think it was important, but it's also really easy to fix,
so I'll do that. (When I first wrote the code, it was not going to be
easy to fix, but I since moved all the code into the helper as you see
it here.)
>
>
>> + 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?
Nothing good :) I believe that Brian's suggested changes to the
validation logic should fix this.
>
>
>> +
>> + 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.
Should texImage->Border be removed? I figured it was easy to support
borders here, and the driver can deal with any issues. [I also haven't
the slightest was texture borders are, beyond the obvious -- borders
on textures. Why do they exist?]
>
>
>> +}
>>
>>
>>
>> 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