Mir

Merge lp:~mir-team/mir/add-non-input-event-ctors-and-port-event-sinks into lp:mir

Proposed by Robert Carr
Status: Merged
Approved by: Cemil Azizoglu
Approved revision: no longer in the source branch.
Merged at revision: 2267
Proposed branch: lp:~mir-team/mir/add-non-input-event-ctors-and-port-event-sinks
Merge into: lp:mir
Prerequisite: lp:~mir-team/mir/add-event-deprecation-guard
Diff against target: 417 lines (+182/-65)
11 files modified
common-ABI-sha1sums (+2/-0)
include/common/mir/events/event_builders.h (+48/-0)
include/common/mir/frontend/surface_id.h (+4/-4)
platform-ABI-sha1sums (+2/-0)
server-ABI-sha1sums (+2/-1)
src/common/CMakeLists.txt (+2/-1)
src/common/events/CMakeLists.txt (+21/-0)
src/common/events/event_builders.cpp (+82/-0)
src/common/symbols.map (+7/-0)
src/server/scene/application_session.cpp (+6/-22)
src/server/scene/surface_event_source.cpp (+6/-37)
To merge this branch: bzr merge lp:~mir-team/mir/add-non-input-event-ctors-and-port-event-sinks
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Cemil Azizoglu (community) Approve
Kevin DuBois (community) Approve
Alan Griffiths Needs Fixing
Review via email: mp+247084@code.launchpad.net

Commit message

Introduce event builders to hide direct access to MirEvent

Description of the change

Introduce some event builder functions and port some code to them. If there is support for this approach I will continue as follows:

1. Port rest of Mir code, e.g. tests to this function to remove a few more requires..
2. Define input-event-builders in order to enable porting qtmir off the direct access.

Once event access is down to only the new headers, and the event builder header, then the underlying structure will be replaced without client API break.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

49 +std::shared_ptr<MirEvent> make_orientation_event(int surface_id, MirOrientation orientation);
50 +std::shared_ptr<MirEvent> make_prompt_session_event(MirPromptSessionState state);
51 +std::shared_ptr<MirEvent> make_resize_event(int surface_id, geometry::Size const& size);
52 +std::shared_ptr<MirEvent> make_surface_event(int surface_id, MirSurfaceAttrib attribute, int value);
53 +std::shared_ptr<MirEvent> make_close_surface_event(int surface_id);

Using these is horribly verbose. Look:

340 + event_sink->handle_event(*mev::make_surface_event(id.as_value(), attrib, value));

C.f.

    event_sink->handle_event(*mev::make_event(id, attrib, value));

353 + event_sink->handle_event(*mev::make_orientation_event(id.as_value(), orientation));

C.f.

    event_sink->handle_event(*mev::make_event(id, orientation));

review: Needs Fixing
Revision history for this message
Robert Carr (robertcarr) wrote :

Hi Alan, Thanks. Addressed :)

Revision history for this message
Robert Carr (robertcarr) wrote :

Had to move SurfaceId in to mircommon incidentally....the input event builders need to be in mircommon for the client input receiver...

Revision history for this message
Robert Carr (robertcarr) wrote :

I just want to emphasize that these are a temporary solution in order to complete the transition.

Revision history for this message
Robert Carr (robertcarr) wrote :

At which point the MirEvent struct can be replaced by a sensible C++ implementation with constructors and such, opaque to the client library.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

No technical issues, just copyright theft:

73 - * Copyright © 2013 Canonical Ltd.
74 + * Copyright © 2015 Canonical Ltd.
...
91 - * Authored By: Robert Carr <email address hidden>
92 + * Authored by: Kevin DuBois <email address hidden>
93 */

These changes are hardly justified when the body of the file is unchanged. (Just a different license.)

review: Needs Fixing
Revision history for this message
Kevin DuBois (kdub) wrote :

lgtm, apart from what Alan brought up, will contingently approve!

review: Approve
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

