[Xquartz-changes] xserver: Branch 'master' - 3 commits
Jeremy Huddleston
jeremyhu at freedesktop.org
Tue Jul 19 19:51:28 PDT 2011
hw/xquartz/GL/indirect.c | 15 ++++++++++-
hw/xquartz/xpr/dri.c | 61 ++++++++++++++++++++++++++++++++++++++---------
2 files changed, 64 insertions(+), 12 deletions(-)
New commits:
commit ec95a9c829b8e37529aa828c05fbaabc45cffe42
Author: George Staplin <gstaplin at apple.com>
Date: Wed Mar 4 02:03:52 2009 -0700
XQuartz: Add diagnostic error checking to xp_destroy_surface.
This occurred to me in hindsight after the last commit. If the
original developer had done this, we would have noticed the
problem sooner.
(cherry picked from commit aa0a57996f3e7d16238f69976958c2526821388b)
diff --git a/hw/xquartz/xpr/dri.c b/hw/xquartz/xpr/dri.c
index 4f9c97c..8bae6b0 100644
--- a/hw/xquartz/xpr/dri.c
+++ b/hw/xquartz/xpr/dri.c
@@ -728,8 +728,13 @@ DRISurfaceNotify(xp_surface_id id, int kind)
if (kind == AppleDRISurfaceNotifyDestroyed)
{
- xp_destroy_surface(pDRIDrawablePriv->sid);
+ xp_error error;
+ error = xp_destroy_surface(pDRIDrawablePriv->sid);
+
+ if(error)
+ ErrorF("%s: xp_destroy_surface failed: %d\n", __func__, error);
+
/* Guard against reuse, even though we are freeing after this. */
pDRIDrawablePriv->sid = 0;
commit 4fe7df265324f63025686efe9d32342e3cef40d3
Author: George Staplin <gstaplin at apple.com>
Date: Wed Mar 4 01:39:58 2009 -0700
XQuartz: Fix a memory leak with surfaces that a new test found.
xp_destroy_surface was called with a surface id of 0, due to some
premature cleanup that set it to 0. This means the surfaces
weren't being destroyed until the window was.
The code that did that was: pDRIDrawablePriv->sid = 0;
In long running applications this leak may or may not have been
harmful. With the old libGL the surfaces weren't destroyed until
the context was destroyed or a new context created. In the new
libGL they are reference counted, and released much sooner, so we
ran into a resource leak more noticeably with some tests.
Make the Apple DRI code dispatch events to the client(s) for
destroyed surfaces, when a resource is destroyed. This seems to
work in my tests, however this clearly wasn't working for a while,
so bugs may result in the future if it enables some new (unexpected)
side effects.
Also add a few helpful comments to aid in understanding the code
in the future.
Tested with the test suite, Pymol, and various Mesa demos.
(cherry picked from commit bede83eb19a1629396fcd5a46441f8476a8fcd1b)
diff --git a/hw/xquartz/xpr/dri.c b/hw/xquartz/xpr/dri.c
index 8fef3b7..4f9c97c 100644
--- a/hw/xquartz/xpr/dri.c
+++ b/hw/xquartz/xpr/dri.c
@@ -395,8 +395,8 @@ DRICreateSurface(ScreenPtr pScreen, Drawable id,
return FALSE; /*error*/
}
#endif
- else { /* for GLX 1.3, a PBuffer */
- /* NOT_DONE */
+ else {
+ /* We handle GLXPbuffers in a different way (via CGL). */
return FALSE;
}
@@ -486,13 +486,27 @@ DRIDestroySurface(ScreenPtr pScreen, Drawable id, DrawablePtr pDrawable,
}
if (pDRIDrawablePriv != NULL) {
+ /*
+ * This doesn't seem to be used, because notify is NULL in all callers.
+ */
+
if (notify != NULL) {
pDRIDrawablePriv->notifiers = x_hook_remove(pDRIDrawablePriv->notifiers,
notify, notify_data);
}
- if (--pDRIDrawablePriv->refCount <= 0) {
- /* This calls back to DRIDrawablePrivDelete
- which frees the private area */
+
+ --pDRIDrawablePriv->refCount;
+
+ /*
+ * Check if the drawable privates still have a reference to the
+ * surface.
+ */
+
+ if (pDRIDrawablePriv->refCount <= 0) {
+ /*
+ * This calls back to DRIDrawablePrivDelete which
+ * frees the private area and dispatches events, if needed.
+ */
FreeResourceByType(id, DRIDrawablePrivResType, FALSE);
}
}
@@ -500,6 +514,10 @@ DRIDestroySurface(ScreenPtr pScreen, Drawable id, DrawablePtr pDrawable,
return TRUE;
}
+/*
+ * The assumption is that this is called when the refCount of a surface
+ * drops to <= 0, or the window/pixmap is destroyed.
+ */
Bool
DRIDrawablePrivDelete(pointer pResource, XID id)
{
@@ -518,18 +536,23 @@ DRIDrawablePrivDelete(pointer pResource, XID id)
}
if (pDRIDrawablePriv == NULL) {
+ /*
+ * We reuse __func__ and the resource type for the GLXPixmap code.
+ * Attempt to free a pixmap buffer associated with the resource
+ * if possible.
+ */
return DRIFreePixmapImp(pDrawable);
}
-
+
if (pDRIDrawablePriv->drawableIndex != -1) {
/* release drawable table entry */
pDRIPriv->DRIDrawables[pDRIDrawablePriv->drawableIndex] = NULL;
}
if (pDRIDrawablePriv->sid != 0) {
- xp_destroy_surface(pDRIDrawablePriv->sid);
- x_hash_table_remove(surface_hash, x_cvt_uint_to_vptr(pDRIDrawablePriv->sid));
+ DRISurfaceNotify(pDRIDrawablePriv->sid, AppleDRISurfaceNotifyDestroyed);
}
+
if (pDRIDrawablePriv->notifiers != NULL)
x_hook_free(pDRIDrawablePriv->notifiers);
@@ -673,6 +696,11 @@ DRIQueryVersion(int *majorVersion,
*patchVersion = APPLE_DRI_PATCH_VERSION;
}
+/*
+ * Note: this also cleans up the hash table in addition to notifying clients.
+ * The sid/surface-id should not be used after this, because it will be
+ * invalid.
+ */
void
DRISurfaceNotify(xp_surface_id id, int kind)
{
@@ -693,21 +721,27 @@ DRISurfaceNotify(xp_surface_id id, int kind)
if (kind == AppleDRISurfaceNotifyDestroyed)
{
- pDRIDrawablePriv->sid = 0;
- x_hash_table_remove(surface_hash, x_cvt_uint_to_vptr(id));
+ x_hash_table_remove(surface_hash, x_cvt_uint_to_vptr(id));
}
x_hook_run(pDRIDrawablePriv->notifiers, &arg);
if (kind == AppleDRISurfaceNotifyDestroyed)
{
- /* Kill off the handle. */
+ xp_destroy_surface(pDRIDrawablePriv->sid);
+
+ /* Guard against reuse, even though we are freeing after this. */
+ pDRIDrawablePriv->sid = 0;
FreeResourceByType(pDRIDrawablePriv->pDraw->id,
DRIDrawablePrivResType, FALSE);
}
}
+/*
+ * This creates a shared memory buffer for use with GLXPixmaps
+ * and AppleSGLX.
+ */
Bool DRICreatePixmap(ScreenPtr pScreen, Drawable id,
DrawablePtr pDrawable, char *path,
size_t pathmax)
commit 0ebe45a717faa6464d3b1ab73e30570518ee4798
Author: Jeremy Huddleston <jeremyhu at apple.com>
Date: Tue Jul 19 19:42:44 2011 -0700
XQuartz: DRI: Dead code removal
Also add some comments that weren't merged in from server-1.4-apple's
99babae1326485c27eb9253db83afdd6aef9e362
Signed-off-by: Jeremy Huddleston <jeremyhu at apple.com>
diff --git a/hw/xquartz/GL/indirect.c b/hw/xquartz/GL/indirect.c
index 4876ab9..27d6dae 100644
--- a/hw/xquartz/GL/indirect.c
+++ b/hw/xquartz/GL/indirect.c
@@ -88,6 +88,15 @@ typedef struct __GLXAquaScreen __GLXAquaScreen;
typedef struct __GLXAquaContext __GLXAquaContext;
typedef struct __GLXAquaDrawable __GLXAquaDrawable;
+/*
+ * The following structs must keep the base as the first member.
+ * It's used to treat the start of the struct as a different struct
+ * in GLX.
+ *
+ * Note: these structs should be initialized with xcalloc or memset
+ * prior to usage, and some of them require initializing
+ * the base with function pointers.
+ */
struct __GLXAquaScreen {
__GLXscreen base;
int index;
@@ -196,7 +205,11 @@ static int __glXAquaContextLoseCurrent(__GLXcontext *baseContext) {
if (gl_err != 0)
ErrorF("CGLSetCurrentContext error: %s\n", CGLErrorString(gl_err));
- __glXLastContext = NULL; // Mesa does this; why?
+ /*
+ * There should be no need to set __glXLastContext to NULL here, because
+ * glxcmds.c does it as part of the context cache flush after calling
+ * this.
+ */
return GL_TRUE;
}
More information about the Xquartz-changes
mailing list