[Mesa-dev] [PATCH v2 0/3] asynchronous pbo transfer with glthread

[Gregory Hainaut]

> Still, I believe there is the following bug in the series:

> glGenBuffers(1, &pbo);
> glBindBuffer(..., pbo);
> glDeleteBuffers(1, &pbo);
> glTexSubImage2D(...); // will be asynchronous, but refers to user memory because the pbo was implicitly unbound

I unbound the buffer when it is deleted in the
track_buffers_destruction function. See code chunk below. So I think
it must work. Or did I miss something ?

> +      if (buffers[i] == glthread->pixel_pack_buffer_bound)
> +         glthread->pixel_pack_buffer_bound = 0;
> +
> +      if (buffers[i] == glthread->pixel_unpack_buffer_bound)
> +         glthread->pixel_unpack_buffer_bound = 0;




> Ctx A: glGenBuffers(1, &pbo); // assume that this gets the same buffer name
You can't have the same name because name is still bound in context A.

So maybe you mean something instead for your example ?

> Ctx A: glGenBuffers(1, &pbo);
> Ctx A: glBindBuffer(PIXEL_UNPACK_BUFFER, pbo);
> Ctx B: glDeleteBuffers(1, &pbo);
...
> Ctx A: glTexSubImage2D(...); // will be synchronous, even though it _could_ be asynchronous (because the PBO that was generated first is still bound!)

And yes I confirm, it isn't optimal. I did it on purpose because it is
much safer/easier and as you said it is a crazy case.


Cheers,
Gregory

On 4/5/17, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> On 01.04.2017 11:42, Gregory Hainaut wrote:
>> Hello,
>>
>> Please find a new version to handle invalid buffer handles.
>>
>> Allow to handle this kind of case:
>>    genBuffer(&pbo);
>>    DeleteBuffer(pbo);
>>    BindBuffer(pbo)
>>    TexSubImage2D(user_memory_pointer); // Data transfer will be
>> synchronous
>>
>> There are various subtely to handle multi threaded shared context. In
>> order to
>> keep the code sane, I've considered a buffer invalid when it is deleted by
>> a
>> context even it is still bound to others contexts. It will force a
>> synchronous
>> transfer which is always safe.
>
> Thanks! I think you're right that tracking pointers is not actually
> needed as long as Create/Delete are synchronous. It's less accurate, but
> the inaccuracy doesn't necessarily lead to correctness problems.
>
> Still, I believe there is the following bug in the series:
>
> glGenBuffers(1, &pbo);
> glBindBuffer(..., pbo);
> glDeleteBuffers(1, &pbo);
> glTexSubImage2D(...); // will be asynchronous, but refers to user memory
> because the pbo was implicitly unbound
>
> Once you fix this bug by updating the current context's bindings in
> glDeleteBuffers, you'll have the following multi-context situation,
> which is sub-optimal but not a bug -- and applications which do that are
> crazy anyway, so let's not worry about them. I'm just mentioning it for
> completeness:
>
> Ctx A: glGenBuffers(1, &pbo);
> Ctx A: glBindBuffer(PIXEL_UNPACK_BUFFER, pbo);
> Ctx B: glDeleteBuffers(1, &pbo);
> Ctx A: glGenBuffers(1, &pbo); // assume that this gets the same buffer name
> Ctx A: glDeleteBuffers(1, &pbo);
> Ctx A: glTexSubImage2D(...); // will be synchronous, even though it
> _could_ be asynchronous (because the PBO that was generated first is
> still bound!)
>
> Cheers,
> Nicolai
>
>
>>
>> Gregory Hainaut (3):
>>   mesa/glthread: track buffer creation/destruction
>>   mesa/glthread: add tracking of PBO binding
>>   mapi/glthread: generate asynchronous code for PBO transfer
>>
>>  src/mapi/glapi/gen/ARB_direct_state_access.xml |  18 +--
>>  src/mapi/glapi/gen/ARB_robustness.xml          |   2 +-
>>  src/mapi/glapi/gen/gl_API.dtd                  |  10 +-
>>  src/mapi/glapi/gen/gl_API.xml                  |  32 +++---
>>  src/mapi/glapi/gen/gl_marshal.py               |  23 +++-
>>  src/mapi/glapi/gen/marshal_XML.py              |  19 +++-
>>  src/mesa/main/glthread.h                       |  10 ++
>>  src/mesa/main/marshal.c                        | 149
>> ++++++++++++++++++++++++-
>>  src/mesa/main/marshal.h                        |  24 ++++
>>  src/mesa/main/mtypes.h                         |   5 +
>>  src/mesa/main/shared.c                         |   4 +
>>  11 files changed, 257 insertions(+), 39 deletions(-)
>>
>
>
> --
> Lerne, wie die Welt wirklich ist,
> Aber vergiss niemals, wie sie sein sollte.
>