[Mesa-dev] [PATCH 3/3] getteximage: assume texture image is empty for non defined levels

[Iago Toral]

On Wed, 2018-04-11 at 19:51 +0200, Juan A. Suarez Romero wrote:
> Current code is returning an INVALID_OPERATION when trying to use
> getTextureImage() on a level that has not been explicitly defined.
> 
> That is, we define a mipmapped Texture2D with 3 levels, and try to
> use
> GetTextureImage() for the 4th levels, and INVALID_OPERATION is
> returned.
> 
> Nevertheless, such case is not listed as an error in OpenGL 4.6 spec,
> section 8.11.4 ("Texture Image Queries"), where all the case errors
> for
> this function are defined. So it seems this is a valid operation.

It says this:

An INVALID_VALUE error is generated if level is negative or larger than
the maximum allowable level.

> On the other hand, in section 8.22 ("Texture State and Proxy State")
> it
> states:
> 
>   "Each initial texture image is null. It has zero width, height, and
>    depth, internal format RGBA, or R8 for buffer textures, component
>    sizes set to zero and component types set to NONE, the compressed
>    flag set to FALSE, a zero compressed size, and the bound buffer
>    object name is zero."
> 
> We can assume that we are reading this initialized empty image when
> calling GetTextureImage() with a non defined level.

Based on the above, I think we should just change the error code to be
INVALID_VALUE instead of INVALID_OPERATION.

> With this assumption, we will reach one of the other error cases
> defined
> for the functions. At the end, this means that we will return a
> different error to the caller.
> 
> This fixes arb_get_texture_sub_image piglit tests.
> 
> Signed-off-by: Juan A. Suarez Romero <jasuarez at igalia.com>
> ---
>  src/mesa/main/texgetimage.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/src/mesa/main/texgetimage.c
> b/src/mesa/main/texgetimage.c
> index 85d0ffd4770..0e4f030cb08 100644
> --- a/src/mesa/main/texgetimage.c
> +++ b/src/mesa/main/texgetimage.c
> @@ -913,6 +913,7 @@ dimensions_error_check(struct gl_context *ctx,
>                         const char *caller)
>  {
>     const struct gl_texture_image *texImage;
> +   GLuint image_width, image_height, image_depth;
>     int i;
>  
>     if (xoffset < 0) {
> @@ -1004,37 +1005,39 @@ dimensions_error_check(struct gl_context
> *ctx,
>  
>     texImage = select_tex_image(texObj, target, level, zoffset);
>     if (!texImage) {
> -      /* missing texture image */
> -      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(missing image)",
> caller);
> -      return true;
> +      /* missing texture image; continue as initialized as empty */
> +      _mesa_warning(ctx, "%s(missing image)", caller);
>     }
>  
> -   if (xoffset + width > texImage->Width) {
> +   image_width = texImage ? texImage->Width : 0;
> +   if (xoffset + width > image_width) {
>        _mesa_error(ctx, GL_INVALID_VALUE,
>                    "%s(xoffset %d + width %d > %u)",
> -                  caller, xoffset, width, texImage->Width);
> +                  caller, xoffset, width, image_width);
>        return true;
>     }
>  
> -   if (yoffset + height > texImage->Height) {
> +   image_height = texImage ? texImage->Height : 0;
> +   if (yoffset + height > image_height) {
>        _mesa_error(ctx, GL_INVALID_VALUE,
>                    "%s(yoffset %d + height %d > %u)",
> -                  caller, yoffset, height, texImage->Height);
> +                  caller, yoffset, height, image_height);
>        return true;
>     }
>  
>     if (target != GL_TEXTURE_CUBE_MAP) {
>        /* Cube map error checking was done above */
> -      if (zoffset + depth > texImage->Depth) {
> +      image_depth = texImage ? texImage->Depth : 0;
> +      if (zoffset + depth > image_depth) {
>           _mesa_error(ctx, GL_INVALID_VALUE,
>                       "%s(zoffset %d + depth %d > %u)",
> -                     caller, zoffset, depth, texImage->Depth);
> +                     caller, zoffset, depth, image_depth);
>           return true;
>        }
>     }
>  
>     /* Extra checks for compressed textures */
> -   {
> +   if (texImage) {
>        GLuint bw, bh, bd;
>        _mesa_get_format_block_size_3d(texImage->TexFormat, &bw, &bh,
> &bd);
>        if (bw > 1 || bh > 1 || bd > 1) {