From 8c3bd691885bef673faf3599ea663d847a17b78f Mon Sep 17 00:00:00 2001
From: Pierre Ossman <ossman@cendio.se>
Date: Thu, 22 Mar 2018 15:58:54 +0100
Subject: [PATCH] Respect request area for cursor rendering

We cannot send updates for a cursor that is outside the requested
region, so make sure we track things properly. This also has the nice
side effect of just re-sending the bits needed when the cursor overlaps
a changed part of the framebuffer.
---
 common/rfb/EncodeManager.cxx    | 31 +++++++++++-----------
 common/rfb/VNCSConnectionST.cxx | 47 ++++++++++++++++-----------------
 2 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/common/rfb/EncodeManager.cxx b/common/rfb/EncodeManager.cxx
index 0cd52060..4f6ad5a2 100644
--- a/common/rfb/EncodeManager.cxx
+++ b/common/rfb/EncodeManager.cxx
@@ -1,6 +1,6 @@
 /* Copyright (C) 2000-2003 Constantin Kaplinsky.  All Rights Reserved.
  * Copyright (C) 2011 D. R. Commander.  All Rights Reserved.
- * Copyright 2014 Pierre Ossman for Cendio AB
+ * Copyright 2014-2018 Pierre Ossman for Cendio AB
  * 
  * This is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -249,20 +249,29 @@ void EncodeManager::writeUpdate(const UpdateInfo& ui, const PixelBuffer* pb,
                                 const RenderedCursor* renderedCursor)
 {
     int nRects;
-    Region changed;
+    Region changed, cursorRegion;
 
     updates++;
 
     prepareEncoders();
 
+    changed.copyFrom(ui.changed);
+
+    /*
+     * We need to render the cursor seperately as it has its own
+     * magical pixel buffer, so split it out from the changed region.
+     */
+    if (renderedCursor != NULL) {
+      cursorRegion = changed.intersect(renderedCursor->getEffectiveRect());
+      changed.assign_subtract(renderedCursor->getEffectiveRect());
+    }
+
     if (conn->cp.supportsLastRect)
       nRects = 0xFFFF;
     else {
       nRects = ui.copied.numRects();
-      nRects += computeNumRects(ui.changed);
-
-      if (renderedCursor != NULL)
-        nRects += 1;
+      nRects += computeNumRects(changed);
+      nRects += computeNumRects(cursorRegion);
     }
 
     conn->writer()->writeFramebufferUpdateStart(nRects);
@@ -273,19 +282,11 @@ void EncodeManager::writeUpdate(const UpdateInfo& ui, const PixelBuffer* pb,
      * We start by searching for solid rects, which are then removed
      * from the changed region.
      */
-    changed.copyFrom(ui.changed);
-
     if (conn->cp.supportsLastRect)
       writeSolidRects(&changed, pb);
 
     writeRects(changed, pb);
-
-    if (renderedCursor != NULL) {
-      Rect renderedCursorRect;
-
-      renderedCursorRect = renderedCursor->getEffectiveRect();
-      writeSubRect(renderedCursorRect, renderedCursor);
-    }
+    writeRects(cursorRegion, renderedCursor);
 
     conn->writer()->writeFramebufferUpdateEnd();
 }
diff --git a/common/rfb/VNCSConnectionST.cxx b/common/rfb/VNCSConnectionST.cxx
index e707e492..924f724b 100644
--- a/common/rfb/VNCSConnectionST.cxx
+++ b/common/rfb/VNCSConnectionST.cxx
@@ -1,5 +1,5 @@
 /* Copyright (C) 2002-2005 RealVNC Ltd.  All Rights Reserved.
- * Copyright 2009-2016 Pierre Ossman for Cendio AB
+ * Copyright 2009-2018 Pierre Ossman for Cendio AB
  * 
  * This is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -1015,9 +1015,18 @@ void VNCSConnectionST::writeDataUpdate()
     removeRenderedCursor = false;
   }
 
+  // If we need a full cursor update then make sure its entire region
+  // is marked as changed.
+
+  if (updateRenderedCursor) {
+    updates.add_changed(server->getRenderedCursor()->getEffectiveRect());
+    needNewUpdateInfo = true;
+    updateRenderedCursor = false;
+  }
+
   // Return if there is nothing to send the client.
 
-  if (updates.is_empty() && !writer()->needFakeUpdate() && !updateRenderedCursor)
+  if (updates.is_empty() && !writer()->needFakeUpdate())
     return;
 
   // The `updates' object could change, make sure we have valid update info.
@@ -1025,38 +1034,28 @@ void VNCSConnectionST::writeDataUpdate()
   if (needNewUpdateInfo)
     updates.getUpdateInfo(&ui, req);
 
-  // If the client needs a server-side rendered cursor, work out the cursor
-  // rectangle.  If it's empty then don't bother drawing it, but if it overlaps
-  // with the update region, we need to draw the rendered cursor regardless of
-  // whether it has changed.
+  // Does the client need a server-side rendered cursor?
 
   cursor = NULL;
   if (needRenderedCursor()) {
     Rect renderedCursorRect;
 
     cursor = server->getRenderedCursor();
-
-    renderedCursorRect
-      = cursor->getEffectiveRect().intersect(req.get_bounding_rect());
-
-    if (renderedCursorRect.is_empty()) {
-      cursor = NULL;
-    } else if (!updateRenderedCursor &&
-               ui.changed.union_(ui.copied)
-               .intersect(renderedCursorRect).is_empty()) {
-      cursor = NULL;
+    renderedCursorRect = cursor->getEffectiveRect();
+
+    // Check that we don't try to copy over the cursor area, and
+    // if that happens we need to treat it as changed so that we can
+    // re-render it
+    if (!ui.copied.intersect(renderedCursorRect).is_empty()) {
+      ui.changed.assign_union(ui.copied.intersect(renderedCursorRect));
+      ui.copied.assign_subtract(renderedCursorRect);
     }
 
-    if (cursor) {
-      updates.subtract(renderedCursorRect);
-      updates.getUpdateInfo(&ui, req);
-    }
-
-    damagedCursorRegion.assign_union(renderedCursorRect);
-    updateRenderedCursor = false;
+    // Track where we've rendered the cursor
+    damagedCursorRegion.assign_union(ui.changed.intersect(renderedCursorRect));
   }
 
-  if (ui.is_empty() && !writer()->needFakeUpdate() && !cursor)
+  if (ui.is_empty() && !writer()->needFakeUpdate())
     return;
 
   writeRTTPing();
-- 
2.39.5