Mir

Merge lp:~alan-griffiths/mir/extend-demo-client-display-config into lp:mir

Proposed by Alan Griffiths on 2017-07-21
Status: Merged
Approved by: Alan Griffiths on 2017-07-26
Approved revision: 4214
Merged at revision: 4214
Proposed branch: lp:~alan-griffiths/mir/extend-demo-client-display-config
Merge into: lp:mir
Diff against target: 254 lines (+106/-30)
1 file modified
examples/demo_client_display_config.c (+106/-30)
To merge this branch: bzr merge lp:~alan-griffiths/mir/extend-demo-client-display-config
Reviewer Review Type Date Requested Status
Chris Halse Rogers 2017-07-21 Approve on 2017-07-26
Mir CI Bot continuous-integration Approve on 2017-07-25
Review via email: mp+327871@code.launchpad.net

Commit message

[mir_demo_client_display_config] add orientation changing

To post a comment you must log in.
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:4212
https://mir-jenkins.ubuntu.com/job/mir-ci/3503/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4793
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4970
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/4959
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/4959
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4959
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4830
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4830/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4830
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4830/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4830
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4830/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4830
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4830/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4830
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4830/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4830
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4830/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4830
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4830/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4830
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4830/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
4213. By Alan Griffiths on 2017-07-21

Add applying config to base display configuration

Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:4213
https://mir-jenkins.ubuntu.com/job/mir-ci/3504/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4794
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4971
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/4960
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/4960
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4960
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4831
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4831/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4831
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4831/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4831
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4831/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4831
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4831/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4831
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4831/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4831
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4831/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4831
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4831/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4831
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4831/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Chris Halse Rogers (raof) wrote :

Nit:
+ configuration_mode_up,
+ configuration_mode_down

We use C99; it'd be nice if you added a trailing ‘,’ here so that future additions don't also include a superfluous deletion.

Also nit:
- print_current_configuration(ctx->connection);
+ case XKB_KEY_p:print_current_configuration();

Missing newline between case XKB_KEY_p: and print_current_configuration()?

I find the use of the conf global strange, but actual use looks fine. The current code does no error-checking, so we're not losing anything.

Actually... this will crash if you press ‘p’ to print the display configuration before you do anything.

Maybe this should be factored into a little accessor function? Either that or print_current_configuration() needs a null-guard.

review: Needs Fixing
4214. By Alan Griffiths on 2017-07-24

Nit fixes

Alan Griffiths (alan-griffiths) wrote :

> I find the use of the conf global strange, but actual use looks fine. The
> current code does no error-checking, so we're not losing anything.

The use of the conf global is necessitated by the way the toolkit API works: it only ever returns the global config. We need to track the session config ourselves.

Nits fixed

Chris Halse Rogers (raof) wrote :

Ah, I didn't make the non-nit part of that review clear:

Non-nit:
mir_demo_client_display_config crashes if you press ‘p’ before making any display configuration changes, due to print_current_configuration() not having a null-check on conf and nothing else assigns to conf in the startup process.

review: Needs Fixing
Alan Griffiths (alan-griffiths) wrote :

> Ah, I didn't make the non-nit part of that review clear:
>
> Non-nit:
> mir_demo_client_display_config crashes if you press ‘p’ before making any
> display configuration changes, due to print_current_configuration() not having
> a null-check on conf and nothing else assigns to conf in the startup process.

No, that was clear. I failed to push the correction.

Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:4214
https://mir-jenkins.ubuntu.com/job/mir-ci/3509/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4802
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4984
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/4973
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/4973
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4973
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4839
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4839/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4839
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4839/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4839
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4839/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4839
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4839/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4839
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4839/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4839
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4839/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4839
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4839/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4839
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4839/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Chris Halse Rogers (raof) wrote :

Wouldn't it be better to construct the MirDisplayConfig in print_display_configuration() if it's null? As it stands, we document that ‘p’ will print the current configuration, but it only does if you've already changed it?

Alan Griffiths (alan-griffiths) wrote :

> Wouldn't it be better to construct the MirDisplayConfig in
> print_display_configuration() if it's null? As it stands, we document that ‘p’
> will print the current configuration, but it only does if you've already
> changed it?

No, if you press 'p' it will print the base configuration whether or not you've changed the base configuration.

