Hi Phil,
> It doesn't sound right to me that this behaviour is an implementation detail
> of X.org.
> Developers need to know that either it does or it doesn't. Were you given a
> good rationale?
The explanation was that Pixmaps are reference counted, and therefor
stay alive if a XRender-picture points to it. A window is not
reference counted, and therefor all its resources are destroyed
immediatly. GC's are not affected because they are not bound to a
drawable, thats why it hasn't been a problem for the JDK to free GCs
later than the Window.
I find this explanation fishy, but the general recommendation was to
free Pictures before the drawable in any case.
The patch attached does free the picture before destroying the window,
however casting surfaceData to XRSurfaceData in XWindow.java is ugly -
and the patch assumes nobody calls XDestroyWindow for a
SurfaceData-Drawable except destroy(). Suggestions welcome ;)
With that patch I get rid of all XErrors and also can't reproduce the
XDnD-Exception anymore.
> I don't see how that helped on its own. If the Picture is implicitly freed
> by XDestroyWindow then
> xsdo->xrPic is still going to be != None .. or am I overlooking something?
My initial analysis was wrong - somehow I thought the problem is
solved, but it wasn't.
Thanks Clemens
PS: Is there some java-side equivalent in SurfaceData for X11SD_Dispose?
I have a GC on the java side with no references on the native side and
if possible would like to free it at dispose time without writing C
code.
--- old/make/sun/awt/mapfile-mawt-vers 2010-09-24 22:25:13.453158179 +0200
+++ new/make/sun/awt/mapfile-mawt-vers 2010-09-24 22:25:13.060159227 +0200
@@ -424,6 +424,7 @@
Java_sun_java2d_xr_XRSurfaceData_initXRPicture;
Java_sun_java2d_xr_XRSurfaceData_initIDs;
Java_sun_java2d_xr_XRSurfaceData_XRInitSurface;
+ Java_sun_java2d_xr_XRSurfaceData_freeXSDOPicture;
Java_sun_java2d_xr_XRBackendNative_initIDs;
Java_sun_java2d_xr_XIDGenerator_bufferXIDs;
Java_sun_java2d_xr_XRBackendNative_freeGC;
--- old/make/sun/xawt/mapfile-vers 2010-09-24 22:25:14.103157831 +0200
+++ new/make/sun/xawt/mapfile-vers 2010-09-24 22:25:13.850167398 +0200
@@ -375,6 +375,7 @@
Java_sun_java2d_xr_XRSurfaceData_initXRPicture;
Java_sun_java2d_xr_XRSurfaceData_initIDs;
Java_sun_java2d_xr_XRSurfaceData_XRInitSurface;
+ Java_sun_java2d_xr_XRSurfaceData_freeXSDOPicture;
Java_sun_java2d_xr_XRBackendNative_initIDs;
Java_sun_java2d_xr_XRBackendNative_freeGC;
Java_sun_java2d_xr_XRBackendNative_createGC;
--- old/src/solaris/classes/sun/awt/X11/XWindow.java 2010-09-24 22:25:14.690157342 +0200
+++ new/src/solaris/classes/sun/awt/X11/XWindow.java 2010-09-24 22:25:14.421408215 +0200
@@ -44,6 +44,8 @@
import sun.java2d.SunGraphics2D;
import sun.java2d.SurfaceData;
+import sun.java2d.xr.XRSurfaceData;
+
public class XWindow extends XBaseWindow implements X11ComponentPeer {
private static PlatformLogger log = PlatformLogger.getLogger("sun.awt.X11.XWindow");
private static PlatformLogger insLog = PlatformLogger.getLogger("sun.awt.X11.insets.XWindow");
@@ -1383,6 +1385,18 @@
destroy();
}
+ void destroy() {
+ XToolkit.awtLock();
+ try {
+ if(surfaceData != null && surfaceData instanceof XRSurfaceData) {
+ ((XRSurfaceData) surfaceData).freeSurfacePicture();
+ }
+ super.destroy();
+ } finally {
+ XToolkit.awtUnlock();
+ }
+ }
+
public Point getLocationOnScreen() {
synchronized (target.getTreeLock()) {
Component comp = target;
--- old/src/solaris/classes/sun/java2d/xr/XRSurfaceData.java 2010-09-24 22:25:15.309407237 +0200
+++ new/src/solaris/classes/sun/java2d/xr/XRSurfaceData.java 2010-09-24 22:25:15.025418970 +0200
@@ -55,6 +55,8 @@
native void initXRPicture(long xsdo, int pictForm);
+ native void freeXSDOPicture(long xsdo);
+
public static final String DESC_BYTE_A8_X11 = "Byte A8 Pixmap";
public static final String DESC_INT_RGB_X11 = "Integer RGB Pixmap";
public static final String DESC_INT_ARGB_X11 = "Integer ARGB-Pre Pixmap";
@@ -396,6 +398,10 @@
int validatedRepeat = XRUtils.RepeatNone;
int validatedFilter = XRUtils.FAST;
+ public void freeSurfacePicture() {
+ freeXSDOPicture(getNativeOps());
+ }
+
/**
* Validates an XRSurfaceData when used as source. Note that the clip is
* applied when used as source as well as destination.
--- old/src/solaris/native/sun/java2d/x11/X11SurfaceData.c 2010-09-24 22:25:15.937407516 +0200
+++ new/src/solaris/native/sun/java2d/x11/X11SurfaceData.c 2010-09-24 22:25:15.665159018 +0200
@@ -375,6 +375,12 @@
AWT_LOCK();
xsdo->invalid = JNI_TRUE;
+
+ if(xsdo->xrPic != None) {
+ XRenderFreePicture(awt_display, xsdo->xrPic);
+ xsdo->xrPic = None;
+ }
+
if (xsdo->isPixmap == JNI_TRUE && xsdo->drawable != 0) {
#ifdef MITSHM
if (xsdo->shmPMData.shmSegInfo != NULL) {
@@ -407,10 +413,6 @@
xsdo->cachedGC = NULL;
}
- if(xsdo->xrPic != None) {
- XRenderFreePicture(awt_display, xsdo->xrPic);
- }
-
AWT_UNLOCK();
#endif /* !HEADLESS */
}
--- old/src/solaris/native/sun/java2d/x11/XRSurfaceData.c 2010-09-24 22:25:16.631157831 +0200
+++ new/src/solaris/native/sun/java2d/x11/XRSurfaceData.c 2010-09-24 22:25:16.299407586 +0200
@@ -114,3 +114,26 @@
XShared_initSurface(env, xsdo, depth, width, height, drawable);
#endif /* !HEADLESS */
}
+
+
+JNIEXPORT void JNICALL
+Java_sun_java2d_xr_XRSurfaceData_freeXSDOPicture(JNIEnv *env, jobject xsd,
+ jlong pXSData)
+{
+#ifndef HEADLESS
+ X11SDOps *xsdo;
+
+ J2dTraceLn(J2D_TRACE_INFO, "in XRSurfaceData_freeXSDOPicture");
+
+ xsdo = X11SurfaceData_GetOps(env, xsd);
+ if (xsdo == NULL) {
+ return;
+ }
+
+ if(xsdo->xrPic != None) {
+ XRenderFreePicture(awt_display, xsdo->xrPic);
+ xsdo->xrPic = None;
+ }
+#endif /* !HEADLESS */
+}
+
