Revision: 276 http://trac.macosforge.org/projects/xquartz/changeset/276 Author: gstaplin@apple.com Date: 2009-03-03 11:39:40 -0800 (Tue, 03 Mar 2009) Log Message: ----------- Add more diagnostics for surface creation an destruction. Add apple_glx_surface_destroy to potentialy destroy a surface soon after the surface has been destroyed in the X server. Reorder the functions that take a flags argument with LOCK and REFERENCE flags. We want the reference before the lock, because the reference uses the lock. This could have lead to deadlock, but didn't in practice, because only the pattern I added today would have used both flags. Improve the glViewport path behavior when the surface is destroyed. It will attempt to clear and destroy the drawable (assuming there are no more references to it in other contexts). Add a pending_destroy to the apple_glx_surface structure. Make apple_glx_surface_destroy set the pending_destroy for use by the glViewport path. Modified Paths: -------------- AppleSGLX/trunk/apple_glx.c AppleSGLX/trunk/apple_glx_context.c AppleSGLX/trunk/apple_glx_context.h AppleSGLX/trunk/apple_glx_drawable.c AppleSGLX/trunk/apple_glx_drawable.h AppleSGLX/trunk/apple_glx_surface.c Modified: AppleSGLX/trunk/apple_glx.c =================================================================== --- AppleSGLX/trunk/apple_glx.c 2009-03-01 01:34:27 UTC (rev 275) +++ AppleSGLX/trunk/apple_glx.c 2009-03-03 19:39:40 UTC (rev 276) @@ -75,25 +75,14 @@ static void surface_notify_handler(Display *dpy, unsigned int uid, int kind) { switch(kind) { - case AppleDRISurfaceNotifyDestroyed: { - xp_surface_id sid; - CGLContextObj contextobj; - - if(apple_glx_get_surface_from_uid(uid, &sid, &contextobj)) { - /* The surface was probably destroyed. */ - return; - } - - /* FIXME this needs more thread safety. */ - - apple_cgl.clear_drawable(contextobj); - xp_destroy_surface(sid); - } + case AppleDRISurfaceNotifyDestroyed: + apple_glx_diagnostic("%s: surface destroyed %u\n", __func__, uid); + apple_glx_surface_destroy(uid); break; - + case AppleDRISurfaceNotifyChanged: { int updated; - + updated = apple_glx_context_surface_changed(uid, pthread_self()); apple_glx_diagnostic("surface notify updated %d\n", updated); Modified: AppleSGLX/trunk/apple_glx_context.c =================================================================== --- AppleSGLX/trunk/apple_glx_context.c 2009-03-01 01:34:27 UTC (rev 275) +++ AppleSGLX/trunk/apple_glx_context.c 2009-03-03 19:39:40 UTC (rev 276) @@ -409,28 +409,6 @@ return (ac->drawable && ac->drawable->drawable == drawable); } -/* Return true if an error occurred. */ -bool apple_glx_get_surface_from_uid(unsigned int uid, xp_surface_id *sid, - CGLContextObj *contextobj) { - struct apple_glx_context *ac; - - lock_context_list(); - - for(ac = context_list; ac; ac = ac->next) { - if(ac->drawable && APPLE_GLX_DRAWABLE_SURFACE == ac->drawable->type - && ac->drawable->types.surface.uid == uid) { - *sid = ac->drawable->types.surface.surface_id; - *contextobj = ac->context_obj; - unlock_context_list(); - return false; - } - } - - unlock_context_list(); - - return true; -} - bool apple_glx_copy_context(void *currentptr, void *srcptr, void *destptr, unsigned long mask, int *errorptr, bool *x11errorptr) { @@ -508,6 +486,34 @@ xp_update_gl_context(ac->context_obj); ac->need_update = false; - apple_glx_diagnostic("updating context %p\n", ptr); + apple_glx_diagnostic("%s: updating context %p\n", __func__, ptr); } + + if(ac->drawable && APPLE_GLX_DRAWABLE_SURFACE == ac->drawable->type + && ac->drawable->types.surface.pending_destroy) { + apple_glx_diagnostic("%s: clearing drawable %p\n", __func__, ptr); + apple_cgl.clear_drawable(ac->context_obj); + + if(ac->drawable) { + struct apple_glx_drawable *d; + + apple_glx_diagnostic("%s: attempting to destroy drawable %p\n", + __func__, ptr); + apple_glx_diagnostic("%s: ac->drawable->drawable is 0x%lx\n", + __func__, ac->drawable->drawable); + + d = ac->drawable; + + ac->drawable = NULL; + + /* + * This will destroy the surface drawable if there are + * no references to it. + * It also subtracts 1 from the reference_count. + * If there are references to it, then it's probably made + * current in another context. + */ + d->destroy(d); + } + } } Modified: AppleSGLX/trunk/apple_glx_context.h =================================================================== --- AppleSGLX/trunk/apple_glx_context.h 2009-03-01 01:34:27 UTC (rev 275) +++ AppleSGLX/trunk/apple_glx_context.h 2009-03-03 19:39:40 UTC (rev 276) @@ -61,9 +61,6 @@ bool apple_glx_make_current_context(Display *dpy, void *oldptr, void *ptr, GLXDrawable drawable); bool apple_glx_is_current_drawable(void *ptr, GLXDrawable drawable); -bool apple_glx_get_surface_from_uid(unsigned int uid, xp_surface_id *sid, - CGLContextObj *contextobj); - bool apple_glx_copy_context(void *currentptr, void *srcptr, void *destptr, unsigned long mask, int *errorptr, bool *x11errorptr); Modified: AppleSGLX/trunk/apple_glx_drawable.c =================================================================== --- AppleSGLX/trunk/apple_glx_drawable.c 2009-03-01 01:34:27 UTC (rev 275) +++ AppleSGLX/trunk/apple_glx_drawable.c 2009-03-03 19:39:40 UTC (rev 276) @@ -70,10 +70,6 @@ lock_drawables_list(); - /* - * Pixmaps aren't required to have a globally unique ID from what I recall. - * so we use the display connection with the drawable lookup. - */ for(i = drawables_list; i; i = i->next) { if(i->drawable == drawable) { agd = i; @@ -147,8 +143,6 @@ if(d->next) d->next->previous = d->previous; - /* Temporarily increase the reference count. */ - d->reference_count++; unlock_drawables_list(); if (d->callbacks.destroy) { @@ -379,12 +373,12 @@ for(d = drawables_list; d; d = d->next) { if(d->type == type && d->drawable == drawable) { + if(flags & APPLE_GLX_DRAWABLE_REFERENCE) + d->reference(d); + if(flags & APPLE_GLX_DRAWABLE_LOCK) d->lock(d); - if(flags & APPLE_GLX_DRAWABLE_REFERENCE) - d->reference(d); - unlock_drawables_list(); return d; @@ -404,12 +398,12 @@ for(d = drawables_list; d; d = d->next) { if(d->drawable == drawable) { + if(flags & APPLE_GLX_DRAWABLE_REFERENCE) + d->reference(d); + if(flags & APPLE_GLX_DRAWABLE_LOCK) d->lock(d); - - if(flags & APPLE_GLX_DRAWABLE_REFERENCE) - d->reference(d); - + unlock_drawables_list(); return d; @@ -453,3 +447,31 @@ return false; } + +struct apple_glx_drawable * +apple_glx_drawable_find_by_uid(unsigned int uid, int flags) { + struct apple_glx_drawable *d; + + lock_drawables_list(); + + for(d = drawables_list; d; d = d->next) { + /* Only surfaces have a uid. */ + if(APPLE_GLX_DRAWABLE_SURFACE == d->type) { + if(d->types.surface.uid == uid) { + if(flags & APPLE_GLX_DRAWABLE_REFERENCE) + d->reference(d); + + if(flags & APPLE_GLX_DRAWABLE_LOCK) + d->lock(d); + + unlock_drawables_list(); + + return d; + } + } + } + + unlock_drawables_list(); + + return NULL; +} Modified: AppleSGLX/trunk/apple_glx_drawable.h =================================================================== --- AppleSGLX/trunk/apple_glx_drawable.h 2009-03-01 01:34:27 UTC (rev 275) +++ AppleSGLX/trunk/apple_glx_drawable.h 2009-03-03 19:39:40 UTC (rev 276) @@ -56,6 +56,7 @@ struct apple_glx_surface { xp_surface_id surface_id; unsigned int uid; + bool pending_destroy; }; struct apple_glx_pbuffer { @@ -165,12 +166,16 @@ bool apple_glx_drawable_destroy_by_type(Display *dpy, GLXDrawable drawable, int type); +struct apple_glx_drawable * +apple_glx_drawable_find_by_uid(unsigned int uid, int flags); /* Surfaces */ bool apple_glx_surface_create(Display *dpy, int screen, GLXDrawable drawable, struct apple_glx_drawable **resultptr); +void apple_glx_surface_destroy(unsigned int uid); + /* Pbuffers */ /* Returns true if an error occurred. */ @@ -205,4 +210,6 @@ bool apple_glx_pixmap_query(GLXPixmap pixmap, int attribute, unsigned int *value); + + #endif Modified: AppleSGLX/trunk/apple_glx_surface.c =================================================================== --- AppleSGLX/trunk/apple_glx_surface.c 2009-03-01 01:34:27 UTC (rev 275) +++ AppleSGLX/trunk/apple_glx_surface.c 2009-03-03 19:39:40 UTC (rev 276) @@ -37,6 +37,7 @@ static void surface_destroy(Display *dpy, struct apple_glx_drawable *d); + static struct apple_glx_drawable_callbacks callbacks = { .type = APPLE_GLX_DRAWABLE_SURFACE, .make_current = surface_make_current, @@ -92,6 +93,8 @@ static void surface_destroy(Display *dpy, struct apple_glx_drawable *d) { struct apple_glx_surface *s = &d->types.surface; + apple_glx_diagnostic("%s: s->surface_id %u\n", __func__, s->surface_id); + xp_error error = xp_destroy_surface(s->surface_id); if(error) { @@ -122,6 +125,8 @@ assert(None != d->drawable); + s->pending_destroy = false; + if(XAppleDRICreateSurface(dpy, screen, d->drawable, id, key, &s->uid)) { xp_error error; @@ -165,3 +170,26 @@ return false; } + +/* + * All surfaces are reference counted, and surfaces are only created + * when the window is made current. When all contexts no longer reference + * a surface drawable the apple_glx_drawable gets destroyed, and thus + * its surface is destroyed. + * + * However we can make the destruction occur a bit sooner by setting + * pending_destroy, which is then checked for in glViewport by + * apple_glx_context_update. + */ +void apple_glx_surface_destroy(unsigned int uid) { + struct apple_glx_drawable *d; + + d = apple_glx_drawable_find_by_uid(uid, APPLE_GLX_DRAWABLE_REFERENCE + | APPLE_GLX_DRAWABLE_LOCK); + + if(d) { + d->types.surface.pending_destroy = true; + d->release(d); + d->unlock(d); + } +}