[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