Merge lp:~dandrader/ubuntu-keyboard/osk_rotation_lp1236489 into lp:ubuntu-keyboard

[Daniel d'Andrada]

ubuntu-keyboard-info socket now reports the keyboard's scene geometry

That's enough to cover keyboard rotations and animations.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:77
http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-ci/157/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-saucy-amd64-ci/158
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-saucy-armhf-ci/157
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-saucy-armhf-ci/157/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-saucy-i386-ci/157

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/ubuntu-keyboard-ci/157/rebuild

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

Using a Timer to poll for geometry changes is not a nice solution IMO. Yes you have a point that it's harder to monitor geometry changes with respect to the scene coordinates, but for this specific use-case, is not the parent effectively the root of the scene anyway?

If yes, then listening for geometryChanged signal gives us all the information we need to send shell.

If you absolutely need to monitor for parent geometry changes, you might be interested in
http://bazaar.launchpad.net/~mir-team/unity-mir/trunk/view/head:/src/modules/Unity/Application/inputarea.cpp

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

> is not the parent effectively the root of the scene anyway?

No, it's not. The current hierarchy is this:

fullscreenItem -> OrientationHelper -> canvas -> swipeArea -> keyboardSurface

>
> If you absolutely need to monitor for parent geometry changes, you might be
> interested in
> http://bazaar.launchpad.net/~mir-team/unity-
> mir/trunk/view/head:/src/modules/Unity/Application/inputarea.cpp

Thanks.
There's a reason for not having an API for notifying changes in scene coordinates: It's expensive and therefore not advisable. Qt has it in its QGraphicsScene API but it comes with a warning notice.

[lp:~dandrader/ubuntu-keyboard/osk_rotation_lp1236489]

78. By Daniel d'Andrada on 2013-10-14

Replace timer with SceneRectWatcher

[Daniel d'Andrada (dandrader)]

Updated

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:78
http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-ci/158/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-saucy-amd64-ci/159
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-saucy-armhf-ci/158
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-saucy-armhf-ci/158/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-saucy-i386-ci/158

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/ubuntu-keyboard-ci/158/rebuild

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

Ok, I'm happy the polling has gone away. Using timer to prevent flood of signals is good idea.

One bug: the height reported by this is too high, as it includes the area above the OSK where the key-down-popup is drawn. As a result, in the Contacts app, when creating a new contact, you cannot click the buttons on top of the OSK.

review: Needs Fixing

[lp:~dandrader/ubuntu-keyboard/osk_rotation_lp1236489]

79. By Daniel d'Andrada on 2013-10-16

Inform a bigger area while the extended keys pop-up is being shown

[Daniel d'Andrada (dandrader)]

> One bug: the height reported by this is too high, as it includes the area
> above the OSK where the key-down-popup is drawn. As a result, in the Contacts
> app, when creating a new contact, you cannot click the buttons on top of the
> OSK.

Fixed.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:79
http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-ci/167/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-saucy-amd64-ci/168
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-saucy-armhf-ci/167
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-saucy-armhf-ci/167/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-saucy-i386-ci/167

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/ubuntu-keyboard-ci/167/rebuild

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

Works great! Code clean and functional, approving!

review: Approve

=== modified file 'qml/Keyboard.qml'
--- qml/Keyboard.qml	2013-10-14 08:02:30 +0000
+++ qml/Keyboard.qml	2013-10-16 13:17:37 +0000
@@ -29,6 +29,13 @@
  *
  */
 
+
+//////////////////////
+// !!!IMPORTANT!!! Heavy modifications here might break the assumptions made by
+// UbuntuApplicationApiWrapper code (e.g. object names and what the items do/contain).
+// Update the code there accordingly, if needed. That code should no longer be needed
+// once the final architecture is in place.
+
 import QtQuick 2.0
 import "constants.js" as Const
 import "keys/"
@@ -112,6 +119,7 @@
 
         Item {
             id: keyboardSurface
+            objectName: "keyboardSurface"
 
             x:0
             y:0
@@ -132,6 +140,7 @@
 
             Item {
                 id: keyboardComp
+                objectName: "keyboardComp"
 
                 height: canvas.keypadHeight - wordRibbon.height
                 width: parent.width

=== modified file 'qml/KeyboardContainer.qml'
--- qml/KeyboardContainer.qml	2013-10-10 09:27:28 +0000
+++ qml/KeyboardContainer.qml	2013-10-16 13:17:37 +0000
@@ -73,6 +73,7 @@
     }
     ExtendedKeysSelector {
         id: extendedKeysSelector
+        objectName: "extendedKeysSelector"
         anchors.fill: parent
     }
 

=== modified file 'src/plugin/inputmethod.cpp'
--- src/plugin/inputmethod.cpp	2013-09-30 12:06:45 +0000
+++ src/plugin/inputmethod.cpp	2013-10-16 13:17:37 +0000
@@ -448,6 +448,8 @@
     {
         d->qmlRootItem = d->view->rootObject()->findChild<QQuickItem*>("ubuntuKeyboard");
         QObject::connect(d->qmlRootItem, SIGNAL(stateChanged(QString)), this, SLOT(onQMLStateChanged(QString)));
+
+        d->applicationApiWrapper->setRootObject(d->view->rootObject());
     }
         break;
     default:

=== modified file 'src/plugin/plugin.pro'
--- src/plugin/plugin.pro	2013-09-26 10:55:56 +0000
+++ src/plugin/plugin.pro	2013-10-16 13:17:37 +0000
@@ -29,6 +29,7 @@
     inputmethod_p.h \
     editor.h \
     keyboadsettings.h \
+    scenerectwatcher.h \
     updatenotifier.h \
     ubuntuapplicationapiwrapper.h
 
@@ -37,6 +38,7 @@
     inputmethod.cpp \
     editor.cpp \
     keyboadsettings.cpp \
+    scenerectwatcher.cpp \
     updatenotifier.cpp \
     ubuntuapplicationapiwrapper.cpp
 

=== added file 'src/plugin/scenerectwatcher.cpp'
--- src/plugin/scenerectwatcher.cpp	1970-01-01 00:00:00 +0000
+++ src/plugin/scenerectwatcher.cpp	2013-10-16 13:17:37 +0000
@@ -0,0 +1,91 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <scenerectwatcher.h>
+
+SceneRectWatcher::SceneRectWatcher(QObject *parent)
+    : QObject(parent)
+{
+    m_sceneRectChangedTimer.setInterval(200);
+    m_sceneRectChangedTimer.setSingleShot(true);
+    connect(&m_sceneRectChangedTimer, &QTimer::timeout,
+            this, &SceneRectWatcher::sceneRectChanged);
+}
+
+void SceneRectWatcher::setItem(QQuickItem *target)
+{
+    if (m_target == target)
+        return;
+
+    m_sceneRectChangedTimer.stop();
+    m_target = target;
+    listenToAscendantsChanges();
+}
+
+void SceneRectWatcher::listenToAscendantsChanges()
+{
+    disconnectFromAscendantsChanges();
+
+    if (!m_target)
+        return;
+
+    QQuickItem* item = m_target.data();
+
+    m_connections.append(connect(item, &QQuickItem::widthChanged,
+                                 this, &SceneRectWatcher::notifySceneRectChange));
+    m_connections.append(connect(item, &QQuickItem::heightChanged,
+                                 this, &SceneRectWatcher::notifySceneRectChange));
+
+    while (item != NULL) {
+        m_connections.append(connect(item, &QQuickItem::parentChanged,
+                                     this, &SceneRectWatcher::onAscendantChanged));
+        m_connections.append(connect(item, &QQuickItem::xChanged,
+                                     this, &SceneRectWatcher::notifySceneRectChange));
+        m_connections.append(connect(item, &QQuickItem::yChanged,
+                                     this, &SceneRectWatcher::notifySceneRectChange));
+        m_connections.append(connect(item, &QQuickItem::transformOriginChanged,
+                                     this, &SceneRectWatcher::notifySceneRectChange));
+        m_connections.append(connect(item, &QQuickItem::rotationChanged,
+                                     this, &SceneRectWatcher::notifySceneRectChange));
+        m_connections.append(connect(item, &QQuickItem::scaleChanged,
+                                     this, &SceneRectWatcher::notifySceneRectChange));
+        item = item->parentItem();
+    }
+}
+
+void SceneRectWatcher::disconnectFromAscendantsChanges()
+{
+    // disconnect all previously connected signals
+    Q_FOREACH (QMetaObject::Connection connection, m_connections) {
+        disconnect(connection);
+    }
+    m_connections.clear();
+}
+
+void SceneRectWatcher::onAscendantChanged()
+{
+    // Simple approach: Just rebuild connections from scratch.
+    listenToAscendantsChanges();
+
+    notifySceneRectChange();
+}
+
+void SceneRectWatcher::notifySceneRectChange()
+{
+    if (!m_sceneRectChangedTimer.isActive()) {
+        m_sceneRectChangedTimer.start();
+    }
+}

=== added file 'src/plugin/scenerectwatcher.h'
--- src/plugin/scenerectwatcher.h	1970-01-01 00:00:00 +0000
+++ src/plugin/scenerectwatcher.h	2013-10-16 13:17:37 +0000
@@ -0,0 +1,54 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef SCENERECTWATCHER_H
+#define SCENERECTWATCHER_H
+
+#include <QObject>
+
+#include <QQuickItem>
+#include <QList>
+#include <QPointer>
+#include <QTimer>
+
+class SceneRectWatcher : public QObject {
+    Q_OBJECT
+public:
+    SceneRectWatcher(QObject *parent = 0);
+    void setItem(QQuickItem *target);
+
+Q_SIGNALS:
+    void sceneRectChanged();
+
+private Q_SLOTS:
+    void onAscendantChanged();
+    void notifySceneRectChange();
+
+private:
+    void listenToAscendantsChanges();
+    void disconnectFromAscendantsChanges();
+
+    QList<QMetaObject::Connection> m_connections;
+    QPointer<QQuickItem> m_target;
+
+    // We send sceneRectChanged() with a delay avoid flooding sceneRectChanged()
+    // listeners during animations and also because it's common that more than
+    // one property changes during the same event loop iteration (e.g. ancestor.x()
+    // and ancestor.y())
+    QTimer m_sceneRectChangedTimer;
+};
+
+#endif // SCENERECTWATCHER_H

=== modified file 'src/plugin/ubuntuapplicationapiwrapper.cpp'
--- src/plugin/ubuntuapplicationapiwrapper.cpp	2013-10-11 15:34:27 +0000
+++ src/plugin/ubuntuapplicationapiwrapper.cpp	2013-10-16 13:17:37 +0000
@@ -25,10 +25,10 @@
   #define HAVE_UBUNTU_PLATFORM_API
 #endif
 
+#include <QByteArray>
 #include <QDir>
 #include <QFile>
 #include <QtGlobal>
-#include <QByteArray>
 
 namespace {
     const char gServerName[] = "ubuntu-keyboard-info";
@@ -42,9 +42,14 @@
         m_runningOnMir = true;
     }
 
+    m_sharedInfo.reset();
+
     if (m_runningOnMir) {
         startLocalServer();
     }
+
+    connect(&m_sceneRectWatcher, &SceneRectWatcher::sceneRectChanged,
+            this, &UbuntuApplicationApiWrapper::updateSharedInfo);
 }
 
 void UbuntuApplicationApiWrapper::startLocalServer()
@@ -86,7 +91,9 @@
     Q_UNUSED(height)
 #endif
 
-    sendInfoToClientConnection(width, height);
+    m_sceneRectWatcher.setItem(m_keyboardComp);
+    startWatchingExtendedKeysSelector();
+    updateSharedInfo();
 }
 
 void UbuntuApplicationApiWrapper::reportOSKInvisible()
@@ -96,6 +103,9 @@
         ubuntu_ui_report_osk_invisible();
     }
 #endif
+
+    m_sceneRectWatcher.setItem(0);
+    stopWatchingExtendedKeysSelector();
 }
 
 int UbuntuApplicationApiWrapper::oskWindowRole() const
