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

[Tapani Pälli]

On 02/20/2013 07:59 PM, Chad Versace wrote:
> 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.

Thanks Chad, I'll follow your instructions with upcoming tests.


// Tapani