[Mesa-dev] [PATCH] Allow glTexImage2D with a depth component cube map

[Kenneth Graunke]

On 11/22/2011 11:42 AM, Anuj Phogat wrote:
[snip]
>  I agree. This is simpler to read and _mesa_is_cube_map_face() can be reused
>  in mesa. Posting an updated patch with your suggestions.
> 
>  > I haven't tested this so the logic might be incorrect.
>  It worked as it is. Just defined a new function _mesa_is_cube_face().
> 
>  Thanks
>  Anuj

Actually, Brian already added that function; it's in teximage.h.

> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> ---
>  src/mesa/main/image.c    |   18 ++++++++++++++++++
>  src/mesa/main/image.h    |    3 +++
>  src/mesa/main/teximage.c |   25 ++++++++++++-------------
>  3 files changed, 33 insertions(+), 13 deletions(-)
> 
> diff --git a/src/mesa/main/image.c b/src/mesa/main/image.c
> index 7d95dd6..9ba5317 100644
> --- a/src/mesa/main/image.c
> +++ b/src/mesa/main/image.c
> @@ -1086,6 +1086,24 @@ _mesa_is_compressed_format(struct gl_context *ctx, GLenum format)
>     }
>  }
>  
> +/**
> + * Test if the given texture target is a cubemap face
> + */
> +GLboolean
> +_mesa_is_cube_map_face(GLenum target)
> +{
> +   switch (target) {
> +      case GL_TEXTURE_CUBE_MAP_POSITIVE_X :
> +      case GL_TEXTURE_CUBE_MAP_NEGATIVE_X :
> +      case GL_TEXTURE_CUBE_MAP_POSITIVE_Y :
> +      case GL_TEXTURE_CUBE_MAP_NEGATIVE_Y :
> +      case GL_TEXTURE_CUBE_MAP_POSITIVE_Z :
> +      case GL_TEXTURE_CUBE_MAP_NEGATIVE_Z :
> +         return GL_TRUE;
> +      default:
> +         return GL_FALSE;
> +   }
> +}
>  
>  /**
>   * Return the address of a specific pixel in an image (1D, 2D or 3D).
> diff --git a/src/mesa/main/image.h b/src/mesa/main/image.h
> index 46adaec..2c88578 100644
> --- a/src/mesa/main/image.h
> +++ b/src/mesa/main/image.h
> @@ -84,6 +84,9 @@ _mesa_is_integer_format(GLenum format);
>  extern GLboolean
>  _mesa_is_compressed_format(struct gl_context *ctx, GLenum format);
>  
> +extern GLboolean
> +_mesa_is_cube_map_face(GLenum target);
> +
>  extern GLvoid *
>  _mesa_image_address( GLuint dimensions,
>                       const struct gl_pixelstore_attrib *packing,
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index acf7187..43eec61 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -1593,28 +1593,27 @@ texture_error_check( struct gl_context *ctx,
>           return GL_TRUE;
>        }
>     }
> -
> -   /* additional checks for depth textures */
> +  
> +  /* additional checks for depth textures */
>     if (_mesa_base_tex_format(ctx, internalFormat) == GL_DEPTH_COMPONENT) {
> -      /* Only 1D, 2D, rect and array textures supported, not 3D or cubes */
> +     /* Only 1D, 2D, rect, array and cube textures supported, not 3D
> +      * Cubemaps are only supported for GL version > 3.0 or with EXT_gpu_shader4 */
>        if (target != GL_TEXTURE_1D &&
>            target != GL_PROXY_TEXTURE_1D &&
>            target != GL_TEXTURE_2D &&
> -          target != GL_PROXY_TEXTURE_2D &&
> -          target != GL_TEXTURE_1D_ARRAY &&
> -          target != GL_PROXY_TEXTURE_1D_ARRAY &&
> -          target != GL_TEXTURE_2D_ARRAY &&
>            target != GL_PROXY_TEXTURE_2D_ARRAY &&
>            target != GL_TEXTURE_RECTANGLE_ARB &&
> -          target != GL_PROXY_TEXTURE_RECTANGLE_ARB) {
> -         if (!isProxy)
> -            _mesa_error(ctx, GL_INVALID_ENUM,
> -                        "glTexImage(target/internalFormat)");
> -         return GL_TRUE;
> +          target != GL_PROXY_TEXTURE_RECTANGLE_ARB &&
> +         !((_mesa_is_cube_map_face(target) || target == GL_PROXY_TEXTURE_CUBE_MAP) &&
> +           (ctx->VersionMajor >= 3 || ctx->Extensions.EXT_gpu_shader4))) {
> +        if (!isProxy)
> +           _mesa_error(ctx, GL_INVALID_ENUM,
> +                       "glTexImage(target/internalFormat)");
> +        return GL_TRUE;
>        }
>     }
>
> -   /* additional checks for compressed textures */
> +  /* additional checks for compressed textures */
>     if (_mesa_is_compressed_format(ctx, internalFormat)) {
>        if (!target_can_be_compressed(ctx, target, internalFormat)) {
>           if (!isProxy)

Seems like some spurious whitespace changes got introduced here...this
isProxy stuff shouldn't be appearing in the diff.

Mesa coding style is 3-space indentation with 8-space tabs.  (Yeah, it's
quite unusual... :))

In vim I accomplish this via the following .vimrc line:
autocmd BufNewFile,BufRead /home/kwg/Projects/mesa/* set noexpandtab
tabstop=8 softtabstop=3 shiftwidth=3
I'm sure there are recipes for other editors.

Also, you might consider using git-send-email to send out your patches:

git send-email --to mesa-dev at lists.freedesktop.org --annotate HEAD~1..HEAD

...sends the top commit from your current branch to the list.  You can
change the HEAD~1 to send a whole series; --compose lets you write a
[PATCH 0/n] mail introducing the series if you like.

The nice thing about git-send-email is that recipients can save the
email as an mbox file (in Thunderbird, just right click and hit save)
and then use 'git am <file>' to apply the patch, committing it with the
proper commit message, authorship, etc.  It's really convenient.

Assuming the spurious hunks get dropped, this patch is
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

Thanks!