[comp.windows.x] Bug in XtGetGC

glasgow@POPEYE.NOSC.MIL (Michael Glasgow) (11/02/89)

The following is the bug report I submitted to xbugs.

			  X Window System Bug Report
			    xbugs@expo.lcs.mit.edu




VERSION:
    R3

CLIENT MACHINE and OPERATING SYSTEM:
	Sun 4/110 ( Navy DTC ) Sun OS 4.0.3 and XNews Server

DISPLAY:
	Sun CG6

WINDOW MANAGER:
	Sun PostScript Window Manager

AREA:
	X Toolkit - XtGetGC

SYNOPSIS:
	XtGetGC causes BadDrawable errors to occur when accessing
	multiple displays.


DESCRIPTION:

	The problem is cuased by the way XtGetGC creates and stores
	drawables to be used in the call to XCreateGC.  A valid
	drawable must be passed to this routine.  It is checked for
	validity by the X server, and then ( apparently ) discarded.
	XtGetGC uses the RootWindow of the display if it matches the
	depth of the requested GC.  However, if it does not match,
	it creates a drawable of the given depth, and stores this in
	an array called GCParents.  This drawable is then used to create
	the GC.  If another GC is requested of the same depth, the Drawable
	that was stored in the GCParents array is used instead of
	creating a new one.

	This works well if only a single display is used.  But if a second
	display is opened, and a GC is requested that has a depth different
	from the RootWindow, a drawable is taken from the array.  This
	drawable was created by the first server, and the result is a
	BadDrawble error.

REPEAT BY:

	Open a display, then create and realize a widget ( such as the HP
	MenuButton Widget ) that uses a GC ( Window ) of different depth than the
	root window.  Open a second display, create and realize this same
	widget on the second display.


SAMPLE FIX:

*** oldXt/GCManager.c	Wed Nov  1 20:52:01 1989
--- Xt/GCManager.c	Wed Nov  1 20:52:14 1989
***************
*** 42,50 ****
      struct _GCrec *next;	/* Next GC for this widgetkind. */
  } GCrec, *GCptr;
  
- static Drawable GCparents[256]; /* static initialized to zero, K&R ss 4.9 */
  static GCrec    *GClist = NULL;
  
  static Bool Matches(ptr, valueMask, v)
  	     GCptr	    ptr;
      register XtValueMask    valueMask;
--- 42,88 ----
      struct _GCrec *next;	/* Next GC for this widgetkind. */
  } GCrec, *GCptr;
  
  static GCrec    *GClist = NULL;
  
+ typedef struct _GCParentRec {
+   Display    *dpy;
+   Drawable   drawables[ 256 ];
+ } GCParentRec;
+ 
+ GCParentRec  parents[10];
+ int          nparents = 0;
+ 
+ static void AddGCParent( dpy, depth, drawable )
+      Display   *dpy;
+      int       depth;
+      Drawable  drawable;
+ {
+   int   i;
+ 
+   for ( i = 0; i < nparents; i++ )
+     if ( parents[ i ].dpy == dpy ) {
+       parents[ i ].drawables[ depth ] = drawable;
+       return;
+     }
+   parents[ nparents ].dpy                = dpy;
+   parents[ nparents ].drawables[ depth ] = drawable;
+   nparents++;
+ }
+ 
+ 
+ static Drawable GetGCParent( dpy, depth )
+      Display   *dpy;
+      int       depth;
+ {
+   int   i;
+ 
+   for ( i = 0; i < nparents; i++ ) {
+     if ( parents[ i ].dpy == dpy )
+       return( parents[ i ].drawables[ depth ] );
+   }
+   return( 0 );
+ }
+ 
  static Bool Matches(ptr, valueMask, v)
  	     GCptr	    ptr;
      register XtValueMask    valueMask;
***************
*** 100,106 ****
   * Return a read-only GC with the given values.  
   */
  
! GC XtGetGC(widget, valueMask, values)
  	     Widget	widget;
      register XtGCMask	valueMask;
  	     XGCValues	*values;
