Merge lp:~raof/mir/nested-scale-setting into lp:mir

[Chris Halse Rogers]

Fix output configuration scale_factor and form_factor for nested servers.

The scale_factor and form_factor aren't exposed in the client MirDisplayOutput struct (and can't easily be, as that's an ABI break), and even if they were it's not clear that it makes sense to let clients set these values.

Store this configuration locally in the NestedDisplayConfiguration instead.

Fixes: https://bugs.launchpad.net/mir/+bug/1535780

Fix output configuration scale_factor and form_factor for nested servers.

The scale_factor and form_factor aren't exposed in the client MirDisplayOutput struct (and can't easily be, as that's an ABI break), and even if they were it's not clear that it makes sense to let clients set these values.

Store this configuration locally in the NestedDisplayConfiguration instead.

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

FAILED: Continuous integration, rev:3254
https://mir-jenkins.ubuntu.com/job/mir-ci/98/
Executed test runs:
    None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/98/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3254
http://jenkins.qa.ubuntu.com/job/mir-ci/6072/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5609/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4516/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5565/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/396/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/396
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/396/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5562/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/6072/rebuild

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:3255
https://mir-jenkins.ubuntu.com/job/mir-ci/107/
Executed test runs:
    None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/107/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3255
http://jenkins.qa.ubuntu.com/job/mir-ci/6081/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5619
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4526
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5575
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/405/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/405
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/405/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5572
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5572/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/8018
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26858

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/6081/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3255
http://jenkins.qa.ubuntu.com/job/mir-ci/6085/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5624
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4531
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5580
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/409
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/409/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/409/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5577
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5577/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/8022
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26868

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/6085/rebuild

review: Needs Fixing (continuous-integration)

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

PASSED: Continuous integration, rev:3256
https://mir-jenkins.ubuntu.com/job/mir-ci/121/
Executed test runs:
    None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/121/console

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

review: Approve (continuous-integration)

[Alberto Aguirre (albaguirre)]

---
179 + auto base_configuration_now_set = std::make_shared<mt::Signal>();
---

Not used anymore.

LGTM otherwise.

review: Approve

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

PASSED: Continuous integration, rev:3257
https://mir-jenkins.ubuntu.com/job/mir-ci/124/
Executed test runs:
    None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/124/console

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

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3256
http://jenkins.qa.ubuntu.com/job/mir-ci/6101/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5646
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4553
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5602
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/319
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/425
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/425/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/425
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/425/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5599
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5599/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/8040
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26912
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/315
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/315/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-xenial-touch/171
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26925

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/6101/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3257
http://jenkins.qa.ubuntu.com/job/mir-ci/6103/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5650
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4557
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5606
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/321
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/427
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/427/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/427
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/427/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5603
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5603/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/8044
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26929
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/317
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/317/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-xenial-touch/173
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26933

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/6103/rebuild

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Ummkay.

review: Approve

[Alan Griffiths (alan-griffiths)]

+ while (!done && (std::chrono::steady_clock::now() < end_time))

Hmm. Can't we use the display report or something to detect change?

review: Approve

[Chris Halse Rogers (raof)]

Hm.

Probably, yes. However...

