[Mesa-dev] [PATCH] Mesa: Return GL error INVALID_OPERATION in case invalid format/type

[Xu, Randy]

Thanks, Matt

I just create a freedesktop issue 
Bug 99076 - dEQP-GLES3.functional.negative_api.texture#teximage3d fails due to wrong Error code

I distinguish GLES an GL as there is below comments in code, the _mesa_error_check_format_and_type function is not only used by glTexImage3D. 

/* From OpenGL 3.3 spec, page 220:
"If the format is DEPTH_STENCIL, then values are taken from
both the depth buffer and the stencil buffer. If there is no
depth buffer or if there is no stencil buffer, then the error
INVALID_OPERATION occurs. If the type parameter is not
UNSIGNED_INT_24_8 or FLOAT_32_UNSIGNED_INT_24_8_REV, then the
error INVALID_ENUM occurs."
*
OpenGL ES still generates GL_INVALID_OPERATION because glReadPixels
cannot be used to read depth or stencil in that API.
*/

Also above GL comment is still valid in 4.5 spec. 
https://www.opengl.org/registry/doc/glspec45.core.pdf page 510
An INVALID_ENUM error is generated if format is DEPTH_STENCIL and
type is not UNSIGNED_INT_24_8 or FLOAT_32_UNSIGNED_INT_24_8_-
REV

Thanks,
Randy

-----Original Message-----
From: Matt Turner [mailto:mattst88 at gmail.com] 
Sent: Wednesday, December 14, 2016 9:57 AM
To: Xu, Randy <randy.xu at intel.com>
Cc: mesa-dev at lists.freedesktop.org; Xu at freedesktop.org
Subject: Re: [Mesa-dev] [PATCH] Mesa: Return GL error INVALID_OPERATION in case invalid format/type

On Tue, Dec 13, 2016 at 4:28 AM, Randy Xu <randy.xu at intel.com> wrote:
> From: "Xu,Randy" <randy.xu at intel.com>

Please configure your name properly:

git config --global user.name "Randy Xu"

>
> Refer to GLES3.2 spec in 8.5
> Textures with a base internal format of DEPTH_COMPONENT, DEPTH_- 
> STENCIL or STENCIL_INDEX are supported by texture image specification 
> commands only if target is TEXTURE_2D, TEXTURE_2D_MULTISAMPLE, 
> TEXTURE_2D_ARRAY, TEXTURE_2D_MULTISAMPLE_ARRAY, TEXTURE_CUBE_- MAP or 
> TEXTURE_CUBE_MAP_ARRAY. Using these formats in conjunction with any 
> other target will result in an INVALID_OPERATION error.
> This patch can fix 
> dEQP-GLES3.functional.negative_api.texture.teximage3d
> failure.
>
> Jira: https://01.org/jira/browse/AIA-9

I work at Intel and I don't even know how to log into this. We really want publicly accessible links.

Wemake an exception for internal Khronos bug reports, since lots of people have access, but I don't think Jira on 01.org is an exceptional case.

Perhaps link to a freedesktop.org bug?

>
> Test: Pass dEQP-GLES3.functional.negative_api.texture.* and no 
> regression.

That's not necessary.

>
> Signed-off-by: Xu,Randy <randy.xu at intel.com>
> ---
>  src/mesa/main/glformats.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c 
> index a95909c..eeee66b 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -2087,6 +2087,8 @@ _mesa_error_check_format_and_type(const struct gl_context *ctx,
>           else if (ctx->Extensions.ARB_depth_buffer_float &&
>               type == GL_FLOAT_32_UNSIGNED_INT_24_8_REV)
>              return GL_NO_ERROR;
> +         else if (!_mesa_is_desktop_gl(ctx))

I looked at the GL 4.5 spec, and it contains effectively the same wording.

Please find when this changed, and then lets decide whether this is a "clarification" (i.e., intended behavior on all previous versions) or an actual change in behavior.

> +            return GL_INVALID_OPERATION;