[PATCH 9/9] glamor: Use buffer_storage

[Markus Wick]

Am 2014-03-09 05:07, schrieb Eric Anholt:
> v2:
>   - Make the default buffer size a #define. (by Markus Wick)
>   - Fix the return offset for mapping with buffer_storage.  (oops!)
> v3:
>   - Avoid GL error at first rendering from unmapping no buffer.
>   - Rebase on the glBindBuffer(GL_ARRAY_BUFFER, 0) change.
> v4: Rebase on Markus's vbo init changes.
> 
> Signed-off-by: Eric Anholt <eric at anholt.net>
> ---
>  glamor/glamor.c      |  2 ++
>  glamor/glamor_priv.h |  1 +
>  glamor/glamor_vbo.c  | 51 
> +++++++++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/glamor/glamor.c b/glamor/glamor.c
> index dc69c72..e856179 100644
> --- a/glamor/glamor.c
> +++ b/glamor/glamor.c
> @@ -353,6 +353,8 @@ glamor_init(ScreenPtr screen, unsigned int flags)
>          glamor_gl_has_extension("GL_MESA_pack_invert");
>      glamor_priv->has_fbo_blit =
>          glamor_gl_has_extension("GL_EXT_framebuffer_blit");
> +    glamor_priv->has_buffer_storage =
> +        glamor_gl_has_extension("GL_ARB_buffer_storage");
>      glGetIntegerv(GL_MAX_RENDERBUFFER_SIZE, 
> &glamor_priv->max_fbo_size);
>  #ifdef MAX_FBO_SIZE
>      glamor_priv->max_fbo_size = MAX_FBO_SIZE;
> diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
> index 986e729..d15eabd 100644
> --- a/glamor/glamor_priv.h
> +++ b/glamor/glamor_priv.h
> @@ -208,6 +208,7 @@ typedef struct glamor_screen_private {
>      enum glamor_gl_flavor gl_flavor;
>      int has_pack_invert;
>      int has_fbo_blit;
> +    int has_buffer_storage;
>      int max_fbo_size;
> 
>      struct xorg_list
> diff --git a/glamor/glamor_vbo.c b/glamor/glamor_vbo.c
> index be2c2af..f736cbe 100644
> --- a/glamor/glamor_vbo.c
> +++ b/glamor/glamor_vbo.c
> @@ -52,7 +52,49 @@ glamor_get_vbo_space(ScreenPtr screen, unsigned
> size, char **vbo_offset)
> 
>      glBindBuffer(GL_ARRAY_BUFFER, glamor_priv->vbo);
> 
> -    if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) {
> +    if (glamor_priv->has_buffer_storage) {
> +        if (glamor_priv->vbo_size < glamor_priv->vbo_offset + size) {
> +            if (glamor_priv->vbo_size)
> +                glUnmapBuffer(GL_ARRAY_BUFFER);
> +
> +            if (size > glamor_priv->vbo_size) {
> +                glamor_priv->vbo_size = MAX(GLAMOR_VBO_SIZE, size);
> +
> +                /* We aren't allowed to resize glBufferStorage()
> +                 * buffers, so we need to gen a new one.
> +                 */
> +                glDeleteBuffers(1, &glamor_priv->vbo);
> +                glGenBuffers(1, &glamor_priv->vbo);
> +                glBindBuffer(GL_ARRAY_BUFFER, glamor_priv->vbo);
> +
> +                assert(glGetError() == GL_NO_ERROR);

On release builds, glGetError won't be called and so this won't clear 
the gl error log. So we'll fall back to the mbr code because of any gl 
error somewhere in glamor.

> +                glBufferStorage(GL_ARRAY_BUFFER, 
> glamor_priv->vbo_size, NULL,
> +                                GL_MAP_WRITE_BIT |
> +                                GL_MAP_PERSISTENT_BIT |
> +                                GL_MAP_COHERENT_BIT);
> +
> +                if (glGetError() != GL_NO_ERROR) {
> +                    /* If the driver failed our coherent mapping, fall
> +                     * back to the ARB_mbr path.
> +                     */
> +                    glamor_priv->has_buffer_storage = false;
> +                    glamor_priv->vbo_size = 0;

glamor_put_context(glamor_priv);

> +                    return glamor_get_vbo_space(screen, size, 
> vbo_offset);
> +                }
> +            }
> +
> +            glamor_priv->vbo_offset = 0;
> +            glamor_priv->vb = glMapBufferRange(GL_ARRAY_BUFFER,
> +                                               0, 
> glamor_priv->vbo_size,
> +                                               GL_MAP_WRITE_BIT |
> +                                               
> GL_MAP_INVALIDATE_BUFFER_BIT |
> +                                               GL_MAP_PERSISTENT_BIT |
> +                                               GL_MAP_COHERENT_BIT);
> +        }
> +        *vbo_offset = (void *)(uintptr_t)glamor_priv->vbo_offset;
> +        data = glamor_priv->vb + glamor_priv->vbo_offset;
> +        glamor_priv->vbo_offset += size;
> +    } else if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) {
>          if (glamor_priv->vbo_size < glamor_priv->vbo_offset + size) {
>              glamor_priv->vbo_size = MAX(GLAMOR_VBO_SIZE, size);
>              glamor_priv->vbo_offset = 0;
> @@ -99,7 +141,12 @@ glamor_put_vbo_space(ScreenPtr screen)
> 
>      glamor_get_context(glamor_priv);
> 
> -    if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) {
> +    if (glamor_priv->has_buffer_storage) {
> +        /* If we're in the ARB_buffer_storage path, we have a
> +         * persistent mapping, so we can leave it around until we
> +         * reach the end of the buffer.
> +         */
> +    } else if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) {
>          glUnmapBuffer(GL_ARRAY_BUFFER);
>      } else {
>          glBufferData(GL_ARRAY_BUFFER, glamor_priv->vbo_offset,

Which kind of error do you expect for a driver which isn't able to map 
coherent? I've only found a GL_OUT_OF_MEMORY, but this will invalidate 
the complete gl state. So I guess there is no legal way to support 
arb_buffer_storage without coherent mapping at all.