@@ -107,7 +117,7 @@
 #endif
 }
 
-void UbuntuApplicationApiWrapper::sendInfoToClientConnection(int width, int height)
+void UbuntuApplicationApiWrapper::sendInfoToClientConnection()
 {
     if (!m_clientConnection
             || m_clientConnection->state() != QLocalSocket::ConnectedState) {
@@ -115,18 +125,14 @@
         return;
     }
 
-    struct SharedInfo sharedInfo;
-    sharedInfo.keyboardWidth = width;
-    sharedInfo.keyboardHeight = height;
-
-    if (sharedInfo == m_lastInfoShared) {
+    if (m_sharedInfo == m_lastInfoShared) {
         // do not flood our listener with redundant info. This also means that
         // were are getting called unnecessarily
         return;
     }
 
     const qint64 sharedInfoSize = sizeof(struct SharedInfo);
-    qint64 bytesWritten = m_clientConnection->write(reinterpret_cast<char *>(&sharedInfo),
+    qint64 bytesWritten = m_clientConnection->write(reinterpret_cast<char *>(&m_sharedInfo),
                                                     sharedInfoSize);
 
     if (bytesWritten < 0) {
@@ -138,7 +144,7 @@
                       "but only" << bytesWritten << "went through";
     }
 
-    m_lastInfoShared = sharedInfo;
+    m_lastInfoShared = m_sharedInfo;
 }
 
 void UbuntuApplicationApiWrapper::onNewConnection()
@@ -175,16 +181,86 @@
     }
 }
 
