pointerClient was still static.

do not make requestedRegion empty without reason.

the cursor handling for clients which don't handle CursorShape updates was
completely broken. It originally was very complicated for performance
reasons, however, in most cases it made performance even worse, because at
idle times there was way too much checking going on, and furthermore,
sometimes unnecessary updates were inevitable.

The code now is much more elegant: the ClientRec structure knows exactly
where it last painted the cursor, and the ScreenInfo structure knows where
the cursor shall be.

As a consequence there is no more rfbDrawCursor()/rfbUndrawCursor(), no more
dontSendFramebufferUpdate, and no more isCursorDrawn.  It is now possible to
have clients which understand CursorShape updates and clients which don't at
the same time.

rfbSetCursor no longer has the option freeOld; this is obsolete, as the cursor
structure knows what to free and what not.

10 files changed, 196 insertions, 245 deletions

diff --git a/ChangeLog b/ChangeLog
index 926bbfa..f37a118 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,24 @@
+2005-01-18  Johannes E. Schindelin  <Johannes.Schindelin@gmx.de>
+	* rfb/rfb.h libvncserver/rfbserver.c: pointerClient was still static
+	* libvncserver/rfbserver.c: do not make requestedRegion empty without
+	  reason.
+	* almost everything: the cursor handling for clients which don't handle
+	  CursorShape updates was completely broken. It originally was very
+	  complicated for performance reasons, however, in most cases it made
+	  performance even worse, because at idle times there was way too much
+	  checking going on, and furthermore, sometimes unnecessary updates
+	  were inevitable.
+	  The code now is much more elegant: the ClientRec structure knows
+	  exactly where it last painted the cursor, and the ScreenInfo
+	  structure knows where the cursor shall be.
+	  As a consequence there is no more rfbDrawCursor()/rfbUndrawCursor(),
+	  no more dontSendFramebufferUpdate, and no more isCursorDrawn.
+	  It is now possible to have clients which understand CursorShape
+	  updates and clients which don't at the same time.
+	* libvncserver/cursor.c: rfbSetCursor no longer has the option
+	  freeOld; this is obsolete, as the cursor structure knows what
+	  to free and what not.
+
 2005-01-15  Karl Runge <runge@karlrunge.com>
 	* rfb/rfb.h: add alphaSource and alphaPreMultiplied to rfbCursor. 
 	* libvncserver/cursor.c: do cursor alpha blending in rfbDrawCursor()
diff --git a/contrib/zippy.c b/contrib/zippy.c
index c535b86..83be59e 100644
--- a/contrib/zippy.c
+++ b/contrib/zippy.c
@@ -145,7 +145,6 @@ void on_key_press (rfbBool down,rfbKeySym key,rfbClientPtr cl)
 
         case XK_b:
         case XK_B:
-                rfbUndrawCursor(cl->screen);
                 blank_framebuffer(cl->screen->frameBuffer, 0, 0, maxx, maxy);
                 rfbDrawString(cl->screen,&default8x16Font,20,maxy-20,"Hello, World!",0xffffff);
                 rfbMarkRectAsModified(cl->screen,0, 0,maxx,maxy);
@@ -153,7 +152,6 @@ void on_key_press (rfbBool down,rfbKeySym key,rfbClientPtr cl)
                 break;
         case XK_p:
         case XK_P:
-                rfbUndrawCursor(cl->screen);
                 /* draw_primary_colors (cl->screen->frameBuffer, 0, 0, maxx, maxy); */
 		draw_primary_colours_generic_ultrafast (cl->screen, 0, 0, maxx, maxy);
                 rfbMarkRectAsModified(cl->screen,0, 0,maxx,maxy);
@@ -165,7 +163,6 @@ void on_key_press (rfbBool down,rfbKeySym key,rfbClientPtr cl)
                 exit(0);
         case XK_C:
         case XK_c:
-                rfbUndrawCursor(cl->screen);
 		rfbDrawString(cl->screen,&default8x16Font,20,100,"Hello, World!",0xffffff);
                 rfbMarkRectAsModified(cl->screen,0, 0,maxx,maxy);
                 break;