=== modified file 'src/server/graphics/nested/nested_display_configuration.cpp'
--- src/server/graphics/nested/nested_display_configuration.cpp	2016-01-20 23:59:18 +0000
+++ src/server/graphics/nested/nested_display_configuration.cpp	2016-01-21 23:53:03 +0000
@@ -59,6 +59,8 @@
     : mg::DisplayConfiguration(),
       display_config{copy_config(other.display_config.get())}
 {
+    std::lock_guard<std::mutex> lock{other.local_config_mutex};
+    local_config = other.local_config;
 }
 
 void mgn::NestedDisplayConfiguration::for_each_card(
@@ -78,7 +80,7 @@
     std::for_each(
         display_config->outputs,
         display_config->outputs+display_config->num_outputs,
-        [&f](MirDisplayOutput const& mir_output)
+        [&f, this](MirDisplayOutput const& mir_output)
         {
             std::vector<MirPixelFormat> formats;
             formats.reserve(mir_output.num_output_formats);
@@ -90,6 +92,8 @@
             for (auto p = mir_output.modes; p != mir_output.modes+mir_output.num_modes; ++p)
                 modes.push_back(DisplayConfigurationMode{{p->horizontal_resolution, p->vertical_resolution}, p->refresh_rate});
 
+            auto local_config = get_local_config_for(mir_output.output_id);
+
             DisplayConfigurationOutput const output{
                 DisplayConfigurationOutputId(mir_output.output_id),
                 DisplayConfigurationCardId(mir_output.card_id),
@@ -105,8 +109,8 @@
                 mir_output.current_format,
                 mir_output.power_mode,
                 mir_output.orientation,
-                1.0f,
-                mir_form_factor_monitor
+                local_config.scale,
+                local_config.form_factor
             };
 
             f(output);
@@ -122,7 +126,7 @@
     std::for_each(
         display_config->outputs,
         display_config->outputs+display_config->num_outputs,
-        [&f](MirDisplayOutput& mir_output)
+        [&f, this](MirDisplayOutput& mir_output)
         {
             std::vector<MirPixelFormat> formats;
             formats.reserve(mir_output.num_output_formats);
@@ -145,6 +149,8 @@
                         p->refresh_rate});
             }
 
+            auto local_config = get_local_config_for(mir_output.output_id);
+
             DisplayConfigurationOutput output{
                 DisplayConfigurationOutputId(mir_output.output_id),
                 DisplayConfigurationCardId(mir_output.card_id),
@@ -161,13 +167,15 @@
                 mir_output.current_format,
                 mir_output.power_mode,
                 mir_output.orientation,
-                1.0f,
-                mir_form_factor_monitor
+                local_config.scale,
+                local_config.form_factor
             };
             UserDisplayConfigurationOutput user(output);
 
             f(user);
 
+            set_local_config_for(mir_output.output_id, {user.scale, user.form_factor });
+
             mir_output.current_mode = output.current_mode_index;
             mir_output.current_format = output.current_format;
             mir_output.position_x = output.top_left.x.as_int();
@@ -182,3 +190,26 @@
 {
     return std::make_unique<mgn::NestedDisplayConfiguration>(*this);
 }
+
+mgn::NestedDisplayConfiguration::LocalOutputConfig
+mgn::NestedDisplayConfiguration::get_local_config_for(uint32_t output_id) const
+{
+    std::lock_guard<std::mutex> lock{local_config_mutex};
+
+    constexpr LocalOutputConfig default_values {1.0f, mir_form_factor_monitor };
+
+    bool inserted;
+    decltype(local_config)::iterator keypair;
+
+    std::tie(keypair, inserted) = local_config.insert(std::make_pair(output_id, default_values));
+
+    // We don't care whether we inserted the default values or retrieved the previously set value.
+    return keypair->second;
+}
+
+void mgn::NestedDisplayConfiguration::set_local_config_for(uint32_t output_id, LocalOutputConfig const& config)
+{
+    std::lock_guard<std::mutex> lock{local_config_mutex};
+
+    local_config[output_id] = config;
+}

=== modified file 'src/server/graphics/nested/nested_display_configuration.h'
--- src/server/graphics/nested/nested_display_configuration.h	2016-01-20 23:59:18 +0000
+++ src/server/graphics/nested/nested_display_configuration.h	2016-01-21 23:53:03 +0000
@@ -22,6 +22,8 @@
 #include "mir/graphics/display_configuration.h"
 #include "mir_toolkit/client_types.h"
 #include <memory>
+#include <mutex>
+#include <unordered_map>
 
 namespace mir
 {
@@ -45,7 +47,25 @@
 
 private:
     std::shared_ptr<MirDisplayConfiguration> display_config;
+
+    /*
+     * The client display config doesn't currently expose the form factor or scaling factor, nor is it
+     * entirely clear that it should allow a client to set them.
+     *
+     * We therefore need to store these explicitly in the NestedConfiguration.
+     */
+    std::mutex mutable local_config_mutex;
+    struct LocalOutputConfig
+    {
+        float scale;
+        MirFormFactor form_factor;
+    };
+    std::unordered_map<uint32_t, LocalOutputConfig> mutable local_config;
+
+    LocalOutputConfig get_local_config_for(uint32_t output_id) const;
+    void set_local_config_for(uint32_t output_id, LocalOutputConfig const& config);
 };
+
 }
 }
 }

