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

[Jose Fonseca]

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.


Jose