[Mesa-dev] [PATCH] mesa/main: add ARB_clear_texture entrypoints
Ian Romanick
idr at freedesktop.org
Mon Mar 10 14:44:23 PDT 2014
On 03/04/2014 10:31 AM, Ilia Mirkin wrote:
> 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)
Maybe. Applictions can still pass generic formats for internalFormat,
and the driver can pick a different format. The use could ask for
GL_RGBA12, but the driver only supports 8-bit or 16-bit components. In
that case, internalFormat is still GL_RGBA12, but TexFormat would be
MESA_FORMAT_RGBA_UNORM16.
>>> 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.
There is an aspect of the spec that isn't clear to me... the query
allows you to say that some formats are not supported. You can imagine
an implementation that uses something like meta to bind the texture to
an FBO and glClearBuffer on it. Some formats are not color-renderable
(e.g., RGB16, shared exponent float, etc.), and these formats won't work
with that implementation.
What is supposed to happen in glClearTexImage for these unsupported
format? Generate an error? Silently do nothing?
Are you allowed to not support some formats if you don't support the
query, or do you have to make everything "just work"?
>>> +
>>> /*@}*/
>>>
>>>
>>> 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.)
It's not major, but it helps when you're trying to debug applications
using MESA_DEBUG to log the error messages.
>>> + 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?]
I think texImage->Border is there so that apps can query back what they
requested... I'm not sure. Brian may have any opion.
Texture borders are a 1-pixel border outside the actual texture. The
primary use was for stitching together larger textures with seamless
filtering across edges. This was mostly interesting when hardware only
supported 256x256 textures. There were similar uses for seamless
filtering across cubemap faces. Large textures, seamless cubemap
filtering, and anisotropic filtering (that requires more than one extra
pixel) have really made texture borders useless.
>>> +}
>>>
>>>
>>>
>>> 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