From 2daba9b2049a9943eaed3dca1c36567ba1a84860 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Mon, 29 Oct 2018 10:03:37 +0100 Subject: [PATCH] Abstract sending cursor and resizing the desktop Avoid having the callers need to know about the different variants of these functions and instead have the writer pick the most appropriate extension. --- common/rfb/ClientParams.cxx | 9 ++ common/rfb/ClientParams.h | 1 + common/rfb/SMsgWriter.cxx | 193 +++++++++++--------------------- common/rfb/SMsgWriter.h | 19 +--- common/rfb/VNCSConnectionST.cxx | 29 ++--- 5 files changed, 93 insertions(+), 158 deletions(-) diff --git a/common/rfb/ClientParams.cxx b/common/rfb/ClientParams.cxx index 78384073..2f8783bb 100644 --- a/common/rfb/ClientParams.cxx +++ b/common/rfb/ClientParams.cxx @@ -147,6 +147,15 @@ bool ClientParams::supportsLocalCursor() const return false; } +bool ClientParams::supportsDesktopSize() const +{ + if (supportsEncoding(pseudoEncodingExtendedDesktopSize)) + return true; + if (supportsEncoding(pseudoEncodingDesktopSize)) + return true; + return false; +} + bool ClientParams::supportsLEDState() const { if (supportsEncoding(pseudoEncodingLEDState)) diff --git a/common/rfb/ClientParams.h b/common/rfb/ClientParams.h index 63d4e192..f7a7044b 100644 --- a/common/rfb/ClientParams.h +++ b/common/rfb/ClientParams.h @@ -87,6 +87,7 @@ namespace rfb { // Wrappers to check for functionality rather than specific // encodings bool supportsLocalCursor() const; + bool supportsDesktopSize() const; bool supportsLEDState() const; bool supportsFence() const; bool supportsContinuousUpdates() const; diff --git a/common/rfb/SMsgWriter.cxx b/common/rfb/SMsgWriter.cxx index 0ca19812..dacba8a5 100644 --- a/common/rfb/SMsgWriter.cxx +++ b/common/rfb/SMsgWriter.cxx @@ -36,9 +36,7 @@ static LogWriter vlog("SMsgWriter"); SMsgWriter::SMsgWriter(ClientParams* client_, rdr::OutStream* os_) : client(client_), os(os_), nRectsInUpdate(0), nRectsInHeader(0), - needSetDesktopSize(false), needExtendedDesktopSize(false), - needSetDesktopName(false), needSetCursor(false), - needSetXCursor(false), needSetCursorWithAlpha(false), + needSetDesktopName(false), needCursor(false), needLEDState(false), needQEMUKeyEvent(false) { } @@ -120,36 +118,18 @@ void SMsgWriter::writeEndOfContinuousUpdates() endMsg(); } -bool SMsgWriter::writeSetDesktopSize() { - if (!client->supportsEncoding(pseudoEncodingDesktopSize)) - return false; - - needSetDesktopSize = true; - - return true; -} - -bool SMsgWriter::writeExtendedDesktopSize() { - if (!client->supportsEncoding(pseudoEncodingExtendedDesktopSize)) - return false; - - needExtendedDesktopSize = true; - - return true; -} - -bool SMsgWriter::writeExtendedDesktopSize(rdr::U16 reason, rdr::U16 result) { +void SMsgWriter::writeDesktopSize(rdr::U16 reason, rdr::U16 result) +{ ExtendedDesktopSizeMsg msg; - if (!client->supportsEncoding(pseudoEncodingExtendedDesktopSize)) - return false; + if (!client->supportsEncoding(pseudoEncodingDesktopSize) && + !client->supportsEncoding(pseudoEncodingExtendedDesktopSize)) + throw Exception("Client does not support desktop size changes"); msg.reason = reason; msg.result = result; extendedDesktopSizeMsgs.push_back(msg); - - return true; } bool SMsgWriter::writeSetDesktopName() { @@ -161,34 +141,14 @@ bool SMsgWriter::writeSetDesktopName() { return true; } -bool SMsgWriter::writeSetCursor() -{ - if (!client->supportsEncoding(pseudoEncodingCursor)) - return false; - - needSetCursor = true; - - return true; -} - -bool SMsgWriter::writeSetXCursor() -{ - if (!client->supportsEncoding(pseudoEncodingXCursor)) - return false; - - needSetXCursor = true; - - return true; -} - -bool SMsgWriter::writeSetCursorWithAlpha() +void SMsgWriter::writeCursor() { - if (!client->supportsEncoding(pseudoEncodingCursorWithAlpha)) - return false; + if (!client->supportsEncoding(pseudoEncodingCursor) && + !client->supportsEncoding(pseudoEncodingXCursor) && + !client->supportsEncoding(pseudoEncodingCursorWithAlpha)) + throw Exception("Client does not support local cursor"); - needSetCursorWithAlpha = true; - - return true; + needCursor = true; } bool SMsgWriter::writeLEDState() @@ -217,7 +177,7 @@ bool SMsgWriter::needFakeUpdate() { if (needSetDesktopName) return true; - if (needSetCursor || needSetXCursor || needSetCursorWithAlpha) + if (needCursor) return true; if (needLEDState) return true; @@ -231,9 +191,7 @@ bool SMsgWriter::needFakeUpdate() bool SMsgWriter::needNoDataUpdate() { - if (needSetDesktopSize) - return true; - if (needExtendedDesktopSize || !extendedDesktopSizeMsgs.empty()) + if (!extendedDesktopSizeMsgs.empty()) return true; return false; @@ -245,12 +203,12 @@ void SMsgWriter::writeNoDataUpdate() nRects = 0; - if (needSetDesktopSize) - nRects++; - if (needExtendedDesktopSize) - nRects++; - if (!extendedDesktopSizeMsgs.empty()) - nRects += extendedDesktopSizeMsgs.size(); + if (!extendedDesktopSizeMsgs.empty()) { + if (client->supportsEncoding(pseudoEncodingExtendedDesktopSize)) + nRects += extendedDesktopSizeMsgs.size(); + else + nRects++; + } writeFramebufferUpdateStart(nRects); writeNoDataRects(); @@ -265,11 +223,7 @@ void SMsgWriter::writeFramebufferUpdateStart(int nRects) if (nRects != 0xFFFF) { if (needSetDesktopName) nRects++; - if (needSetCursor) - nRects++; - if (needSetXCursor) - nRects++; - if (needSetCursorWithAlpha) + if (needCursor) nRects++; if (needLEDState) nRects++; @@ -343,47 +297,43 @@ void SMsgWriter::endMsg() void SMsgWriter::writePseudoRects() { - if (needSetCursor) { + if (needCursor) { const Cursor& cursor = client->cursor(); - rdr::U8Array data(cursor.width()*cursor.height() * (client->pf().bpp/8)); - rdr::U8Array mask(cursor.getMask()); - - const rdr::U8* in; - rdr::U8* out; - - in = cursor.getBuffer(); - out = data.buf; - for (int i = 0;i < cursor.width()*cursor.height();i++) { - client->pf().bufferFromRGB(out, in, 1); - in += 4; - out += client->pf().bpp/8; + if (client->supportsEncoding(pseudoEncodingCursorWithAlpha)) { + writeSetCursorWithAlphaRect(cursor.width(), cursor.height(), + cursor.hotspot().x, cursor.hotspot().y, + cursor.getBuffer()); + } else if (client->supportsEncoding(pseudoEncodingCursor)) { + rdr::U8Array data(cursor.width()*cursor.height() * (client->pf().bpp/8)); + rdr::U8Array mask(cursor.getMask()); + + const rdr::U8* in; + rdr::U8* out; + + in = cursor.getBuffer(); + out = data.buf; + for (int i = 0;i < cursor.width()*cursor.height();i++) { + client->pf().bufferFromRGB(out, in, 1); + in += 4; + out += client->pf().bpp/8; + } + + writeSetCursorRect(cursor.width(), cursor.height(), + cursor.hotspot().x, cursor.hotspot().y, + data.buf, mask.buf); + } else if (client->supportsEncoding(pseudoEncodingXCursor)) { + rdr::U8Array bitmap(cursor.getBitmap()); + rdr::U8Array mask(cursor.getMask()); + + writeSetXCursorRect(cursor.width(), cursor.height(), + cursor.hotspot().x, cursor.hotspot().y, + bitmap.buf, mask.buf); + } else { + throw Exception("Client does not support local cursor"); } - writeSetCursorRect(cursor.width(), cursor.height(), - cursor.hotspot().x, cursor.hotspot().y, - data.buf, mask.buf); - needSetCursor = false; - } - - if (needSetXCursor) { - const Cursor& cursor = client->cursor(); - rdr::U8Array bitmap(cursor.getBitmap()); - rdr::U8Array mask(cursor.getMask()); - - writeSetXCursorRect(cursor.width(), cursor.height(), - cursor.hotspot().x, cursor.hotspot().y, - bitmap.buf, mask.buf); - needSetXCursor = false; - } - - if (needSetCursorWithAlpha) { - const Cursor& cursor = client->cursor(); - - writeSetCursorWithAlphaRect(cursor.width(), cursor.height(), - cursor.hotspot().x, cursor.hotspot().y, - cursor.getBuffer()); - needSetCursorWithAlpha = false; + needCursor = false; } if (needSetDesktopName) { @@ -404,32 +354,25 @@ void SMsgWriter::writePseudoRects() void SMsgWriter::writeNoDataRects() { - // Start with specific ExtendedDesktopSize messages if (!extendedDesktopSizeMsgs.empty()) { - std::list::const_iterator ri; - - for (ri = extendedDesktopSizeMsgs.begin();ri != extendedDesktopSizeMsgs.end();++ri) { - writeExtendedDesktopSizeRect(ri->reason, ri->result, - client->width(), client->height(), - client->screenLayout()); + if (client->supportsEncoding(pseudoEncodingExtendedDesktopSize)) { + std::list::const_iterator ri; + for (ri = extendedDesktopSizeMsgs.begin();ri != extendedDesktopSizeMsgs.end();++ri) { + // FIXME: We can probably skip multiple reasonServer entries + writeExtendedDesktopSizeRect(ri->reason, ri->result, + client->width(), client->height(), + client->screenLayout()); + } + } else if (client->supportsEncoding(pseudoEncodingDesktopSize)) { + // Some clients assume this is the last rectangle so don't send anything + // more after this + writeSetDesktopSizeRect(client->width(), client->height()); + } else { + throw Exception("Client does not support desktop size changes"); } extendedDesktopSizeMsgs.clear(); } - - // Send this before SetDesktopSize to make life easier on the clients - if (needExtendedDesktopSize) { - writeExtendedDesktopSizeRect(0, 0, client->width(), client->height(), - client->screenLayout()); - needExtendedDesktopSize = false; - } - - // Some clients assume this is the last rectangle so don't send anything - // more after this - if (needSetDesktopSize) { - writeSetDesktopSizeRect(client->width(), client->height()); - needSetDesktopSize = false; - } } void SMsgWriter::writeSetDesktopSizeRect(int width, int height) diff --git a/common/rfb/SMsgWriter.h b/common/rfb/SMsgWriter.h index 1fe50434..19b013f6 100644 --- a/common/rfb/SMsgWriter.h +++ b/common/rfb/SMsgWriter.h @@ -65,22 +65,15 @@ namespace rfb { // updates mode. void writeEndOfContinuousUpdates(); - // writeSetDesktopSize() won't actually write immediately, but will + // writeDesktopSize() won't actually write immediately, but will // write the relevant pseudo-rectangle as part of the next update. - bool writeSetDesktopSize(); - // Same thing for the extended version. The first version queues up a - // generic update of the current server state, but the second queues a - // specific message. - bool writeExtendedDesktopSize(); - bool writeExtendedDesktopSize(rdr::U16 reason, rdr::U16 result); + void writeDesktopSize(rdr::U16 reason, rdr::U16 result=0); bool writeSetDesktopName(); // Like setDesktopSize, we can't just write out a cursor message // immediately. - bool writeSetCursor(); - bool writeSetXCursor(); - bool writeSetCursorWithAlpha(); + void writeCursor(); // Same for LED state message bool writeLEDState(); @@ -146,12 +139,8 @@ namespace rfb { int nRectsInUpdate; int nRectsInHeader; - bool needSetDesktopSize; - bool needExtendedDesktopSize; bool needSetDesktopName; - bool needSetCursor; - bool needSetXCursor; - bool needSetCursorWithAlpha; + bool needCursor; bool needLEDState; bool needQEMUKeyEvent; diff --git a/common/rfb/VNCSConnectionST.cxx b/common/rfb/VNCSConnectionST.cxx index e4e5ab3e..d936573d 100644 --- a/common/rfb/VNCSConnectionST.cxx +++ b/common/rfb/VNCSConnectionST.cxx @@ -216,13 +216,11 @@ void VNCSConnectionST::pixelBufferChange() client.setDimensions(server->pb->width(), server->pb->height(), server->screenLayout); if (state() == RFBSTATE_NORMAL) { - // We should only send EDS to client asking for both - if (!writer()->writeExtendedDesktopSize()) { - if (!writer()->writeSetDesktopSize()) { - close("Client does not support desktop resize"); - return; - } + if (!client.supportsDesktopSize()) { + close("Client does not support desktop resize"); + return; } + writer()->writeDesktopSize(reasonServer); } // Drop any lossy tracking that is now outside the framebuffer @@ -697,7 +695,8 @@ void VNCSConnectionST::framebufferUpdateRequest(const Rect& r,bool incremental) // And send the screen layout to the client (which, unlike the // framebuffer dimensions, the client doesn't get during init) - writer()->writeExtendedDesktopSize(); + if (client.supportsEncoding(pseudoEncodingExtendedDesktopSize)) + writer()->writeDesktopSize(reasonServer); // We do not send a DesktopSize since it only contains the // framebuffer size (which the client already should know) and @@ -716,7 +715,7 @@ void VNCSConnectionST::setDesktopSize(int fb_width, int fb_height, // Don't bother the desktop with an invalid configuration if (!layout.validate(fb_width, fb_height)) { - writer()->writeExtendedDesktopSize(reasonClient, resultInvalid); + writer()->writeDesktopSize(reasonClient, resultInvalid); return; } @@ -725,7 +724,7 @@ void VNCSConnectionST::setDesktopSize(int fb_width, int fb_height, // protocol-wise, but unnecessary. result = server->desktop->setScreenLayout(fb_width, fb_height, layout); - writer()->writeExtendedDesktopSize(reasonClient, result); + writer()->writeDesktopSize(reasonClient, result); // Only notify other clients on success if (result == resultSuccess) { @@ -1125,7 +1124,7 @@ void VNCSConnectionST::screenLayoutChange(rdr::U16 reason) if (state() != RFBSTATE_NORMAL) return; - writer()->writeExtendedDesktopSize(reason, 0); + writer()->writeDesktopSize(reason); } @@ -1147,14 +1146,8 @@ void VNCSConnectionST::setCursor() clientHasCursor = true; } - if (!writer()->writeSetCursorWithAlpha()) { - if (!writer()->writeSetCursor()) { - if (!writer()->writeSetXCursor()) { - // No client support - return; - } - } - } + if (client.supportsLocalCursor()) + writer()->writeCursor(); } void VNCSConnectionST::setDesktopName(const char *name) -- 2.39.5