Merge lp:~dandrader/qtmir/multimonitorNext into lp:qtmir

[Daniel d'Andrada]

Improve multimonitor support

* Removed magic:
 - Don't automagically select the screen where a window will be show.
   Let shell decide.
 - Don't automagically focus a window. Let shell handle it.

* Let shell know when a screen is about to be removed so that it has
  the opportunity to move or destroy a window in it before it's too late.
  - Added QGuiApplication::onScreenAboutToBeRemoved

* Added logging to key events

Known issues:
- Input events come at very irregular intervals (if at all) when an external monitor is connected to a phone or tablet. Works fine on a laptop connected to an external monitor.

* Are there any related MPs required for this MP to build/function as expected? Please list.
No, but to see any difference you should use this untiy8 branch:
https://code.launchpad.net/~unity-team/unity8/externalMonitor/+merge/273829

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:387
http://jenkins.qa.ubuntu.com/job/qtmir-ci/478/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/174/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/174/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/56/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/211/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/211/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/56/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:388
http://jenkins.qa.ubuntu.com/job/qtmir-ci/480/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/176/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/176/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/58/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/213/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/213/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/58/console

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

review: Needs Fixing (continuous-integration)

[Gerry Boland (gerboland)]

QGuiApplication::onScreenAboutToBeRemoved
How necessary is this? The QWindow has been hidden, so it has released its GL context and should be safe to hold onto until you get the screenRemoved signal.

review: Needs Information

[Daniel d'Andrada (dandrader)]

On 06/10/2015 19:16, Gerry Boland wrote:
> Review: Needs Information
>
> QGuiApplication::onScreenAboutToBeRemoved
> How necessary is this? The QWindow has been hidden, so it has released its GL context and should be safe to hold onto until you get the screenRemoved signal.
Experience tells otherwise (it would crash).

[Gerry Boland (gerboland)]

> On 06/10/2015 19:16, Gerry Boland wrote:
> > Review: Needs Information
> >
> > QGuiApplication::onScreenAboutToBeRemoved
> > How necessary is this? The QWindow has been hidden, so it has released its
> GL context and should be safe to hold onto until you get the screenRemoved
> signal.
> Experience tells otherwise (it would crash).

It's not supposed to! :) But this it technically more correct, so am ok with it.

[Gerry Boland (gerboland)]

review: Approve

[lp:~dandrader/qtmir/multimonitorNext]

389. By Daniel d'Andrada on 2015-10-14

Merge lp:~dandrader/qtmir/surviveEmptyTexture again

Unmerged revisions

=== modified file 'src/common/debughelpers.cpp'
--- src/common/debughelpers.cpp	2015-08-27 16:10:20 +0000
+++ src/common/debughelpers.cpp	2015-10-14 13:01:18 +0000
@@ -223,7 +223,7 @@
 {
     const int pointerCount = mir_touch_event_point_count(event);
 
-    QString string("MirTouchInputEvent(");
+    QString string("MirTouchEvent(");
 
     for (int i = 0; i < pointerCount; ++i) {
 
@@ -265,3 +265,32 @@
         break;
     }
 }
+
+namespace {
+const char *mirKeyboardActionToString(MirKeyboardAction keyboardAction)
+{
+    switch (keyboardAction)
+    {
+    case mir_keyboard_action_up:
+        return "up";
+    case mir_keyboard_action_down:
+        return "down";
+    case mir_keyboard_action_repeat:
+        return "repeat";
+    default:
+        return "???";
+        break;
+    }
+}
+}
+
+QString mirKeyboardEventToString(MirKeyboardEvent const* event)
+{
+    MirKeyboardAction keyboardAction = mir_keyboard_event_action(event);
+
+    xkb_keysym_t keyCode = mir_keyboard_event_key_code(event);
+
+    return QString("MirKeyboardEvent(action=%1,key_code=0x%2)")
+        .arg(mirKeyboardActionToString(keyboardAction))
+        .arg(keyCode, 4, 16, QLatin1Char('0'));
+}

=== modified file 'src/common/debughelpers.h'
--- src/common/debughelpers.h	2015-08-27 16:10:20 +0000
+++ src/common/debughelpers.h	2015-10-14 13:01:18 +0000
@@ -38,6 +38,7 @@
 
 QString mirPointerEventToString(MirPointerEvent const* event);
 QString mirTouchEventToString(MirTouchEvent const* event);
+QString mirKeyboardEventToString(MirKeyboardEvent const* event);
 const char *mirTouchActionToString(MirTouchAction touchAction);
 
 #endif // UBUNTUGESTURES_DEBUG_HELPER_H

=== modified file 'src/platforms/mirserver/mirserver.cpp'
--- src/platforms/mirserver/mirserver.cpp	2015-10-14 13:01:18 +0000
+++ src/platforms/mirserver/mirserver.cpp	2015-10-14 13:01:18 +0000
@@ -124,7 +124,13 @@
     apply_settings();
 
     // We will draw our own cursor.
-    add_init_callback([this](){ the_cursor()->hide(); });
+    // FIXME: Call override_the_cusor() instead once this method becomes available in a
+    //        future version of Mir.
+    add_init_callback([this]() {
+        the_cursor()->hide();
+        // Hack to work around https://bugs.launchpad.net/mir/+bug/1502200
+        static_cast<QtCompositor*>(the_compositor().get())->setCursor(the_cursor());
+    });
 
     qCDebug(QTMIR_MIR_MESSAGES) << "MirServer created";
 }

=== modified file 'src/platforms/mirserver/mirserverintegration.cpp'
--- src/platforms/mirserver/mirserverintegration.cpp	2015-10-14 13:01:18 +0000
+++ src/platforms/mirserver/mirserverintegration.cpp	2015-10-14 13:01:18 +0000
@@ -103,28 +103,20 @@
 {
     QWindowSystemInterface::flushWindowSystemEvents();
 
-    // FIXME: QWindow can be created specifying a destination QScreen. For now we
-    // will ignore it and just associate any unused Screen, if available.
     auto screens = m_mirServer->screenController().lock();
     if (!screens) {
         qCritical("Screens are not initialized, unable to create a new QWindow/ScreenWindow");
         return nullptr;
     }
-    Screen *screen = screens->getUnusedScreen();
-    if (!screen) {
-        qCritical("No available Screens to create a new QWindow/ScreenWindow for");
-        return nullptr;
-    }
-    QScreen *qscreen = screen->screen();
-    window->setScreen(qscreen);
 
     auto platformWindow = new ScreenWindow(window);
     if (screens->compositing()) {
         platformWindow->setExposed(true);
     }
 
-    qCDebug(QTMIR_SCREENS) << "New" << window << "with geom" << window->geometry()
-                           << "is backed by a" << screen << "with geometry" << screen->geometry();
+    qCDebug(QTMIR_SCREENS) << "QWindow" << window << "with geom" << window->geometry()
+                           << "is backed by a" << static_cast<Screen *>(window->screen()->handle())
+                           << "with geometry" << window->screen()->geometry();
     return platformWindow;
 }
 

=== modified file 'src/platforms/mirserver/qtcompositor.cpp'
--- src/platforms/mirserver/qtcompositor.cpp	2015-10-14 13:01:18 +0000
+++ src/platforms/mirserver/qtcompositor.cpp	2015-10-14 13:01:18 +0000
@@ -17,11 +17,19 @@
 #include "qtcompositor.h"
 #include "logging.h"
 
+#include <mir/graphics/cursor.h>
+
 // Lives in a Mir thread
 void QtCompositor::start()
 {
     qCDebug(QTMIR_SCREENS) << "QtCompositor::start";
 
+    // FIXME: Hack to work around https://bugs.launchpad.net/mir/+bug/1502200
+    //        See the FIXME in mirserver.cpp
+    if (m_cursor) {
+        m_cursor->hide();
+    }
+
     Q_EMIT starting(); // blocks
 }
 
@@ -31,3 +39,8 @@
 
     Q_EMIT stopping(); // blocks
 }
