Mir

Merge lp:~alan-griffiths/mir/add-DisplayConfigurationObserver-base_configuration_updated into lp:mir

  1. add-DisplayConfigurationObserver-base_configuration_updated
  2. Merge into development-branch
Proposed by Alan Griffiths
Status: Merged
Approved by: Alan Griffiths
Approved revision: no longer in the source branch.
Merged at revision: 3848
Proposed branch: lp:~alan-griffiths/mir/add-DisplayConfigurationObserver-base_configuration_updated
Merge into: lp:mir
Diff against target: 192 lines (+59/-4)
8 files modified
include/server/mir/graphics/display_configuration_observer.h (+7/-0)
src/server/graphics/display_configuration_observer_multiplexer.cpp (+6/-0)
src/server/graphics/display_configuration_observer_multiplexer.h (+2/-0)
src/server/report/logging/display_configuration_report.cpp (+11/-4)
src/server/report/logging/display_configuration_report.h (+2/-0)
src/server/scene/mediating_display_changer.cpp (+2/-0)
tests/acceptance-tests/test_nested_mir.cpp (+1/-0)
tests/unit-tests/scene/test_mediating_display_changer.cpp (+28/-0)
To merge this branch: bzr merge lp:~alan-griffiths/mir/add-DisplayConfigurationObserver-base_configuration_updated
Reviewer Review Type Date Requested Status
Andreas Pokorny (community) Approve
Kevin DuBois (community) Approve
Mir CI Bot continuous-integration Approve
Review via email: mp+311811@code.launchpad.net

Commit message

Add a base_configuration_updated() notification to DisplayConfigurationObserver

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

PASSED: Continuous integration, rev:3847
https://mir-jenkins.ubuntu.com/job/mir-ci/2274/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2946
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3011
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3003
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3003
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3003
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2975
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2975/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/2975
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2975/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2975
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2975/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/2975
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2975/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/2975
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2975/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/2975
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2975/artifact/output/*zip*/output.zip

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

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

okay by me, is handing out the std::shared_ptr<DisplayConfiguration> useful (as opposed to DisplayConfiguration const&)

review: Approve
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> okay by me, is handing out the std::shared_ptr<DisplayConfiguration> useful
> (as opposed to DisplayConfiguration const&)

In short: yes.

While you and I were no looking someone decided to action the notifications on another second thread. Because of that the lifetime of the config needs to be extended until that notification runs.

Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

