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.

4 files changed, 148 insertions, 158 deletions

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;