[Mesa-dev] [PATCH] Implement the ARB_clear_texture extension
Ilia Mirkin
imirkin at alum.mit.edu
Wed Jun 4 16:15:55 PDT 2014
On Wed, Jun 4, 2014 at 6:42 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 06/04/2014 11:12 AM, Neil Roberts wrote:
>> The clear texture extension is used to clear a texture to a given value
>> without having to provide a buffer for the whole texture and without having to
>> create an FBO. This patch provides a generic implementation that works with
>> any driver. There are two approaches, the first being in meta.c which tries to
>> create a GL framebuffer to the texture and calls glClear. This can fail
>> if the FBO extension is not supported or if the texture can't be used as a
>> render target. In that case it will fall back to an implementation in
>> texstore.c which maps a region of the texture and just directly writes in the
>> values.
>
> GL_EXT_framebuffer_object is always supported, so that shouldn't be a
> concern.
>
>> A small problem with this patch is that the fallback approach that maps the
>> texture doesn't seem to work with depth-stencil textures. However I think this
>> may be a general bug with mapping depth-stencil textures because I seem to get
>> the same issue if I try to update the texture using glTexSubImage2D as well.
>> You can replicate this if you run the Piglit test
>> arb_clear_texture-depth-stencil and set MESA_EXTENSION_OVERRIDE to
>> -GL_ARB_framebuffer_object.
>> ---
>> src/mapi/glapi/gen/ARB_clear_texture.xml | 32 ++++
>> src/mapi/glapi/gen/gl_API.xml | 6 +-
>> src/mesa/drivers/common/driverfuncs.c | 1 +
>> src/mesa/drivers/common/meta.c | 143 ++++++++++++++++++
>> src/mesa/drivers/common/meta.h | 14 ++
>> src/mesa/main/dd.h | 14 ++
>> src/mesa/main/extensions.c | 1 +
>> src/mesa/main/teximage.c | 251 ++++++++++++++++++++++++++++++-
>> src/mesa/main/teximage.h | 12 ++
>> src/mesa/main/texstore.c | 70 +++++++++
>> src/mesa/main/texstore.h | 7 +
>
> Also need changes to src/mesa/main/tests/dispatch_sanity.cpp. I guess
> you didn't 'make check'. :)
>
> Below I've deleted any files where I didn't have comments... otherwise I
> think my comments may have gotten lost. That's part of the reason more
> small patches is better than a signle large patch.
>
>> 11 files changed, 549 insertions(+), 2 deletions(-)
>> create mode 100644 src/mapi/glapi/gen/ARB_clear_texture.xml
>>
>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>> index 845ba80..43c5889 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"
>>
>>
>> /**
>> @@ -4544,7 +4545,6 @@ _mesa_TexStorage2DMultisample(GLenum target, GLsizei samples,
>> "glTexStorage2DMultisample");
>> }
>>
>> -
>> void GLAPIENTRY
>> _mesa_TexStorage3DMultisample(GLenum target, GLsizei samples,
>> GLenum internalformat, GLsizei width,
>> @@ -4555,3 +4555,252 @@ _mesa_TexStorage3DMultisample(GLenum target, GLsizei samples,
>> width, height, depth, fixedsamplelocations, GL_TRUE,
>> "glTexStorage3DMultisample");
>> }
>> +
>> +static void
>> +clear_tex_image(struct gl_context *ctx,
>> + 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;
>> + GLint dimensions;
>> + GLenum err;
>> +
>> + dimensions = _mesa_get_texture_dimensions(texObj->Target);
>> +
>> + if (texObj->Target == GL_TEXTURE_BUFFER) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> + "glClear*TexImage(buffer texture)");
>> + return;
>> + }
>> +
>> + if (_mesa_is_compressed_format(ctx, texImage->InternalFormat)) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> + "glClear*TexImage(compressed texture)");
>> + return;
>> + }
>> +
>> + /* OpenGL ES 1.x and OpenGL ES 2.0 impose additional restrictions on the
>> + * combinations of format and type that can be used. Formats and types
>> + * that require additional extensions (e.g., GL_FLOAT requires
>> + * GL_OES_texture_float) are filtered elsewhere.
>> + */
>
> This extension doesn't exist in ES, so I don't think any of these ES
> checks are useful.
>
>> + if (_mesa_is_gles(ctx) && !_mesa_is_gles3(ctx)) {
>> + err = _mesa_es_error_check_format_and_type(format, type, dimensions);
>> + if (err != GL_NO_ERROR) {
>> + _mesa_error(ctx, err,
>> + "glClearTex*Image(format = %s, type = %s)",
>> + _mesa_lookup_enum_by_nr(format),
>> + _mesa_lookup_enum_by_nr(type));
>> + return;
>> + }
>> + }
>> +
>> + err = _mesa_error_check_format_and_type(ctx, format, type);
>> + if (err != GL_NO_ERROR) {
>> + _mesa_error(ctx, err,
>> + "glClearTex*Image(incompatible format = %s, type = %s)",
>> + _mesa_lookup_enum_by_nr(format),
>> + _mesa_lookup_enum_by_nr(type));
>> + return;
>> + }
>> +
>> + 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,
>> + "glClearTex*Image(integer/non-integer format mismatch)");
>> + return;
>> + }
>> + }
>> +
>> + 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, "glClearTex*Image");
>> + return;
>> + }
>> +
>> + ctx->Driver.ClearTexSubImage(ctx,
>> + texImage,
>> + xoffset, yoffset, zoffset,
>> + width, height, depth,
>> + data ? clearValue : NULL);
>
> Gallium drivers don't use meta at all, so ctx->Driver.ClearTexSubImage
> will be NULL in all Gallium drivers. Until there is a Gallium
> implementation (Ilia may have done one already...), I don't think we can
> enable this extension by default.
Well, the patches I had made just add a pipe->clear_texture callback
(and implementation for nv50). After discussions spawned by my
(semi-related) ->clear_buffer addition (for ARB_clear_buffer_object
acceleration), it seems like it'll take a little more than this --
I'll also need to unify the existing ->clear_render_target and
->clear_depth_stencil functions into one big happy do-everything API
call.
However my understanding (I'm a newbie at this, so take with a grain
of salt) was that a generic implementation was impossible due to the
need to handle MS textures, and there being no "spec" for how they're
laid out. e.g. is 4x MS just 4x wider? 4x higher? 2x on both? [And
non-renderable texture formats present issues as well.]
-ilia
>
>> +}
>> +
>> +static struct gl_texture_object *
>> +get_tex_obj_for_clear(struct gl_context *ctx,
>> + GLuint texture)
>> +{
>> + struct gl_texture_object *texObj;
>> +
>> + if (texture == 0) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION, "glClear*TexImage(zero texture)");
>> + return NULL;
>> + }
>> +
>> + texObj = _mesa_lookup_texture(ctx, texture);
>> +
>> + if (texObj == NULL) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION, "glClear*TexImage(non-gen name)");
>> + return NULL;
>> + }
>> +
>> + if (texObj->Target == 0) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION, "glClear*TexImage(unbound tex)");
>> + return NULL;
>> + }
>> +
>> + return texObj;
>> +}
>> +
>> +static int
>> +get_tex_images_for_clear(struct gl_context *ctx,
>> + struct gl_texture_object *texObj,
>> + GLint level,
>> + struct gl_texture_image **texImages)
>> +{
>> + GLenum target;
>> + int i;
>> +
>> + if (level < 0 || level >= MAX_TEXTURE_LEVELS) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION, "glClear*TexImage(invalid level)");
>> + return 0;
>> + }
>> +
>> + if (texObj->Target == GL_TEXTURE_CUBE_MAP) {
>> + for (i = 0; i < MAX_FACES; i++) {
>> + target = GL_TEXTURE_CUBE_MAP_POSITIVE_X + i;
>> +
>> + texImages[i] = _mesa_select_tex_image(ctx, texObj, target, level);
>> + if (texImages[i] == NULL) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> + "glClear*TexImage(invalid level)");
>> + return 0;
>> + }
>> + }
>> +
>> + return MAX_FACES;
>> + }
>> +
>> + texImages[0] = _mesa_select_tex_image(ctx, texObj, texObj->Target, level);
>> +
>> + if (texImages[0] == NULL) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION, "glClear*TexImage(invalid level)");
>> + return 0;
>> + }
>> +
>> + return 1;
>> +}
>> +
>> +void GLAPIENTRY
>> +_mesa_ClearTexImage(GLuint texture, GLint level,
>> + GLenum format, GLenum type,
>> + const void *data)
>> +{
>> + GET_CURRENT_CONTEXT(ctx);
>> + struct gl_texture_object *texObj;
>> + struct gl_texture_image *texImages[MAX_FACES];
>> + int i, numImages;
>> +
>> + texObj = get_tex_obj_for_clear(ctx, texture);
>> +
>> + if (texObj == NULL)
>> + return;
>> +
>> + _mesa_lock_texture(ctx, texObj);
>> +
>> + numImages = get_tex_images_for_clear(ctx, texObj, level, texImages);
>> + if (numImages == 0)
>> + goto out;
>
> Is this actually necessary / better? With the loop condition below,
> this seems like extra code...
>
>> +
>> + for (i = 0; i < numImages; i++) {
>> + clear_tex_image(ctx, texImages[i], level,
>> + -texImages[i]->Border, /* xoffset */
>> + -texImages[i]->Border, /* yoffset */
>> + -texImages[i]->Border, /* zoffset */
>> + texImages[i]->Width,
>> + texImages[i]->Height,
>> + texImages[i]->Depth,
>> + format, type, data);
>> + }
>> +
>> + out:
>> + _mesa_unlock_texture(ctx, texObj);
>> +}
>> +
>> +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_object *texObj;
>> + struct gl_texture_image *texImages[MAX_FACES];
>> + int i, numImages;
>> + int minDepth, maxDepth;
>> +
>> + texObj = get_tex_obj_for_clear(ctx, texture);
>> +
>> + if (texObj == NULL)
>> + return;
>> +
>> + _mesa_lock_texture(ctx, texObj);
>> +
>> + numImages = get_tex_images_for_clear(ctx, texObj, level, texImages);
>> + if (numImages == 0)
>> + goto out;
>> +
>> + if (numImages == 1) {
>> + minDepth = -texImages[0]->Border;
>> + maxDepth = texImages[0]->Depth;
>> + } else {
>> + minDepth = 0;
>> + maxDepth = numImages;
>> + }
>> +
>> + if (xoffset < -texImages[0]->Border ||
>> + yoffset < -texImages[0]->Border ||
>> + zoffset < minDepth ||
>> + width < 0 ||
>> + height < 0 ||
>> + depth < 0 ||
>> + xoffset + width > texImages[0]->Width ||
>> + yoffset + height > texImages[0]->Height ||
>> + zoffset + depth > maxDepth) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> + "glClearSubTexImage(invalid dimensions)");
>> + goto out;
>> + }
>> +
>> + if (numImages == 1) {
>> + clear_tex_image(ctx, texImages[0], level,
>> + xoffset, yoffset, zoffset,
>> + width, height, depth,
>> + format, type, data);
>> + } else {
>> + for (i = zoffset; i < zoffset + depth; i++)
>> + clear_tex_image(ctx, texImages[i], level,
>> + xoffset, yoffset, 0,
>> + width, height, 1,
>> + format, type, data);
>> + }
>> +
>> + out:
>> + _mesa_unlock_texture(ctx, texObj);
>> +}
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list