+
+void QtCompositor::setCursor(std::shared_ptr<mir::graphics::Cursor> cursor)
+{
+    m_cursor = cursor;
+}

=== modified file 'src/platforms/mirserver/qtcompositor.h'
--- src/platforms/mirserver/qtcompositor.h	2015-10-14 13:01:18 +0000
+++ src/platforms/mirserver/qtcompositor.h	2015-10-14 13:01:18 +0000
@@ -19,9 +19,18 @@
 
 #include <mir/compositor/compositor.h>
 
+// std lib
+#include <memory>
+
 // Qt
 #include <QObject>
 
+namespace mir {
+    namespace graphics {
+        class Cursor;
+    }
+}
+
 class QtCompositor : public QObject, public mir::compositor::Compositor
 {
     Q_OBJECT
@@ -32,11 +41,14 @@
     void start();
     void stop();
 
+    void setCursor(std::shared_ptr<mir::graphics::Cursor>);
+
 Q_SIGNALS:
     void starting();
     void stopping();
 
 private:
+    std::shared_ptr<mir::graphics::Cursor> m_cursor;
 };
 
 #endif // QTCOMPOSITOR_H

=== modified file 'src/platforms/mirserver/qteventfeeder.cpp'
--- src/platforms/mirserver/qteventfeeder.cpp	2015-10-14 13:01:18 +0000
+++ src/platforms/mirserver/qteventfeeder.cpp	2015-10-14 13:01:18 +0000
@@ -412,7 +412,7 @@
             const QList<struct QWindowSystemInterface::TouchPoint> &points, Qt::KeyboardModifiers mods) override
     {
         QWindowSystemInterface::handleTouchEvent(window, timestamp, device, points, mods);
-    }    
+    }
 
     void handleMouseEvent(ulong timestamp, QPointF movement, Qt::MouseButtons buttons, Qt::KeyboardModifiers modifiers) override
     {
@@ -587,11 +587,15 @@
                             text, is_auto_rep);
         qKeyEvent.setTimestamp(timestamp);
         if (context->filterEvent(&qKeyEvent)) {
-            // key event filtered out by input context
+            qCDebug(QTMIR_MIR_INPUT) << "Received" << qPrintable(mirKeyboardEventToString(kev))
+                << "but not dispatching as it was filtered out by input context";
             return;
         }
     }
 
