Clean up server rendered cursor logic
Variables were reused a bit too heavily and it was possible to get
the logic at a point where the server would try to render a cursor
where it wasn't needed, and the empty update rect would cause a
crash. Clear things up by introducing some more explicit variables.
diff --git a/common/rfb/VNCSConnectionST.cxx b/common/rfb/VNCSConnectionST.cxx
index 932abc0..f45ee7e 100644
--- a/common/rfb/VNCSConnectionST.cxx
+++ b/common/rfb/VNCSConnectionST.cxx
@@ -74,7 +74,7 @@
minRTT(-1), seenCongestion(false),
pingCounter(0), congestionTimer(this),
server(server_), updates(false),
- drawRenderedCursor(false), removeRenderedCursor(false),
+ updateRenderedCursor(false), removeRenderedCursor(false),
continuousUpdates(false), encodeManager(this),
updateTimer(this), pointerEventTime(0),
accessRights(AccessDefault), startTime(time(0))
@@ -198,7 +198,7 @@
// We need to clip the next update to the new size, but also add any
// extra bits if it's bigger. If we wanted to do this exactly, something
// like the code below would do it, but at the moment we just update the
- // entire new size. However, we do need to clip the renderedCursorRect
+ // entire new size. However, we do need to clip the damagedCursorRegion
// because that might be added to updates in writeFramebufferUpdate().
//updates.intersect(server->pb->getRect());
@@ -210,7 +210,7 @@
// updates.add_changed(Rect(0, cp.height, cp.width,
// server->pb->height()));
- renderedCursorRect = renderedCursorRect.intersect(server->pb->getRect());
+ damagedCursorRegion.assign_intersect(server->pb->getRect());
cp.width = server->pb->width();
cp.height = server->pb->height();
@@ -341,10 +341,10 @@
void VNCSConnectionST::renderedCursorChange()
{
if (state() != RFBSTATE_NORMAL) return;
- if (!renderedCursorRect.is_empty())
+ if (!damagedCursorRegion.is_empty())
removeRenderedCursor = true;
if (needRenderedCursor()) {
- drawRenderedCursor = true;
+ updateRenderedCursor = true;
writeFramebufferUpdateOrClose();
}
}
@@ -688,9 +688,8 @@
void VNCSConnectionST::supportsLocalCursor()
{
if (cp.supportsLocalCursor || cp.supportsLocalXCursor) {
- if (!renderedCursorRect.is_empty())
+ if (!damagedCursorRegion.is_empty())
removeRenderedCursor = true;
- drawRenderedCursor = false;
setCursor();
}
}
@@ -910,6 +909,7 @@
Region req;
UpdateInfo ui;
bool needNewUpdateInfo;
+ bool drawRenderedCursor;
updateTimer.stop();
@@ -975,28 +975,31 @@
// copy, then when the copy happens the corresponding rectangle in the
// destination will be wrong, so add it to the changed region.
- if (!ui.copied.is_empty() && !renderedCursorRect.is_empty()) {
- Rect bogusCopiedCursor = (renderedCursorRect.translate(ui.copy_delta)
- .intersect(server->pb->getRect()));
+ if (!ui.copied.is_empty() && !damagedCursorRegion.is_empty()) {
+ Region bogusCopiedCursor;
+
+ bogusCopiedCursor.copyFrom(damagedCursorRegion);
+ bogusCopiedCursor.translate(ui.copy_delta);
+ bogusCopiedCursor.assign_intersect(server->pb->getRect());
if (!ui.copied.intersect(bogusCopiedCursor).is_empty()) {
updates.add_changed(bogusCopiedCursor);
needNewUpdateInfo = true;
}
}
- // If we need to remove the old rendered cursor, just add the rectangle to
+ // If we need to remove the old rendered cursor, just add the region to
// the changed region.
if (removeRenderedCursor) {
- updates.add_changed(renderedCursorRect);
+ updates.add_changed(damagedCursorRegion);
needNewUpdateInfo = true;
- renderedCursorRect.clear();
+ damagedCursorRegion.clear();
removeRenderedCursor = false;
}
// Return if there is nothing to send the client.
- if (updates.is_empty() && !writer()->needFakeUpdate() && !drawRenderedCursor)
+ if (updates.is_empty() && !writer()->needFakeUpdate() && !updateRenderedCursor)
goto out;
// The `updates' object could change, make sure we have valid update info.
@@ -1009,13 +1012,18 @@
// with the update region, we need to draw the rendered cursor regardless of
// whether it has changed.
+ drawRenderedCursor = false;
if (needRenderedCursor()) {
+ Rect renderedCursorRect;
+
renderedCursorRect
= server->renderedCursor.getEffectiveRect()
.intersect(req.get_bounding_rect());
if (renderedCursorRect.is_empty()) {
drawRenderedCursor = false;
+ } else if (updateRenderedCursor) {
+ drawRenderedCursor = true;
} else if (!ui.changed.union_(ui.copied)
.intersect(renderedCursorRect).is_empty()) {
drawRenderedCursor = true;
@@ -1030,6 +1038,9 @@
// updates.subtract(renderedCursorRect);
// updates.getUpdateInfo(&ui, req);
//}
+
+ damagedCursorRegion.assign_union(renderedCursorRect);
+ updateRenderedCursor = false;
}
if (!ui.is_empty() || writer()->needFakeUpdate() || drawRenderedCursor) {
@@ -1045,7 +1056,6 @@
writeRTTPing();
- drawRenderedCursor = false;
requested.clear();
updates.clear();
}
diff --git a/common/rfb/VNCSConnectionST.h b/common/rfb/VNCSConnectionST.h
index 978879f..bc6ae5b 100644
--- a/common/rfb/VNCSConnectionST.h
+++ b/common/rfb/VNCSConnectionST.h
@@ -196,8 +196,8 @@
VNCServerST* server;
SimpleUpdateTracker updates;
Region requested;
- bool drawRenderedCursor, removeRenderedCursor;
- Rect renderedCursorRect;
+ bool updateRenderedCursor, removeRenderedCursor;
+ Region damagedCursorRegion;
bool continuousUpdates;
Region cuRegion;
EncodeManager encodeManager;