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

Proposed by Daniel d'Andrada
Status: Merged
Approved by: Gerry Boland
Approved revision: 629
Merged at revision: 630
Proposed branch: lp:~dandrader/qtmir/keyState
Merge into: lp:qtmir
Prerequisite: lp:~gerboland/qtmir/availableDesktopAreaTrial
Diff against target: 211 lines (+115/-6)
3 files modified
src/modules/Unity/Application/mirsurface.cpp (+94/-4)
src/modules/Unity/Application/mirsurface.h (+20/-1)
src/platforms/mirserver/eventbuilder.h (+1/-1)
To merge this branch: bzr merge lp:~dandrader/qtmir/keyState
Reviewer Review Type Date Requested Status
Gerry Boland (community) Approve
Unity8 CI Bot (community) continuous-integration Approve
Review via email: mp+320930@code.launchpad.net

This proposal supersedes a proposal from 2017-03-22.

Commit message

Ensure the window that got a key down also gets the corresponding key up

Otherwise it will be left in a inconsistent state (with a pressed key hanging around).

QQuickWindow's input dispatching doesn't guarantee that for its QQuickItem.
So we have to do it ourselves.

This can happen when qml active focus changes in response to a key press.
Eg: client creates a child window in response to a Ctrl+O. By the time the user
releases the Ctrl, active focus will already be in the child window, so the child window
will get the release event instead of the top-level one.

Description of the change

* Are there any related MPs required for this MP to build/function as expected? Please list.
https://code.launchpad.net/~dandrader/unity8/childWindowFocus/+merge/320712

To post a comment you must log in.
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Gerry Boland (gerboland) wrote : Posted in a previous version of this proposal

If the surface looses active focus, don't we inform Mir? Why not send the key release event then? Would be more efficient than checking on every key_up

Also, please avoid using singleton pattern, it makes testing impossible. And speaking of which, can you add test?

review: Needs Information
Revision history for this message
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

On 20/03/2017 14:08, Gerry Boland wrote:
> Review: Needs Information
>
> If the surface looses active focus, don't we inform Mir?

It's the other way around. Mir surface focus is what drives qml active
focus. Nowadays qml active focus by itself doesn't carry much meaning in
terms of window management.

> Why not send the key release event then? Would be more efficient than checking on every key_up

Because the key is actually still pressed. If the same window gets focus
again moments later and only then the key is lifted the client would
receive a second key_up event. Unless we keep track of what lies we have
told already to the clients or devise some other special rules.

> Also, please avoid using singleton pattern, it makes testing impossible.

Yeah, that was the easiest (laziest) way to get the two talking (event
feeder and policy). Will see if I can get something nicer. Suggestions?

But before I spend too much time on the implementation details of this
MP I would like to get feedback on the overall approach first.

> And speaking of which, can you add test?

For WindowManagementPolicy?

Revision history for this message
Gerry Boland (gerboland) wrote : Posted in a previous version of this proposal

> On 20/03/2017 14:08, Gerry Boland wrote:
> > Review: Needs Information
> >
> > If the surface looses active focus, don't we inform Mir?
>
> It's the other way around. Mir surface focus is what drives qml active
> focus. Nowadays qml active focus by itself doesn't carry much meaning in
> terms of window management.

There are more things in the QML scene that just MirSurfaces that can get activeFocus though. When activeFocus does switch to the search box in the AppDrawer (as example), we are indeed telling Mir somehow that the MirSurface has not got input focus (I just tested gedit, the cursor does stop flashing when the text box in the app drawer is focused). Since this bug occurs due to focus changing while modifier key is down, my initial thinking was we could use the focus change to implement a workaround.

What I don't know is if the WMpolicy if notified when the MirSurface it has selected to be focused has lost actual input focus (activeFocus). Do you know?

> > Why not send the key release event then? Would be more efficient than
> checking on every key_up
>
> Because the key is actually still pressed. If the same window gets focus
> again moments later and only then the key is lifted the client would
> receive a second key_up event. Unless we keep track of what lies we have
> told already to the clients or devise some other special rules.

