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

[Alan Griffiths]

[mir_demo_client_display_config] add orientation changing

[Mir CI Bot (mir-ci-bot)]

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)

[Mir CI Bot (mir-ci-bot)]

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)]

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

[Alan Griffiths (alan-griffiths)]

> 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)]

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)]

> 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)]

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)]

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)]

> 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)]

Ah, fair enough.

review: Approve

=== modified file 'examples/demo_client_display_config.c'
--- examples/demo_client_display_config.c	2017-05-08 03:04:26 +0000
+++ examples/demo_client_display_config.c	2017-07-25 07:47:20 +0000
@@ -1,7 +1,7 @@
 /*
  * Client-side display configuration demo.
  *
- * Copyright © 2013 Canonical Ltd.
+ * Copyright © 2013, 2017 Canonical Ltd.
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
@@ -37,7 +37,11 @@
     configuration_mode_clone,
     configuration_mode_horizontal,
     configuration_mode_vertical,
-    configuration_mode_single
+    configuration_mode_single,
+    configuration_mode_left,
+    configuration_mode_right,
+    configuration_mode_up,
+    configuration_mode_down,
 } ConfigurationMode;
 
 struct ClientContext
@@ -49,9 +53,13 @@
     volatile sig_atomic_t reconfigure;
 };
 
-static void print_current_configuration(MirConnection *connection)
+static MirDisplayConfig *conf = NULL;
+
+static void print_current_configuration()
 {
-    MirDisplayConfig *conf = mir_connection_create_display_configuration(connection);
+    if (!conf)
+        return;
+
     size_t num_outputs = mir_display_config_get_num_outputs(conf);
 
     for (uint32_t i = 0; i < num_outputs; i++)
@@ -64,8 +72,8 @@
         bool connected = mir_output_get_connection_state(output) ==
             mir_output_connection_state_connected;
 
-        printf("Output id: %d connected: %d used: %d position_x: %d position_y: %d",
-               id, connected, used, position_x, position_y);
+        printf("Output id: %d connected: %d used: %d position_x: %d position_y: %d orientation: %d",
+               id, connected, used, position_x, position_y, mir_output_get_orientation(output));
 
         MirOutputMode const* current = mir_output_get_current_mode(output);
         if (current)
@@ -80,8 +88,6 @@
             printf("\n");
         }
     }
-
-    mir_display_config_release(conf);
 }
 
 static int apply_configuration(MirConnection *connection, MirDisplayConfig *conf)
@@ -208,31 +214,67 @@
     }
 }
 
+static void orient_display(MirDisplayConfig *conf, MirOrientation orientation)
+{
+    size_t num_outputs = mir_display_config_get_num_outputs(conf);
+
+    for (size_t i = 0; i < num_outputs; i++)
+    {
+        MirOutput *output = mir_display_config_get_mutable_output(conf, i);
+        mir_output_set_orientation(output, orientation);
+    }
+}
+
 static void configure_display(struct ClientContext *context, ConfigurationMode mode,
                               int mode_data)
 {
-    MirDisplayConfig *conf =
-        mir_connection_create_display_configuration(context->connection);
+    if (!conf)
+        conf = mir_connection_create_display_configuration(context->connection);
 
-    if (mode == configuration_mode_clone)
+    switch (mode)
     {
+    case configuration_mode_clone:
         configure_display_clone(conf);
         printf("Applying clone configuration: ");
-    }
-    else if (mode == configuration_mode_vertical)
-    {
+        break;
+
+    case configuration_mode_vertical:
         configure_display_vertical(conf);
         printf("Applying vertical configuration: ");
-    }
-    else if (mode == configuration_mode_horizontal)
-    {
+        break;
+
+    case configuration_mode_horizontal:
         configure_display_horizontal(conf);
         printf("Applying horizontal configuration: ");
-    }
-    else if (mode == configuration_mode_single)
-    {
+        break;
+
+    case configuration_mode_single:
         configure_display_single(conf, mode_data);
         printf("Applying single configuration for output %d: ", mode_data);
+        break;
+
+    case configuration_mode_left:
+        orient_display(conf, mir_orientation_left);
+        printf("Applying orientation left: ");
+        break;
+
+    case configuration_mode_right:
+        orient_display(conf, mir_orientation_right);
+        printf("Applying orientation right: ");
+        break;
+
+    case configuration_mode_up:
+        orient_display(conf, mir_orientation_inverted);
+        printf("Applying orientation up: ");
+        break;
+
+    case configuration_mode_down:
+        orient_display(conf, mir_orientation_normal);
+        printf("Applying orientation down: ");
+        break;
+
+    default:
+        break;
     }
 
     if (apply_configuration(context->connection, conf))
@@ -240,22 +282,33 @@
         context->mode = mode;
         context->mode_data = mode_data;
     }
-
-    mir_display_config_release(conf);
 }
 
 static void display_change_callback(MirConnection *connection, void *context)
 {
-    (void)context;
-
     printf("=== Display configuration changed === \n");
 
-    print_current_configuration(connection);
+    if (conf)
+        mir_display_config_release(conf);
+
+    conf = mir_connection_create_display_configuration(connection);
+
+    print_current_configuration();
 
     struct ClientContext *ctx = (struct ClientContext*) context;
     ctx->reconfigure = 1;
 }
 
+static void apply_to_base_configuration(MirConnection *connection)
+{
+    if (!conf)
+        return;
+
+    mir_connection_preview_base_display_configuration(connection, conf, 2);
+    puts("Applying to base configuration");
+    mir_connection_confirm_base_display_configuration(connection, conf);
+}
+
 static void handle_keyboard_event(struct ClientContext *ctx, MirKeyboardEvent const* event)
 {
     if (mir_keyboard_event_action(event) != mir_keyboard_action_up)
@@ -271,6 +324,9 @@
 
     switch (key_code)
     {
+    case XKB_KEY_a:
+        apply_to_base_configuration(ctx->connection);
+        break;
     case XKB_KEY_q:
         ctx->running = 0;
         break;
@@ -284,7 +340,21 @@
         configure_display(ctx, configuration_mode_vertical, 0);
         break;
     case XKB_KEY_p:
-        print_current_configuration(ctx->connection);
+        if (!conf)
+            conf = mir_connection_create_display_configuration(ctx->connection);
+        print_current_configuration();
+        break;
+    case XKB_KEY_Left:
+        configure_display(ctx, configuration_mode_right, 0);
+        break;
+    case XKB_KEY_Up:
+        configure_display(ctx, configuration_mode_up, 0);
+        break;
+    case XKB_KEY_Right:
+        configure_display(ctx, configuration_mode_left, 0);
+        break;
+    case XKB_KEY_Down:
+        configure_display(ctx, configuration_mode_down, 0);
         break;
     }
 }
@@ -292,15 +362,16 @@
 static void event_callback(
     MirWindow* surface, MirEvent const* event, void* context)
 {
-    (void) surface;
+    mir_eglapp_handle_event(surface, event, context);
+
     struct ClientContext *ctx = (struct ClientContext*) context;
-    
+
     if (mir_event_get_type(event) != mir_event_type_input)
         return;
     MirInputEvent const* input_event = mir_event_get_input_event(event);
     if (mir_input_event_get_type(input_event) != mir_input_event_type_key)
         return;
-    
+
     handle_keyboard_event(ctx, mir_input_event_get_keyboard_event(input_event));
 }
 
@@ -317,7 +388,9 @@
                " h: arrange outputs horizontally in the virtual space\n"
                " v: arrange outputs vertically in the virtual space\n"
                " 1-9: enable only the Nth connected output (in the order returned by the hardware)\n"
-               " p: print current display configuration\n");
+               " Arrows: orient display (sets \"down\" direction)\n"
+               " p: print current display configuration\n"
+               " a: apply current display configuration globally\n");
 
         return 1;
     }
@@ -351,6 +424,9 @@
         }
     }
 
+    if (conf)
+        mir_display_config_release(conf);
+
     mir_eglapp_cleanup();
 
     return 0;