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

[Iago Toral]

On Thu, 2018-04-12 at 17:59 +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.
> 
> 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.
> 
> 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.

I would chancge the last sentence to:

"In the end this means that we would end up returning INVALID_VALUE to
the caller"

> 
> This fixes arb_get_texture_sub_image piglit tests.
> 
> v2: just return INVALID_VALUE if there isno defined level (Iago)
                                           ^
                                          white space

Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

> 
> Signed-off-by: Juan A. Suarez Romero <jasuarez at igalia.com>
> ---
>  src/mesa/main/texgetimage.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/main/texgetimage.c
> b/src/mesa/main/texgetimage.c
> index 85d0ffd4770..69521c5dd05 100644
> --- a/src/mesa/main/texgetimage.c
> +++ b/src/mesa/main/texgetimage.c
> @@ -1004,8 +1004,31 @@ 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);
> +      /* Trying to return a non-defined level is a valid operation
> per se, as
> +       * OpenGL 4.6 spec, section 8.11.4 ("Texture Image Queries")
> does not
> +       * handle this case as an error.
> +       *
> +       * Rather, we need to look at section 8.22 ("Texture State and
> Proxy
> +       * State"):
> +       *
> +       *   "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."
> +       *
> +       * This means we need to assume the image for the non-defined
> level is
> +       * an empty image. With this assumption, we can go back to
> section
> +       * 8.11.4 and checking again the errors:
> +       *
> +       *   "An INVALID_VALUE error is generated if xoffset + width
> is greater
> +       *    than the texture’s width, yoffset + height is greater
> than the
> +       *    texture’s height, or zoffset + depth is greater than the
> texture’s
> +       *    depth."
> +       *
> +       * Thus why we return INVALID_VALUE.
> +       */
> +      _mesa_error(ctx, GL_INVALID_VALUE, "%s(missing image)",
> caller);
>        return true;
>     }
>