From: Pierre Ossman Date: Tue, 8 Nov 2011 12:44:10 +0000 (+0000) Subject: Reimplement the deferred update handling, this time in a more robust and X-Git-Tag: v1.1.90~37 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=bbf955ebd77320fb7f95efc3ac140feced109ed8;p=tigervnc.git Reimplement the deferred update handling, this time in a more robust and well-behaved manner. git-svn-id: svn://svn.code.sf.net/p/tigervnc/code/trunk@4784 3789f03b-4d11-0410-bbf8-ca57d06f2519 --- diff --git a/common/rfb/VNCSConnectionST.cxx b/common/rfb/VNCSConnectionST.cxx index bcc7f234..30d9b6ad 100644 --- a/common/rfb/VNCSConnectionST.cxx +++ b/common/rfb/VNCSConnectionST.cxx @@ -668,7 +668,11 @@ void VNCSConnectionST::writeFramebufferUpdate() updates.enable_copyrect(cp.useCopyRect); - server->checkUpdate(); + // Fetch updates from server object, and see if we are allowed to send + // anything right now (the framebuffer might have changed in ways we + // haven't yet been informed of). + if (!server->checkUpdate()) + return; // Get the lists of updates. Prior to exporting the data to the `ui' object, // getUpdateInfo() will normalize the `updates' object such way that its diff --git a/common/rfb/VNCServer.h b/common/rfb/VNCServer.h index 1c8d38e0..4e554792 100644 --- a/common/rfb/VNCServer.h +++ b/common/rfb/VNCServer.h @@ -62,10 +62,6 @@ namespace rfb { // their close() method with the supplied reason. virtual void closeClients(const char* reason) = 0; - // tryUpdate() causes the server to attempt to send updates to any waiting - // clients. - virtual void tryUpdate() = 0; - // setCursor() tells the server that the cursor has changed. The // cursorData argument contains width*height pixel values in the pixel // buffer's format. The mask argument is a bitmask with a 1-bit meaning diff --git a/common/rfb/VNCServerST.cxx b/common/rfb/VNCServerST.cxx index eea6565d..5c135969 100644 --- a/common/rfb/VNCServerST.cxx +++ b/common/rfb/VNCServerST.cxx @@ -64,6 +64,12 @@ using namespace rfb; static LogWriter slog("VNCServerST"); LogWriter VNCServerST::connectionsLog("Connections"); +rfb::IntParameter deferUpdateTime("DeferUpdate", + "Time in milliseconds to defer updates",10); + +rfb::BoolParameter alwaysSetDeferUpdateTimer("AlwaysSetDeferUpdateTimer", + "Always reset the defer update timer on every change",false); + // // -=- VNCServerST Implementation // @@ -76,7 +82,8 @@ VNCServerST::VNCServerST(const char* name_, SDesktop* desktop_) renderedCursorInvalid(false), queryConnectionHandler(0), keyRemapper(&KeyRemapper::defInstance), useEconomicTranslate(false), - lastConnectionTime(0), disableclients(false) + lastConnectionTime(0), disableclients(false), + deferTimer(this), deferPending(false) { lastUserInputTime = lastDisconnectTime = time(0); slog.debug("creating single-threaded server %s", name.buf); @@ -373,25 +380,22 @@ void VNCServerST::setName(const char* name_) void VNCServerST::add_changed(const Region& region) { - if (comparer != 0) { - comparer->add_changed(region); - } + if (comparer == NULL) + return; + + comparer->add_changed(region); + startDefer(); + tryUpdate(); } void VNCServerST::add_copied(const Region& dest, const Point& delta) { - if (comparer != 0) { - comparer->add_copied(dest, delta); - } -} + if (comparer == NULL) + return; -void VNCServerST::tryUpdate() -{ - std::list::iterator ci, ci_next; - for (ci = clients.begin(); ci != clients.end(); ci = ci_next) { - ci_next = ci; ci_next++; - (*ci)->writeFramebufferUpdateOrClose(); - } + comparer->add_copied(dest, delta); + startDefer(); + tryUpdate(); } void VNCServerST::setCursor(int width, int height, const Point& newHotspot, @@ -471,6 +475,15 @@ SConnection* VNCServerST::getSConnection(network::Socket* sock) { return 0; } +bool VNCServerST::handleTimeout(Timer* t) +{ + if (t != &deferTimer) + return false; + + tryUpdate(); + + return false; +} // -=- Internal methods @@ -503,6 +516,44 @@ inline bool VNCServerST::needRenderedCursor() return false; } +inline void VNCServerST::startDefer() +{ + if (deferUpdateTime == 0) + return; + + if (deferPending && !alwaysSetDeferUpdateTimer) + return; + + gettimeofday(&deferStart, NULL); + deferTimer.start(deferUpdateTime); + + deferPending = true; +} + +inline bool VNCServerST::checkDefer() +{ + if (!deferPending) + return true; + + if (msSince(&deferStart) >= deferUpdateTime) + return true; + + return false; +} + +void VNCServerST::tryUpdate() +{ + std::list::iterator ci, ci_next; + + if (!checkDefer()) + return; + + for (ci = clients.begin(); ci != clients.end(); ci = ci_next) { + ci_next = ci; ci_next++; + (*ci)->writeFramebufferUpdateOrClose(); + } +} + // checkUpdate() is called just before sending an update. It checks to see // what updates are pending and propagates them to the update tracker for each // client. It uses the ComparingUpdateTracker's compare() method to filter out @@ -510,7 +561,7 @@ inline bool VNCServerST::needRenderedCursor() // state of the (server-side) rendered cursor, if necessary rendering it again // with the correct background. -void VNCServerST::checkUpdate() +bool VNCServerST::checkUpdate() { UpdateInfo ui; comparer->getUpdateInfo(&ui, pb->getRect()); @@ -518,7 +569,13 @@ void VNCServerST::checkUpdate() bool renderCursor = needRenderedCursor(); if (ui.is_empty() && !(renderCursor && renderedCursorInvalid)) - return; + return true; + + // Block client from updating if we are currently deferring updates + if (!checkDefer()) + return false; + + deferPending = false; Region toCheck = ui.changed.union_(ui.copied); @@ -561,6 +618,8 @@ void VNCServerST::checkUpdate() } comparer->clear(); + + return true; } void VNCServerST::getConnInfo(ListConnInfo * listConn) diff --git a/common/rfb/VNCServerST.h b/common/rfb/VNCServerST.h index c74be903..47a48019 100644 --- a/common/rfb/VNCServerST.h +++ b/common/rfb/VNCServerST.h @@ -23,6 +23,8 @@ #ifndef __RFB_VNCSERVERST_H__ #define __RFB_VNCSERVERST_H__ +#include + #include #include @@ -31,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -42,7 +45,9 @@ namespace rfb { class PixelBuffer; class KeyRemapper; - class VNCServerST : public VNCServer, public network::SocketServer { + class VNCServerST : public VNCServer, + public Timer::Callback, + public network::SocketServer { public: // -=- Constructors @@ -85,7 +90,6 @@ namespace rfb { virtual void serverCutText(const char* str, int len); virtual void add_changed(const Region ®ion); virtual void add_copied(const Region &dest, const Point &delta); - virtual void tryUpdate(); virtual void setCursor(int width, int height, const Point& hotspot, void* cursorData, void* mask); virtual void setCursorPos(const Point& p); @@ -192,6 +196,11 @@ namespace rfb { friend class VNCSConnectionST; + // Timer callbacks + virtual bool handleTimeout(Timer* t); + + // - Internal methods + void startDesktop(); static LogWriter connectionsLog; @@ -222,7 +231,10 @@ namespace rfb { int authClientCount(); bool needRenderedCursor(); - void checkUpdate(); + void startDefer(); + bool checkDefer(); + void tryUpdate(); + bool checkUpdate(); void notifyScreenLayoutChange(VNCSConnectionST *requester); @@ -235,6 +247,10 @@ namespace rfb { time_t lastConnectionTime; bool disableclients; + + Timer deferTimer; + bool deferPending; + struct timeval deferStart; }; }; diff --git a/unix/x0vncserver/PollingManager.cxx b/unix/x0vncserver/PollingManager.cxx index a1534d93..3a040e23 100644 --- a/unix/x0vncserver/PollingManager.cxx +++ b/unix/x0vncserver/PollingManager.cxx @@ -123,9 +123,7 @@ void PollingManager::poll(VNCServer *server) debugBeforePoll(); #endif - // Perform polling and try update clients if changes were detected. - if (pollScreen(server)) - server->tryUpdate(); + pollScreen(server); #ifdef DEBUG debugAfterPoll(); diff --git a/unix/xserver/hw/vnc/Input.cc b/unix/xserver/hw/vnc/Input.cc index f304d0e0..7247eae8 100644 --- a/unix/xserver/hw/vnc/Input.cc +++ b/unix/xserver/hw/vnc/Input.cc @@ -207,7 +207,6 @@ void InputDevice::PointerSync(void) oldCursorPos = cursorPos; server->setCursorPos(cursorPos); - server->tryUpdate(); } static int pointerProc(DeviceIntPtr pDevice, int onoff) diff --git a/unix/xserver/hw/vnc/XserverDesktop.cc b/unix/xserver/hw/vnc/XserverDesktop.cc index 60254aec..23dbee0c 100644 --- a/unix/xserver/hw/vnc/XserverDesktop.cc +++ b/unix/xserver/hw/vnc/XserverDesktop.cc @@ -64,12 +64,6 @@ using namespace network; static LogWriter vlog("XserverDesktop"); -rfb::IntParameter deferUpdateTime("DeferUpdate", - "Time in milliseconds to defer updates",1); - -rfb::BoolParameter alwaysSetDeferUpdateTimer("AlwaysSetDeferUpdateTimer", - "Always reset the defer update timer on every change",false); - IntParameter queryConnectTimeout("QueryConnectTimeout", "Number of seconds to show the Accept Connection dialog before " "rejecting the connection", @@ -145,7 +139,7 @@ XserverDesktop::XserverDesktop(ScreenPtr pScreen_, network::TcpListener* httpListener_, const char* name, const rfb::PixelFormat &pf, void* fbptr, int stride) - : pScreen(pScreen_), deferredUpdateTimer(0), + : pScreen(pScreen_), server(0), httpServer(0), listener(listener_), httpListener(httpListener_), cmap(0), deferredUpdateTimerSet(false), @@ -171,7 +165,6 @@ XserverDesktop::~XserverDesktop() { if (!directFbptr) delete [] data; - TimerFree(deferredUpdateTimer); delete inputDevice; delete httpServer; delete server; @@ -441,7 +434,6 @@ void XserverDesktop::setCursor(CursorPtr cursor) server->setCursor(cursor->bits->width, cursor->bits->height, Point(cursor->bits->xhot, cursor->bits->yhot), cursorData, cursorMask); - server->tryUpdate(); delete [] cursorData; delete [] cursorMask; } catch (rdr::Exception& e) { @@ -449,33 +441,6 @@ void XserverDesktop::setCursor(CursorPtr cursor) } } -CARD32 XserverDesktop::deferredUpdateTimerCallback(OsTimerPtr timer, - CARD32 now, pointer arg) -{ - XserverDesktop* desktop = (XserverDesktop*)arg; - desktop->deferredUpdateTimerSet = false; - try { - desktop->server->tryUpdate(); - } catch (rdr::Exception& e) { - vlog.error("XserverDesktop::deferredUpdateTimerCallback: %s",e.str()); - } - return 0; -} - -void XserverDesktop::deferUpdate() -{ - if (deferUpdateTime != 0) { - if (!deferredUpdateTimerSet || alwaysSetDeferUpdateTimer) { - deferredUpdateTimerSet = true; - deferredUpdateTimer = TimerSet(deferredUpdateTimer, 0, - deferUpdateTime, - deferredUpdateTimerCallback, this); - } - } else { - server->tryUpdate(); - } -} - void XserverDesktop::add_changed(RegionPtr reg) { if (ignoreHooks_) return; @@ -486,7 +451,6 @@ void XserverDesktop::add_changed(RegionPtr reg) REGION_NUM_RECTS(reg), (ShortRect*)REGION_RECTS(reg)); server->add_changed(rfbReg); - deferUpdate(); } catch (rdr::Exception& e) { vlog.error("XserverDesktop::add_changed: %s",e.str()); } @@ -502,7 +466,6 @@ void XserverDesktop::add_copied(RegionPtr dst, int dx, int dy) REGION_NUM_RECTS(dst), (ShortRect*)REGION_RECTS(dst)); server->add_copied(rfbReg, rfb::Point(dx, dy)); - deferUpdate(); } catch (rdr::Exception& e) { vlog.error("XserverDesktop::add_copied: %s",e.str()); } diff --git a/unix/xserver/hw/vnc/XserverDesktop.h b/unix/xserver/hw/vnc/XserverDesktop.h index 4a1705ea..39045835 100644 --- a/unix/xserver/hw/vnc/XserverDesktop.h +++ b/unix/xserver/hw/vnc/XserverDesktop.h @@ -121,12 +121,8 @@ public: private: void setColourMapEntries(int firstColour, int nColours); - static CARD32 deferredUpdateTimerCallback(OsTimerPtr timer, CARD32 now, - pointer arg); - void deferUpdate(); ScreenPtr pScreen; InputDevice *inputDevice; - OsTimerPtr deferredUpdateTimer; rfb::VNCServerST* server; rfb::HTTPServer* httpServer; network::TcpListener* listener; diff --git a/win/rfb_win32/SDisplay.cxx b/win/rfb_win32/SDisplay.cxx index 05ddce34..583b4aba 100644 --- a/win/rfb_win32/SDisplay.cxx +++ b/win/rfb_win32/SDisplay.cxx @@ -378,8 +378,6 @@ SDisplay::processEvent(HANDLE event) { // - Only process updates if the server is ready if (server) { - bool try_update = false; - // - Check that the SDesktop doesn't need restarting if (isRestartRequired()) { restartCore(); @@ -411,16 +409,12 @@ SDisplay::processEvent(HANDLE event) { // NB: First translate from Screen coordinates to Desktop Point desktopPos = info.position.translate(screenRect.tl.negate()); server->setCursorPos(desktopPos); - try_update = true; old_cursor = info; } // Flush any changes to the server - try_update = flushChangeTracker() || try_update; - if (try_update) { - server->tryUpdate(); - } + flushChangeTracker(); } return; }