Mir

Merge lp:~vanvugt/mir/nested-subpixel-arrangement into lp:mir

  1. nested-subpixel-arrangement
  2. Merge into development-branch
Proposed by Daniel van Vugt
Status: Merged
Approved by: Chris Halse Rogers
Approved revision: no longer in the source branch.
Merged at revision: 4082
Proposed branch: lp:~vanvugt/mir/nested-subpixel-arrangement
Merge into: lp:mir
Diff against target: 93 lines (+27/-6)
4 files modified
src/server/graphics/nested/nested_display_configuration.cpp (+8/-5)
src/server/graphics/nested/nested_display_configuration.h (+1/-1)
tests/unit-tests/platforms/nested/mir_display_configuration_builder.cpp (+2/-0)
tests/unit-tests/platforms/nested/test_nested_display_configuration.cpp (+16/-0)
To merge this branch: bzr merge lp:~vanvugt/mir/nested-subpixel-arrangement
Reviewer Review Type Date Requested Status
Chris Halse Rogers Approve
Mir CI Bot continuous-integration Approve
Kevin DuBois (community) Approve
Review via email: mp+317586@code.launchpad.net

Commit message

Expose the hardware subpixel arrangement in nested servers.
This is the final missing piece for LP: #1393578.

Description of the change

Same problem as we had with EDID. Looks like we'll also have the same problem with scale, form_factor and gamma.

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

PASSED: Continuous integration, rev:4044
https://mir-jenkins.ubuntu.com/job/mir-ci/3010/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4020
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4107
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4097
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4097
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4097
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4047
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4047/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4047
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4047/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4047
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4047/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4047
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4047/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4047
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4047/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4047
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4047/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:4045
https://mir-jenkins.ubuntu.com/job/mir-ci/3012/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4022
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4109
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4099
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4099
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4099
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4049
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4049/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4049
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4049/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4049
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4049/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4049
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4049/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4049
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4049/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4049
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4049/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

lgtm

review: Approve
Revision history for this message
Chris Halse Rogers (raof) wrote :

This breaks nested servers' ability to set the subpixel arrangement. LocalOutputConfig is where we store the output details that aren't appropriate for clients to set, and so don't have setters in the client API.

review: Needs Fixing
Revision history for this message
Chris Halse Rogers (raof) wrote :

To be more constructive, what we probably need to do here is initialise the local_config.subpixel_arrangement from mir_output_get_subpixel_arrangement() rather than unknown.

We largely don't have to worry about what happens if the host server changes subpixel arrangement, because that's a read-only property. I *think* the shell has enough control to do whatever it wants in this case, though.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

That problem would be solved by landing: lp:~vanvugt/mir/set-subpixel
but I somewhat agree...

More concerning than subpixel arrangement is the "scale" field though. Getting and setting "scale" does not talk to the hardware, so "scale" probably should not exist in the display config.

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:4050
https://mir-jenkins.ubuntu.com/job/mir-ci/3135/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4208
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4295
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4285
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4285
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4285
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4235
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4235/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4235
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4235/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4235
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4235/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4235
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4235/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4235
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4235/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4235
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4235/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