+    qCDebug(QTMIR_MIR_INPUT).nospace() << "Received" << qPrintable(mirKeyboardEventToString(kev))
+        << ". Dispatching to " << mQtWindowSystem->focusedWindow();
+
     mQtWindowSystem->handleExtendedKeyEvent(mQtWindowSystem->focusedWindow(),
         timestamp, keyType, keyCode, modifiers,
         mir_keyboard_event_scan_code(kev),

=== modified file 'src/platforms/mirserver/screencontroller.cpp'
--- src/platforms/mirserver/screencontroller.cpp	2015-10-14 13:01:18 +0000
+++ src/platforms/mirserver/screencontroller.cpp	2015-10-14 13:01:18 +0000
@@ -30,6 +30,7 @@
 #include <QScreen>
 #include <QQuickWindow>
 #include <qpa/qwindowsysteminterface.h>
+#include <QGuiApplication> // for qApp
 
 // std
 #include <memory>
@@ -145,6 +146,10 @@
         if (window && window->window() && window->isExposed()) {
             window->window()->hide();
         }
+        bool ok = QMetaObject::invokeMethod(qApp, "onScreenAboutToBeRemoved", Qt::DirectConnection, Q_ARG(QScreen*, screen->screen()));
+        if (!ok) {
+            qCWarning(QTMIR_SCREENS) << "Failed to invoke QGuiApplication::onScreenAboutToBeRemoved(QScreen*) slot.";
+        }
         delete screen;
     }
 
@@ -185,58 +190,6 @@
     return new Screen(output);
 }
 
