[Piglit] [PATCH 04/13] util: Allow non-ES2 code to draw_rect with non-fixed-function arrays
Ian Romanick
idr at freedesktop.org
Wed Aug 28 10:55:06 PDT 2013
On 08/28/2013 09:36 AM, Paul Berry wrote:
> On 27 August 2013 18:45, Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
>
> From: Ian Romanick <ian.d.romanick at intel.com
> <mailto:ian.d.romanick at intel.com>>
>
> This will be used in future commits.
>
> v2: Use current program state to determine whether to use fixed-function
> attributes. There are two proposals in this patch. Hopefully reviewers
> will pick one. The first method checks whether the current program
> contains an attribute named "piglit_vertex". If it does, fixed-function
> attributes are not used. The other method check whether the current
> program contains an attribute whose name starts with "gl_". If it does,
> fixed-function attributes are used.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com
> <mailto:ian.d.romanick at intel.com>>
> Cc: Matt Turner <mattst88 at gmail.com <mailto:mattst88 at gmail.com>>
> Cc: Paul Berry <stereotype441 at gmail.com
> <mailto:stereotype441 at gmail.com>>
> Cc: Eric Anholt <eric at anholt.net <mailto:eric at anholt.net>>
> ---
> tests/util/piglit-util-gl-common.c | 123
> ++++++++++++++++++++++++++++---------
> 1 file changed, 95 insertions(+), 28 deletions(-)
>
> diff --git a/tests/util/piglit-util-gl-common.c
> b/tests/util/piglit-util-gl-common.c
> index b5e87bf..ec66a65 100644
> --- a/tests/util/piglit-util-gl-common.c
> +++ b/tests/util/piglit-util-gl-common.c
> @@ -603,42 +603,109 @@
> required_gl_version_from_glsl_version(unsigned glsl_version)
> void
> piglit_draw_rect_from_arrays(const void *verts, const void *tex)
> {
> -#if defined(PIGLIT_USE_OPENGL_ES1) || defined(PIGLIT_USE_OPENGL)
> - if (verts) {
> - glVertexPointer(4, GL_FLOAT, 0, verts);
> - glEnableClientState(GL_VERTEX_ARRAY);
> - }
> +#if defined(PIGLIT_USE_OPENGL_ES1)
> + static const bool use_fixed_function_attributes = true;
> +#elif defined(PIGLIT_USE_OPENGL_ES2) || defined(PIGLIT_USE_OPENGL_ES3)
> + static const bool use_fixed_function_attributes = false;
>
>
> I realize I'm quibbling here, but using "static" on these consts seems
> strange to me. Since the compiler is going to optimize them away
> anyhow, it seems silly to ask it to put them in the data segment. In
> fact, it makes me worry that it might defeat constant folding for some
> stupid reason.
>
>
> +#elif defined(PIGLIT_USE_OPENGL)
> + bool use_fixed_function_attributes = true;
> +#else
> +#error "don't know how to draw arrays"
> +#endif
>
> - if (tex) {
> - glTexCoordPointer(2, GL_FLOAT, 0, tex);
> - glEnableClientState(GL_TEXTURE_COORD_ARRAY);
> +#if defined(PIGLIT_USE_OPENGL)
> + if (piglit_get_gl_version() >= 20
> + || piglit_is_extension_supported("GL_ARB_shader_objects")) {
> + GLuint prog;
> +
> + glGetIntegerv(GL_CURRENT_PROGRAM, (GLint *) &prog);
> +
> +#define PLAN_A
> +#ifdef PLAN_A
> + /* If there is a current program and that program has an
> + * active attribute named piglit_vertex, don't use
> the fixed
> + * function inputs.
> + */
> + use_fixed_function_attributes = prog == 0
> + || glGetAttribLocation(prog,
> "piglit_vertex") == -1;
> +#elif defined PLAN_B
> + if (prog != 0) {
> + GLint num_attribs;
> + int i;
> + char name[64];
> + GLint size;
> + GLenum type;
> +
> + /* Assume that fixed-function attributes
> won't be used
> + * until an attribute with the name "gl_*"
> is found.
> + */
> + use_fixed_function_attributes = false;
> +
> + glGetProgramiv(prog,
> + GL_ACTIVE_ATTRIBUTES,
> + &num_attribs);
> +
> + for (i = 0; i < num_attribs; i++) {
> + glGetActiveAttrib(prog,
> + i,
> + sizeof(name),
> + NULL,
> + &size,
> + &type,
> + name);
> +
> + if (strncmp("gl_", name, 3) == 0) {
> +
> use_fixed_function_attributes = true;
> + break;
> + }
> + }
> + }
> +#endif
>
>
> I have a strong preference for plan A. Here's my reasons:
>
> 1. It's simpler.
That is its primary advantage. As a disadvantage, it means that we have
to have a variable named "piglit_vertex". I've noticed that most of the
existing test that use 'draw arrays' name the vertex input "vertex".
Plan B would correctly identify those.
What I wanted as a query that would tell me what name was bound to
attribute 0. I believe ARB_program_interface_query gives this ability,
but alas.
> 2. I didn't have to look at the GL spec to reassure myself that it was
> correct (with plan B, I was briefly worried that glGetActiveAttrib would
> only return information about user-defined vertex inputs).
>
> 3. It correctly classifies shaders like this:
>
> in vec4 piglit_vertex;
> out vec4 vertex_id;
> void main()
> {
> gl_Position = piglit_vertex;
> vertex_id = gl_VertexID;
> }
Are you sure? That seems surprising to me. It's a vertex shader input,
but it's not really an attribute (you can't source it's value from
anywhere). I wouldn't have thought that any of the gl_*ID variables
would be returned by glGetActiveAttrib.
> (plan B would have incorrectly classified this as using fixed function
> attributes, because of the presence of gl_VertexID).
>
> If we choose plan A, this patch is:
>
> Reviewed-by: Paul Berry <stereotype441 at gmail.com
> <mailto:stereotype441 at gmail.com>>
>
>
>
> }
> +#endif
>
> - glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
> +#if defined(PIGLIT_USE_OPENGL_ES1) || defined(PIGLIT_USE_OPENGL)
> + if (use_fixed_function_attributes) {
> + if (verts) {
> + glVertexPointer(4, GL_FLOAT, 0, verts);
> + glEnableClientState(GL_VERTEX_ARRAY);
> + }
>
> - if (verts)
> - glDisableClientState(GL_VERTEX_ARRAY);
> - if (tex)
> - glDisableClientState(GL_TEXTURE_COORD_ARRAY);
> -#elif defined(PIGLIT_USE_OPENGL_ES2) ||defined(PIGLIT_USE_OPENGL_ES3)
> - if (verts) {
> - glVertexAttribPointer(PIGLIT_ATTRIB_POS, 4,
> GL_FLOAT, GL_FALSE, 0, verts);
> - glEnableVertexAttribArray(PIGLIT_ATTRIB_POS);
> - }
> + if (tex) {
> + glTexCoordPointer(2, GL_FLOAT, 0, tex);
> + glEnableClientState(GL_TEXTURE_COORD_ARRAY);
> + }
> +
> + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
>
> - if (tex) {
> - glVertexAttribPointer(PIGLIT_ATTRIB_TEX, 2,
> GL_FLOAT, GL_FALSE, 0, tex);
> - glEnableVertexAttribArray(PIGLIT_ATTRIB_TEX);
> + if (verts)
> + glDisableClientState(GL_VERTEX_ARRAY);
> + if (tex)
> + glDisableClientState(GL_TEXTURE_COORD_ARRAY);
> }
> +#endif
> +#if defined(PIGLIT_USE_OPENGL_ES2) || defined(PIGLIT_USE_OPENGL_ES3) \
> + || defined(PIGLIT_USE_OPENGL)
> + if (!use_fixed_function_attributes) {
> + if (verts) {
> + glVertexAttribPointer(PIGLIT_ATTRIB_POS, 4,
> GL_FLOAT,
> + GL_FALSE, 0, verts);
> + glEnableVertexAttribArray(PIGLIT_ATTRIB_POS);
> + }
>
> - glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
> + if (tex) {
> + glVertexAttribPointer(PIGLIT_ATTRIB_TEX, 2,
> GL_FLOAT,
> + GL_FALSE, 0, tex);
> + glEnableVertexAttribArray(PIGLIT_ATTRIB_TEX);
> + }
>
> - if (verts)
> - glDisableVertexAttribArray(PIGLIT_ATTRIB_POS);
> - if (tex)
> - glDisableVertexAttribArray(PIGLIT_ATTRIB_TEX);
> -#else
> -# error "don't know how to draw arrays"
> + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
> +
> + if (verts)
> + glDisableVertexAttribArray(PIGLIT_ATTRIB_POS);
> + if (tex)
> + glDisableVertexAttribArray(PIGLIT_ATTRIB_TEX);
> + }
> #endif
> }
>
> --
> 1.8.1.4
More information about the Piglit
mailing list