[client] Rewrite thread sync for VNC yet again

Move processing of image (scaling) to GUI thread.
Get rid of second (scaled) image buffer. Instead,
whenever we redraw parts of the VNC viewer, the
according image parts will be copied and scaled
from the buffer the vncclient thread is using.
The buffer is wrapped in a QImage and handed over
using a QSharedPointer, so reinitializing the
buffer on the fly should yield no problems.

4 files changed, 116 insertions, 138 deletions

diff --git a/src/client/vnc/vncthread.cpp b/src/client/vnc/vncthread.cpp
index dcc61d0..1dbe005 100644
--- a/src/client/vnc/vncthread.cpp
+++ b/src/client/vnc/vncthread.cpp
@@ -22,20 +22,6 @@
 #include <netinet/tcp.h>
 
 /**
- * Calc greatest common divisor.
- *
- * @param a one number
- * @param b another number
- * @return greatest common divisor of a and b
- */
-static int gcd(int a, int b)
-{
-	if (b == 0)
-		return a;
-	return gcd(b, a % b);
-}
-
-/**
  * Initialize this VNC client connection worker thread.
  *
  * @param host The IP address of the VNC server we're going to connect to
@@ -44,9 +30,8 @@ static int gcd(int a, int b)
  * @param quality The desired quality level for the VNC stream
  */
 VncThread::VncThread(QString host, int port, QString passwd, int quality) :
