Merge into development-branch : dont-tear-down-compositor-if-unnecessary : Code : Mir

Reviewer	Review Type	Date Requested	Status
Mir CI Bot	continuous-integration	2016-09-15	Approve on 2016-09-21
Cemil Azizoglu (community)			Approve on 2016-09-20
Alan Griffiths		2016-09-15	Approve on 2016-09-15
Review via email: mp+305778@code.launchpad.net

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-09-09: Posted in a previous version of this proposal

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2016-09-12: Posted in a previous version of this proposal

#

08:42:03 clang: error: invalid argument '/build/mir' to -fdebug-prefix-map

Again. ???

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-09-12: Posted in a previous version of this proposal

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-09-13: Posted in a previous version of this proposal

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2016-09-13: Posted in a previous version of this proposal

#

02:05:53 cc1plus: error: invalid argument ��/build/mir�� to -fdebug-prefix-map
Retriggering

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-09-13: Posted in a previous version of this proposal

#

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

PASSED: Continuous integration, rev:3704
https://mir-jenkins.ubuntu.com/job/mir-ci/1699/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2127
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2189
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2180
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2180
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2180
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2155
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2155/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/2155
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2155/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2155
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2155/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/2155
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2155/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/2155
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2155/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-09-14: Posted in a previous version of this proposal

#

+ virtual bool configuration_will_preserve_display_buffers(DisplayConfiguration const& conf) const = 0;

While not an issue in the current usage I worry that this style invites racy code. I would prefer something that's obviously atomic:

bool apply_configuration_if_it_preserves_display_buffers(DisplayConfiguration const& conf) const = 0;

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-09-14: Posted in a previous version of this proposal

#

> + virtual bool
> configuration_will_preserve_display_buffers(DisplayConfiguration const& conf)
> const = 0;
>
> While not an issue in the current usage I worry that this style invites racy
> code. I would prefer something that's obviously atomic:
>
> bool apply_configuration_if_it_preserves_display_buffers(DisplayConfiguration
> const& conf) const = 0;

I'm not going to block on this, but please consider it before TA.

review: Approve

Revision history for this message

Chris Halse Rogers (raof) wrote on 2016-09-14: Posted in a previous version of this proposal

#

Hm.

It's pretty easy to use correctly now - the results of will_preserve_display_buffers() are valid until the next configure() call, we serialize all of those, and they're easy to notice.

But apply_configuration_if_it_preserves_display_buffers() would indeed be even easier to use correctly, and might even be easier to implement in the platforms. Thanks for the review!

Revision history for this message

Chris Halse Rogers (raof) wrote on 2016-09-15:

#

Now with atomic test-and-set semantics.

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-09-15:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-09-15:

#

^^^
Just the usual bug 1523621

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-09-15:

#

Looks good

review: Approve

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-09-20:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Cemil Azizoglu (cemil-azizoglu) wrote on 2016-09-20:

#

lgtmt

review: Approve

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-09-21:

#

^^^
Jenkins failures are just bug 1523621 as usual.

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-09-21:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-09-21:

#

^^^
Bug 1523621 again

Revision history for this message

Mir CI Bot (mir-ci-bot) on 2016-09-21:

#

review: Approve (continuous-integration)

Mir

Merge lp:~raof/mir/dont-tear-down-compositor-if-unnecessary into lp:mir

Commit message

Description of the change

Preview Diff

Subscribers

 === modified file 'include/platform/mir/graphics/display.h'
 --- include/platform/mir/graphics/display.h	2016-09-08 15:28:25 +0000
 +++ include/platform/mir/graphics/display.h	2016-09-15 01:44:40 +0000
@@ -110,6 +110,24 @@
      virtual std::unique_ptr<DisplayConfiguration> configuration() const = 0;
      /**
++     * Applying a display configuration only if it will not invalidate existing DisplayBuffers
++     *
++     * The Display must guarantee that the references to the DisplayBuffer acquired via
++     * DisplaySyncGroup::for_each_display_buffer() remain valid until the Display is destroyed or
++     * Display::configure() is called.
++     *
++     * If this function returns \c true then the new display configuration has been applied.
++     * If this function returns \c false then the new display configuration has not been applied.
++     *
++     * In either case this function guarantees that existing DisplayBuffer references will remain
++     * valid.
++     *
++     * \param conf [in] Configuration to possibly apply.
++     * \return      \c true if \p conf has been applied as the new output configuration.
++     */
++    virtual bool apply_if_configuration_preserves_display_buffers(DisplayConfiguration const& conf) const = 0;
++
++    /**
       * Sets a new output configuration.
       */
      virtual void configure(DisplayConfiguration const& conf) = 0;
 === modified file 'include/test/mir/test/doubles/null_display.h'
 --- include/test/mir/test/doubles/null_display.h	2016-08-01 09:37:45 +0000
 +++ include/test/mir/test/doubles/null_display.h	2016-09-15 01:44:40 +0000
