Merge lp:~lukas-kde/qtmir/defaultKeymap into lp:qtmir

Proposed by Lukáš Tinkl on 2017-01-04
Status: Merged
Approved by: Lukáš Tinkl on 2017-01-18
Approved revision: 587
Merged at revision: 586
Proposed branch: lp:~lukas-kde/qtmir/defaultKeymap
Merge into: lp:qtmir
Prerequisite: lp:~lukas-kde/qtmir/cleanups
Diff against target: 565 lines (+284/-36)
17 files modified
CMakeLists.txt (+1/-1)
debian/control (+1/-1)
src/modules/Unity/Application/mirsurface.cpp (+2/-8)
src/platforms/mirserver/CMakeLists.txt (+1/-0)
src/platforms/mirserver/inputdeviceobserver.cpp (+106/-0)
src/platforms/mirserver/inputdeviceobserver.h (+55/-0)
src/platforms/mirserver/logging.cpp (+1/-0)
src/platforms/mirserver/logging.h (+1/-0)
src/platforms/mirserver/mirserverhooks.cpp (+17/-0)
src/platforms/mirserver/mirserverhooks.h (+3/-0)
src/platforms/mirserver/mirsingleton.cpp (+30/-0)
src/platforms/mirserver/mirsingleton.h (+5/-1)
src/platforms/mirserver/qmirserver_p.cpp (+1/-0)
src/platforms/mirserver/qteventfeeder.cpp (+0/-17)
src/platforms/mirserver/surfaceobserver.h (+0/-1)
tests/mirserver/QtEventFeeder/mock_qtwindowsystem.h (+18/-7)
tests/mirserver/QtEventFeeder/qteventfeeder_test.cpp (+42/-0)
To merge this branch: bzr merge lp:~lukas-kde/qtmir/defaultKeymap
Reviewer Review Type Date Requested Status
Unity8 CI Bot continuous-integration 2017-01-04 Approve on 2017-01-17
Gerry Boland 2017-01-04 Approve on 2017-01-04
Daniel d'Andrada 2017-01-04 Pending
Review via email: mp+314079@code.launchpad.net

This proposal supersedes a proposal from 2016-09-22.

Commit message

Apply default device keymap

Description of the change

Prereq-archive: ppa:ci-train-ppa-service/2272

Apply default device keymap (thereby fixing lp:1626435 - Keyboard layout not applied on the shell)

Drop unused code from SurfaceObserver

Needs unity-api branch: https://code.launchpad.net/~lukas-kde/unity-api/defaultKeymap/+merge/307309

And a small patch for unity8: https://code.launchpad.net/~lukas-kde/unity8/defaultKeymap/+merge/306761

To post a comment you must log in.
Andreas Pokorny (andreas-pokorny) : Posted in a previous version of this proposal
Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:565
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/386/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/2965/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2993
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2851
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2851/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2851
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2851/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2851
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2851/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2851
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2851/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2851/console
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2851/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2851
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2851/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2851
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2851/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2851
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2851/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2851
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2851/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/386/rebuild

review: Needs Fixing (continuous-integration)
Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:566
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/387/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/2970
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2998
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2856
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2856/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2856
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2856/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2856
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2856/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2856
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2856/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2856
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2856/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2856
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2856/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2856
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2856/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2856
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2856/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2856
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2856/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/387/rebuild

review: Approve (continuous-integration)
Gerry Boland (gerboland) wrote : Posted in a previous version of this proposal

+++ src/platforms/mirserver/inputdeviceobserver.cpp
+#include <QDebug>
logging.h includes this.

I see lots of namespace qualifiers here. Could you please do
using mi = mir::input
just to mask the worst of the noise.

+MirInputDeviceObserver::~MirInputDeviceObserver()
+{
+ m_devices.clear();
+}
redundant? QVector effectively does this in destruction anyway, no?

+++ src/platforms/mirserver/inputdeviceobserver.h
+class MirInputDeviceObserver
Please stick this into the qtmir namespace.

+ std::shared_ptr<mir::input::InputDeviceHub> m_hub;
I don't see this being used anywhere.

Other cleanups are nice, thanks, but mixing them with a new feature is a possible danger. Say if cleanup breaks something, the whole commit gets reverted/removed from the silo, so we loose the feature.

But overall very nice!

review: Needs Fixing
Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:567
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/388/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3014/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3042
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2899
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2899/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2899
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2899/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2899
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2899/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2899
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2899/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2899
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2899/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2899
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2899/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2899
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2899/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2899
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2899/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2899
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2899/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/388/rebuild

review: Needs Fixing (continuous-integration)
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

In src/platforms/mirserver/inputdeviceobserver.h:

"""
namespace mi = mir::input;
"""

Not a fan of aliasing namespaces in headers as that will contaminate any cpp file that #includes this header. Theoretically some cpp file might already use "mi" for something else and you would cause a collision, which is exactly what namespaces are there for.

Aliasing namespaces is a decision that only implementation code should make, locally.

Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

In src/platforms/mirserver/mirsingleton.h:

"""
Q_PROPERTY(QString currentKeymap READ currentKeymap WRITE setCurrentKeymap NOTIFY currentKeymapChanged)
"""