sure

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/server/mir/graphics/display_configuration_observer.h'
2--- include/server/mir/graphics/display_configuration_observer.h 2016-11-02 05:07:18 +0000
3+++ include/server/mir/graphics/display_configuration_observer.h 2016-11-25 12:01:34 +0000
4@@ -51,6 +51,13 @@
5 virtual void configuration_applied(std::shared_ptr<DisplayConfiguration const> const& config) = 0;
6
7 /**
8+ * Notification after updating base display configuration.
9+ *
10+ * \param [in] config The configuration that has just been updated.
11+ */
12+ virtual void base_configuration_updated(std::shared_ptr<DisplayConfiguration const> const& base_config) = 0;
13+
14+ /**
15 * Notification after every failed display configuration attempt.
16 *
17 * This is called if the graphics platform throws an exception when trying
18
19=== modified file 'src/server/graphics/display_configuration_observer_multiplexer.cpp'
20--- src/server/graphics/display_configuration_observer_multiplexer.cpp 2016-11-02 05:07:18 +0000
21+++ src/server/graphics/display_configuration_observer_multiplexer.cpp 2016-11-25 12:01:34 +0000
22@@ -32,6 +32,12 @@
23 for_each_observer(&mg::DisplayConfigurationObserver::configuration_applied, config);
24 }
25
26+void mg::DisplayConfigurationObserverMultiplexer::base_configuration_updated(
27+ std::shared_ptr<DisplayConfiguration const> const& base_config)
28+{
29+ for_each_observer(&mg::DisplayConfigurationObserver::base_configuration_updated, base_config);
30+}
31+
32 void mg::DisplayConfigurationObserverMultiplexer::configuration_failed(
33 std::shared_ptr<mg::DisplayConfiguration const> const& attempted,
34 std::exception const& error)
35
36=== modified file 'src/server/graphics/display_configuration_observer_multiplexer.h'
37--- src/server/graphics/display_configuration_observer_multiplexer.h 2016-11-02 05:07:18 +0000
38+++ src/server/graphics/display_configuration_observer_multiplexer.h 2016-11-25 12:01:34 +0000
39@@ -36,6 +36,8 @@
40
41 void configuration_applied(std::shared_ptr<DisplayConfiguration const> const& config) override;
42
43+ void base_configuration_updated(std::shared_ptr<DisplayConfiguration const> const& base_config) override;
44+
45 void configuration_failed(
46 std::shared_ptr<DisplayConfiguration const> const& attempted,
47 std::exception const& error) override;
48
49=== modified file 'src/server/report/logging/display_configuration_report.cpp'
50--- src/server/report/logging/display_configuration_report.cpp 2016-11-25 05:38:23 +0000
51+++ src/server/report/logging/display_configuration_report.cpp 2016-11-25 12:01:34 +0000
52@@ -28,7 +28,7 @@
53 namespace ml = mir::logging;
54 namespace mrl= mir::report::logging;
55
56-namespace
57+namespace
58 {
59 auto const component = MIR_LOG_COMPONENT_FALLBACK;
60 auto const severity = ml::Severity::informational;
61@@ -57,6 +57,13 @@
62 log_configuration(severity, *config);
63 }
64
65+void mrl::DisplayConfigurationReport::base_configuration_updated(
66+ std::shared_ptr<mg::DisplayConfiguration const> const& base_config)
67+{
68+ logger->log(component, severity, "New base display configuration:");
69+ log_configuration(severity, *base_config);
70+}
71+
72 void mrl::DisplayConfigurationReport::configuration_failed(
73 std::shared_ptr<mg::DisplayConfiguration const> const& attempted,
74 std::exception const& error)
75@@ -91,7 +98,7 @@
76 float inches =
77 sqrtf(width_mm * width_mm + height_mm * height_mm) / 25.4;
78
79- logger->log(component, severity,
80+ logger->log(component, severity,
81 "%sPhysical size %.1f\" %dx%dmm",
82 indent, inches, width_mm, height_mm);
83
84@@ -112,7 +119,7 @@
85 mode.size.height.as_int(),
86 mode.vrefresh_hz);
87 }
88-
89+
90 if (out.preferred_mode_index < out.modes.size())
91 {
92 auto const& mode = out.modes[out.preferred_mode_index];
93@@ -123,7 +130,7 @@
94 mode.size.height.as_int(),
95 mode.vrefresh_hz);
96 }
97-
98+
99 static const char* const orientation[] =
100 {"normal", "left", "inverted", "right"};
101 int degrees_ccw = out.orientation;
102
103=== modified file 'src/server/report/logging/display_configuration_report.h'
104--- src/server/report/logging/display_configuration_report.h 2016-11-02 05:07:18 +0000
105+++ src/server/report/logging/display_configuration_report.h 2016-11-25 12:01:34 +0000
106@@ -40,6 +40,8 @@
107 void configuration_applied(
108 std::shared_ptr<graphics::DisplayConfiguration const> const& config) override;
109
110+ void base_configuration_updated(std::shared_ptr<graphics::DisplayConfiguration const> const& base_config) override;
111+
112 void configuration_failed(
113 std::shared_ptr<graphics::DisplayConfiguration const> const& attempted,
114 std::exception const& error) override;
115
116=== modified file 'src/server/scene/mediating_display_changer.cpp'
117--- src/server/scene/mediating_display_changer.cpp 2016-11-15 23:48:01 +0000
118+++ src/server/scene/mediating_display_changer.cpp 2016-11-25 12:01:34 +0000
119@@ -349,6 +349,7 @@
120 try
121 {
122 apply_base_config();
123+ observer->base_configuration_updated(base_configuration_);
124 }
125 catch (std::exception const&)
126 {
127@@ -529,6 +530,7 @@
128 if (base_configuration_applied)
129 apply_base_config();
130
131+ observer->base_configuration_updated(conf);
132 send_config_to_all_sessions(conf);
133 });
134 }
135
136=== modified file 'tests/acceptance-tests/test_nested_mir.cpp'
137--- tests/acceptance-tests/test_nested_mir.cpp 2016-11-24 10:50:40 +0000
138+++ tests/acceptance-tests/test_nested_mir.cpp 2016-11-25 12:01:34 +0000
139@@ -127,6 +127,7 @@
140 {
141 MOCK_METHOD1(initial_configuration, void (std::shared_ptr<mg::DisplayConfiguration const> const& configuration));
142 MOCK_METHOD1(configuration_applied, void (std::shared_ptr<mg::DisplayConfiguration const> const& configuration));
143+ MOCK_METHOD1(base_configuration_updated, void (std::shared_ptr<mg::DisplayConfiguration const> const& base_config));
144 MOCK_METHOD2(configuration_failed, void(std::shared_ptr<mg::DisplayConfiguration const> const&, std::exception const&));
145 MOCK_METHOD2(catastrophic_configuration_error, void(std::shared_ptr<mg::DisplayConfiguration const> const&, std::exception const&));
146 };
147
148=== modified file 'tests/unit-tests/scene/test_mediating_display_changer.cpp'
149--- tests/unit-tests/scene/test_mediating_display_changer.cpp 2016-11-15 23:48:01 +0000
150+++ tests/unit-tests/scene/test_mediating_display_changer.cpp 2016-11-25 12:01:34 +0000
151@@ -172,6 +172,7 @@
152 {
153 void initial_configuration(std::shared_ptr<mg::DisplayConfiguration const> const&) override {}
154 void configuration_applied(std::shared_ptr<mg::DisplayConfiguration const> const&) override {}
155+ void base_configuration_updated(std::shared_ptr<mg::DisplayConfiguration const> const&) override {}
156 void configuration_failed(
157 std::shared_ptr<mg::DisplayConfiguration const> const&,
158 std::exception const&) override {}
159@@ -913,6 +914,33 @@
160 changer->set_base_configuration(mt::fake_shared(conf));
161 }
162
163+TEST_F(MediatingDisplayChangerTest, notifies_observer_on_set_base_configuration)
164+{
165+ using namespace testing;
166+
167+ struct MockDisplayConfigurationObserver : StubDisplayConfigurationObserver
168+ {
169+ MOCK_METHOD1(base_configuration_updated, void (std::shared_ptr<mg::DisplayConfiguration const> const& base_config));
170+ } display_configuration_observer;
171+
172+ changer = std::make_shared<ms::MediatingDisplayChanger>(
173+ mt::fake_shared(mock_display),
174+ mt::fake_shared(mock_compositor),
175+ mt::fake_shared(mock_conf_policy),
176+ mt::fake_shared(stub_session_container),
177+ mt::fake_shared(session_event_sink),
178+ mt::fake_shared(server_action_queue),
179+ mt::fake_shared(display_configuration_observer),
180+ mt::fake_shared(mock_input_region),
181+ mt::fake_shared(alarm_factory));
182+
183+ mtd::NullDisplayConfiguration conf;
184+
185+ EXPECT_CALL(display_configuration_observer, base_configuration_updated(_));
186+
187+ changer->set_base_configuration(mt::fake_shared(conf));
188+}
189+
190 TEST_F(MediatingDisplayChangerTest, input_region_receives_display_configuration_on_start)
191 {
192 using namespace testing;

Subscribers

People subscribed via source and target branches