Merge lp:~vanvugt/unity-mir/fix-1255045 into lp:unity-mir

[Daniel van Vugt]

Force the screen to redraw after turning the display back on (LP: #1255045).
Also stop the compositor when the screen is off. Otherwise it will spin in
the background, eating battery.

UNTESTED PROOF OF CONCEPT

Someone please try this out and let me know how it goes.

[Daniel van Vugt (vanvugt)]

Also, I will now be away until Monday. If this doesn't quite suit your needs, please copy it, modify, and resubmit as you wish.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:154
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/167/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/28/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/28/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/28/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/167/rebuild

review: Needs Fixing (continuous-integration)

[Michał Sawicz (saviq)]

Yup, this seems to be working!

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:154
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/168/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/29/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/29/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/29/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/168/rebuild

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

Land this to fix bug by all means, but is it only the compositor that needs to be paused/resumed? Not the input and connector services too (via display->pause()/resume())?

review: Needs Information

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:154
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/169/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/30/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/30/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/30/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/169/rebuild

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

> Land this to fix bug by all means, but is it only the compositor that needs to
> be paused/resumed? Not the input and connector services too (via
> display->pause()/resume())?

OK can we clarify the intent?

Are we intentionally pausing and resuming the compositing service or are we only doing that as a way to run:

    for (auto& f : thread_functors)
        f->schedule_compositing();

If, as I suspect, the latter then Compositor probably ought to publish a schedule_compositing() function (ideally with a parameter specifying what area(s) need drawing).

In any case this notification (or the pause/resume cycle if that's really needed) probably ought to come from the Display::configure() implementation, not from unity-mir.

[Alexandros Frantzis (afrantzis)]

The MediatingDisplayChanger class holds the logic for properly reconfiguring the display (pausing/resuming), so that's what we should expose (probably through a new interface). The Display::configure() method can't pause/resume internally
because 1. it has no knowledge of the compositor 2. in certain circumstances we don't want to pause/resume the compositor when reconfiguring (e.g. see resume() in display_server.cpp).

About using schedule_compositing(): when we reconfigure the display, we currently always create new display buffers (for gbm, at least, which supports multiple displays), and *have* to restart the compositor to use them. Also, it seems that our display classes don't deal very well with posting buffers when the display is off. So, even if we decide to pursue the schedule_compositing() direction in the future, we are not there yet.

I am OK with this fix until we expose the proper interfaces to unity-mir.

review: Approve

[Alan Griffiths (alan-griffiths)]

> The MediatingDisplayChanger class holds the logic for properly reconfiguring
> the display (pausing/resuming), so that's what we should expose (probably
> through a new interface). The Display::configure() method can't pause/resume
> internally
...
> I am OK with this fix until we expose the proper interfaces to unity-mir.

+1

review: Approve

[Gerry Boland (gerboland)]

Had expected Display::configure() to control the compositor, so yeah I see the need for this fix. I do think however that Mir should handle it in future, am looking forward to the new interfaces.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:154
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/170/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/31/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/32/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/31/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/170/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:154
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/171/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/33/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/34/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/33/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/171/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:154
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/172/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/34/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/34/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/172/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'src/unity-mir/dbusscreen.cpp'
--- src/unity-mir/dbusscreen.cpp	2013-09-25 09:30:08 +0000
+++ src/unity-mir/dbusscreen.cpp	2013-11-28 07:50:58 +0000
@@ -17,6 +17,7 @@
 // mir
 #include <mir/graphics/display.h>
 #include <mir/graphics/display_configuration.h>
+#include <mir/compositor/compositor.h>
 #include <mir_toolkit/common.h>
 
 // qt
@@ -62,6 +63,7 @@
 
     std::shared_ptr<mg::Display> display = m_serverConfig->the_display();
     std::shared_ptr<mg::DisplayConfiguration> displayConfig = display->configuration();
+    std::shared_ptr<mc::Compositor> compositor = m_serverConfig->the_compositor();
 
     displayConfig->for_each_output([&](const mg::DisplayConfigurationOutput displayConfigOutput) {
         if (displayConfigOutput.power_mode != newPowerMode) {
@@ -75,6 +77,13 @@
         }
     });
 
+    if (newPowerMode != MirPowerMode::mir_power_mode_on)
+        compositor->stop();
+
     display->configure(*displayConfig.get());
+
+    if (newPowerMode == MirPowerMode::mir_power_mode_on)
+        compositor->start();
+
     return true;
 }