Merge lp:~dandrader/qtubuntu/resizeCatchUp into lp:qtubuntu

[Daniel d'Andrada]

UbuntuWindow - keep redrawing a bit until you get the promised resized buffer

Usually you get a resized buffer on the next buffer swap, but it's not uncommon
that it shows up only after the second one.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:275
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/242/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/33
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/33/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtubuntu-ci/242/rebuild

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

Put the logging change in a different MR, not this one. I'm not convinced by it, we've used the logging.h macros for some time. If it is faulty, we should either fix it, or replace it everywhere, not just a solution in this file.

Please put in a note saying that the resizeCatchup stuff is working around a bug in Mir. It should not be necessary

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

On 27/08/2015 15:06, Gerry Boland wrote:
> I'm not convinced by it, we've used the logging.h macros for some time.

No, they've been silent for some time. At for the apps that come with
the phone And I've been replacing them with printf()s for some time already.

[Gerry Boland (gerboland)]

> On 27/08/2015 15:06, Gerry Boland wrote:
> > I'm not convinced by it, we've used the logging.h macros for some time.
>
> No, they've been silent for some time. At for the apps that come with
> the phone And I've been replacing them with printf()s for some time already.

That doesn't negate my initial complaint. Please remove those log changes from this MR, they don't belong.

[Daniel d'Andrada (dandrader)]

> Put the logging change in a different MR, not this one. I'm not convinced by
> it, we've used the logging.h macros for some time. If it is faulty, we should
> either fix it, or replace it everywhere, not just a solution in this file.
>
> Please put in a note saying that the resizeCatchup stuff is working around a
> bug in Mir. It should not be necessary

Done.

[lp:~dandrader/qtubuntu/resizeCatchUp]

275. By Daniel d'Andrada on 2015-08-27

UbuntuWindow - keep redrawing a bit until you get the promised resized buffer

Usually you get a resized buffer on the next buffer swap, but it's not uncommon
that it shows up only after the second one.

[Gerry Boland (gerboland)]

LGTM, thank you

review: Approve

[lp:~dandrader/qtubuntu/resizeCatchUp]

276. By Daniel d'Andrada on 2015-08-28

Fix build

=== modified file 'src/ubuntumirclient/window.cpp'
--- src/ubuntumirclient/window.cpp	2015-07-15 12:43:21 +0000
+++ src/ubuntumirclient/window.cpp	2015-08-28 08:12:52 +0000
@@ -100,6 +100,10 @@
     QSize bufferSize;
     QMutex mutex;
     QSharedPointer<UbuntuClipboard> clipboard;
+    int resizeCatchUpAttempts;
+#if !defined(QT_NO_DEBUG)
+    int frameNumber;
+#endif
 };
 
 static void eventCallback(MirSurface* surface, const MirEvent *event, void* context)
@@ -132,10 +136,15 @@
     d->state = window()->windowState();
     d->connection = connection;
     d->clipboard = clipboard;
+    d->resizeCatchUpAttempts = 0;
 
     static int id = 1;
     d->id = id++;
 
+#if !defined(QT_NO_DEBUG)
+    d->frameNumber = 0;
+#endif
+
     // Use client geometry if set explicitly, use available screen geometry otherwise.
     QPlatformWindow::setGeometry(window()->geometry() != screen->geometry() ?
         window()->geometry() : screen->availableGeometry());
@@ -314,7 +323,8 @@
 void UbuntuWindow::handleSurfaceResize(int width, int height)
 {
     QMutexLocker(&d->mutex);
-    LOG("UbuntuWindow::handleSurfaceResize(width=%d, height=%d)", width, height);
+    DLOG("UbuntuWindow::handleSurfaceResize(width=%d, height=%d) [%d]", width, height,
+        d->frameNumber);
 
     // The current buffer size hasn't actually changed. so just render on it and swap
     // buffers in the hope that the next buffer will match the surface size advertised
@@ -324,6 +334,12 @@
     // no synchronicity whatsoever between the processing of resize events and the
     // consumption of buffers.
     if (d->bufferSize.width() != width || d->bufferSize.height() != height) {
+        // if the next buffer doesn't have a different size, try some
+        // more
+        // FIXME: This is working around a mir bug! We really shound't have to
+        // swap more than once to get a buffer with the new size!
+        d->resizeCatchUpAttempts = 2;
+
         QWindowSystemInterface::handleExposeEvent(window(), geometry());
         QWindowSystemInterface::flushWindowSystemEvents();
     }
@@ -410,11 +426,18 @@
 
     bool sizeKnown = newBufferWidth > 0 && newBufferHeight > 0;
 
+#if !defined(QT_NO_DEBUG)
+    ++d->frameNumber;
+#endif
+
     if (sizeKnown && (d->bufferSize.width() != newBufferWidth ||
                 d->bufferSize.height() != newBufferHeight)) {
+        d->resizeCatchUpAttempts = 0;
 
-        DLOG("UbuntuWindow::onBuffersSwapped_threadSafe - buffer size changed from (%d,%d) to (%d,%d)",
-                d->bufferSize.width(), d->bufferSize.height(), newBufferWidth, newBufferHeight);
+        DLOG("UbuntuWindow::onBuffersSwapped_threadSafe [%d] - buffer size changed from (%d,%d) to (%d,%d)"
+               " resizeCatchUpAttempts=%d",
+               d->frameNumber, d->bufferSize.width(), d->bufferSize.height(), newBufferWidth, newBufferHeight,
+               d->resizeCatchUpAttempts);
 
         d->bufferSize.rwidth() = newBufferWidth;
         d->bufferSize.rheight() = newBufferHeight;
@@ -427,5 +450,14 @@
 
         QPlatformWindow::setGeometry(newGeometry);
         QWindowSystemInterface::handleGeometryChange(window(), newGeometry, QRect());
+    } else if (d->resizeCatchUpAttempts > 0) {
+        --d->resizeCatchUpAttempts;
+        DLOG("UbuntuWindow::onBuffersSwapped_threadSafe [%d] - buffer size (%d,%d). Redrawing to catch up a resized buffer."
+               " resizeCatchUpAttempts=%d",
+               d->frameNumber, d->bufferSize.width(), d->bufferSize.height(), d->resizeCatchUpAttempts);
+        QWindowSystemInterface::handleExposeEvent(window(), geometry());
+    } else {
+        DLOG("UbuntuWindow::onBuffersSwapped_threadSafe [%d] - buffer size (%d,%d). resizeCatchUpAttempts=%d",
+               d->frameNumber, d->bufferSize.width(), d->bufferSize.height(), d->resizeCatchUpAttempts);
     }
 }