[Mesa-dev] [PATCH] Mesa: Fix error code for glTexImage3D in GLES

[Chad Versace]

On Fri 16 Dec 2016, Randy Xu wrote:
> From: "Xu,Randy" <randy.xu at intel.com>
> 
> The ES specification says that TexImage3D should return GL_INVALID_OPERATION
> if the internal format is DEPTH_COMPONENT, DEPTH_-STENCIL or STENCIL_INDEX.
> The current code returns INVALID_ENUM as _mesa_error_check_format_and_type is
> used by glReadPixels also and the GL specification defines "INVALID_ENUM is
> generated if format is DEPTH_STENCIL and type is not UNSIGNED_INT_24_8 or
> FLOAT_32_UNSIGNED_INT_24_8_- REV".
> 
> This patch only impacts GLES, which can generate GL_INVALID_OPERATION because
> glReadPixels cannot be used to read depth or stencil buffer.
> Fixes dEQP-GLES3.functional.negative_api.texture.teximage3d.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99076
> 
> Signed-off-by: Xu,Randy <randy.xu at intel.com>
> ---
>  src/mesa/main/glformats.c | 2 ++
>  1 file changed, 2 insertions(+)

Thanks for fixing the dEQP failure. But I think your patch applies the
fix to wrong portion of code.

The commit message mentions the internalFormat, but the patch updates
a function to which validates the *format* (not internalFormat).
I believe the change should instead be placed in
teximage.c:texture_format_error_check_gles(), which is better for
2 reasons:
    - That function specifically checks GLES-specific requirements like
      this.
    - It checks the *internalFormat* in addition to the *format*.

Also, in the future, please remove the empty lines between the tags
(Bugzilla:, Signed-of-by:) in the commit message. The empty lines can
confuse scripts that parse those tags.