[Mesa-dev] [PATCH 12/23] copytexture: update error checking for GLES3

[Anuj Phogat]

On Wed, Jan 9, 2013 at 3:38 AM, Jordan Justen <jljusten at gmail.com> wrote:
> On Mon, Jan 7, 2013 at 12:32 AM, Anuj Phogat <anuj.phogat at gmail.com> wrote:
>> On Sat, Jan 5, 2013 at 8:11 AM, Jordan Justen <jordan.l.justen at intel.com> wrote:
>>> Changes based on GTF/gles3 conformance test suite.
>>>
>>> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
>>> ---
>>>  src/mesa/main/teximage.c |   62 +++++++++++++++++++++++++++++++++-------------
>>>  1 file changed, 45 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>>> index 169e768..cb0084a 100644
>>> --- a/src/mesa/main/teximage.c
>>> +++ b/src/mesa/main/teximage.c
>>> @@ -2370,6 +2370,9 @@ copytexture_error_check( struct gl_context *ctx, GLuint dimensions,
>>>                           GLint width, GLint height, GLint border )
>>>  {
>>>     GLint baseFormat;
>>> +   GLint rb_base_format;
>>> +   struct gl_renderbuffer *rb;
>>> +   GLenum rb_internal_format;
>>>
>>>     /* check target */
>>>     if (!legal_texsubimage_target(ctx, dimensions, target)) {
>>> @@ -2414,31 +2417,56 @@ copytexture_error_check( struct gl_context *ctx, GLuint dimensions,
>>>        return GL_TRUE;
>>>     }
>>>
>>> -   /* OpenGL ES 1.x and OpenGL ES 2.0 impose additional restrictions on the
>>> -    * internalFormat.
>>> -    */
>>> -   if (_mesa_is_gles(ctx) && !_mesa_is_gles3(ctx)) {
>>> -      switch (internalFormat) {
>>> -      case GL_ALPHA:
>>> -      case GL_RGB:
>>> -      case GL_RGBA:
>>> -      case GL_LUMINANCE:
>>> -      case GL_LUMINANCE_ALPHA:
>>> -         break;
>>> -      default:
>>> -         _mesa_error(ctx, GL_INVALID_VALUE,
>>> -                     "glCopyTexImage%dD(internalFormat)", dimensions);
>>> -         return GL_TRUE;
>>> -      }
>>> +   rb = _mesa_get_read_renderbuffer(ctx, internalFormat);
>>> +   if (rb == NULL) {
>>> +      _mesa_error(ctx, GL_INVALID_OPERATION,
>>> +                  "glCopyTexImage%dD(read buffer)", dimensions);
>>> +      return GL_TRUE;
>>>     }
>>>
>>> +   rb_internal_format = rb->InternalFormat;
>>>     baseFormat = _mesa_base_tex_format(ctx, internalFormat);
>>> +   rb_base_format = _mesa_base_tex_format(ctx, rb->InternalFormat);
>>>     if (baseFormat < 0) {
>>> -      _mesa_error(ctx, GL_INVALID_VALUE,
>>> +      _mesa_error(ctx, GL_INVALID_OPERATION,
>>>                    "glCopyTexImage%dD(internalFormat)", dimensions);
>>>        return GL_TRUE;
>>>     }
>>>
>>> +   if (_mesa_is_color_format(internalFormat)) {
>>> +      if (rb_base_format < 0) {
>>> +         _mesa_error(ctx, GL_INVALID_VALUE,
>>> +                     "glCopyTexImage%dD(internalFormat)", dimensions);
>>> +         return GL_TRUE;
>>> +      }
>>> +   }
>>> +
>>> +   if (_mesa_is_gles(ctx)) {
>>> +      bool valid = true;
>>> +      if (_mesa_base_format_component_count(baseFormat) >
>>> +          _mesa_base_format_component_count(rb_base_format)) {
>>> +         valid = false;
>>> +      }
>>> +      if (baseFormat == GL_DEPTH_COMPONENT ||
>>> +          baseFormat == GL_DEPTH_STENCIL ||
>>> +          rb_base_format == GL_DEPTH_COMPONENT ||
>>> +          rb_base_format == GL_DEPTH_STENCIL ||
>>> +          ((baseFormat == GL_LUMINANCE_ALPHA ||
>>> +            baseFormat == GL_ALPHA) &&
>>> +           rb_base_format != GL_RGBA) ||
>>> +          internalFormat == GL_RGB9_E5) {
>>> +         valid = false;
>>> +      }
>>> +      if (internalFormat == GL_RGB9_E5) {
>>> +         valid = false;
>>> +      }
>>> +      if (!valid) {
>>> +         _mesa_error(ctx, GL_INVALID_OPERATION,
>>> +                     "glCopyTexImage%dD(internalFormat)", dimensions);
>>> +         return GL_TRUE;
>>> +      }
>>> +   }
>>> +
>> This looks incorrect as baseFormat=GL_DEPTH_COMPONENT is allowed in
>> gles3. It was not allowed in gles 1.1 and 2.0 You need to put an additional
>> check to test  ctx->Version < 30.
>
> I think depth/stencil is allowed for GLES3 with TexImage, but not for
> CopyTexImage. This code will cause CopyTexImage to be rejected on any
> GLES when the source or dest is depth/stencil.
>
> Does that seem correct?
Yes, Table 3.15 in gles 3.0 spec doesn't allow depth/stencil for CopyTexImage.
I was referring to gles 3.0 reference pages which now looks incorrect:
http://www.khronos.org/opengles/sdk/docs/man3/

I'll modify meta implementation of BlitFramebuffer() to account for this change.

>
> -Jordan