From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35518) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbWBg-0004nC-VD for qemu-devel@nongnu.org; Mon, 14 Sep 2015 12:01:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZbWBc-0001p7-IZ for qemu-devel@nongnu.org; Mon, 14 Sep 2015 12:01:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45379) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbWBc-0001ot-Ag for qemu-devel@nongnu.org; Mon, 14 Sep 2015 12:00:56 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id DC280A37E2 for ; Mon, 14 Sep 2015 16:00:55 +0000 (UTC) References: <1441797654-15350-1-git-send-email-kraxel@redhat.com> <1441797654-15350-2-git-send-email-kraxel@redhat.com> From: Max Reitz Message-ID: <55F6EF34.4010703@redhat.com> Date: Mon, 14 Sep 2015 18:00:52 +0200 MIME-Version: 1.0 In-Reply-To: <1441797654-15350-2-git-send-email-kraxel@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="sLtjuEm7fEmfsq7FhJEGELNiVN9AGeLdT" Subject: Re: [Qemu-devel] [PATCH 1/9] shaders: initialize vertexes once List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --sLtjuEm7fEmfsq7FhJEGELNiVN9AGeLdT Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 09.09.2015 13:20, Gerd Hoffmann wrote: > Create a buffer for the vertex data and place vertexes > there at initialization time. Then just use the buffer > for each texture blit. >=20 > Signed-off-by: Gerd Hoffmann > --- > include/ui/shader.h | 4 +++- > ui/console-gl.c | 7 ++++++- > ui/shader.c | 32 +++++++++++++++++++++++++++----- > 3 files changed, 36 insertions(+), 7 deletions(-) >=20 > diff --git a/include/ui/shader.h b/include/ui/shader.h > index 8509596..f7d8618 100644 > --- a/include/ui/shader.h > +++ b/include/ui/shader.h > @@ -3,7 +3,9 @@ > =20 > #include > =20 > -void qemu_gl_run_texture_blit(GLint texture_blit_prog); > +GLuint qemu_gl_init_texture_blit(GLint texture_blit_prog); > +void qemu_gl_run_texture_blit(GLint texture_blit_prog, > + GLint texture_blit_vao); > =20 > GLuint qemu_gl_create_compile_shader(GLenum type, const GLchar *src); > GLuint qemu_gl_create_link_program(GLuint vert, GLuint frag); > diff --git a/ui/console-gl.c b/ui/console-gl.c > index cb45cf8..baf397b 100644 > --- a/ui/console-gl.c > +++ b/ui/console-gl.c > @@ -33,6 +33,7 @@ > =20 > struct ConsoleGLState { > GLint texture_blit_prog; > + GLint texture_blit_vao; > }; > =20 > /* -------------------------------------------------------------------= --- */ > @@ -47,6 +48,9 @@ ConsoleGLState *console_gl_init_context(void) > exit(1); > } > =20 > + gls->texture_blit_vao =3D > + qemu_gl_init_texture_blit(gls->texture_blit_prog); > + > return gls; > } > =20 > @@ -131,7 +135,8 @@ void surface_gl_render_texture(ConsoleGLState *gls,= > glClearColor(0.1f, 0.1f, 0.1f, 0.0f); > glClear(GL_COLOR_BUFFER_BIT); > =20 > - qemu_gl_run_texture_blit(gls->texture_blit_prog); > + qemu_gl_run_texture_blit(gls->texture_blit_prog, > + gls->texture_blit_vao); > } > =20 > void surface_gl_destroy_texture(ConsoleGLState *gls, > diff --git a/ui/shader.c b/ui/shader.c > index 52a4632..ada1c4c 100644 > --- a/ui/shader.c > +++ b/ui/shader.c > @@ -29,21 +29,43 @@ > =20 > /* -------------------------------------------------------------------= --- */ > =20 > -void qemu_gl_run_texture_blit(GLint texture_blit_prog) > +GLuint qemu_gl_init_texture_blit(GLint texture_blit_prog) > { > - GLfloat in_position[] =3D { > + static const GLfloat in_position[] =3D { > -1, -1, > 1, -1, > -1, 1, > 1, 1, > }; > GLint l_position; > + GLuint vao, buffer; > + > + glGenVertexArrays(1, &vao); > + glBindVertexArray(vao); > + > + /* this is the VBO that holds the vertex data */ > + glGenBuffers(1, &buffer); > + glBindBuffer(GL_ARRAY_BUFFER, buffer); > + glBufferData(GL_ARRAY_BUFFER, sizeof(in_position), in_position, > + GL_STATIC_DRAW); > =20 > - glUseProgram(texture_blit_prog); > l_position =3D glGetAttribLocation(texture_blit_prog, "in_position= "); > - glVertexAttribPointer(l_position, 2, GL_FLOAT, GL_FALSE, 0, in_pos= ition); > + glVertexAttribPointer(l_position, 2, GL_FLOAT, GL_FALSE, 0, 0); > glEnableVertexAttribArray(l_position); > - glDrawArrays(GL_TRIANGLE_STRIP, l_position, 4); > + > + glBindBuffer (GL_ARRAY_BUFFER, 0); > + glBindVertexArray (0); > + glDeleteBuffers (1, &buffer); Although I may be wrong, I don't think glVertexAttribPointer() loads the buffer data into the given vertex attribute. As far as I can see from the specification regarding vertex array objects, they only represent which data is to be used for vertex attributes ("All state related to the definition of data used by the vertex processor is encapsulated in a vertex array object."). glVertexAttribPointer(): (1) determines the attribute's format, (2) binds a vertex attribute index to a shader attribute index, (3) binds the data to the vertex attribute. Step (3) (glBindVertexBuffer()) does not appear to load the data, but only define its origin ("The source of data for a generic vertex attribute may be determined by attaching a buffer object to a vertex array object with [glBindVertexBuffer()]"). Therefore, I'm not sure whether deleting the buffer is right here. Maybe OpenGL uses reference counting here, too, so it remembers that the buffer is still in use by the VAO, and so the glDeleteBuffers() operation will only decrease its refcount, but not actually end up deleting it. But I don't think so (compare the documentation of glDeleteBuffers() with glDeleteShader(); the former says it will delete the buffer, whereas the latter explicitly mentions that the shader will not be deleted as long as it is attached to a program). All in all I don't think we should delete the buffer as long as it is in use by the VAO. Max > + > + return vao; > +} > + > +void qemu_gl_run_texture_blit(GLint texture_blit_prog, > + GLint texture_blit_vao) > +{ > + glUseProgram(texture_blit_prog); > + glBindVertexArray(texture_blit_vao); > + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); > } > =20 > /* -------------------------------------------------------------------= --- */ >=20 --sLtjuEm7fEmfsq7FhJEGELNiVN9AGeLdT Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJV9u80AAoJEDuxQgLoOKytZNYH/3NOevbXuBDX7BVlbvflMpEt ZT1fFyMgXwYgFsjhXg1gnpFwcXawU2y/okuII7RyZLwLTpj0JL3H+iLTxxHL83/P +G1ArXJ55h7649EmcwQQDA61WiM88zJuPcKPaRh2MGO2B/Vp1h+muGxF1u4bWMsE QPZ4Ax+NEbUfjOrdPtR2wh3ZbYHIBFhhLwmTxmGPOgG7QzWvw0LPvQDxqIn+4RLD T5Amoi0eOiNtTdjb1mybJrfyq46hDXESKSws1jW2XMfHUHEIT3imVAcddeQ31YFm I35/HfTLt9qLRMg/TphDbbRBAKAF95PRTXTM9fOx2JGtpyXAjqAKxm2heRlPdx4= =VNON -----END PGP SIGNATURE----- --sLtjuEm7fEmfsq7FhJEGELNiVN9AGeLdT--