From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40871) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbWTT-0006rv-5g for qemu-devel@nongnu.org; Mon, 14 Sep 2015 12:19:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZbWTP-00088S-VF for qemu-devel@nongnu.org; Mon, 14 Sep 2015 12:19:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38143) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbWTP-00087s-Mv for qemu-devel@nongnu.org; Mon, 14 Sep 2015 12:19:19 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 21956A076A for ; Mon, 14 Sep 2015 16:19:19 +0000 (UTC) References: <1441797654-15350-1-git-send-email-kraxel@redhat.com> <1441797654-15350-2-git-send-email-kraxel@redhat.com> <55F6EF34.4010703@redhat.com> From: Max Reitz Message-ID: <55F6F384.1040107@redhat.com> Date: Mon, 14 Sep 2015 18:19:16 +0200 MIME-Version: 1.0 In-Reply-To: <55F6EF34.4010703@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Hj5J2OM5H9BShxLI1osxP67eHKntfVKbq" 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) --Hj5J2OM5H9BShxLI1osxP67eHKntfVKbq Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 14.09.2015 18:00, Max Reitz wrote: > 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. >> >> 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(-) >> >> 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_positio= n"); >> - glVertexAttribPointer(l_position, 2, GL_FLOAT, GL_FALSE, 0, in_po= sition); >> + 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); >=20 > Although I may be wrong, I don't think glVertexAttribPointer() loads th= e > buffer data into the given vertex attribute. >=20 > 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."). >=20 > 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. >=20 > 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()]"). >=20 > Therefore, I'm not sure whether deleting the buffer is right here. Mayb= e > 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). >=20 > All in all I don't think we should delete the buffer as long as it is i= n > use by the VAO. Oh, and I just noticed: With glDeleteBuffers(), I get a segmentation fault when qemu exits (somewhere deep in fglrx_dri.so). Without, the segfault disappears. 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 >=20 --Hj5J2OM5H9BShxLI1osxP67eHKntfVKbq 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 iQEcBAEBCAAGBQJV9vOEAAoJEDuxQgLoOKyttosIAJw8yI91FJGPxEfS9nWBfTcv PTs7cM63KxHCnBjTyIRRU0nlZlMJp7e7g/rX0twG9Lc/VUUe44QJw0WoJjfoq7zz 6gIl/MGToiUONVw6U+satNqsWJuVjxLjgWbbRVLgvlnZbWf+3mYV4+f36tRy7PHd GiAUxOGNqJt2MpflUmLUJiOnOt/K7oeDjIrLF6VvLqIUKOaLIiXEHOJLf5aoyaBl +jZm6ZmX2rCKzbn6rAx0Mfq+/k8hOiWvJvTFepInzvIpQIw3svWL0BfnCCM4bQI1 3XfNpnQhG78e2Qh60ya1x+VU3g5t9lyaWQrmZXIAFecErMVu9yjbO6QfN1R3lCQ= =pvCy -----END PGP SIGNATURE----- --Hj5J2OM5H9BShxLI1osxP67eHKntfVKbq--