[Piglit] [PATCH] attach a unbound renderbuffer should generate a GL_INVALID_OPERATION error instead of GL_INVALID_VALUE

[Zhou, Jin]

I agree. The new comment is very good.

Thanks,
Jin
-----Original Message-----
From: piglit-bounces at lists.freedesktop.org [mailto:piglit-bounces at lists.freedesktop.org] On Behalf Of Ian Romanick
Sent: Saturday, December 14, 2013 4:05 AM
To: Jin Zhou; piglit at lists.freedesktop.org
Subject: Re: [Piglit] [PATCH] attach a unbound renderbuffer should generate a GL_INVALID_OPERATION error instead of GL_INVALID_VALUE

On 12/10/2013 04:12 AM, Jin Zhou wrote:
> From: Jin Zhou <jin.zhou at amd.com>
> 
> a GL_INVALID_OPERATION should be generated instead of GL_INVALID_VALUE
> ---
>  tests/fbo/fbo-bind-renderbuffer.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/fbo/fbo-bind-renderbuffer.c b/tests/fbo/fbo-bind-renderbuffer.c
> index 60fbbf2..3b2b305 100644
> --- a/tests/fbo/fbo-bind-renderbuffer.c
> +++ b/tests/fbo/fbo-bind-renderbuffer.c
> @@ -22,7 +22,7 @@
>   */
>  
>  /*
> - * Does this sequence generate GL_INVALID_VALUE for the last command?
> + * Does this sequence generate GL_INVALID_OPERATION for the last command?
>   * It should generate an error since the renderbuffer was never bound.
>   *   glGenFramebuffers(1, &fb);
>   *   glGenRenderbuffers(1, &rb);

I think this test is from the time before we included spec quotations.
Let's update this comment with a spec quotation.  How about:

/* The GL_EXT_framebuffer_object spec says:
 *
 *    "<renderbuffer> must be either zero or the name of an existing
 *    renderbuffer object of type <renderbuffertarget>, otherwise
 *    INVALID_OPERATION is generated."
 *
 * This sequence should generate GL_INVALID_OPERATION since the renderbuffer
 * was never bound:
 *
 *   glGenFramebuffers(1, &fb);
 *   glGenRenderbuffers(1, &rb);
 *   glBindFramebuffer(GL_FRAMEBUFFER, fb);
 *   glFramebufferRenderbuffer(GL_FRAMEBUFFER_EXT, GL_COLOR_ATTACHMENT0_EXT,
 *                             GL_RENDERBUFFER_EXT, rb);
 */

With that change, this patch is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> @@ -77,7 +77,7 @@ piglit_display(void)
>        return PIGLIT_FAIL;
>     }
>  
> -   if (test_binding(GL_FALSE) != GL_INVALID_VALUE) {
> +   if (test_binding(GL_FALSE) != GL_INVALID_OPERATION) {
>        printf("fbo-bind-renderbuffer: failed to generate expected error\n");
>        return PIGLIT_FAIL;
>     }
> 

_______________________________________________
Piglit mailing list
Piglit at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit