[Xquartz-changes] [326] AppleSGLX/trunk
source_changes at macosforge.org
source_changes at macosforge.org
Fri Mar 27 12:02:55 PDT 2009
Revision: 326
http://trac.macosforge.org/projects/xquartz/changeset/326
Author: gstaplin at apple.com
Date: 2009-03-27 12:02:55 -0700 (Fri, 27 Mar 2009)
Log Message:
-----------
Fix the surface destruction issue more cleanly with a slightly better design.
The test case still passes, and this should prevent a memory leak pattern that
the previous fix would have suffered from.
Tested with a few apps, and triangle_glx_destroy_relation (the specific test
case for this).
Modified Paths:
--------------
AppleSGLX/trunk/apple_glx_context.c
AppleSGLX/trunk/apple_glx_surface.c
Modified: AppleSGLX/trunk/apple_glx_context.c
===================================================================
--- AppleSGLX/trunk/apple_glx_context.c 2009-03-26 07:53:24 UTC (rev 325)
+++ AppleSGLX/trunk/apple_glx_context.c 2009-03-27 19:02:55 UTC (rev 326)
@@ -572,14 +572,7 @@
* If there are references to it, then it's probably made
* current in another context.
*/
- d->destroy(d);
-
- /*
- * FIXME: We retain 2 references to surfaces to prevent
- * premature destruction. See also: the comment in the
- * make current path.
- */
- d->destroy(d);
+ d->destroy(d);
}
}
}
Modified: AppleSGLX/trunk/apple_glx_surface.c
===================================================================
--- AppleSGLX/trunk/apple_glx_surface.c 2009-03-26 07:53:24 UTC (rev 325)
+++ AppleSGLX/trunk/apple_glx_surface.c 2009-03-27 19:02:55 UTC (rev 326)
@@ -101,16 +101,23 @@
fprintf(stderr, "xp_destroy_surface error: %d\n", (int)error);
}
- /*
- * Warning: this causes other routines to be called (potentially)
- * from surface_notify_handler. It's probably best to not have
- * any locks at this point locked.
+ /*
+ * Check if this surface destroy came from the surface being destroyed
+ * on the server. If s->pending_destroy is true, then it did, and
+ * we don't want to try to destroy the surface on the server.
*/
- XAppleDRIDestroySurface(d->display, DefaultScreen(d->display),
- d->drawable);
-
- apple_glx_diagnostic("%s: destroyed a surface for drawable 0x%lx uid %u\n",
- __func__, d->drawable, s->uid);
+ if(!s->pending_destroy) {
+ /*
+ * Warning: this causes other routines to be called (potentially)
+ * from surface_notify_handler. It's probably best to not have
+ * any locks at this point locked.
+ */
+ XAppleDRIDestroySurface(d->display, DefaultScreen(d->display),
+ d->drawable);
+
+ apple_glx_diagnostic("%s: destroyed a surface for drawable 0x%lx uid %u\n",
+ __func__, d->drawable, s->uid);
+ }
}
/* Return true if an error occured. */
@@ -193,6 +200,18 @@
if(d) {
d->types.surface.pending_destroy = true;
d->release(d);
+ /*
+ * We release 2 references to the surface. One was acquired by
+ * the find, and the other was leftover from a context, or
+ * the surface being displayed, so the destroy() will decrease it
+ * once more.
+ *
+ * If the surface is in a context, it will take one d->destroy(d);
+ * to actually destroy it when the pending_destroy is processed
+ * by a glViewport callback (see apple_glx_context_update()).
+ */
+ d->destroy(d);
+
d->unlock(d);
}
}
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.macosforge.org/pipermail/xquartz-changes/attachments/20090327/da0fb54f/attachment-0001.html>
More information about the Xquartz-changes
mailing list