<div dir="ltr">On 27 August 2013 18:45, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>><br>
<br>
This will be used in future commits.<br>
<br>
</div>v2: Use current program state to determine whether to use fixed-function<br>
attributes. There are two proposals in this patch. Hopefully reviewers<br>
will pick one. The first method checks whether the current program<br>
contains an attribute named "piglit_vertex". If it does, fixed-function<br>
attributes are not used. The other method check whether the current<br>
program contains an attribute whose name starts with "gl_". If it does,<br>
fixed-function attributes are used.<br>
<div class="im"><br>
Signed-off-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>><br>
Cc: Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>><br>
</div>Cc: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br>
Cc: Eric Anholt <<a href="mailto:eric@anholt.net">eric@anholt.net</a>><br>
---<br>
tests/util/piglit-util-gl-common.c | 123 ++++++++++++++++++++++++++++---------<br>
1 file changed, 95 insertions(+), 28 deletions(-)<br>
<br>
diff --git a/tests/util/piglit-util-gl-common.c b/tests/util/piglit-util-gl-common.c<br>
index b5e87bf..ec66a65 100644<br>
--- a/tests/util/piglit-util-gl-common.c<br>
+++ b/tests/util/piglit-util-gl-common.c<br>
@@ -603,42 +603,109 @@ required_gl_version_from_glsl_version(unsigned glsl_version)<br>
void<br>
<div class="im"> piglit_draw_rect_from_arrays(const void *verts, const void *tex)<br>
</div> {<br>
-#if defined(PIGLIT_USE_OPENGL_ES1) || defined(PIGLIT_USE_OPENGL)<br>
<div class="im">- if (verts) {<br>
- glVertexPointer(4, GL_FLOAT, 0, verts);<br>
- glEnableClientState(GL_VERTEX_ARRAY);<br>
- }<br>
</div>+#if defined(PIGLIT_USE_OPENGL_ES1)<br>
+ static const bool use_fixed_function_attributes = true;<br>
+#elif defined(PIGLIT_USE_OPENGL_ES2) || defined(PIGLIT_USE_OPENGL_ES3)<br>
+ static const bool use_fixed_function_attributes = false;<br></blockquote><div><br></div><div>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.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+#elif defined(PIGLIT_USE_OPENGL)<br>
+ bool use_fixed_function_attributes = true;<br>
<div class="im">+#else<br>
+#error "don't know how to draw arrays"<br>
+#endif<br>
<br>
</div><div class="im">- if (tex) {<br>
- glTexCoordPointer(2, GL_FLOAT, 0, tex);<br>
- glEnableClientState(GL_TEXTURE_COORD_ARRAY);<br>
</div>+#if defined(PIGLIT_USE_OPENGL)<br>
+ if (piglit_get_gl_version() >= 20<br>
+ || piglit_is_extension_supported("GL_ARB_shader_objects")) {<br>
+ GLuint prog;<br>
+<br>
+ glGetIntegerv(GL_CURRENT_PROGRAM, (GLint *) &prog);<br>
+<br>
+#define PLAN_A<br>
+#ifdef PLAN_A<br>
+ /* If there is a current program and that program has an<br>
+ * active attribute named piglit_vertex, don't use the fixed<br>
+ * function inputs.<br>
+ */<br>
+ use_fixed_function_attributes = prog == 0<br>
+ || glGetAttribLocation(prog, "piglit_vertex") == -1;<br>
+#elif defined PLAN_B<br>
+ if (prog != 0) {<br>
+ GLint num_attribs;<br>
+ int i;<br>
+ char name[64];<br>
+ GLint size;<br>
+ GLenum type;<br>
+<br>
+ /* Assume that fixed-function attributes won't be used<br>
+ * until an attribute with the name "gl_*" is found.<br>
+ */<br>
+ use_fixed_function_attributes = false;<br>
+<br>
+ glGetProgramiv(prog,<br>
+ GL_ACTIVE_ATTRIBUTES,<br>
+ &num_attribs);<br>
+<br>
+ for (i = 0; i < num_attribs; i++) {<br>
+ glGetActiveAttrib(prog,<br>
+ i,<br>
+ sizeof(name),<br>
+ NULL,<br>
+ &size,<br>
+ &type,<br>
+ name);<br>
+<br>
+ if (strncmp("gl_", name, 3) == 0) {<br>
+ use_fixed_function_attributes = true;<br>
+ break;<br>
+ }<br>
+ }<br>
+ }<br>
+#endif<br></blockquote><div><br></div><div>I have a strong preference for plan A. Here's my reasons:<br><br></div><div>1. It's simpler.<br><br></div><div>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).<br>
<br>3. It correctly classifies shaders like this:<br><br>in vec4 piglit_vertex;<br>out vec4 vertex_id;<br>void main()<br>{<br></div><div> gl_Position = piglit_vertex;<br></div><div> vertex_id = gl_VertexID;<br>}<br><br>
</div><div>(plan B would have incorrectly classified this as using fixed function attributes, because of the presence of gl_VertexID).<br><br></div><div>If we choose plan A, this patch is:<br><br>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br>
</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
}<br>
+#endif<br>
<br>
- glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);<br>
+#if defined(PIGLIT_USE_OPENGL_ES1) || defined(PIGLIT_USE_OPENGL)<br>
+ if (use_fixed_function_attributes) {<br>
<div class="im">+ if (verts) {<br>
+ glVertexPointer(4, GL_FLOAT, 0, verts);<br>
+ glEnableClientState(GL_VERTEX_ARRAY);<br>
+ }<br>
<br>
</div><div class="im">- if (verts)<br>
- glDisableClientState(GL_VERTEX_ARRAY);<br>
- if (tex)<br>
- glDisableClientState(GL_TEXTURE_COORD_ARRAY);<br>
-#elif defined(PIGLIT_USE_OPENGL_ES2) ||defined(PIGLIT_USE_OPENGL_ES3)<br>
- if (verts) {<br>
- glVertexAttribPointer(PIGLIT_ATTRIB_POS, 4, GL_FLOAT, GL_FALSE, 0, verts);<br>
- glEnableVertexAttribArray(PIGLIT_ATTRIB_POS);<br>
</div><div class="im">- }<br>
+ if (tex) {<br>
+ glTexCoordPointer(2, GL_FLOAT, 0, tex);<br>
+ glEnableClientState(GL_TEXTURE_COORD_ARRAY);<br>
+ }<br>
</div><div class="im">+<br>
+ glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);<br>
<br>
</div>- if (tex) {<br>
<div class="im">- glVertexAttribPointer(PIGLIT_ATTRIB_TEX, 2, GL_FLOAT, GL_FALSE, 0, tex);<br>
- glEnableVertexAttribArray(PIGLIT_ATTRIB_TEX);<br>
</div><div class="im">+ if (verts)<br>
+ glDisableClientState(GL_VERTEX_ARRAY);<br>
+ if (tex)<br>
+ glDisableClientState(GL_TEXTURE_COORD_ARRAY);<br>
}<br>
+#endif<br>
+#if defined(PIGLIT_USE_OPENGL_ES2) || defined(PIGLIT_USE_OPENGL_ES3) \<br>
+ || defined(PIGLIT_USE_OPENGL)<br>
</div>+ if (!use_fixed_function_attributes) {<br>
<div class="im">+ if (verts) {<br>
+ glVertexAttribPointer(PIGLIT_ATTRIB_POS, 4, GL_FLOAT,<br>
+ GL_FALSE, 0, verts);<br>
+ glEnableVertexAttribArray(PIGLIT_ATTRIB_POS);<br>
+ }<br>
<br>
</div>- glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);<br>
<div class="im">+ if (tex) {<br>
+ glVertexAttribPointer(PIGLIT_ATTRIB_TEX, 2, GL_FLOAT,<br>
+ GL_FALSE, 0, tex);<br>
+ glEnableVertexAttribArray(PIGLIT_ATTRIB_TEX);<br>
+ }<br>
<br>
</div>- if (verts)<br>
<div class="im">- glDisableVertexAttribArray(PIGLIT_ATTRIB_POS);<br>
- if (tex)<br>
- glDisableVertexAttribArray(PIGLIT_ATTRIB_TEX);<br>
-#else<br>
</div>-# error "don't know how to draw arrays"<br>
+ glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);<br>
+<br>
<div class="HOEnZb"><div class="h5">+ if (verts)<br>
+ glDisableVertexAttribArray(PIGLIT_ATTRIB_POS);<br>
+ if (tex)<br>
+ glDisableVertexAttribArray(PIGLIT_ATTRIB_TEX);<br>
+ }<br>
#endif<br>
}<br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
1.8.1.4<br>
<br>
</font></span></blockquote></div><br></div></div>