+void UbuntuApplicationApiWrapper::updateSharedInfo()
+{
+    if (m_keyboardComp.isNull() || m_keyboardSurface.isNull()
+            || m_extendedKeysSelector.isNull()) {
+        return;
+    }
+
+    QRectF keyboardSceneRect;
+
+    if (m_extendedKeysSelector->isEnabled() && m_extendedKeysSelector->isVisible()) {
+        // The pop-up could be above the keyboard, so we need a bigger area that's guaranteed
+        // to contain both the keyboard and that extended keys selector pop-up, which is the
+        // keyboardSurface.
+        keyboardSceneRect = m_keyboardSurface->mapRectToScene(QRectF(0, 0,
+                                                              m_keyboardSurface->width(),
+                                                              m_keyboardSurface->height()));
+    } else {
+        // Regular case, just the keyboard area.
+        keyboardSceneRect = m_keyboardComp->mapRectToScene(QRectF(0, 0,
+                                                           m_keyboardComp->width(),
+                                                           m_keyboardComp->height()));
+    }
+
+    m_sharedInfo.keyboardX = keyboardSceneRect.x();
+    m_sharedInfo.keyboardY = keyboardSceneRect.y();
+    m_sharedInfo.keyboardWidth = keyboardSceneRect.width();
+    m_sharedInfo.keyboardHeight = keyboardSceneRect.height();
+
+    sendInfoToClientConnection();
+}
+
+void UbuntuApplicationApiWrapper::setRootObject(QQuickItem *rootObject)
+{
+    // Getting items from qml/Keyboard.qml
+
+    m_keyboardSurface = rootObject->findChild<QQuickItem*>("keyboardSurface");
+    if (m_keyboardSurface.isNull()) {
+        qFatal("UbuntuApplicationApiWrapper: couldn't find \"keyboardSurface\" QML item");
+    }
+
+    m_keyboardComp = rootObject->findChild<QQuickItem*>("keyboardComp");
+    if (m_keyboardComp.isNull()) {
+        qFatal("UbuntuApplicationApiWrapper: couldn't find \"keyboardComp\" QML item");
+    }
+
+    // In qml/KeyboardContainer.qml, a child of qml/Keyboard.qml
+    m_extendedKeysSelector = rootObject->findChild<QQuickItem*>("extendedKeysSelector");
+    if (m_extendedKeysSelector.isNull()) {
+        qFatal("UbuntuApplicationApiWrapper: couldn't find \"extendedKeysSelector\" QML item");
+    }
+}
+
+void UbuntuApplicationApiWrapper::startWatchingExtendedKeysSelector()
+{
+    connect(m_extendedKeysSelector.data(), &QQuickItem::enabledChanged,
+            this, &UbuntuApplicationApiWrapper::updateSharedInfo);
+    connect(m_extendedKeysSelector.data(), &QQuickItem::visibleChanged,
+            this, &UbuntuApplicationApiWrapper::updateSharedInfo);
+}
+
+void UbuntuApplicationApiWrapper::stopWatchingExtendedKeysSelector()
+{
+    disconnect(m_extendedKeysSelector.data(), 0, this, 0);
+}
+
+
 // ------------------------------- SharedInfo ----------------------------
 
 bool UbuntuApplicationApiWrapper::SharedInfo::operator ==(const struct SharedInfo &other)
 {
-    return keyboardWidth == other.keyboardWidth
+    return keyboardX == other.keyboardX
+        && keyboardY == other.keyboardY
+        && keyboardWidth == other.keyboardWidth
         && keyboardHeight == other.keyboardHeight;
 }
 
 void UbuntuApplicationApiWrapper::SharedInfo::reset()
 {
+    keyboardX = -1;
+    keyboardY = -1;
     keyboardWidth = 0;
     keyboardHeight = 0;
 }

