From b9442affc0eb877603766452601d9db8fd4ef79a Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 29 Feb 2024 09:23:01 +0100 Subject: Detect conflicting timer rescheduling Repeating a timer can be done in two ways: * Returning true from the handler * Calling start() again in the handler The latter is useful if you want to change the timer interval. If both are used, then it becomes ambiguous when the timer should fire again. Detect this case and warn about it. Current implementation will respect the new interval given to start(), rather than the interval set before running the handler. --- common/rfb/Timer.cxx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/common/rfb/Timer.cxx b/common/rfb/Timer.cxx index 4ff15bc5..b43490d7 100644 --- a/common/rfb/Timer.cxx +++ b/common/rfb/Timer.cxx @@ -1,5 +1,5 @@ /* Copyright (C) 2002-2005 RealVNC Ltd. All Rights Reserved. - * Copyright 2016-2018 Pierre Ossman for Cendio AB + * Copyright 2016-2024 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 @@ -66,15 +66,21 @@ int Timer::checkTimeouts() { gettimeofday(&start, 0); while (pending.front()->isBefore(start)) { Timer* timer; - timeval before; + timeval before, dueTime; timer = pending.front(); pending.pop_front(); + dueTime = timer->dueTime; gettimeofday(&before, 0); if (timer->cb->handleTimeout(timer)) { timeval now; + if (msBetween(&dueTime, &timer->dueTime) != 0) { + vlog.error("Timer incorrectly modified whilst repeating"); + timer->dueTime = dueTime; + } + gettimeofday(&now, 0); timer->dueTime = addMillis(timer->dueTime, timer->timeoutMs); -- cgit v1.2.3 From 37cf0ffaba7f4e855a909f57adf72da43c0ac275 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 3 Jan 2023 09:20:45 +0100 Subject: Remove unneeded iteration This should have been done in a4308c9. --- win/rfb_win32/SocketManager.cxx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/win/rfb_win32/SocketManager.cxx b/win/rfb_win32/SocketManager.cxx index b7cc1cce..027536fd 100644 --- a/win/rfb_win32/SocketManager.cxx +++ b/win/rfb_win32/SocketManager.cxx @@ -165,9 +165,7 @@ void SocketManager::setDisable(network::SocketServer* srvr, bool disable) int SocketManager::checkTimeouts() { int timeout = EventManager::checkTimeouts(); - std::map::iterator i; - for (i=listeners.begin(); i!=listeners.end(); i++) - soonestTimeout(&timeout, Timer::checkTimeouts()); + soonestTimeout(&timeout, Timer::checkTimeouts()); std::list shutdownSocks; std::map::iterator j, j_next; -- cgit v1.2.3 From bf286837db638a67bde0ab9be0baa621f863b8d5 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 3 Jan 2023 09:24:51 +0100 Subject: Stop treating "0" as "no timeouts" It is much more sane to treat "0" as "a timer is ready NOW", so let's change to using -1 as the invalid timeout value. --- common/rfb/Timer.cxx | 11 ++++------- common/rfb/util.h | 7 ------- unix/vncconfig/vncconfig.cxx | 2 +- unix/x0vncserver/x0vncserver.cxx | 7 +++++-- unix/xserver/hw/vnc/XserverDesktop.cc | 2 +- vncviewer/vncviewer.cxx | 2 +- win/rfb_win32/EventManager.cxx | 6 +++--- win/rfb_win32/SocketManager.cxx | 4 +++- 8 files changed, 18 insertions(+), 23 deletions(-) diff --git a/common/rfb/Timer.cxx b/common/rfb/Timer.cxx index b43490d7..d1a6373b 100644 --- a/common/rfb/Timer.cxx +++ b/common/rfb/Timer.cxx @@ -61,7 +61,7 @@ int Timer::checkTimeouts() { timeval start; if (pending.empty()) - return 0; + return -1; gettimeofday(&start, 0); while (pending.front()->isBefore(start)) { @@ -95,7 +95,7 @@ int Timer::checkTimeouts() { insertTimer(timer); } else if (pending.empty()) { - return 0; + return -1; } } return getNextTimeout(); @@ -104,7 +104,7 @@ int Timer::checkTimeouts() { int Timer::getNextTimeout() { timeval now; gettimeofday(&now, 0); - int toWait = __rfbmax(1, pending.front()->getRemainingMs()); + int toWait = pending.front()->getRemainingMs(); if (toWait > pending.front()->timeoutMs) { if (toWait - pending.front()->timeoutMs < 1000) { vlog.info("gettimeofday is broken..."); @@ -113,7 +113,7 @@ int Timer::getNextTimeout() { // Time has jumped backwards! vlog.info("time has moved backwards!"); pending.front()->dueTime = now; - toWait = 1; + toWait = 0; } return toWait; } @@ -134,9 +134,6 @@ void Timer::start(int timeoutMs_) { gettimeofday(&now, 0); stop(); timeoutMs = timeoutMs_; - // The rest of the code assumes non-zero timeout - if (timeoutMs <= 0) - timeoutMs = 1; dueTime = addMillis(now, timeoutMs); insertTimer(this); } diff --git a/common/rfb/util.h b/common/rfb/util.h index cafea209..b47ac4c9 100644 --- a/common/rfb/util.h +++ b/common/rfb/util.h @@ -73,13 +73,6 @@ namespace rfb { // HELPER functions for timeout handling - // soonestTimeout() is a function to help work out the soonest of several - // timeouts. - inline void soonestTimeout(int* timeout, int newTimeout) { - if (newTimeout && (!*timeout || newTimeout < *timeout)) - *timeout = newTimeout; - } - // secsToMillis() turns seconds into milliseconds, capping the value so it // can't wrap round and become -ve inline int secsToMillis(int secs) { diff --git a/unix/vncconfig/vncconfig.cxx b/unix/vncconfig/vncconfig.cxx index f39e9934..be8c8195 100644 --- a/unix/vncconfig/vncconfig.cxx +++ b/unix/vncconfig/vncconfig.cxx @@ -308,7 +308,7 @@ int main(int argc, char** argv) // Process expired timers and get the time until the next one int timeoutMs = Timer::checkTimeouts(); - if (timeoutMs) { + if (timeoutMs >= 0) { tv.tv_sec = timeoutMs / 1000; tv.tv_usec = (timeoutMs % 1000) * 1000; tvp = &tv; diff --git a/unix/x0vncserver/x0vncserver.cxx b/unix/x0vncserver/x0vncserver.cxx index 8e27e62b..ffaf5788 100644 --- a/unix/x0vncserver/x0vncserver.cxx +++ b/unix/x0vncserver/x0vncserver.cxx @@ -382,7 +382,7 @@ int main(int argc, char** argv) PollingScheduler sched((int)pollingCycle, (int)maxProcessorUsage); while (!caughtSignal) { - int wait_ms; + int wait_ms, nextTimeout; struct timeval tv; fd_set rfds, wfds; std::list sockets; @@ -426,7 +426,10 @@ int main(int argc, char** argv) } } - soonestTimeout(&wait_ms, Timer::checkTimeouts()); + // Trigger timers and check when the next will expire + nextTimeout = Timer::checkTimeouts(); + if (nextTimeout >= 0 && nextTimeout < wait_ms) + wait_ms = nextTimeout; tv.tv_sec = wait_ms / 1000; tv.tv_usec = (wait_ms % 1000) * 1000; diff --git a/unix/xserver/hw/vnc/XserverDesktop.cc b/unix/xserver/hw/vnc/XserverDesktop.cc index b5a58671..64b2e614 100644 --- a/unix/xserver/hw/vnc/XserverDesktop.cc +++ b/unix/xserver/hw/vnc/XserverDesktop.cc @@ -395,7 +395,7 @@ void XserverDesktop::blockHandler(int* timeout) // Trigger timers and check when the next will expire int nextTimeout = Timer::checkTimeouts(); - if (nextTimeout > 0 && (*timeout == -1 || nextTimeout < *timeout)) + if (nextTimeout >= 0 && (*timeout == -1 || nextTimeout < *timeout)) *timeout = nextTimeout; } catch (rdr::Exception& e) { vlog.error("XserverDesktop::blockHandler: %s",e.str()); diff --git a/vncviewer/vncviewer.cxx b/vncviewer/vncviewer.cxx index 6eebf3d0..a5604f76 100644 --- a/vncviewer/vncviewer.cxx +++ b/vncviewer/vncviewer.cxx @@ -189,7 +189,7 @@ static void mainloop(const char* vncserver, network::Socket* sock) int next_timer; next_timer = Timer::checkTimeouts(); - if (next_timer == 0) + if (next_timer < 0) next_timer = INT_MAX; if (Fl::wait((double)next_timer / 1000.0) < 0.0) { diff --git a/win/rfb_win32/EventManager.cxx b/win/rfb_win32/EventManager.cxx index 0f33f5ac..f034d36d 100644 --- a/win/rfb_win32/EventManager.cxx +++ b/win/rfb_win32/EventManager.cxx @@ -64,14 +64,14 @@ void EventManager::removeEvent(HANDLE event) { int EventManager::checkTimeouts() { - return 0; + return -1; } BOOL EventManager::getMessage(MSG* msg, HWND hwnd, UINT minMsg, UINT maxMsg) { while (true) { // - Process any pending timeouts - DWORD timeout = checkTimeouts(); - if (timeout == 0) + int timeout = checkTimeouts(); + if (timeout < 0) timeout = INFINITE; // - Events take precedence over messages diff --git a/win/rfb_win32/SocketManager.cxx b/win/rfb_win32/SocketManager.cxx index 027536fd..c2fba0fb 100644 --- a/win/rfb_win32/SocketManager.cxx +++ b/win/rfb_win32/SocketManager.cxx @@ -165,7 +165,9 @@ void SocketManager::setDisable(network::SocketServer* srvr, bool disable) int SocketManager::checkTimeouts() { int timeout = EventManager::checkTimeouts(); - soonestTimeout(&timeout, Timer::checkTimeouts()); + int nextTimeout = Timer::checkTimeouts(); + if (nextTimeout >= 0 && nextTimeout < timeout) + timeout = nextTimeout; std::list shutdownSocks; std::map::iterator j, j_next; -- cgit v1.2.3 From 265c50f5af024d20191eb9d37ee21112593e3aa1 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 3 Jan 2023 09:27:46 +0100 Subject: Add safety check to getNextTimeout() It currently won't ever be called with an empty list of timers, but it is a public function so that might happen in the future. Make sure this case is handled without crashes. --- common/rfb/Timer.cxx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/common/rfb/Timer.cxx b/common/rfb/Timer.cxx index d1a6373b..dc17606e 100644 --- a/common/rfb/Timer.cxx +++ b/common/rfb/Timer.cxx @@ -104,7 +104,12 @@ int Timer::checkTimeouts() { int Timer::getNextTimeout() { timeval now; gettimeofday(&now, 0); + + if (pending.empty()) + return -1; + int toWait = pending.front()->getRemainingMs(); + if (toWait > pending.front()->timeoutMs) { if (toWait - pending.front()->timeoutMs < 1000) { vlog.info("gettimeofday is broken..."); @@ -115,6 +120,7 @@ int Timer::getNextTimeout() { pending.front()->dueTime = now; toWait = 0; } + return toWait; } -- cgit v1.2.3 From db68216c721763fb2f252d8f3c7c47e6be3998c6 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Fri, 18 Nov 2022 11:20:57 +0100 Subject: Fix up Timer comments They were badly formatted, way longer than the normal 72 columns. --- common/rfb/Timer.h | 61 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/common/rfb/Timer.h b/common/rfb/Timer.h index ddfce1b2..3059a9b3 100644 --- a/common/rfb/Timer.h +++ b/common/rfb/Timer.h @@ -27,25 +27,29 @@ namespace rfb { /* Timer - Cross-platform timeout handling. The caller creates instances of Timer and passes a - Callback implementation to each. The Callback will then be called with a pointer to - the Timer instance that timed-out when the timeout occurs. - - The static methods of Timer are used by the main loop of the application both to - dispatch elapsed Timer callbacks and to determine how long to wait in select() for - the next timeout to occur. - - For classes that can be derived it's best to use MethodTimer which can call a specific - method on the class, thus avoiding conflicts when subclassing. + Cross-platform timeout handling. The caller creates instances of + Timer and passes a Callback implementation to each. The Callback + will then be called with a pointer to the Timer instance that + timed-out when the timeout occurs. + + The static methods of Timer are used by the main loop of the + application both to dispatch elapsed Timer callbacks and to + determine how long to wait in select() for the next timeout to + occur. + + For classes that can be derived it's best to use MethodTimer which + can call a specific method on the class, thus avoiding conflicts + when subclassing. */ struct Timer { struct Callback { // handleTimeout - // Passed a pointer to the Timer that has timed out. If the handler returns true - // then the Timer is reset and left running, causing another timeout after the - // appropriate interval. + // Passed a pointer to the Timer that has timed out. If the + // handler returns true then the Timer is reset and left + // running, causing another timeout after the appropriate + // interval. // If the handler returns false then the Timer is cancelled. virtual bool handleTimeout(Timer* t) = 0; @@ -53,44 +57,46 @@ namespace rfb { }; // checkTimeouts() - // Dispatches any elapsed Timers, and returns the number of milliseconds until the - // next Timer will timeout. + // Dispatches any elapsed Timers, and returns the number of + // milliseconds until the next Timer will timeout. static int checkTimeouts(); // getNextTimeout() - // Returns the number of milliseconds until the next timeout, without dispatching - // any elapsed Timers. + // Returns the number of milliseconds until the next timeout, + // without dispatching any elapsed Timers. static int getNextTimeout(); // Create a Timer with the specified callback handler Timer(Callback* cb_) {cb = cb_;} ~Timer() {stop();} - // startTimer - // Starts the timer, causing a timeout after the specified number of milliseconds. - // If the timer is already active then it will be implicitly cancelled and re-started. + // start() + // Starts the timer, causing a timeout after the specified number + // of milliseconds. If the timer is already active then it will + // be implicitly cancelled and re-started. void start(int timeoutMs_); - // stopTimer + // stop() // Cancels the timer. void stop(); - // isStarted + // isStarted() // Determines whether the timer is started. bool isStarted(); - // getTimeoutMs + // getTimeoutMs() // Determines the previously used timeout value, if any. // Usually used with isStarted() to get the _current_ timeout. int getTimeoutMs(); - // getRemainingMs + // getRemainingMs() // Determines how many milliseconds are left before the Timer // will timeout. Only valid for an active timer. int getRemainingMs(); - // isBefore - // Determine whether the Timer will timeout before the specified time. + // isBefore() + // Determine whether the Timer will timeout before the specified + // time. bool isBefore(timeval other); protected: @@ -99,7 +105,8 @@ namespace rfb { Callback* cb; static void insertTimer(Timer* t); - // The list of currently active Timers, ordered by time left until timeout. + // The list of currently active Timers, ordered by time left until + // timeout. static std::list pending; }; -- cgit v1.2.3 From bc760d93b71070c7d65588686a494dcd5f228dc6 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Fri, 18 Nov 2022 16:31:41 +0100 Subject: Explicitly request timer repetition One-shot timers are more common, so let's change the API a bit to make that use case simpler. This API also makes it more clear what is happening. --- common/rfb/EncodeManager.cxx | 6 ++--- common/rfb/EncodeManager.h | 2 +- common/rfb/SConnection.cxx | 8 +++--- common/rfb/SConnection.h | 2 +- common/rfb/Timer.cxx | 51 ++++++++++++++++++----------------- common/rfb/Timer.h | 18 ++++++++----- common/rfb/VNCSConnectionST.cxx | 4 +-- common/rfb/VNCSConnectionST.h | 2 +- common/rfb/VNCServerST.cxx | 14 ++++------ common/rfb/VNCServerST.h | 2 +- unix/vncconfig/QueryConnectDialog.cxx | 5 ++-- unix/vncconfig/QueryConnectDialog.h | 2 +- unix/xserver/hw/vnc/XserverDesktop.cc | 4 +-- unix/xserver/hw/vnc/XserverDesktop.h | 2 +- vncviewer/EmulateMB.cxx | 6 ++--- vncviewer/EmulateMB.h | 2 +- vncviewer/GestureHandler.cxx | 4 +-- vncviewer/GestureHandler.h | 2 +- 18 files changed, 63 insertions(+), 73 deletions(-) diff --git a/common/rfb/EncodeManager.cxx b/common/rfb/EncodeManager.cxx index bc15e74a..c2658a70 100644 --- a/common/rfb/EncodeManager.cxx +++ b/common/rfb/EncodeManager.cxx @@ -297,7 +297,7 @@ void EncodeManager::writeLosslessRefresh(const Region& req, const PixelBuffer* p Region(), Point(), pb, renderedCursor); } -bool EncodeManager::handleTimeout(Timer* t) +void EncodeManager::handleTimeout(Timer* t) { if (t == &recentChangeTimer) { // Any lossy region that wasn't recently updated can @@ -307,10 +307,8 @@ bool EncodeManager::handleTimeout(Timer* t) // Will there be more to do? (i.e. do we need another round) if (!lossyRegion.subtract(pendingRefreshRegion).is_empty()) - return true; + t->repeat(); } - - return false; } void EncodeManager::doUpdate(bool allowLossy, const Region& changed_, diff --git a/common/rfb/EncodeManager.h b/common/rfb/EncodeManager.h index f2fd4ca4..33484db8 100644 --- a/common/rfb/EncodeManager.h +++ b/common/rfb/EncodeManager.h @@ -61,7 +61,7 @@ namespace rfb { size_t maxUpdateSize); protected: - virtual bool handleTimeout(Timer* t); + virtual void handleTimeout(Timer* t); void doUpdate(bool allowLossy, const Region& changed, const Region& copied, const Point& copy_delta, diff --git a/common/rfb/SConnection.cxx b/common/rfb/SConnection.cxx index 33b2d850..402b0c04 100644 --- a/common/rfb/SConnection.cxx +++ b/common/rfb/SConnection.cxx @@ -287,11 +287,11 @@ bool SConnection::processInitMsg() return reader_->readClientInit(); } -bool SConnection::handleAuthFailureTimeout(Timer* /*t*/) +void SConnection::handleAuthFailureTimeout(Timer* /*t*/) { if (state_ != RFBSTATE_SECURITY_FAILURE) { close("SConnection::handleAuthFailureTimeout: invalid state"); - return false; + return; } try { @@ -304,12 +304,10 @@ bool SConnection::handleAuthFailureTimeout(Timer* /*t*/) os->flush(); } catch (rdr::Exception& e) { close(e.str()); - return false; + return; } close(authFailureMsg.c_str()); - - return false; } void SConnection::throwConnFailedException(const char* format, ...) diff --git a/common/rfb/SConnection.h b/common/rfb/SConnection.h index b163d627..0bd6afdb 100644 --- a/common/rfb/SConnection.h +++ b/common/rfb/SConnection.h @@ -245,7 +245,7 @@ namespace rfb { bool processSecurityFailure(); bool processInitMsg(); - bool handleAuthFailureTimeout(Timer* t); + void handleAuthFailureTimeout(Timer* t); int defaultMajorVersion, defaultMinorVersion; diff --git a/common/rfb/Timer.cxx b/common/rfb/Timer.cxx index dc17606e..37aaad83 100644 --- a/common/rfb/Timer.cxx +++ b/common/rfb/Timer.cxx @@ -66,37 +66,15 @@ int Timer::checkTimeouts() { gettimeofday(&start, 0); while (pending.front()->isBefore(start)) { Timer* timer; - timeval before, dueTime; timer = pending.front(); pending.pop_front(); - dueTime = timer->dueTime; - gettimeofday(&before, 0); - if (timer->cb->handleTimeout(timer)) { - timeval now; + timer->lastDueTime = timer->dueTime; + timer->cb->handleTimeout(timer); - if (msBetween(&dueTime, &timer->dueTime) != 0) { - vlog.error("Timer incorrectly modified whilst repeating"); - timer->dueTime = dueTime; - } - - gettimeofday(&now, 0); - - timer->dueTime = addMillis(timer->dueTime, timer->timeoutMs); - if (timer->isBefore(now)) { - // Time has jumped forwards, or we're not getting enough - // CPU time for the timers - - timer->dueTime = addMillis(before, timer->timeoutMs); - if (timer->isBefore(now)) - timer->dueTime = now; - } - - insertTimer(timer); - } else if (pending.empty()) { + if (pending.empty()) return -1; - } } return getNextTimeout(); } @@ -144,6 +122,29 @@ void Timer::start(int timeoutMs_) { insertTimer(this); } +void Timer::repeat() { + timeval now; + + gettimeofday(&now, 0); + + if (isStarted()) { + vlog.error("Incorrectly repeating already running timer"); + stop(); + } + + if (msBetween(&lastDueTime, &dueTime) != 0) + vlog.error("Timer incorrectly modified whilst repeating"); + + dueTime = addMillis(lastDueTime, timeoutMs); + if (isBefore(now)) { + // Time has jumped forwards, or we're not getting enough + // CPU time for the timers + dueTime = now; + } + + insertTimer(this); +} + void Timer::stop() { pending.remove(this); } diff --git a/common/rfb/Timer.h b/common/rfb/Timer.h index 3059a9b3..9775afd5 100644 --- a/common/rfb/Timer.h +++ b/common/rfb/Timer.h @@ -1,5 +1,5 @@ /* Copyright (C) 2002-2005 RealVNC Ltd. All Rights Reserved. - * Copyright 2018 Pierre Ossman for Cendio AB + * Copyright 2018-2024 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 @@ -51,7 +51,7 @@ namespace rfb { // running, causing another timeout after the appropriate // interval. // If the handler returns false then the Timer is cancelled. - virtual bool handleTimeout(Timer* t) = 0; + virtual void handleTimeout(Timer* t) = 0; virtual ~Callback() {} }; @@ -76,6 +76,12 @@ namespace rfb { // be implicitly cancelled and re-started. void start(int timeoutMs_); + // repeat() + // Restarts the timer in a way that repeats that last timeout. + // This allows you to have a periodic timer without the risk of + // accumulating drift caused by processing delays. + void repeat(); + // stop() // Cancels the timer. void stop(); @@ -100,7 +106,7 @@ namespace rfb { bool isBefore(timeval other); protected: - timeval dueTime; + timeval dueTime, lastDueTime; int timeoutMs; Callback* cb; @@ -113,14 +119,14 @@ namespace rfb { template class MethodTimer : public Timer, public Timer::Callback { public: - MethodTimer(T* obj_, bool (T::*cb_)(Timer*)) + MethodTimer(T* obj_, void (T::*cb_)(Timer*)) : Timer(this), obj(obj_), cb(cb_) {} - virtual bool handleTimeout(Timer* t) { return (obj->*cb)(t); } + virtual void handleTimeout(Timer* t) { (obj->*cb)(t); } private: T* obj; - bool (T::*cb)(Timer*); + void (T::*cb)(Timer*); }; }; diff --git a/common/rfb/VNCSConnectionST.cxx b/common/rfb/VNCSConnectionST.cxx index 906f5f66..ffbf8be7 100644 --- a/common/rfb/VNCSConnectionST.cxx +++ b/common/rfb/VNCSConnectionST.cxx @@ -799,7 +799,7 @@ void VNCSConnectionST::supportsLEDState() writer()->writeLEDState(); } -bool VNCSConnectionST::handleTimeout(Timer* t) +void VNCSConnectionST::handleTimeout(Timer* t) { try { if ((t == &congestionTimer) || @@ -811,8 +811,6 @@ bool VNCSConnectionST::handleTimeout(Timer* t) if (t == &idleTimer) close("Idle timeout"); - - return false; } bool VNCSConnectionST::isShiftPressed() diff --git a/common/rfb/VNCSConnectionST.h b/common/rfb/VNCSConnectionST.h index edc0391e..85bfd38f 100644 --- a/common/rfb/VNCSConnectionST.h +++ b/common/rfb/VNCSConnectionST.h @@ -140,7 +140,7 @@ namespace rfb { virtual void supportsLEDState(); // Timer callbacks - virtual bool handleTimeout(Timer* t); + virtual void handleTimeout(Timer* t); // Internal methods diff --git a/common/rfb/VNCServerST.cxx b/common/rfb/VNCServerST.cxx index 560a0ffa..bd1fecf3 100644 --- a/common/rfb/VNCServerST.cxx +++ b/common/rfb/VNCServerST.cxx @@ -623,22 +623,20 @@ SConnection* VNCServerST::getConnection(network::Socket* sock) { return 0; } -bool VNCServerST::handleTimeout(Timer* t) +void VNCServerST::handleTimeout(Timer* t) { if (t == &frameTimer) { // We keep running until we go a full interval without any updates if (comparer->is_empty()) - return false; + return; writeUpdate(); // If this is the first iteration then we need to adjust the timeout - if (frameTimer.getTimeoutMs() != 1000/rfb::Server::frameRate) { + if (frameTimer.getTimeoutMs() != 1000/rfb::Server::frameRate) frameTimer.start(1000/rfb::Server::frameRate); - return false; - } - - return true; + else + frameTimer.repeat(); } else if (t == &idleTimer) { slog.info("MaxIdleTime reached, exiting"); desktop->terminate(); @@ -649,8 +647,6 @@ bool VNCServerST::handleTimeout(Timer* t) slog.info("MaxConnectionTime reached, exiting"); desktop->terminate(); } - - return false; } void VNCServerST::queryConnection(VNCSConnectionST* client, diff --git a/common/rfb/VNCServerST.h b/common/rfb/VNCServerST.h index f688b317..55d0c889 100644 --- a/common/rfb/VNCServerST.h +++ b/common/rfb/VNCServerST.h @@ -155,7 +155,7 @@ namespace rfb { protected: // Timer callbacks - virtual bool handleTimeout(Timer* t); + virtual void handleTimeout(Timer* t); // - Internal methods diff --git a/unix/vncconfig/QueryConnectDialog.cxx b/unix/vncconfig/QueryConnectDialog.cxx index e13af34b..e725de7d 100644 --- a/unix/vncconfig/QueryConnectDialog.cxx +++ b/unix/vncconfig/QueryConnectDialog.cxx @@ -74,14 +74,13 @@ void QueryConnectDialog::buttonActivate(TXButton* b) { callback->queryRejected(); } -bool QueryConnectDialog::handleTimeout(rfb::Timer* /*t*/) { +void QueryConnectDialog::handleTimeout(rfb::Timer* t) { if (timeUntilReject-- == 0) { unmap(); callback->queryTimedOut(); - return false; } else { refreshTimeout(); - return true; + t->repeat(); } } diff --git a/unix/vncconfig/QueryConnectDialog.h b/unix/vncconfig/QueryConnectDialog.h index f685dc34..dcf64e40 100644 --- a/unix/vncconfig/QueryConnectDialog.h +++ b/unix/vncconfig/QueryConnectDialog.h @@ -43,7 +43,7 @@ class QueryConnectDialog : public TXDialog, public TXEventHandler, void handleEvent(TXWindow*, XEvent* ) { } void deleteWindow(TXWindow*); void buttonActivate(TXButton* b); - bool handleTimeout(rfb::Timer* t); + void handleTimeout(rfb::Timer* t); private: void refreshTimeout(); TXLabel addressLbl, address, userLbl, user, timeoutLbl, timeout; diff --git a/unix/xserver/hw/vnc/XserverDesktop.cc b/unix/xserver/hw/vnc/XserverDesktop.cc index 64b2e614..ab7f29d2 100644 --- a/unix/xserver/hw/vnc/XserverDesktop.cc +++ b/unix/xserver/hw/vnc/XserverDesktop.cc @@ -518,13 +518,11 @@ void XserverDesktop::keyEvent(uint32_t keysym, uint32_t keycode, bool down) vncKeyboardEvent(keysym, keycode, down); } -bool XserverDesktop::handleTimeout(Timer* t) +void XserverDesktop::handleTimeout(Timer* t) { if (t == &queryConnectTimer) { server->approveConnection(queryConnectSocket, false, "The attempt to prompt the user to " "accept the connection failed"); } - - return false; } diff --git a/unix/xserver/hw/vnc/XserverDesktop.h b/unix/xserver/hw/vnc/XserverDesktop.h index 677097a6..8aa97d97 100644 --- a/unix/xserver/hw/vnc/XserverDesktop.h +++ b/unix/xserver/hw/vnc/XserverDesktop.h @@ -112,7 +112,7 @@ protected: network::SocketServer* sockserv, bool read, bool write); - virtual bool handleTimeout(rfb::Timer* t); + virtual void handleTimeout(rfb::Timer* t); private: diff --git a/vncviewer/EmulateMB.cxx b/vncviewer/EmulateMB.cxx index 72335eb8..cc680df4 100644 --- a/vncviewer/EmulateMB.cxx +++ b/vncviewer/EmulateMB.cxx @@ -277,13 +277,13 @@ void EmulateMB::filterPointerEvent(const rfb::Point& pos, int buttonMask) } } -bool EmulateMB::handleTimeout(rfb::Timer *t) +void EmulateMB::handleTimeout(rfb::Timer *t) { int action1, action2; int buttonMask; if (&timer != t) - return false; + return; if ((state > 10) || (state < 0)) throw rfb::Exception(_("Invalid state for 3 button emulation")); @@ -310,8 +310,6 @@ bool EmulateMB::handleTimeout(rfb::Timer *t) } state = stateTab[state][4][2]; - - return false; } void EmulateMB::sendAction(const rfb::Point& pos, int buttonMask, int action) diff --git a/vncviewer/EmulateMB.h b/vncviewer/EmulateMB.h index 132f44fe..77fdec66 100644 --- a/vncviewer/EmulateMB.h +++ b/vncviewer/EmulateMB.h @@ -31,7 +31,7 @@ public: protected: virtual void sendPointerEvent(const rfb::Point& pos, int buttonMask)=0; - virtual bool handleTimeout(rfb::Timer *t); + virtual void handleTimeout(rfb::Timer *t); private: void sendAction(const rfb::Point& pos, int buttonMask, int action); diff --git a/vncviewer/GestureHandler.cxx b/vncviewer/GestureHandler.cxx index c3cc1531..ed99555e 100644 --- a/vncviewer/GestureHandler.cxx +++ b/vncviewer/GestureHandler.cxx @@ -323,14 +323,12 @@ bool GestureHandler::hasDetectedGesture() return true; } -bool GestureHandler::handleTimeout(rfb::Timer* t) +void GestureHandler::handleTimeout(rfb::Timer* t) { if (t == &longpressTimer) longpressTimeout(); else if (t == &twoTouchTimer) twoTouchTimeout(); - - return false; } void GestureHandler::longpressTimeout() diff --git a/vncviewer/GestureHandler.h b/vncviewer/GestureHandler.h index 372b7865..b07454df 100644 --- a/vncviewer/GestureHandler.h +++ b/vncviewer/GestureHandler.h @@ -42,7 +42,7 @@ class GestureHandler : public rfb::Timer::Callback { private: bool hasDetectedGesture(); - virtual bool handleTimeout(rfb::Timer* t); + virtual void handleTimeout(rfb::Timer* t); void longpressTimeout(); void twoTouchTimeout(); -- cgit v1.2.3 From 30bacaeadf83935c15f403e26e5e72f392f8c879 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 29 Feb 2024 16:01:48 +0100 Subject: Make it easier to change timer interval --- common/rfb/Timer.cxx | 5 ++++- common/rfb/Timer.h | 4 +++- common/rfb/VNCServerST.cxx | 5 +---- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/common/rfb/Timer.cxx b/common/rfb/Timer.cxx index 37aaad83..e9ae5227 100644 --- a/common/rfb/Timer.cxx +++ b/common/rfb/Timer.cxx @@ -122,7 +122,7 @@ void Timer::start(int timeoutMs_) { insertTimer(this); } -void Timer::repeat() { +void Timer::repeat(int timeoutMs_) { timeval now; gettimeofday(&now, 0); @@ -135,6 +135,9 @@ void Timer::repeat() { if (msBetween(&lastDueTime, &dueTime) != 0) vlog.error("Timer incorrectly modified whilst repeating"); + if (timeoutMs_ != -1) + timeoutMs = timeoutMs_; + dueTime = addMillis(lastDueTime, timeoutMs); if (isBefore(now)) { // Time has jumped forwards, or we're not getting enough diff --git a/common/rfb/Timer.h b/common/rfb/Timer.h index 9775afd5..36ec46c5 100644 --- a/common/rfb/Timer.h +++ b/common/rfb/Timer.h @@ -80,7 +80,9 @@ namespace rfb { // Restarts the timer in a way that repeats that last timeout. // This allows you to have a periodic timer without the risk of // accumulating drift caused by processing delays. - void repeat(); + // A new interval can be specified, otherwise the previous + // interval is reused. + void repeat(int timeoutMs_=-1); // stop() // Cancels the timer. diff --git a/common/rfb/VNCServerST.cxx b/common/rfb/VNCServerST.cxx index bd1fecf3..68379f24 100644 --- a/common/rfb/VNCServerST.cxx +++ b/common/rfb/VNCServerST.cxx @@ -633,10 +633,7 @@ void VNCServerST::handleTimeout(Timer* t) writeUpdate(); // If this is the first iteration then we need to adjust the timeout - if (frameTimer.getTimeoutMs() != 1000/rfb::Server::frameRate) - frameTimer.start(1000/rfb::Server::frameRate); - else - frameTimer.repeat(); + frameTimer.repeat(1000/rfb::Server::frameRate); } else if (t == &idleTimer) { slog.info("MaxIdleTime reached, exiting"); desktop->terminate(); -- cgit v1.2.3 From 5ddb06e6b07880a34ca07cbb3c0b220c1b6b6bd1 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 28 Feb 2024 13:54:23 +0100 Subject: Maintain a constant VNCServer/SDesktop connection The desktop isn't completely paused just because there are no clients, so it might still need some support from the server object. This is primarily an issue for headless servers, where they need to continue emulating things even without clients. A scraping server can generally go completely passive if there are no clients. --- common/rfb/SDesktop.h | 17 ++++++++++------- common/rfb/VNCServerST.cxx | 4 +++- unix/x0vncserver/XDesktop.cxx | 10 ++++++---- unix/x0vncserver/XDesktop.h | 3 ++- unix/xserver/hw/vnc/XserverDesktop.cc | 7 +++++-- unix/xserver/hw/vnc/XserverDesktop.h | 3 ++- win/rfb_win32/SDisplay.cxx | 12 +++++++----- win/rfb_win32/SDisplay.h | 3 ++- 8 files changed, 37 insertions(+), 22 deletions(-) diff --git a/common/rfb/SDesktop.h b/common/rfb/SDesktop.h index 9db08116..e3871616 100644 --- a/common/rfb/SDesktop.h +++ b/common/rfb/SDesktop.h @@ -1,5 +1,5 @@ /* Copyright (C) 2002-2005 RealVNC Ltd. All Rights Reserved. - * Copyright 2009-2019 Pierre Ossman for Cendio AB + * Copyright 2009-2024 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 @@ -49,18 +49,21 @@ namespace rfb { class SDesktop : public InputHandler { public: + // init() is called immediately when the VNCServer gets a reference + // to the SDesktop, so that a reverse reference can be set up. + virtual void init(rfb::VNCServer* vs) = 0; + // start() is called by the server when the first client authenticates // successfully, and can be used to begin any expensive tasks which are not // needed when there are no clients. A valid PixelBuffer must have been // set via the VNCServer's setPixelBuffer() method by the time this call // returns. - virtual void start(VNCServer* vs) = 0; + virtual void start() = 0; // stop() is called by the server when there are no longer any // authenticated clients, and therefore the desktop can cease any - // expensive tasks. No further calls to the VNCServer passed to start() - // can be made once stop has returned. + // expensive tasks. virtual void stop() = 0; @@ -136,13 +139,13 @@ namespace rfb { if (buffer) delete buffer; } - virtual void start(VNCServer* vs) { + virtual void init(VNCServer* vs) { server = vs; server->setPixelBuffer(buffer); } + virtual void start() { + } virtual void stop() { - server->setPixelBuffer(0); - server = 0; } virtual void queryConnection(network::Socket* sock, const char* /*userName*/) { diff --git a/common/rfb/VNCServerST.cxx b/common/rfb/VNCServerST.cxx index 68379f24..8d8dbfd7 100644 --- a/common/rfb/VNCServerST.cxx +++ b/common/rfb/VNCServerST.cxx @@ -92,6 +92,8 @@ VNCServerST::VNCServerST(const char* name_, SDesktop* desktop_) { slog.debug("creating single-threaded server %s", name.c_str()); + desktop_->init(this); + // FIXME: Do we really want to kick off these right away? if (rfb::Server::maxIdleTime) idleTimer.start(secsToMillis(rfb::Server::maxIdleTime)); @@ -707,7 +709,7 @@ void VNCServerST::startDesktop() { if (!desktopStarted) { slog.debug("starting desktop"); - desktop->start(this); + desktop->start(); if (!pb) throw Exception("SDesktop::start() did not set a valid PixelBuffer"); desktopStarted = true; diff --git a/unix/x0vncserver/XDesktop.cxx b/unix/x0vncserver/XDesktop.cxx index b3d5e54c..1f1f7481 100644 --- a/unix/x0vncserver/XDesktop.cxx +++ b/unix/x0vncserver/XDesktop.cxx @@ -230,9 +230,13 @@ void XDesktop::poll() { } } +void XDesktop::init(VNCServer* vs) +{ + server = vs; +} -void XDesktop::start(VNCServer* vs) { - +void XDesktop::start() +{ // Determine actual number of buttons of the X pointer device. unsigned char btnMap[8]; int numButtons = XGetPointerMapping(dpy, btnMap, 8); @@ -247,7 +251,6 @@ void XDesktop::start(VNCServer* vs) { pb = new XPixelBuffer(dpy, factory, geometry->getRect()); vlog.info("Allocated %s", pb->getImage()->classDesc()); - server = vs; server->setPixelBuffer(pb, computeScreenLayout()); #ifdef HAVE_XDAMAGE @@ -290,7 +293,6 @@ void XDesktop::stop() { queryConnectDialog = 0; server->setPixelBuffer(0); - server = 0; delete pb; pb = 0; diff --git a/unix/x0vncserver/XDesktop.h b/unix/x0vncserver/XDesktop.h index 1cb73f43..fc230e5b 100644 --- a/unix/x0vncserver/XDesktop.h +++ b/unix/x0vncserver/XDesktop.h @@ -47,7 +47,8 @@ public: virtual ~XDesktop(); void poll(); // -=- SDesktop interface - virtual void start(rfb::VNCServer* vs); + virtual void init(rfb::VNCServer* vs); + virtual void start(); virtual void stop(); virtual void terminate(); bool isRunning(); diff --git a/unix/xserver/hw/vnc/XserverDesktop.cc b/unix/xserver/hw/vnc/XserverDesktop.cc index ab7f29d2..7de6143b 100644 --- a/unix/xserver/hw/vnc/XserverDesktop.cc +++ b/unix/xserver/hw/vnc/XserverDesktop.cc @@ -145,11 +145,14 @@ void XserverDesktop::refreshScreenLayout() server->setScreenLayout(::computeScreenLayout(&outputIdMap)); } -void XserverDesktop::start(rfb::VNCServer* vs) +void XserverDesktop::init(rfb::VNCServer* vs) { // We already own the server object, and we always keep it in a // ready state - assert(vs == server); +} + +void XserverDesktop::start() +{ } void XserverDesktop::stop() diff --git a/unix/xserver/hw/vnc/XserverDesktop.h b/unix/xserver/hw/vnc/XserverDesktop.h index 8aa97d97..6c4f0b68 100644 --- a/unix/xserver/hw/vnc/XserverDesktop.h +++ b/unix/xserver/hw/vnc/XserverDesktop.h @@ -88,7 +88,8 @@ public: const char* rejectMsg=0); // rfb::SDesktop callbacks - virtual void start(rfb::VNCServer* vs); + virtual void init(rfb::VNCServer* vs); + virtual void start(); virtual void stop(); virtual void terminate(); virtual void queryConnection(network::Socket* sock, diff --git a/win/rfb_win32/SDisplay.cxx b/win/rfb_win32/SDisplay.cxx index 612f883b..dd1ac7da 100644 --- a/win/rfb_win32/SDisplay.cxx +++ b/win/rfb_win32/SDisplay.cxx @@ -96,7 +96,12 @@ SDisplay::~SDisplay() // -=- SDesktop interface -void SDisplay::start(VNCServer* vs) +void SDisplay::init(VNCServer* vs) +{ + server = vs; +} + +void SDisplay::start() { vlog.debug("starting"); @@ -105,7 +110,6 @@ void SDisplay::start(VNCServer* vs) setConsoleSession(); // Start the SDisplay core - server = vs; startCore(); vlog.debug("started"); @@ -135,10 +139,8 @@ void SDisplay::stop() } // Stop the SDisplayCore - if (server) - server->setPixelBuffer(0); + server->setPixelBuffer(0); stopCore(); - server = 0; vlog.debug("stopped"); diff --git a/win/rfb_win32/SDisplay.h b/win/rfb_win32/SDisplay.h index febc720e..5b55cd66 100644 --- a/win/rfb_win32/SDisplay.h +++ b/win/rfb_win32/SDisplay.h @@ -71,7 +71,8 @@ namespace rfb { // -=- SDesktop interface - virtual void start(VNCServer* vs); + virtual void init(VNCServer* vs); + virtual void start(); virtual void stop(); virtual void terminate(); virtual void queryConnection(network::Socket* sock, -- cgit v1.2.3 From bc0461fd952e691d30fbadfd4c208318fa69ef09 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 28 Feb 2024 14:02:20 +0100 Subject: Make SDesktop::start()/stop() optional Let's avoid requring these as a desktop implementation can now set up everything in the init() method. --- common/rfb/SDesktop.h | 10 ++-------- unix/xserver/hw/vnc/XserverDesktop.cc | 8 -------- unix/xserver/hw/vnc/XserverDesktop.h | 2 -- 3 files changed, 2 insertions(+), 18 deletions(-) diff --git a/common/rfb/SDesktop.h b/common/rfb/SDesktop.h index e3871616..560ee7ff 100644 --- a/common/rfb/SDesktop.h +++ b/common/rfb/SDesktop.h @@ -58,14 +58,12 @@ namespace rfb { // needed when there are no clients. A valid PixelBuffer must have been // set via the VNCServer's setPixelBuffer() method by the time this call // returns. - - virtual void start() = 0; + virtual void start() {} // stop() is called by the server when there are no longer any // authenticated clients, and therefore the desktop can cease any // expensive tasks. - - virtual void stop() = 0; + virtual void stop() {} // queryConnection() is called when a connection has been // successfully authenticated. The sock and userName arguments @@ -143,10 +141,6 @@ namespace rfb { server = vs; server->setPixelBuffer(buffer); } - virtual void start() { - } - virtual void stop() { - } virtual void queryConnection(network::Socket* sock, const char* /*userName*/) { server->approveConnection(sock, true, NULL); diff --git a/unix/xserver/hw/vnc/XserverDesktop.cc b/unix/xserver/hw/vnc/XserverDesktop.cc index 7de6143b..c4117c74 100644 --- a/unix/xserver/hw/vnc/XserverDesktop.cc +++ b/unix/xserver/hw/vnc/XserverDesktop.cc @@ -151,14 +151,6 @@ void XserverDesktop::init(rfb::VNCServer* vs) // ready state } -void XserverDesktop::start() -{ -} - -void XserverDesktop::stop() -{ -} - void XserverDesktop::queryConnection(network::Socket* sock, const char* userName) { diff --git a/unix/xserver/hw/vnc/XserverDesktop.h b/unix/xserver/hw/vnc/XserverDesktop.h index 6c4f0b68..3cac2121 100644 --- a/unix/xserver/hw/vnc/XserverDesktop.h +++ b/unix/xserver/hw/vnc/XserverDesktop.h @@ -89,8 +89,6 @@ public: // rfb::SDesktop callbacks virtual void init(rfb::VNCServer* vs); - virtual void start(); - virtual void stop(); virtual void terminate(); virtual void queryConnection(network::Socket* sock, const char* userName); -- cgit v1.2.3 From 5322b8ffcdaa8479093917729acc987672546592 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Fri, 16 Feb 2024 14:31:29 +0100 Subject: Add missing ErrorF() newlines --- unix/xserver/hw/vnc/xvnc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/unix/xserver/hw/vnc/xvnc.c b/unix/xserver/hw/vnc/xvnc.c index 16a28831..a953240d 100644 --- a/unix/xserver/hw/vnc/xvnc.c +++ b/unix/xserver/hw/vnc/xvnc.c @@ -670,14 +670,14 @@ vncRandRScreenSetSize(ScreenPtr pScreen, ret = vncRandRCrtcSet(pScreen, crtc, NULL, crtc->x, crtc->y, crtc->rotation, 0, NULL); if (!ret) - ErrorF("Warning: Unable to disable CRTC that is outside of new screen dimensions"); + ErrorF("Warning: Unable to disable CRTC that is outside of new screen dimensions\n"); continue; } /* Just needs to be resized to a temporary mode */ mode = vncRandRModeGet(width - crtc->x, height - crtc->y); if (mode == NULL) { - ErrorF("Warning: Unable to create custom mode for %dx%d", + ErrorF("Warning: Unable to create custom mode for %dx%d\n", width - crtc->x, height - crtc->y); continue; } @@ -687,7 +687,7 @@ vncRandRScreenSetSize(ScreenPtr pScreen, crtc->numOutputs, crtc->outputs); RRModeDestroy(mode); if (!ret) - ErrorF("Warning: Unable to crop CRTC to new screen dimensions"); + ErrorF("Warning: Unable to crop CRTC to new screen dimensions\n"); } return TRUE; -- cgit v1.2.3 From 3875912fb65c0710f666efd739a27d8c8835f209 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Fri, 16 Feb 2024 14:31:54 +0100 Subject: Add support for X Present extension This makes it possible for applications to synchronize their updates to the updates sent out to clients. This avoids tearing, and could in the future also help with rate limiting applications to what the client can actually show. --- common/rfb/SDesktop.h | 4 ++ common/rfb/VNCServer.h | 2 + common/rfb/VNCServerST.cxx | 12 ++++- common/rfb/VNCServerST.h | 2 + unix/xserver/hw/vnc/Makefile.am | 4 +- unix/xserver/hw/vnc/XserverDesktop.cc | 34 ++++++++++++- unix/xserver/hw/vnc/XserverDesktop.h | 8 ++- unix/xserver/hw/vnc/vncExtInit.cc | 29 ++++++++++- unix/xserver/hw/vnc/vncExtInit.h | 6 ++- unix/xserver/hw/vnc/vncPresent.c | 93 +++++++++++++++++++++++++++++++++++ unix/xserver/hw/vnc/vncPresent.h | 27 ++++++++++ unix/xserver/hw/vnc/xvnc.c | 5 ++ 12 files changed, 218 insertions(+), 8 deletions(-) create mode 100644 unix/xserver/hw/vnc/vncPresent.c create mode 100644 unix/xserver/hw/vnc/vncPresent.h diff --git a/common/rfb/SDesktop.h b/common/rfb/SDesktop.h index 560ee7ff..94e4b028 100644 --- a/common/rfb/SDesktop.h +++ b/common/rfb/SDesktop.h @@ -87,6 +87,10 @@ namespace rfb { return resultProhibited; } + // frameTick() is called whenever a frame update has been processed, + // signalling that a good time to render new data + virtual void frameTick(uint64_t msc) { (void)msc; } + // InputHandler interface // pointerEvent(), keyEvent() and clientCutText() are called in response to // the relevant RFB protocol messages from clients. diff --git a/common/rfb/VNCServer.h b/common/rfb/VNCServer.h index 3f97634b..314367eb 100644 --- a/common/rfb/VNCServer.h +++ b/common/rfb/VNCServer.h @@ -42,6 +42,8 @@ namespace rfb { virtual void blockUpdates() = 0; virtual void unblockUpdates() = 0; + virtual uint64_t getMsc() = 0; + // setPixelBuffer() tells the server to use the given pixel buffer (and // optionally a modified screen layout). If this differs in size from // the previous pixel buffer, this may result in protocol messages being diff --git a/common/rfb/VNCServerST.cxx b/common/rfb/VNCServerST.cxx index 8d8dbfd7..bea32abe 100644 --- a/common/rfb/VNCServerST.cxx +++ b/common/rfb/VNCServerST.cxx @@ -1,5 +1,5 @@ /* Copyright (C) 2002-2005 RealVNC Ltd. All Rights Reserved. - * Copyright 2009-2019 Pierre Ossman for Cendio AB + * Copyright 2009-2024 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 @@ -88,7 +88,7 @@ VNCServerST::VNCServerST(const char* name_, SDesktop* desktop_) renderedCursorInvalid(false), keyRemapper(&KeyRemapper::defInstance), idleTimer(this), disconnectTimer(this), connectTimer(this), - frameTimer(this) + msc(0), frameTimer(this) { slog.debug("creating single-threaded server %s", name.c_str()); @@ -257,6 +257,11 @@ void VNCServerST::unblockUpdates() } } +uint64_t VNCServerST::getMsc() +{ + return msc; +} + void VNCServerST::setPixelBuffer(PixelBuffer* pb_, const ScreenSet& layout) { if (comparer) @@ -634,6 +639,9 @@ void VNCServerST::handleTimeout(Timer* t) writeUpdate(); + msc++; + desktop->frameTick(msc); + // If this is the first iteration then we need to adjust the timeout frameTimer.repeat(1000/rfb::Server::frameRate); } else if (t == &idleTimer) { diff --git a/common/rfb/VNCServerST.h b/common/rfb/VNCServerST.h index 55d0c889..719b3f36 100644 --- a/common/rfb/VNCServerST.h +++ b/common/rfb/VNCServerST.h @@ -81,6 +81,7 @@ namespace rfb { virtual void blockUpdates(); virtual void unblockUpdates(); + virtual uint64_t getMsc(); virtual void setPixelBuffer(PixelBuffer* pb, const ScreenSet& layout); virtual void setPixelBuffer(PixelBuffer* pb); virtual void setScreenLayout(const ScreenSet& layout); @@ -206,6 +207,7 @@ namespace rfb { Timer disconnectTimer; Timer connectTimer; + uint64_t msc; Timer frameTimer; }; diff --git a/unix/xserver/hw/vnc/Makefile.am b/unix/xserver/hw/vnc/Makefile.am index 1e985966..40eba4f2 100644 --- a/unix/xserver/hw/vnc/Makefile.am +++ b/unix/xserver/hw/vnc/Makefile.am @@ -11,12 +11,12 @@ COMMON_LIBS=$(NETWORK_LIB) $(RFB_LIB) $(RDR_LIB) $(OS_LIB) $(UNIXCOMMON_LIB) noinst_LTLIBRARIES = libvnccommon.la HDRS = vncExtInit.h vncHooks.h \ - vncBlockHandler.h vncSelection.h \ + vncBlockHandler.h vncPresent.h vncSelection.h \ XorgGlue.h XserverDesktop.h xorg-version.h \ vncInput.h RFBGlue.h libvnccommon_la_SOURCES = $(HDRS) \ - vncExt.c vncExtInit.cc vncHooks.c vncSelection.c \ + vncExt.c vncExtInit.cc vncHooks.c vncPresent.c vncSelection.c \ vncBlockHandler.c XorgGlue.c RandrGlue.c RFBGlue.cc XserverDesktop.cc \ vncInput.c vncInputXKB.c qnum_to_xorgevdev.c qnum_to_xorgkbd.c diff --git a/unix/xserver/hw/vnc/XserverDesktop.cc b/unix/xserver/hw/vnc/XserverDesktop.cc index c4117c74..6fe055ea 100644 --- a/unix/xserver/hw/vnc/XserverDesktop.cc +++ b/unix/xserver/hw/vnc/XserverDesktop.cc @@ -1,5 +1,5 @@ /* Copyright (C) 2002-2005 RealVNC Ltd. All Rights Reserved. - * Copyright 2009-2019 Pierre Ossman for Cendio AB + * Copyright 2009-2024 Pierre Ossman for Cendio AB * Copyright 2014 Brian P. Hinz * * This is free software; you can redistribute it and/or modify @@ -54,6 +54,7 @@ extern "C" { void vncSetGlueContext(int screenIndex); +void vncPresentMscEvent(uint64_t id, uint64_t msc); } using namespace rfb; @@ -145,6 +146,21 @@ void XserverDesktop::refreshScreenLayout() server->setScreenLayout(::computeScreenLayout(&outputIdMap)); } +uint64_t XserverDesktop::getMsc() +{ + return server->getMsc(); +} + +void XserverDesktop::queueMsc(uint64_t id, uint64_t msc) +{ + pendingMsc[id] = msc; +} + +void XserverDesktop::abortMsc(uint64_t id) +{ + pendingMsc.erase(id); +} + void XserverDesktop::init(rfb::VNCServer* vs) { // We already own the server object, and we always keep it in a @@ -471,6 +487,22 @@ unsigned int XserverDesktop::setScreenLayout(int fb_width, int fb_height, return result; } +void XserverDesktop::frameTick(uint64_t msc) +{ + std::map::iterator iter, next; + + for (iter = pendingMsc.begin(); iter != pendingMsc.end();) { + next = iter; next++; + + if (iter->second <= msc) { + pendingMsc.erase(iter->first); + vncPresentMscEvent(iter->first, msc); + } + + iter = next; + } +} + void XserverDesktop::handleClipboardRequest() { vncHandleClipboardRequest(); diff --git a/unix/xserver/hw/vnc/XserverDesktop.h b/unix/xserver/hw/vnc/XserverDesktop.h index 3cac2121..f3c684c7 100644 --- a/unix/xserver/hw/vnc/XserverDesktop.h +++ b/unix/xserver/hw/vnc/XserverDesktop.h @@ -1,5 +1,5 @@ /* Copyright (C) 2002-2005 RealVNC Ltd. All Rights Reserved. - * Copyright 2009-2019 Pierre Ossman for Cendio AB + * Copyright 2009-2024 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 @@ -59,6 +59,9 @@ public: void unblockUpdates(); void setFramebuffer(int w, int h, void* fbptr, int stride); void refreshScreenLayout(); + uint64_t getMsc(); + void queueMsc(uint64_t id, uint64_t msc); + void abortMsc(uint64_t id); void requestClipboard(); void announceClipboard(bool available); void sendClipboardData(const char* data); @@ -96,6 +99,7 @@ public: virtual void keyEvent(uint32_t keysym, uint32_t keycode, bool down); virtual unsigned int setScreenLayout(int fb_width, int fb_height, const rfb::ScreenSet& layout); + virtual void frameTick(uint64_t msc); virtual void handleClipboardRequest(); virtual void handleClipboardAnnounce(bool available); virtual void handleClipboardData(const char* data); @@ -128,6 +132,8 @@ private: OutputIdMap outputIdMap; + std::map pendingMsc; + rfb::Point oldCursorPos; }; #endif diff --git a/unix/xserver/hw/vnc/vncExtInit.cc b/unix/xserver/hw/vnc/vncExtInit.cc index 1dfe76d7..c0610fe8 100644 --- a/unix/xserver/hw/vnc/vncExtInit.cc +++ b/unix/xserver/hw/vnc/vncExtInit.cc @@ -1,5 +1,5 @@ /* Copyright (C) 2002-2005 RealVNC Ltd. All Rights Reserved. - * Copyright 2011-2019 Pierre Ossman for Cendio AB + * Copyright 2011-2024 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 @@ -482,6 +482,33 @@ void vncRefreshScreenLayout(int scrIdx) } } +uint64_t vncGetMsc(int scrIdx) +{ + try { + return desktop[scrIdx]->getMsc(); + } catch (rdr::Exception& e) { + vncFatalError("vncGetMsc: %s\n", e.str()); + } +} + +void vncQueueMsc(int scrIdx, uint64_t id, uint64_t msc) +{ + try { + desktop[scrIdx]->queueMsc(id, msc); + } catch (rdr::Exception& e) { + vncFatalError("vncQueueMsc: %s\n", e.str()); + } +} + +void vncAbortMsc(int scrIdx, uint64_t id) +{ + try { + desktop[scrIdx]->abortMsc(id); + } catch (rdr::Exception& e) { + vncFatalError("vncAbortMsc: %s\n", e.str()); + } +} + int vncOverrideParam(const char *nameAndValue) { const char* equalSign = strchr(nameAndValue, '='); diff --git a/unix/xserver/hw/vnc/vncExtInit.h b/unix/xserver/hw/vnc/vncExtInit.h index c317d8a2..9ca00b06 100644 --- a/unix/xserver/hw/vnc/vncExtInit.h +++ b/unix/xserver/hw/vnc/vncExtInit.h @@ -1,5 +1,5 @@ /* Copyright (C) 2002-2005 RealVNC Ltd. All Rights Reserved. - * Copyright 2011-2019 Pierre Ossman for Cendio AB + * Copyright 2011-2024 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 @@ -87,6 +87,10 @@ void vncPreScreenResize(int scrIdx); void vncPostScreenResize(int scrIdx, int success, int width, int height); void vncRefreshScreenLayout(int scrIdx); +uint64_t vncGetMsc(int scrIdx); +void vncQueueMsc(int scrIdx, uint64_t id, uint64_t msc); +void vncAbortMsc(int scrIdx, uint64_t id); + int vncOverrideParam(const char *nameAndValue); #ifdef __cplusplus diff --git a/unix/xserver/hw/vnc/vncPresent.c b/unix/xserver/hw/vnc/vncPresent.c new file mode 100644 index 00000000..89dcc1d0 --- /dev/null +++ b/unix/xserver/hw/vnc/vncPresent.c @@ -0,0 +1,93 @@ +/* Copyright 2024 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 + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this software; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, + * USA. + */ + +#ifdef HAVE_DIX_CONFIG_H +#include +#endif + +#include "vncExtInit.h" +#include "vncPresent.h" + +#include + +static RRCrtcPtr vncPresentGetCrtc(WindowPtr window) +{ + ScreenPtr pScreen = window->drawable.pScreen; + rrScrPrivPtr rp = rrGetScrPriv(pScreen); + + /* All output is synchronized, so just pick the first active crtc */ + for (int c = 0; c < rp->numCrtcs; c++) { + RRCrtcPtr crtc; + + crtc = rp->crtcs[c]; + if (crtc->mode == NULL) + continue; + + return crtc; + } + + return NULL; +} + +static int vncPresentGetUstMsc(RRCrtcPtr crtc, CARD64 *ust, CARD64 *msc) +{ + *ust = GetTimeInMicros(); + *msc = vncGetMsc(crtc->pScreen->myNum); + + return Success; +} + +static int vncPresentQueueVBlank(RRCrtcPtr crtc, uint64_t event_id, + uint64_t msc) +{ + vncQueueMsc(crtc->pScreen->myNum, event_id, msc); + return Success; +} + +void vncPresentMscEvent(uint64_t id, uint64_t msc) +{ + present_event_notify(id, GetTimeInMicros(), msc); +} + +static void vncPresentAbortVBlank(RRCrtcPtr crtc, uint64_t event_id, + uint64_t msc) +{ + vncAbortMsc(crtc->pScreen->myNum, event_id); +} + +static void vncPresentFlush(WindowPtr window) +{ +} + +static present_screen_info_rec vncPresentScreenInfo = { + .version = PRESENT_SCREEN_INFO_VERSION, + + .get_crtc = vncPresentGetCrtc, + .get_ust_msc = vncPresentGetUstMsc, + .queue_vblank = vncPresentQueueVBlank, + .abort_vblank = vncPresentAbortVBlank, + .flush = vncPresentFlush, + + .capabilities = PresentCapabilityNone, +}; + +Bool +vncPresentInit(ScreenPtr screen) +{ + return present_screen_init(screen, &vncPresentScreenInfo); +} \ No newline at end of file diff --git a/unix/xserver/hw/vnc/vncPresent.h b/unix/xserver/hw/vnc/vncPresent.h new file mode 100644 index 00000000..17407402 --- /dev/null +++ b/unix/xserver/hw/vnc/vncPresent.h @@ -0,0 +1,27 @@ +/* Copyright 2024 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 + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this software; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, + * USA. + */ + +#ifndef __PRESENT_H__ +#define __PRESENT_H__ + +#include + +Bool vncPresentInit(ScreenPtr screen); +void vncPresentMscEvent(uint64_t id, uint64_t msc); + +#endif diff --git a/unix/xserver/hw/vnc/xvnc.c b/unix/xserver/hw/vnc/xvnc.c index a953240d..706c9d5a 100644 --- a/unix/xserver/hw/vnc/xvnc.c +++ b/unix/xserver/hw/vnc/xvnc.c @@ -36,6 +36,7 @@ from the X Consortium. #include "RFBGlue.h" #include "XorgGlue.h" #include "RandrGlue.h" +#include "vncPresent.h" #include "xorg-version.h" #include @@ -1085,6 +1086,10 @@ vncScreenInit(ScreenPtr pScreen, int argc, char **argv) if (!ret) return FALSE; + ret = vncPresentInit(pScreen); + if (!ret) + ErrorF("Failed to initialize Present extension\n"); + return TRUE; } /* end vncScreenInit */ -- cgit v1.2.3 From d226d98269dca1a5c52f13a681e4740fe0a3d39d Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 29 Feb 2024 17:33:43 +0100 Subject: Restart the frame timer immediately This marks the timer as started again, before we call out to various external places that might be confused by the frame timer reporting that it is stopped. --- common/rfb/VNCServerST.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/rfb/VNCServerST.cxx b/common/rfb/VNCServerST.cxx index bea32abe..296ddd8e 100644 --- a/common/rfb/VNCServerST.cxx +++ b/common/rfb/VNCServerST.cxx @@ -637,13 +637,13 @@ void VNCServerST::handleTimeout(Timer* t) if (comparer->is_empty()) return; + // If this is the first iteration then we need to adjust the timeout + frameTimer.repeat(1000/rfb::Server::frameRate); + writeUpdate(); msc++; desktop->frameTick(msc); - - // If this is the first iteration then we need to adjust the timeout - frameTimer.repeat(1000/rfb::Server::frameRate); } else if (t == &idleTimer) { slog.info("MaxIdleTime reached, exiting"); desktop->terminate(); -- cgit v1.2.3 From 5242bfb39648855acd928b186b7c7b6b1f84313c Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 28 Feb 2024 14:20:54 +0100 Subject: Keep frame clock running if waiting for frame tick If there is something interested in synchronizing to a frame tick, then keep the frame clock running, even if there are no updates. This is need mainly when something starts rendering, but also when something renders much slower than the frame clock (so it is essentially constantly "starting"). Such an application will not draw anything until it gets a new frame tick, which it won't get as the frame clock is waiting for something to start drawing. --- common/rfb/VNCServer.h | 1 + common/rfb/VNCServerST.cxx | 20 ++++++++++++++++---- common/rfb/VNCServerST.h | 3 ++- unix/xserver/hw/vnc/XserverDesktop.cc | 1 + 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/common/rfb/VNCServer.h b/common/rfb/VNCServer.h index 314367eb..b49dbfe3 100644 --- a/common/rfb/VNCServer.h +++ b/common/rfb/VNCServer.h @@ -43,6 +43,7 @@ namespace rfb { virtual void unblockUpdates() = 0; virtual uint64_t getMsc() = 0; + virtual void queueMsc(uint64_t target) = 0; // setPixelBuffer() tells the server to use the given pixel buffer (and // optionally a modified screen layout). If this differs in size from diff --git a/common/rfb/VNCServerST.cxx b/common/rfb/VNCServerST.cxx index 296ddd8e..f091ddd5 100644 --- a/common/rfb/VNCServerST.cxx +++ b/common/rfb/VNCServerST.cxx @@ -88,7 +88,7 @@ VNCServerST::VNCServerST(const char* name_, SDesktop* desktop_) renderedCursorInvalid(false), keyRemapper(&KeyRemapper::defInstance), idleTimer(this), disconnectTimer(this), connectTimer(this), - msc(0), frameTimer(this) + msc(0), queuedMsc(0), frameTimer(this) { slog.debug("creating single-threaded server %s", name.c_str()); @@ -262,6 +262,14 @@ uint64_t VNCServerST::getMsc() return msc; } +void VNCServerST::queueMsc(uint64_t target) +{ + if (target > queuedMsc) + queuedMsc = target; + + startFrameClock(); +} + void VNCServerST::setPixelBuffer(PixelBuffer* pb_, const ScreenSet& layout) { if (comparer) @@ -634,13 +642,17 @@ void VNCServerST::handleTimeout(Timer* t) { if (t == &frameTimer) { // We keep running until we go a full interval without any updates - if (comparer->is_empty()) - return; + if (comparer->is_empty()) { + // Unless something waits for us to advance the frame count + if (queuedMsc < msc) + return; + } // If this is the first iteration then we need to adjust the timeout frameTimer.repeat(1000/rfb::Server::frameRate); - writeUpdate(); + if (!comparer->is_empty()) + writeUpdate(); msc++; desktop->frameTick(msc); diff --git a/common/rfb/VNCServerST.h b/common/rfb/VNCServerST.h index 719b3f36..3436d333 100644 --- a/common/rfb/VNCServerST.h +++ b/common/rfb/VNCServerST.h @@ -82,6 +82,7 @@ namespace rfb { virtual void blockUpdates(); virtual void unblockUpdates(); virtual uint64_t getMsc(); + virtual void queueMsc(uint64_t target); virtual void setPixelBuffer(PixelBuffer* pb, const ScreenSet& layout); virtual void setPixelBuffer(PixelBuffer* pb); virtual void setScreenLayout(const ScreenSet& layout); @@ -207,7 +208,7 @@ namespace rfb { Timer disconnectTimer; Timer connectTimer; - uint64_t msc; + uint64_t msc, queuedMsc; Timer frameTimer; }; diff --git a/unix/xserver/hw/vnc/XserverDesktop.cc b/unix/xserver/hw/vnc/XserverDesktop.cc index 6fe055ea..eaf6f901 100644 --- a/unix/xserver/hw/vnc/XserverDesktop.cc +++ b/unix/xserver/hw/vnc/XserverDesktop.cc @@ -154,6 +154,7 @@ uint64_t XserverDesktop::getMsc() void XserverDesktop::queueMsc(uint64_t id, uint64_t msc) { pendingMsc[id] = msc; + server->queueMsc(msc); } void XserverDesktop::abortMsc(uint64_t id) -- cgit v1.2.3 From 28c3f121613807df6d53dde9ac653916dcf8902d Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Mon, 19 Feb 2024 15:28:12 +0100 Subject: Slow down fake clock when no clients Run the frame clock at a slow 1 Hz if there are no clients connected. This is similar to what a normal X server does when the screen is blanked, and should keep applications waiting for the frame tick happy. Note that we still only keep the frame clock running if there is any application that are interested in it. Otherwise we still stop it completely. --- common/rfb/VNCServerST.cxx | 43 ++++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/common/rfb/VNCServerST.cxx b/common/rfb/VNCServerST.cxx index f091ddd5..72cf942d 100644 --- a/common/rfb/VNCServerST.cxx +++ b/common/rfb/VNCServerST.cxx @@ -250,11 +250,9 @@ void VNCServerST::unblockUpdates() blockCounter--; - // Restart the frame clock if we have updates - if (blockCounter == 0) { - if (!comparer->is_empty()) - startFrameClock(); - } + // Restart the frame clock in case we have updates + if (blockCounter == 0) + startFrameClock(); } uint64_t VNCServerST::getMsc() @@ -641,17 +639,26 @@ SConnection* VNCServerST::getConnection(network::Socket* sock) { void VNCServerST::handleTimeout(Timer* t) { if (t == &frameTimer) { - // We keep running until we go a full interval without any updates - if (comparer->is_empty()) { + int timeout; + + // We keep running until we go a full interval without any updates, + // or there are no active clients anymore + if (comparer->is_empty() || !desktopStarted) { // Unless something waits for us to advance the frame count if (queuedMsc < msc) return; } // If this is the first iteration then we need to adjust the timeout - frameTimer.repeat(1000/rfb::Server::frameRate); + timeout = 1000/rfb::Server::frameRate; - if (!comparer->is_empty()) + // If there are no clients, then slow down the clock + if (!desktopStarted) + timeout = 1000; + + frameTimer.repeat(timeout); + + if (!comparer->is_empty() && desktopStarted) writeUpdate(); msc++; @@ -737,6 +744,12 @@ void VNCServerST::startDesktop() // stopped, so flush those out if (!comparer->is_empty()) writeUpdate(); + // If the frame clock is running, then it will be running slowly, + // so give it a kick to run at normal speed right away + if (frameTimer.isStarted()) { + stopFrameClock(); + startFrameClock(); + } } } @@ -746,7 +759,6 @@ void VNCServerST::stopDesktop() slog.debug("stopping desktop"); desktopStarted = false; desktop->stop(); - stopFrameClock(); } } @@ -774,9 +786,18 @@ void VNCServerST::startFrameClock() return; if (blockCounter > 0) return; - if (!desktopStarted) + + // Anyone actually interested in frames? + if (comparer->is_empty() && (queuedMsc <= msc)) return; + // Run the frame clock very slowly if there are no clients to actually + // send updates to + if (!desktopStarted) { + frameTimer.start(1000); + return; + } + // The first iteration will be just half a frame as we get a very // unstable update rate if we happen to be perfectly in sync with // the application's update rate -- cgit v1.2.3