[Piglit] [PATCH v2] ARB_vertex_attrib_64bit: add doubles support to attributes test

[Alejandro Piñeiro]

LGTM. Just a nitpick below. But in any case:
Reviewed-by: Alejandro Piñeiro <apinheiro at igalia.com>

On 25/06/18 10:45, Timothy Arceri wrote:
> ---
>
>  V2: Add missing opengl.py entry.
>
>  tests/general/attribs.c | 68 +++++++++++++++++++++++++++++++++++++++--
>  tests/opengl.py         |  3 +-
>  2 files changed, 67 insertions(+), 4 deletions(-)
>
> diff --git a/tests/general/attribs.c b/tests/general/attribs.c
> index 05a0d4a1e..bf627dece 100644
> --- a/tests/general/attribs.c
> +++ b/tests/general/attribs.c
> @@ -53,6 +53,7 @@ enum {
>  };
>  
>  enum {
> +	DOUBLE_TYPE,
>  	FLOAT_TYPE,
>  	INT_TYPE,
>  	UINT_TYPE
> @@ -120,12 +121,13 @@ static GLboolean test(int x, int y, const char *shaderfunc,
>  		      const char *info)
>  {
>  	static const char *templ = {
> +		"%s \n"

Is this extra line really needed?

>  		"%s \n"
>  		"#extension GL_ARB_explicit_attrib_location : require \n"
>  		"layout(location = 1) in %s attr; \n"
>  		"void main() { \n"
>  		"  gl_Position = gl_ModelViewProjectionMatrix * gl_Vertex; \n"
> -		"  gl_FrontColor = (%s) * vec4(1.0, 1.0, 1.0, 0.5); \n"
> +		"  gl_FrontColor = vec4(%s %s) * vec4(1.0, 1.0, 1.0, 0.5); \n"
>  		"} \n"
>  	};
>  	GLuint prog, vs;
> @@ -139,10 +141,56 @@ static GLboolean test(int x, int y, const char *shaderfunc,
>  		{0.5, 0.3, 0.9, 0.2}
>  	};
>  
> +	char *defaults;
> +	char *type_string;
> +
> +	/* From Section 11.1.1 (Vertex Attributes) of the OpenGL 4.6
> +	 * Compatibility Profile spec:
> +	 *
> +	 *    "Scalar and vector vertex attribute types and VertexAttrib*
> +	 *    commands used to set the values of the corresponding generic
> +	 *    attribute. values are provided if the values of the vertex
> +	 *    attribute variable are specified with fewer components than
> +	 *    required for the attribute variable. For example, the fourth
> +	 *    component of a variable of type dvec4 will be undefined if
> +	 *    specified using VertexAttribL3dv, or using a vertex array
> +	 *    specified with VertexAttribLPointer and a size of three.
> +	 *
> +	 * TODO: We should probably also be doing this for attribute functions
> +	 *       other than doubles.
> +	 */
> +	if (type == DOUBLE_TYPE) {
> +		switch (mask) {
> +		case R:
> +			type_string = "double";
> +			defaults = ", dvec3(0.0, 0.0, 1.0)";
> +			break;
> +		case RG:
> +			type_string = "dvec2";
> +			defaults = ", dvec2(0.0, 1.0)";
> +			break;
> +		case RGB:
> +			type_string = "dvec3";
> +			defaults = ", 1.0";
> +			break;
> +		case RGBA:
> +			type_string = "dvec4";
> +			defaults = "";
> +			break;
> +		default:
> +			assert(0);
> +		}
> +	} else {
> +		defaults = "";
> +		type_string =
> +			type == INT_TYPE ? "ivec4" : type == UINT_TYPE ? "uvec4" : "vec4";
> +	}
> +
>  	sprintf(vstext, templ,
>  		type != FLOAT_TYPE ? "#version 130" : "",
> -		type == INT_TYPE ? "ivec4" : type == UINT_TYPE ? "uvec4" : "vec4",
> -		shaderfunc);
> +		type == DOUBLE_TYPE ?
> +		"#extension GL_ARB_gpu_shader_fp64: enable\n#extension GL_ARB_vertex_attrib_64bit: enable" : "",
> +		type_string, shaderfunc, defaults);
>  
>  	/* Create the shader. */
>  	vs = piglit_compile_shader_text(GL_VERTEX_SHADER, vstext);
> @@ -343,6 +391,19 @@ static test_func tests_GL3[] = {
>  	test_glVertexAttribI4ui,
>  };
>  
> +/* ARB_vertex_attrib_64bit */
> +DEFINE_TEST(glVertexAttribL1d,,  (1, x),		"attr", R, DOUBLE_TYPE, "")
> +DEFINE_TEST(glVertexAttribL2d,,  (1, x, y),		"attr", RG, DOUBLE_TYPE, "")
> +DEFINE_TEST(glVertexAttribL3d,,  (1, x, y, z),		"attr", RGB, DOUBLE_TYPE, "")
> +DEFINE_TEST(glVertexAttribL4d,,  (1, x, y, z, w),	"attr", RGBA, DOUBLE_TYPE, "")
> +
> +static test_func tests_GL_ARB_vertex_attrib_64bit[] = {
> +	test_glVertexAttribL1d,
> +	test_glVertexAttribL2d,
> +	test_glVertexAttribL3d,
> +	test_glVertexAttribL4d,
> +};
> +
>  /* ARB_vertex_type_2_10_10_10_rev */
>  /* Packing functions for a signed normalized 2-bit component.
>   * These are based on equation 2.2 and 2.3 from the opengl specification, see:
> @@ -503,6 +564,7 @@ struct test_set {
>  } test_sets[] = {
>  	{ TESTS(GL2) },
>  	{ TESTS(GL3), 30 },
> +	{ TESTS(GL_ARB_vertex_attrib_64bit), 32 },
>  	{ TESTS(GL_ARB_vertex_type_2_10_10_10_rev), 0, "GL_ARB_vertex_type_2_10_10_10_rev" },
>  };
>  
> diff --git a/tests/opengl.py b/tests/opengl.py
> index 669d9055b..3ed00e5b7 100644
> --- a/tests/opengl.py
> +++ b/tests/opengl.py
> @@ -4697,7 +4697,8 @@ with profile.test_list.group_manager(
>  with profile.test_list.group_manager(
>          PiglitGLTest,
>          grouptools.join('spec', 'arb_vertex_attrib_64bit')) as g:
> -    g(['arb_vertex_attrib_64bit-double_attribs'], 'double_attribs')
> +    g(['attribs', 'GL_ARB_vertex_attrib_64bit'], 'attribs')
> +    g(['arb_vertex_attrib_64bit-double_attribs'], 'get_double_attribs')
>      g(['arb_vertex_attrib_64bit-check-explicit-location'], 'check-explicit-location')
>      g(['arb_vertex_attrib_64bit-getactiveattrib'], 'getactiveattrib')
>      g(['arb_vertex_attrib_64bit-max-vertex-attrib'], 'max-vertex-attrib')