[Mesa-dev] [PATCH 20/23] meta: Don't pollute the buffer object namespace in _mesa_meta_DrawTex

Anuj Phogat anuj.phogat at gmail.com
Fri Nov 13 10:43:46 PST 2015


On Mon, Nov 9, 2015 at 4:56 PM, Ian Romanick <idr at freedesktop.org> wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> tl;dr: For many types of GL object, we can *NEVER* use the Gen function.
>
> In OpenGL ES (all versions!) and OpenGL compatibility profile,
> applications don't have to call Gen functions.  The GL spec is very
> clear about how you can mix-and-match generated names and non-generated
> names: you can use any name you want for a particular object type until
> you call the Gen function for that object type.
>
> Here's the problem scenario:
>
>  - Application calls a meta function that generates a name.  The first
>    Gen will probably return 1.
>
>  - Application decides to use the same name for an object of the same
>    type without calling Gen.  Many demo programs use names 1, 2, 3,
>    etc. without calling Gen.
>
>  - Application calls the meta function again, and the meta function
>    replaces the data.  The application's data is lost, and the app
>    fails.  Have fun debugging that.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92363
> ---
>  src/mesa/drivers/common/meta.c                 | 51 +++++++++++---------------
>  src/mesa/drivers/common/meta.h                 |  5 ++-
>  src/mesa/drivers/common/meta_blit.c            |  5 +--
>  src/mesa/drivers/common/meta_generate_mipmap.c |  6 +--
>  4 files changed, 29 insertions(+), 38 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
> index 57993cf..b06f683 100644
> --- a/src/mesa/drivers/common/meta.c
> +++ b/src/mesa/drivers/common/meta.c
> @@ -95,9 +95,12 @@ static struct blit_shader *
>  choose_blit_shader(GLenum target, struct blit_shader_table *table);
>
>  static void cleanup_temp_texture(struct temp_texture *tex);
> -static void meta_glsl_clear_cleanup(struct clear_state *clear);
> -static void meta_decompress_cleanup(struct decompress_state *decompress);
> -static void meta_drawpix_cleanup(struct drawpix_state *drawpix);
> +static void meta_glsl_clear_cleanup(struct gl_context *ctx,
> +                                    struct clear_state *clear);
> +static void meta_decompress_cleanup(struct gl_context *ctx,
> +                                    struct decompress_state *decompress);
> +static void meta_drawpix_cleanup(struct gl_context *ctx,
> +                                 struct drawpix_state *drawpix);
>
>  void
>  _mesa_meta_bind_fbo_image(GLenum fboTarget, GLenum attachment,
> @@ -435,12 +438,12 @@ _mesa_meta_free(struct gl_context *ctx)
>  {
>     GET_CURRENT_CONTEXT(old_context);
>     _mesa_make_current(ctx, NULL, NULL);
> -   _mesa_meta_glsl_blit_cleanup(&ctx->Meta->Blit);
> -   meta_glsl_clear_cleanup(&ctx->Meta->Clear);
> -   _mesa_meta_glsl_generate_mipmap_cleanup(&ctx->Meta->Mipmap);
> +   _mesa_meta_glsl_blit_cleanup(ctx, &ctx->Meta->Blit);
> +   meta_glsl_clear_cleanup(ctx, &ctx->Meta->Clear);
> +   _mesa_meta_glsl_generate_mipmap_cleanup(ctx, &ctx->Meta->Mipmap);
>     cleanup_temp_texture(&ctx->Meta->TempTex);
> -   meta_decompress_cleanup(&ctx->Meta->Decompress);
> -   meta_drawpix_cleanup(&ctx->Meta->DrawPix);
> +   meta_decompress_cleanup(ctx, &ctx->Meta->Decompress);
> +   meta_drawpix_cleanup(ctx, &ctx->Meta->DrawPix);
>     if (old_context)
>        _mesa_make_current(old_context, old_context->WinSysDrawBuffer, old_context->WinSysReadBuffer);
>     else
> @@ -1638,14 +1641,13 @@ meta_glsl_clear_init(struct gl_context *ctx, struct clear_state *clear)
>  }
>
>  static void
> -meta_glsl_clear_cleanup(struct clear_state *clear)
> +meta_glsl_clear_cleanup(struct gl_context *ctx, struct clear_state *clear)
>  {
>     if (clear->VAO == 0)
>        return;
>     _mesa_DeleteVertexArrays(1, &clear->VAO);
>     clear->VAO = 0;
> -   _mesa_DeleteBuffers(1, &clear->buf_obj->Name);
> -   clear->buf_obj = NULL;
> +   _mesa_reference_buffer_object(ctx, &clear->buf_obj, NULL);
>     _mesa_DeleteProgram(clear->ShaderProg);
>     clear->ShaderProg = 0;
>
> @@ -1939,14 +1941,13 @@ _mesa_meta_CopyPixels(struct gl_context *ctx, GLint srcX, GLint srcY,
>  }
>
>  static void
> -meta_drawpix_cleanup(struct drawpix_state *drawpix)
> +meta_drawpix_cleanup(struct gl_context *ctx, struct drawpix_state *drawpix)
>  {
>     if (drawpix->VAO != 0) {
>        _mesa_DeleteVertexArrays(1, &drawpix->VAO);
>        drawpix->VAO = 0;
>
> -      _mesa_DeleteBuffers(1, &drawpix->buf_obj->Name);
> -      drawpix->buf_obj = NULL;
> +      _mesa_reference_buffer_object(ctx, &drawpix->buf_obj, NULL);
>     }
>
>     if (drawpix->StencilFP != 0) {
> @@ -2975,14 +2976,15 @@ meta_decompress_fbo_cleanup(struct decompress_fbo_state *decompress_fbo)
>  }
>
>  static void
> -meta_decompress_cleanup(struct decompress_state *decompress)
> +meta_decompress_cleanup(struct gl_context *ctx,
> +                        struct decompress_state *decompress)
>  {
>     meta_decompress_fbo_cleanup(&decompress->byteFBO);
>     meta_decompress_fbo_cleanup(&decompress->floatFBO);
>
>     if (decompress->VAO != 0) {
>        _mesa_DeleteVertexArrays(1, &decompress->VAO);
> -      _mesa_DeleteBuffers(1, &decompress->buf_obj->Name);
> +      _mesa_reference_buffer_object(ctx, &decompress->buf_obj, NULL);
>     }
>
>     if (decompress->Sampler != 0)
> @@ -3299,7 +3301,6 @@ _mesa_meta_DrawTex(struct gl_context *ctx, GLfloat x, GLfloat y, GLfloat z,
>     if (drawtex->VAO == 0) {
>        /* one-time setup */
>        struct gl_vertex_array_object *array_obj;
> -      GLuint VBO;
>
>        /* create vertex array object */
>        _mesa_GenVertexArrays(1, &drawtex->VAO);
> @@ -3309,23 +3310,13 @@ _mesa_meta_DrawTex(struct gl_context *ctx, GLfloat x, GLfloat y, GLfloat z,
>        assert(array_obj != NULL);
>
>        /* create vertex array buffer */
> -      _mesa_CreateBuffers(1, &VBO);
> -      drawtex->buf_obj = _mesa_lookup_bufferobj(ctx, VBO);
> -
> -      /* _mesa_lookup_bufferobj only returns NULL if name is 0.  If the object
> -       * does not yet exist (i.e., hasn't been bound) it will return a dummy
> -       * object that you can't do anything with.
> -       */
> -      assert(drawtex->buf_obj != NULL && (drawtex->buf_obj)->Name == VBO);
> -      assert(drawtex->buf_obj == ctx->Array.ArrayBufferObj);
> +      drawtex->buf_obj = ctx->Driver.NewBufferObject(ctx, 0xDEADBEEF);
> +      if (drawtex->buf_obj == NULL)
> +         return;
>
>        _mesa_buffer_data(ctx, drawtex->buf_obj, GL_NONE, sizeof(verts), verts,
>                          GL_DYNAMIC_DRAW, __func__);
>
> -      assert(drawtex->buf_obj->Size == sizeof(verts));
> -
> -      _mesa_BindBuffer(GL_ARRAY_BUFFER_ARB, VBO);
> -
>        /* setup vertex arrays */
>        _mesa_update_array_format(ctx, array_obj, VERT_ATTRIB_POS,
>                                  3, GL_FLOAT, GL_RGBA, GL_FALSE,
> diff --git a/src/mesa/drivers/common/meta.h b/src/mesa/drivers/common/meta.h
> index 4d25957..503e743 100644
> --- a/src/mesa/drivers/common/meta.h
> +++ b/src/mesa/drivers/common/meta.h
> @@ -652,13 +652,14 @@ _mesa_meta_setup_blit_shader(struct gl_context *ctx,
>                               struct blit_shader_table *table);
>
>  void
> -_mesa_meta_glsl_blit_cleanup(struct blit_state *blit);
> +_mesa_meta_glsl_blit_cleanup(struct gl_context *ctx, struct blit_state *blit);
>
>  void
>  _mesa_meta_blit_shader_table_cleanup(struct blit_shader_table *table);
>
>  void
> -_mesa_meta_glsl_generate_mipmap_cleanup(struct gen_mipmap_state *mipmap);
> +_mesa_meta_glsl_generate_mipmap_cleanup(struct gl_context *ctx,
> +                                        struct gen_mipmap_state *mipmap);
>
>  void
>  _mesa_meta_bind_fbo_image(GLenum target, GLenum attachment,
> diff --git a/src/mesa/drivers/common/meta_blit.c b/src/mesa/drivers/common/meta_blit.c
> index 1336b7c..8905fd7 100644
> --- a/src/mesa/drivers/common/meta_blit.c
> +++ b/src/mesa/drivers/common/meta_blit.c
> @@ -974,13 +974,12 @@ _mesa_meta_BlitFramebuffer(struct gl_context *ctx,
>  }
>
>  void
> -_mesa_meta_glsl_blit_cleanup(struct blit_state *blit)
> +_mesa_meta_glsl_blit_cleanup(struct gl_context *ctx, struct blit_state *blit)
>  {
>     if (blit->VAO) {
>        _mesa_DeleteVertexArrays(1, &blit->VAO);
>        blit->VAO = 0;
> -      _mesa_DeleteBuffers(1, &blit->buf_obj->Name);
> -      blit->buf_obj = NULL;
> +      _mesa_reference_buffer_object(ctx, &blit->buf_obj, NULL);
>     }
>
>     _mesa_meta_blit_shader_table_cleanup(&blit->shaders_with_depth);
> diff --git a/src/mesa/drivers/common/meta_generate_mipmap.c b/src/mesa/drivers/common/meta_generate_mipmap.c
> index 7fb82e1..ecdc073 100644
> --- a/src/mesa/drivers/common/meta_generate_mipmap.c
> +++ b/src/mesa/drivers/common/meta_generate_mipmap.c
> @@ -120,14 +120,14 @@ fallback_required(struct gl_context *ctx, GLenum target,
>  }
>
>  void
> -_mesa_meta_glsl_generate_mipmap_cleanup(struct gen_mipmap_state *mipmap)
> +_mesa_meta_glsl_generate_mipmap_cleanup(struct gl_context *ctx,
> +                                        struct gen_mipmap_state *mipmap)
>  {
>     if (mipmap->VAO == 0)
>        return;
>     _mesa_DeleteVertexArrays(1, &mipmap->VAO);
>     mipmap->VAO = 0;
> -   _mesa_DeleteBuffers(1, &mipmap->buf_obj->Name);
> -   mipmap->buf_obj->Name = NULL;
> +   _mesa_reference_buffer_object(ctx, &mipmap->buf_obj, NULL);
>
>     _mesa_meta_blit_shader_table_cleanup(&mipmap->shaders);
>  }
> --
> 2.1.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>


More information about the mesa-dev mailing list