diff --git a/examples/example.c b/examples/example.c
index 7c6c3e3..da1385e 100644
--- a/examples/example.c
+++ b/examples/example.c
@@ -125,9 +125,6 @@ void doptr(int buttonMask,int x,int y,rfbClientPtr cl)
 {
    ClientData* cd=cl->clientData;
 
-   if(cl->screen->cursorIsDrawn)
-     rfbUndrawCursor(cl->screen);
-
    if(x>=0 && y>=0 && x<maxx && y<maxy) {
       if(buttonMask) {
 	 int i,j,x1,x2,y1,y2;
@@ -172,8 +169,6 @@ void dokey(rfbBool down,rfbKeySym key,rfbClientPtr cl)
     if(key==XK_Escape)
       rfbCloseClient(cl);
     else if(key==XK_Page_Up) {
-      if(cl->screen->cursorIsDrawn)
-	rfbUndrawCursor(cl->screen);
       initBuffer((unsigned char*)cl->screen->frameBuffer);
       rfbMarkRectAsModified(cl->screen,0,0,maxx,maxy);
     } else if (key == XK_Up) {
@@ -195,8 +190,6 @@ void dokey(rfbBool down,rfbKeySym key,rfbClientPtr cl)
     } else if(key>=' ' && key<0x100) {
       ClientData* cd=cl->clientData;
       int x1=cd->oldx,y1=cd->oldy,x2,y2;
-      if(cl->screen->cursorIsDrawn)
-	rfbUndrawCursor(cl->screen);
       cd->oldx+=rfbDrawCharWithClip(cl->screen,&radonFont,cd->oldx,cd->oldy,(char)key,0,0,cl->screen->width,cl->screen->height,0x00ffffff,0x00ffffff);
       rfbFontBBox(&radonFont,(char)key,&x1,&y1,&x2,&y2);
       rfbMarkRectAsModified(cl->screen,x1,y1,x2-1,y2-1);
diff --git a/libvncserver/cursor.c b/libvncserver/cursor.c
index 1600cb5..60730d4 100644
--- a/libvncserver/cursor.c
+++ b/libvncserver/cursor.c
@@ -23,6 +23,7 @@
  */
 
 #include <rfb/rfb.h>
+#include <rfb/rfbregion.h>
 
 /*
  * Send cursor shape either in X-style format or in client pixel format.
@@ -387,19 +388,20 @@ void rfbMakeRichCursorFromXCursor(rfbScreenInfoPtr rfbScreen,rfbCursorPtr cursor
 
 /* functions to draw/hide cursor directly in the frame buffer */
 
-void rfbUndrawCursor(rfbScreenInfoPtr s)
+void rfbHideCursor(rfbClientPtr cl)
 {
+   rfbScreenInfoPtr s=cl->screen;
    rfbCursorPtr c=s->cursor;
    int j,x1,x2,y1,y2,bpp=s->serverFormat.bitsPerPixel/8,
      rowstride=s->paddedWidthInBytes;
    LOCK(s->cursorMutex);
-   if(!s->cursorIsDrawn || !c) {
+   if(!c) {
      UNLOCK(s->cursorMutex);
      return;
    }
    
    /* restore what is under the cursor */
-   x1=s->cursorX-c->xhot;
+   x1=cl->cursorX-c->xhot;
    x2=x1+c->width;
    if(x1<0) x1=0;
    if(x2>=s->width) x2=s->width-1;
@@ -407,7 +409,7 @@ void rfbUndrawCursor(rfbScreenInfoPtr s)
      UNLOCK(s->cursorMutex);
      return;
    }
-   y1=s->cursorY-c->yhot;
+   y1=cl->cursorY-c->yhot;
    y2=y1+c->height;
    if(y1<0) y1=0;
    if(y2>=s->height) y2=s->height-1;
@@ -422,15 +424,12 @@ void rfbUndrawCursor(rfbScreenInfoPtr s)
 	    s->underCursorBuffer+j*x2*bpp,
 	    x2*bpp);
    
-   /* rfbMarkRectAsModified(s,x1,y1,x1+x2,y1+y2); */
-   s->cursorIsDrawn = FALSE;
-   s->oldCursorX=s->cursorX;
-   s->oldCursorY=s->cursorY;
    UNLOCK(s->cursorMutex);
 }
 
-void rfbDrawCursor(rfbScreenInfoPtr s)
+void rfbShowCursor(rfbClientPtr cl)
 {
+   rfbScreenInfoPtr s=cl->screen;
    rfbCursorPtr c=s->cursor;
    int i,j,x1,x2,y1,y2,i1,j1,bpp=s->serverFormat.bitsPerPixel/8,
      rowstride=s->paddedWidthInBytes,
@@ -439,18 +438,7 @@ void rfbDrawCursor(rfbScreenInfoPtr s)
 
    if(!c) return;
    LOCK(s->cursorMutex);
-   if(s->cursorIsDrawn) {
-     /* is already drawn */
-     UNLOCK(s->cursorMutex);
-     return;
-   }
 
-   if(s->cursor && s->underCursorBuffer &&
-		   (s->cursorX!=s->oldCursorX || s->cursorY!=s->oldCursorY)) {
-	   int x1=s->oldCursorX-s->cursor->xhot,x2=x1+s->cursor->width;
-	   int y1=s->oldCursorY-s->cursor->yhot,y2=y1+s->cursor->height;
-	   rfbMarkRectAsModified(s,x1,y1,x2,y2);
-   }
    bufSize=c->width*c->height*bpp;
    w=(c->width+7)/8;
    if(s->underCursorBufferLen<bufSize) {
@@ -459,9 +447,10 @@ void rfbDrawCursor(rfbScreenInfoPtr s)
       s->underCursorBuffer=malloc(bufSize);
       s->underCursorBufferLen=bufSize;
    }
+
    /* save what is under the cursor */
    i1=j1=0; /* offset in cursor */
-   x1=s->cursorX-c->xhot;
+   x1=cl->cursorX-c->xhot;
    x2=x1+c->width;
    if(x1<0) { i1=-x1; x1=0; }
    if(x2>=s->width) x2=s->width-1;
@@ -469,7 +458,8 @@ void rfbDrawCursor(rfbScreenInfoPtr s)
      UNLOCK(s->cursorMutex);
      return; /* nothing to do */
    }
-   y1=s->cursorY-c->yhot;
+
+   y1=cl->cursorY-c->yhot;
    y2=y1+c->height;
    if(y1<0) { j1=-y1; y1=0; }
    if(y2>=s->height) y2=s->height-1;
@@ -491,7 +481,9 @@ void rfbDrawCursor(rfbScreenInfoPtr s)
    
    if(!c->richSource)
      rfbMakeRichCursorFromXCursor(s,c);
-   
+  
+   fprintf(stderr,"show cursor at %d %d\n",x1,y1);
+
    if (c->alphaSource) {
 	int rmax, rshift;
 	int gmax, gshift;
@@ -571,12 +563,38 @@ void rfbDrawCursor(rfbScreenInfoPtr s)
    		c->richSource+(j+j1)*c->width*bpp+(i+i1)*bpp,bpp);
    }
 
-   if(wasChanged)
-     rfbMarkRectAsModified(s,x1,y1,x1+x2,y1+y2);
-   s->cursorIsDrawn = TRUE;
    UNLOCK(s->cursorMutex);
 }
 
+/* 
+ * If enableCursorShapeUpdates is FALSE, and the cursor is hidden, make sure
+ * that if the frameBuffer was transmitted with a cursor drawn, then that
+ * region gets redrawn.
+ */
+
+void rfbRedrawAfterHideCursor(rfbClientPtr cl)
+{
+    rfbScreenInfoPtr s = cl->screen;
+    rfbCursorPtr c = s->cursor;
+    
+    if(c) {
+	int x,y,x2,y2;
+
+	x = cl->cursorX-c->xhot;
+	y = cl->cursorY-c->yhot;
+	x2 = x+c->width;
+	y2 = y+c->height;
+
+	if(sraClipRect2(&x,&y,&x2,&y2,0,0,s->width,s->height)) {
+	    sraRegionPtr rect;
+	    fprintf(stderr,"%d %d %d %d\n",x,y,x2,y2);
+	    rect = sraRgnCreateRect(x,y,x2,y2);
+	    sraRgnOr(cl->modifiedRegion,rect);
+	    sraRgnDestroy(rect);
+	}
+    }
+}
+
 /* for debugging */
 
 void rfbPrintXCursor(rfbCursorPtr cursor)
@@ -593,23 +611,33 @@ void rfbPrintXCursor(rfbCursorPtr cursor)
    }
 }
 
