Merge lp:~alan-griffiths/mir/extend-demo-client-display-config into lp:mir
- extend-demo-client-display-config
- Merge into development-branch
Status: | Merged |
---|---|
Approved by: | Alan Griffiths |
Approved revision: | no longer in the source branch. |
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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Chris Halse Rogers | Approve | ||
Mir CI Bot | continuous-integration | Approve | |
Review via email: mp+327871@code.launchpad.net |
Commit message
[mir_demo_
Description of the change
Mir CI Bot (mir-ci-bot) wrote : | # |
Mir CI Bot (mir-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:4213
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
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:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Chris Halse Rogers (raof) wrote : | # |
Nit:
+ configuration_
+ configuration_
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_
+ case XKB_KEY_
Missing newline between case XKB_KEY_p: and print_current_
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_
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_
Alan Griffiths (alan-griffiths) wrote : | # |
> Ah, I didn't make the non-nit part of that review clear:
>
> Non-nit:
> mir_demo_
> display configuration changes, due to print_current_
> 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:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
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:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Chris Halse Rogers (raof) wrote : | # |
Wouldn't it be better to construct the MirDisplayConfig in print_display_
Alan Griffiths (alan-griffiths) wrote : | # |
> Wouldn't it be better to construct the MirDisplayConfig in
> print_display_
> 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.
Preview Diff
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 | 1 | /* | 1 | /* |
6 | 2 | * Client-side display configuration demo. | 2 | * Client-side display configuration demo. |
7 | 3 | * | 3 | * |
9 | 4 | * Copyright © 2013 Canonical Ltd. | 4 | * Copyright © 2013, 2017 Canonical Ltd. |
10 | 5 | * | 5 | * |
11 | 6 | * This program is free software: you can redistribute it and/or modify | 6 | * This program is free software: you can redistribute it and/or modify |
12 | 7 | * it under the terms of the GNU General Public License version 3 as | 7 | * it under the terms of the GNU General Public License version 3 as |
13 | @@ -37,7 +37,11 @@ | |||
14 | 37 | configuration_mode_clone, | 37 | configuration_mode_clone, |
15 | 38 | configuration_mode_horizontal, | 38 | configuration_mode_horizontal, |
16 | 39 | configuration_mode_vertical, | 39 | configuration_mode_vertical, |
18 | 40 | configuration_mode_single | 40 | configuration_mode_single, |
19 | 41 | configuration_mode_left, | ||
20 | 42 | configuration_mode_right, | ||
21 | 43 | configuration_mode_up, | ||
22 | 44 | configuration_mode_down, | ||
23 | 41 | } ConfigurationMode; | 45 | } ConfigurationMode; |
24 | 42 | 46 | ||
25 | 43 | struct ClientContext | 47 | struct ClientContext |
26 | @@ -49,9 +53,13 @@ | |||
27 | 49 | volatile sig_atomic_t reconfigure; | 53 | volatile sig_atomic_t reconfigure; |
28 | 50 | }; | 54 | }; |
29 | 51 | 55 | ||
31 | 52 | static void print_current_configuration(MirConnection *connection) | 56 | static MirDisplayConfig *conf = NULL; |
32 | 57 | |||
33 | 58 | static void print_current_configuration() | ||
34 | 53 | { | 59 | { |
36 | 54 | MirDisplayConfig *conf = mir_connection_create_display_configuration(connection); | 60 | if (!conf) |
37 | 61 | return; | ||
38 | 62 | |||
39 | 55 | size_t num_outputs = mir_display_config_get_num_outputs(conf); | 63 | size_t num_outputs = mir_display_config_get_num_outputs(conf); |
40 | 56 | 64 | ||
41 | 57 | for (uint32_t i = 0; i < num_outputs; i++) | 65 | for (uint32_t i = 0; i < num_outputs; i++) |
42 | @@ -64,8 +72,8 @@ | |||
43 | 64 | bool connected = mir_output_get_connection_state(output) == | 72 | bool connected = mir_output_get_connection_state(output) == |
44 | 65 | mir_output_connection_state_connected; | 73 | mir_output_connection_state_connected; |
45 | 66 | 74 | ||
48 | 67 | printf("Output id: %d connected: %d used: %d position_x: %d position_y: %d", | 75 | printf("Output id: %d connected: %d used: %d position_x: %d position_y: %d orientation: %d", |
49 | 68 | id, connected, used, position_x, position_y); | 76 | id, connected, used, position_x, position_y, mir_output_get_orientation(output)); |
50 | 69 | 77 | ||
51 | 70 | MirOutputMode const* current = mir_output_get_current_mode(output); | 78 | MirOutputMode const* current = mir_output_get_current_mode(output); |
52 | 71 | if (current) | 79 | if (current) |
53 | @@ -80,8 +88,6 @@ | |||
54 | 80 | printf("\n"); | 88 | printf("\n"); |
55 | 81 | } | 89 | } |
56 | 82 | } | 90 | } |
57 | 83 | |||
58 | 84 | mir_display_config_release(conf); | ||
59 | 85 | } | 91 | } |
60 | 86 | 92 | ||
61 | 87 | static int apply_configuration(MirConnection *connection, MirDisplayConfig *conf) | 93 | static int apply_configuration(MirConnection *connection, MirDisplayConfig *conf) |
62 | @@ -208,31 +214,67 @@ | |||
63 | 208 | } | 214 | } |
64 | 209 | } | 215 | } |
65 | 210 | 216 | ||
66 | 217 | static void orient_display(MirDisplayConfig *conf, MirOrientation orientation) | ||
67 | 218 | { | ||
68 | 219 | size_t num_outputs = mir_display_config_get_num_outputs(conf); | ||
69 | 220 | |||
70 | 221 | for (size_t i = 0; i < num_outputs; i++) | ||
71 | 222 | { | ||
72 | 223 | MirOutput *output = mir_display_config_get_mutable_output(conf, i); | ||
73 | 224 | mir_output_set_orientation(output, orientation); | ||
74 | 225 | } | ||
75 | 226 | } | ||
76 | 227 | |||
77 | 211 | static void configure_display(struct ClientContext *context, ConfigurationMode mode, | 228 | static void configure_display(struct ClientContext *context, ConfigurationMode mode, |
78 | 212 | int mode_data) | 229 | int mode_data) |
79 | 213 | { | 230 | { |
82 | 214 | MirDisplayConfig *conf = | 231 | if (!conf) |
83 | 215 | mir_connection_create_display_configuration(context->connection); | 232 | conf = mir_connection_create_display_configuration(context->connection); |
84 | 216 | 233 | ||
86 | 217 | if (mode == configuration_mode_clone) | 234 | switch (mode) |
87 | 218 | { | 235 | { |
88 | 236 | case configuration_mode_clone: | ||
89 | 219 | configure_display_clone(conf); | 237 | configure_display_clone(conf); |
90 | 220 | printf("Applying clone configuration: "); | 238 | printf("Applying clone configuration: "); |
94 | 221 | } | 239 | break; |
95 | 222 | else if (mode == configuration_mode_vertical) | 240 | |
96 | 223 | { | 241 | case configuration_mode_vertical: |
97 | 224 | configure_display_vertical(conf); | 242 | configure_display_vertical(conf); |
98 | 225 | printf("Applying vertical configuration: "); | 243 | printf("Applying vertical configuration: "); |
102 | 226 | } | 244 | break; |
103 | 227 | else if (mode == configuration_mode_horizontal) | 245 | |
104 | 228 | { | 246 | case configuration_mode_horizontal: |
105 | 229 | configure_display_horizontal(conf); | 247 | configure_display_horizontal(conf); |
106 | 230 | printf("Applying horizontal configuration: "); | 248 | printf("Applying horizontal configuration: "); |
110 | 231 | } | 249 | break; |
111 | 232 | else if (mode == configuration_mode_single) | 250 | |
112 | 233 | { | 251 | case configuration_mode_single: |
113 | 234 | configure_display_single(conf, mode_data); | 252 | configure_display_single(conf, mode_data); |
114 | 235 | printf("Applying single configuration for output %d: ", mode_data); | 253 | printf("Applying single configuration for output %d: ", mode_data); |
115 | 254 | break; | ||
116 | 255 | |||
117 | 256 | case configuration_mode_left: | ||
118 | 257 | orient_display(conf, mir_orientation_left); | ||
119 | 258 | printf("Applying orientation left: "); | ||
120 | 259 | break; | ||
121 | 260 | |||
122 | 261 | case configuration_mode_right: | ||
123 | 262 | orient_display(conf, mir_orientation_right); | ||
124 | 263 | printf("Applying orientation right: "); | ||
125 | 264 | break; | ||
126 | 265 | |||
127 | 266 | case configuration_mode_up: | ||
128 | 267 | orient_display(conf, mir_orientation_inverted); | ||
129 | 268 | printf("Applying orientation up: "); | ||
130 | 269 | break; | ||
131 | 270 | |||
132 | 271 | case configuration_mode_down: | ||
133 | 272 | orient_display(conf, mir_orientation_normal); | ||
134 | 273 | printf("Applying orientation down: "); | ||
135 | 274 | break; | ||
136 | 275 | |||
137 | 276 | default: | ||
138 | 277 | break; | ||
139 | 236 | } | 278 | } |
140 | 237 | 279 | ||
141 | 238 | if (apply_configuration(context->connection, conf)) | 280 | if (apply_configuration(context->connection, conf)) |
142 | @@ -240,22 +282,33 @@ | |||
143 | 240 | context->mode = mode; | 282 | context->mode = mode; |
144 | 241 | context->mode_data = mode_data; | 283 | context->mode_data = mode_data; |
145 | 242 | } | 284 | } |
146 | 243 | |||
147 | 244 | mir_display_config_release(conf); | ||
148 | 245 | } | 285 | } |
149 | 246 | 286 | ||
150 | 247 | static void display_change_callback(MirConnection *connection, void *context) | 287 | static void display_change_callback(MirConnection *connection, void *context) |
151 | 248 | { | 288 | { |
152 | 249 | (void)context; | ||
153 | 250 | |||
154 | 251 | printf("=== Display configuration changed === \n"); | 289 | printf("=== Display configuration changed === \n"); |
155 | 252 | 290 | ||
157 | 253 | print_current_configuration(connection); | 291 | if (conf) |
158 | 292 | mir_display_config_release(conf); | ||
159 | 293 | |||
160 | 294 | conf = mir_connection_create_display_configuration(connection); | ||
161 | 295 | |||
162 | 296 | print_current_configuration(); | ||
163 | 254 | 297 | ||
164 | 255 | struct ClientContext *ctx = (struct ClientContext*) context; | 298 | struct ClientContext *ctx = (struct ClientContext*) context; |
165 | 256 | ctx->reconfigure = 1; | 299 | ctx->reconfigure = 1; |
166 | 257 | } | 300 | } |
167 | 258 | 301 | ||
168 | 302 | static void apply_to_base_configuration(MirConnection *connection) | ||
169 | 303 | { | ||
170 | 304 | if (!conf) | ||
171 | 305 | return; | ||
172 | 306 | |||
173 | 307 | mir_connection_preview_base_display_configuration(connection, conf, 2); | ||
174 | 308 | puts("Applying to base configuration"); | ||
175 | 309 | mir_connection_confirm_base_display_configuration(connection, conf); | ||
176 | 310 | } | ||
177 | 311 | |||
178 | 259 | static void handle_keyboard_event(struct ClientContext *ctx, MirKeyboardEvent const* event) | 312 | static void handle_keyboard_event(struct ClientContext *ctx, MirKeyboardEvent const* event) |
179 | 260 | { | 313 | { |
180 | 261 | if (mir_keyboard_event_action(event) != mir_keyboard_action_up) | 314 | if (mir_keyboard_event_action(event) != mir_keyboard_action_up) |
181 | @@ -271,6 +324,9 @@ | |||
182 | 271 | 324 | ||
183 | 272 | switch (key_code) | 325 | switch (key_code) |
184 | 273 | { | 326 | { |
185 | 327 | case XKB_KEY_a: | ||
186 | 328 | apply_to_base_configuration(ctx->connection); | ||
187 | 329 | break; | ||
188 | 274 | case XKB_KEY_q: | 330 | case XKB_KEY_q: |
189 | 275 | ctx->running = 0; | 331 | ctx->running = 0; |
190 | 276 | break; | 332 | break; |
191 | @@ -284,7 +340,21 @@ | |||
192 | 284 | configure_display(ctx, configuration_mode_vertical, 0); | 340 | configure_display(ctx, configuration_mode_vertical, 0); |
193 | 285 | break; | 341 | break; |
194 | 286 | case XKB_KEY_p: | 342 | case XKB_KEY_p: |
196 | 287 | print_current_configuration(ctx->connection); | 343 | if (!conf) |
197 | 344 | conf = mir_connection_create_display_configuration(ctx->connection); | ||
198 | 345 | print_current_configuration(); | ||
199 | 346 | break; | ||
200 | 347 | case XKB_KEY_Left: | ||
201 | 348 | configure_display(ctx, configuration_mode_right, 0); | ||
202 | 349 | break; | ||
203 | 350 | case XKB_KEY_Up: | ||
204 | 351 | configure_display(ctx, configuration_mode_up, 0); | ||
205 | 352 | break; | ||
206 | 353 | case XKB_KEY_Right: | ||
207 | 354 | configure_display(ctx, configuration_mode_left, 0); | ||
208 | 355 | break; | ||
209 | 356 | case XKB_KEY_Down: | ||
210 | 357 | configure_display(ctx, configuration_mode_down, 0); | ||
211 | 288 | break; | 358 | break; |
212 | 289 | } | 359 | } |
213 | 290 | } | 360 | } |
214 | @@ -292,15 +362,16 @@ | |||
215 | 292 | static void event_callback( | 362 | static void event_callback( |
216 | 293 | MirWindow* surface, MirEvent const* event, void* context) | 363 | MirWindow* surface, MirEvent const* event, void* context) |
217 | 294 | { | 364 | { |
219 | 295 | (void) surface; | 365 | mir_eglapp_handle_event(surface, event, context); |
220 | 366 | |||
221 | 296 | struct ClientContext *ctx = (struct ClientContext*) context; | 367 | struct ClientContext *ctx = (struct ClientContext*) context; |
223 | 297 | 368 | ||
224 | 298 | if (mir_event_get_type(event) != mir_event_type_input) | 369 | if (mir_event_get_type(event) != mir_event_type_input) |
225 | 299 | return; | 370 | return; |
226 | 300 | MirInputEvent const* input_event = mir_event_get_input_event(event); | 371 | MirInputEvent const* input_event = mir_event_get_input_event(event); |
227 | 301 | if (mir_input_event_get_type(input_event) != mir_input_event_type_key) | 372 | if (mir_input_event_get_type(input_event) != mir_input_event_type_key) |
228 | 302 | return; | 373 | return; |
230 | 303 | 374 | ||
231 | 304 | handle_keyboard_event(ctx, mir_input_event_get_keyboard_event(input_event)); | 375 | handle_keyboard_event(ctx, mir_input_event_get_keyboard_event(input_event)); |
232 | 305 | } | 376 | } |
233 | 306 | 377 | ||
234 | @@ -317,7 +388,9 @@ | |||
235 | 317 | " h: arrange outputs horizontally in the virtual space\n" | 388 | " h: arrange outputs horizontally in the virtual space\n" |
236 | 318 | " v: arrange outputs vertically in the virtual space\n" | 389 | " v: arrange outputs vertically in the virtual space\n" |
237 | 319 | " 1-9: enable only the Nth connected output (in the order returned by the hardware)\n" | 390 | " 1-9: enable only the Nth connected output (in the order returned by the hardware)\n" |
239 | 320 | " p: print current display configuration\n"); | 391 | " Arrows: orient display (sets \"down\" direction)\n" |
240 | 392 | " p: print current display configuration\n" | ||
241 | 393 | " a: apply current display configuration globally\n"); | ||
242 | 321 | 394 | ||
243 | 322 | return 1; | 395 | return 1; |
244 | 323 | } | 396 | } |
245 | @@ -351,6 +424,9 @@ | |||
246 | 351 | } | 424 | } |
247 | 352 | } | 425 | } |
248 | 353 | 426 | ||
249 | 427 | if (conf) | ||
250 | 428 | mir_display_config_release(conf); | ||
251 | 429 | |||
252 | 354 | mir_eglapp_cleanup(); | 430 | mir_eglapp_cleanup(); |
253 | 355 | 431 | ||
254 | 356 | return 0; | 432 | return 0; |
PASSED: Continuous integration, rev:4212 /mir-jenkins. ubuntu. com/job/ mir-ci/ 3503/ /mir-jenkins. ubuntu. com/job/ build-mir/ 4793 /mir-jenkins. ubuntu. com/job/ build-0- fetch/4970 /mir-jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= artful/ 4959 /mir-jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= xenial/ 4959 /mir-jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= zesty/4959 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= clang,platform= mesa,release= artful/ 4830 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= clang,platform= mesa,release= artful/ 4830/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= clang,platform= mesa,release= zesty/4830 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= clang,platform= mesa,release= zesty/4830/ artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= artful/ 4830 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= artful/ 4830/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= xenial/ 4830 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= xenial/ 4830/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= zesty/4830 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= zesty/4830/ artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= cross-armhf, compiler= gcc,platform= mesa,release= artful/ 4830 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= cross-armhf, compiler= gcc,platform= mesa,release= artful/ 4830/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= cross-armhf, compiler= gcc,platform= mesa,release= zesty/4830 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= cross-armhf, compiler= gcc,platform= mesa,release= zesty/4830/ artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= mesa,release= xenial/ 4830 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= mesa,release= xenial/ 4830/artifact/ output/ *zip*/output. zip
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
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:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild: /mir-jenkins. ubuntu. com/job/ mir-ci/ 3503/rebuild
https:/