-Screen* ScreenController::getUnusedScreen()
-{
-    if (m_screenList.empty()) {
-        return nullptr;
-    } else if (m_screenList.size() == 1) {
-        return m_screenList.at(0);
-    }
-
-    // FIXME: Until we have better way of identifying screens, prioritize outputs based on their output type.
-    // Note the priorities defined here are nothing more than guesses. It tries to select internal displays first,
-    // then digital outputs, and finally analogue.
-    QMap <int, Screen*> priorityList;
-    auto prioritize = [](const mg::DisplayConfigurationOutputType &type) {
-        using out = mg::DisplayConfigurationOutputType;
-        switch(type) {
-        case out::lvds:
-        case out::edp:
-            return 0;
-        case out::displayport:
-        case out::hdmia:
-        case out::hdmib:
-            return 1;
-        case out::dvii:
-        case out::dvid:
-        case out::dvia:
-            return 2;
-        case out::vga:
-            return 3;
-        case out::ninepindin:
-            return 4;
-        case out::component:
-        case out::composite:
-        case out::svideo:
-            return 5;
-        case out::tv:
-            return 6;
-        case out::unknown:
-        default:
-            return 9;
-        }
-    };
-
-    for (auto screen : m_screenList) {
-        if (!screen->window()) {
-            priorityList.insert(prioritize(screen->outputType()), screen);
-        }
-    }
-
-    qCDebug(QTMIR_SCREENS) << "Prioritized list of available outputs:" << priorityList;
-    return priorityList.first(); // Map sorted by key, so first is the key with highest priority.
-}
-
 Screen* ScreenController::findScreenWithId(const QList<Screen *> &list, const mg::DisplayConfigurationOutputId id)
 {
     for (Screen *screen : list) {

=== modified file 'src/platforms/mirserver/screencontroller.h'
--- src/platforms/mirserver/screencontroller.h	2015-10-14 13:01:18 +0000
+++ src/platforms/mirserver/screencontroller.h	2015-10-14 13:01:18 +0000
@@ -59,7 +59,6 @@
 public:
     explicit ScreenController(QObject *parent = 0);
 
-    Screen* getUnusedScreen();
     QList<Screen*> screens() const { return m_screenList; }
     bool compositing() const { return m_compositing; }
 

=== modified file 'src/platforms/mirserver/screenwindow.cpp'
--- src/platforms/mirserver/screenwindow.cpp	2015-10-14 13:01:18 +0000
+++ src/platforms/mirserver/screenwindow.cpp	2015-10-14 13:01:18 +0000
@@ -58,10 +58,6 @@
         window->setGeometry(screenGeometry);
     }
     window->setSurfaceType(QSurface::OpenGLSurface);
-
-    // The compositor window is always active. I.e., it's always focused so that
-    // it always processes key events, etc
-    requestActivateWindow();
 }
 
 ScreenWindow::~ScreenWindow()

=== modified file 'tests/mirserver/ScreenController/screencontroller_test.cpp'
--- tests/mirserver/ScreenController/screencontroller_test.cpp	2015-10-14 13:01:18 +0000
+++ tests/mirserver/ScreenController/screencontroller_test.cpp	2015-10-14 13:01:18 +0000
@@ -135,18 +135,6 @@
     EXPECT_EQ(QRect(500, 600, 1500, 2000), screenController->screens().at(0)->geometry());
 }
 
-TEST_F(ScreenControllerTest, CheckPrioritizedGetUnusedScreen)
-{
-    std::vector<mg::DisplayConfigurationOutput> config{fakeOutput2, fakeOutput1};
-    std::vector<MockDisplayBuffer*> bufferConfig; // only used to match buffer with display, unecessary here
-    display->setFakeConfiguration(config, bufferConfig);
-
-    screenController->update();
-
-    auto screen = screenController->getUnusedScreen();
-    EXPECT_EQ(mg::DisplayConfigurationOutputType::lvds, screen->outputType());
-}
-
 TEST_F(ScreenControllerTest, MatchBufferWithDisplay)
 {
     std::vector<mg::DisplayConfigurationOutput> config{fakeOutput1};