-void rfbSetCursor(rfbScreenInfoPtr rfbScreen,rfbCursorPtr c,rfbBool freeOld)
+void rfbSetCursor(rfbScreenInfoPtr rfbScreen,rfbCursorPtr c)
 {
+  rfbClientIteratorPtr iterator;
+  rfbClientPtr cl;
+
   LOCK(rfbScreen->cursorMutex);
-  while(rfbScreen->cursorIsDrawn) {
-    UNLOCK(rfbScreen->cursorMutex);
-    rfbUndrawCursor(rfbScreen);
-    LOCK(rfbScreen->cursorMutex);
-  }
 
-  free(rfbScreen->underCursorBuffer);
-  rfbScreen->underCursorBuffer=0;
-  rfbScreen->underCursorBufferLen=0;
+  if(rfbScreen->cursor && rfbScreen->cursor->cleanup) {
+    iterator=rfbGetClientIterator(rfbScreen);
+    while((cl=rfbClientIteratorNext(iterator)))
+	if(!cl->enableCursorShapeUpdates)
+	  rfbRedrawAfterHideCursor(cl);
+    rfbReleaseClientIterator(iterator);
 
-  if(rfbScreen->cursor && (freeOld || rfbScreen->cursor->cleanup))
     rfbFreeCursor(rfbScreen->cursor);
+  }
 
   rfbScreen->cursor = c;
 
+  iterator=rfbGetClientIterator(rfbScreen);
+  while((cl=rfbClientIteratorNext(iterator))) {
+    cl->cursorWasChanged = TRUE;
+    if(!cl->enableCursorShapeUpdates)
+      rfbRedrawAfterHideCursor(cl);
+  }
+  rfbReleaseClientIterator(iterator);
+
   UNLOCK(rfbScreen->cursorMutex);
 }
+
diff --git a/libvncserver/main.c b/libvncserver/main.c
index 9340073..102cebb 100644
--- a/libvncserver/main.c
+++ b/libvncserver/main.c
@@ -50,6 +50,9 @@ char rfbEndianTest = -1;
 void rfbIncrClientRef(rfbClientPtr cl);
 void rfbDecrClientRef(rfbClientPtr cl);
 
+/* cursor.c */
+void rfbRedrawAfterHideCursor(rfbClientPtr cl);
+
 void rfbLogEnable(int enabled) {
   rfbEnableLogging=enabled;
 }