--- 138,144 ----
   * Return a read-only GC with the given values.  
   */
  
! GC XtGetGC( widget, valueMask, values )
  	     Widget	widget;
      register XtGCMask	valueMask;
  	     XGCValues	*values;
***************
*** 114,122 ****
  
      /* Search for existing GC that matches exactly */
      for (cur = GClist, prev = NULL; cur != NULL; prev = cur, cur = cur->next) {
! 	if (cur->valueMask == valueMask && cur->depth == depth
! 		&& cur->screen == screen && cur->dpy == dpy
! 		&& Matches(cur, valueMask, values)) {
              cur->ref_count++;
  	    /* Move this GC to front of list if not already there */
  	    if (prev != NULL) {
--- 152,163 ----
  
      /* Search for existing GC that matches exactly */
      for (cur = GClist, prev = NULL; cur != NULL; prev = cur, cur = cur->next) {
! 	if ( ( cur->valueMask == valueMask ) &&
! 	     ( cur->depth == depth ) &&
! 	     ( cur->screen == screen ) &&
! 	     ( cur->dpy == dpy ) &&
! 	     ( Matches(cur, valueMask, values ) ) ) {
! 
              cur->ref_count++;
  	    /* Move this GC to front of list if not already there */
  	    if (prev != NULL) {
***************
*** 124,160 ****
  		cur->next = GClist;
  		GClist = cur;
  	    }
! 	    return cur->gc;
  	}
      }
  
      /* No matches, have to create a new one */
!     cur		= XtNew(GCrec);
      cur->next   = GClist;
      GClist      = cur;
  
!     cur->dpy	    = XtDisplay(widget);
      cur->screen     = screen;
      cur->depth      = depth;
      cur->ref_count  = 1;
      cur->valueMask  = valueMask;
!     cur->values     = *values;
!     if (XtWindow(widget) == NULL) {
  	/* Have to create a bogus pixmap for the GC.  Stupid X protocol. */
! 	if (GCparents[depth] != 0) {
! 	    drawable = GCparents[depth];
!         } else {
  	    if (depth == DefaultDepthOfScreen(screen))
  		drawable = RootWindowOfScreen(screen);
  	    else 
! 		drawable = XCreatePixmap(cur->dpy, screen->root, 1, 1, depth);
!            GCparents[depth] = drawable;
          }
      } else {
! 	drawable = XtWindow(widget);
      }
      cur->gc = XCreateGC(cur->dpy, drawable, valueMask, values);
!     return cur->gc;
  } /* XtGetGC */
  
  void  XtReleaseGC(widget, gc)
--- 165,201 ----
  		cur->next = GClist;
  		GClist = cur;
  	    }
! 	    return( cur->gc );
  	}
      }
  
      /* No matches, have to create a new one */
!     cur		= XtNew( GCrec );
      cur->next   = GClist;
      GClist      = cur;
  
!     cur->dpy	    = XtDisplay( widget );
      cur->screen     = screen;
      cur->depth      = depth;
      cur->ref_count  = 1;
      cur->valueMask  = valueMask;
! /*    cur->values     = *values;  */
!     memcpy( ( char * )&( cur->values ), ( char * )values, sizeof( XGCValues ) );
!     if ( XtWindow( widget ) == NULL ) {
  	/* Have to create a bogus pixmap for the GC.  Stupid X protocol. */
!         drawable = GetGCParent( cur->dpy, depth );
! 	if ( drawable == 0 ) {
  	    if (depth == DefaultDepthOfScreen(screen))
  		drawable = RootWindowOfScreen(screen);
  	    else 
! 		drawable = XCreatePixmap( cur->dpy, screen->root, 1, 1, depth );
! 	    AddGCParent( cur->dpy, depth, drawable );
          }
      } else {
! 	drawable = XtWindow( widget );
      }
      cur->gc = XCreateGC(cur->dpy, drawable, valueMask, values);
!     return( cur->gc );
  } /* XtGetGC */
  
  void  XtReleaseGC(widget, gc)


Michael Glasgow
glasogw@popeye.nosc.mil