This has to go into unity/shell/application/Mir.h interface from unity-api.

review: Needs Fixing
Lukáš Tinkl (lukas-kde) wrote : Posted in a previous version of this proposal

> In src/platforms/mirserver/inputdeviceobserver.h:
>
> """
> namespace mi = mir::input;
> """
>
> Not a fan of aliasing namespaces in headers as that will contaminate any cpp
> file that #includes this header. Theoretically some cpp file might already use
> "mi" for something else and you would cause a collision, which is exactly what
> namespaces are there for.
>
> Aliasing namespaces is a decision that only implementation code should make,
> locally.

Although I agree in general, this is what Gerry suggested above :)

Lukáš Tinkl (lukas-kde) wrote : Posted in a previous version of this proposal

> In src/platforms/mirserver/mirsingleton.h:
>
> """
> Q_PROPERTY(QString currentKeymap READ currentKeymap WRITE setCurrentKeymap
> NOTIFY currentKeymapChanged)
> """
>
> This has to go into unity/shell/application/Mir.h interface from unity-api.

Yup, I can definitely move it over there if you insist but isn't this too implementation specific?

Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

> > In src/platforms/mirserver/mirsingleton.h:
> >
> > """
> > Q_PROPERTY(QString currentKeymap READ currentKeymap WRITE setCurrentKeymap
> > NOTIFY currentKeymapChanged)
> > """
> >
> > This has to go into unity/shell/application/Mir.h interface from unity-api.
>
> Yup, I can definitely move it over there if you insist but isn't this too
> implementation specific?

If unity8 uses it, it has to be in unity-api. That's the whole point of unity-api: define/expose the API that unity8 uses from qtmir.

Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

> > In src/platforms/mirserver/inputdeviceobserver.h:
> >
> > """
> > namespace mi = mir::input;
> > """
> >
> > Not a fan of aliasing namespaces in headers as that will contaminate any cpp
> > file that #includes this header. Theoretically some cpp file might already
> use
> > "mi" for something else and you would cause a collision, which is exactly
> what
> > namespaces are there for.
> >
> > Aliasing namespaces is a decision that only implementation code should make,
> > locally.
>
> Although I agree in general, this is what Gerry suggested above :)

You do that in src/platforms/mirserver/inputdeviceobserver.cpp, not in its header.

Lukáš Tinkl (lukas-kde) wrote : Posted in a previous version of this proposal

> > > In src/platforms/mirserver/inputdeviceobserver.h:
> > >
> > > """
> > > namespace mi = mir::input;
> > > """
> > >
> > > Not a fan of aliasing namespaces in headers as that will contaminate any
> cpp
> > > file that #includes this header. Theoretically some cpp file might already
> > use
> > > "mi" for something else and you would cause a collision, which is exactly
> > what
> > > namespaces are there for.
> > >
> > > Aliasing namespaces is a decision that only implementation code should
> make,
> > > locally.
> >
> > Although I agree in general, this is what Gerry suggested above :)
>
> You do that in src/platforms/mirserver/inputdeviceobserver.cpp, not in its
> header.

Sure, fixed

Lukáš Tinkl (lukas-kde) wrote : Posted in a previous version of this proposal

> > > In src/platforms/mirserver/mirsingleton.h:
> > >
> > > """
> > > Q_PROPERTY(QString currentKeymap READ currentKeymap WRITE setCurrentKeymap
> > > NOTIFY currentKeymapChanged)
> > > """
> > >
> > > This has to go into unity/shell/application/Mir.h interface from unity-
> api.
> >
> > Yup, I can definitely move it over there if you insist but isn't this too
> > implementation specific?
>
> If unity8 uses it, it has to be in unity-api. That's the whole point of unity-
> api: define/expose the API that unity8 uses from qtmir.

Yeah done; see also https://code.launchpad.net/~lukas-kde/unity-api/defaultKeymap/+merge/307309

Gerry Boland (gerboland) wrote :

+ connect(Mir::instance(), &Mir::currentKeymapChanged, this, &MirInputDeviceObserver::setKeymap, Qt::DirectConnection);
Why specifying direct? Is it that the slot doesn't live in an object belonging to a QThread (which has no event loop, so queued signal/slot fails to work).

+MirInputDeviceObserver::~MirInputDeviceObserver()
+{
+ m_devices.clear();
+}
is this necessary? Surely the deletion of the QVector will achieve this too?

+ const QString layout = stringList.at(0);
could this be a reference and save a string copy? Similar for variant.

I'm a bit concerned about thread safety. The device_{add,chang,remov}ed
methods are called by Mir, and that is usually done on some Mir internal thread. The object itself belongs to that thread too. However the Mir::currentKeymapChanged signal fires on the QtGui thread, so it is possible for those 2 threads to call applyKeymap almost simultaneously.

I think a little locking is necessary for safety.

Also, could you please add comments on each method explaining the thread situation, as this stuff is super easy to forget.

review: Needs Fixing
Lukáš Tinkl (lukas-kde) wrote :