@@ -95,8 +98,6 @@ void rfbScheduleCopyRegion(rfbScreenInfoPtr rfbScreen,sraRegionPtr copyRegion,in
    rfbClientIteratorPtr iterator;
    rfbClientPtr cl;
 
-   rfbUndrawCursor(rfbScreen);
-  
    iterator=rfbGetClientIterator(rfbScreen);
    while((cl=rfbClientIteratorNext(iterator))) {
      LOCK(cl->updateMutex);
@@ -132,23 +133,6 @@ void rfbScheduleCopyRegion(rfbScreenInfoPtr rfbScreen,sraRegionPtr copyRegion,in
        sraRgnAnd(modifiedRegionBackup,cl->copyRegion);
        sraRgnOr(cl->modifiedRegion,modifiedRegionBackup);
        sraRgnDestroy(modifiedRegionBackup);
-
-#if 0
-       /* TODO: is this needed? Or does it mess up deferring? */
-       /* while(!sraRgnEmpty(cl->copyRegion)) */ {
-#ifdef LIBVNCSERVER_HAVE_LIBPTHREAD
-	 if(!cl->screen->backgroundLoop)
-#endif
-	   {
-	     sraRegionPtr updateRegion = sraRgnCreateRgn(cl->modifiedRegion);
-	     sraRgnOr(updateRegion,cl->copyRegion);
-	     UNLOCK(cl->updateMutex);
-	     rfbSendFramebufferUpdate(cl,updateRegion);
-	     sraRgnDestroy(updateRegion);
-	     continue;
-	   }
-       }
-#endif
      } else {
        sraRgnOr(cl->modifiedRegion,copyRegion);
      }
@@ -167,8 +151,6 @@ void rfbDoCopyRegion(rfbScreenInfoPtr screen,sraRegionPtr copyRegion,int dx,int
     rowstride=screen->paddedWidthInBytes;
    char *in,*out;
 
-   rfbUndrawCursor(screen);
-  
    /* copy it, really */
    i = sraRgnGetReverseIterator(copyRegion,dx<0,dy<0);
    while(sraRgnIteratorNext(i,&rect)) {
@@ -372,23 +354,21 @@ rfbDefaultPtrAddEvent(int buttonMask, int x, int y, rfbClientPtr cl)
 {
   rfbClientIteratorPtr iterator;
   rfbClientPtr other_client;
+  rfbScreenInfoPtr s = cl->screen;
+  rfbCursorPtr c = s->cursor;
 
-  if (x != cl->screen->cursorX || y != cl->screen->cursorY) {
-    if (cl->screen->cursorIsDrawn)
-      rfbUndrawCursor(cl->screen);
-    LOCK(cl->screen->cursorMutex);
-    if (!cl->screen->cursorIsDrawn) {
-      cl->screen->cursorX = x;
-      cl->screen->cursorY = y;
-    }
-    UNLOCK(cl->screen->cursorMutex);
+  if (x != s->cursorX || y != s->cursorY) {
+    LOCK(s->cursorMutex);
+    s->cursorX = x;
+    s->cursorY = y;
+    UNLOCK(s->cursorMutex);
 
     /* The cursor was moved by this client, so don't send CursorPos. */
     if (cl->enableCursorPosUpdates)
       cl->cursorWasMoved = FALSE;
 
     /* But inform all remaining clients about this cursor movement. */
-    iterator = rfbGetClientIterator(cl->screen);
+    iterator = rfbGetClientIterator(s);
     while ((other_client = rfbClientIteratorNext(iterator)) != NULL) {
       if (other_client != cl && other_client->enableCursorPosUpdates) {
 	other_client->cursorWasMoved = TRUE;
@@ -562,6 +542,7 @@ rfbScreenInfoPtr rfbGetScreen(int* argc,char** argv,
 
    screen->autoPort=FALSE;
    screen->clientHead=0;
+   screen->pointerClient=0;
    screen->port=5900;
    screen->socketInitDone=FALSE;
 
@@ -623,8 +604,6 @@ rfbScreenInfoPtr rfbGetScreen(int* argc,char** argv,
 
    /* cursor */
 
-   screen->cursorIsDrawn = FALSE;
-   screen->dontSendFramebufferUpdate = FALSE;
    screen->cursorX=screen->cursorY=screen->underCursorBufferLen=0;
    screen->underCursorBuffer=NULL;
    screen->dontConvertRichCursorToXCursor = FALSE;
@@ -672,10 +651,6 @@ void rfbNewFramebuffer(rfbScreenInfoPtr screen, char *framebuffer,
   rfbClientIteratorPtr iterator;
   rfbClientPtr cl;
 
-  /* Remove the pointer */
-
-  rfbUndrawCursor(screen);
-
   /* Update information in the screenInfo structure */
 
   old_format = screen->serverFormat;
diff --git a/libvncserver/rfbserver.c b/libvncserver/rfbserver.c
index 84b5035..f9ab595 100644
--- a/libvncserver/rfbserver.c
+++ b/libvncserver/rfbserver.c
@@ -60,7 +60,11 @@
 #define DEBUGPROTO(x)
 #endif
 
-static rfbClientPtr pointerClient = NULL;  /* "Mutex" for pointer events */
+/* from cursor.c */
+
+void rfbShowCursor(rfbClientPtr cl);
+void rfbHideCursor(rfbClientPtr cl);
+void rfbRedrawAfterHideCursor(rfbClientPtr cl);
 
 static void rfbProcessClientProtocolVersion(rfbClientPtr cl);
 static void rfbProcessClientNormalMessage(rfbClientPtr cl);
@@ -327,6 +331,8 @@ rfbNewTCPOrUDPClient(rfbScreen,sock,isUDP)
       cl->enableCursorPosUpdates = FALSE;
       cl->useRichCursorEncoding = FALSE;
       cl->enableLastRectEncoding = FALSE;
+      cl->cursorX = rfbScreen->cursorX;
+      cl->cursorY = rfbScreen->cursorY;
       cl->useNewFBSize = FALSE;
 
 #ifdef LIBVNCSERVER_HAVE_LIBZ
@@ -452,8 +458,8 @@ rfbClientConnectionGone(cl)
 #endif
 #endif
 
-    if (pointerClient == cl)
-        pointerClient = NULL;
+    if (cl->screen->pointerClient == cl)
+        cl->screen->pointerClient = 0;
 
     sraRgnDestroy(cl->modifiedRegion);
     sraRgnDestroy(cl->requestedRegion);
@@ -756,13 +762,6 @@ rfbProcessClientNormalMessage(cl)
 
         msg.se.nEncodings = Swap16IfLE(msg.se.nEncodings);
 
-        cl->preferredEncoding = -1;
-	cl->useCopyRect = FALSE;
-	cl->enableCursorShapeUpdates = FALSE;
-	cl->enableCursorPosUpdates = FALSE;
-	cl->enableLastRectEncoding = FALSE;
-        cl->useNewFBSize = FALSE;
-
         for (i = 0; i < msg.se.nEncodings; i++) {
             if ((n = rfbReadExact(cl, (char *)&enc, 4)) <= 0) {
                 if (n != 0)
@@ -827,6 +826,10 @@ rfbProcessClientNormalMessage(cl)
 		if(!cl->screen->dontConvertRichCursorToXCursor) {
 		    rfbLog("Enabling X-style cursor updates for client %s\n",
 			   cl->host);
+		    /* if cursor was drawn, hide the cursor */
+		    if(!cl->enableCursorShapeUpdates)
+		        rfbRedrawAfterHideCursor(cl);
+
 		    cl->enableCursorShapeUpdates = TRUE;
 		    cl->cursorWasChanged = TRUE;
 		}
@@ -834,6 +837,10 @@ rfbProcessClientNormalMessage(cl)
 	    case rfbEncodingRichCursor:
 	        rfbLog("Enabling full-color cursor updates for client %s\n",
 		       cl->host);
+		/* if cursor was drawn, hide the cursor */
+		if(!cl->enableCursorShapeUpdates)
+		    rfbRedrawAfterHideCursor(cl);
+
 	        cl->enableCursorShapeUpdates = TRUE;
 	        cl->useRichCursorEncoding = TRUE;
 	        cl->cursorWasChanged = TRUE;
@@ -994,13 +1001,13 @@ rfbProcessClientNormalMessage(cl)
 	    return;
 	}
 
-	if (pointerClient && (pointerClient != cl))
+	if (cl->screen->pointerClient && cl->screen->pointerClient != cl)
 	    return;
 
 	if (msg.pe.buttonMask == 0)
-	    pointerClient = NULL;
+	    cl->screen->pointerClient = 0;
 	else
-	    pointerClient = cl;
+	    cl->screen->pointerClient = cl;
 
 	if(!cl->viewOnly) {
 	    cl->screen->ptrAddEvent(msg.pe.buttonMask,
@@ -1066,7 +1073,7 @@ rfbSendFramebufferUpdate(cl, givenUpdateRegion)
      rfbClientPtr cl;
      sraRegionPtr givenUpdateRegion;
 {
-    sraRectangleIterator* i;
+    sraRectangleIterator* i=0;
     sraRect rect;
     int nUpdateRegionRects;
     rfbFramebufferUpdateMsg *fu = (rfbFramebufferUpdateMsg *)cl->updateBuf;
@@ -1074,6 +1081,7 @@ rfbSendFramebufferUpdate(cl, givenUpdateRegion)
     int dx, dy;
     rfbBool sendCursorShape = FALSE;
     rfbBool sendCursorPos = FALSE;
+    rfbBool result = TRUE;
 
     if(cl->screen->displayHook)
       cl->screen->displayHook(cl);
@@ -1103,16 +1111,8 @@ rfbSendFramebufferUpdate(cl, givenUpdateRegion)
      */
 
     if (cl->enableCursorShapeUpdates) {
-      if (cl->screen->cursorIsDrawn) {
-	rfbUndrawCursor(cl->screen);
-      }
-      if (!cl->screen->cursorIsDrawn && cl->cursorWasChanged &&
-	  cl->readyForSetColourMapEntries)
+      if (cl->cursorWasChanged && cl->readyForSetColourMapEntries)
 	  sendCursorShape = TRUE;
-    } else {
-      if (!cl->screen->cursorIsDrawn) {
-	rfbDrawCursor(cl->screen);
-      }
     }
 
     /*
@@ -1162,6 +1162,9 @@ rfbSendFramebufferUpdate(cl, givenUpdateRegion)
 
     sraRgnOr(updateRegion,cl->copyRegion);
     if(!sraRgnAnd(updateRegion,cl->requestedRegion) &&
+       sraRgnEmpty(updateRegion) &&
+       (cl->enableCursorShapeUpdates ||
+	(cl->cursorX == cl->screen->cursorX && cl->cursorY == cl->screen->cursorY)) &&
        !sendCursorShape && !sendCursorPos) {
       sraRgnDestroy(updateRegion);
       UNLOCK(cl->updateMutex);
@@ -1201,22 +1204,32 @@ rfbSendFramebufferUpdate(cl, givenUpdateRegion)
      * carry over a copyRegion for a future update.
      */
 
-
      sraRgnOr(cl->modifiedRegion,cl->copyRegion);
      sraRgnSubtract(cl->modifiedRegion,updateRegion);
      sraRgnSubtract(cl->modifiedRegion,updateCopyRegion);
 
-     sraRgnMakeEmpty(cl->requestedRegion);
+     /* TODO: is this sensible? sraRgnMakeEmpty(cl->requestedRegion); */
      sraRgnMakeEmpty(cl->copyRegion);
      cl->copyDX = 0;
      cl->copyDY = 0;
    
      UNLOCK(cl->updateMutex);
    
+    if (!cl->enableCursorShapeUpdates) {
+      if(cl->cursorX != cl->screen->cursorX || cl->cursorY != cl->screen->cursorY) {
+	rfbRedrawAfterHideCursor(cl);
+	LOCK(cl->screen->cursorMutex);
+	cl->cursorX = cl->screen->cursorX;
+	cl->cursorY = cl->screen->cursorY;
+	UNLOCK(cl->screen->cursorMutex);
+	rfbRedrawAfterHideCursor(cl);
+      }
+      rfbShowCursor(cl);
+    }
+
    /*
      * Now send the update.
      */
-
     cl->framebufferUpdateMessagesSent++;
 
     if (cl->preferredEncoding == rfbEncodingCoRRE) {
@@ -1291,26 +1304,19 @@ rfbSendFramebufferUpdate(cl, givenUpdateRegion)
 
    if (sendCursorShape) {
 	cl->cursorWasChanged = FALSE;
-	if (!rfbSendCursorShape(cl)) {
-	    sraRgnDestroy(updateRegion);
-	    return FALSE;
-	}
+	if (!rfbSendCursorShape(cl))
+	    goto updateFailed;
     }
    
    if (sendCursorPos) {
 	cl->cursorWasMoved = FALSE;
-	if (!rfbSendCursorPos(cl)) {
-	    sraRgnDestroy(updateRegion);
-	    return FALSE;
-	}
-    }
+	if (!rfbSendCursorPos(cl))
+	        goto updateFailed;
+   }
    
     if (!sraRgnEmpty(updateCopyRegion)) {
-	if (!rfbSendCopyRegion(cl,updateCopyRegion,dx,dy)) {
-	    sraRgnDestroy(updateRegion);
-	    sraRgnDestroy(updateCopyRegion);
-	    return FALSE;
-	}
+	if (!rfbSendCopyRegion(cl,updateCopyRegion,dx,dy))
+	        goto updateFailed;
     }
 
     sraRgnDestroy(updateCopyRegion);
@@ -1326,77 +1332,59 @@ rfbSendFramebufferUpdate(cl, givenUpdateRegion)
 
         switch (cl->preferredEncoding) {
         case rfbEncodingRaw:
-            if (!rfbSendRectEncodingRaw(cl, x, y, w, h)) {
-	        sraRgnDestroy(updateRegion);
-		sraRgnReleaseIterator(i);
-                return FALSE;
-            }
+            if (!rfbSendRectEncodingRaw(cl, x, y, w, h))
+	        goto updateFailed;
             break;
         case rfbEncodingRRE:
-            if (!rfbSendRectEncodingRRE(cl, x, y, w, h)) {
-	        sraRgnDestroy(updateRegion);
-		sraRgnReleaseIterator(i);
-                return FALSE;
-            }
+            if (!rfbSendRectEncodingRRE(cl, x, y, w, h))
+	        goto updateFailed;
             break;
         case rfbEncodingCoRRE:
-            if (!rfbSendRectEncodingCoRRE(cl, x, y, w, h)) {
-	        sraRgnDestroy(updateRegion);
-		sraRgnReleaseIterator(i);
-                return FALSE;
-            }
-            break;
+            if (!rfbSendRectEncodingCoRRE(cl, x, y, w, h))
+	        goto updateFailed;
+	    break;
         case rfbEncodingHextile:
-            if (!rfbSendRectEncodingHextile(cl, x, y, w, h)) {
-	        sraRgnDestroy(updateRegion);
-		sraRgnReleaseIterator(i);
-                return FALSE;
-            }
+            if (!rfbSendRectEncodingHextile(cl, x, y, w, h))
+	        goto updateFailed;
             break;
 #ifdef LIBVNCSERVER_HAVE_LIBZ
 	case rfbEncodingZlib:
-	    if (!rfbSendRectEncodingZlib(cl, x, y, w, h)) {
-	        sraRgnDestroy(updateRegion);
-		sraRgnReleaseIterator(i);
-		return FALSE;
-	    }
+	    if (!rfbSendRectEncodingZlib(cl, x, y, w, h))
+	        goto updateFailed;
 	    break;
 #ifdef LIBVNCSERVER_HAVE_LIBJPEG
 	case rfbEncodingTight:
-	    if (!rfbSendRectEncodingTight(cl, x, y, w, h)) {
-	        sraRgnDestroy(updateRegion);
-		sraRgnReleaseIterator(i);
-		return FALSE;
-	    }
+	    if (!rfbSendRectEncodingTight(cl, x, y, w, h))
+	        goto updateFailed;
 	    break;
 #endif
 #endif
 #ifdef LIBVNCSERVER_HAVE_LIBZ
        case rfbEncodingZRLE:
-           if (!rfbSendRectEncodingZRLE(cl, x, y, w, h)) {
-	       sraRgnDestroy(updateRegion);
-		sraRgnReleaseIterator(i);
-               return FALSE;
-           }
+           if (!rfbSendRectEncodingZRLE(cl, x, y, w, h))
+	       goto updateFailed;
            break;
 #endif
         }
     }
-    sraRgnReleaseIterator(i);
 
     if ( nUpdateRegionRects == 0xFFFF &&
-	 !rfbSendLastRectMarker(cl) ) {
-        sraRgnDestroy(updateRegion);
-	return FALSE;
-    }
+	 !rfbSendLastRectMarker(cl) )
+	    goto updateFailed;
 
     if (!rfbSendUpdateBuf(cl)) {
-        sraRgnDestroy(updateRegion);
-        return FALSE;
+updateFailed:
+	result = FALSE;
+    }
+
+    if (!cl->enableCursorShapeUpdates) {
+      rfbHideCursor(cl);
     }
 
+    if(i)
+        sraRgnReleaseIterator(i);
     sraRgnDestroy(updateRegion);
-    return TRUE;
+    return result;
 }
 
 
diff --git a/libvncserver/selbox.c b/libvncserver/selbox.c
index c8b9cc5..3633d06 100755
--- a/libvncserver/selbox.c
+++ b/libvncserver/selbox.c
@@ -244,7 +244,6 @@ int rfbSelectBox(rfbScreenInfoPtr rfbScreen,rfbFontDataPtr font,
    selData.cancelX = selData.cancelBX+(k-j)/2;
    selData.okY = y2-border;
 
-   rfbUndrawCursor(rfbScreen);
    frameBufferBackup = (char*)malloc(bpp*(x2-x1)*(y2-y1));
 
    selData.state = SELECTING;
diff --git a/rfb/rfb.h b/rfb/rfb.h
index b3775bf..e911fa1 100644
--- a/rfb/rfb.h
+++ b/rfb/rfb.h
@@ -160,47 +160,6 @@ typedef struct _rfbScreenInfo
      */
     void* screenData;
   
-    /* The following two members are used to minimise the amount of unnecessary
-       drawing caused by cursor movement.  Whenever any drawing affects the
-       part of the screen where the cursor is, the cursor is removed first and
-       then the drawing is done (this is what the sprite routines test for).
-       Afterwards, however, we do not replace the cursor, even when the cursor
-       is logically being moved across the screen.  We only draw the cursor
-       again just as we are about to send the client a framebuffer update.
-
-       We need to be careful when removing and drawing the cursor because of
-       their relationship with the normal drawing routines.  The drawing
-       routines can invoke the cursor routines, but also the cursor routines
-       themselves end up invoking drawing routines.
-
-       Removing the cursor (rfbUndrawCursor) is eventually achieved by
-       doing a CopyArea from a pixmap to the screen, where the pixmap contains
-       the saved contents of the screen under the cursor.  Before doing this,
-       however, we set cursorIsDrawn to FALSE.  Then, when CopyArea is called,
-       it sees that cursorIsDrawn is FALSE and so doesn't feel the need to
-       (recursively!) remove the cursor before doing it.
-
-       Putting up the cursor (rfbDrawCursor) involves a call to
-       PushPixels.  While this is happening, cursorIsDrawn must be FALSE so
-       that PushPixels doesn't think it has to remove the cursor first.
-       Obviously cursorIsDrawn is set to TRUE afterwards.
-
-       Another problem we face is that drawing routines sometimes cause a
-       framebuffer update to be sent to the RFB client.  When the RFB client is
-       already waiting for a framebuffer update and some drawing to the
-       framebuffer then happens, the drawing routine sees that the client is
-       ready, so it calls rfbSendFramebufferUpdate.  If the cursor is not drawn
-       at this stage, it must be put up, and so rfbSpriteRestoreCursor is
-       called.  However, if the original drawing routine was actually called
-       from within rfbSpriteRestoreCursor or rfbSpriteRemoveCursor we don't
-       want this to happen.  So both the cursor routines set
-       dontSendFramebufferUpdate to TRUE, and all the drawing routines check
-       this before calling rfbSendFramebufferUpdate. */
-
-    rfbBool cursorIsDrawn;		    /* TRUE if the cursor is currently drawn */
-    rfbBool dontSendFramebufferUpdate; /* TRUE while removing or drawing the
-				       cursor */
-   
     /* additions by libvncserver */
 
     rfbPixelFormat serverFormat;
@@ -257,9 +216,11 @@ typedef struct _rfbScreenInfo
     rfbBool neverShared;
     rfbBool dontDisconnect;
     struct _rfbClientRec* clientHead;
+    struct _rfbClientRec* pointerClient;  /* "Mutex" for pointer events */
+
 
     /* cursor */
-    int cursorX, cursorY,oldCursorX,oldCursorY,underCursorBufferLen;
+    int cursorX, cursorY,underCursorBufferLen;
     char* underCursorBuffer;
     rfbBool dontConvertRichCursorToXCursor;
     struct rfbCursor* cursor;
@@ -460,6 +421,8 @@ typedef struct _rfbClientRec {
     rfbBool useRichCursorEncoding;    /* rfbEncodingRichCursor is preferred */
     rfbBool cursorWasChanged;         /* cursor shape update should be sent */
     rfbBool cursorWasMoved;           /* cursor position update should be sent */
+    int cursorX,cursorY;	      /* the coordinates of the cursor,
+					 if enableCursorShapeUpdates = FALSE */
 
     rfbBool useNewFBSize;             /* client supports NewFBSize encoding */
     rfbBool newFBSizePending;         /* framebuffer size was changed */
@@ -501,8 +464,10 @@ typedef struct _rfbClientRec {
  */
 
 #define FB_UPDATE_PENDING(cl)                                              \
-     ((!(cl)->enableCursorShapeUpdates && !(cl)->screen->cursorIsDrawn) || \
-     ((cl)->enableCursorShapeUpdates && (cl)->cursorWasChanged) ||         \
+     (((cl)->enableCursorShapeUpdates && (cl)->cursorWasChanged) ||        \
+     (((cl)->enableCursorShapeUpdates == FALSE &&                          \
+       ((cl)->cursorX != (cl)->screen->cursorX ||                          \
+	(cl)->cursorY != (cl)->screen->cursorY))) ||                       \
      ((cl)->useNewFBSize && (cl)->newFBSizePending) ||                     \
      ((cl)->enableCursorPosUpdates && (cl)->cursorWasMoved) ||             \
      !sraRgnEmpty((cl)->copyRegion) || !sraRgnEmpty((cl)->modifiedRegion))
@@ -677,9 +642,7 @@ extern char* rfbMakeMaskForXCursor(int width,int height,char* cursorString);
 extern void rfbMakeXCursorFromRichCursor(rfbScreenInfoPtr rfbScreen,rfbCursorPtr cursor);
 extern void rfbMakeRichCursorFromXCursor(rfbScreenInfoPtr rfbScreen,rfbCursorPtr cursor);
 extern void rfbFreeCursor(rfbCursorPtr cursor);
-extern void rfbDrawCursor(rfbScreenInfoPtr rfbScreen);
-extern void rfbUndrawCursor(rfbScreenInfoPtr rfbScreen);
-extern void rfbSetCursor(rfbScreenInfoPtr rfbScreen,rfbCursorPtr c,rfbBool freeOld);
+extern void rfbSetCursor(rfbScreenInfoPtr rfbScreen,rfbCursorPtr c);
 
 /* cursor handling for the pointer */
 extern void rfbDefaultPtrAddEvent(int buttonMask,int x,int y,rfbClientPtr cl);
@@ -725,7 +688,6 @@ void rfbFreeFont(rfbFontDataPtr font);
 
 /* draw.c */
 
-/* You have to call rfbUndrawCursor before using these functions */
 void rfbFillRect(rfbScreenInfoPtr s,int x1,int y1,int x2,int y2,rfbPixel col);
 void rfbDrawPixel(rfbScreenInfoPtr s,int x,int y,rfbPixel col);
 void rfbDrawLine(rfbScreenInfoPtr s,int x1,int y1,int x2,int y2,rfbPixel col);
diff --git a/vncterm/VNConsole.c b/vncterm/VNConsole.c
index ac8e260..651546a 100644
--- a/vncterm/VNConsole.c
+++ b/vncterm/VNConsole.c
@@ -50,8 +50,7 @@ void vcDrawOrHideCursor(vncConsolePtr c)
 
 void vcDrawCursor(vncConsolePtr c)
 {
-  rfbDrawCursor(c->screen);
-  if(c->cursorActive && !c->cursorIsDrawn && c->y<c->height && c->x<c->width) {
+  if(c->cursorActive && c->y<c->height && c->x<c->width) {
     /* rfbLog("DrawCursor: %d,%d\n",c->x,c->y); */
     vcDrawOrHideCursor(c);
   }
@@ -59,13 +58,9 @@ void vcDrawCursor(vncConsolePtr c)
 
 void vcHideCursor(vncConsolePtr c)
 {
-  rfbUndrawCursor(c->screen);
   if(c->currentlyMarking)
     vcUnmark(c);
-  if(c->cursorIsDrawn) {
-    /* rfbLog("HideCursor: %d,%d\n",c->x,c->y); */
-    vcDrawOrHideCursor(c);
-  }
+  vcDrawOrHideCursor(c);
 }
 
 void vcMakeSureCursorIsDrawn(rfbClientPtr cl)
@@ -394,8 +389,6 @@ void vcPtrAddEventProc(int buttonMask,int x,int y,rfbClientPtr cl)
 {
   vncConsolePtr c=(vncConsolePtr)cl->screen->screenData;
 
-  rfbUndrawCursor(c->screen);
-
   if(c->wasRightButtonDown) {
     if((buttonMask&4)==0) {
       if(c->selection) {
diff --git a/x11vnc/x11vnc.c b/x11vnc/x11vnc.c
index 43c6ea5..8c26ee2 100644
--- a/x11vnc/x11vnc.c
+++ b/x11vnc/x11vnc.c
@@ -7162,7 +7162,6 @@ char *process_remote_cmd(char *cmd, int stringonly) {
 		}
 		rfbLog("process_remote_cmd: turning on cursorshape mode.\n");
 
-		rfbUndrawCursor(screen);
 		set_no_cursor();
 		cursor_shape_updates = 1;
 		restore_cursor_shape_updates(screen);
@@ -7176,7 +7175,6 @@ char *process_remote_cmd(char *cmd, int stringonly) {
 		}
 		rfbLog("process_remote_cmd: turning off cursorshape mode.\n");
 		
-		rfbUndrawCursor(screen);
 		set_no_cursor();
 		for (i=0; i<max; i++) {
 			/* XXX: try to force empty cursor back to client */
@@ -8319,7 +8317,6 @@ void setup_cursors(void) {
 	first = 0;
 
 	if (screen) {
-		rfbUndrawCursor(screen);
 		screen->cursor = NULL;
 		LOCK(screen->cursorMutex);
 	}
@@ -8686,9 +8683,6 @@ int get_xfixes_cursor(int init) {
 			}
 		}
 
-		if (screen) {
-			rfbUndrawCursor(screen);
-		}
 		/* we need to create the cursor and overwrite oldest */
 		use = oldest;
 		if (cursors[use]->rfb) {
@@ -9080,11 +9074,12 @@ void mark_cursor_patch_modified(rfbScreenInfoPtr s, int old) {
 		return;
 	}
 
-	if (old) {
-		/* use oldCursor pos */
+	/* TODO Karl: is this needed any longer? */
+	/* if (old) {
+		/* use oldCursor pos *//*
 		curx = s->oldCursorX;
 		cury = s->oldCursorY;
-	} else {
+	} else */ {
 		curx = s->cursorX;
 		cury = s->cursorY;
 	}
@@ -9257,18 +9252,15 @@ void cursor_position(int x, int y) {
 	if (x == screen->cursorX && y == screen->cursorY) {
 		return;
 	}
+	/* TODO Karl: do we really need x_old,y_old? */
+	/*
 	x_old = screen->oldCursorX;
 	y_old = screen->oldCursorY;
-
-	if (screen->cursorIsDrawn) {
-		rfbUndrawCursor(screen);
-	}
+	*/
 
 	LOCK(screen->cursorMutex);
-	if (! screen->cursorIsDrawn) {
-		screen->cursorX = x;
-		screen->cursorY = y;
-	}
+	screen->cursorX = x;
+	screen->cursorY = y;
 	UNLOCK(screen->cursorMutex);
 
 	iter = rfbGetClientIterator(screen);
@@ -9304,11 +9296,13 @@ void cursor_position(int x, int y) {
 	}
 	rfbReleaseClientIterator(iter);
 
+	/* TODO Karl: do we need x_old, y_old? */
+	/*
 	if (nonCursorPosUpdates_clients && show_cursor) {
 		if (x_old != x || y_old != y) {
 			mark_cursor_patch_modified(screen, 0);
 		}
-	}
+	}*/
 
 	if (debug_pointer && cnt) {
 		rfbLog("cursor_position: sent position x=%3d y=%3d to %d"
@@ -9355,9 +9349,10 @@ void set_rfb_cursor(int which) {
 		rfbLog("non-existent cursor: which=%d\n", which);
 		return;
 	} else {
-		rfbSetCursor(screen, cursors[which]->rfb, FALSE);
+		rfbSetCursor(screen, cursors[which]->rfb);
 	}
 
+	/* TODO Karl: is this still necessary? */
 	/* this is a 2nd workaround for rfbSetCursor() */
 	if (workaround > 1) {
 		if (screen->underCursorBuffer == NULL &&
@@ -9368,6 +9363,7 @@ void set_rfb_cursor(int which) {
 		}
 	}
 
+	/* TODO Karl: is this still necessary? */
 	if (workaround) {
 		set_cursor_was_changed(screen);
 	}
@@ -14366,7 +14362,6 @@ static void watch_loop(void) {
 			double tm = 0.0;
 			dtime(&tm);
 
-			rfbUndrawCursor(screen);
 			if (use_snapfb) {
 				int t, tries = 5;
 				copy_snap();