[Piglit] [PATCH 06/16] util: Use a VAO and VBO for generic attributes in piglit_draw_rect_from_arrays

Ian Romanick idr at freedesktop.org
Tue Aug 27 07:56:15 PDT 2013


On 08/27/2013 07:50 AM, Paul Berry wrote:
> On 26 August 2013 11:37, 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 allow users of piglit_draw_rect_from_arrays to function in
>     desktop OpenGL core profile.
>
>     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 <stereoytpe441 at gmail.com
>     <mailto:stereoytpe441 at gmail.com>>
>     ---
>       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?

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.

>     +
>                      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).

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.

> 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 <stereotype441 at gmail.com
> <mailto:stereotype441 at gmail.com>>
>
>              }
>       #endif
>       }
>     --
>     1.8.1.4
>
>     _______________________________________________
>     Piglit mailing list
>     Piglit at lists.freedesktop.org <mailto:Piglit at lists.freedesktop.org>
>     http://lists.freedesktop.org/mailman/listinfo/piglit



More information about the Piglit mailing list