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

Paul Berry stereotype441 at gmail.com
Tue Aug 27 08:10:03 PDT 2013


On 27 August 2013 07:56, Ian Romanick <idr at freedesktop.org> wrote:

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


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.


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

Oops, you're right. Patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>


>
>  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 <stereotype441 at gmail.com>>>
>>
>>
>>              }
>>       #endif
>>       }
>>     --
>>     1.8.1.4
>>
>>     ______________________________**_________________
>>     Piglit mailing list
>>     Piglit at lists.freedesktop.org <mailto:Piglit at lists.**freedesktop.org<Piglit at lists.freedesktop.org>
>> >
>>     http://lists.freedesktop.org/**mailman/listinfo/piglit<http://lists.freedesktop.org/mailman/listinfo/piglit>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130827/35f91d6a/attachment-0001.html>


More information about the Piglit mailing list