Mir

Merge lp:~cemil-azizoglu/mir/fix-1575765 into lp:mir

  1. fix-1575765
  2. Merge into development-branch
Proposed by Cemil Azizoglu
Status: Merged
Approved by: Cemil Azizoglu
Approved revision: no longer in the source branch.
Merged at revision: 3487
Proposed branch: lp:~cemil-azizoglu/mir/fix-1575765
Merge into: lp:mir
Diff against target: 324 lines (+202/-11)
6 files modified
src/platforms/mesa/server/x11/graphics/display.cpp (+2/-0)
src/platforms/mesa/server/x11/input/input_platform.cpp (+38/-5)
src/platforms/mesa/server/x11/input/input_platform.h (+2/-0)
tests/include/mir/test/doubles/mock_x11.h (+4/-0)
tests/mir_test_doubles/mock_x11.cpp (+17/-0)
tests/unit-tests/input/test_x11_platform.cpp (+139/-6)
To merge this branch: bzr merge lp:~cemil-azizoglu/mir/fix-1575765
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Approve
Alan Griffiths Approve
Review via email: mp+293267@code.launchpad.net

Commit message

Implement pointer grabbing so some keyboard/mouse-button combos can work (like Alt-primary to move a Mir window).

Description of the change

Implement pointer grabbing so some keyboard/mouse-button combos can work (like Alt-primary to move a Mir window).

Pointer is grabbed only if mir server has focus.

I have observed some flakiness on the X server-side like some events going to windows on the bottom of the mir-on-X window.

To post a comment you must log in.
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Works for me, and also fixes lp:1575192!

review: Approve
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3481
https://mir-jenkins.ubuntu.com/job/mir-ci/912/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/958/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/999
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/990
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/990
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/968
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/968/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/968
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/968/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/968
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/968/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/968
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/968/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/968/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/912/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

Failure is unrelated (lp:1570698)

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/234/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/963/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/253/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1007
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/998
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/998
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/973
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/973/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/973
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/973/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/973
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/973/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/973
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/973/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/973/console

review: Needs Fixing (continuous-integration)
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

Ugh, failure is lp:1523621. ReTAing

