<div dir="ltr">On 26 August 2013 11:37, 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">
From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>><br>
<br>
This will allow users of piglit_draw_rect_from_arrays to function in<br>
desktop OpenGL core profile.<br>
<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>
Cc: Paul Berry <<a href="mailto:stereoytpe441@gmail.com">stereoytpe441@gmail.com</a>><br>
---<br>
 tests/util/piglit-util-gl-common.c | 53 ++++++++++++++++++++++++++++++++++++--<br>
 1 file changed, 51 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/tests/util/piglit-util-gl-common.c b/tests/util/piglit-util-gl-common.c<br>
index d95eeac..79edd76 100644<br>
--- a/tests/util/piglit-util-gl-common.c<br>
+++ b/tests/util/piglit-util-gl-common.c<br>
@@ -24,6 +24,7 @@<br>
 #include "piglit-util-gl-common.h"<br>
 #include <ctype.h><br>
<br>
+#define BUFFER_OFFSET(i) ((char *)NULL + (i))<br>
<br>
 /**<br>
  * An array of pointers to extension strings.<br>
@@ -642,15 +643,55 @@ piglit_draw_rect_from_arrays(const void *verts, const void *tex,<br>
 #if defined(PIGLIT_USE_OPENGL_ES2) || defined(PIGLIT_USE_OPENGL_ES3) \<br>
        || defined(PIGLIT_USE_OPENGL)<br>
        if (!USE_FF(fixed_function_attributes)) {<br>
+               GLuint buf = 0;<br>
+               GLuint old_buf = 0;<br>
+               GLuint vao = 0;<br>
+               GLuint old_vao = 0;<br>
+<br>
+               /* Vertex array objects were added in both OpenGL 3.0 and<br>
+                * OpenGL ES 3.0.  The use of VAOs is required in desktop<br>
+                * OpenGL 3.1 (without GL_ARB_compatibility) and all desktop<br>
+                * OpenGL core profiles.  If the functionality is supported,<br>
+                * just use it.<br>
+                */<br>
+               if (piglit_get_gl_version() >= 30<br>
+                   || piglit_is_extension_supported("GL_OES_vertex_array_object")<br>
+                   || piglit_is_extension_supported("GL_ARB_vertex_array_object")) {<br>
+                       glGetIntegerv(GL_VERTEX_ARRAY_BINDING,<br>
+                                     (GLint *) &old_vao);<br>
+                       glGenVertexArrays(1, &vao);<br>
+                       glBindVertexArray(vao);<br>
+               }<br>
+<br>
+               glGetIntegerv(GL_ARRAY_BUFFER_BINDING,<br>
+                             (GLint *) &old_buf);<br>
+               glGenBuffers(1, &buf);<br>
+               glBindBuffer(GL_ARRAY_BUFFER, buf);<br>
+               glBufferData(GL_ARRAY_BUFFER,<br>
+                            (sizeof(GLfloat) * 4 * 4)<br>
+                            + (sizeof(GLfloat) * 4 * 2),<br>
+                       NULL,<br>
+                       GL_STATIC_DRAW);<br></blockquote><div><br></div><div>Shouldn't we also condition this code on the presence of ARB_vertex_buffer_object?  Or has that extension been around long enough that we don't care to test systems that don't support it?<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
                if (verts) {<br>
+                       glBufferSubData(GL_ARRAY_BUFFER,<br>
+                                       0,<br>
+                                       sizeof(GLfloat) * 4 * 4,<br>
+                                       verts);<br>
                        glVertexAttribPointer(PIGLIT_ATTRIB_POS, 4, GL_FLOAT,<br>
-                                             GL_FALSE, 0, verts);<br>
+                                             GL_FALSE, 0,<br>
+                                             BUFFER_OFFSET(0));<br>
                        glEnableVertexAttribArray(PIGLIT_ATTRIB_POS);<br>
                }<br>
<br>
                if (tex) {<br>
+                       glBufferSubData(GL_ARRAY_BUFFER,<br>
+                                       sizeof(GLfloat) * 4 * 4,<br>
+                                       sizeof(GLfloat) * 4 * 2,<br>
+                                       tex);<br>
                        glVertexAttribPointer(PIGLIT_ATTRIB_TEX, 2, GL_FLOAT,<br>
-                                             GL_FALSE, 0, tex);<br>
+                                             GL_FALSE, 0,<br>
+                                             BUFFER_OFFSET(sizeof(GLfloat) * 4 * 4));<br>
                        glEnableVertexAttribArray(PIGLIT_ATTRIB_TEX);<br>
                }<br>
<br>
@@ -660,6 +701,14 @@ piglit_draw_rect_from_arrays(const void *verts, const void *tex,<br>
                        glDisableVertexAttribArray(PIGLIT_ATTRIB_POS);<br>
                if (tex)<br>
                        glDisableVertexAttribArray(PIGLIT_ATTRIB_TEX);<br>
+<br>
+               glBindBuffer(GL_ARRAY_BUFFER, old_buf);<br>
+               glDeleteBuffers(1, &buf);<br>
+<br>
+               if (vao != 0) {<br>
+                       glBindVertexArray(old_vao);<br>
+                       glDeleteVertexArrays(1, &vao);<br>
+               }<br></blockquote><div><br></div><div>I don't think this does what you want in the case where the implementation supports vao's, but there was no vao bound before the call to piglit_draw_rect_from_arrays().  (What it does is: generate a vao, bind it, use it, and then leave it bound.  What I think you want is: generate a vao, bind it, use it, unbind it, and delete it).<br>
<br></div><div>I think you need to do something like this instead:<br><br>bool use_vaos = piglit_get_gl_version() >= 30 || ...;<br></div><div>if (use_vaos) {<br></div><div>   glGetIntegerv(GL_VERTEX_ARRAY_BINDING...);<br>
   ...<br>}<br><br>...<br><br></div><div>if (use_vaos) {<br></div><div>   glBindVertexArray(old_vao);<br></div><div>   glDeleteVertexArrays(1, &vao);<br>}<br><br></div><div>With that fixed, this patch is:<br><br>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><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>
<span class="HOEnZb"><font color="#888888">--<br>
1.8.1.4<br>
<br>
_______________________________________________<br>
Piglit mailing list<br>
<a href="mailto:Piglit@lists.freedesktop.org">Piglit@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a><br>
</font></span></blockquote></div><br></div></div>