Suggested patch. It does the following:

- CUT block size changed to 64 pixels. This size in most cases has a
  CPU usage similar to 256 pixels, but a coverage more similar to 16
  pixels.

- Default compression level changed to 2. Based on your numbers, this
  seems to get us most of the compression, whilst avoiding most of the
  CPU usage (compared to level 3).

- CUT enabled by default. Will be disabled if all clients advertise a
  compression level of 1 or below. compareFB=0 will forcefully disable
  it at all times. There is currently no way to enable it as well as
  have a low compression setting.

Does this seem ok to you?

Rgds
-- 
Pierre Ossman            OpenSource-based Thin Client Technology
System Developer         Telephone: +46-13-21 46 00
Cendio AB                Web: http://www.cendio.com

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
Index: common/rfb/ComparingUpdateTracker.cxx
===================================================================
--- common/rfb/ComparingUpdateTracker.cxx	(revision 4807)
+++ common/rfb/ComparingUpdateTracker.cxx	(working copy)
@@ -25,7 +25,7 @@
 using namespace rfb;
 
 ComparingUpdateTracker::ComparingUpdateTracker(PixelBuffer* buffer)
-  : fb(buffer), oldFb(fb->getPF(), 0, 0), firstCompare(true)
+  : fb(buffer), oldFb(fb->getPF(), 0, 0), firstCompare(true), enabled(true)
 {
     changed.assign_union(fb->getRect());
 }
@@ -35,39 +35,64 @@
 }
 
 
-#define BLOCK_SIZE 16
+#define BLOCK_SIZE 64
 
-void ComparingUpdateTracker::compare()
+bool ComparingUpdateTracker::compare()
 {
   std::vector<Rect> rects;
   std::vector<Rect>::iterator i;
 
+  if (!enabled)
+    return false;
+
   if (firstCompare) {
     // NB: We leave the change region untouched on this iteration,
     // since in effect the entire framebuffer has changed.
     oldFb.setSize(fb->width(), fb->height());
+
     for (int y=0; y<fb->height(); y+=BLOCK_SIZE) {
       Rect pos(0, y, fb->width(), __rfbmin(fb->height(), y+BLOCK_SIZE));
       int srcStride;
       const rdr::U8* srcData = fb->getPixelsR(pos, &srcStride);
       oldFb.imageRect(pos, srcData, srcStride);
     }
+
     firstCompare = false;
-  } else {
-    copied.get_rects(&rects, copy_delta.x<=0, copy_delta.y<=0);
-    for (i = rects.begin(); i != rects.end(); i++)
-      oldFb.copyRect(*i, copy_delta);
 
-    changed.get_rects(&rects);
+    return false;
+  }
 
-    Region newChanged;
-    for (i = rects.begin(); i != rects.end(); i++)
-      compareRect(*i, &newChanged);
+  copied.get_rects(&rects, copy_delta.x<=0, copy_delta.y<=0);
+  for (i = rects.begin(); i != rects.end(); i++)
+    oldFb.copyRect(*i, copy_delta);
 
-    changed = newChanged;
-  }
+  changed.get_rects(&rects);
+
+  Region newChanged;
+  for (i = rects.begin(); i != rects.end(); i++)
+    compareRect(*i, &newChanged);
+
+  if (changed.equals(newChanged))
+    return false;
+
+  changed = newChanged;
+
+  return true;
 }
 
+void ComparingUpdateTracker::enable()
+{
+  enabled = true;
+}
+
+void ComparingUpdateTracker::disable()
+{
+  enabled = false;
+
+  // Make sure we update the framebuffer next time we get enabled
+  firstCompare = true;
+}
+
 void ComparingUpdateTracker::compareRect(const Rect& r, Region* newChanged)
 {
   if (!r.enclosed_by(fb->getRect())) {
Index: common/rfb/VNCServerST.h
===================================================================
--- common/rfb/VNCServerST.h	(revision 4807)
+++ common/rfb/VNCServerST.h	(working copy)
@@ -238,6 +238,8 @@
 
     void notifyScreenLayoutChange(VNCSConnectionST *requester);
 
+    bool getComparerState();
+
     QueryConnectionHandler* queryConnectionHandler;
     KeyRemapper* keyRemapper;
     bool useEconomicTranslate;
Index: common/rfb/ComparingUpdateTracker.h
===================================================================
--- common/rfb/ComparingUpdateTracker.h	(revision 4807)
+++ common/rfb/ComparingUpdateTracker.h	(working copy)
@@ -29,14 +29,23 @@
     ~ComparingUpdateTracker();
 
     // compare() does the comparison and reduces its changed and copied regions
-    // as appropriate.
+    // as appropriate. Returns true if the regions were altered.
 
-    virtual void compare();
+    virtual bool compare();
+
+    // enable()/disable() turns the comparing functionality on/off. With it
+    // disabled, the object will behave like a dumb update tracker (i.e.
+    // compare() will be a no-op). It is harmless to repeatedly call these
+    // methods.
+
+    virtual void enable();
+    virtual void disable();
   private:
     void compareRect(const Rect& r, Region* newchanged);
     PixelBuffer* fb;
     ManagedPixelBuffer oldFb;
     bool firstCompare;
+    bool enabled;
   };
 
 }
Index: common/rfb/ServerCore.cxx
===================================================================
--- common/rfb/ServerCore.cxx	(revision 4807)
+++ common/rfb/ServerCore.cxx	(working copy)
@@ -47,10 +47,12 @@
  "The number of milliseconds to wait for a client which is no longer "
  "responding",
  20000, 0);