Revision history for this message
Mir CI Bot (mir-ci-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/platforms/mesa/server/x11/graphics/display.cpp'
2--- src/platforms/mesa/server/x11/graphics/display.cpp 2016-04-18 17:11:56 +0000
3+++ src/platforms/mesa/server/x11/graphics/display.cpp 2016-04-28 13:30:29 +0000
4@@ -122,6 +122,8 @@
5 ButtonPressMask |
6 ButtonReleaseMask |
7 FocusChangeMask |
8+ EnterWindowMask |
9+ LeaveWindowMask |
10 PointerMotionMask;
11
12 auto mask = CWBackPixel | CWBorderPixel | CWColormap | CWEventMask;
13
14=== modified file 'src/platforms/mesa/server/x11/input/input_platform.cpp'
15--- src/platforms/mesa/server/x11/input/input_platform.cpp 2016-03-23 06:39:56 +0000
16+++ src/platforms/mesa/server/x11/input/input_platform.cpp 2016-04-28 13:30:29 +0000
17@@ -57,7 +57,9 @@
18 core_keyboard(std::make_shared<mix::XInputDevice>(
19 mi::InputDeviceInfo{"x11-keyboard-device", "x11-key-dev-1", mi::DeviceCapability::keyboard})),
20 core_pointer(std::make_shared<mix::XInputDevice>(
21- mi::InputDeviceInfo{"x11-mouse-device", "x11-mouse-dev-1", mi::DeviceCapability::pointer}))
22+ mi::InputDeviceInfo{"x11-mouse-device", "x11-mouse-dev-1", mi::DeviceCapability::pointer})),
23+ kbd_grabbed{false},
24+ ptr_grabbed{false}
25 {
26 }
27
28@@ -95,18 +97,49 @@
29 #ifdef GRAB_KBD
30 case FocusIn:
31 {
32- auto const& xfiev = xev.xfocus;
33- XGrabKeyboard(xfiev.display, xfiev.window, True, GrabModeAsync, GrabModeAsync, CurrentTime);
34+ if (!kbd_grabbed)
35+ {
36+ auto const& xfiev = xev.xfocus;
37+ XGrabKeyboard(xfiev.display, xfiev.window, True, GrabModeAsync, GrabModeAsync, CurrentTime);
38+ kbd_grabbed = true;
39+ }
40 break;
41 }
42
43 case FocusOut:
44 {
45- auto const& xfoev = xev.xfocus;
46- XUngrabKeyboard(xfoev.display, CurrentTime);
47+ if (kbd_grabbed)
48+ {
49+ auto const& xfoev = xev.xfocus;
50+ XUngrabKeyboard(xfoev.display, CurrentTime);
51+ kbd_grabbed = false;
52+ }
53 break;
54 }
55 #endif
56+ case EnterNotify:
57+ {
58+ if (!ptr_grabbed && kbd_grabbed)
59+ {
60+ auto const& xenev = xev.xcrossing;
61+ XGrabPointer(xenev.display, xenev.window, True, 0, GrabModeAsync,
62+ GrabModeAsync, None, None, CurrentTime);
63+ ptr_grabbed = true;
64+ }
65+ break;
66+ }
67+
68+ case LeaveNotify:
69+ {
70+ if (ptr_grabbed)
71+ {
72+ auto const& xlnev = xev.xcrossing;
73+ XUngrabPointer(xlnev.display, CurrentTime);
74+ ptr_grabbed = false;
75+ }
76+ break;
77+ }
78+
79 case KeyPress:
80 case KeyRelease:
81 {
82
83=== modified file 'src/platforms/mesa/server/x11/input/input_platform.h'
84--- src/platforms/mesa/server/x11/input/input_platform.h 2016-01-29 08:18:22 +0000
85+++ src/platforms/mesa/server/x11/input/input_platform.h 2016-04-28 13:30:29 +0000
86@@ -56,6 +56,8 @@
87 std::shared_ptr<input::InputDeviceRegistry> const registry;
88 std::shared_ptr<XInputDevice> const core_keyboard;
89 std::shared_ptr<XInputDevice> const core_pointer;
90+ bool kbd_grabbed;
91+ bool ptr_grabbed;
92 };
93
94 }
95
96=== modified file 'tests/include/mir/test/doubles/mock_x11.h'
97--- tests/include/mir/test/doubles/mock_x11.h 2016-03-23 06:39:56 +0000
98+++ tests/include/mir/test/doubles/mock_x11.h 2016-04-28 13:30:29 +0000
99@@ -47,6 +47,8 @@
100 XEvent focus_out_event_return = { 0 };
101 XEvent vscroll_event_return = { 0 };
102 XEvent motion_event_return = { 0 };
103+ XEvent enter_notify_event_return = { 0 };
104+ XEvent leave_notify_event_return = { 0 };
105 int pending_events = 1;
106 };
107
108@@ -81,6 +83,8 @@
109 MOCK_METHOD1(XSetErrorHandler, XErrorHandler(XErrorHandler));
110 MOCK_METHOD0(XInitThreads, Status());
111 MOCK_METHOD3(XSetWMHints, int(Display*, Window, XWMHints*));
112+ MOCK_METHOD9(XGrabPointer, int(Display*, Window, Bool, unsigned int, int, int, Window, Cursor, Time));
113+ MOCK_METHOD2(XUngrabPointer, int(Display*, Time));
114
115 FakeX11Resources fake_x11;
116 };
117
118=== modified file 'tests/mir_test_doubles/mock_x11.cpp'
119--- tests/mir_test_doubles/mock_x11.cpp 2016-04-18 20:31:18 +0000
120+++ tests/mir_test_doubles/mock_x11.cpp 2016-04-28 13:30:29 +0000
121@@ -40,6 +40,8 @@
122 std::memset(&focus_out_event_return, 0, sizeof(XEvent));
123 std::memset(&vscroll_event_return, 0, sizeof(XEvent));
124 std::memset(&motion_event_return, 0, sizeof(XEvent));
125+ std::memset(&enter_notify_event_return, 0, sizeof(XEvent));
126+ std::memset(&leave_notify_event_return, 0, sizeof(XEvent));
127 std::memset(&visual_info, 0, sizeof(XVisualInfo));
128 visual_info.red_mask = 0xFF0000;
129 keypress_event_return.type = KeyPress;
130@@ -52,6 +54,8 @@
131 XButtonEvent& xbev = (XButtonEvent&)vscroll_event_return;
132 xbev.button = Button4;
133 motion_event_return.type = MotionNotify;
134+ enter_notify_event_return.type = EnterNotify;
135+ leave_notify_event_return.type = LeaveNotify;
136 }
137
138 mtd::MockX11::MockX11()
139@@ -197,3 +201,16 @@
140 {
141 return global_mock->XPending(display);
142 }
143+
144+int XGrabPointer(Display* display, Window grab_window, Bool owner_events,
145+ unsigned int event_mask, int pointer_mode, int keyboard_mode,
146+ Window confine_to, Cursor cursor, Time time)
147+{
148+ return global_mock->XGrabPointer(display, grab_window, owner_events, event_mask,
149+ pointer_mode, keyboard_mode, confine_to, cursor, time);
150+}
151+
152+int XUngrabPointer(Display* display, Time time)
153+{
154+ return global_mock->XUngrabKeyboard(display, time);
155+}
156
157=== modified file 'tests/unit-tests/input/test_x11_platform.cpp'
158--- tests/unit-tests/input/test_x11_platform.cpp 2016-03-23 06:39:56 +0000
159+++ tests/unit-tests/input/test_x11_platform.cpp 2016-04-28 13:30:29 +0000
160@@ -59,6 +59,8 @@
161 [](::Display* display) { XCloseDisplay(display); })};
162
163 std::vector<std::shared_ptr<mi::InputDevice>> devices;
164+ enum class GrabState { FOCUSIN, FOCUSOUT, ENTERNOTIFY, LEAVENOTIFY };
165+ GrabState grab_state{GrabState::FOCUSIN};
166
167 void capture_devices()
168 {
169@@ -217,6 +219,33 @@
170
171 TEST_F(X11PlatformTest, ungrabs_keyboard)
172 {
173+ mock_x11.fake_x11.pending_events = 2;
174+
175+ prepare_event_processing();
176+
177+ ON_CALL(mock_x11, XNextEvent(_,_))
178+ .WillByDefault(Invoke([this](Display*, XEvent* xev)
179+ {
180+ *xev = (grab_state == GrabState::FOCUSIN)
181+ ? mock_x11.fake_x11.focus_in_event_return
182+ : mock_x11.fake_x11.focus_out_event_return;
183+ grab_state = GrabState::FOCUSOUT;
184+ return 1;
185+ }));
186+
187+ EXPECT_CALL(mock_x11, XUngrabKeyboard(_,_))
188+ .Times(Exactly(1));
189+ EXPECT_CALL(mock_pointer_sink, handle_input(_))
190+ .Times(Exactly(0));
191+ EXPECT_CALL(mock_keyboard_sink, handle_input(_))
192+ .Times(Exactly(0));
193+
194+ process_input_event();
195+ process_input_event();
196+}
197+
198+TEST_F(X11PlatformTest, does_not_ungrab_keyboard_without_grabbing_first)
199+{
200 prepare_event_processing();
201
202 ON_CALL(mock_x11, XNextEvent(_,_))
203@@ -224,12 +253,116 @@
204 Return(1)));
205
206 EXPECT_CALL(mock_x11, XUngrabKeyboard(_,_))
207- .Times(Exactly(1));
208- EXPECT_CALL(mock_pointer_sink, handle_input(_))
209- .Times(Exactly(0));
210- EXPECT_CALL(mock_keyboard_sink, handle_input(_))
211- .Times(Exactly(0));
212-
213+ .Times(Exactly(0));
214+ EXPECT_CALL(mock_pointer_sink, handle_input(_))
215+ .Times(Exactly(0));
216+ EXPECT_CALL(mock_keyboard_sink, handle_input(_))
217+ .Times(Exactly(0));
218+
219+ process_input_event();
220+}
221+
222+TEST_F(X11PlatformTest, does_not_grab_pointer_unfocussed)
223+{
224+ prepare_event_processing();
225+
226+ ON_CALL(mock_x11, XNextEvent(_,_))
227+ .WillByDefault(DoAll(SetArgPointee<1>(mock_x11.fake_x11.enter_notify_event_return),
228+ Return(1)));
229+
230+ EXPECT_CALL(mock_x11, XGrabPointer(_,_,_,_,_,_,_,_,_))
231+ .Times(Exactly(0));
232+ EXPECT_CALL(mock_pointer_sink, handle_input(_))
233+ .Times(Exactly(0));
234+ EXPECT_CALL(mock_keyboard_sink, handle_input(_))
235+ .Times(Exactly(0));
236+
237+ process_input_event();
238+}
239+
240+TEST_F(X11PlatformTest, grabs_pointer_when_focussed)
241+{
242+ mock_x11.fake_x11.pending_events = 2;
243+
244+ prepare_event_processing();
245+
246+ ON_CALL(mock_x11, XNextEvent(_,_))
247+ .WillByDefault(Invoke([this](Display*, XEvent* xev)
248+ {
249+ *xev = (grab_state == GrabState::FOCUSIN)
250+ ? mock_x11.fake_x11.focus_in_event_return
251+ : mock_x11.fake_x11.enter_notify_event_return;
252+ grab_state = GrabState::ENTERNOTIFY;
253+ return 1;
254+ }));
255+
256+ EXPECT_CALL(mock_x11, XGrabPointer(_,_,_,_,_,_,_,_,_))
257+ .Times(Exactly(1));
258+ EXPECT_CALL(mock_pointer_sink, handle_input(_))
259+ .Times(Exactly(0));
260+ EXPECT_CALL(mock_keyboard_sink, handle_input(_))
261+ .Times(Exactly(0));
262+
263+ process_input_event();
264+ process_input_event();
265+}
266+
267+TEST_F(X11PlatformTest, does_not_ungrab_pointer_without_grabbing_first)
268+{
269+ prepare_event_processing();
270+
271+ ON_CALL(mock_x11, XNextEvent(_,_))
272+ .WillByDefault(DoAll(SetArgPointee<1>(mock_x11.fake_x11.leave_notify_event_return),
273+ Return(1)));
274+
275+ EXPECT_CALL(mock_x11, XUngrabPointer(_,_))
276+ .Times(Exactly(0));
277+ EXPECT_CALL(mock_pointer_sink, handle_input(_))
278+ .Times(Exactly(0));
279+ EXPECT_CALL(mock_keyboard_sink, handle_input(_))
280+ .Times(Exactly(0));
281+
282+ process_input_event();
283+}
284+
285+TEST_F(X11PlatformTest, ungrabs_pointer)
286+{
287+ mock_x11.fake_x11.pending_events = 3;
288+
289+ prepare_event_processing();
290+
291+ ON_CALL(mock_x11, XNextEvent(_,_))
292+ .WillByDefault(Invoke([this](Display*, XEvent* xev)
293+ {
294+ switch(grab_state)
295+ {
296+ case GrabState::FOCUSIN:
297+ *xev = mock_x11.fake_x11.focus_in_event_return;
298+ grab_state = GrabState::ENTERNOTIFY;
299+ break;
300+ case GrabState::ENTERNOTIFY:
301+ *xev = mock_x11.fake_x11.enter_notify_event_return;
302+ grab_state = GrabState::LEAVENOTIFY;
303+ break;
304+ case GrabState::LEAVENOTIFY:
305+ *xev = mock_x11.fake_x11.leave_notify_event_return;
306+ break;
307+ case GrabState::FOCUSOUT: //only needed to appease the compiler.
308+ break;
309+ }
310+
311+ return 1;
312+ }));
313+
314+ EXPECT_CALL(mock_x11, XUngrabKeyboard(_,_))
315+ .Times(Exactly(1));
316+ EXPECT_CALL(mock_pointer_sink, handle_input(_))
317+ .Times(Exactly(0));
318+ EXPECT_CALL(mock_keyboard_sink, handle_input(_))
319+ .Times(Exactly(0));
320+
321+ process_input_event();
322+ process_input_event();
323 process_input_event();
324 }
325

Subscribers

People subscribed via source and target branches