[Piglit] [PATCH 04/13] util: Allow non-ES2 code to draw_rect with non-fixed-function arrays
Paul Berry
stereotype441 at gmail.com
Wed Aug 28 09:36:36 PDT 2013
On 27 August 2013 18:45, Ian Romanick <idr at freedesktop.org> wrote:
> From: Ian Romanick <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>
> Cc: Matt Turner <mattst88 at gmail.com>
> Cc: Paul Berry <stereotype441 at gmail.com>
> Cc: Eric Anholt <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.
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;
}
(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>
> }
> +#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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130828/42b91d55/attachment.html>
More information about the Piglit
mailing list