<div dir="ltr">On 27 August 2013 07:56, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> wrote: <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>On 08/27/2013 07:50 AM, Paul Berry wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div> On 26 August 2013 11:37, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a> </div><div> <mailto:<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>>> wrote: From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a> </div> <mailto:<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a>>><div> This will allow users of piglit_draw_rect_from_arrays to function in desktop OpenGL core profile. Signed-off-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a> </div> <mailto:<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a>>> Cc: Matt Turner <<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a> <mailto:<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>>> Cc: Paul Berry <<a href="mailto:stereoytpe441@gmail.com" target="_blank">stereoytpe441@gmail.com</a> <mailto:<a href="mailto:stereoytpe441@gmail.com" target="_blank">stereoytpe441@gmail.com</a>>><div><div> --- tests/util/piglit-util-gl-common.c | 53 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/tests/util/piglit-util-gl-common.c b/tests/util/piglit-util-gl-common.c index d95eeac..79edd76 100644 --- a/tests/util/piglit-util-gl-common.c +++ b/tests/util/piglit-util-gl-common.c @@ -24,6 +24,7 @@ #include "piglit-util-gl-common.h" #include <ctype.h> +#define BUFFER_OFFSET(i) ((char *)NULL + (i)) /** * An array of pointers to extension strings. @@ -642,15 +643,55 @@ piglit_draw_rect_from_arrays(const void *verts, const void *tex, #if defined(PIGLIT_USE_OPENGL_ES2) || defined(PIGLIT_USE_OPENGL_ES3) \ || defined(PIGLIT_USE_OPENGL) if (!USE_FF(fixed_function_attributes)) { + GLuint buf = 0; + GLuint old_buf = 0; + GLuint vao = 0; + GLuint old_vao = 0; + + /* Vertex array objects were added in both OpenGL 3.0 and + * OpenGL ES 3.0. The use of VAOs is required in desktop + * OpenGL 3.1 (without GL_ARB_compatibility) and all desktop + * OpenGL core profiles. If the functionality is supported, + * just use it. + */ + if (piglit_get_gl_version() >= 30 + || piglit_is_extension_supported("GL_OES_vertex_array_object") + || piglit_is_extension_supported("GL_ARB_vertex_array_object")) { + glGetIntegerv(GL_VERTEX_ARRAY_BINDING, + (GLint *) &old_vao); + glGenVertexArrays(1, &vao); + glBindVertexArray(vao); + } + + glGetIntegerv(GL_ARRAY_BUFFER_BINDING, + (GLint *) &old_buf); + glGenBuffers(1, &buf); + glBindBuffer(GL_ARRAY_BUFFER, buf); + glBufferData(GL_ARRAY_BUFFER, + (sizeof(GLfloat) * 4 * 4) + + (sizeof(GLfloat) * 4 * 2), + NULL, + GL_STATIC_DRAW); 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? </div></div></blockquote> I guess I'm assuming that any implementation that can do shaders can also do VBOs. Since VBOs predate shaders by several years, this is probably a reasonable assumption. Not checking also made it easier to transparently support the ES2 case.</blockquote> <div> </div><div>Ah, ok. I failed to think about the fact that this code is inside the if(!USE_FF(...)) block, and therefore won't get executed when testing really old OpenGL functionality. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div><div> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + if (verts) { + glBufferSubData(GL_ARRAY_BUFFER, + 0, + sizeof(GLfloat) * 4 * 4, + verts); glVertexAttribPointer(PIGLIT_ATTRIB_POS, 4, GL_FLOAT, - GL_FALSE, 0, verts); + GL_FALSE, 0, + BUFFER_OFFSET(0)); glEnableVertexAttribArray(PIGLIT_ATTRIB_POS); } if (tex) { + glBufferSubData(GL_ARRAY_BUFFER, + sizeof(GLfloat) * 4 * 4, + sizeof(GLfloat) * 4 * 2, + tex); glVertexAttribPointer(PIGLIT_ATTRIB_TEX, 2, GL_FLOAT, - GL_FALSE, 0, tex); + GL_FALSE, 0, + BUFFER_OFFSET(sizeof(GLfloat) * 4 * 4)); glEnableVertexAttribArray(PIGLIT_ATTRIB_TEX); } @@ -660,6 +701,14 @@ piglit_draw_rect_from_arrays(const void *verts, const void *tex, glDisableVertexAttribArray(PIGLIT_ATTRIB_POS); if (tex) glDisableVertexAttribArray(PIGLIT_ATTRIB_TEX); + + glBindBuffer(GL_ARRAY_BUFFER, old_buf); + glDeleteBuffers(1, &buf); + + if (vao != 0) { + glBindVertexArray(old_vao); + glDeleteVertexArrays(1, &vao); + } 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). </blockquote> </div></div> No, I think it does the right thing. If it calls glGenVertexArrays, vao will not be zero. Right? The if-test checks vao, not old_vao. </blockquote><div> </div><div>Oops, you're right. Patch is: </div> <div>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div> I think you need to do something like this instead: bool use_vaos = piglit_get_gl_version() >= 30 || ...; if (use_vaos) { glGetIntegerv(GL_VERTEX_ARRAY_BINDING...); ... } ... if (use_vaos) { glBindVertexArray(old_vao); glDeleteVertexArrays(1, &vao); } With that fixed, this patch is: Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a> </div> <mailto:<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>>><div> } #endif } -- 1.8.1.4 _______________________________________________ Piglit mailing list </div> <a href="mailto:Piglit@lists.freedesktop.org" target="_blank">Piglit@lists.freedesktop.org</a> <mailto:<a href="mailto:Piglit@lists.freedesktop.org" target="_blank">Piglit@lists.freedesktop.org</a>> <a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a> </blockquote> </blockquote></div> </div></div>