Hi Gregory,

Sorry, this got dropped somehow.


On 13.04.2017 17:49, gregory hainaut wrote:
On Wed, 5 Apr 2017 12:52:03 +0200
Gregory Hainaut <gregory.hain...@gmail.com> wrote:

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;


Hello Nicolai,

Do we agree that above pattern will work with my patches ?

Yes, I'm convinced now.




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.
Actually I'm wrong here. glDeleteBuffers makes the name available again. Which
lead to interesting question if the buffer is bound again after the name is 
reused
by a glGenBuffers call. Anyway it isn't important.

Also agreed.

There's still the comment on patch 2&3.

Cheers,
Nicolai


Chapter 5.1.3 of 4.5 core spec
"Since the name is marked unused, binding the name will create a new object with
the same name, and attaching the name will generate an error."


Cheers,
Gregory

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 <nhaeh...@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.



--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to