+// FIXME: Should probably make this a tristate as "on" can currently
+//        still disable it under some circumstances.
 rfb::BoolParameter rfb::Server::compareFB
 ("CompareFB",
  "Perform pixel comparison on framebuffer to reduce unnecessary updates",
- false);
+ true);
 rfb::BoolParameter rfb::Server::protocol3_3
 ("Protocol3.3",
  "Always use protocol version 3.3 for backwards compatibility with "
Index: common/rfb/VNCSConnectionST.h
===================================================================
--- common/rfb/VNCSConnectionST.h	(revision 4807)
+++ common/rfb/VNCSConnectionST.h	(working copy)
@@ -85,6 +85,10 @@
     // The following methods never throw exceptions nor do they ever delete the
     // SConnectionST object.
 
+    // getComparerState() returns if this client would like the framebuffer
+    // comparer to be enabled.
+    bool getComparerState();
+
     // renderedCursorChange() is called whenever the server-side rendered
     // cursor changes shape or position.  It ensures that the next update will
     // clean up the old rendered cursor and if necessary draw the new rendered
Index: common/rfb/TightEncoder.cxx
===================================================================
--- common/rfb/TightEncoder.cxx	(revision 4807)
+++ common/rfb/TightEncoder.cxx	(working copy)
@@ -74,8 +74,9 @@
   { 65536, 2048,  24, 9, 9, 7,  64, 96, 92, SUBSAMP_NONE }, // 8
   { 65536, 2048,  32, 9, 9, 9,  96, 96,100, SUBSAMP_NONE }  // 9
 };
-const int TightEncoder::defaultCompressLevel = 1;
 
+const int TightEncoder::defaultCompressLevel = 2;
+
 //
 // Including BPP-dependent implementation of the encoder.
 //
Index: common/rfb/VNCServerST.cxx
===================================================================
--- common/rfb/VNCServerST.cxx	(revision 4807)
+++ common/rfb/VNCServerST.cxx	(working copy)
@@ -596,10 +596,13 @@
 
   pb->grabRegion(toCheck);
 
-  if (rfb::Server::compareFB) {
-    comparer->compare();
+  if (getComparerState())
+    comparer->enable();
+  else
+    comparer->disable();
+
+  if (comparer->compare())
     comparer->getUpdateInfo(&ui, pb->getRect());
-  }
 
   if (renderCursor) {
     pb->getImage(renderedCursor.data,
@@ -665,3 +668,17 @@
     (*ci)->screenLayoutChangeOrClose(reasonOtherClient);
   }
 }
+
+bool VNCServerST::getComparerState()
+{
+  if (!rfb::Server::compareFB)
+    return false;
+
+  std::list<VNCSConnectionST*>::iterator ci, ci_next;
+  for (ci=clients.begin();ci!=clients.end();ci=ci_next) {
+    ci_next = ci; ci_next++;
+    if ((*ci)->getComparerState())
+      return true;
+  }
+  return false;
+}
Index: common/rfb/ConnParams.cxx
===================================================================
--- common/rfb/ConnParams.cxx	(revision 4807)
+++ common/rfb/ConnParams.cxx	(working copy)
@@ -35,7 +35,7 @@
     supportsDesktopRename(false), supportsLastRect(false),
     supportsSetDesktopSize(false), supportsFence(false),
     supportsContinuousUpdates(false),
-    customCompressLevel(false), compressLevel(6),
+    customCompressLevel(false), compressLevel(2),
     noJpeg(false), qualityLevel(-1), fineQualityLevel(-1),
     subsampling(SUBSAMP_UNDEFINED),
     name_(0), nEncodings_(0), encodings_(0),
Index: common/rfb/VNCSConnectionST.cxx
===================================================================
--- common/rfb/VNCSConnectionST.cxx	(revision 4807)
+++ common/rfb/VNCSConnectionST.cxx	(working copy)
@@ -326,6 +326,16 @@
   return secsToMillis(timeLeft);
 }
 
+
+bool VNCSConnectionST::getComparerState()
+{
+  // We interpret a low compression level as an indication that the client
+  // wants to prioritise CPU usage over bandwidth, and hence disable the
+  // comparing update tracker.
+  return (cp.compressLevel == -1) || (cp.compressLevel > 1);
+}
+
+
 // renderedCursorChange() is called whenever the server-side rendered cursor
 // changes shape or position.  It ensures that the next update will clean up
 // the old rendered cursor and if necessary draw the new rendered cursor.
Index: vncviewer/parameters.cxx
===================================================================
--- vncviewer/parameters.cxx	(revision 4807)
+++ vncviewer/parameters.cxx	(working copy)
@@ -56,7 +56,7 @@
                                   "Default if CompressLevel is specified.", false);
 IntParameter compressLevel("CompressLevel",
                            "Use specified compression level 0 = Low, 6 = High",
-                           1);
+                           2);
 BoolParameter noJpeg("NoJPEG",
                      "Disable lossy JPEG compression in Tight encoding.",
                      false);

Attachment: signature.asc
Description: PGP signature

------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure 
contains a definitive record of customers, application performance, 
security threats, fraudulent activity, and more. Splunk takes this 
data and makes sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-novd2d
_______________________________________________
Tigervnc-devel mailing list
Tigervnc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tigervnc-devel

Reply via email to