Yup, seems sensible.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/server/graphics/nested/nested_display_configuration.cpp'
2--- src/server/graphics/nested/nested_display_configuration.cpp 2017-01-18 13:59:18 +0000
3+++ src/server/graphics/nested/nested_display_configuration.cpp 2017-03-13 09:14:01 +0000
4@@ -103,7 +103,7 @@
5 auto current_format = mir_output_get_current_pixel_format(output);
6 auto power_mode = mir_output_get_power_mode(output);
7 auto orientation = mir_output_get_orientation(output);
8- auto local_config = get_local_config_for(output_id);
9+ auto local_config = get_local_config_for(output);
10 uint32_t preferred_index = mir_output_get_preferred_mode_index(output);
11 uint32_t current_index = mir_output_get_current_mode_index(output);
12
13@@ -198,13 +198,16 @@
14 }
15
16 mgn::NestedDisplayConfiguration::LocalOutputConfig
17-mgn::NestedDisplayConfiguration::get_local_config_for(uint32_t output_id) const
18+mgn::NestedDisplayConfiguration::get_local_config_for(MirOutput const* output) const
19 {
20 std::lock_guard<std::mutex> lock{local_config_mutex};
21
22- LocalOutputConfig const default_values {1.0f, mir_form_factor_monitor,
23- mir_subpixel_arrangement_unknown,
24- {}, mir_output_gamma_unsupported};
25+ auto const output_id = mir_output_get_id(output);
26+
27+ LocalOutputConfig const default_values{
28+ 1.0f, mir_form_factor_monitor,
29+ mir_output_get_subpixel_arrangement(output),
30+ {}, mir_output_gamma_unsupported};
31
32 bool inserted;
33 decltype(local_config)::iterator keypair;
34
35=== modified file 'src/server/graphics/nested/nested_display_configuration.h'
36--- src/server/graphics/nested/nested_display_configuration.h 2017-01-18 02:29:37 +0000
37+++ src/server/graphics/nested/nested_display_configuration.h 2017-03-13 09:14:01 +0000
38@@ -67,7 +67,7 @@
39 };
40 std::unordered_map<uint32_t, LocalOutputConfig> mutable local_config;
41
42- LocalOutputConfig get_local_config_for(uint32_t output_id) const;
43+ LocalOutputConfig get_local_config_for(MirOutput const* output) const;
44 void set_local_config_for(uint32_t output_id, LocalOutputConfig const& config);
45 };
46
47
48=== modified file 'tests/unit-tests/platforms/nested/mir_display_configuration_builder.cpp'
49--- tests/unit-tests/platforms/nested/mir_display_configuration_builder.cpp 2017-01-18 13:59:18 +0000
50+++ tests/unit-tests/platforms/nested/mir_display_configuration_builder.cpp 2017-03-13 09:14:01 +0000
51@@ -81,6 +81,7 @@
52 output->set_power_mode(mir_power_mode_on);
53 output->set_orientation(mir_orientation_normal);
54 output->set_edid(valid_edid, sizeof(valid_edid));
55+ output->set_subpixel_arrangement(mir_subpixel_arrangement_horizontal_rgb);
56
57 return std::make_shared<MirDisplayConfig>(conf);
58 }
59@@ -121,6 +122,7 @@
60 output->set_power_mode(mir_power_mode_on);
61 output->set_orientation(mir_orientation_normal);
62 output->set_edid(valid_edid, sizeof(valid_edid));
63+ output->set_subpixel_arrangement(mir_subpixel_arrangement_horizontal_rgb);
64 }
65
66 return std::make_shared<MirDisplayConfig>(conf);
67
68=== modified file 'tests/unit-tests/platforms/nested/test_nested_display_configuration.cpp'
69--- tests/unit-tests/platforms/nested/test_nested_display_configuration.cpp 2017-01-18 13:59:18 +0000
70+++ tests/unit-tests/platforms/nested/test_nested_display_configuration.cpp 2017-03-13 09:14:01 +0000
71@@ -169,6 +169,22 @@
72 EXPECT_NE(0, matches);
73 }
74
75+TEST(NestedDisplayConfiguration, includes_host_subpixel_arrangement)
76+{
77+ auto host_conf = mt::build_trivial_configuration();
78+ auto const output = mir_display_config_get_output(host_conf.get(), 0);
79+ auto host_subpixel = mir_output_get_subpixel_arrangement(output);
80+
81+ mgn::NestedDisplayConfiguration nested_conf(host_conf);
82+ int matches = 0;
83+ nested_conf.for_each_output([&](mg::DisplayConfigurationOutput const& output)
84+ {
85+ ASSERT_EQ(host_subpixel, output.subpixel_arrangement);
86+ ++matches;
87+ });
88+ EXPECT_NE(0, matches);
89+}
90+
91 TEST(NestedDisplayConfiguration, clone_matches_original_configuration)
92 {
93 mgn::NestedDisplayConfiguration config(mt::build_non_trivial_configuration());

Subscribers

People subscribed via source and target branches