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
=== modified file 'include/server/mir/graphics/display_configuration_observer.h'
--- include/server/mir/graphics/display_configuration_observer.h 2016-11-02 05:07:18 +0000
+++ include/server/mir/graphics/display_configuration_observer.h 2016-11-25 12:01:34 +0000
@@ -51,6 +51,13 @@
51 virtual void configuration_applied(std::shared_ptr<DisplayConfiguration const> const& config) = 0;51 virtual void configuration_applied(std::shared_ptr<DisplayConfiguration const> const& config) = 0;
5252
53 /**53 /**
54 * Notification after updating base display configuration.
55 *
56 * \param [in] config The configuration that has just been updated.
57 */
58 virtual void base_configuration_updated(std::shared_ptr<DisplayConfiguration const> const& base_config) = 0;
59
60 /**
54 * Notification after every failed display configuration attempt.61 * Notification after every failed display configuration attempt.
55 *62 *
56 * This is called if the graphics platform throws an exception when trying63 * This is called if the graphics platform throws an exception when trying
5764
=== modified file 'src/server/graphics/display_configuration_observer_multiplexer.cpp'
--- src/server/graphics/display_configuration_observer_multiplexer.cpp 2016-11-02 05:07:18 +0000
+++ src/server/graphics/display_configuration_observer_multiplexer.cpp 2016-11-25 12:01:34 +0000
@@ -32,6 +32,12 @@
32 for_each_observer(&mg::DisplayConfigurationObserver::configuration_applied, config);32 for_each_observer(&mg::DisplayConfigurationObserver::configuration_applied, config);
33}33}
3434
35void mg::DisplayConfigurationObserverMultiplexer::base_configuration_updated(
36 std::shared_ptr<DisplayConfiguration const> const& base_config)
37{
38 for_each_observer(&mg::DisplayConfigurationObserver::base_configuration_updated, base_config);
39}
40
35void mg::DisplayConfigurationObserverMultiplexer::configuration_failed(41void mg::DisplayConfigurationObserverMultiplexer::configuration_failed(
36 std::shared_ptr<mg::DisplayConfiguration const> const& attempted,42 std::shared_ptr<mg::DisplayConfiguration const> const& attempted,
37 std::exception const& error)43 std::exception const& error)
3844
=== modified file 'src/server/graphics/display_configuration_observer_multiplexer.h'
--- src/server/graphics/display_configuration_observer_multiplexer.h 2016-11-02 05:07:18 +0000
+++ src/server/graphics/display_configuration_observer_multiplexer.h 2016-11-25 12:01:34 +0000
@@ -36,6 +36,8 @@
3636
37 void configuration_applied(std::shared_ptr<DisplayConfiguration const> const& config) override;37 void configuration_applied(std::shared_ptr<DisplayConfiguration const> const& config) override;
3838
39 void base_configuration_updated(std::shared_ptr<DisplayConfiguration const> const& base_config) override;
40
39 void configuration_failed(41 void configuration_failed(
40 std::shared_ptr<DisplayConfiguration const> const& attempted,42 std::shared_ptr<DisplayConfiguration const> const& attempted,
41 std::exception const& error) override;43 std::exception const& error) override;
4244
=== modified file 'src/server/report/logging/display_configuration_report.cpp'
--- src/server/report/logging/display_configuration_report.cpp 2016-11-25 05:38:23 +0000
+++ src/server/report/logging/display_configuration_report.cpp 2016-11-25 12:01:34 +0000
@@ -28,7 +28,7 @@
28namespace ml = mir::logging;28namespace ml = mir::logging;
29namespace mrl= mir::report::logging;29namespace mrl= mir::report::logging;
3030
31namespace 31namespace
32{32{
33auto const component = MIR_LOG_COMPONENT_FALLBACK;33auto const component = MIR_LOG_COMPONENT_FALLBACK;
34auto const severity = ml::Severity::informational;34auto const severity = ml::Severity::informational;
@@ -57,6 +57,13 @@
57 log_configuration(severity, *config);57 log_configuration(severity, *config);
58}58}
5959
60void mrl::DisplayConfigurationReport::base_configuration_updated(
61 std::shared_ptr<mg::DisplayConfiguration const> const& base_config)
62{
63 logger->log(component, severity, "New base display configuration:");
64 log_configuration(severity, *base_config);
65}
66
60void mrl::DisplayConfigurationReport::configuration_failed(67void mrl::DisplayConfigurationReport::configuration_failed(
61 std::shared_ptr<mg::DisplayConfiguration const> const& attempted,68 std::shared_ptr<mg::DisplayConfiguration const> const& attempted,
62 std::exception const& error)69 std::exception const& error)
@@ -91,7 +98,7 @@
91 float inches =98 float inches =
92 sqrtf(width_mm * width_mm + height_mm * height_mm) / 25.4;99 sqrtf(width_mm * width_mm + height_mm * height_mm) / 25.4;
93100
94 logger->log(component, severity, 101 logger->log(component, severity,
95 "%sPhysical size %.1f\" %dx%dmm",102 "%sPhysical size %.1f\" %dx%dmm",
96 indent, inches, width_mm, height_mm);103 indent, inches, width_mm, height_mm);
97104
@@ -112,7 +119,7 @@
112 mode.size.height.as_int(),119 mode.size.height.as_int(),
113 mode.vrefresh_hz);120 mode.vrefresh_hz);
114 }121 }
115 122
116 if (out.preferred_mode_index < out.modes.size())123 if (out.preferred_mode_index < out.modes.size())
117 {124 {
118 auto const& mode = out.modes[out.preferred_mode_index];125 auto const& mode = out.modes[out.preferred_mode_index];
@@ -123,7 +130,7 @@
123 mode.size.height.as_int(),130 mode.size.height.as_int(),
124 mode.vrefresh_hz);131 mode.vrefresh_hz);
125 }132 }
126 133
127 static const char* const orientation[] =134 static const char* const orientation[] =
128 {"normal", "left", "inverted", "right"};135 {"normal", "left", "inverted", "right"};
129 int degrees_ccw = out.orientation;136 int degrees_ccw = out.orientation;
130137
=== modified file 'src/server/report/logging/display_configuration_report.h'
--- src/server/report/logging/display_configuration_report.h 2016-11-02 05:07:18 +0000
+++ src/server/report/logging/display_configuration_report.h 2016-11-25 12:01:34 +0000
@@ -40,6 +40,8 @@
40 void configuration_applied(40 void configuration_applied(
41 std::shared_ptr<graphics::DisplayConfiguration const> const& config) override;41 std::shared_ptr<graphics::DisplayConfiguration const> const& config) override;
4242
43 void base_configuration_updated(std::shared_ptr<graphics::DisplayConfiguration const> const& base_config) override;
44
43 void configuration_failed(45 void configuration_failed(
44 std::shared_ptr<graphics::DisplayConfiguration const> const& attempted,46 std::shared_ptr<graphics::DisplayConfiguration const> const& attempted,
45 std::exception const& error) override;47 std::exception const& error) override;
4648
=== modified file 'src/server/scene/mediating_display_changer.cpp'
--- src/server/scene/mediating_display_changer.cpp 2016-11-15 23:48:01 +0000
+++ src/server/scene/mediating_display_changer.cpp 2016-11-25 12:01:34 +0000
@@ -349,6 +349,7 @@
349 try349 try
350 {350 {
351 apply_base_config();351 apply_base_config();
352 observer->base_configuration_updated(base_configuration_);
352 }353 }
353 catch (std::exception const&)354 catch (std::exception const&)
354 {355 {
@@ -529,6 +530,7 @@
529 if (base_configuration_applied)530 if (base_configuration_applied)
530 apply_base_config();531 apply_base_config();
531532
533 observer->base_configuration_updated(conf);
532 send_config_to_all_sessions(conf);534 send_config_to_all_sessions(conf);
533 });535 });
534}536}
535537
=== modified file 'tests/acceptance-tests/test_nested_mir.cpp'
--- tests/acceptance-tests/test_nested_mir.cpp 2016-11-24 10:50:40 +0000
+++ tests/acceptance-tests/test_nested_mir.cpp 2016-11-25 12:01:34 +0000
@@ -127,6 +127,7 @@
127{127{
128 MOCK_METHOD1(initial_configuration, void (std::shared_ptr<mg::DisplayConfiguration const> const& configuration));128 MOCK_METHOD1(initial_configuration, void (std::shared_ptr<mg::DisplayConfiguration const> const& configuration));
129 MOCK_METHOD1(configuration_applied, void (std::shared_ptr<mg::DisplayConfiguration const> const& configuration));129 MOCK_METHOD1(configuration_applied, void (std::shared_ptr<mg::DisplayConfiguration const> const& configuration));
130 MOCK_METHOD1(base_configuration_updated, void (std::shared_ptr<mg::DisplayConfiguration const> const& base_config));
130 MOCK_METHOD2(configuration_failed, void(std::shared_ptr<mg::DisplayConfiguration const> const&, std::exception const&));131 MOCK_METHOD2(configuration_failed, void(std::shared_ptr<mg::DisplayConfiguration const> const&, std::exception const&));
131 MOCK_METHOD2(catastrophic_configuration_error, void(std::shared_ptr<mg::DisplayConfiguration const> const&, std::exception const&));132 MOCK_METHOD2(catastrophic_configuration_error, void(std::shared_ptr<mg::DisplayConfiguration const> const&, std::exception const&));
132};133};
133134
=== modified file 'tests/unit-tests/scene/test_mediating_display_changer.cpp'
--- tests/unit-tests/scene/test_mediating_display_changer.cpp 2016-11-15 23:48:01 +0000
+++ tests/unit-tests/scene/test_mediating_display_changer.cpp 2016-11-25 12:01:34 +0000
@@ -172,6 +172,7 @@
172{172{
173 void initial_configuration(std::shared_ptr<mg::DisplayConfiguration const> const&) override {}173 void initial_configuration(std::shared_ptr<mg::DisplayConfiguration const> const&) override {}
174 void configuration_applied(std::shared_ptr<mg::DisplayConfiguration const> const&) override {}174 void configuration_applied(std::shared_ptr<mg::DisplayConfiguration const> const&) override {}
175 void base_configuration_updated(std::shared_ptr<mg::DisplayConfiguration const> const&) override {}
175 void configuration_failed(176 void configuration_failed(
176 std::shared_ptr<mg::DisplayConfiguration const> const&,177 std::shared_ptr<mg::DisplayConfiguration const> const&,
177 std::exception const&) override {}178 std::exception const&) override {}
@@ -913,6 +914,33 @@
913 changer->set_base_configuration(mt::fake_shared(conf));914 changer->set_base_configuration(mt::fake_shared(conf));
914}915}
915916
917TEST_F(MediatingDisplayChangerTest, notifies_observer_on_set_base_configuration)
918{
919 using namespace testing;
920
921 struct MockDisplayConfigurationObserver : StubDisplayConfigurationObserver
922 {
923 MOCK_METHOD1(base_configuration_updated, void (std::shared_ptr<mg::DisplayConfiguration const> const& base_config));
924 } display_configuration_observer;
925
926 changer = std::make_shared<ms::MediatingDisplayChanger>(
927 mt::fake_shared(mock_display),
928 mt::fake_shared(mock_compositor),
929 mt::fake_shared(mock_conf_policy),
930 mt::fake_shared(stub_session_container),
931 mt::fake_shared(session_event_sink),
932 mt::fake_shared(server_action_queue),
933 mt::fake_shared(display_configuration_observer),
934 mt::fake_shared(mock_input_region),
935 mt::fake_shared(alarm_factory));
936
937 mtd::NullDisplayConfiguration conf;
938
939 EXPECT_CALL(display_configuration_observer, base_configuration_updated(_));
940
941 changer->set_base_configuration(mt::fake_shared(conf));
942}
943
916TEST_F(MediatingDisplayChangerTest, input_region_receives_display_configuration_on_start)944TEST_F(MediatingDisplayChangerTest, input_region_receives_display_configuration_on_start)
917{945{
918 using namespace testing;946 using namespace testing;

Subscribers

People subscribed via source and target branches