This appears to be a QML subtlety we're working around, as a non-QML Qt shell would not suffer from this issue, right? So could the MirSurfaceItem keep track of the key state itself? If the MSI looses activeFocus, it send key-release to the client. Then if it gets focus again, check if key is still down (not sure how tbh, maybe QML sends the key to the Item again immediately??), and if so, re-send it to client.

I'm not 100% certain what is the best approach yet.

> > Also, please avoid using singleton pattern, it makes testing impossible.
>
> Yeah, that was the easiest (laziest) way to get the two talking (event
> feeder and policy). Will see if I can get something nicer. Suggestions?

Nothing better than the usual piping an object around, so everyone who needs it gets a pointer to it. Sorry :(

> But before I spend too much time on the implementation details of this
> MP I would like to get feedback on the overall approach first.

Ah ok, I hope I've supplied that at least

>
> > And speaking of which, can you add test?
>
> For WindowManagementPolicy?

More the logic you're adding. But please ignore me until you settle on the approach you want.

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

On 21/03/2017 09:37, Gerry Boland wrote:
> This appears to be a QML subtlety we're working around, as a non-QML Qt shell would not suffer from this issue, right?

Yes. As I said, QQuickWindow input dispatching not perfectly suited for
shells.

> So could the MirSurfaceItem keep track of the key state itself?
Sure.

> If the MSI looses activeFocus, it send key-release to the client. Then if it gets focus again, check if key is still down (not sure how tbh, maybe QML sends the key to the Item again immediately??), and if so, re-send it to client.

It can't check the key state, but it can ignore a key-up for a key that
it didn't tell its surface it was down (because lied or whatever).

> I'm not 100% certain what is the best approach yet.

Working around QQuickWindow input dispatching will never be beautiful. So, should I go for the MirSurfaceItem-based key filtering/manipulation?

I'm fine either way. I put in the policy to avoid sending fake input to clients and as a general rule-of-thumb of "putting things in the miral policy"/"keep the policy in the loop".

Revision history for this message
Gerry Boland (gerboland) wrote : Posted in a previous version of this proposal

I think I'd prefer using MirSurfaceItem-based key filtering. Since it's working around the QQuickWindow/QML problem, it makes sense to me to confine the workaround in code only used in code exposed to QQuickWindow/QML.

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Gerry Boland (gerboland) wrote : Posted in a previous version of this proposal

+++ src/modules/Unity/Application/mirsurface.h
Could you move the implementation of the PressedKey constructor into the cpp file please, it is big enough a header file.

+++ src/modules/Unity/Application/mirsurface.cpp
You're using QElapsedTimer to get the current value of the monotonic clock, yes? If so, we really only need one QElapsedTimer everywhere, so could make it static singeton and add a static function that calls msecsSinceReference on it and returns the val. wdyt?

Code makes sense, I just need to test. It does need a big comment explaining why we're doing this though. Do the QtWayland guys have the same issue, wonder if they do a similar fix or not...

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

> +++ src/modules/Unity/Application/mirsurface.h
> Could you move the implementation of the PressedKey constructor into the cpp
> file please, it is big enough a header file.

Done.

>
> +++ src/modules/Unity/Application/mirsurface.cpp
> You're using QElapsedTimer to get the current value of the monotonic clock,
> yes? If so, we really only need one QElapsedTimer everywhere, so could make it
> static singeton and add a static function that calls msecsSinceReference on it
> and returns the val. wdyt?

Right. Done.

> Code makes sense, I just need to test. It does need a big comment explaining
> why we're doing this though. Do the QtWayland guys have the same issue, wonder
> if they do a similar fix or not...

Added a big fat comment.

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

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

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

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

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

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

review: Approve (continuous-integration)
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

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

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

review: Approve (continuous-integration)
Revision history for this message
Gerry Boland (gerboland) wrote :

How about using the anonymous namespace we have? I can stick this in there

static qint64 msecsSinceReference()
{
    static QElapsedTimer elapsedTimer;
    elapsedTimer.start();
    return elapsedTimer.msecsSinceReference();
}

and it works ok. After all it is just a function to return a timestamp, which is a purely internal implementation thing that doesn't need to be a class member IMO.

+ if (m_focused) {
   releaseAllPressedKeys();
Why not on unfocus?

lp:~dandrader/qtmir/keyState updated
628. By Daniel d'Andrada

Some refactoring as per review feedback

629. By Daniel d'Andrada

update copyright header

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

On 24/03/2017 15:37, Gerry Boland wrote:
> How about using the anonymous namespace we have? I can stick this in there
>
> static qint64 msecsSinceReference()
> {
> static QElapsedTimer elapsedTimer;
> elapsedTimer.start();
> return elapsedTimer.msecsSinceReference();
> }
>
> and it works ok. After all it is just a function to return a timestamp, which is a purely internal implementation thing that doesn't need to be a class member IMO.

Yes, makes for less code. Done.

> + if (m_focused) {
> releaseAllPressedKeys();
> Why not on unfocus?

Because an unfocused surface, by definition, shouldn't be receiving
input events.

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

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

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

review: Approve (continuous-integration)
Revision history for this message
Gerry Boland (gerboland) wrote :

> Because an unfocused surface, by definition, shouldn't be receiving
> input events.
Yeah. Not ideal, but as we have no reliable way to send those events just before a surface is send the unfocus event, this is the best that can be done.

Tested and works ok

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/modules/Unity/Application/mirsurface.cpp'
--- src/modules/Unity/Application/mirsurface.cpp 2017-03-24 18:56:33 +0000
+++ src/modules/Unity/Application/mirsurface.cpp 2017-03-24 18:56:33 +0000
@@ -41,6 +41,7 @@
41#include <logging.h>41#include <logging.h>
4242
43// Qt43// Qt
44#include <QElapsedTimer>
44#include <QQmlEngine>45#include <QQmlEngine>
45#include <QScreen>46#include <QScreen>
4647
@@ -68,8 +69,16 @@
68};69};
69Q_DECLARE_FLAGS(DirtyStates, DirtyState)70Q_DECLARE_FLAGS(DirtyStates, DirtyState)
7071
72qint64 msecsSinceReference()
73{
74 static QElapsedTimer elapsedTimer;
75 elapsedTimer.start();
76 return elapsedTimer.msecsSinceReference();
77}
78
71} // namespace {79} // namespace {
7280
81
73class MirSurface::SurfaceObserverImpl : public SurfaceObserver, public mir::scene::SurfaceObserver82class MirSurface::SurfaceObserverImpl : public SurfaceObserver, public mir::scene::SurfaceObserver
74{83{
75public:84public:
@@ -409,6 +418,25 @@
409418
410 m_focused = value;419 m_focused = value;
411 Q_EMIT focusedChanged(value);420 Q_EMIT focusedChanged(value);
421
422 if (m_focused) {
423 /*
424 Ensure the window that got a key down also gets the corresponding key up
425
426 Otherwise it will be left in a inconsistent state (with a pressed key hanging around).
427
428 QQuickWindow's input dispatching doesn't guarantee that for its QQuickItems.
429 So we have to do it ourselves.
430
431 This can happen when qml active focus changes in response to a key press.
432 Eg: client creates a child window in response to a Ctrl+O. By the time the user
433 releases the Ctrl, active focus will already be in the child window, so the child window
434 will get the release event instead of the top-level one. To solve this, once the top-level
435 window gets active focus again, we synthesize KeyRelease events for all keys this window
436 thinks are still pressed.
437 */
438 releaseAllPressedKeys();
439 }
412}440}
413441
414void MirSurface::setViewActiveFocus(qintptr viewId, bool value)442void MirSurface::setViewActiveFocus(qintptr viewId, bool value)
@@ -661,6 +689,18 @@
661689
662void MirSurface::keyPressEvent(QKeyEvent *qtEvent)690void MirSurface::keyPressEvent(QKeyEvent *qtEvent)
663{691{
692 {
693 if (!qtEvent->isAutoRepeat()) {
694 Q_ASSERT(!isKeyPressed(qtEvent->nativeVirtualKey()));
695 PressedKey pressedKey(qtEvent, msecsSinceReference());
696 auto info = EventBuilder::instance()->findInfo(qtEvent->timestamp());
697 if (info) {
698 pressedKey.deviceId = info->deviceId;
699 }
700 m_pressedKeys.append(std::move(pressedKey));
701 }
702 }
703
664 auto ev = EventBuilder::instance()->makeMirEvent(qtEvent);704 auto ev = EventBuilder::instance()->makeMirEvent(qtEvent);
665 auto ev1 = reinterpret_cast<MirKeyboardEvent const*>(ev.get());705 auto ev1 = reinterpret_cast<MirKeyboardEvent const*>(ev.get());
666 m_controller->deliverKeyboardEvent(m_window, ev1);706 m_controller->deliverKeyboardEvent(m_window, ev1);
@@ -669,10 +709,14 @@
669709
670void MirSurface::keyReleaseEvent(QKeyEvent *qtEvent)710void MirSurface::keyReleaseEvent(QKeyEvent *qtEvent)
671{711{
672 auto ev = EventBuilder::instance()->makeMirEvent(qtEvent);712 if (isKeyPressed(qtEvent->nativeVirtualKey())) {
673 auto ev1 = reinterpret_cast<MirKeyboardEvent const*>(ev.get());713 forgetPressedKey(qtEvent->nativeVirtualKey());
674 m_controller->deliverKeyboardEvent(m_window, ev1);714 auto ev = EventBuilder::instance()->makeMirEvent(qtEvent);
675 qtEvent->accept();715 auto ev1 = reinterpret_cast<MirKeyboardEvent const*>(ev.get());
716 m_controller->deliverKeyboardEvent(m_window, ev1);
717 } else {
718 // don't send a release event for a key for which we did not send a press in the first place
719 }
676}720}
677721
678void MirSurface::touchEvent(Qt::KeyboardModifiers mods,722void MirSurface::touchEvent(Qt::KeyboardModifiers mods,
@@ -1264,3 +1308,49 @@
1264 QPoint point(m_surface->top_left().x.as_int(),m_surface->top_left().y.as_int());1308 QPoint point(m_surface->top_left().x.as_int(),m_surface->top_left().y.as_int());
1265 setPosition(point);1309 setPosition(point);
1266}1310}
1311
1312bool MirSurface::isKeyPressed(quint32 nativeVirtualKey) const
1313{
1314 for (const auto &pressedKey : m_pressedKeys) {
1315 if (pressedKey.nativeVirtualKey == nativeVirtualKey) {
1316 return true;
1317 }
1318 }
1319 return false;
1320}
1321
1322void MirSurface::forgetPressedKey(quint32 nativeVirtualKey)
1323{
1324 for (int i = 0; i < m_pressedKeys.count(); ++i) {
1325 if (m_pressedKeys[i].nativeVirtualKey == nativeVirtualKey) {
1326 m_pressedKeys.removeAt(i);
1327 return;
1328 }
1329 }
1330}
1331
1332void MirSurface::releaseAllPressedKeys()
1333{
1334 for (auto &pressedKey : m_pressedKeys) {
1335 auto deltaMs = (ulong)(msecsSinceReference() - pressedKey.msecsSinceReference);
1336 ulong timestamp = pressedKey.timestamp + deltaMs;
1337 std::vector<uint8_t> cookie{};
1338
1339 auto ev = mir::events::make_event(pressedKey.deviceId,
1340 uncompressTimestamp<qtmir::Timestamp>(qtmir::Timestamp(timestamp)),
1341 cookie, mir_keyboard_action_up, pressedKey.nativeVirtualKey, pressedKey.nativeScanCode,
1342 mir_input_event_modifier_none);
1343
1344 auto ev1 = reinterpret_cast<MirKeyboardEvent const*>(ev.get());
1345 m_controller->deliverKeyboardEvent(m_window, ev1);
1346 }
1347 m_pressedKeys.clear();
1348}
1349
1350MirSurface::PressedKey::PressedKey(QKeyEvent *qtEvent, qint64 msecsSinceReference)
1351 : nativeVirtualKey(qtEvent->nativeVirtualKey())
1352 , nativeScanCode(qtEvent->nativeScanCode())
1353 , timestamp(qtEvent->timestamp())
1354 , msecsSinceReference(msecsSinceReference)
1355{
1356}
12671357
=== modified file 'src/modules/Unity/Application/mirsurface.h'
--- src/modules/Unity/Application/mirsurface.h 2017-03-24 18:56:33 +0000
+++ src/modules/Unity/Application/mirsurface.h 2017-03-24 18:56:33 +0000
@@ -1,5 +1,5 @@
1/*1/*
2 * Copyright (C) 2015-2016 Canonical, Ltd.2 * Copyright (C) 2015-2017 Canonical, Ltd.
3 *3 *
4 * This program is free software: you can redistribute it and/or modify it under4 * This program is free software: you can redistribute it and/or modify it under
5 * the terms of the GNU Lesser General Public License version 3, as published by5 * the terms of the GNU Lesser General Public License version 3, as published by
@@ -29,6 +29,8 @@
29#include <QWeakPointer>29#include <QWeakPointer>
30#include <QSet>30#include <QSet>
31#include <QTimer>31#include <QTimer>
32#include <QVector>
33#include <QKeyEvent>
3234
33#include "mirbuffersgtexture.h"35#include "mirbuffersgtexture.h"
34#include "windowcontrollerinterface.h"36#include "windowcontrollerinterface.h"
@@ -214,6 +216,11 @@
214 QPoint convertLocalToDisplayCoords(const QPoint &localPos) const;216 QPoint convertLocalToDisplayCoords(const QPoint &localPos) const;
215 void updatePosition();217 void updatePosition();
216218
219 // Handling of missing key release events from Qt
220 bool isKeyPressed(quint32 nativeVirtualKey) const;
221 void forgetPressedKey(quint32 nativeVirtualKey);
222 void releaseAllPressedKeys();
223
217 const miral::Window m_window;224 const miral::Window m_window;
218 const std::shared_ptr<ExtraWindowInfo> m_extraInfo;225 const std::shared_ptr<ExtraWindowInfo> m_extraInfo;
219 QString m_name;226 QString m_name;
@@ -281,6 +288,18 @@
281 MirSurface *m_parentSurface;288 MirSurface *m_parentSurface;
282289
283 MirSurfaceListModel *m_childSurfaceList;290 MirSurfaceListModel *m_childSurfaceList;
291
292 // Track all keys that we told our mir window are currently pressed
293 struct PressedKey {
294 PressedKey() {}
295 PressedKey(QKeyEvent *qtEvent, qint64 msecsSinceReference);
296 quint32 nativeVirtualKey{0};
297 quint32 nativeScanCode{0};
298 ulong timestamp{0};
299 MirInputDeviceId deviceId{0};
300 qint64 msecsSinceReference{0};
301 };
302 QVector<PressedKey> m_pressedKeys;
284};303};
285304
286} // namespace qtmir305} // namespace qtmir
287306
=== modified file 'src/platforms/mirserver/eventbuilder.h'
--- src/platforms/mirserver/eventbuilder.h 2017-03-03 10:39:24 +0000
+++ src/platforms/mirserver/eventbuilder.h 2017-03-24 18:56:33 +0000
@@ -69,7 +69,6 @@
69 const QList<QTouchEvent::TouchPoint> &qtTouchPoints,69 const QList<QTouchEvent::TouchPoint> &qtTouchPoints,
70 Qt::TouchPointStates /* qtTouchPointStates */,70 Qt::TouchPointStates /* qtTouchPointStates */,
71 ulong qtTimestamp);71 ulong qtTimestamp);
72private:
73 class EventInfo {72 class EventInfo {
74 public:73 public:
75 void store(const MirInputEvent *mirInputEvent, ulong qtTimestamp);74 void store(const MirInputEvent *mirInputEvent, ulong qtTimestamp);
@@ -82,6 +81,7 @@
8281
83 EventInfo *findInfo(ulong qtTimestamp);82 EventInfo *findInfo(ulong qtTimestamp);
8483
84private:
85 mir::EventUPtr makeMirEvent(QInputEvent *qtEvent, int x, int y, MirPointerButtons buttons);85 mir::EventUPtr makeMirEvent(QInputEvent *qtEvent, int x, int y, MirPointerButtons buttons);
8686
8787

Subscribers

People subscribed via source and target branches