[Mesa-dev] [PATCH 2/3] main: Checking for cube completeness in GetTextureImage.

[Chad Versace]

On 12/31/2014 05:26 PM, Laura Ekstrand wrote:
> This is part of a potential solution to Khronos Bug 13223.  Cube completeness
> is a concept from glGenerateMipmap, but it seems reasonable to check for it in
> GetTextureImage when the target is GL_TEXTURE_CUBE_MAP.
> ---
>  src/mesa/main/texgetimage.c | 41 +++++++++++++++++++++++++++++------------
>  1 file changed, 29 insertions(+), 12 deletions(-)
> 
> diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
> index 5b4d294..060c68a 100644
> --- a/src/mesa/main/texgetimage.c
> +++ b/src/mesa/main/texgetimage.c
> @@ -1105,18 +1105,35 @@ _mesa_GetTextureImage( GLuint texture, GLint level, GLenum format,
>                       "glGetTextureImage(insufficient cube map storage)");
>           return;
>        }
> -      for (i = 0; i < 6; ++i) { /* For each face. */
> -         if (!texObj->Image[i][level]) {
> -            /* Not enough image planes for a cube map.  The spec does not say
> -             * what should happen in this case because the user has always
> -             * specified each cube face separately (using
> -             * GL_TEXTURE_CUBE_MAP_POSITIVE_X+i) in previous GL versions.
> -             * This is addressed in Khronos Bug 13223.
> -             */
> -            _mesa_error(ctx, GL_INVALID_OPERATION,
> -                        "glGetTextureImage(insufficient cube map storage)");
> -            return;
> -         }
> +
> +      /*
> +       * This is part of a potential solution to a bug in the spec. According
> +       * to Section 8.17 Texture Completeness in the OpenGL 4.5 Core Profile
> +       * spec (30.10.2014):
> +       *    "[A] cube map texture is cube complete if the
> +       *    following conditions all hold true: The [base level] texture
> +       *    images of each of the six cube map faces have identical, positive,
> +       *    and square dimensions. The [base level] images were each specified
> +       *    with the same internal format."
> +       *
> +       * It seems reasonable to check for cube completeness of an arbitrary
> +       * level here so that the returned data has a consistent format and size
> +       * and therefore fits in the user's buffer.
> +       */

The above comment is very confusing, unless the reader has had a conversation with
you about the topic ;). The comment alludes to a "bug in the spec" and "a potential solution",
but the comment does not state what the bug is. The reader has to guess based on the clues
you give. For the sake of the future devs, please directly describe the bug.

> +      if (!_mesa_cube_level_complete(texObj, level)) {
> +         /*
> +          * Precedence for this error:
> +          * In Section 8.14.4 Manual Mipmap Generation, the OpenGL 4.5 Core
> +          * Profile spec (30.10.2014) says:
> +          *    "An INVALID_OPERATION error is generated by
> +          *    GenerateTextureMipmap if the effective target is
> +          *    TEXTURE_CUBE_MAP or TEXTURE_CUBE_MAP_ARRAY, and the texture
> +          *    object is not cube complete or cube array complete,
> +          *    respectively."
> +          */

I suggest removing the comment above. I don't believe you need to provide precedent
for emitting GL_INVALID_OPERATION here, since it is a catch-all generic error.
Just a suggestion.

> +         _mesa_error(ctx, GL_INVALID_OPERATION,
> +                     "glGetTextureImage(cube map incomplete)");
> +         return;
>        }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 884 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150101/c9504208/attachment.sig>