> +MirInputDeviceObserver::~MirInputDeviceObserver()
> +{
> + m_devices.clear();
> +}
> is this necessary? Surely the deletion of the QVector will achieve this too?

I think so, QVector imo doesn't do that by itself.

Lukáš Tinkl (lukas-kde) wrote :

> + const QString layout = stringList.at(0);
> could this be a reference and save a string copy? Similar for variant.

Done for layout, variant might be empty so it won't work there

Gerry Boland (gerboland) wrote :

Ok, much improved, thank you.

But please add comments explaining the thread situation. We remember it now, but someone else won't have a clue, and could introduce subtle errors if they forget the locking, or remove the DirectConnection specifier thinking it extraneous.

review: Needs Fixing
Gerry Boland (gerboland) wrote :

> > +MirInputDeviceObserver::~MirInputDeviceObserver()
> > +{
> > + m_devices.clear();
> > +}
> > is this necessary? Surely the deletion of the QVector will achieve this too?
>
> I think so, QVector imo doesn't do that by itself.

/me was curious, looked at the source:

template <typename T>
class QVector
{
    typedef QTypedArrayData<T> Data;
    Data *d;

public:
    inline ~QVector() { if (!d->ref.deref()) freeData(d); }

};

template <typename T>
void QVector<T>::freeData(Data *x)
{
    destruct(x->begin(), x->end());
    Data::deallocate(x);
}

template <typename T>
void QVector<T>::destruct(T *from, T *to)
{
    if (QTypeInfo<T>::isComplex) {
        while (from != to) {
            from++->~T();
        }
    }
}

It does look like ~QVector calls freeData(Data*), which calls destruct() that explicitly calls the destructor of each member of the type T in the container (if T is complex which I think std::shared_ptr is).

How clear() works is less obvious to me, I think we both trust it does.

I'd be surprised that a container would leak its members on destruction, requiring us to explicitly clear() it before letting it go out of scope. Is why I suspect the clear() is unnecessary.

Lukáš Tinkl (lukas-kde) wrote :

Addressed the above comments

Gerry Boland (gerboland) wrote :

Perfect, thanks!

review: Approve
Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:587
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/444/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3794/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3822
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3665
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3665/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3665
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3665/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3665
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3665/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3665/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3665
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3665/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3665/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/444/rebuild

review: Needs Fixing (continuous-integration)
Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:587
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/445/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3800/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3828
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3671
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3671/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3671
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3671/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3671
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3671/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3671/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3671
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3671/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3671/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/445/rebuild

review: Needs Fixing (continuous-integration)
Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:587
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/448/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/448/rebuild

review: Needs Fixing (continuous-integration)
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:587
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/447/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3819
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3847
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3690
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3690/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3690
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3690/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3690
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3690/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3690
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3690/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3690
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3690/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3690
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3690/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/447/rebuild

review: Approve (continuous-integration)
Lukáš Tinkl (lukas-kde) wrote :

