Merge lp:~dandrader/qtmir/keyState into lp:qtmir
- keyState
- Merge into trunk
Status: | Superseded | ||||
---|---|---|---|---|---|
Proposed branch: | lp:~dandrader/qtmir/keyState | ||||
Merge into: | lp:qtmir | ||||
Prerequisite: | lp:~ci-train-bot/qtmir/qtmir-ubuntu-zesty-2555 | ||||
Diff against target: |
587 lines (+300/-12) 13 files modified
src/common/mirqtconversion.h (+16/-0) src/common/windowcontrollerinterface.h (+4/-0) src/modules/Unity/Application/mirsurface.cpp (+107/-4) src/modules/Unity/Application/mirsurface.h (+24/-0) src/platforms/mirserver/eventbuilder.h (+1/-1) src/platforms/mirserver/nativeinterface.cpp (+40/-0) src/platforms/mirserver/nativeinterface.h (+1/-0) src/platforms/mirserver/windowcontroller.cpp (+14/-0) src/platforms/mirserver/windowcontroller.h (+3/-0) src/platforms/mirserver/windowmanagementpolicy.cpp (+75/-4) src/platforms/mirserver/windowmanagementpolicy.h (+9/-3) tests/framework/mock_window_controller.h (+3/-0) tests/framework/stub_windowcontroller.h (+3/-0) |
||||
To merge this branch: | bzr merge lp:~dandrader/qtmir/keyState | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Unity8 CI Bot (community) | continuous-integration | Approve | |
Gerry Boland | Pending | ||
Review via email: mp+320710@code.launchpad.net |
This proposal supersedes a proposal from 2017-03-20.
This proposal has been superseded by a proposal from 2017-03-24.
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:/
Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal | # |
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?
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 WindowManagemen
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 WindowManagemen
More the logic you're adding. But please ignore me until you settle on the approach you want.
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-
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".
Gerry Boland (gerboland) wrote : Posted in a previous version of this proposal | # |
I think I'd prefer using MirSurfaceItem-
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:626
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Gerry Boland (gerboland) wrote : | # |
+++ src/modules/
Could you move the implementation of the PressedKey constructor into the cpp file please, it is big enough a header file.
+++ src/modules/
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...
Daniel d'Andrada (dandrader) wrote : | # |
> +++ src/modules/
> Could you move the implementation of the PressedKey constructor into the cpp
> file please, it is big enough a header file.
Done.
>
> +++ src/modules/
> 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.
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:627
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:627
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:627
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
- 626. By Daniel d'Andrada
- 627. By Daniel d'Andrada
-
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. - 628. By Daniel d'Andrada
-
Some refactoring as per review feedback
- 629. By Daniel d'Andrada
-
update copyright header
Unmerged revisions
Preview Diff
1 | === modified file 'src/common/mirqtconversion.h' |
2 | --- src/common/mirqtconversion.h 2017-02-07 15:02:20 +0000 |
3 | +++ src/common/mirqtconversion.h 2017-03-24 13:53:28 +0000 |
4 | @@ -113,6 +113,22 @@ |
5 | } |
6 | } |
7 | |
8 | +inline MirWindowType toMirType(Mir::Type type) |
9 | +{ |
10 | + switch (type) { |
11 | + case Mir::NormalType: return mir_window_type_normal; |
12 | + case Mir::UtilityType: return mir_window_type_utility; |
13 | + case Mir::DialogType: return mir_window_type_dialog; |
14 | + case Mir::GlossType: return mir_window_type_gloss; |
15 | + case Mir::FreeStyleType: return mir_window_type_freestyle; |
16 | + case Mir::MenuType: return mir_window_type_menu; |
17 | + case Mir::InputMethodType: return mir_window_type_inputmethod; |
18 | + case Mir::SatelliteType: return mir_window_type_satellite; |
19 | + case Mir::TipType: return mir_window_type_tip; |
20 | + default: Q_UNREACHABLE(); |
21 | + } |
22 | +} |
23 | + |
24 | inline Mir::ShellChrome toQtShellChrome(MirShellChrome chrome) |
25 | { |
26 | switch (chrome) { |
27 | |
28 | === modified file 'src/common/windowcontrollerinterface.h' |
29 | --- src/common/windowcontrollerinterface.h 2016-11-03 20:17:46 +0000 |
30 | +++ src/common/windowcontrollerinterface.h 2017-03-24 13:53:28 +0000 |
31 | @@ -23,6 +23,7 @@ |
32 | |
33 | #include <QPoint> |
34 | #include <QSize> |
35 | +#include <QMargins> |
36 | |
37 | // Unity API |
38 | #include <unity/shell/application/Mir.h> |
39 | @@ -51,6 +52,9 @@ |
40 | virtual void deliverKeyboardEvent(const miral::Window &window, const MirKeyboardEvent *event) = 0; |
41 | virtual void deliverTouchEvent (const miral::Window &window, const MirTouchEvent *event) = 0; |
42 | virtual void deliverPointerEvent (const miral::Window &window, const MirPointerEvent *event) = 0; |
43 | + |
44 | + virtual void setWindowConfinementRegions(const QVector<QRect> ®ions) = 0; |
45 | + virtual void setWindowMargins(Mir::Type windowType, const QMargins &margins) = 0; |
46 | }; |
47 | |
48 | } // namespace qtmir |
49 | |
50 | === modified file 'src/modules/Unity/Application/mirsurface.cpp' |
51 | --- src/modules/Unity/Application/mirsurface.cpp 2017-03-20 21:15:13 +0000 |
52 | +++ src/modules/Unity/Application/mirsurface.cpp 2017-03-24 13:53:28 +0000 |
53 | @@ -70,6 +70,9 @@ |
54 | |
55 | } // namespace { |
56 | |
57 | + |
58 | +QElapsedTimer MirSurface::m_elapsedTimer; |
59 | + |
60 | class MirSurface::SurfaceObserverImpl : public SurfaceObserver, public mir::scene::SurfaceObserver |
61 | { |
62 | public: |
63 | @@ -409,6 +412,25 @@ |
64 | |
65 | m_focused = value; |
66 | Q_EMIT focusedChanged(value); |
67 | + |
68 | + if (m_focused) { |
69 | + /* |
70 | + Ensure the window that got a key down also gets the corresponding key up |
71 | + |
72 | + Otherwise it will be left in a inconsistent state (with a pressed key hanging around). |
73 | + |
74 | + QQuickWindow's input dispatching doesn't guarantee that for its QQuickItems. |
75 | + So we have to do it ourselves. |
76 | + |
77 | + This can happen when qml active focus changes in response to a key press. |
78 | + Eg: client creates a child window in response to a Ctrl+O. By the time the user |
79 | + releases the Ctrl, active focus will already be in the child window, so the child window |
80 | + will get the release event instead of the top-level one. To solve this, once the top-level |
81 | + window gets active focus again, we synthesize KeyRelease events for all keys this window |
82 | + thinks are still pressed. |
83 | + */ |
84 | + releaseAllPressedKeys(); |
85 | + } |
86 | } |
87 | |
88 | void MirSurface::setViewActiveFocus(qintptr viewId, bool value) |
89 | @@ -508,6 +530,13 @@ |
90 | if (m_position != newPosition) { |
91 | m_position = newPosition; |
92 | Q_EMIT positionChanged(newPosition); |
93 | + |
94 | + // Parent might have moved but children might stay put (in display coords). This |
95 | + // means different local child coords, hence the need to update them. |
96 | + for (int i = 0; i < m_childSurfaceList->count(); ++i) { |
97 | + auto childSurface = static_cast<MirSurface*>(m_childSurfaceList->get(i)); |
98 | + childSurface->updatePosition(); |
99 | + } |
100 | } |
101 | } |
102 | |
103 | @@ -654,6 +683,18 @@ |
104 | |
105 | void MirSurface::keyPressEvent(QKeyEvent *qtEvent) |
106 | { |
107 | + { |
108 | + if (!qtEvent->isAutoRepeat()) { |
109 | + Q_ASSERT(!isKeyPressed(qtEvent->nativeVirtualKey())); |
110 | + PressedKey pressedKey(qtEvent, msecsSinceReference()); |
111 | + auto info = EventBuilder::instance()->findInfo(qtEvent->timestamp()); |
112 | + if (info) { |
113 | + pressedKey.deviceId = info->deviceId; |
114 | + } |
115 | + m_pressedKeys.append(std::move(pressedKey)); |
116 | + } |
117 | + } |
118 | + |
119 | auto ev = EventBuilder::instance()->makeMirEvent(qtEvent); |
120 | auto ev1 = reinterpret_cast<MirKeyboardEvent const*>(ev.get()); |
121 | m_controller->deliverKeyboardEvent(m_window, ev1); |
122 | @@ -662,10 +703,14 @@ |
123 | |
124 | void MirSurface::keyReleaseEvent(QKeyEvent *qtEvent) |
125 | { |
126 | - auto ev = EventBuilder::instance()->makeMirEvent(qtEvent); |
127 | - auto ev1 = reinterpret_cast<MirKeyboardEvent const*>(ev.get()); |
128 | - m_controller->deliverKeyboardEvent(m_window, ev1); |
129 | - qtEvent->accept(); |
130 | + if (isKeyPressed(qtEvent->nativeVirtualKey())) { |
131 | + forgetPressedKey(qtEvent->nativeVirtualKey()); |
132 | + auto ev = EventBuilder::instance()->makeMirEvent(qtEvent); |
133 | + auto ev1 = reinterpret_cast<MirKeyboardEvent const*>(ev.get()); |
134 | + m_controller->deliverKeyboardEvent(m_window, ev1); |
135 | + } else { |
136 | + // don't send a release event for a key for which we did not send a press in the first place |
137 | + } |
138 | } |
139 | |
140 | void MirSurface::touchEvent(Qt::KeyboardModifiers mods, |
141 | @@ -1251,3 +1296,61 @@ |
142 | { |
143 | return m_childSurfaceList; |
144 | } |
145 | + |
146 | +void MirSurface::updatePosition() |
147 | +{ |
148 | + QPoint point(m_surface->top_left().x.as_int(),m_surface->top_left().y.as_int()); |
149 | + setPosition(point); |
150 | +} |
151 | + |
152 | +bool MirSurface::isKeyPressed(quint32 nativeVirtualKey) const |
153 | +{ |
154 | + for (const auto &pressedKey : m_pressedKeys) { |
155 | + if (pressedKey.nativeVirtualKey == nativeVirtualKey) { |
156 | + return true; |
157 | + } |
158 | + } |
159 | + return false; |
160 | +} |
161 | + |
162 | +void MirSurface::forgetPressedKey(quint32 nativeVirtualKey) |
163 | +{ |
164 | + for (int i = 0; i < m_pressedKeys.count(); ++i) { |
165 | + if (m_pressedKeys[i].nativeVirtualKey == nativeVirtualKey) { |
166 | + m_pressedKeys.removeAt(i); |
167 | + return; |
168 | + } |
169 | + } |
170 | +} |
171 | + |
172 | +void MirSurface::releaseAllPressedKeys() |
173 | +{ |
174 | + for (auto &pressedKey : m_pressedKeys) { |
175 | + auto deltaMs = (ulong)(msecsSinceReference() - pressedKey.msecsSinceReference); |
176 | + ulong timestamp = pressedKey.timestamp + deltaMs; |
177 | + std::vector<uint8_t> cookie{}; |
178 | + |
179 | + auto ev = mir::events::make_event(pressedKey.deviceId, |
180 | + uncompressTimestamp<qtmir::Timestamp>(qtmir::Timestamp(timestamp)), |
181 | + cookie, mir_keyboard_action_up, pressedKey.nativeVirtualKey, pressedKey.nativeScanCode, |
182 | + mir_input_event_modifier_none); |
183 | + |
184 | + auto ev1 = reinterpret_cast<MirKeyboardEvent const*>(ev.get()); |
185 | + m_controller->deliverKeyboardEvent(m_window, ev1); |
186 | + } |
187 | + m_pressedKeys.clear(); |
188 | +} |
189 | + |
190 | +qint64 MirSurface::msecsSinceReference() |
191 | +{ |
192 | + m_elapsedTimer.start(); |
193 | + return m_elapsedTimer.msecsSinceReference(); |
194 | +} |
195 | + |
196 | +MirSurface::PressedKey::PressedKey(QKeyEvent *qtEvent, qint64 msecsSinceReference) |
197 | + : nativeVirtualKey(qtEvent->nativeVirtualKey()) |
198 | + , nativeScanCode(qtEvent->nativeScanCode()) |
199 | + , timestamp(qtEvent->timestamp()) |
200 | + , msecsSinceReference(msecsSinceReference) |
201 | +{ |
202 | +} |
203 | |
204 | === modified file 'src/modules/Unity/Application/mirsurface.h' |
205 | --- src/modules/Unity/Application/mirsurface.h 2017-03-20 21:15:13 +0000 |
206 | +++ src/modules/Unity/Application/mirsurface.h 2017-03-24 13:53:28 +0000 |
207 | @@ -22,6 +22,7 @@ |
208 | |
209 | // Qt |
210 | #include <QCursor> |
211 | +#include <QElapsedTimer> |
212 | #include <QMutex> |
213 | #include <QPointer> |
214 | #include <QRect> |
215 | @@ -29,6 +30,8 @@ |
216 | #include <QWeakPointer> |
217 | #include <QSet> |
218 | #include <QTimer> |
219 | +#include <QVector> |
220 | +#include <QKeyEvent> |
221 | |
222 | #include "mirbuffersgtexture.h" |
223 | #include "windowcontrollerinterface.h" |
224 | @@ -212,6 +215,13 @@ |
225 | void onHeightIncrementChanged(int incHeight); |
226 | QPoint convertDisplayToLocalCoords(const QPoint &displayPos) const; |
227 | QPoint convertLocalToDisplayCoords(const QPoint &localPos) const; |
228 | + void updatePosition(); |
229 | + |
230 | + // Handling of missing key release events from Qt |
231 | + bool isKeyPressed(quint32 nativeVirtualKey) const; |
232 | + void forgetPressedKey(quint32 nativeVirtualKey); |
233 | + void releaseAllPressedKeys(); |
234 | + static qint64 msecsSinceReference(); |
235 | |
236 | const miral::Window m_window; |
237 | const std::shared_ptr<ExtraWindowInfo> m_extraInfo; |
238 | @@ -280,6 +290,20 @@ |
239 | MirSurface *m_parentSurface; |
240 | |
241 | MirSurfaceListModel *m_childSurfaceList; |
242 | + |
243 | + // Track all keys that we told our mir window are currently pressed |
244 | + struct PressedKey { |
245 | + PressedKey() {} |
246 | + PressedKey(QKeyEvent *qtEvent, qint64 msecsSinceReference); |
247 | + quint32 nativeVirtualKey{0}; |
248 | + quint32 nativeScanCode{0}; |
249 | + ulong timestamp{0}; |
250 | + MirInputDeviceId deviceId{0}; |
251 | + qint64 msecsSinceReference{0}; |
252 | + }; |
253 | + QVector<PressedKey> m_pressedKeys; |
254 | + |
255 | + static QElapsedTimer m_elapsedTimer; |
256 | }; |
257 | |
258 | } // namespace qtmir |
259 | |
260 | === modified file 'src/platforms/mirserver/eventbuilder.h' |
261 | --- src/platforms/mirserver/eventbuilder.h 2017-03-03 10:39:24 +0000 |
262 | +++ src/platforms/mirserver/eventbuilder.h 2017-03-24 13:53:28 +0000 |
263 | @@ -69,7 +69,6 @@ |
264 | const QList<QTouchEvent::TouchPoint> &qtTouchPoints, |
265 | Qt::TouchPointStates /* qtTouchPointStates */, |
266 | ulong qtTimestamp); |
267 | -private: |
268 | class EventInfo { |
269 | public: |
270 | void store(const MirInputEvent *mirInputEvent, ulong qtTimestamp); |
271 | @@ -82,6 +81,7 @@ |
272 | |
273 | EventInfo *findInfo(ulong qtTimestamp); |
274 | |
275 | +private: |
276 | mir::EventUPtr makeMirEvent(QInputEvent *qtEvent, int x, int y, MirPointerButtons buttons); |
277 | |
278 | |
279 | |
280 | === modified file 'src/platforms/mirserver/nativeinterface.cpp' |
281 | --- src/platforms/mirserver/nativeinterface.cpp 2016-11-03 20:17:46 +0000 |
282 | +++ src/platforms/mirserver/nativeinterface.cpp 2017-03-24 13:53:28 +0000 |
283 | @@ -18,6 +18,9 @@ |
284 | |
285 | #include "qmirserver.h" |
286 | #include "screen.h" |
287 | +#include "windowcontrollerinterface.h" |
288 | + |
289 | +#include <QDebug> |
290 | |
291 | NativeInterface::NativeInterface(QMirServer *server) |
292 | : m_qMirServer(server) |
293 | @@ -73,6 +76,43 @@ |
294 | } |
295 | } |
296 | |
297 | +void NativeInterface::setWindowProperty(QPlatformWindow */*window*/, const QString &name, const QVariant &value) |
298 | +{ |
299 | + if (name.isNull()) { |
300 | + return; |
301 | + } |
302 | + |
303 | + // TODO remove this hack and expose this properly when QtMir can share WindowController in a C++ header |
304 | + |
305 | + // Get WindowController |
306 | + auto windowController = static_cast<qtmir::WindowControllerInterface *>(m_qMirServer->nativeResourceForIntegration("WindowController")); |
307 | + |
308 | + if (name == QStringLiteral("availableDesktopArea")) { |
309 | + QRect rect = value.toRect(); |
310 | + if (rect.isValid()) { |
311 | + windowController->setWindowConfinementRegions({rect}); |
312 | + } else { |
313 | + qWarning().nospace() << "NativeInterface::setWindowProperty(" |
314 | + << name << "," << value << ") - value is not a QRect"; |
315 | + } |
316 | + } else if (name.endsWith(QStringLiteral("WindowMargins"))) { |
317 | + if (value.canConvert(QMetaType::QRect)) { |
318 | + QRect rect = value.toRect(); |
319 | + QMargins margin(rect.x(), rect.y(), rect.width(), rect.height()); |
320 | + if (name == QStringLiteral("normalWindowMargins")) { |
321 | + windowController->setWindowMargins(Mir::NormalType, margin); |
322 | + } else if (name == QStringLiteral("dialogWindowMargins")) { |
323 | + windowController->setWindowMargins(Mir::DialogType, margin); |
324 | + } else { |
325 | + qWarning() << "NativeInterface::setWindowProperty missing support for:" << name; |
326 | + } |
327 | + } else { |
328 | + qWarning().nospace() << "NativeInterface::setWindowProperty(" |
329 | + << name << "," << value << ") - value is not a QRect"; |
330 | + } |
331 | + } |
332 | +} |
333 | + |
334 | std::shared_ptr<qtmir::PromptSessionManager> NativeInterface::thePromptSessionManager() const |
335 | { |
336 | return m_qMirServer->thePromptSessionManager(); |
337 | |
338 | === modified file 'src/platforms/mirserver/nativeinterface.h' |
339 | --- src/platforms/mirserver/nativeinterface.h 2016-11-03 20:17:46 +0000 |
340 | +++ src/platforms/mirserver/nativeinterface.h 2017-03-24 13:53:28 +0000 |
341 | @@ -41,6 +41,7 @@ |
342 | QVariantMap windowProperties(QPlatformWindow *window) const override; |
343 | QVariant windowProperty(QPlatformWindow *window, const QString &name) const override; |
344 | QVariant windowProperty(QPlatformWindow *window, const QString &name, const QVariant &defaultValue) const override; |
345 | + void setWindowProperty(QPlatformWindow *, const QString &name, const QVariant &value) override; |
346 | |
347 | std::shared_ptr<qtmir::PromptSessionManager> thePromptSessionManager() const; |
348 | std::shared_ptr<mir::shell::PersistentSurfaceStore> thePersistentSurfaceStore() const; |
349 | |
350 | === modified file 'src/platforms/mirserver/windowcontroller.cpp' |
351 | --- src/platforms/mirserver/windowcontroller.cpp 2016-11-03 20:17:46 +0000 |
352 | +++ src/platforms/mirserver/windowcontroller.cpp 2017-03-24 13:53:28 +0000 |
353 | @@ -97,6 +97,20 @@ |
354 | } |
355 | } |
356 | |
357 | +void WindowController::setWindowConfinementRegions(const QVector<QRect> ®ions) |
358 | +{ |
359 | + if (m_policy) { |
360 | + m_policy->set_window_confinement_regions(regions); |
361 | + } |
362 | +} |
363 | + |
364 | +void WindowController::setWindowMargins(Mir::Type windowType, const QMargins &margins) |
365 | +{ |
366 | + if (m_policy) { |
367 | + m_policy->set_window_margins(toMirType(windowType), margins); |
368 | + } |
369 | +} |
370 | + |
371 | void WindowController::setPolicy(WindowManagementPolicy * const policy) |
372 | { |
373 | m_policy = policy; |
374 | |
375 | === modified file 'src/platforms/mirserver/windowcontroller.h' |
376 | --- src/platforms/mirserver/windowcontroller.h 2016-11-03 20:17:46 +0000 |
377 | +++ src/platforms/mirserver/windowcontroller.h 2017-03-24 13:53:28 +0000 |
378 | @@ -43,6 +43,9 @@ |
379 | void deliverTouchEvent (const miral::Window &window, const MirTouchEvent *event) override; |
380 | void deliverPointerEvent (const miral::Window &window, const MirPointerEvent *event) override; |
381 | |
382 | + void setWindowConfinementRegions(const QVector<QRect> ®ions) override; |
383 | + void setWindowMargins(Mir::Type windowType, const QMargins &margins) override; |
384 | + |
385 | void setPolicy(WindowManagementPolicy *policy); |
386 | |
387 | protected: |
388 | |
389 | === modified file 'src/platforms/mirserver/windowmanagementpolicy.cpp' |
390 | --- src/platforms/mirserver/windowmanagementpolicy.cpp 2017-03-08 13:27:33 +0000 |
391 | +++ src/platforms/mirserver/windowmanagementpolicy.cpp 2017-03-24 13:53:28 +0000 |
392 | @@ -27,8 +27,6 @@ |
393 | #include "mirqtconversion.h" |
394 | #include "tracepoints.h" |
395 | |
396 | -#include <QDebug> |
397 | - |
398 | namespace qtmir { |
399 | std::shared_ptr<ExtraWindowInfo> getExtraInfo(const miral::WindowInfo &windowInfo) { |
400 | return std::static_pointer_cast<ExtraWindowInfo>(windowInfo.userdata()); |
401 | @@ -36,7 +34,6 @@ |
402 | } |
403 | |
404 | using namespace qtmir; |
405 | -using namespace mir::geometry; |
406 | |
407 | WindowManagementPolicy::WindowManagementPolicy(const miral::WindowManagerTools &tools, |
408 | qtmir::WindowModelNotifier &windowModel, |
409 | @@ -69,7 +66,7 @@ |
410 | QSize initialSize = InitialSurfaceSizes::get(miral::pid_of(appInfo.application())); |
411 | |
412 | if (initialSize.isValid() && surfaceType == mir_surface_type_normal) { |
413 | - parameters.size() = Size{Width(initialSize.width()), Height(initialSize.height())}; |
414 | + parameters.size() = toMirSize(initialSize); |
415 | } |
416 | } |
417 | |
418 | @@ -227,6 +224,19 @@ |
419 | }); |
420 | } |
421 | |
422 | +QRect WindowManagementPolicy::getConfinementRect(const QRect rect) const |
423 | +{ |
424 | + QRect confinementRect; |
425 | + for (const QRect r : m_confinementRegions) { |
426 | + if (r.intersects(rect)) { |
427 | + confinementRect = r; |
428 | + // TODO: What if there are multiple confinement regions and they intersect?? |
429 | + break; |
430 | + } |
431 | + } |
432 | + return confinementRect; |
433 | +} |
434 | + |
435 | /* Following methods all called from the Qt GUI thread to deliver events to clients */ |
436 | void WindowManagementPolicy::deliver_keyboard_event(const MirKeyboardEvent *event, |
437 | const miral::Window &window) |
438 | @@ -330,6 +340,20 @@ |
439 | }); |
440 | } |
441 | |
442 | +void WindowManagementPolicy::set_window_confinement_regions(const QVector<QRect> ®ions) |
443 | +{ |
444 | + m_confinementRegions = regions; |
445 | + |
446 | + // TODO: update window positions to respect new boundary. |
447 | +} |
448 | + |
449 | +void WindowManagementPolicy::set_window_margins(MirWindowType windowType, const QMargins &margins) |
450 | +{ |
451 | + m_windowMargins[windowType] = margins; |
452 | + |
453 | + // TODO: update window positions/sizes to respect new margins. |
454 | +} |
455 | + |
456 | void WindowManagementPolicy::requestState(const miral::Window &window, const Mir::State state) |
457 | { |
458 | auto &windowInfo = m_tools.info_for(window); |
459 | @@ -354,3 +378,50 @@ |
460 | }); |
461 | } |
462 | } |
463 | + |
464 | +Rectangle WindowManagementPolicy::confirm_inherited_move(miral::WindowInfo const& windowInfo, Displacement movement) |
465 | +{ |
466 | + if (m_confinementRegions.isEmpty()) { |
467 | + return CanonicalWindowManagerPolicy::confirm_inherited_move(windowInfo, movement); |
468 | + } |
469 | + |
470 | + auto window = windowInfo.window(); |
471 | + const QMargins windowMargins = m_windowMargins[windowInfo.type()]; |
472 | + QRect windowGeom(toQPoint(window.top_left()), toQSize(window.size())); |
473 | + |
474 | + QRect geom = windowGeom.marginsAdded(windowMargins); |
475 | + const QRect confinementRect = getConfinementRect(geom); |
476 | + |
477 | + int x = geom.x(); |
478 | + int y = geom.y(); |
479 | + int moveX = movement.dx.as_int(); |
480 | + int moveY = movement.dy.as_int(); |
481 | + |
482 | + // If the child window is already partially beyond the available desktop area (most likely because the user |
483 | + // explicitly moved it there) we won't pull it back, unless the inherited movement is this direction, but also won't |
484 | + // push it even further astray. But if it currently is completely within the available desktop area boundaries |
485 | + // we won't let go beyond it. |
486 | + |
487 | + if (moveX > 0) { |
488 | + if (geom.right() < confinementRect.right()) { |
489 | + x = qMin(x + moveX, confinementRect.right() + 1 - geom.width()); // +1 because QRect::right() weird |
490 | + } |
491 | + } else { |
492 | + if (geom.x() > confinementRect.left()) { |
493 | + x = qMax(x + moveX, confinementRect.left()); |
494 | + } |
495 | + } |
496 | + |
497 | + if (moveY > 0) { |
498 | + if (geom.bottom() < confinementRect.bottom()) { |
499 | + y = qMin(y + moveY, confinementRect.bottom() + 1 - geom.height()); // +1 because QRect::bottom() weird |
500 | + } |
501 | + } else { |
502 | + if (geom.y() > confinementRect.top()) { |
503 | + y = qMax(y + moveY, confinementRect.top()); |
504 | + } |
505 | + } |
506 | + |
507 | + geom.moveTo(x, y); |
508 | + return toMirRectangle(geom.marginsRemoved(windowMargins)); |
509 | +} |
510 | |
511 | === modified file 'src/platforms/mirserver/windowmanagementpolicy.h' |
512 | --- src/platforms/mirserver/windowmanagementpolicy.h 2017-02-06 09:10:20 +0000 |
513 | +++ src/platforms/mirserver/windowmanagementpolicy.h 2017-03-24 13:53:28 +0000 |
514 | @@ -1,5 +1,5 @@ |
515 | /* |
516 | - * Copyright (C) 2016 Canonical, Ltd. |
517 | + * Copyright (C) 2016-2017 Canonical, Ltd. |
518 | * |
519 | * This program is free software: you can redistribute it and/or modify it under |
520 | * the terms of the GNU Lesser General Public License version 3, as published by |
521 | @@ -25,7 +25,6 @@ |
522 | #include "windowmodelnotifier.h" |
523 | |
524 | #include <QScopedPointer> |
525 | -#include <QSize> |
526 | |
527 | using namespace mir::geometry; |
528 | |
529 | @@ -70,6 +69,9 @@ |
530 | void advise_delete_window(const miral::WindowInfo &windowInfo) override; |
531 | void advise_raise(const std::vector<miral::Window> &windows) override; |
532 | |
533 | + Rectangle confirm_inherited_move(miral::WindowInfo const& windowInfo, Displacement movement) override; |
534 | + |
535 | + |
536 | // Methods for consumption by WindowControllerInterface |
537 | void deliver_keyboard_event(const MirKeyboardEvent *event, const miral::Window &window); |
538 | void deliver_touch_event (const MirTouchEvent *event, const miral::Window &window); |
539 | @@ -84,15 +86,19 @@ |
540 | void ask_client_to_close(const miral::Window &window); |
541 | void forceClose(const miral::Window &window); |
542 | |
543 | -Q_SIGNALS: |
544 | + void set_window_confinement_regions(const QVector<QRect> ®ions); |
545 | + void set_window_margins(MirWindowType windowType, const QMargins &margins); |
546 | |
547 | private: |
548 | void ensureWindowIsActive(const miral::Window &window); |
549 | + QRect getConfinementRect(const QRect rect) const; |
550 | |
551 | miral::WindowManagerTools m_tools; |
552 | qtmir::WindowModelNotifier &m_windowModel; |
553 | qtmir::AppNotifier &m_appNotifier; |
554 | const QScopedPointer<QtEventFeeder> m_eventFeeder; |
555 | + QVector<QRect> m_confinementRegions; |
556 | + QMargins m_windowMargins[mir_window_types]; |
557 | }; |
558 | |
559 | #endif // WINDOWMANAGEMENTPOLICY_H |
560 | |
561 | === modified file 'tests/framework/mock_window_controller.h' |
562 | --- tests/framework/mock_window_controller.h 2017-03-20 16:19:42 +0000 |
563 | +++ tests/framework/mock_window_controller.h 2017-03-24 13:53:28 +0000 |
564 | @@ -38,6 +38,9 @@ |
565 | MOCK_METHOD2(deliverKeyboardEvent, void(const miral::Window &, const MirKeyboardEvent *)); |
566 | MOCK_METHOD2(deliverTouchEvent, void(const miral::Window &, const MirTouchEvent *)); |
567 | MOCK_METHOD2(deliverPointerEvent, void(const miral::Window &, const MirPointerEvent *)); |
568 | + |
569 | + MOCK_METHOD1(setWindowConfinementRegions, void(const QVector<QRect> ®ions)); |
570 | + MOCK_METHOD2(setWindowMargins, void(Mir::Type windowType, const QMargins &margins)); |
571 | }; |
572 | |
573 | #endif // MOCK_WINDOW_CONTROLLER_H |
574 | |
575 | === modified file 'tests/framework/stub_windowcontroller.h' |
576 | --- tests/framework/stub_windowcontroller.h 2016-11-03 20:17:46 +0000 |
577 | +++ tests/framework/stub_windowcontroller.h 2017-03-24 13:53:28 +0000 |
578 | @@ -37,6 +37,9 @@ |
579 | void deliverKeyboardEvent(const miral::Window &/*window*/, const MirKeyboardEvent */*event*/) override { return; } |
580 | void deliverTouchEvent (const miral::Window &/*window*/, const MirTouchEvent */*event*/) override { return; } |
581 | void deliverPointerEvent (const miral::Window &/*window*/, const MirPointerEvent */*event*/) override { return; } |
582 | + |
583 | + void setWindowConfinementRegions(const QVector<QRect> &/*regions*/) override { return; } |
584 | + void setWindowMargins(Mir::Type /*windowType*/, const QMargins &/*margins*/) override { return; } |
585 | }; |
586 | |
587 | } //namespace qtmir |
FAILED: Continuous integration, rev:619 /unity8- jenkins. ubuntu. com/job/ lp-qtmir- ci/592/ /unity8- jenkins. ubuntu. com/job/ build/4555/ console /unity8- jenkins. ubuntu. com/job/ build-0- fetch/4583 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 4410/console /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= zesty/4410/ console /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 4410/console /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= zesty/4410/ console /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 4410/console /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= zesty/4410/ console
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild: /unity8- jenkins. ubuntu. com/job/ lp-qtmir- ci/592/ rebuild
https:/