Good (modulo Alan's comments).

review: Approve
Revision history for this message
Robert Carr (robertcarr) wrote :

>> No technical issues, just copyright theft:

And I almost got away with it!!

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'common-ABI-sha1sums'
2--- common-ABI-sha1sums 2015-01-27 16:04:31 +0000
3+++ common-ABI-sha1sums 2015-01-27 19:47:07 +0000
4@@ -1,5 +1,7 @@
5 3329ada91412ded2f127aee9a92f065e57b81cb2 include/common/mir/cached_ptr.h
6+6473b6b17e72053a4ca3f7dcf8e2d81156fbfb85 include/common/mir/events/event_builders.h
7 82ff9499ef62739379616e02164dc98f9914c329 include/common/mir/fd.h
8+fe0275d9c64e7a2d99990382d04b1fe956d7b7e4 include/common/mir/frontend/surface_id.h
9 212be468fdca8134c1dff7a9cd61ecec76b26967 include/common/mir/geometry/dimensions.h
10 dd2eb355262622f08280e63dbc2680068fe81d9e include/common/mir/geometry/displacement.h
11 283cb0ecf9d544300681fc2ab86bec568e2d20ff include/common/mir/geometry/forward.h
12
13=== added directory 'include/common/mir/events'
14=== added file 'include/common/mir/events/event_builders.h'
15--- include/common/mir/events/event_builders.h 1970-01-01 00:00:00 +0000
16+++ include/common/mir/events/event_builders.h 2015-01-27 19:47:07 +0000
17@@ -0,0 +1,48 @@
18+/*
19+ * Copyright © 2015 Canonical Ltd.
20+ *
21+ * This program is free software: you can redistribute it and/or modify it
22+ * under the terms of the GNU Lesser General Public License version 3,
23+ * as published by the Free Software Foundation.
24+ *
25+ * This program is distributed in the hope that it will be useful,
26+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
27+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
28+ * GNU Lesser General Public License for more details.
29+ *
30+ * You should have received a copy of the GNU Lesser General Public License
31+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
32+ *
33+ * Author: Robert Carr <robert.carr@canonical.com>
34+ */
35+
36+#define MIR_INCLUDE_DEPRECATED_EVENT_HEADER
37+
38+#ifndef MIR_EVENT_BUILDERS_H_
39+#define MIR_EVENT_BUILDERS_H_
40+
41+#include "mir_toolkit/event.h"
42+
43+#include "mir/geometry/size.h"
44+#include "mir/frontend/surface_id.h"
45+
46+#include <memory>
47+
48+namespace mir
49+{
50+namespace events
51+{
52+// Surface orientation change event
53+std::shared_ptr<MirEvent> make_event(frontend::SurfaceId const& surface_id, MirOrientation orientation);
54+// Prompt session state change event
55+std::shared_ptr<MirEvent> make_event(MirPromptSessionState state);
56+// Surface resize event
57+std::shared_ptr<MirEvent> make_event(frontend::SurfaceId const& surface_id, geometry::Size const& size);
58+// Surface configure event
59+std::shared_ptr<MirEvent> make_event(frontend::SurfaceId const& surface_id, MirSurfaceAttrib attribute, int value);
60+// Close surface event
61+std::shared_ptr<MirEvent> make_event(frontend::SurfaceId const& surface_id);
62+}
63+}
64+
65+#endif // MIR_EVENT_BUILDERS_H_
66
67=== added directory 'include/common/mir/frontend'
68=== renamed file 'include/server/mir/frontend/surface_id.h' => 'include/common/mir/frontend/surface_id.h'
69--- include/server/mir/frontend/surface_id.h 2015-01-14 06:39:13 +0000
70+++ include/common/mir/frontend/surface_id.h 2015-01-27 19:47:07 +0000
71@@ -2,18 +2,18 @@
72 * Copyright © 2013 Canonical Ltd.
73 *
74 * This program is free software: you can redistribute it and/or modify it
75- * under the terms of the GNU General Public License version 3,
76+ * under the terms of the GNU Lesser General Public License version 3,
77 * as published by the Free Software Foundation.
78 *
79 * This program is distributed in the hope that it will be useful,
80 * but WITHOUT ANY WARRANTY; without even the implied warranty of
81 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
82- * GNU General Public License for more details.
83+ * GNU Lesser General Public License for more details.
84 *
85- * You should have received a copy of the GNU General Public License
86+ * You should have received a copy of the GNU Lesser General Public License
87 * along with this program. If not, see <http://www.gnu.org/licenses/>.
88 *
89- * Authored By: Robert Carr <racarr@canonical.com>
90+ * Authored by: Robert Carr <robert.carr@canonical.com>
91 */
92
93 #ifndef MIR_FRONTEND_SURFACE_ID_H_
94
95=== modified file 'platform-ABI-sha1sums'
96--- platform-ABI-sha1sums 2015-01-27 16:04:31 +0000
97+++ platform-ABI-sha1sums 2015-01-27 19:47:07 +0000
98@@ -1,5 +1,7 @@
99 3329ada91412ded2f127aee9a92f065e57b81cb2 include/common/mir/cached_ptr.h
100+6473b6b17e72053a4ca3f7dcf8e2d81156fbfb85 include/common/mir/events/event_builders.h
101 82ff9499ef62739379616e02164dc98f9914c329 include/common/mir/fd.h
102+fe0275d9c64e7a2d99990382d04b1fe956d7b7e4 include/common/mir/frontend/surface_id.h
103 212be468fdca8134c1dff7a9cd61ecec76b26967 include/common/mir/geometry/dimensions.h
104 dd2eb355262622f08280e63dbc2680068fe81d9e include/common/mir/geometry/displacement.h
105 283cb0ecf9d544300681fc2ab86bec568e2d20ff include/common/mir/geometry/forward.h
106
107=== modified file 'server-ABI-sha1sums'
108--- server-ABI-sha1sums 2015-01-27 16:04:31 +0000
109+++ server-ABI-sha1sums 2015-01-27 19:47:07 +0000
110@@ -1,5 +1,7 @@
111 3329ada91412ded2f127aee9a92f065e57b81cb2 include/common/mir/cached_ptr.h
112+6473b6b17e72053a4ca3f7dcf8e2d81156fbfb85 include/common/mir/events/event_builders.h
113 82ff9499ef62739379616e02164dc98f9914c329 include/common/mir/fd.h
114+fe0275d9c64e7a2d99990382d04b1fe956d7b7e4 include/common/mir/frontend/surface_id.h
115 212be468fdca8134c1dff7a9cd61ecec76b26967 include/common/mir/geometry/dimensions.h
116 dd2eb355262622f08280e63dbc2680068fe81d9e include/common/mir/geometry/displacement.h
117 283cb0ecf9d544300681fc2ab86bec568e2d20ff include/common/mir/geometry/forward.h
118@@ -67,7 +69,6 @@
119 d754e5bb057f3018d7228170cf9a0c66d75d8cc2 include/server/mir/frontend/session_mediator_report.h
120 471fafe5234816663e9a6988e090f94e2c8209fc include/server/mir/frontend/shell.h
121 8903a35ecdf09fc2e63b9b7cbd8b2442fa0a4255 include/server/mir/frontend/surface.h
122-618b43a84cce0ad671ed68fe2ba796fbc7b79e31 include/server/mir/frontend/surface_id.h
123 f95c2bddf13d15993ef5d6a0ad7b9106ae550b87 include/server/mir/input/composite_event_filter.h
124 67719acb03b35d383dfefd65e8dfb872c42bcc11 include/server/mir/input/cursor_images.h
125 cef18b7215fbe00550d18c0aa0f79b641cff494e include/server/mir/input/cursor_listener.h
126
127=== modified file 'src/common/CMakeLists.txt'
128--- src/common/CMakeLists.txt 2015-01-14 06:39:13 +0000
129+++ src/common/CMakeLists.txt 2015-01-27 19:47:07 +0000
130@@ -20,7 +20,8 @@
131 list(APPEND MIR_COMMON_SOURCES
132 $<TARGET_OBJECTS:mirtime>
133 ${CMAKE_CURRENT_SOURCE_DIR}/log.cpp
134-)
135+ )
136+add_subdirectory(events)
137
138 set(PREFIX "${CMAKE_INSTALL_PREFIX}")
139 set(EXEC_PREFIX "${CMAKE_INSTALL_PREFIX}")
140
141=== added directory 'src/common/events'
142=== added file 'src/common/events/CMakeLists.txt'
143--- src/common/events/CMakeLists.txt 1970-01-01 00:00:00 +0000
144+++ src/common/events/CMakeLists.txt 2015-01-27 19:47:07 +0000
145@@ -0,0 +1,21 @@
146+# Copyright © 2015 Canonical Ltd.
147+#
148+# This program is free software: you can redistribute it and/or modify
149+# it under the terms of the GNU Lesser General Public License version 3 as
150+# published by the Free Software Foundation.
151+#
152+# This program is distributed in the hope that it will be useful,
153+# but WITHOUT ANY WARRANTY; without even the implied warranty of
154+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
155+# GNU Lesser General Public License for more details.
156+#
157+# You should have received a copy of the GNU Lesser General Public License
158+# along with this program. If not, see <http://www.gnu.org/licenses/>.
159+#
160+# Authored by: Robert Carr <robert.carr@canonical.com>
161+
162+list(APPEND MIR_COMMON_SOURCES
163+ ${CMAKE_CURRENT_SOURCE_DIR}/event_builders.cpp
164+)
165+
166+set(MIR_COMMON_SOURCES ${MIR_COMMON_SOURCES} PARENT_SCOPE)
167
168=== added file 'src/common/events/event_builders.cpp'
169--- src/common/events/event_builders.cpp 1970-01-01 00:00:00 +0000
170+++ src/common/events/event_builders.cpp 2015-01-27 19:47:07 +0000
171@@ -0,0 +1,82 @@
172+/*
173+ * Copyright © 2015 Canonical Ltd.
174+ *
175+ * This program is free software: you can redistribute it and/or modify it
176+ * under the terms of the GNU Lesser General Public License version 3,
177+ * as published by the Free Software Foundation.
178+ *
179+ * This program is distributed in the hope that it will be useful,
180+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
181+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
182+ * GNU Lesser General Public License for more details.
183+ *
184+ * You should have received a copy of the GNU Lesser General Public License
185+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
186+ *
187+ * Author: Robert Carr <robert.carr@canonical.com>
188+ */
189+
190+#define MIR_INCLUDE_DEPRECATED_EVENT_HEADER
191+
192+#include "mir/events/event_builders.h"
193+
194+#include <string.h>
195+
196+namespace mf = mir::frontend;
197+namespace mev = mir::events;
198+namespace geom = mir::geometry;
199+
200+std::shared_ptr<MirEvent> mev::make_event(mf::SurfaceId const& surface_id, MirOrientation orientation)
201+{
202+ MirEvent *e = new MirEvent;
203+ memset(e, 0, sizeof (MirEvent));
204+
205+ e->type = mir_event_type_orientation;
206+ e->orientation.surface_id = surface_id.as_value();
207+ e->orientation.direction = orientation;
208+ return std::shared_ptr<MirEvent>(e);
209+}
210+
211+std::shared_ptr<MirEvent> mev::make_event(MirPromptSessionState state)
212+{
213+ MirEvent *e = new MirEvent;
214+ memset(e, 0, sizeof (MirEvent));
215+
216+ e->type = mir_event_type_prompt_session_state_change;
217+ e->prompt_session.new_state = state;
218+ return std::shared_ptr<MirEvent>(e);
219+}
220+
221+std::shared_ptr<MirEvent> mev::make_event(mf::SurfaceId const& surface_id, geom::Size const& size)
222+{
223+ MirEvent *e = new MirEvent;
224+ memset(e, 0, sizeof (MirEvent));
225+
226+ e->type = mir_event_type_resize;
227+ e->resize.surface_id = surface_id.as_value();
228+ e->resize.width = size.width.as_int();
229+ e->resize.height = size.height.as_int();
230+ return std::shared_ptr<MirEvent>(e);
231+}
232+
233+std::shared_ptr<MirEvent> mev::make_event(mf::SurfaceId const& surface_id, MirSurfaceAttrib attribute, int value)
234+{
235+ MirEvent *e = new MirEvent;
236+ memset(e, 0, sizeof (MirEvent));
237+
238+ e->type = mir_event_type_surface;
239+ e->surface.id = surface_id.as_value();
240+ e->surface.attrib = attribute;
241+ e->surface.value = value;
242+ return std::shared_ptr<MirEvent>(e);
243+}
244+
245+std::shared_ptr<MirEvent> mev::make_event(mf::SurfaceId const& surface_id)
246+{
247+ MirEvent *e = new MirEvent;
248+ memset(e, 0, sizeof (MirEvent));
249+
250+ e->type = mir_event_type_close_surface;
251+ e->close_surface.surface_id = surface_id.as_value();
252+ return std::shared_ptr<MirEvent>(e);
253+}
254
255=== modified file 'src/common/symbols.map'
256--- src/common/symbols.map 2015-01-23 16:42:01 +0000
257+++ src/common/symbols.map 2015-01-27 19:47:07 +0000
258@@ -212,3 +212,10 @@
259 mir::libraries_for_path*;
260 };
261 } MIR_COMMON_3;
262+
263+MIR_COMMON_3.2 {
264+ global:
265+ extern "C++" {
266+ mir::events::*
267+ };
268+} MIR_COMMON_3;
269
270=== modified file 'src/server/scene/application_session.cpp'
271--- src/server/scene/application_session.cpp 2015-01-22 21:44:49 +0000
272+++ src/server/scene/application_session.cpp 2015-01-27 19:47:07 +0000
273@@ -16,8 +16,6 @@
274 * Authored by: Robert Carr <racarr@canonical.com>
275 */
276
277-#define MIR_INCLUDE_DEPRECATED_EVENT_HEADER
278-
279 #include "application_session.h"
280 #include "snapshot_strategy.h"
281 #include "default_session_container.h"
282@@ -27,6 +25,7 @@
283 #include "mir/scene/surface_coordinator.h"
284 #include "mir/scene/surface_creation_parameters.h"
285 #include "mir/scene/session_listener.h"
286+#include "mir/events/event_builders.h"
287 #include "mir/frontend/event_sink.h"
288
289 #include <boost/throw_exception.hpp>
290@@ -41,6 +40,7 @@
291 namespace ms = mir::scene;
292 namespace msh = mir::shell;
293 namespace mg = mir::graphics;
294+namespace mev = mir::events;
295
296 ms::ApplicationSession::ApplicationSession(
297 std::shared_ptr<ms::SurfaceCoordinator> const& surface_coordinator,
298@@ -204,36 +204,20 @@
299 void ms::ApplicationSession::start_prompt_session()
300 {
301 // All sessions which are part of the prompt session get this event.
302- MirEvent start_event;
303- memset(&start_event, 0, sizeof start_event);
304- start_event.type = mir_event_type_prompt_session_state_change;
305- start_event.prompt_session.new_state = mir_prompt_session_state_started;
306- event_sink->handle_event(start_event);
307+ event_sink->handle_event(*mev::make_event(mir_prompt_session_state_started));
308 }
309
310 void ms::ApplicationSession::stop_prompt_session()
311 {
312- MirEvent stop_event;
313- memset(&stop_event, 0, sizeof stop_event);
314- stop_event.type = mir_event_type_prompt_session_state_change;
315- stop_event.prompt_session.new_state = mir_prompt_session_state_stopped;
316- event_sink->handle_event(stop_event);
317+ event_sink->handle_event(*mev::make_event(mir_prompt_session_state_stopped));
318 }
319
320 void ms::ApplicationSession::suspend_prompt_session()
321 {
322- MirEvent start_event;
323- memset(&start_event, 0, sizeof start_event);
324- start_event.type = mir_event_type_prompt_session_state_change;
325- start_event.prompt_session.new_state = mir_prompt_session_state_suspended;
326- event_sink->handle_event(start_event);
327+ event_sink->handle_event(*mev::make_event(mir_prompt_session_state_suspended));
328 }
329
330 void ms::ApplicationSession::resume_prompt_session()
331 {
332- MirEvent start_event;
333- memset(&start_event, 0, sizeof start_event);
334- start_event.type = mir_event_type_prompt_session_state_change;
335- start_event.prompt_session.new_state = mir_prompt_session_state_started;
336- event_sink->handle_event(start_event);
337+ start_prompt_session();
338 }
339
340=== modified file 'src/server/scene/surface_event_source.cpp'
341--- src/server/scene/surface_event_source.cpp 2015-01-21 00:49:28 +0000
342+++ src/server/scene/surface_event_source.cpp 2015-01-27 19:47:07 +0000
343@@ -16,8 +16,7 @@
344 * Authored by: Alan Griffiths <alan@octopull.co.uk>
345 */
346
347-#define MIR_INCLUDE_DEPRECATED_EVENT_HEADER
348-
349+#include "mir/events/event_builders.h"
350 #include "mir/scene/surface_event_source.h"
351
352 #include "mir/geometry/size.h"
353@@ -26,6 +25,7 @@
354 #include <algorithm>
355
356 namespace ms = mir::scene;
357+namespace mev = mir::events;
358 namespace geom = mir::geometry;
359
360 ms::SurfaceEventSource::SurfaceEventSource(
361@@ -38,52 +38,21 @@
362
363 void ms::SurfaceEventSource::resized_to(geom::Size const& size)
364 {
365- MirEvent e;
366- memset(&e, 0, sizeof e);
367- e.type = mir_event_type_resize;
368- e.resize.surface_id = id.as_value();
369- e.resize.width = size.width.as_int();
370- e.resize.height = size.height.as_int();
371- event_sink->handle_event(e);
372+ event_sink->handle_event(*mev::make_event(id, size));
373 }
374
375
376 void ms::SurfaceEventSource::attrib_changed(MirSurfaceAttrib attrib, int value)
377 {
378- MirEvent e;
379-
380- // This memset is not really required. However it does avoid some
381- // harmless uninitialized memory reads that valgrind will complain
382- // about, due to gaps in MirEvent.
383- memset(&e, 0, sizeof e);
384-
385- e.type = mir_event_type_surface;
386- e.surface.id = id.as_value();
387- e.surface.attrib = attrib;
388- e.surface.value = value;
389-
390- event_sink->handle_event(e);
391+ event_sink->handle_event(*mev::make_event(id, attrib, value));
392 }
393
394 void ms::SurfaceEventSource::orientation_set_to(MirOrientation orientation)
395 {
396- MirEvent e;
397- memset(&e, 0, sizeof e);
398-
399- e.type = mir_event_type_orientation;
400- e.orientation.surface_id = id.as_value();
401- e.orientation.direction = orientation;
402-
403- event_sink->handle_event(e);
404+ event_sink->handle_event(*mev::make_event(id, orientation));
405 }
406
407 void ms::SurfaceEventSource::client_surface_close_requested()
408 {
409- MirEvent e;
410- memset(&e, 0, sizeof e);
411-
412- e.type = mir_event_type_close_surface;
413- e.close_surface.surface_id = id.as_value();
414-
415- event_sink->handle_event(e);
416+ event_sink->handle_event(*mev::make_event(id));
417 }

Subscribers

People subscribed via source and target branches