On 02/12/2016 06:56 AM, Michel Dänzer wrote:
On 12.02.2016 05:58, Miklós Máté wrote:
On 02/09/2016 05:02 PM, Ian Romanick wrote:
On 02/08/2016 05:11 PM, Ian Romanick wrote:
On 02/05/2016 01:11 PM, Miklós Máté wrote:
dri drawables must never be released when unbound from a context
as long as their corresponding glx objects (window, pixmap, pbuffer)
still exist
I'd really like to have Kristian weigh in, since DRI2 was his design,
and this is all his code being affected.  That said, I'm a little unsure
about this change.

The DRI drawables and associated resources will still get freed when the
application eventually calls glXDestroyPBuffer or glXDestroyWindow.

But I'm not 100% sure what will happen with GLX 1.2 applications...
there are no glXDestroy* functions in GLX 1.2.  In that case the
application will have a soft leak because calling XDestroyWindow won't
do anything on the client (or server?) for the DRI resources.  I suspect
that's what this code was trying to prevent.
And this commit affirms that:

commit bf69ce37f0dcbb479078ee676d5100ac63e20750
Author: Stéphane Marchesin <marc...@chromium.org>
Date:   Wed Jun 15 15:09:12 2011 -0700

      glx: implement drawable refcounting.
           The current dri context unbind logic will leak drawables
until the process
      dies (they will then get released by the GEM code). There are two
ways to fix
      this: either always call driReleaseDrawables every time we unbind
a context
      (but that costs us round trips to the X server at getbuffers()
time) or
      implement proper drawable refcounting. This patch implements the
latter.
           Signed-off-by: Antoine Labour <pi...@chromium.org>
      Signed-off-by: Stéphane Marchesin <marc...@chromium.org>
      Reviewed-by: Adam Jackson <a...@redhat.com>

Since we don't have any way to know when the corresponding GLX object
ceases to exist, we don't know when it's safe to destroy the DRI
object.  I don't know how we can not leak and not prematurely destroy
the object. :(
That commit message exaggerates a little when it says "proper
refcounting", because it only refcounts on bind (in driFetchDrawable),
but not on create/destroy. With current Mesa the following happens:
create glXPbuffer: dri drawable is created, refcount=0
bind it: refcount++
unbind it: refcount--, drawable is destroyed
bind again: create new drawable, refcount=1
unbind again: refcount--, drawable is destroyed
etc.
Could that be fixed by starting with refcount=1 instead of 0?
In theory, yes. In practice, no. I tried to add refcount=1 to the glXCreate* functions, but
for example, you don't need to call glXCreateWindow:
 win=XCreateWindow(....); glXMakeCurrent(dpy, win, glc);
is fine. This is what the example on OpenGL wiki does, and more importantly, Wine does this, too.
When should refcount=1 be set in such cases? I couldn't figure it out.



They also say that "current unbind logic will leak drawables until the
process dies", which is simply not true. The glXDestroy* calls properly
dispose of the corresponding dri objects.

With GLX 1.3 the dri drawables are correctly created and destroyed along
with their corresponding glx objects even without the above commit. IMHO
if someone deliberately uses GLX 1.2 or earlier, they know what they are
doing (1.3 was released in 1998). I'd say that it's never safe to
destroy a dri object until it's explicitly destroyed by the user, so I'm
still convinced that my patch is correct.
Unless I'm missing something, one issue I see with your patch is that a
DRI drawable could be destroyed while it's still current for a context.
This is an interesting question. The GLX 1.4 specification says on page 28 that "If draw is destroyed after glXMakeContextCurrent is called, then subsequent rendering commands will be processed and the context state will be updated, but the frame buffer state becomes undefined." This suggests that the lifetime of the draw buffer ends with destroy, not unbind.
If this is true, we don't need refcount on makecurrent.

However, on page 23 it says "The storage for the GLX pixmap will be freed when it is not current to any client." This is interesting, because one pixmap can be current to more than one client, and thus it needs to be refcounted across multiple processes. I think "storage" in this case means the server-side resources, and not the dri drawable, but I could be wrong.

MM
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to