=== modified file 'tests/acceptance-tests/test_nested_mir.cpp'
--- tests/acceptance-tests/test_nested_mir.cpp	2016-01-20 23:59:18 +0000
+++ tests/acceptance-tests/test_nested_mir.cpp	2016-01-21 23:53:03 +0000
@@ -42,6 +42,7 @@
 #include "mir/test/doubles/fake_display.h"
 #include "mir/test/doubles/stub_cursor.h"
 #include "mir/test/doubles/stub_display_configuration.h"
+#include "mir/test/signal.h"
 
 #include "mir/test/doubles/nested_mock_egl.h"
 #include "mir/test/fake_shared.h"
@@ -469,6 +470,98 @@
     EXPECT_THAT(outputs, ContainerEq(display_geometry));
 }
 
+TEST_F(NestedServer, shell_sees_set_scaling_factor)
+{
+    using namespace std::literals::chrono_literals;
+    NestedMirRunner nested_mir{new_connection()};
+
+    constexpr float expected_scale{2.3f};
+    constexpr MirFormFactor expected_form_factor{mir_form_factor_tv};
+
+    auto const configurator = nested_mir.server.the_display_configuration_controller();
+    std::shared_ptr<mg::DisplayConfiguration> config = nested_mir.server.the_display()->configuration();
+
+    config->for_each_output([] (mg::UserDisplayConfigurationOutput& output)
+        {
+            output.scale = expected_scale;
+            output.form_factor = expected_form_factor;
+        });
+
+    configurator->set_base_configuration(config);
+
+    /* Now, because we have absolutely no idea when the call to set_base_configuration will *actually*
+     * set the base configuration we get to poll configuration() until we see that it's actually changed.
+     */
+    auto const end_time = std::chrono::steady_clock::now() + 10s;
+    bool done{false};
+    while (!done && (std::chrono::steady_clock::now() < end_time))
+    {
+        auto const new_config = nested_mir.server.the_display()->configuration();
+
+        new_config->for_each_output([&done, expected_scale, expected_form_factor] (auto const& output)
+                                    {
+                                        if (output.scale == expected_scale &&
+                                            output.form_factor == expected_form_factor)
+                                        {
+                                            done = true;
+                                        }
+                                    });
+    }
+
+    EXPECT_THAT(std::chrono::steady_clock::now(), Le(end_time)) << "Timed out waiting for display configuration to propagate";
+}
+
+TEST_F(NestedServer, client_sees_set_scaling_factor)
+{
+    using namespace std::literals::chrono_literals;
+
+    NestedMirRunner nested_mir{new_connection()};
+
+    constexpr float expected_scale{2.3f};
+    constexpr MirFormFactor expected_form_factor{mir_form_factor_tv};
+
+    auto const configurator = nested_mir.server.the_display_configuration_controller();
+    std::shared_ptr<mg::DisplayConfiguration> config = nested_mir.server.the_display()->configuration();
+
+    config->for_each_output([] (mg::UserDisplayConfigurationOutput& output)
+                            {
+                                output.scale = expected_scale;
+                                output.form_factor = expected_form_factor;
+                            });
+
+    configurator->set_base_configuration(config);
+
+    Client client{nested_mir};
+
+    auto spec = mir_connection_create_spec_for_normal_surface(client.connection,
+        800, 600,
+        mir_pixel_format_abgr_8888);
+
+    mt::Signal surface_event_received;
+    mir_surface_spec_set_event_handler(spec, [](MirSurface*, MirEvent const* event, void* ctx)
+        {
+            if (mir_event_get_type(event) == mir_event_type_surface_output)
+            {
+                auto surface_event = mir_event_get_surface_output_event(event);
+                EXPECT_THAT(mir_surface_output_event_get_form_factor(surface_event), Eq(expected_form_factor));
+                EXPECT_THAT(mir_surface_output_event_get_scale(surface_event), Eq(expected_scale));
+                auto signal = static_cast<mt::Signal*>(ctx);
+                signal->raise();
+            }
+        },
+        &surface_event_received);
+
+    auto surface = mir_surface_create_sync(spec);
+    mir_surface_spec_release(spec);
+
+    auto stream = mir_surface_get_buffer_stream(surface);
+    mir_buffer_stream_swap_buffers_sync(stream);
+
+    mir_surface_release_sync(surface);
+
+    EXPECT_TRUE(surface_event_received.wait_for(30s));
+}
+
 //////////////////////////////////////////////////////////////////
 // TODO the following test was used in investigating lifetime issues.
 // TODO it may not have much long term value, but decide that later.