@@ -48,6 +48,10 @@
              new NullDisplayConfiguration
          );
+     }
++    bool apply_if_configuration_preserves_display_buffers(graphics::DisplayConfiguration const&) const override
++    {
++        return false;
++    }
      void configure(graphics::DisplayConfiguration const&)  override{}
      void register_configuration_change_handler(
          graphics::EventHandlerRegister&,
 === modified file 'src/platforms/android/server/display.cpp'
 --- src/platforms/android/server/display.cpp	2016-08-01 09:37:45 +0000
 +++ src/platforms/android/server/display.cpp	2016-09-15 01:44:40 +0000
@@ -358,3 +358,9 @@
+ {
      return std::make_unique<mga::PbufferGLContext>(gl_context);
+ }
++
++bool mga::Display::apply_if_configuration_preserves_display_buffers(
++    mg::DisplayConfiguration const& /*conf*/) const
++{
++    return false;
++}
 === modified file 'src/platforms/android/server/display.h'
 --- src/platforms/android/server/display.h	2016-08-01 09:37:45 +0000
 +++ src/platforms/android/server/display.h	2016-09-15 01:44:40 +0000
@@ -66,6 +66,7 @@
      void for_each_display_sync_group(std::function<void(graphics::DisplaySyncGroup&)> const& f) override;
      std::unique_ptr<graphics::DisplayConfiguration> configuration() const override;
++    bool apply_if_configuration_preserves_display_buffers(graphics::DisplayConfiguration const& conf) const override;
      void configure(graphics::DisplayConfiguration const&) override;
      void register_configuration_change_handler(
 === modified file 'src/platforms/eglstream-kms/server/display.cpp'
 --- src/platforms/eglstream-kms/server/display.cpp	2016-08-01 09:37:45 +0000
 +++ src/platforms/eglstream-kms/server/display.cpp	2016-09-15 01:44:40 +0000
@@ -367,3 +367,9 @@
      return std::make_unique<GLContext>(display, context);
+ }
++bool mge::Display::apply_if_configuration_preserves_display_buffers(
++    mg::DisplayConfiguration const& /*conf*/) const
++{
++    return false;
++}
++
 === modified file 'src/platforms/eglstream-kms/server/display.h'
 --- src/platforms/eglstream-kms/server/display.h	2016-08-01 09:37:45 +0000
 +++ src/platforms/eglstream-kms/server/display.h	2016-09-15 01:44:40 +0000
@@ -49,6 +49,8 @@
      std::unique_ptr<DisplayConfiguration> configuration() const override;
++    bool apply_if_configuration_preserves_display_buffers(DisplayConfiguration const& conf) const override;
++
      void configure(DisplayConfiguration const& conf) override;
      void register_configuration_change_handler(EventHandlerRegister& handlers,
 === modified file 'src/platforms/mesa/server/kms/display.cpp'
 --- src/platforms/mesa/server/kms/display.cpp	2016-08-23 20:21:27 +0000
 +++ src/platforms/mesa/server/kms/display.cpp	2016-09-15 01:44:40 +0000
@@ -415,3 +415,9 @@
+ {
      return std::make_unique<GBMGLContext>(*gbm, *gl_config, shared_egl.context());
+ }
++
++bool mgm::Display::apply_if_configuration_preserves_display_buffers(
++    mg::DisplayConfiguration const& /*conf*/) const
++{
++    return false;
++}
 === modified file 'src/platforms/mesa/server/kms/display.h'
 --- src/platforms/mesa/server/kms/display.h	2016-08-01 09:37:45 +0000
 +++ src/platforms/mesa/server/kms/display.h	2016-09-15 01:44:40 +0000
@@ -78,6 +78,7 @@
          std::function<void(graphics::DisplaySyncGroup&)> const& f) override;
      std::unique_ptr<DisplayConfiguration> configuration() const override;
++    bool apply_if_configuration_preserves_display_buffers(DisplayConfiguration const& conf) const override;
      void configure(DisplayConfiguration const& conf) override;
      void register_configuration_change_handler(
 === modified file 'src/platforms/mesa/server/x11/graphics/display.cpp'
 --- src/platforms/mesa/server/x11/graphics/display.cpp	2016-09-07 04:54:17 +0000
 +++ src/platforms/mesa/server/x11/graphics/display.cpp	2016-09-15 01:44:40 +0000
@@ -380,3 +380,9 @@
+ {
      return std::make_unique<mgx::XGLContext>(egl_display, egl_surface, egl_context);
+ }
++
++bool mgx::Display::apply_if_configuration_preserves_display_buffers(
++    mg::DisplayConfiguration const& /*conf*/) const
++{
++    return false;
++}
 === modified file 'src/platforms/mesa/server/x11/graphics/display.h'
 --- src/platforms/mesa/server/x11/graphics/display.h	2016-09-07 03:37:03 +0000
 +++ src/platforms/mesa/server/x11/graphics/display.h	2016-09-15 01:44:40 +0000
@@ -111,6 +111,9 @@
      void for_each_display_sync_group(std::function<void(graphics::DisplaySyncGroup&)> const& f) override;
      std::unique_ptr<graphics::DisplayConfiguration> configuration() const override;
++
++    bool apply_if_configuration_preserves_display_buffers(graphics::DisplayConfiguration const& conf) const override;
++
      void configure(graphics::DisplayConfiguration const&) override;
      void register_configuration_change_handler(
 === modified file 'src/server/graphics/nested/display.cpp'
 --- src/server/graphics/nested/display.cpp	2016-08-25 12:50:09 +0000
 +++ src/server/graphics/nested/display.cpp	2016-09-15 01:44:40 +0000
@@ -366,3 +366,9 @@
+ {
      return egl_display.create_gl_context();
+ }
++
++bool mgn::Display::apply_if_configuration_preserves_display_buffers(
++    mg::DisplayConfiguration const& /*conf*/) const
++{
++    return false;
++}
 === modified file 'src/server/graphics/nested/display.h'
 --- src/server/graphics/nested/display.h	2016-08-01 09:37:45 +0000
 +++ src/server/graphics/nested/display.h	2016-09-15 01:44:40 +0000
@@ -128,6 +128,9 @@
      void for_each_display_sync_group(std::function<void(DisplaySyncGroup&)>const& f) override;
      std::unique_ptr<DisplayConfiguration> configuration() const override;
++
++    bool apply_if_configuration_preserves_display_buffers(DisplayConfiguration const& conf) const override;
++
      void configure(DisplayConfiguration const&) override;
      void register_configuration_change_handler(
 === modified file 'src/server/scene/mediating_display_changer.cpp'
 --- src/server/scene/mediating_display_changer.cpp	2016-09-12 02:58:30 +0000
 +++ src/server/scene/mediating_display_changer.cpp	2016-09-15 01:44:40 +0000
@@ -18,6 +18,7 @@
  #include <condition_variable>
  #include <boost/throw_exception.hpp>
++#include <unordered_set>
  #include "mediating_display_changer.h"
  #include "session_container.h"
  #include "mir/scene/session.h"
@@ -330,15 +331,47 @@
      server_action_queue->resume_processing_for(this);
+ }
++namespace
++{
++bool configuration_has_new_outputs_enabled(
++    mg::DisplayConfiguration const& existing,
++    mg::DisplayConfiguration const& updated)
++{
++    std::unordered_set<mg::DisplayConfigurationOutputId> currently_enabled_outputs;
++    existing.for_each_output(
++        [&currently_enabled_outputs](auto const& output)
++        {
++            if (output.used)
++            {
++                currently_enabled_outputs.insert(output.id);
++            }
++        });
++    bool has_new_output{false};
++    updated.for_each_output(
++        [&currently_enabled_outputs, &has_new_output](auto const& output)
++        {
++            if (output.used)
++            {
++                has_new_output |= (currently_enabled_outputs.count(output.id) == 0);
++            }
++        });
++    return has_new_output;
++}
++}
++
  void ms::MediatingDisplayChanger::apply_config(
      std::shared_ptr<graphics::DisplayConfiguration> const& conf)
+ {
      report->new_configuration(*conf);
--    ApplyNowAndRevertOnScopeExit comp{
--        [this] { compositor->stop(); },
--        [this] { compositor->start(); }};
--    display->configure(*conf);
++    if (configuration_has_new_outputs_enabled(*display->configuration(), *conf) ||
++        !display->apply_if_configuration_preserves_display_buffers(*conf))
++    {
++        ApplyNowAndRevertOnScopeExit comp{
++            [this] { compositor->stop(); },
++            [this] { compositor->start(); }};
++        display->configure(*conf);
++    }
      update_input_rectangles(*conf);
      base_configuration_applied = false;
 === modified file 'tests/include/mir/test/doubles/mock_display.h'
 --- tests/include/mir/test/doubles/mock_display.h	2016-08-01 09:37:45 +0000
 +++ tests/include/mir/test/doubles/mock_display.h	2016-09-15 01:44:40 +0000
@@ -37,6 +37,8 @@
  public:
      MOCK_METHOD1(for_each_display_sync_group, void (std::function<void(graphics::DisplaySyncGroup&)> const&));
      MOCK_CONST_METHOD0(configuration, std::unique_ptr<graphics::DisplayConfiguration>());
++    MOCK_CONST_METHOD1(apply_if_configuration_preserves_display_buffers, bool(graphics::DisplayConfiguration const
++        &));
      MOCK_METHOD1(configure, void(graphics::DisplayConfiguration const&));
      MOCK_METHOD2(register_configuration_change_handler,
                   void(graphics::EventHandlerRegister&, graphics::DisplayConfigurationChangeHandler const&));
 === modified file 'tests/mir_test_framework/stubbed_graphics_platform.cpp'
 --- tests/mir_test_framework/stubbed_graphics_platform.cpp	2016-09-13 08:09:53 +0000
 +++ tests/mir_test_framework/stubbed_graphics_platform.cpp	2016-09-15 01:44:40 +0000
@@ -62,6 +62,10 @@
+     {
          return display->configuration();
+     }
++    bool apply_if_configuration_preserves_display_buffers(mg::DisplayConfiguration const& /*conf*/) const override
++    {
++        return false;
++    }
      void configure(mg::DisplayConfiguration const& conf) override
+     {
          display->configure(conf);
 === modified file 'tests/unit-tests/scene/test_mediating_display_changer.cpp'
 --- tests/unit-tests/scene/test_mediating_display_changer.cpp	2016-09-12 02:58:30 +0000
 +++ tests/unit-tests/scene/test_mediating_display_changer.cpp	2016-09-15 01:44:40 +0000
@@ -209,12 +209,15 @@
      EXPECT_THAT(*base_conf, mt::DisplayConfigMatches(std::ref(*mock_display.conf_ptr)));
+ }
--TEST_F(MediatingDisplayChangerTest, pauses_system_when_applying_new_configuration_for_focused_session)
++TEST_F(MediatingDisplayChangerTest, pauses_system_when_applying_new_configuration_for_focused_session_would_invalidate_display_buffers)
+ {
      using namespace testing;
      mtd::NullDisplayConfiguration conf;
      auto session = std::make_shared<mtd::StubSession>();
++    ON_CALL(mock_display, apply_if_configuration_preserves_display_buffers(_))
++        .WillByDefault(Return(false));
++
      InSequence s;
      EXPECT_CALL(mock_compositor, stop());
@@ -227,13 +230,32 @@
                         mt::fake_shared(conf));
+ }
++TEST_F(MediatingDisplayChangerTest, does_not_pause_system_when_applying_new_configuration_for_focused_session_would_preserve_display_buffers)
++{
++    using namespace testing;
++    mtd::NullDisplayConfiguration conf;
++    auto session = std::make_shared<mtd::StubSession>();
++
++    EXPECT_CALL(mock_display, apply_if_configuration_preserves_display_buffers(Ref(conf)))
++        .WillOnce(Return(true));
++
++    EXPECT_CALL(mock_compositor, stop()).Times(0);
++    EXPECT_CALL(mock_compositor, start()).Times(0);
++    EXPECT_CALL(mock_display, configure(_)).Times(0);
++
++    session_event_sink.handle_focus_change(session);
++    changer->configure(session,
++                       mt::fake_shared(conf));
++}
++
++
  TEST_F(MediatingDisplayChangerTest, doesnt_apply_config_for_unfocused_session)
+ {
      using namespace testing;
      mtd::NullDisplayConfiguration conf;
      EXPECT_CALL(mock_compositor, stop()).Times(0);
--    EXPECT_CALL(mock_display, configure(Ref(conf))).Times(0);
++    EXPECT_CALL(mock_display, configure(_)).Times(0);
      EXPECT_CALL(mock_compositor, start()).Times(0);
      changer->configure(std::make_shared<mtd::StubSession>(),
@@ -253,11 +275,14 @@
      EXPECT_THAT(*base_conf, mt::DisplayConfigMatches(conf));
+ }
--TEST_F(MediatingDisplayChangerTest, handles_hardware_change_properly_when_pausing_system)
++TEST_F(MediatingDisplayChangerTest, handles_hardware_change_when_display_buffers_are_invalidated)
+ {
      using namespace testing;
      mtd::NullDisplayConfiguration conf;
++    ON_CALL(mock_display, apply_if_configuration_preserves_display_buffers(_))
++        .WillByDefault(Return(false));
++
      InSequence s;
      EXPECT_CALL(mock_conf_policy, apply_to(Ref(conf)));
@@ -268,6 +293,61 @@
      changer->configure_for_hardware_change(mt::fake_shared(conf));
+ }
++TEST_F(MediatingDisplayChangerTest, handles_hardware_change_when_display_buffers_are_preserved)
++{
++    using namespace testing;
++    mtd::NullDisplayConfiguration conf;
++
++    {
++        InSequence s;
++        EXPECT_CALL(mock_conf_policy, apply_to(Ref(conf)));
++
++        EXPECT_CALL(mock_display, apply_if_configuration_preserves_display_buffers(Ref(conf)))
++            .WillOnce(Return(true));
++    }
++
++    EXPECT_CALL(mock_compositor, stop()).Times(0);
++    EXPECT_CALL(mock_display, configure(_)).Times(0);
++    EXPECT_CALL(mock_compositor, start()).Times(0);
++
++    changer->configure_for_hardware_change(mt::fake_shared(conf));
++}
++
++TEST_F(MediatingDisplayChangerTest, handles_hardware_change_when_display_buffers_are_preserved_but_new_outputs_are_enabled)
++{
++    using namespace testing;
++
++    auto conf = changer->base_configuration();
++
++    bool new_output_enabled{false};
++    conf->for_each_output(
++        [&new_output_enabled](mg::UserDisplayConfigurationOutput& output)
++        {
++            if (!output.used)
++            {
++                new_output_enabled = true;
++                output.used = true;
++            }
++        });
++    ASSERT_TRUE(new_output_enabled);
++
++    ON_CALL(mock_display, apply_if_configuration_preserves_display_buffers(_))
++        .WillByDefault(Return(true));
++
++    InSequence s;
++    EXPECT_CALL(mock_conf_policy, apply_to(Ref(*conf)));
++
++    /*
++     * Currently we have to tear down and recreate the compositor in order to add
++     * a new output. This is not an inherent limitation, and we might do better later.
++     */
++    EXPECT_CALL(mock_compositor, stop()).Times(1);
++    EXPECT_CALL(mock_display, configure(Ref(*conf)));
++    EXPECT_CALL(mock_compositor, start()).Times(1);
++
++    changer->configure_for_hardware_change(conf);
++}
++
  TEST_F(MediatingDisplayChangerTest, hardware_change_doesnt_apply_base_config_if_per_session_config_is_active)
+ {
      using namespace testing;
@@ -307,11 +387,70 @@
      changer->configure_for_hardware_change(mt::fake_shared(conf));
+ }
--TEST_F(MediatingDisplayChangerTest, focusing_a_session_with_attached_config_applies_config)
--{
--    using namespace testing;
--    auto conf = std::make_shared<mtd::NullDisplayConfiguration>();
--    auto session1 = std::make_shared<mtd::StubSession>();
++TEST_F(MediatingDisplayChangerTest, focusing_a_session_with_db_preserving_attached_config_applies_config)
++{
++    using namespace testing;
++    auto conf = std::make_shared<mtd::NullDisplayConfiguration>();
++    auto session1 = std::make_shared<mtd::StubSession>();
++
++    stub_session_container.insert_session(session1);
++    changer->configure(session1, conf);
++
++    EXPECT_CALL(mock_display, apply_if_configuration_preserves_display_buffers(Ref(*conf)))
++        .WillOnce(Return(true));
++
++    EXPECT_CALL(mock_compositor, stop()).Times(0);
++    EXPECT_CALL(mock_display, configure(_)).Times(0);
++    EXPECT_CALL(mock_compositor, start()).Times(0);
++
++    session_event_sink.handle_focus_change(session1);
++}
++
++TEST_F(MediatingDisplayChangerTest, focusing_a_session_with_db_preserving_but_output_adding_attached_config_applies_config)
++{
++    using namespace testing;
++    auto session1 = std::make_shared<mtd::StubSession>();
++
++    auto conf = changer->base_configuration();
++
++    bool new_output_enabled{false};
++    conf->for_each_output(
++        [&new_output_enabled](mg::UserDisplayConfigurationOutput& output)
++        {
++            if (!output.used)
++            {
++                new_output_enabled = true;
++                output.used = true;
++            }
++        });
++    ASSERT_TRUE(new_output_enabled);
++
++    ON_CALL(mock_display, apply_if_configuration_preserves_display_buffers(_))
++        .WillByDefault(Return(true));
++
++    stub_session_container.insert_session(session1);
++    changer->configure(session1, conf);
++
++    /*
++     * Currently we have to tear down and recreate the compositor in order to add
++     * a new output. This is not an inherent limitation, and we might do better later.
++     */
++    InSequence s;
++    EXPECT_CALL(mock_compositor, stop()).Times(1);
++    EXPECT_CALL(mock_display, configure(Ref(*conf)));
++    EXPECT_CALL(mock_compositor, start()).Times(1);
++
++    session_event_sink.handle_focus_change(session1);
++}
++
++TEST_F(MediatingDisplayChangerTest, focusing_a_session_with_db_invalidating_attached_config_applies_config)
++{
++    using namespace testing;
++    auto conf = std::make_shared<mtd::NullDisplayConfiguration>();
++    auto session1 = std::make_shared<mtd::StubSession>();
++
++    ON_CALL(mock_display, apply_if_configuration_preserves_display_buffers(_))
++        .WillByDefault(Return(false));
      stub_session_container.insert_session(session1);
      changer->configure(session1, conf);
@@ -324,13 +463,16 @@
      session_event_sink.handle_focus_change(session1);
+ }
--TEST_F(MediatingDisplayChangerTest, focusing_a_session_without_attached_config_applies_base_config)
++TEST_F(MediatingDisplayChangerTest, focusing_a_session_without_attached_config_applies_base_config_pausing_if_db_invalidated)
+ {
      using namespace testing;
      auto conf = std::make_shared<mtd::NullDisplayConfiguration>();
      auto session1 = std::make_shared<mtd::StubSession>();
      auto session2 = std::make_shared<mtd::StubSession>();
++    ON_CALL(mock_display, apply_if_configuration_preserves_display_buffers(_))
++        .WillByDefault(Return(false));
++
      stub_session_container.insert_session(session1);
      changer->configure(session1, conf);
@@ -347,6 +489,43 @@
      session_event_sink.handle_focus_change(session2);
+ }
++TEST_F(MediatingDisplayChangerTest, focusing_a_session_without_attached_config_applies_base_config_not_pausing_if_db_preserved)
++{
++    using namespace testing;
++    std::shared_ptr<mg::DisplayConfiguration> conf = base_config.clone();
++    conf->for_each_output(
++        [](mg::UserDisplayConfigurationOutput& output)
++        {
++            if (output.used)
++            {
++                output.orientation =
++                    output.orientation == mir_orientation_normal ? mir_orientation_left : mir_orientation_normal;
++            }
++        });
++
++    auto session1 = std::make_shared<mtd::StubSession>();
++    auto session2 = std::make_shared<mtd::StubSession>();
++
++    stub_session_container.insert_session(session1);
++    changer->configure(session1, conf);
++
++    session_event_sink.handle_focus_change(session1);
++
++    Mock::VerifyAndClearExpectations(&mock_compositor);
++    Mock::VerifyAndClearExpectations(&mock_display);
++
++    EXPECT_CALL(
++        mock_display,
++        apply_if_configuration_preserves_display_buffers(mt::DisplayConfigMatches(std::cref(base_config))))
++            .WillOnce(Return(true));
++
++    EXPECT_CALL(mock_compositor, stop()).Times(0);
++    EXPECT_CALL(mock_display, configure(_)).Times(0);
++    EXPECT_CALL(mock_compositor, start()).Times(0);
++
++    session_event_sink.handle_focus_change(session2);
++}
++
  TEST_F(MediatingDisplayChangerTest, losing_focus_applies_base_config)
+ {
      using namespace testing;