[Piglit] [PATCH] colormaterial-query test

[Brian Paul]

On 05/23/2012 09:15 AM, Tapani Pälli wrote:
> On 05/23/2012 06:03 PM, Brian Paul wrote:
>> On 05/23/2012 12:01 AM, Tapani Pälli wrote:
>>> introduce a new test which checks that GL_COLOR_MATERIAL state
>>> does not get lost with sequential queries.
>>>
>>> Signed-off-by: Tapani Pälli<tapani.palli at intel.com>
>>> ---
>>>    tests/all.tests                     |    1 +
>>>    tests/general/CMakeLists.gl.txt     |    1 +
>>>    tests/general/colormaterial-query.c |   80
>>> +++++++++++++++++++++++++++++++++++
>>>    3 files changed, 82 insertions(+), 0 deletions(-)
>>>    create mode 100644 tests/general/colormaterial-query.c
>>>
>>> diff --git a/tests/all.tests b/tests/all.tests
>>> index 59d2983..5286f55 100644
>>> --- a/tests/all.tests
>>> +++ b/tests/all.tests
>>> @@ -260,6 +260,7 @@ add_plain_test(general, 'dlist-fdo3129-01')
>>>    add_plain_test(general, 'dlist-fdo3129-02')
>>>    add_plain_test(general, 'dlist-fdo31590')
>>>    add_plain_test(general, 'draw-arrays-colormaterial')
>>> +add_plain_test(general, 'colormaterial-query')
>>>    add_plain_test(general, 'draw-batch')
>>>    add_plain_test(general, 'draw-copypixels-sync')
>>>    add_plain_test(general, 'draw-elements')
>>> diff --git a/tests/general/CMakeLists.gl.txt
>>> b/tests/general/CMakeLists.gl.txt
>>> index 2c93b56..5929d3b 100644
>>> --- a/tests/general/CMakeLists.gl.txt
>>> +++ b/tests/general/CMakeLists.gl.txt
>>> @@ -31,6 +31,7 @@ piglit_add_executable (depth_clamp depth_clamp.c)
>>>    piglit_add_executable (depthfunc depthfunc.c)
>>>    piglit_add_executable (depthrange-clear depthrange-clear.c)
>>>    piglit_add_executable (draw-arrays-colormaterial
>>> draw-arrays-colormaterial.c)
>>> +piglit_add_executable (colormaterial-query colormaterial-query.c)
>>>    piglit_add_executable (draw-batch draw-batch.c)
>>>    piglit_add_executable (draw-copypixels-sync draw-copypixels-sync.c)
>>>    piglit_add_executable (draw-elements draw-elements.c)
>>> diff --git a/tests/general/colormaterial-query.c
>>> b/tests/general/colormaterial-query.c
>>> new file mode 100644
>>> index 0000000..4934dca
>>> --- /dev/null
>>> +++ b/tests/general/colormaterial-query.c
>>> @@ -0,0 +1,80 @@
>>> +/*
>>> + * Copyright © 2012 Intel Corporation
>>> + *
>>> + * Permission is hereby granted, free of charge, to any person
>>> obtaining a
>>> + * copy of this software and associated documentation files (the
>>> "Software"),
>>> + * to deal in the Software without restriction, including without
>>> limitation
>>> + * the rights to use, copy, modify, merge, publish, distribute,
>>> sublicense,
>>> + * and/or sell copies of the Software, and to permit persons to whom the
>>> + * Software is furnished to do so, subject to the following conditions:
>>> + *
>>> + * The above copyright notice and this permission notice (including
>>> the next
>>> + * paragraph) shall be included in all copies or substantial portions
>>> of the
>>> + * Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>> EXPRESS OR
>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>> MERCHANTABILITY,
>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
>>> SHALL
>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
>>> OR OTHER
>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>>> ARISING
>>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>>> OTHER DEALINGS
>>> + * IN THE SOFTWARE.
>>> + *
>>> + */
>>> +
>>> +/**
>>> + * \file colormaterial-query.c
>>> + * Test sequential calls to glMaterialfv and glGetMaterialfv
>>> + *
>>> + * This test exercises a Mesa bug where 2 sequential queries with
>>> + * glGetMaterialfv get wrong result.
>>> + *
>>> + * \author Tapani Pälli<tapani.palli at intel.com>
>>> + */
>>> +
>>> +#include "piglit-util.h"
>>> +
>>> +int piglit_width = 100, piglit_height = 100;
>>> +int piglit_window_mode = GLUT_RGBA | GLUT_ALPHA | GLUT_DOUBLE;
>>> +
>>> +enum piglit_result
>>> +piglit_display(void)
>>> +{
>>> +    static const GLfloat ambient[4] = { 0.6f, 0.6f, 0.6f, 1.0f};
>>> +    static const GLfloat color[4] = { 1.0f, 0.0f, 0.0f, 1.0f};
>>> +    static GLfloat result[4];
>>> +
>>> +    GLboolean pass = GL_TRUE;
>>> +
>>> +    glEnable(GL_COLOR_MATERIAL);
>>
>> You're not calling glColorMaterial() to indicate which material values
>> are tracking glColor.  The defaults are GL_FRONT_AND_BACK /
>> GL_AMBIENT_AND_DIFFUSE.  It might be nice to have a call to
>> glColorMaterial() even with the default values to make things a little
>> more obvious to the reader.
>
> Yep, I originally had this but I did not want to do this as
> glColorMaterial is not specified in OpenGL ES and I want the test to
> work with that as well, also it is not necessary for triggering the bug.

OK, so let's at least add a comment about what materials (AMBIENT and 
DIFFUSE) are being set by the glColor calls.  I had to look it up in 
Mesa to know.

I haven't checked the ES spec, but if it supports GL_COLOR_MATERIAL 
but not glColorMaterial, does glColor always override the ambient and 
diffuse values only?


>> Ideally, a test called "colormaterial-query" would test querying all the
>> different material attributes in various circumstances.  But I guess
>> this is OK.  Others might feel differently.
>
> Yep, it could iterate through different properties, it just feels a bit
> unnecessary as this is really about specific bug and the bug would
> happen with any property or with any face parameter.

I saw the other message about putting this in the bugs/ directory, but 
I seem to recall that some people want to get rid of that.  I'm sure 
someone will chime in if that's the case.

-Brian