CI good again

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2016-12-16 08:24:29 +0000
3+++ CMakeLists.txt 2017-01-04 17:43:31 +0000
4@@ -88,7 +88,7 @@
5 pkg_check_modules(GSETTINGS_QT REQUIRED gsettings-qt)
6 pkg_check_modules(QTDBUSTEST libqtdbustest-1 REQUIRED)
7 pkg_check_modules(QTDBUSMOCK libqtdbusmock-1 REQUIRED)
8-pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application=23)
9+pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application=25)
10 pkg_check_modules(CGMANAGER libcgmanager REQUIRED)
11 pkg_check_modules(CONTENT_HUB libcontent-hub>=0.2 REQUIRED)
12
13
14=== modified file 'debian/control'
15--- debian/control 2016-12-13 15:51:14 +0000
16+++ debian/control 2017-01-04 17:43:31 +0000
17@@ -25,7 +25,7 @@
18 libubuntu-app-launch2-dev (>= 0.9),
19 libubuntu-application-api-dev (>= 2.1.0),
20 libudev-dev,
21- libunity-api-dev (>= 8.0),
22+ libunity-api-dev (>= 8.1),
23 liburl-dispatcher1-dev,
24 libxkbcommon-dev,
25 libxrender-dev,
26
27=== modified file 'src/modules/Unity/Application/mirsurface.cpp'
28--- src/modules/Unity/Application/mirsurface.cpp 2017-01-04 17:43:31 +0000
29+++ src/modules/Unity/Application/mirsurface.cpp 2017-01-04 17:43:31 +0000
30@@ -89,8 +89,8 @@
31 void cursor_image_set_to(mir::graphics::CursorImage const&) override;
32 void orientation_set_to(MirOrientation) override {}
33 void client_surface_close_requested() override {}
34- void keymap_changed(MirInputDeviceId, std::string const& model, std::string const& layout,
35- std::string const& variant, std::string const& options) override;
36+ void keymap_changed(MirInputDeviceId, std::string const&, std::string const&,
37+ std::string const&, std::string const&) override {}
38 void renamed(char const * name) override;
39 void cursor_image_removed() override;
40
41@@ -1142,12 +1142,6 @@
42 Q_EMIT cursorChanged(qcursor);
43 }
44
45-void MirSurface::SurfaceObserverImpl::keymap_changed(MirInputDeviceId, const std::string &, const std::string &layout,
46- const std::string &variant, const std::string &)
47-{
48- Q_EMIT keymapChanged(QString::fromStdString(layout), QString::fromStdString(variant));
49-}
50-
51 QCursor MirSurface::SurfaceObserverImpl::createQCursorFromMirCursorImage(const mir::graphics::CursorImage &cursorImage) {
52 if (cursorImage.as_argb_8888() == nullptr) {
53 // Must be a named cursor
54
55=== modified file 'src/platforms/mirserver/CMakeLists.txt'
56--- src/platforms/mirserver/CMakeLists.txt 2016-11-03 20:17:46 +0000
57+++ src/platforms/mirserver/CMakeLists.txt 2017-01-04 17:43:31 +0000
58@@ -101,6 +101,7 @@
59 mirserverintegration.cpp
60 miropenglcontext.cpp
61 offscreensurface.cpp
62+ inputdeviceobserver.cpp
63 promptsessionmanager.cpp promptsessionmanager.h promptsession.h
64 # We need to run moc on these headers
65 ${APPLICATION_API_INCLUDEDIR}/unity/shell/application/MirMousePointerInterface.h
66
67=== added file 'src/platforms/mirserver/inputdeviceobserver.cpp'
68--- src/platforms/mirserver/inputdeviceobserver.cpp 1970-01-01 00:00:00 +0000
69+++ src/platforms/mirserver/inputdeviceobserver.cpp 2017-01-04 17:43:31 +0000
70@@ -0,0 +1,106 @@
71+/*
72+ * Copyright (C) 2016-2017 Canonical, Ltd.
73+ *
74+ * This program is free software: you can redistribute it and/or modify it under
75+ * the terms of the GNU Lesser General Public License version 3, as published by
76+ * the Free Software Foundation.
77+ *
78+ * This program is distributed in the hope that it will be useful, but WITHOUT
79+ * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
80+ * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
81+ * Lesser General Public License for more details.
82+ *
83+ * You should have received a copy of the GNU Lesser General Public License
84+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
85+ */
86+
87+#include <mir/input/device.h>
88+#include <mir/input/device_capability.h>
89+#include <mir/input/keyboard_configuration.h>
90+
91+#include <Qt>
92+#include <QTimer>
93+
94+#include "inputdeviceobserver.h"
95+#include "mirsingleton.h"
96+#include "logging.h"
97+
98+using namespace qtmir;
99+namespace mi = mir::input;
100+
101+// Note: MirInputDeviceObserver has affinity to a Mir thread, but it is expected setKeymap will be called from the Qt GUI thread
102+
103+MirInputDeviceObserver::MirInputDeviceObserver(QObject *parent):
104+ QObject(parent)
105+{
106+ // NB: have to use a Direct connection here, as it's called from Qt GUI thread
107+ connect(Mir::instance(), &Mir::currentKeymapChanged, this, &MirInputDeviceObserver::setKeymap, Qt::DirectConnection);
108+}
109+
110+void MirInputDeviceObserver::setKeymap(const QString &keymap)
111+{
112+ QMutexLocker locker(&m_mutex); // lock so that Qt and Mir don't apply the keymap at the same time
113+
114+ if (keymap != m_keymap) {
115+ qCDebug(QTMIR_MIR_KEYMAP) << "SET KEYMAP" << keymap;
116+ m_keymap = keymap;
117+ applyKeymap();
118+ }
119+}
120+
121+void MirInputDeviceObserver::applyKeymap()
122+{
123+ Q_FOREACH(const auto &device, m_devices) {
124+ applyKeymap(device);
125+ }
126+}
127+
128+void MirInputDeviceObserver::device_added(const std::shared_ptr<mi::Device> &device)
129+{
130+ QMutexLocker locker(&m_mutex); // lock so that Qt and Mir don't apply the keymap at the same time
131+
132+ if (mir::contains(device->capabilities(), mi::DeviceCapability::keyboard) &&
133+ mir::contains(device->capabilities(), mi::DeviceCapability::alpha_numeric)) {
134+ qCDebug(QTMIR_MIR_KEYMAP) << "Device added" << device->id();
135+ m_devices.append(device);
136+ applyKeymap(device);
137+ }
138+}
139+
140+void MirInputDeviceObserver::device_removed(const std::shared_ptr<mi::Device> &device)
141+{
142+ QMutexLocker locker(&m_mutex); // lock so that Qt and Mir don't apply the keymap at the same time
143+
144+ if (device && m_devices.contains(device)) {
145+ qCDebug(QTMIR_MIR_KEYMAP) << "Device removed" << device->id();
146+ m_devices.removeAll(device);
147+ }
148+}
149+
150+void MirInputDeviceObserver::applyKeymap(const std::shared_ptr<mi::Device> &device)
151+{
152+ if (!m_keymap.isEmpty()) {
153+ const QStringList stringList = m_keymap.split('+', QString::SkipEmptyParts);
154+
155+ const QString &layout = stringList.at(0);
156+ QString variant;
157+
158+ if (stringList.count() > 1) {
159+ variant = stringList.at(1);
160+ }
161+
162+ qCDebug(QTMIR_MIR_KEYMAP) << "Applying keymap" << layout << variant << "on" << device->id() << QString::fromStdString(device->name());
163+ mi::KeyboardConfiguration oldConfig;
164+ mi::Keymap keymap;
165+ if (device->keyboard_configuration().is_set()) { // preserve the model and options
166+ oldConfig = device->keyboard_configuration().value();
167+ keymap.model = oldConfig.device_keymap.model;
168+ keymap.options = oldConfig.device_keymap.options;
169+ }
170+ keymap.layout = layout.toStdString();
171+ keymap.variant = variant.toStdString();
172+
173+ device->apply_keyboard_configuration(std::move(keymap));
174+ qCDebug(QTMIR_MIR_KEYMAP) << "Keymap applied";
175+ }
176+}
177
178=== added file 'src/platforms/mirserver/inputdeviceobserver.h'
179--- src/platforms/mirserver/inputdeviceobserver.h 1970-01-01 00:00:00 +0000
180+++ src/platforms/mirserver/inputdeviceobserver.h 2017-01-04 17:43:31 +0000
181@@ -0,0 +1,55 @@
182+/*
183+ * Copyright (C) 2016-2017 Canonical, Ltd.
184+ *
185+ * This program is free software: you can redistribute it and/or modify it under
186+ * the terms of the GNU Lesser General Public License version 3, as published by
187+ * the Free Software Foundation.
188+ *
189+ * This program is distributed in the hope that it will be useful, but WITHOUT
190+ * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
191+ * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
192+ * Lesser General Public License for more details.
193+ *
194+ * You should have received a copy of the GNU Lesser General Public License
195+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
196+ */
197+
198+#ifndef INPUTDEVICEOBSERVER_H
199+#define INPUTDEVICEOBSERVER_H
200+
201+#include <mir/input/input_device_observer.h>
202+
203+#include <QObject>
204+#include <QString>
205+#include <QVector>
206+#include <QMutex>
207+
208+namespace qtmir {
209+
210+class MirInputDeviceObserver: public QObject, public mir::input::InputDeviceObserver
211+{
212+ Q_OBJECT
213+public:
214+ MirInputDeviceObserver(QObject * parent = nullptr);
215+ ~MirInputDeviceObserver() = default;
216+
217+protected:
218+ void device_added(std::shared_ptr<mir::input::Device> const& device) override;
219+ void device_changed(std::shared_ptr<mir::input::Device> const& /*device*/) override {}
220+ void device_removed(std::shared_ptr<mir::input::Device> const& device) override;
221+ void changes_complete() override {}
222+
223+private Q_SLOTS:
224+ void setKeymap(const QString &keymap);
225+
226+private:
227+ void applyKeymap();
228+ void applyKeymap(const std::shared_ptr<mir::input::Device> &device);
229+ QString m_keymap;
230+ QVector<std::shared_ptr<mir::input::Device>> m_devices;
231+ QMutex m_mutex;
232+};
233+
234+} // namespace qtmir
235+
236+#endif
237
238=== modified file 'src/platforms/mirserver/logging.cpp'
239--- src/platforms/mirserver/logging.cpp 2016-07-01 16:15:54 +0000
240+++ src/platforms/mirserver/logging.cpp 2017-01-04 17:43:31 +0000
241@@ -21,6 +21,7 @@
242 Q_LOGGING_CATEGORY(QTMIR_SURFACES, "qtmir.surfaces")
243 Q_LOGGING_CATEGORY(QTMIR_MIR_INPUT, "qtmir.mir.input", QtWarningMsg)
244 Q_LOGGING_CATEGORY(QTMIR_MIR_MESSAGES, "qtmir.mir")
245+Q_LOGGING_CATEGORY(QTMIR_MIR_KEYMAP, "qtmir.mir.keymap")
246 Q_LOGGING_CATEGORY(QTMIR_CLIPBOARD, "qtmir.clipboard")
247 Q_LOGGING_CATEGORY(QTMIR_SENSOR_MESSAGES, "qtmir.sensor")
248 Q_LOGGING_CATEGORY(QTMIR_SCREENS, "qtmir.screens")
249
250=== modified file 'src/platforms/mirserver/logging.h'
251--- src/platforms/mirserver/logging.h 2016-07-01 16:15:54 +0000
252+++ src/platforms/mirserver/logging.h 2017-01-04 17:43:31 +0000
253@@ -24,6 +24,7 @@
254 Q_DECLARE_LOGGING_CATEGORY(QTMIR_MIR_MESSAGES)
255 Q_DECLARE_LOGGING_CATEGORY(QTMIR_SENSOR_MESSAGES)
256 Q_DECLARE_LOGGING_CATEGORY(QTMIR_MIR_INPUT)
257+Q_DECLARE_LOGGING_CATEGORY(QTMIR_MIR_KEYMAP)
258 Q_DECLARE_LOGGING_CATEGORY(QTMIR_CLIPBOARD)
259 Q_DECLARE_LOGGING_CATEGORY(QTMIR_SCREENS)
260 Q_DECLARE_LOGGING_CATEGORY(QTMIR_DBUS)
261
262=== modified file 'src/platforms/mirserver/mirserverhooks.cpp'
263--- src/platforms/mirserver/mirserverhooks.cpp 2016-11-03 20:17:46 +0000
264+++ src/platforms/mirserver/mirserverhooks.cpp 2017-01-04 17:43:31 +0000
265@@ -23,11 +23,13 @@
266 #include "promptsessionlistener.h"
267 #include "screenscontroller.h"
268 #include "logging.h"
269+#include "inputdeviceobserver.h"
270
271 // mir
272 #include <mir/server.h>
273 #include <mir/graphics/cursor.h>
274 #include <mir/scene/prompt_session_listener.h>
275+#include <mir/input/input_device_hub.h>
276
277 namespace mg = mir::graphics;
278 namespace ms = mir::scene;
279@@ -74,6 +76,7 @@
280 std::weak_ptr<mir::graphics::Display> m_mirDisplay;
281 std::weak_ptr<mir::shell::DisplayConfigurationController> m_mirDisplayConfigurationController;
282 std::weak_ptr<mir::scene::PromptSessionManager> m_mirPromptSessionManager;
283+ std::weak_ptr<mir::input::InputDeviceHub> m_inputDeviceHub;
284 };
285
286 qtmir::MirServerHooks::MirServerHooks() :
287@@ -104,6 +107,7 @@
288 self->m_mirDisplay = server.the_display();
289 self->m_mirDisplayConfigurationController = server.the_display_configuration_controller();
290 self->m_mirPromptSessionManager = server.the_prompt_session_manager();
291+ self->m_inputDeviceHub = server.the_input_device_hub();
292 });
293 }
294
295@@ -131,12 +135,25 @@
296 throw std::logic_error("No display available. Server not running?");
297 }
298
299+std::shared_ptr<mir::input::InputDeviceHub> qtmir::MirServerHooks::theInputDeviceHub() const
300+{
301+ if (auto result = self->m_inputDeviceHub.lock())
302+ return result;
303+
304+ throw std::logic_error("No input device hub available. Server not running?");
305+}
306+
307 QSharedPointer<ScreensController> qtmir::MirServerHooks::createScreensController(QSharedPointer<ScreensModel> const &screensModel) const
308 {
309 return QSharedPointer<ScreensController>(
310 new ScreensController(screensModel, theMirDisplay(), self->m_mirDisplayConfigurationController.lock()));
311 }
312
313+void qtmir::MirServerHooks::createInputDeviceObserver()
314+{
315+ theInputDeviceHub()->add_observer(std::make_shared<qtmir::MirInputDeviceObserver>());
316+}
317+
318 PromptSessionListenerImpl::~PromptSessionListenerImpl() = default;
319
320 void PromptSessionListenerImpl::starting(std::shared_ptr<ms::PromptSession> const& prompt_session)
321
322=== modified file 'src/platforms/mirserver/mirserverhooks.h'
323--- src/platforms/mirserver/mirserverhooks.h 2016-11-03 20:17:46 +0000
324+++ src/platforms/mirserver/mirserverhooks.h 2017-01-04 17:43:31 +0000
325@@ -25,6 +25,7 @@
326 namespace mir { class Server; }
327 namespace mir { namespace scene { class PromptSessionManager; }}
328 namespace mir { namespace graphics { class Display; }}
329+namespace mir { namespace input { class InputDeviceHub; } }
330
331 class PromptSessionListener;
332 class ScreensController;
333@@ -42,8 +43,10 @@
334 PromptSessionListener *promptSessionListener() const;
335 std::shared_ptr<mir::scene::PromptSessionManager> thePromptSessionManager() const;
336 std::shared_ptr<mir::graphics::Display> theMirDisplay() const;
337+ std::shared_ptr<mir::input::InputDeviceHub> theInputDeviceHub() const;
338
339 QSharedPointer<ScreensController> createScreensController(QSharedPointer<ScreensModel> const &screensModel) const;
340+ void createInputDeviceObserver();
341
342 private:
343 struct Self;
344
345=== modified file 'src/platforms/mirserver/mirsingleton.cpp'
346--- src/platforms/mirserver/mirsingleton.cpp 2016-11-03 20:17:46 +0000
347+++ src/platforms/mirserver/mirsingleton.cpp 2017-01-04 17:43:31 +0000
348@@ -1,3 +1,19 @@
349+/*
350+ * Copyright (C) 2015-2016 Canonical, Ltd.
351+ *
352+ * This program is free software: you can redistribute it and/or modify it under
353+ * the terms of the GNU Lesser General Public License version 3, as published by
354+ * the Free Software Foundation.
355+ *
356+ * This program is distributed in the hope that it will be useful, but WITHOUT
357+ * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
358+ * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
359+ * Lesser General Public License for more details.
360+ *
361+ * You should have received a copy of the GNU Lesser General Public License
362+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
363+ */
364+
365 #include "mirsingleton.h"
366
367 qtmir::Mir *qtmir::Mir::m_instance = nullptr;
368@@ -32,3 +48,17 @@
369 {
370 return m_cursorName;
371 }
372+
373+QString qtmir::Mir::currentKeymap() const
374+{
375+ return m_currentKeymap;
376+}
377+
378+void qtmir::Mir::setCurrentKeymap(const QString &currentKeymap)
379+{
380+ if (m_currentKeymap == currentKeymap)
381+ return;
382+
383+ m_currentKeymap = currentKeymap;
384+ Q_EMIT currentKeymapChanged(m_currentKeymap);
385+}
386
387=== modified file 'src/platforms/mirserver/mirsingleton.h'
388--- src/platforms/mirserver/mirsingleton.h 2015-09-22 20:22:20 +0000
389+++ src/platforms/mirserver/mirsingleton.h 2017-01-04 17:43:31 +0000
390@@ -1,5 +1,5 @@
391 /*
392- * Copyright (C) 2015 Canonical, Ltd.
393+ * Copyright (C) 2015-2016 Canonical, Ltd.
394 *
395 * This program is free software: you can redistribute it and/or modify it under
396 * the terms of the GNU Lesser General Public License version 3, as published by
397@@ -33,11 +33,15 @@
398 void setCursorName(const QString &cursorName) override;
399 QString cursorName() const override;
400
401+ QString currentKeymap() const override;
402+ void setCurrentKeymap(const QString &currentKeymap) override;
403+
404 private:
405 Mir();
406 Q_DISABLE_COPY(Mir)
407
408 QString m_cursorName;
409+ QString m_currentKeymap;
410 static qtmir::Mir *m_instance;
411 };
412
413
414=== modified file 'src/platforms/mirserver/qmirserver_p.cpp'
415--- src/platforms/mirserver/qmirserver_p.cpp 2016-11-16 17:39:06 +0000
416+++ src/platforms/mirserver/qmirserver_p.cpp 2017-01-04 17:43:31 +0000
417@@ -117,6 +117,7 @@
418 {
419 screensModel->update();
420 screensController = m_mirServerHooks.createScreensController(screensModel);
421+ m_mirServerHooks.createInputDeviceObserver();
422 });
423
424 runner.add_start_callback(startCallback);
425
426=== modified file 'src/platforms/mirserver/qteventfeeder.cpp'
427--- src/platforms/mirserver/qteventfeeder.cpp 2016-11-03 20:17:46 +0000
428+++ src/platforms/mirserver/qteventfeeder.cpp 2017-01-04 17:43:31 +0000
429@@ -27,7 +27,6 @@
430 #include <qpa/qplatformintegration.h>
431 #include <qpa/qwindowsysteminterface_p.h>
432 #include <QGuiApplication>
433-#include <private/qguiapplication_p.h>
434 #include <QTextCodec>
435 #include <QDebug>
436
437@@ -677,22 +676,6 @@
438 }
439 int keyCode = translateKeysym(xk_sym, text);
440
441- QPlatformInputContext* context = QGuiApplicationPrivate::platformIntegration()->inputContext();
442- if (context) {
443- // TODO: consider event.repeat_count
444- QKeyEvent qKeyEvent(keyType, keyCode, modifiers,
445- mir_keyboard_event_scan_code(kev),
446- mir_keyboard_event_key_code(kev),
447- mir_keyboard_event_modifiers(kev),
448- text, is_auto_rep);
449- qKeyEvent.setTimestamp(timestamp.count());
450- if (context->filterEvent(&qKeyEvent)) {
451- qCDebug(QTMIR_MIR_INPUT) << "Received" << qPrintable(mirKeyboardEventToString(kev))
452- << "but not dispatching as it was filtered out by input context";
453- return;
454- }
455- }
456-
457 qCDebug(QTMIR_MIR_INPUT).nospace() << "Received " << qPrintable(mirKeyboardEventToString(kev))
458 << ". Dispatching to " << mQtWindowSystem->focusedWindow();
459
460
461=== modified file 'src/platforms/mirserver/surfaceobserver.h'
462--- src/platforms/mirserver/surfaceobserver.h 2016-12-13 15:31:26 +0000
463+++ src/platforms/mirserver/surfaceobserver.h 2017-01-04 17:43:31 +0000
464@@ -50,7 +50,6 @@
465 void attributeChanged(const MirSurfaceAttrib attribute, const int value);
466 void framesPosted();
467 void resized(const QSize &size);
468- void keymapChanged(const QString &rules, const QString &variant);
469 void nameChanged(const QString &name);
470 void cursorChanged(const QCursor &cursor);
471
472
473=== modified file 'tests/mirserver/QtEventFeeder/mock_qtwindowsystem.h'
474--- tests/mirserver/QtEventFeeder/mock_qtwindowsystem.h 2016-06-23 13:22:42 +0000
475+++ tests/mirserver/QtEventFeeder/mock_qtwindowsystem.h 2017-01-04 17:43:31 +0000
476@@ -28,14 +28,25 @@
477 MOCK_METHOD1(getWindowForTouchPoint, QWindow*(const QPoint &point));
478 MOCK_METHOD0(lastWindow, QWindow*());
479 MOCK_METHOD0(focusedWindow, QWindow*());
480+ // ignores the last parameter count, due to parameter limit in gmock
481+ MOCK_METHOD10(handleExtendedKeyEvent,
482+ void(QWindow *window, ulong timestamp, QEvent::Type type,
483+ int key, Qt::KeyboardModifiers modifiers,
484+ quint32 nativeScanCode, quint32 nativeVirtualKey,
485+ quint32 nativeModifiers, const QString &text,
486+ bool autorep));
487
488- // Wanted to use GMock, but MOCK_METHOD11 not implemented
489- void handleExtendedKeyEvent(QWindow */*window*/, ulong /*timestamp*/, QEvent::Type /*type*/, int /*key*/,
490- Qt::KeyboardModifiers /*modifiers*/,
491- quint32 /*nativeScanCode*/, quint32 /*nativeVirtualKey*/,
492- quint32 /*nativeModifiers*/,
493- const QString& /*text*/ = QString(), bool /*autorep*/ = false,
494- ushort /*count*/ = 1) {}
495+ void handleExtendedKeyEvent(QWindow *window, ulong timestamp, QEvent::Type type, int key,
496+ Qt::KeyboardModifiers modifiers,
497+ quint32 nativeScanCode, quint32 nativeVirtualKey,
498+ quint32 nativeModifiers,
499+ const QString& text, bool autorep,
500+ ushort /*count*/ )
501+ {
502+ handleExtendedKeyEvent(window, timestamp, type, key, modifiers,
503+ nativeScanCode, nativeVirtualKey,
504+ nativeModifiers, text, autorep);
505+ }
506
507 MOCK_METHOD5(handleTouchEvent, void(QWindow *window, ulong timestamp, QTouchDevice *device,
508 const QList<struct QWindowSystemInterface::TouchPoint> &points,
509
510=== modified file 'tests/mirserver/QtEventFeeder/qteventfeeder_test.cpp'
511--- tests/mirserver/QtEventFeeder/qteventfeeder_test.cpp 2016-05-30 10:23:52 +0000
512+++ tests/mirserver/QtEventFeeder/qteventfeeder_test.cpp 2017-01-04 17:43:31 +0000
513@@ -27,6 +27,9 @@
514
515 #include "mock_qtwindowsystem.h"
516
517+#include <linux/input.h>
518+#include <xkbcommon/xkbcommon-keysyms.h>
519+
520 using ::testing::_;
521 using ::testing::AllOf;
522 using ::testing::AnyNumber;
523@@ -280,3 +283,42 @@
524 qtEventFeeder->dispatch(*ev2);
525 ASSERT_TRUE(Mock::VerifyAndClearExpectations(mockWindowSystem));
526 }
527+
528+TEST_F(QtEventFeederTest, composeKeysNotFilteredOut)
529+{
530+ auto down = mir_keyboard_action_down;
531+ auto up = mir_keyboard_action_up;
532+
533+ auto dispatch_key_event = [&](MirKeyboardAction action, int scan_code, xkb_keysym_t key_sym)
534+ {
535+ qtEventFeeder->dispatch(*mev::make_event(
536+ MirInputDeviceId{0}, std::chrono::nanoseconds{0}, std::vector<uint8_t>{},
537+ action, key_sym, scan_code, mir_input_event_modifier_none));
538+ };
539+
540+ InSequence seq;
541+ EXPECT_CALL(
542+ *mockWindowSystem,
543+ handleExtendedKeyEvent(_, _, QEvent::KeyPress, _, _, KEY_RIGHTSHIFT,
544+ XKB_KEY_Shift_R, _, _, _));
545+ EXPECT_CALL(*mockWindowSystem,
546+ handleExtendedKeyEvent(_, _, QEvent::KeyPress, _, _,
547+ KEY_APOSTROPHE, XKB_KEY_NoSymbol,
548+ _, _, _));
549+ EXPECT_CALL(*mockWindowSystem,
550+ handleExtendedKeyEvent(_, _, QEvent::KeyRelease, _, _,
551+ KEY_APOSTROPHE, XKB_KEY_NoSymbol,
552+ _, _, _));
553+ EXPECT_CALL(
554+ *mockWindowSystem,
555+ handleExtendedKeyEvent(_, _, QEvent::KeyRelease, _, _, KEY_RIGHTSHIFT,
556+ XKB_KEY_Shift_R, _, _, _));
557+ EXPECT_CALL(*mockWindowSystem,
558+ handleExtendedKeyEvent(_, _, QEvent::KeyPress, _, _, KEY_U,
559+ XKB_KEY_udiaeresis, _, _, _));
560+ dispatch_key_event(down, KEY_RIGHTSHIFT, XKB_KEY_Shift_R);
561+ dispatch_key_event(down, KEY_APOSTROPHE, XKB_KEY_NoSymbol);
562+ dispatch_key_event(up, KEY_APOSTROPHE, XKB_KEY_NoSymbol);
563+ dispatch_key_event(up, KEY_RIGHTSHIFT, XKB_KEY_Shift_R);
564+ dispatch_key_event(down, KEY_U, XKB_KEY_udiaeresis);
565+}

Subscribers

People subscribed via source and target branches