]> source.dussan.org Git - tigervnc.git/commitdiff
Encapsulate screen layout storage in ConnParams
authorPierre Ossman <ossman@cendio.se>
Wed, 20 Jun 2018 10:25:14 +0000 (12:25 +0200)
committerPierre Ossman <ossman@cendio.se>
Thu, 1 Nov 2018 15:11:42 +0000 (16:11 +0100)
Avoid direct access to the screen dimensions and layout so that we
can make sure it stays sane. This also makes sure the layout is
properly updated when we only get the screen dimensions from the
server.

12 files changed:
common/rfb/CMsgHandler.cxx
common/rfb/CMsgReader.cxx
common/rfb/CMsgWriter.cxx
common/rfb/ConnParams.cxx
common/rfb/ConnParams.h
common/rfb/SMsgHandler.cxx
common/rfb/SMsgWriter.cxx
common/rfb/VNCSConnectionST.cxx
tests/decperf.cxx
tests/encperf.cxx
vncviewer/CConn.cxx
vncviewer/DesktopWindow.cxx

index b89bc1842b93c92eda0f4e15e4211b7c3c462bab..03e66e83f95d0309cdbadf83a0915850a29c3930 100644 (file)
@@ -34,8 +34,7 @@ CMsgHandler::~CMsgHandler()
 
 void CMsgHandler::setDesktopSize(int width, int height)
 {
-  cp.width = width;
-  cp.height = height;
+  cp.setDimensions(width, height);
 }
 
 void CMsgHandler::setExtendedDesktopSize(unsigned reason, unsigned result,
@@ -47,12 +46,7 @@ void CMsgHandler::setExtendedDesktopSize(unsigned reason, unsigned result,
   if ((reason == reasonClient) && (result != resultSuccess))
     return;
 
-  if (!layout.validate(width, height))
-    fprintf(stderr, "Server sent us an invalid screen layout\n");
-
-  cp.width = width;
-  cp.height = height;
-  cp.screenLayout = layout;
+  cp.setDimensions(width, height, layout);
 }
 
 void CMsgHandler::setPixelFormat(const PixelFormat& pf)
index 1d359d2c74e22b1a682e53e037d48281fd969036..7502df6a5fe2d353b9fb7cd73d0a8d0389b6db50 100644 (file)
@@ -192,10 +192,10 @@ void CMsgReader::readFramebufferUpdate()
 
 void CMsgReader::readRect(const Rect& r, int encoding)
 {
-  if ((r.br.x > handler->cp.width) || (r.br.y > handler->cp.height)) {
+  if ((r.br.x > handler->cp.width()) || (r.br.y > handler->cp.height())) {
     fprintf(stderr, "Rect too big: %dx%d at %d,%d exceeds %dx%d\n",
            r.width(), r.height(), r.tl.x, r.tl.y,
-            handler->cp.width, handler->cp.height);
+            handler->cp.width(), handler->cp.height());
     throw Exception("Rect too big");
   }
 
index 44b73dab89133e60cdf9a9d540964661bd324978..97c33363b1c4f5a142822cced6be3301733dd3a9 100644 (file)
@@ -245,8 +245,8 @@ void CMsgWriter::writePointerEvent(const Point& pos, int buttonMask)
   Point p(pos);
   if (p.x < 0) p.x = 0;
   if (p.y < 0) p.y = 0;
-  if (p.x >= cp->width) p.x = cp->width - 1;
-  if (p.y >= cp->height) p.y = cp->height - 1;
+  if (p.x >= cp->width()) p.x = cp->width() - 1;
+  if (p.y >= cp->height()) p.y = cp->height() - 1;
 
   startMsg(msgTypePointerEvent);
   os->writeU8(buttonMask);
index 405a99ccfa1c5da902a623d3c92a8132db258ba1..1fdf8f38c161f1058cd697333d2928ff3eb0cfea 100644 (file)
@@ -28,7 +28,7 @@ using namespace rfb;
 
 ConnParams::ConnParams()
   : majorVersion(0), minorVersion(0),
-    width(0), height(0), useCopyRect(false),
+    useCopyRect(false),
     supportsLocalCursor(false), supportsLocalXCursor(false),
     supportsLocalCursorWithAlpha(false),
     supportsDesktopResize(false), supportsExtendedDesktopSize(false),
@@ -37,7 +37,8 @@ ConnParams::ConnParams()
     supportsSetDesktopSize(false), supportsFence(false),
     supportsContinuousUpdates(false),
     compressLevel(2), qualityLevel(-1), fineQualityLevel(-1),
-    subsampling(subsampleUndefined), name_(0),
+    subsampling(subsampleUndefined),
+    width_(0), height_(0), name_(0),
     ledState_(ledUnknown)
 {
   setName("");
@@ -50,6 +51,23 @@ ConnParams::~ConnParams()
   delete cursor_;
 }
 
+void ConnParams::setDimensions(int width, int height)
+{
+  ScreenSet layout;
+  layout.add_screen(rfb::Screen(0, 0, 0, width, height, 0));
+  setDimensions(width, height, layout);
+}
+
+void ConnParams::setDimensions(int width, int height, const ScreenSet& layout)
+{
+  if (!layout.validate(width, height))
+    throw Exception("Attempted to configure an invalid screen layout");
+
+  width_ = width;
+  height_ = height;
+  screenLayout_ = layout;
+}
+
 void ConnParams::setPF(const PixelFormat& pf)
 {
   pf_ = pf;
index b56c94074b1f86bc0908c5ca15767245399d1b0f..1640efccb5beeb252826c8abe11579ad8d8fc2fa 100644 (file)
@@ -62,9 +62,11 @@ namespace rfb {
       return !beforeVersion(major,minor+1);
     }
 
-    int width;
-    int height;
-    ScreenSet screenLayout;
+    const int width() const { return width_; }
+    const int height() const { return height_; }
+    const ScreenSet& screenLayout() const { return screenLayout_; }
+    void setDimensions(int width, int height);
+    void setDimensions(int width, int height, const ScreenSet& layout);
 
     const PixelFormat& pf() const { return pf_; }
     void setPF(const PixelFormat& pf);
@@ -105,6 +107,10 @@ namespace rfb {
 
   private:
 
+    int width_;
+    int height_;
+    ScreenSet screenLayout_;
+
     PixelFormat pf_;
     char* name_;
     Cursor* cursor_;
index c38458c3df96f0adbb1cfc72903e1af2fe1c3f63..137734b3d75715cb4ce38936e894ff4b27d5345d 100644 (file)
@@ -86,8 +86,6 @@ void SMsgHandler::supportsQEMUKeyEvent()
 void SMsgHandler::setDesktopSize(int fb_width, int fb_height,
                                  const ScreenSet& layout)
 {
-  cp.width = fb_width;
-  cp.height = fb_height;
-  cp.screenLayout = layout;
+  cp.setDimensions(fb_width, fb_height, layout);
 }
 
index 3da9413f4da18a25bb5e94d4bff4a84ac92b587b..96df6533e82361b0f3f89bef30931ad13eada85b 100644 (file)
@@ -49,8 +49,8 @@ SMsgWriter::~SMsgWriter()
 
 void SMsgWriter::writeServerInit()
 {
-  os->writeU16(cp->width);
-  os->writeU16(cp->height);
+  os->writeU16(cp->width());
+  os->writeU16(cp->height());
   cp->pf().write(os);
   os->writeString(cp->name());
   endMsg();
@@ -422,15 +422,15 @@ void SMsgWriter::writeNoDataRects()
 
   // Send this before SetDesktopSize to make life easier on the clients
   if (needExtendedDesktopSize) {
-    writeExtendedDesktopSizeRect(0, 0, cp->width, cp->height,
-                                 cp->screenLayout);
+    writeExtendedDesktopSizeRect(0, 0, cp->width(), cp->height(),
+                                 cp->screenLayout());
     needExtendedDesktopSize = false;
   }
 
   // Some clients assume this is the last rectangle so don't send anything
   // more after this
   if (needSetDesktopSize) {
-    writeSetDesktopSizeRect(cp->width, cp->height);
+    writeSetDesktopSizeRect(cp->width(), cp->height());
     needSetDesktopSize = false;
   }
 }
index f1591f4c2939f4c7d7e3731c2edbf2c05aeb90e9..4dd00354bb84ca795d45a066c7322f1fcf189997 100644 (file)
@@ -192,8 +192,9 @@ void VNCSConnectionST::pixelBufferChange()
 {
   try {
     if (!authenticated()) return;
-    if (cp.width && cp.height && (server->pb->width() != cp.width ||
-                                  server->pb->height() != cp.height))
+    if (cp.width() && cp.height() &&
+        (server->pb->width() != cp.width() ||
+         server->pb->height() != cp.height()))
     {
       // We need to clip the next update to the new size, but also add any
       // extra bits if it's bigger.  If we wanted to do this exactly, something
@@ -203,18 +204,17 @@ void VNCSConnectionST::pixelBufferChange()
 
       //updates.intersect(server->pb->getRect());
       //
-      //if (server->pb->width() > cp.width)
-      //  updates.add_changed(Rect(cp.width, 0, server->pb->width(),
+      //if (server->pb->width() > cp.width())
+      //  updates.add_changed(Rect(cp.width(), 0, server->pb->width(),
       //                           server->pb->height()));
-      //if (server->pb->height() > cp.height)
-      //  updates.add_changed(Rect(0, cp.height, cp.width,
+      //if (server->pb->height() > cp.height())
+      //  updates.add_changed(Rect(0, cp.height(), client.width(),
       //                           server->pb->height()));
 
       damagedCursorRegion.assign_intersect(server->pb->getRect());
 
-      cp.width = server->pb->width();
-      cp.height = server->pb->height();
-      cp.screenLayout = server->screenLayout;
+      cp.setDimensions(server->pb->width(), server->pb->height(),
+                       server->screenLayout);
       if (state() == RFBSTATE_NORMAL) {
         // We should only send EDS to client asking for both
         if (!writer()->writeExtendedDesktopSize()) {
@@ -417,9 +417,8 @@ void VNCSConnectionST::authSuccess()
   server->startDesktop();
 
   // - Set the connection parameters appropriately
-  cp.width = server->pb->width();
-  cp.height = server->pb->height();
-  cp.screenLayout = server->screenLayout;
+  cp.setDimensions(server->pb->width(), server->pb->height(),
+                   server->screenLayout);
   cp.setName(server->getName());
   cp.setLEDState(server->ledState);
   
@@ -678,10 +677,11 @@ void VNCSConnectionST::framebufferUpdateRequest(const Rect& r,bool incremental)
   SConnection::framebufferUpdateRequest(r, incremental);
 
   // Check that the client isn't sending crappy requests
-  if (!r.enclosed_by(Rect(0, 0, cp.width, cp.height))) {
+  if (!r.enclosed_by(Rect(0, 0, cp.width(), cp.height()))) {
     vlog.error("FramebufferUpdateRequest %dx%d at %d,%d exceeds framebuffer %dx%d",
-               r.width(), r.height(), r.tl.x, r.tl.y, cp.width, cp.height);
-    safeRect = r.intersect(Rect(0, 0, cp.width, cp.height));
+               r.width(), r.height(), r.tl.x, r.tl.y,
+               cp.width(), cp.height());
+    safeRect = r.intersect(Rect(0, 0, cp.width(), cp.height()));
   } else {
     safeRect = r;
   }
@@ -1124,13 +1124,15 @@ void VNCSConnectionST::screenLayoutChange(rdr::U16 reason)
   if (!authenticated())
     return;
 
-  cp.screenLayout = server->screenLayout;
+  cp.setDimensions(cp.width(), cp.height(),
+                   server->screenLayout);
 
   if (state() != RFBSTATE_NORMAL)
     return;
 
-  writer()->writeExtendedDesktopSize(reason, 0, cp.width, cp.height,
-                                     cp.screenLayout);
+  writer()->writeExtendedDesktopSize(reason, 0,
+                                     cp.width(), cp.height(),
+                                     cp.screenLayout());
 }
 
 
index 9061cb5353af0965d8feb180b471cceca8b32c16..24f660429fab2958bb6feda825edb5a2271f98bc 100644 (file)
@@ -85,7 +85,7 @@ void CConn::setDesktopSize(int w, int h)
 {
   CConnection::setDesktopSize(w, h);
 
-  setFramebuffer(new rfb::ManagedPixelBuffer(filePF, cp.width, cp.height));
+  setFramebuffer(new rfb::ManagedPixelBuffer(filePF, cp.width(), cp.height()));
 }
 
 void CConn::setPixelFormat(const rfb::PixelFormat& pf)
index 4e7038fdd5bb5c86eda4c3f2f3401533fcc7be45..a232fb925d4bccb4149e809a237fb57f0ad2d200 100644 (file)
@@ -203,7 +203,7 @@ void CConn::setDesktopSize(int w, int h)
   CConnection::setDesktopSize(w, h);
 
   pb = new rfb::ManagedPixelBuffer((bool)translate ? fbPF : cp.pf(),
-                                   cp.width, cp.height);
+                                   cp.width(), cp.height());
   setFramebuffer(pb);
 }
 
index 69186c5584da6d54b7fbd4edb1d472d64f4aca80..e87ebcdef9849f3cf982d0b13674cfb40881c24a 100644 (file)
@@ -191,7 +191,7 @@ const char *CConn::connectionInfo()
   strcat(infoText, "\n");
 
   snprintf(scratch, sizeof(scratch),
-           _("Size: %d x %d"), cp.width, cp.height);
+           _("Size: %d x %d"), cp.width(), cp.height());
   strcat(infoText, scratch);
   strcat(infoText, "\n");
 
@@ -324,7 +324,8 @@ void CConn::serverInit()
 
   serverPF = cp.pf();
 
-  desktop = new DesktopWindow(cp.width, cp.height, cp.name(), serverPF, this);
+  desktop = new DesktopWindow(cp.width(), cp.height(),
+                              cp.name(), serverPF, this);
   fullColourPF = desktop->getPreferredPF();
 
   // Force a switch to the format and encoding we'd like
@@ -478,7 +479,8 @@ void CConn::fence(rdr::U32 flags, unsigned len, const char data[])
       if (cp.supportsContinuousUpdates) {
         vlog.info(_("Enabling continuous updates"));
         continuousUpdates = true;
-        writer()->writeEnableContinuousUpdates(true, 0, 0, cp.width, cp.height);
+        writer()->writeEnableContinuousUpdates(true, 0, 0,
+                                               cp.width(), cp.height());
       }
     }
   } else {
@@ -508,9 +510,10 @@ void CConn::resizeFramebuffer()
     return;
 
   if (continuousUpdates)
-    writer()->writeEnableContinuousUpdates(true, 0, 0, cp.width, cp.height);
+    writer()->writeEnableContinuousUpdates(true, 0, 0,
+                                           cp.width(), cp.height());
 
-  desktop->resizeFramebuffer(cp.width, cp.height);
+  desktop->resizeFramebuffer(cp.width(), cp.height());
 }
 
 // autoSelectFormatAndEncoding() chooses the format and encoding appropriate
@@ -647,7 +650,7 @@ void CConn::requestNewUpdate()
 
   if (forceNonincremental || !continuousUpdates) {
     pendingUpdate = true;
-    writer()->writeFramebufferUpdateRequest(Rect(0, 0, cp.width, cp.height),
+    writer()->writeFramebufferUpdateRequest(Rect(0, 0, cp.width(), cp.height()),
                                             !forceNonincremental);
   }
  
index d070b648113799476bf16c78fa2add5198d6447e..e6d0ca7cba85ea6ce7540102aec87048a4f8daaf 100644 (file)
@@ -1014,14 +1014,14 @@ void DesktopWindow::handleResizeTimeout(void *data)
 void DesktopWindow::remoteResize(int width, int height)
 {
   ScreenSet layout;
-  ScreenSet::iterator iter;
+  ScreenSet::const_iterator iter;
 
   if (!fullscreen_active() || (width > w()) || (height > h())) {
     // In windowed mode (or the framebuffer is so large that we need
     // to scroll) we just report a single virtual screen that covers
     // the entire framebuffer.
 
-    layout = cc->cp.screenLayout;
+    layout = cc->cp.screenLayout();
 
     // Not sure why we have no screens, but adding a new one should be
     // safe as there is nothing to conflict with...
@@ -1077,8 +1077,8 @@ void DesktopWindow::remoteResize(int width, int height)
       sy -= viewport_rect.tl.y;
 
       // Look for perfectly matching existing screen...
-      for (iter = cc->cp.screenLayout.begin();
-           iter != cc->cp.screenLayout.end(); ++iter) {
+      for (iter = cc->cp.screenLayout().begin();
+           iter != cc->cp.screenLayout().end(); ++iter) {
         if ((iter->dimensions.tl.x == sx) &&
             (iter->dimensions.tl.y == sy) &&
             (iter->dimensions.width() == sw) &&
@@ -1087,7 +1087,7 @@ void DesktopWindow::remoteResize(int width, int height)
       }
 
       // Found it?
-      if (iter != cc->cp.screenLayout.end()) {
+      if (iter != cc->cp.screenLayout().end()) {
         layout.add_screen(*iter);
         continue;
       }
@@ -1095,13 +1095,13 @@ void DesktopWindow::remoteResize(int width, int height)
       // Need to add a new one, which means we need to find an unused id
       while (true) {
         id = rand();
-        for (iter = cc->cp.screenLayout.begin();
-             iter != cc->cp.screenLayout.end(); ++iter) {
+        for (iter = cc->cp.screenLayout().begin();
+             iter != cc->cp.screenLayout().end(); ++iter) {
           if (iter->id == id)
             break;
         }
 
-        if (iter == cc->cp.screenLayout.end())
+        if (iter == cc->cp.screenLayout().end())
           break;
       }
 
@@ -1115,14 +1115,14 @@ void DesktopWindow::remoteResize(int width, int height)
   }
 
   // Do we actually change anything?
-  if ((width == cc->cp.width) &&
-      (height == cc->cp.height) &&
-      (layout == cc->cp.screenLayout))
+  if ((width == cc->cp.width()) &&
+      (height == cc->cp.height()) &&
+      (layout == cc->cp.screenLayout()))
     return;
 
   char buffer[2048];
   vlog.debug("Requesting framebuffer resize from %dx%d to %dx%d",
-             cc->cp.width, cc->cp.height, width, height);
+             cc->cp.width(), cc->cp.height(), width, height);
   layout.print(buffer, sizeof(buffer));
   vlog.debug("%s", buffer);