[Piglit] [PATCH] util: specify channel sizes for choose_config_attribs()

[Brian Paul]

On 06/20/2016 10:25 AM, Jose Fonseca wrote:
> On 17/06/16 21:46, Brian Paul wrote:
>> WAFFLE_RED_SIZE=1 (for example) gets passed to glXChooseFBConfig as
>> GLX_RED_SIZE=1.  GLX chooses the deepest pixel format which meets that
>> minimum value so we typically get an 8 bit/channel format.
>>
>> But with wglChoosePixelFormatARB() we may get the shallowest pixel
>> format that meets that value so we may wind up getting a 5/6/5 format.
>> Many piglit tests aren't prepared for that and report failures because
>> the color tolerances are too tight.  In any case, we really want to test
>> 8-bit formats.
>>
>> This change tells waffle to first try 8/8/8 color, 24-bit Z.  And if
>> that fails, try 5/6/5 with 16-bit Z.
>> ---
>>   .../piglit-framework-gl/piglit_winsys_framework.c  | 28
>> ++++++++++++++++------
>>   1 file changed, 21 insertions(+), 7 deletions(-)
>>
>> diff --git a/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>> b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>> index ff3428a..315d6de 100644
>> --- a/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>> +++ b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>> @@ -102,8 +102,15 @@ post_redisplay(struct piglit_gl_framework *gl_fw)
>>       piglit_winsys_framework(gl_fw)->need_redisplay = true;
>>   }
>>
>> +/**
>> + * Create a integer array of WAFFLE_x attribute/value pairs from
>> + * the given test_config (which specifies color, depth, samples, etc.
>> + * \param min_color  minimum bits per channel for color components.
>> + * \param min_z  minimum bits for depth/Z buffer.
>> + */
>>   static const int32_t*
>> -choose_config_attribs(const struct piglit_gl_test_config *test_config)
>> +choose_config_attribs(const struct piglit_gl_test_config *test_config,
>> +              int min_color, int min_z)
>>   {
>>       static int32_t attrib_list[64];
>>       int i = 0;
>> @@ -112,21 +119,21 @@ choose_config_attribs(const struct
>> piglit_gl_test_config *test_config)
>>       if (test_config->window_visual &
>>           (PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_RGBA)) {
>>           attrib_list[i++] = WAFFLE_RED_SIZE;
>> -        attrib_list[i++] = 1;
>> +        attrib_list[i++] = min_color;
>>           attrib_list[i++] = WAFFLE_GREEN_SIZE;
>> -        attrib_list[i++] = 1;
>> +        attrib_list[i++] = min_color;
>>           attrib_list[i++] = WAFFLE_BLUE_SIZE;
>> -        attrib_list[i++] = 1;
>> +        attrib_list[i++] = min_color;
>>       }
>>
>>       if (test_config->window_visual & PIGLIT_GL_VISUAL_RGBA) {
>>           attrib_list[i++] = WAFFLE_ALPHA_SIZE;
>> -        attrib_list[i++] = 1;
>> +        attrib_list[i++] = min_color;
>>       }
>>
>>       if (test_config->window_visual & PIGLIT_GL_VISUAL_DEPTH) {
>>           attrib_list[i++] = WAFFLE_DEPTH_SIZE;
>> -        attrib_list[i++] = 1;
>> +        attrib_list[i++] = min_z;
>>       }
>>
>>       if (test_config->window_visual & PIGLIT_GL_VISUAL_STENCIL) {
>> @@ -198,8 +205,15 @@ piglit_winsys_framework_init(struct
>> piglit_winsys_framework *winsys_fw,
>>       struct piglit_gl_framework *gl_fw = &wfl_fw->gl_fw;
>>       bool ok = true;
>>
>> +    // First try to get 8-bit color channels, 24-bit Z
>>       ok = piglit_wfl_framework_init(wfl_fw, test_config, platform,
>> -                                   choose_config_attribs(test_config));
>> +                choose_config_attribs(test_config, 8, 24));
>> +    if (!ok) {
>> +        // Try shallower color/Z.  We may get 565 color and 16-bit Z.
>> +        ok = piglit_wfl_framework_init(wfl_fw, test_config, platform,
>> +                choose_config_attribs(test_config, 1, 1));
>> +    }
>> +
>>       if (!ok)
>>           goto fail;
>>
>>
>
>
> Seems a good idea to me.
>
> Reviewed-by: Jose Fonseca <jfonseca at vmware.com>
>
>
> Given certain tests appear to rely on having 8 bit precision, I think we
> should warn, and possibly even fail, when we don't get such visuals.
> Anything but silently fallback to lower precision.  We could maybe even
> check things like GL_RED_BITS / GL_GREEN_BITS / GL_BLUE_BITS for extra
> assurance, which could be done in a place agnostic to Waffle/GLUT/etc.

Yeah, we should probably at least print a warning when we don't get 
8-bit color.

If we do get a 5/6/5 format, the other thing we could do is set the 
piglit_tolerance[] values larger by calling piglit_set_tolerance_for_bits().

I'll take a closer look, but commit this patch in the mean time.

-Brian