Merge lp:~afrantzis/mir/fix-1217877 into lp:mir

[Alexandros Frantzis]

graphics,examples: Don't enable more outputs than supported when changing the display configuration

graphics,examples: Don't enable more outputs than supported when changing the display configuration

[Alan Griffiths (alan-griffiths)]

LGTM

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1152
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/208/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2436
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2321
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/205
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/205/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/mir-team-mir-development-branch-ci/208/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

looks okay to me too

review: Approve

[Robert Carr (robertcarr)]

+1

review: Approve

=== modified file 'examples/demo_client_display_config.c'
--- examples/demo_client_display_config.c	2013-09-25 07:51:36 +0000
+++ examples/demo_client_display_config.c	2013-10-21 10:42:37 +0000
@@ -29,6 +29,7 @@
 #include <signal.h>
 #include <string.h>
 #include <time.h>
+#include <stdlib.h>
 
 typedef enum
 {
@@ -48,6 +49,52 @@
     volatile sig_atomic_t reconfigure;
 };
 
+struct Card
+{
+    uint32_t card_id;
+    uint32_t available_outputs;
+};
+
+struct Cards
+{
+    size_t num_cards;
+    struct Card *cards;
+};
+
+static struct Cards *cards_create(struct MirDisplayConfiguration *conf)
+{
+    struct Cards *cards = (struct Cards*) calloc(1, sizeof(struct Cards));
+    cards->num_cards = conf->num_cards;
+    cards->cards = (struct Card*) calloc(cards->num_cards, sizeof(struct Card));
+
+    for (size_t i = 0; i < cards->num_cards; i++)
+    {
+        cards->cards[i].card_id = conf->cards[i].card_id;
+        cards->cards[i].available_outputs =
+            conf->cards[i].max_simultaneous_outputs;
+    }
+
+    return cards;
+}
+
+static void cards_free(struct Cards *cards)
+{
+    free(cards->cards);
+    free(cards);
+}
+
+static struct Card *cards_find_card(struct Cards *cards, uint32_t card_id)
+{
+    for (size_t i = 0; i < cards->num_cards; i++)
+    {
+        if (cards->cards[i].card_id == card_id)
+            return &cards->cards[i];
+    }
+
+    fprintf(stderr, "Error: Couldn't find card with id: %u\n", card_id);
+    abort();
+}
+
 static void print_current_configuration(MirConnection *connection)
 {
     MirDisplayConfiguration *conf = mir_connection_create_display_config(connection);
@@ -101,52 +148,75 @@
 
 static void configure_display_clone(struct MirDisplayConfiguration *conf)
 {
+    struct Cards *cards = cards_create(conf);
+
     for (uint32_t i = 0; i < conf->num_outputs; i++)
     {
         MirDisplayOutput *output = &conf->outputs[i];
-        if (output->connected && output->num_modes > 0)
+        struct Card *card = cards_find_card(cards, output->card_id);
+
+        if (output->connected && output->num_modes > 0 &&
+            card->available_outputs > 0)
         {
             output->used = 1;
             output->current_mode = 0;
             output->position_x = 0;
             output->position_y = 0;
+            --card->available_outputs;
         }
     }
+
+    cards_free(cards);
 }
 
 static void configure_display_horizontal(struct MirDisplayConfiguration *conf)
 {
+    struct Cards *cards = cards_create(conf);
+
     uint32_t max_x = 0;
     for (uint32_t i = 0; i < conf->num_outputs; i++)
     {
         MirDisplayOutput *output = &conf->outputs[i];
-        if (output->connected && output->num_modes > 0)
+        struct Card *card = cards_find_card(cards, output->card_id);
+
+        if (output->connected && output->num_modes > 0 &&
+            card->available_outputs > 0)
         {
             output->used = 1;
             output->current_mode = 0;
             output->position_x = max_x;
             output->position_y = 0;
             max_x += output->modes[0].horizontal_resolution;
+            --card->available_outputs;
         }
     }
 
+    cards_free(cards);
 }
 
 static void configure_display_vertical(struct MirDisplayConfiguration *conf)
 {
+    struct Cards *cards = cards_create(conf);
+
     uint32_t max_y = 0;
     for (uint32_t i = 0; i < conf->num_outputs; i++)
     {
         MirDisplayOutput *output = &conf->outputs[i];
-        if (output->connected && output->num_modes > 0)
+        struct Card *card = cards_find_card(cards, output->card_id);
+
+        if (output->connected && output->num_modes > 0 &&
+            card->available_outputs > 0)
         {
             output->used = 1;
             output->current_mode = 0;
             output->position_x = 0;
             output->position_y = max_y;
             max_y += output->modes[0].vertical_resolution;
+            --card->available_outputs;
         }
     }
+
+    cards_free(cards);
 }
 
 static void configure_display_single(struct MirDisplayConfiguration *conf, int output_num)

=== modified file 'examples/server_configuration.cpp'
--- examples/server_configuration.cpp	2013-09-12 21:36:55 +0000
+++ examples/server_configuration.cpp	2013-10-21 10:42:37 +0000
@@ -25,6 +25,7 @@
 #include <string>
 
 #include <linux/input.h>
+#include <unordered_map>
 
 namespace me = mir::examples;
 namespace mg = mir::graphics;
@@ -45,15 +46,24 @@
     {
         size_t const preferred_mode_index{0};
         int max_x = 0;
+        std::unordered_map<mg::DisplayConfigurationCardId, size_t> available_outputs_for_card;
+
+        conf.for_each_card(
+            [&](mg::DisplayConfigurationCard const& card)
+            {
+                available_outputs_for_card[card.id] = card.max_simultaneous_outputs;
+            });
 
         conf.for_each_output(
             [&](mg::DisplayConfigurationOutput const& conf_output)
             {
-                if (conf_output.connected && conf_output.modes.size() > 0)
+                if (conf_output.connected && conf_output.modes.size() > 0 &&
+                    available_outputs_for_card[conf_output.card_id] > 0)
                 {
                     conf.configure_output(conf_output.id, true, geom::Point{max_x, 0},
                                           preferred_mode_index, mir_power_mode_on);
                     max_x += conf_output.modes[preferred_mode_index].size.width.as_int();
+                    --available_outputs_for_card[conf_output.card_id];
                 }
                 else
                 {

=== modified file 'src/platform/graphics/default_display_configuration_policy.cpp'
--- src/platform/graphics/default_display_configuration_policy.cpp	2013-09-12 21:36:55 +0000
+++ src/platform/graphics/default_display_configuration_policy.cpp	2013-10-21 10:42:37 +0000
@@ -19,17 +19,27 @@
 #include "mir/graphics/default_display_configuration_policy.h"
 #include "mir/graphics/display_configuration.h"
 
+#include <unordered_map>
+
 namespace mg = mir::graphics;
 namespace geom = mir::geometry;
 
 void mg::DefaultDisplayConfigurationPolicy::apply_to(DisplayConfiguration& conf)
 {
     static MirPowerMode const default_power_state = mir_power_mode_on;
+    std::unordered_map<DisplayConfigurationCardId, size_t> available_outputs_for_card;
+
+    conf.for_each_card(
+        [&](DisplayConfigurationCard const& card)
+        {
+            available_outputs_for_card[card.id] = card.max_simultaneous_outputs;
+        });
 
     conf.for_each_output(
-        [&conf](DisplayConfigurationOutput const& conf_output)
+        [&](DisplayConfigurationOutput const& conf_output)
         {
-            if (conf_output.connected && conf_output.modes.size() > 0)
+            if (conf_output.connected && conf_output.modes.size() > 0 &&
+                available_outputs_for_card[conf_output.card_id] > 0)
             {
                 size_t preferred_mode_index{conf_output.preferred_mode_index};
                 if (preferred_mode_index > conf_output.modes.size())
@@ -37,6 +47,8 @@
 
                 conf.configure_output(conf_output.id, true, geom::Point(),
                                       preferred_mode_index, default_power_state);
+
+                --available_outputs_for_card[conf_output.card_id];
             }
             else
             {

=== modified file 'tests/unit-tests/graphics/test_default_display_configuration_policy.cpp'
--- tests/unit-tests/graphics/test_default_display_configuration_policy.cpp	2013-09-12 21:36:55 +0000
+++ tests/unit-tests/graphics/test_default_display_configuration_policy.cpp	2013-10-21 10:42:37 +0000
@@ -31,8 +31,8 @@
 class MockDisplayConfiguration : public mg::DisplayConfiguration
 {
 public:
-    MockDisplayConfiguration()
-        : card_id{1}
+    MockDisplayConfiguration(size_t max_simultaneous_outputs)
+        : card_id{1}, max_simultaneous_outputs{max_simultaneous_outputs}
     {
         /* Connected with modes */
         outputs.push_back(
@@ -114,11 +114,19 @@
                 0,
                 mir_power_mode_on
             });
+
+        if (max_simultaneous_outputs == max_simultaneous_outputs_all)
+            max_simultaneous_outputs = outputs.size();
+    }
+
+    MockDisplayConfiguration()
+        : MockDisplayConfiguration{max_simultaneous_outputs_all}
+    {
     }
 
     void for_each_card(std::function<void(mg::DisplayConfigurationCard const&)> f) const
     {
-        f({card_id, outputs.size()});
+        f({card_id, max_simultaneous_outputs});
     }
 
     void for_each_output(std::function<void(mg::DisplayConfigurationOutput const&)> f) const
@@ -131,7 +139,9 @@
                                         geom::Point, size_t, MirPowerMode));
 
 private:
+    static const size_t max_simultaneous_outputs_all{std::numeric_limits<size_t>::max()};
     mg::DisplayConfigurationCardId const card_id;
+    size_t max_simultaneous_outputs;
     std::vector<mg::DisplayConfigurationOutput> outputs;
 };
 
@@ -176,3 +186,25 @@
     policy.apply_to(conf);
 }
 
+TEST(DefaultDisplayConfigurationPolicyTest, does_not_enable_more_outputs_than_supported)
+{
+    using namespace ::testing;
+
+    size_t const max_simultaneous_outputs{1};
+    mg::DefaultDisplayConfigurationPolicy policy;
+    MockDisplayConfiguration conf{max_simultaneous_outputs};
+
+    size_t output_count{0};
+    conf.for_each_output([&output_count](mg::DisplayConfigurationOutput const&)
+    {
+        ++output_count;
+    });
+
+    EXPECT_CALL(conf, configure_output(_, true, _, _, _))
+        .Times(AtMost(max_simultaneous_outputs));
+
+    EXPECT_CALL(conf, configure_output(_, false, _, _, _))
+        .Times(AtLeast(output_count - max_simultaneous_outputs));
+
+    policy.apply_to(conf);
+}