Move update request handling in to CConnection

It's a generic client thing, so abstract it in to the common library.
Makes it easier to integrate with other common code.

common/rfb/CConnection.cxx

@@ -43,6 +43,10 @@ CConnection::CConnection()
   : csecurity(0), is(0), os(0), reader_(0), writer_(0),
     shared(false),
     state_(RFBSTATE_UNINITIALISED), useProtocol3_3(false),
+    pendingPFChange(false), preferredEncoding(encodingTight),
+    formatChange(false), encodingChange(false),
+    firstUpdate(true), pendingUpdate(false), continuousUpdates(false),
+    forceNonincremental(true),
     framebuffer(NULL), decoder(this)
 {
 }
@@ -324,6 +328,11 @@ void CConnection::setDesktopSize(int w, int h)
   decoder.flush();
 
   CMsgHandler::setDesktopSize(w,h);
+
+  if (continuousUpdates)
+    writer()->writeEnableContinuousUpdates(true, 0, 0,
+                                           server.width(),
+                                           server.height());
 }
 
 void CConnection::setExtendedDesktopSize(unsigned reason,
@@ -334,6 +343,27 @@ void CConnection::setExtendedDesktopSize(unsigned reason,
   decoder.flush();
 
   CMsgHandler::setExtendedDesktopSize(reason, result, w, h, layout);
+
+  if (continuousUpdates)
+    writer()->writeEnableContinuousUpdates(true, 0, 0,
+                                           server.width(),
+                                           server.height());
+}
+
+void CConnection::endOfContinuousUpdates()
+{
+  CMsgHandler::endOfContinuousUpdates();
+
+  // We've gotten the marker for a format change, so make the pending
+  // one active
+  if (pendingPFChange) {
+    server.setPF(pendingPF);
+    pendingPFChange = false;
+
+    // We might have another change pending
+    if (formatChange)
+      requestNewUpdate();
+  }
 }
 
 void CConnection::serverInit(int width, int height,
@@ -349,6 +379,18 @@ void CConnection::serverInit(int width, int height,
   assert(framebuffer != NULL);
   assert(framebuffer->width() == server.width());
   assert(framebuffer->height() == server.height());
+
+  // We want to make sure we call SetEncodings at least once
+  encodingChange = true;
+
+  requestNewUpdate();
+
+  // This initial update request is a bit of a corner case, so we need
+  // to help out setting the correct format here.
+  if (pendingPFChange) {
+    server.setPF(pendingPF);
+    pendingPFChange = false;
+  }
 }
 
 void CConnection::readAndDecodeRect(const Rect& r, int encoding,
@@ -361,6 +403,13 @@ void CConnection::readAndDecodeRect(const Rect& r, int encoding,
 void CConnection::framebufferUpdateStart()
 {
   CMsgHandler::framebufferUpdateStart();
+
+  assert(framebuffer != NULL);
+
+  // Note: This might not be true if continuous updates are supported
+  pendingUpdate = false;
+
+  requestNewUpdate();
 }
 
 void CConnection::framebufferUpdateEnd()
@@ -368,6 +417,25 @@ void CConnection::framebufferUpdateEnd()
   decoder.flush();
 
   CMsgHandler::framebufferUpdateEnd();
+
+  // A format change has been scheduled and we are now past the update
+  // with the old format. Time to active the new one.
+  if (pendingPFChange && !continuousUpdates) {
+    server.setPF(pendingPF);
+    pendingPFChange = false;
+  }
+
+  if (firstUpdate) {
+    if (server.supportsContinuousUpdates) {
+      vlog.info("Enabling continuous updates");
+      continuousUpdates = true;
+      writer()->writeEnableContinuousUpdates(true, 0, 0,
+                                             server.width(),
+                                             server.height());
+    }
+
+    firstUpdate = false;
+  }
 }
 
 void CConnection::dataRect(const Rect& r, int encoding)
@@ -383,6 +451,57 @@ void CConnection::initDone()
 {
 }
 
+void CConnection::refreshFramebuffer()
+{
+  forceNonincremental = true;
+
+  // Without continuous updates we have to make sure we only have a
+  // single update in flight, so we'll have to wait to do the refresh
+  if (continuousUpdates)
+    requestNewUpdate();
+}
+
+void CConnection::setPreferredEncoding(int encoding)
+{
+  if (preferredEncoding == encoding)
+    return;
+
+  preferredEncoding = encoding;
+  encodingChange = true;
+}
+
+int CConnection::getPreferredEncoding()
+{
+  return preferredEncoding;
+}
+
+void CConnection::setCompressLevel(int level)
+{
+  if (server.compressLevel == level)
+    return;
+
+  server.compressLevel = level;
+  encodingChange = true;
+}
+
+void CConnection::setQualityLevel(int level)
+{
+  if (server.qualityLevel == level)
+    return;
+
+  server.qualityLevel = level;
+  encodingChange = true;
+}
+
+void CConnection::setPF(const PixelFormat& pf)
+{
+  if (server.pf().equal(pf) && !formatChange)
+    return;
+
+  nextPF = pf;
+  formatChange = true;
+}
+
 void CConnection::fence(rdr::U32 flags, unsigned len, const char data[])
 {
   CMsgHandler::fence(flags, len, data);
@@ -395,3 +514,50 @@ void CConnection::fence(rdr::U32 flags, unsigned len, const char data[])
 
   writer()->writeFence(flags, len, data);
 }
+
+// requestNewUpdate() requests an update from the server, having set the
+// format and encoding appropriately.
+void CConnection::requestNewUpdate()
+{
+  if (formatChange && !pendingPFChange) {
+    /* Catch incorrect requestNewUpdate calls */
+    assert(!pendingUpdate || continuousUpdates);
+
+    // We have to make sure we switch the internal format at a safe
+    // time. For continuous updates we temporarily disable updates and
+    // look for a EndOfContinuousUpdates message to see when to switch.
+    // For classical updates we just got a new update right before this
+    // function was called, so we need to make sure we finish that
+    // update before we can switch.
+
+    pendingPFChange = true;
+    pendingPF = nextPF;
+
+    if (continuousUpdates)
+      writer()->writeEnableContinuousUpdates(false, 0, 0, 0, 0);
+
+    writer()->writeSetPixelFormat(pendingPF);
+
+    if (continuousUpdates)
+      writer()->writeEnableContinuousUpdates(true, 0, 0,
+                                             server.width(),
+                                             server.height());
+
+    formatChange = false;
+  }
+
+  if (encodingChange) {
+    writer()->writeSetEncodings(preferredEncoding, true);
+    encodingChange = false;
+  }
+
+  if (forceNonincremental || !continuousUpdates) {
+    pendingUpdate = true;
+    writer()->writeFramebufferUpdateRequest(Rect(0, 0,
+                                                 server.width(),
+                                                 server.height()),
+                                            !forceNonincremental);
+  }
+
+  forceNonincremental = false;
+}

common/rfb/CConnection.h

@@ -100,6 +100,8 @@ namespace rfb {
                                         int w, int h,
                                         const ScreenSet& layout);
 
+    virtual void endOfContinuousUpdates();
+
     virtual void serverInit(int width, int height,
                             const PixelFormat& pf,
                             const char* name);
@@ -128,6 +130,24 @@ namespace rfb {
 
     // Other methods
 
+    // refreshFramebuffer() forces a complete refresh of the entire
+    // framebuffer
+    void refreshFramebuffer();
+
+    // setPreferredEncoding()/getPreferredEncoding() adjusts which
+    // encoding is listed first as a hint to the server that it is the
+    // preferred one
+    void setPreferredEncoding(int encoding);
+    int getPreferredEncoding();
+    // setCompressLevel()/setQualityLevel() controls the encoding hints
+    // sent to the server
+    void setCompressLevel(int level);
+    void setQualityLevel(int level);
+    // setPF() controls the pixel format requested from the server.
+    // server.pf() will automatically be adjusted once the new format
+    // is active.
+    void setPF(const PixelFormat& pf);
+
     CMsgReader* reader() { return reader_; }
     CMsgWriter* writer() { return writer_; }
 
@@ -180,6 +200,8 @@ namespace rfb {
     void throwConnFailedException();
     void securityCompleted();
 
+    void requestNewUpdate();
+
     rdr::InStream* is;
     rdr::OutStream* os;
     CMsgReader* reader_;
@@ -192,6 +214,21 @@ namespace rfb {
 
     bool useProtocol3_3;
 
+    bool pendingPFChange;
+    rfb::PixelFormat pendingPF;
+
+    int preferredEncoding;
+
+    bool formatChange;
+    rfb::PixelFormat nextPF;
+    bool encodingChange;
+
+    bool firstUpdate;
+    bool pendingUpdate;
+    bool continuousUpdates;
+
+    bool forceNonincremental;
+
     ModifiablePixelBuffer* framebuffer;
     DecodeManager decoder;
   };

vncviewer/CConn.cxx

@@ -74,19 +74,12 @@ static const PixelFormat mediumColourPF(8, 8, false, true,
 
 CConn::CConn(const char* vncServerName, network::Socket* socket=NULL)
   : serverHost(0), serverPort(0), desktop(NULL),
-    updateCount(0), pixelCount(0), pendingPFChange(false),
-    currentEncoding(encodingTight), lastServerEncoding((unsigned int)-1),
-    formatChange(false), encodingChange(false),
-    firstUpdate(true), pendingUpdate(false), continuousUpdates(false),
-    forceNonincremental(true)
+    updateCount(0), pixelCount(0),
+    lastServerEncoding((unsigned int)-1)
 {
   setShared(::shared);
   sock = socket;
 
-  int encNum = encodingNum(preferredEncoding);
-  if (encNum != -1)
-    currentEncoding = encNum;
-
   server.supportsLocalCursor = true;
 
   server.supportsDesktopResize = true;
@@ -96,14 +89,10 @@ CConn::CConn(const char* vncServerName, network::Socket* socket=NULL)
   server.supportsLEDState = true;
 
   if (customCompressLevel)
-    server.compressLevel = compressLevel;
-  else
-    server.compressLevel = -1;
+    setCompressLevel(compressLevel);
 
   if (!noJpeg)
-    server.qualityLevel = qualityLevel;
-  else
-    server.qualityLevel = -1;
+    setQualityLevel(qualityLevel);
 
   if(sock == NULL) {
     try {
@@ -156,16 +145,6 @@ CConn::~CConn()
   delete sock;
 }
 
-void CConn::refreshFramebuffer()
-{
-  forceNonincremental = true;
-
-  // Without continuous updates we have to make sure we only have a
-  // single update in flight, so we'll have to wait to do the refresh
-  if (continuousUpdates)
-    requestNewUpdate();
-}
-
 const char *CConn::connectionInfo()
 {
   static char infoText[1024] = "";
@@ -209,7 +188,7 @@ const char *CConn::connectionInfo()
   strcat(infoText, "\n");
 
   snprintf(scratch, sizeof(scratch),
-           _("Requested encoding: %s"), encodingName(currentEncoding));
+           _("Requested encoding: %s"), encodingName(getPreferredEncoding()));
   strcat(infoText, scratch);
   strcat(infoText, "\n");
 
@@ -325,16 +304,10 @@ void CConn::initDone()
   fullColourPF = desktop->getPreferredPF();
 
   // Force a switch to the format and encoding we'd like
-  formatChange = encodingChange = true;
-
-  // And kick off the update cycle
-  requestNewUpdate();
-
-  // This initial update request is a bit of a corner case, so we need
-  // to help out setting the correct format here.
-  assert(pendingPFChange);
-  server.setPF(pendingPF);
-  pendingPFChange = false;
+  updatePixelFormat();
+  int encNum = encodingNum(::preferredEncoding);
+  if (encNum != -1)
+    setPreferredEncoding(encNum);
 }
 
 // setDesktopSize() is called when the desktop size changes (including when
@@ -374,11 +347,6 @@ void CConn::framebufferUpdateStart()
 {
   CConnection::framebufferUpdateStart();
 
-  // Note: This might not be true if sync fences are supported
-  pendingUpdate = false;
-
-  requestNewUpdate();
-
   // Update the screen prematurely for very slow updates
   Fl::add_timeout(1.0, handleUpdateTimeout, this);
 }
@@ -396,25 +364,6 @@ void CConn::framebufferUpdateEnd()
   Fl::remove_timeout(handleUpdateTimeout, this);
   desktop->updateWindow();
 
-  // A format change has been scheduled and we are now past the update
-  // with the old format. Time to active the new one.
-  if (pendingPFChange && !continuousUpdates) {
-    server.setPF(pendingPF);
-    pendingPFChange = false;
-  }
-
-  if (firstUpdate) {
-    if (server.supportsContinuousUpdates) {
-      vlog.info(_("Enabling continuous updates"));
-      continuousUpdates = true;
-      writer()->writeEnableContinuousUpdates(true, 0, 0,
-                                             server.width(),
-                                             server.height());
-    }
-
-    firstUpdate = false;
-  }
-
   // Compute new settings based on updated bandwidth values
   if (autoSelect)
     autoSelectFormatAndEncoding();
@@ -470,18 +419,6 @@ void CConn::fence(rdr::U32 flags, unsigned len, const char data[])
   }
 }
 
-void CConn::endOfContinuousUpdates()
-{
-  CConnection::endOfContinuousUpdates();
-
-  // We've gotten the marker for a format change, so make the pending
-  // one active
-  if (pendingPFChange) {
-    server.setPF(pendingPF);
-    pendingPFChange = false;
-  }
-}
-
 void CConn::setLEDState(unsigned int state)
 {
   CConnection::setLEDState(state);
@@ -494,11 +431,6 @@ void CConn::setLEDState(unsigned int state)
 
 void CConn::resizeFramebuffer()
 {
-  if (continuousUpdates)
-    writer()->writeEnableContinuousUpdates(true, 0, 0,
-                                           server.width(),
-                                           server.height());
-
   desktop->resizeFramebuffer(server.width(), server.height());
 }
 
@@ -525,10 +457,7 @@ void CConn::autoSelectFormatAndEncoding()
   int newQualityLevel = qualityLevel;
 
   // Always use Tight
-  if (currentEncoding != encodingTight) {
-    currentEncoding = encodingTight;
-    encodingChange = true;
-  }
+  setPreferredEncoding(encodingTight);
 
   // Check that we have a decent bandwidth measurement
   if ((kbitsPerSecond == 0) || (timeWaited < 10000))
@@ -544,9 +473,8 @@ void CConn::autoSelectFormatAndEncoding()
     if (newQualityLevel != qualityLevel) {
       vlog.info(_("Throughput %d kbit/s - changing to quality %d"),
                 kbitsPerSecond, newQualityLevel);
-      server.qualityLevel = newQualityLevel;
       qualityLevel.setParam(newQualityLevel);
-      encodingChange = true;
+      setQualityLevel(newQualityLevel);
     }
   }
 
@@ -571,78 +499,31 @@ void CConn::autoSelectFormatAndEncoding()
       vlog.info(_("Throughput %d kbit/s - full color is now disabled"),
                 kbitsPerSecond);
     fullColour.setParam(newFullColour);
-    formatChange = true;
+    updatePixelFormat();
   } 
 }
 
-// checkEncodings() sends a setEncodings message if one is needed.
-void CConn::checkEncodings()
-{
-  if (encodingChange && writer()) {
-    vlog.info(_("Using %s encoding"),encodingName(currentEncoding));
-    writer()->writeSetEncodings(currentEncoding, true);
-    encodingChange = false;
-  }
-}
-
 // requestNewUpdate() requests an update from the server, having set the
 // format and encoding appropriately.
-void CConn::requestNewUpdate()
+void CConn::updatePixelFormat()
 {
-  if (formatChange && !pendingPFChange) {
-    PixelFormat pf;
-
-    /* Catch incorrect requestNewUpdate calls */
-    assert(!pendingUpdate || continuousUpdates);
-
-    if (fullColour) {
-      pf = fullColourPF;
-    } else {
-      if (lowColourLevel == 0)
-        pf = verylowColourPF;
-      else if (lowColourLevel == 1)
-        pf = lowColourPF;
-      else
-        pf = mediumColourPF;
-    }
-
-    // We have to make sure we switch the internal format at a safe
-    // time. For continuous updates we temporarily disable updates and
-    // look for a EndOfContinuousUpdates message to see when to switch.
-    // For classical updates we just got a new update right before this
-    // function was called, so we need to make sure we finish that
-    // update before we can switch.
-
-    pendingPFChange = true;
-    pendingPF = pf;
-
-    if (continuousUpdates)
-      writer()->writeEnableContinuousUpdates(false, 0, 0, 0, 0);
-
-    char str[256];
-    pf.print(str, 256);
-    vlog.info(_("Using pixel format %s"),str);
-    writer()->writeSetPixelFormat(pf);
-
-    if (continuousUpdates)
-      writer()->writeEnableContinuousUpdates(true, 0, 0,
-                                             server.width(),
-                                             server.height());
+  PixelFormat pf;
 
-    formatChange = false;
+  if (fullColour) {
+    pf = fullColourPF;
+  } else {
+    if (lowColourLevel == 0)
+      pf = verylowColourPF;
+    else if (lowColourLevel == 1)
+      pf = lowColourPF;
+    else
+      pf = mediumColourPF;
   }
 
-  checkEncodings();
-
-  if (forceNonincremental || !continuousUpdates) {
-    pendingUpdate = true;
-    writer()->writeFramebufferUpdateRequest(Rect(0, 0,
-                                                 server.width(),
-                                                 server.height()),
-                                            !forceNonincremental);
-  }
- 
-  forceNonincremental = false;
+  char str[256];
+  pf.print(str, 256);
+  vlog.info(_("Using pixel format %s"),str);
+  setPF(pf);
 }
 
 void CConn::handleOptions(void *data)
@@ -654,50 +535,23 @@ void CConn::handleOptions(void *data)
   // list is cheap. Avoid overriding what the auto logic has selected
   // though.
   if (!autoSelect) {
-    int encNum = encodingNum(preferredEncoding);
+    int encNum = encodingNum(::preferredEncoding);
 
     if (encNum != -1)
-      self->currentEncoding = encNum;
+      self->setPreferredEncoding(encNum);
   }
 
-  self->server.supportsLocalCursor = true;
-
   if (customCompressLevel)
-    self->server.compressLevel = compressLevel;
+    self->setCompressLevel(compressLevel);
   else
-    self->server.compressLevel = -1;
+    self->setCompressLevel(-1);
 
   if (!noJpeg && !autoSelect)
-    self->server.qualityLevel = qualityLevel;
+    self->setQualityLevel(qualityLevel);
   else
-    self->server.qualityLevel = -1;
+    self->setQualityLevel(-1);
 
-  self->encodingChange = true;
-
-  // Format changes refreshes the entire screen though and are therefore
-  // very costly. It's probably worth the effort to see if it is necessary
-  // here.
-  PixelFormat pf;
-
-  if (fullColour) {
-    pf = self->fullColourPF;
-  } else {
-    if (lowColourLevel == 0)
-      pf = verylowColourPF;
-    else if (lowColourLevel == 1)
-      pf = lowColourPF;
-    else
-      pf = mediumColourPF;
-  }
-
-  if (!pf.equal(self->server.pf())) {
-    self->formatChange = true;
-
-    // Without fences, we cannot safely trigger an update request directly
-    // but must wait for the next update to arrive.
-    if (self->continuousUpdates)
-      self->requestNewUpdate();
-  }
+  self->updatePixelFormat();
 }
 
 void CConn::handleUpdateTimeout(void *data)

vncviewer/CConn.h

@@ -36,8 +36,6 @@ public:
   CConn(const char* vncServerName, network::Socket* sock);
   ~CConn();
 
-  void refreshFramebuffer();
-
   const char *connectionInfo();
 
   unsigned getUpdateCount();
@@ -74,8 +72,6 @@ public:
 
   void fence(rdr::U32 flags, unsigned len, const char data[]);
 
-  void endOfContinuousUpdates();
-
   void setLEDState(unsigned int state);
 
 private:
@@ -83,8 +79,7 @@ private:
   void resizeFramebuffer();
 
   void autoSelectFormatAndEncoding();
-  void checkEncodings();
-  void requestNewUpdate();
+  void updatePixelFormat();
 
   static void handleOptions(void *data);
 
@@ -103,19 +98,7 @@ private:
   rfb::PixelFormat serverPF;
   rfb::PixelFormat fullColourPF;
 
-  bool pendingPFChange;
-  rfb::PixelFormat pendingPF;
-
-  int currentEncoding, lastServerEncoding;
-
-  bool formatChange;
-  bool encodingChange;
-
-  bool firstUpdate;
-  bool pendingUpdate;
-  bool continuousUpdates;
-
-  bool forceNonincremental;
+  int lastServerEncoding;
 };
 
 #endif