[Piglit] [PATCH v4] gles2: add fbo_discard_gles2 testcase

[Chad Versace]

Comments below.

On 02/19/2013 10:31 PM, Tapani Pälli wrote:
> tests GL_EXT_discard_framebuffer implementation
> 
> v2: changed to use piglit_check_gl_error instead of own macro
> v3: use correct enums, def_attach <-> usr_attach
> v4: quote errors section from extension specification
> 
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> ---
>  tests/all.tests                          |   1 +
>  tests/spec/gles-2.0/CMakeLists.gles2.txt |   1 +
>  tests/spec/gles-2.0/fbo-discard.c        | 151 +++++++++++++++++++++++++++++++
>  3 files changed, 153 insertions(+)
>  create mode 100644 tests/spec/gles-2.0/fbo-discard.c


> +/** @file fbo-discard.c
> + *
> + * Tests GL_EXT_discard_framebuffer implementation
> + *
> + * Test iterates over valid and invalid arguments and checks
> + * that the implementation returns correct error codes.
> + *
> + * GL_EXT_discard_framebuffer specification "Errors" section states:
> + *
> + *  "The error INVALID_ENUM is generated if DiscardFramebufferEXT is called
> + *   with a <target> that is not FRAMEBUFFER.
> + *
> + *   The error INVALID_ENUM is generated if DiscardFramebufferEXT is called with
> + *   a token other than COLOR_ATTACHMENT0, DEPTH_ATTACHMENT, or
> + *   STENCIL_ATTACHMENT in its <attachments> list when a framebuffer object is
> + *   bound to <target>.
> + *
> + *   The error INVALID_ENUM is generated if DiscardFramebufferEXT is called with
> + *   a token other than COLOR_EXT, DEPTH_EXT, or STENCIL_EXT in its
> + *   <attachments> list when the default framebuffer is bound to <target>.
> + *
> + *   The error INVALID_VALUE is generated if DiscardFramebufferEXT is called
> + *   with <numAttachments> less than zero."
> + */


Thanks for quoting the spec. This makes the test much easier to review for
correctness.


> +	glDiscardFramebufferEXT(GL_FRAMEBUFFER, 3, def_attach);
> +	for(p = def_attach; *p != 0; p++)
> +		glDiscardFramebufferEXT(GL_FRAMEBUFFER, 1, p);


Piglit mostly follows the Linux kernel style. So the above should be ``for (p =``;
space after the `for`, no space after the paren. This problem appears on
all the for-loops in the test.

Also, the test should check that each call to glDiscardFramebufferEXT above
produces no error. To do that, use ``pass &= piglit_check_gl_error(GL_NO_ERROR)``.


Otherwise, the test looks good.

To avoid another lengthy turnaround due to the timezone difference, and so
that we have another checked box in tomorrow's Android meeting, I took the
liberty to make the above changes and committed the patch.