=== modified file 'src/plugin/ubuntuapplicationapiwrapper.h'
--- src/plugin/ubuntuapplicationapiwrapper.h	2013-10-11 15:34:27 +0000
+++ src/plugin/ubuntuapplicationapiwrapper.h	2013-10-16 13:17:37 +0000
@@ -20,6 +20,10 @@
 #include <QObject>
 #include <QLocalServer>
 #include <QLocalSocket>
+#include <QQuickItem>
+#include <QPointer>
+
+#include "scenerectwatcher.h"
 
 /*
  * Class: UbuntuApplicationApiWrapper
@@ -41,28 +45,50 @@
     void reportOSKVisible(const int, const int, const int, const int);
     void reportOSKInvisible();
 
+    void setRootObject(QQuickItem *rootObject);
 private Q_SLOTS:
     void onNewConnection();
     void onClientDisconnected();
+    void updateSharedInfo();
 
 private:
     // NB! Must match the definition in unity-mir. Not worth creating a shared header
     // just for that.
     struct SharedInfo {
+        qint32 keyboardX;
+        qint32 keyboardY;
         qint32 keyboardWidth;
         qint32 keyboardHeight;
 
         bool operator ==(const struct SharedInfo &other);
         void reset();
     };
-    void sendInfoToClientConnection(int width, int height);
+    void sendInfoToClientConnection();
     void startLocalServer();
     QString buildSocketFilePath() const;
+    void startWatchingExtendedKeysSelector();
+    void stopWatchingExtendedKeysSelector();
 
     bool m_runningOnMir;
     QLocalServer m_localServer;
     QLocalSocket *m_clientConnection;
+    struct SharedInfo m_sharedInfo;
     struct SharedInfo m_lastInfoShared;
+
+    /////
+    // Items from qml/Keyboard.qml
+
+    // Just the keyboard area
+    QPointer<QQuickItem> m_keyboardComp;
+
+    // Keyboard + space for the word ribbon on top
+    QPointer<QQuickItem> m_keyboardSurface;
+
+    // The baloon that pops-up when you long press a character key to get
+    // its accented versions etc.
+    QPointer<QQuickItem> m_extendedKeysSelector;
+
+    SceneRectWatcher m_sceneRectWatcher;
 };
 
 #endif // UBUNTUAPPLICATIONAPIWRAPPER_H