[Mesa-dev] [PATCH 21/23] meta/TexSubImage: Don't pollute the buffer object namespace

[Anuj Phogat]

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_tex_subimage.c | 42 ++++++++++++++++-------------
>  1 file changed, 24 insertions(+), 18 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta_tex_subimage.c b/src/mesa/drivers/common/meta_tex_subimage.c
> index b0ac677..4adaad7 100644
> --- a/src/mesa/drivers/common/meta_tex_subimage.c
> +++ b/src/mesa/drivers/common/meta_tex_subimage.c
> @@ -69,7 +69,7 @@ create_texture_for_pbo(struct gl_context *ctx,
>                         int dims, int width, int height, int depth,
>                         GLenum format, GLenum type, const void *pixels,
>                         const struct gl_pixelstore_attrib *packing,
> -                       GLuint *tmp_pbo, GLuint *tmp_tex)
> +                       struct gl_buffer_object **tmp_pbo, GLuint *tmp_tex)
>  {
>     uint32_t pbo_format;
>     GLenum internal_format;
> @@ -101,7 +101,7 @@ create_texture_for_pbo(struct gl_context *ctx,
>     row_stride = _mesa_image_row_stride(packing, width, format, type);
>
>     if (_mesa_is_bufferobj(packing->BufferObj)) {
> -      *tmp_pbo = 0;
> +      *tmp_pbo = NULL;
>        buffer_obj = packing->BufferObj;
>        first_pixel += (intptr_t)pixels;
>     } else {
> @@ -109,23 +109,27 @@ create_texture_for_pbo(struct gl_context *ctx,
>
>        assert(create_pbo);
>
> -      _mesa_CreateBuffers(1, tmp_pbo);
> +      *tmp_pbo = ctx->Driver.NewBufferObject(ctx, 0xDEADBEEF);
> +      if (*tmp_pbo == NULL)
> +         return NULL;
>
>        /* In case of GL_PIXEL_PACK_BUFFER, pass null pointer for the pixel
> -       * data to avoid unnecessary data copying in _mesa_NamedBufferData().
> +       * data to avoid unnecessary data copying in _mesa_buffer_data.
>         */
>        if (is_pixel_pack)
> -         _mesa_NamedBufferData(*tmp_pbo,
> -                               last_pixel - first_pixel,
> -                               NULL,
> -                               GL_STREAM_READ);
> +         _mesa_buffer_data(ctx, *tmp_pbo, GL_NONE,
> +                           last_pixel - first_pixel,
> +                           NULL,
> +                           GL_STREAM_READ,
> +                           __func__);
>        else
> -         _mesa_NamedBufferData(*tmp_pbo,
> -                               last_pixel - first_pixel,
> -                               (char *)pixels + first_pixel,
> -                               GL_STREAM_DRAW);
> +         _mesa_buffer_data(ctx, *tmp_pbo, GL_NONE,
> +                           last_pixel - first_pixel,
> +                           (char *)pixels + first_pixel,
> +                           GL_STREAM_DRAW,
> +                           __func__);
>
> -      buffer_obj = _mesa_lookup_bufferobj(ctx, *tmp_pbo);
> +      buffer_obj = *tmp_pbo;
>        first_pixel = 0;
>     }
>
> @@ -157,7 +161,7 @@ create_texture_for_pbo(struct gl_context *ctx,
>                                                       row_stride,
>                                                       read_only)) {
>        _mesa_DeleteTextures(1, tmp_tex);
> -      _mesa_DeleteBuffers(1, tmp_pbo);
> +      _mesa_reference_buffer_object(ctx, tmp_pbo, NULL);
>        return NULL;
>     }
>
> @@ -173,7 +177,8 @@ _mesa_meta_pbo_TexSubImage(struct gl_context *ctx, GLuint dims,
>                             bool allocate_storage, bool create_pbo,
>                             const struct gl_pixelstore_attrib *packing)
>  {
> -   GLuint pbo = 0, pbo_tex = 0, fbos[2] = { 0, 0 };
> +   struct gl_buffer_object *pbo = NULL;
> +   GLuint pbo_tex = 0, fbos[2] = { 0, 0 };
>     int image_height;
>     struct gl_texture_image *pbo_tex_image;
>     GLenum status;
> @@ -276,7 +281,7 @@ _mesa_meta_pbo_TexSubImage(struct gl_context *ctx, GLuint dims,
>  fail:
>     _mesa_DeleteFramebuffers(2, fbos);
>     _mesa_DeleteTextures(1, &pbo_tex);
> -   _mesa_DeleteBuffers(1, &pbo);
> +   _mesa_reference_buffer_object(ctx, &pbo, NULL);
>
>     _mesa_meta_end(ctx);
>
> @@ -291,7 +296,8 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, GLuint dims,
>                                GLenum format, GLenum type, const void *pixels,
>                                const struct gl_pixelstore_attrib *packing)
>  {
> -   GLuint pbo = 0, pbo_tex = 0, fbos[2] = { 0, 0 };
> +   struct gl_buffer_object *pbo = NULL;
> +   GLuint pbo_tex = 0, fbos[2] = { 0, 0 };
>     int image_height;
>     struct gl_texture_image *pbo_tex_image;
>     struct gl_renderbuffer *rb = NULL;
> @@ -448,7 +454,7 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, GLuint dims,
>  fail:
>     _mesa_DeleteFramebuffers(2, fbos);
>     _mesa_DeleteTextures(1, &pbo_tex);
> -   _mesa_DeleteBuffers(1, &pbo);
> +   _mesa_reference_buffer_object(ctx, &pbo, NULL);
>
>     _mesa_meta_end(ctx);
>
> --
> 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>