-	QThread(), _frameBuffer(NULL), _run(true), _started(false)
+	QThread(), _run(true), _started(false)
 {
-	_srcStepX = _srcStepY = _dstStepX = _dstStepY = 0;
 	_host = host;
 	_port = port;
 	_passwd = passwd;
@@ -60,57 +45,18 @@ VncThread::~VncThread()
 {
 	qDebug("VNC worker destructor called.");
 	Q_ASSERT(_run == false);
-	if (_frameBuffer != NULL)
-		delete[] _frameBuffer;
 	if (_client != NULL) {
 		if (_client->sock != -1)
 			::close(_client->sock);
 		_client->sock = -1;
-		_client->frameBuffer = NULL;
 		rfbClientCleanup(_client);
 	}
 }
 
-/**
- * When using image scaling, calc matching pixel borders for both
- * resolutions to prevent artifacts through bilinear scaling.
- * If you simply round to the nearest number when scaling, you would
- * slightly stretch or shrink the image area, which leads to strange
- * sub-pixel-movements of parts of the image when being updated.
- * The values calculated here are used to determine a larger area around
- * the area that should actually be updated, that will not cause any
- * artifacts when scaling down/up.
- */
-void VncThread::calcScaling()
-{
-	if (_localSize.isEmpty())
-		return;
-	if (_clientSize.isEmpty())
-		return;
-	const int gcdX = gcd(_localSize.width(), _clientSize.width());
-	const int gcdY = gcd(_localSize.height(), _clientSize.height());
-	_srcStepX = _clientSize.width() / gcdX;
-	_srcStepY = _clientSize.height() / gcdY;
-	_dstStepX = _localSize.width() / gcdX;
-	_dstStepY = _localSize.height() / gcdY;
-	qDebug() << "Scaling updated to " << _clientSize << " -> " << _localSize;
-	emit imageUpdated(0, 0, _localSize.width(), _localSize.height());
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // Public
 
 /**
- * Tell the client that the size of the viewer window has changed.
- *
- * @param size The new size of the viewer window
- */
-void VncThread::setTargetBuffer(QSharedPointer<QImage> &buffer)
-{
-	_imgScaled = buffer;
-}
-
-/**
  * Worker thread's mainloop, connecting to the VNC server and handling the
  * connection.  The vnc client library is written in a blocking manner, so we
  * just do everything in our own thread and just hand over the image updates to
@@ -207,41 +153,6 @@ const QString VncThread::getDesktopName() const
  */
 void VncThread::processImageUpdate(int x, int y, int w, int h)
 {
-	QSharedPointer<QImage> buffer = _imgScaled;
-	QPainter painter(buffer.data());
-	if (buffer->size() != _localSize) {
-		_localSize = buffer->size();
-		this->calcScaling();
-		x = 0;
-		y = 0;
-		w = _img.width();
-		h = _img.height();
-	}
-	if (_srcStepX > 1 || _srcStepY > 1) {
-		// Scaling is required as vnc server and client are using different resolutions
-		// Calc section offsets first
-		const int startX = x / _srcStepX;
-		const int startY = y / _srcStepY;
-		const int endX = (x + w - 1) / _srcStepX + 1;
-		const int endY = (y + h - 1) / _srcStepY + 1;
-		// Now pixel offsets for source
-		const int srcX = startX * _srcStepX;
-		const int srcY = startY * _srcStepY;
-		const int srcW = endX * _srcStepX - srcX;
-		const int srcH = endY * _srcStepY - srcY;
-		// Pixel offsets for destination
-		x = startX * _dstStepX;
-		y = startY * _dstStepY;
-		w = endX * _dstStepX - x;
-		h = endY * _dstStepY - y;
-		// Rescale
-		QImage scaled(
-			_img.copy(srcX, srcY, srcW, srcH).scaled(w, h, Qt::IgnoreAspectRatio, Qt::SmoothTransformation));
-		painter.drawImage(x, y, scaled, 0, 0, w, h);
-	} else {
-		// Same resolution, nothing to do
-		painter.drawImage(x, y, _img, x, y, w, h);
-	}
 	emit imageUpdated(x, y, w, h);
 }
 
@@ -275,16 +186,18 @@ char* VncThread::passwdHandler(rfbClient *client)
 rfbBool VncThread::frameBufferHandler(rfbClient *client)
 {
 	VncThread *t = (VncThread*)rfbClientGetClientData(client, 0);
-	const int width = client->width, height = client->height, depth = 32; // client->format.bitsPerPixel;
+	const int width = client->width, height = client->height, depth = 32;
 	const int size = width * height * (depth / 8);
 	qDebug("[%s] Remote desktop: %ix%ix%i", t->metaObject()->className(), width, height, depth);
 
-	if (t->_frameBuffer != NULL)
-		delete[] t->_frameBuffer;
-
-	t->_frameBuffer = new uint8_t[size];
-	client->frameBuffer = t->_frameBuffer;
+	t->_img = QSharedPointer<QImage>(new QImage(width, height, QImage::Format_RGB32));
+	if (size > t->_img->byteCount()) {
+		qDebug() << "Fatal: Created image too small:" << t->_img->byteCount() << "<" << size;
+		::exit(1);
+	}
+	client->frameBuffer = t->_img->bits();
 	memset(client->frameBuffer, '\0', size);
+	client->format.trueColour = 1;
 	client->format.bitsPerPixel = depth;
 	client->format.redShift = 16;
 	client->format.greenShift = 8;
@@ -320,12 +233,6 @@ rfbBool VncThread::frameBufferHandler(rfbClient *client)
 	}
 	SetFormatAndEncodings(client);
 
-	t->_clientSize = QSize(width, height);
-
-	t->_img = QImage(client->frameBuffer, client->width, client->height, QImage::Format_RGB32);
-
-	t->calcScaling();
-
 	SendFramebufferUpdateRequest(client, 0, 0, width, height, false);
 
 	if (!t->_started) {
diff --git a/src/client/vnc/vncthread.h b/src/client/vnc/vncthread.h
index 38f5e17..16491fe 100644
--- a/src/client/vnc/vncthread.h
+++ b/src/client/vnc/vncthread.h
@@ -39,19 +39,13 @@ class VncThread : public QThread
 
 private:
 	rfbClient *_client;
-	quint8 *_frameBuffer;
 
 	QString _host;
 	int _port;
 	QString _passwd;
 	int _quality;
 
-	QImage _img;
-	QSharedPointer<QImage> _imgScaled;
-	QSize _clientSize;
-	QSize _localSize;
-
-	int _srcStepX, _srcStepY, _dstStepX, _dstStepY;
+	QSharedPointer<QImage> _img;
 
 	volatile bool _connected;
 	volatile bool _run;
@@ -71,11 +65,10 @@ public:
 	VncThread(QString host, int port, QString passwd, int quality);
 	~VncThread();
 
-	const QSize& getSourceSize() const { return _clientSize; }
 	const QString getDesktopName() const;
 	bool isConnected() { return _connected; }
 	void stop() { _run = false; }
-	void setTargetBuffer(QSharedPointer<QImage> &buffer);
+	const QSharedPointer<QImage>& getFrameBuffer() { return _img; }
 	void run();
 
 	int const static HIGH = 0;
diff --git a/src/client/vnc/vncwindow.cpp b/src/client/vnc/vncwindow.cpp
index f5a9337..79352ae 100644
--- a/src/client/vnc/vncwindow.cpp
+++ b/src/client/vnc/vncwindow.cpp
@@ -21,9 +21,23 @@
 
 #include <QTimer>
 
+/**
+ * Calc greatest common divisor.
+ *
+ * @param a one number
+ * @param b another number
+ * @return greatest common divisor of a and b
+ */
+static int gcd(int a, int b)
+{
+	if (b == 0)
+		return a;
+	return gcd(b, a % b);
+}
+
 VncWindow::VncWindow(QWidget *parent) :
-	QWidget(parent), _vncWorker(NULL), _viewOnly(true), _multiScreen(false), _clientId(0), _redrawTimer(0), _tcpTimeoutTimer(0),
-	_image(new QImage)
+	QWidget(parent), _srcStepX(1), _srcStepY(1), _dstStepX(1), _dstStepY(1),
+	_vncWorker(NULL), _viewOnly(true), _multiScreen(false), _clientId(0), _redrawTimer(0), _tcpTimeoutTimer(0)
 {
 	QTimer *upper = new QTimer(this);
 	connect(upper, SIGNAL(timeout()), this, SLOT(timer_moveToTop()));
@@ -74,7 +88,6 @@ void VncWindow::deleteVncThread()
 
 /**
  * Draws given part of the current VNC frame buffer to the window.
- * Gets the image from the _vncWorker thread in an unsafe way. :(
  *
  * @param x X offset of the region to draw
  * @param y Y offset of the region to draw
@@ -85,11 +98,75 @@ void VncWindow::draw(const int x, const int y, const int w, const int h)
 {
 	if (_vncWorker == NULL)
 		return;
+	QSharedPointer<QImage> buffer = _vncWorker->getFrameBuffer();
+	if (buffer.isNull())
+		return;
+	this->calcScaling(buffer.data());
+
 	QPainter painter(this);
-	painter.drawImage(x, y, *_image, x, y, w, h);
+	if (_dstStepX > 1 || _dstStepY > 1) {
+		// Scaling is required as vnc server and client are using different resolutions
+		// Calc section offsets first
+		const int startX = x / _dstStepX;
+		const int startY = y / _dstStepY;
+		const int endX = (x + w - 1) / _dstStepX + 1;
+		const int endY = (y + h - 1) / _dstStepY + 1;
+		// Now pixel offsets for source
+		const int dstX = startX * _dstStepX;
+		const int dstY = startY * _dstStepY;
+		const int dstW = endX * _dstStepX - dstX;
+		const int dstH = endY * _dstStepY - dstY;
+		// Pixel offsets for destination
+		const int srcX = startX * _srcStepX;
+		const int srcY = startY * _srcStepY;
+		const int srcW = endX * _srcStepX - srcX;
+		const int srcH = endY * _srcStepY - srcY;
+		// Rescale
+		QImage scaled(
+			buffer->copy(srcX, srcY, srcW, srcH).scaled(dstW, dstH, Qt::IgnoreAspectRatio, Qt::SmoothTransformation));
+		painter.drawImage(dstX, dstY, scaled, 0, 0, dstW, dstH);
+	} else {
+		// Same resolution, nothing to do
+		painter.drawImage(x, y, *buffer, x, y, w, h);
+	}
 	//painter.drawRect(x,y,w,h); // for debugging updated area
 }
 
+/**
+ * When using image scaling, calc matching pixel borders for both
+ * resolutions to prevent artifacts through bilinear scaling.
+ * If you simply round to the nearest number when scaling, you would
+ * slightly stretch or shrink the image area, which leads to strange
+ * sub-pixel-movements of parts of the image when being updated.
+ * The values calculated here are used to determine a larger area around
+ * the area that should actually be updated, that will not cause any
+ * artifacts when scaling down/up.
+ *
+ * @return whether scaling factors changed
+ */
+bool VncWindow::calcScaling(const QImage *remote)
+{
+	if (this->size() == _desiredSize &&
+			(remote == NULL || remote->size() == _remoteSize))
+		return false;
+	const QSize mySize = this->size();
+	const QSize remoteSize = remote == NULL ? _remoteSize : remote->size();
+	if (mySize.isEmpty())
+		return false;
+	if (remoteSize.isEmpty())
+		return false;
+	_remoteSize = remoteSize;
+	_desiredSize = mySize;
+	const int gcdX = gcd(_desiredSize.width(), _remoteSize.width());
+	const int gcdY = gcd(_desiredSize.height(), _remoteSize.height());
+	_srcStepX = _remoteSize.width() / gcdX;
+	_srcStepY = _remoteSize.height() / gcdY;
+	_dstStepX = _desiredSize.width() / gcdX;
+	_dstStepY = _desiredSize.height() / gcdY;
+	qDebug() << "Scaling updated to" << _remoteSize << "->" << _desiredSize;
+	return true;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Public
 
@@ -154,8 +231,7 @@ void VncWindow::open(const QString& host, int port, const QString& passwd, bool
 		showNormal();
 	}
 
-	this->repaint();
-	_vncWorker->setTargetBuffer(_image);
+	this->update();
 
 	_tcpTimeoutTimer = startTimer(10000);
 	_vncWorker->start(QThread::LowPriority);
@@ -183,8 +259,9 @@ void VncWindow::closeEvent(QCloseEvent * /* e */ )
 /**
  * Triggered by imageUpdate signal from the _vncWorker thread telling
  * that the given region of the image changed. Simply repaints the
- * given area of the window. Coordinates have already been scaled
- * accordingly.
+ * given area of the window.
+ * Since these are client coordinates we might have to do
+ * some scaling.
  *
  * @param x X offset of the region to update
  * @param y Y offset of the region to update
@@ -193,7 +270,21 @@ void VncWindow::closeEvent(QCloseEvent * /* e */ )
  */
 void VncWindow::onUpdateImage(const int x, const int y, const int w, const int h)
 {
-	this->repaint(x, y, w, h);
+	if (_vncWorker == NULL)
+		return;
+	if (!this->calcScaling(_vncWorker->getFrameBuffer().data()) && !_remoteSize.isEmpty()) {
+		if (_srcStepX > 1 || _srcStepY > 1) {
+			this->update((x * _desiredSize.width()) / _remoteSize.width() - 2,
+						 (y * _desiredSize.height()) / _remoteSize.height() - 2,
+						 (w * _desiredSize.width()) / _remoteSize.width() + 4,
+						 (h * _desiredSize.height()) / _remoteSize.height() + 4);
+		} else {
+			this->update(x, y, w, h);
+		}
+	} else {
+		// Changed, redraw everything
+		this->update();
+	}
 }
 
 /**
@@ -290,22 +381,6 @@ void VncWindow::paintEvent(QPaintEvent *event)
 }
 
 /**
- * Called by Qt if the window is being resized.
- *
- * @param event the resize event
- */
-void VncWindow::resizeEvent(QResizeEvent* event)
-{
-	const QSize size = event->size();
-	if (size != _image->size()) {
-		_image = QSharedPointer<QImage>(new QImage(size, QImage::Format_RGB32));
-	}
-	if (_vncWorker != NULL) {
-		_vncWorker->setTargetBuffer(_image);
-	}
-}
-
-/**
  * Called when user releases a pressed key and the window has focus
  */
 void VncWindow::keyReleaseEvent(QKeyEvent* event)
diff --git a/src/client/vnc/vncwindow.h b/src/client/vnc/vncwindow.h
index cdcf51f..7124ddc 100644
--- a/src/client/vnc/vncwindow.h
+++ b/src/client/vnc/vncwindow.h
@@ -44,12 +44,13 @@ signals:
 
 protected:
 	void paintEvent(QPaintEvent *event);
-	void resizeEvent(QResizeEvent* event);
+	void resizeEvent(QResizeEvent*) { this->update(); }
 	void closeEvent(QCloseEvent *e);
 	void timerEvent(QTimerEvent *event);
 	void keyReleaseEvent(QKeyEvent *event);
 
 private:
+	int _srcStepX, _srcStepY, _dstStepX, _dstStepY;
 	VncThread *_vncWorker;
 	bool _viewOnly;
 	bool _multiScreen;
@@ -57,10 +58,12 @@ private:
 	int _redrawTimer;
 	int _tcpTimeoutTimer;
 	QPixmap _remoteThumb;
-	QSharedPointer<QImage> _image;
+	QSize _remoteSize;
+	QSize _desiredSize;
 
 	void draw(const int x, const int y, const int w, const int h);
 	void terminateVncThread();
+	bool calcScaling(const QImage *remote);
 
 };