All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 1/9] shaders: initialize vertexes once
Date: Mon, 14 Sep 2015 18:00:52 +0200	[thread overview]
Message-ID: <55F6EF34.4010703@redhat.com> (raw)
In-Reply-To: <1441797654-15350-2-git-send-email-kraxel@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 5280 bytes --]

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 <kraxel@redhat.com>
> ---
>  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 @@
>  
>  #include <epoxy/gl.h>
>  
> -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);
>  
>  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 @@
>  
>  struct ConsoleGLState {
>      GLint texture_blit_prog;
> +    GLint texture_blit_vao;
>  };
>  
>  /* ---------------------------------------------------------------------- */
> @@ -47,6 +48,9 @@ ConsoleGLState *console_gl_init_context(void)
>          exit(1);
>      }
>  
> +    gls->texture_blit_vao =
> +        qemu_gl_init_texture_blit(gls->texture_blit_prog);
> +
>      return gls;
>  }
>  
> @@ -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);
>  
> -    qemu_gl_run_texture_blit(gls->texture_blit_prog);
> +    qemu_gl_run_texture_blit(gls->texture_blit_prog,
> +                             gls->texture_blit_vao);
>  }
>  
>  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 @@
>  
>  /* ---------------------------------------------------------------------- */
>  
> -void qemu_gl_run_texture_blit(GLint texture_blit_prog)
> +GLuint qemu_gl_init_texture_blit(GLint texture_blit_prog)
>  {
> -    GLfloat in_position[] = {
> +    static const GLfloat in_position[] = {
>          -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);
>  
> -    glUseProgram(texture_blit_prog);
>      l_position = glGetAttribLocation(texture_blit_prog, "in_position");
> -    glVertexAttribPointer(l_position, 2, GL_FLOAT, GL_FALSE, 0, in_position);
> +    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);
>  }
>  
>  /* ---------------------------------------------------------------------- */
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

  parent reply	other threads:[~2015-09-14 16:01 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-09 11:20 [Qemu-devel] [PATCH 0/9] add virgl rendering support Gerd Hoffmann
2015-09-09 11:20 ` [Qemu-devel] [PATCH 1/9] shaders: initialize vertexes once Gerd Hoffmann
2015-09-10 12:40   ` Marc-André Lureau
2015-09-10 13:22     ` Gerd Hoffmann
2015-09-10 13:42       ` Marc-André Lureau
2015-09-10 14:59         ` Gerd Hoffmann
2015-09-14 16:00   ` Max Reitz [this message]
2015-09-14 16:19     ` Max Reitz
2015-09-15  7:10       ` Gerd Hoffmann
2015-09-09 11:20 ` [Qemu-devel] [PATCH 2/9] sdl2: quick & dirty flicker workaround Gerd Hoffmann
2015-09-10 12:40   ` Marc-André Lureau
2015-09-14 16:16   ` Max Reitz
2015-09-09 11:20 ` [Qemu-devel] [PATCH 3/9] ui/console: add opengl context and scanout support interfaces Gerd Hoffmann
2015-09-10 12:40   ` Marc-André Lureau
2015-09-15  8:30   ` Paolo Bonzini
2015-09-09 11:20 ` [Qemu-devel] [PATCH 4/9] virtio-gpu: update headers for virgl/3d Gerd Hoffmann
2015-09-10 12:41   ` Marc-André Lureau
2015-09-09 11:20 ` [Qemu-devel] [PATCH 5/9] virtio-gpu: add 3d mode and virgl rendering support Gerd Hoffmann
2015-09-14 18:14   ` Max Reitz
2015-09-15  7:33     ` Gerd Hoffmann
2015-09-15  8:33   ` Paolo Bonzini
2015-09-09 11:20 ` [Qemu-devel] [PATCH 6/9] sdl2/opengl: add opengl context and scanout support Gerd Hoffmann
2015-09-14 18:49   ` Max Reitz
2015-09-15  7:54     ` Gerd Hoffmann
2015-09-16 13:44       ` Max Reitz
2015-09-09 11:20 ` [Qemu-devel] [PATCH 7/9] opengl: add egl-context.[ch] helpers Gerd Hoffmann
2015-09-11 14:13   ` Marc-André Lureau
2015-09-09 11:20 ` [Qemu-devel] [PATCH 8/9] gtk/opengl: add opengl context and scanout support (egl) Gerd Hoffmann
2015-09-11 14:36   ` Marc-André Lureau
2015-09-09 11:20 ` [Qemu-devel] [PATCH 9/9] gtk/opengl: add opengl context and scanout support (GtkGLArea) Gerd Hoffmann
2015-09-11 14:44   ` Marc-André Lureau
2015-09-14 13:50     ` Gerd Hoffmann
2015-09-11 16:10 ` [Qemu-devel] [PATCH 0/9] add virgl rendering support Marc-André Lureau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55F6EF34.4010703@redhat.com \
    --to=mreitz@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.