Chris Halse Rogers (raof) wrote :

Ah, fair enough.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'examples/demo_client_display_config.c'
2--- examples/demo_client_display_config.c 2017-05-08 03:04:26 +0000
3+++ examples/demo_client_display_config.c 2017-07-25 07:47:20 +0000
4@@ -1,7 +1,7 @@
5 /*
6 * Client-side display configuration demo.
7 *
8- * Copyright © 2013 Canonical Ltd.
9+ * Copyright © 2013, 2017 Canonical Ltd.
10 *
11 * This program is free software: you can redistribute it and/or modify
12 * it under the terms of the GNU General Public License version 3 as
13@@ -37,7 +37,11 @@
14 configuration_mode_clone,
15 configuration_mode_horizontal,
16 configuration_mode_vertical,
17- configuration_mode_single
18+ configuration_mode_single,
19+ configuration_mode_left,
20+ configuration_mode_right,
21+ configuration_mode_up,
22+ configuration_mode_down,
23 } ConfigurationMode;
24
25 struct ClientContext
26@@ -49,9 +53,13 @@
27 volatile sig_atomic_t reconfigure;
28 };
29
30-static void print_current_configuration(MirConnection *connection)
31+static MirDisplayConfig *conf = NULL;
32+
33+static void print_current_configuration()
34 {
35- MirDisplayConfig *conf = mir_connection_create_display_configuration(connection);
36+ if (!conf)
37+ return;
38+
39 size_t num_outputs = mir_display_config_get_num_outputs(conf);
40
41 for (uint32_t i = 0; i < num_outputs; i++)
42@@ -64,8 +72,8 @@
43 bool connected = mir_output_get_connection_state(output) ==
44 mir_output_connection_state_connected;
45
46- printf("Output id: %d connected: %d used: %d position_x: %d position_y: %d",
47- id, connected, used, position_x, position_y);
48+ printf("Output id: %d connected: %d used: %d position_x: %d position_y: %d orientation: %d",
49+ id, connected, used, position_x, position_y, mir_output_get_orientation(output));
50
51 MirOutputMode const* current = mir_output_get_current_mode(output);
52 if (current)
53@@ -80,8 +88,6 @@
54 printf("\n");
55 }
56 }
57-
58- mir_display_config_release(conf);
59 }
60
61 static int apply_configuration(MirConnection *connection, MirDisplayConfig *conf)
62@@ -208,31 +214,67 @@
63 }
64 }
65
66+static void orient_display(MirDisplayConfig *conf, MirOrientation orientation)
67+{
68+ size_t num_outputs = mir_display_config_get_num_outputs(conf);
69+
70+ for (size_t i = 0; i < num_outputs; i++)
71+ {
72+ MirOutput *output = mir_display_config_get_mutable_output(conf, i);
73+ mir_output_set_orientation(output, orientation);
74+ }
75+}
76+
77 static void configure_display(struct ClientContext *context, ConfigurationMode mode,
78 int mode_data)
79 {
80- MirDisplayConfig *conf =
81- mir_connection_create_display_configuration(context->connection);
82+ if (!conf)
83+ conf = mir_connection_create_display_configuration(context->connection);
84
85- if (mode == configuration_mode_clone)
86+ switch (mode)
87 {
88+ case configuration_mode_clone:
89 configure_display_clone(conf);
90 printf("Applying clone configuration: ");
91- }
92- else if (mode == configuration_mode_vertical)
93- {
94+ break;
95+
96+ case configuration_mode_vertical:
97 configure_display_vertical(conf);
98 printf("Applying vertical configuration: ");
99- }
100- else if (mode == configuration_mode_horizontal)
101- {
102+ break;
103+
104+ case configuration_mode_horizontal:
105 configure_display_horizontal(conf);
106 printf("Applying horizontal configuration: ");
107- }
108- else if (mode == configuration_mode_single)
109- {
110+ break;
111+
112+ case configuration_mode_single:
113 configure_display_single(conf, mode_data);
114 printf("Applying single configuration for output %d: ", mode_data);
115+ break;
116+
117+ case configuration_mode_left:
118+ orient_display(conf, mir_orientation_left);
119+ printf("Applying orientation left: ");
120+ break;
121+
122+ case configuration_mode_right:
123+ orient_display(conf, mir_orientation_right);
124+ printf("Applying orientation right: ");
125+ break;
126+
127+ case configuration_mode_up:
128+ orient_display(conf, mir_orientation_inverted);
129+ printf("Applying orientation up: ");
130+ break;
131+
132+ case configuration_mode_down:
133+ orient_display(conf, mir_orientation_normal);
134+ printf("Applying orientation down: ");
135+ break;
136+
137+ default:
138+ break;
139 }
140
141 if (apply_configuration(context->connection, conf))
142@@ -240,22 +282,33 @@
143 context->mode = mode;
144 context->mode_data = mode_data;
145 }
146-
147- mir_display_config_release(conf);
148 }
149
150 static void display_change_callback(MirConnection *connection, void *context)
151 {
152- (void)context;
153-
154 printf("=== Display configuration changed === \n");
155
156- print_current_configuration(connection);
157+ if (conf)
158+ mir_display_config_release(conf);
159+
160+ conf = mir_connection_create_display_configuration(connection);
161+
162+ print_current_configuration();
163
164 struct ClientContext *ctx = (struct ClientContext*) context;
165 ctx->reconfigure = 1;
166 }
167
168+static void apply_to_base_configuration(MirConnection *connection)
169+{
170+ if (!conf)
171+ return;
172+
173+ mir_connection_preview_base_display_configuration(connection, conf, 2);
174+ puts("Applying to base configuration");
175+ mir_connection_confirm_base_display_configuration(connection, conf);
176+}
177+
178 static void handle_keyboard_event(struct ClientContext *ctx, MirKeyboardEvent const* event)
179 {
180 if (mir_keyboard_event_action(event) != mir_keyboard_action_up)
181@@ -271,6 +324,9 @@
182
183 switch (key_code)
184 {
185+ case XKB_KEY_a:
186+ apply_to_base_configuration(ctx->connection);
187+ break;
188 case XKB_KEY_q:
189 ctx->running = 0;
190 break;
191@@ -284,7 +340,21 @@
192 configure_display(ctx, configuration_mode_vertical, 0);
193 break;
194 case XKB_KEY_p:
195- print_current_configuration(ctx->connection);
196+ if (!conf)
197+ conf = mir_connection_create_display_configuration(ctx->connection);
198+ print_current_configuration();
199+ break;
200+ case XKB_KEY_Left:
201+ configure_display(ctx, configuration_mode_right, 0);
202+ break;
203+ case XKB_KEY_Up:
204+ configure_display(ctx, configuration_mode_up, 0);
205+ break;
206+ case XKB_KEY_Right:
207+ configure_display(ctx, configuration_mode_left, 0);
208+ break;
209+ case XKB_KEY_Down:
210+ configure_display(ctx, configuration_mode_down, 0);
211 break;
212 }
213 }
214@@ -292,15 +362,16 @@
215 static void event_callback(
216 MirWindow* surface, MirEvent const* event, void* context)
217 {
218- (void) surface;
219+ mir_eglapp_handle_event(surface, event, context);
220+
221 struct ClientContext *ctx = (struct ClientContext*) context;
222-
223+
224 if (mir_event_get_type(event) != mir_event_type_input)
225 return;
226 MirInputEvent const* input_event = mir_event_get_input_event(event);
227 if (mir_input_event_get_type(input_event) != mir_input_event_type_key)
228 return;
229-
230+
231 handle_keyboard_event(ctx, mir_input_event_get_keyboard_event(input_event));
232 }
233
234@@ -317,7 +388,9 @@
235 " h: arrange outputs horizontally in the virtual space\n"
236 " v: arrange outputs vertically in the virtual space\n"
237 " 1-9: enable only the Nth connected output (in the order returned by the hardware)\n"
238- " p: print current display configuration\n");
239+ " Arrows: orient display (sets \"down\" direction)\n"
240+ " p: print current display configuration\n"
241+ " a: apply current display configuration globally\n");
242
243 return 1;
244 }
245@@ -351,6 +424,9 @@
246 }
247 }
248
249+ if (conf)
250+ mir_display_config_release(conf);
251+
252 mir_eglapp_cleanup();
253
254 return 0;

Subscribers

People subscribed via source and target branches