Merge lp:~mterry/unity-mir/alpha-greeter into lp:unity-mir

[Michael Terry]

When part of a greeter shell, use a transparent background instead of an opaque one.

When part of a greeter shell, use a transparent background instead of an opaque one.

The way the unity greeter is designed, it wants to show the session beneath it as the user swipes away. This way, the background of the greeter's Mir surface won't be black.

The DisplayConfigurationPolicy class is a copy of the mg::DefaultDisplayConfigurationPolicy but with the option to search for a transparent background instead of being hard-coded to look for an opaque one.

== Checklist ==

 * Are there any related MPs required for this MP to build/function as expected?
 - No
 * Did you perform an exploratory manual test run of your code change and any related functionality?
 - Yes
 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
 - N/A

[PS Jenkins bot (ps-jenkins)]

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

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

review: Needs Fixing (continuous-integration)

[Gerry Boland (gerboland)]

We really need a better way of configuring displays at startup (I've been looking into a similar problem just today actually).

Completely replacing the Mir implementation of DefaultDisplayConfigurationPolicy with a copy/paste just opens up code divergence problems. We should ask for something more robust for a shell to use.

[Michael Terry (mterry)]

I agree it's not the best. But this current system is already the result of asking mir-team to give us a way to selectively enable alpha for the greeter.

Code divergence isn't *so* bad here, because the DefaultPolicy will never be used. So anyone that wants to make a change would be doing it in unity-mir.

[Alan Griffiths (alan-griffiths)]

> I agree it's not the best. But this current system is already the result of
> asking mir-team to give us a way to selectively enable alpha for the greeter.
>
> Code divergence isn't *so* bad here, because the DefaultPolicy will never be
> used. So anyone that wants to make a change would be doing it in unity-mir.

Its always hard to get interfaces and defaults right until real users comes along.

Maybe explaining what you each need will help the Mir team come up with something better.

review: Needs Information

[Andreas Pokorny (andreas-pokorny)]

If it is just alpha true/false this can be done simpler:

http://bazaar.launchpad.net/~mir-team/mir/development-branch/view/head:/examples/pixel_format_selector.cpp
http://bazaar.launchpad.net/~mir-team/mir/development-branch/view/head:/examples/translucent_server_configuration.cpp

[Gerry Boland (gerboland)]

> Its always hard to get interfaces and defaults right until real users comes
> along.
>
> Maybe explaining what you each need will help the Mir team come up with
> something better.

Hey Alan, what I need is the ability to rotate the display on startup for some devices (use-case Nexus 7: display hardwired as portrait, but we want UI in landscape).

It looks like Michael needs the ability to enable the alpha channel.

I must admit, thinking about it and possible future requirements (multiple displays especially) an obvious API doesn't strike me. Could something that could be stuck in before the configure_output calls that can let a custom implementation modify the decided parameters? Or is that just making it worse?

[Michael Terry (mterry)]

Ah, thank you Andreas! I missed that example when I was looking at your alpha-mode merge. That makes sense and fixes the code duplication concerns.

[Michael Terry (mterry)]

OK, now that Mir 0.1.4 has landed, I'll open this up for review again.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:174
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/247/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/110
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/111
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/111/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/110

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

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

It might be worth nothing that (with current Mir) DisplayConfigurationPolicy::apply_to is only called if the instance of Mir is a GBM or nested session (i.e. it isn't called when running on android as I found to my surprise!).

> auto const& pos
name doesn't imply it is a pixel format. Since you're use auto, please have a variable name that indicates the purpose. At a glance, I'd read it as "position".

No other complaints code wise.

Some instructions on how to properly test would be appreciated.

review: Needs Fixing

[lp:~mterry/unity-mir/alpha-greeter]

175. By Michael Terry on 2014-02-12

Make variable name clearer

[Michael Terry (mterry)]

Here's how to test. It's a bear, be warned.

1) Install latest image (one with nested Mir mode).
2) Install lp:~mterry/unity-system-compositor/greeter-depth
3) Install lp:~mterry/unity8/split
4) Edit /etc/lightdm/lightdm.conf.d/90-phablet.conf to be:
[SeatDefaults]
autologin-in-background=true
autologin-user=phablet

This all will install a split greeter that is started at the same time as the session. There's a bit of a race at the moment, so you might see the session for a second or two first. But the greeter will come up.

If you swipe away the greeter, you'll see black as you swipe. Swipe far enough and it will disappear into the session. Then:

5) Install lp:~mterry/unity-mir/alpha-greeter (this branch)

Now when you reboot, you will notice that instead of black, you can see through the greeter into the session as you swipe.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:175
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/249/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/112
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/113
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/113/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/112

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

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

Tests ok, code fine. +1 from me

review: Approve

[Gerry Boland (gerboland)]

@mterry: please add the descriptiong the review checklist questions & your answers from https://wiki.ubuntu.com/Process/Merges/Checklists/Unity-Mir

• Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

• Did CI run pass? If not, please explain why.
It did

[Michael Terry (mterry)]

Checklist added!

[lp:~mterry/unity-mir/alpha-greeter]

176. By Michael Terry on 2014-02-19

Merge from trunk

=== modified file 'src/unity-mir/CMakeLists.txt'
--- src/unity-mir/CMakeLists.txt	2014-02-07 16:10:12 +0000
+++ src/unity-mir/CMakeLists.txt	2014-02-19 23:28:37 +0000
@@ -15,6 +15,7 @@
   UNITY_MIR_HEADERS
 
   dbusscreen.h
+  displayconfigurationpolicy.h
   initialsurfaceplacementstrategy.h
   qmirserverapplication.h
   qmirserver.h
@@ -36,6 +37,7 @@
   unity-mir SHARED
 
   dbusscreen.cpp
+  displayconfigurationpolicy.cpp
   initialsurfaceplacementstrategy.cpp
   qmirserverapplication.cpp
   qmirserver.cpp

=== added file 'src/unity-mir/displayconfigurationpolicy.cpp'
--- src/unity-mir/displayconfigurationpolicy.cpp	1970-01-01 00:00:00 +0000
+++ src/unity-mir/displayconfigurationpolicy.cpp	2014-02-19 23:28:37 +0000
@@ -0,0 +1,63 @@
+/*
+ * Copyright (C) 2013,2014 Canonical, Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU Lesser General Public License version 3, as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+ * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "displayconfigurationpolicy.h"
+#include "logging.h"
+
+#include "mir/graphics/display_configuration.h"
+#include "mir/graphics/pixel_format_utils.h"
+
+#include <algorithm>
+
+namespace geom = mir::geometry;
+namespace mg = mir::graphics;
+
+DisplayConfigurationPolicy::DisplayConfigurationPolicy(std::shared_ptr<mg::DisplayConfigurationPolicy> const& base_policy, bool with_alpha)
+    :   base_policy{base_policy},
+        with_alpha{with_alpha}
+{
+}
+
+void DisplayConfigurationPolicy::apply_to(mg::DisplayConfiguration& conf)
+{
+    DLOG("DisplayConfigurationPolicy::apply_to (this=%p, conf=%p)", this, &conf);
+
+    base_policy->apply_to(conf);
+    conf.for_each_output(
+        [&](mg::DisplayConfigurationOutput const& conf_output)
+        {
+            if (!conf_output.connected || !conf_output.used) return;
+
+            auto const& format = find_if(conf_output.pixel_formats.begin(),
+                                         conf_output.pixel_formats.end(),
+                                         [&](MirPixelFormat format) -> bool
+                                             {
+                                                 return mg::contains_alpha(format) == with_alpha;
+                                             }
+                                        );
+
+            // keep the default settings if nothing was found
+            if (format == conf_output.pixel_formats.end())
+                return;
+
+            conf.configure_output(conf_output.id, true, conf_output.top_left,
+                                  conf_output.current_mode_index,
+                                  *format,
+                                  conf_output.power_mode,
+                                  conf_output.orientation
+                                 );
+        });
+}

=== added file 'src/unity-mir/displayconfigurationpolicy.h'
--- src/unity-mir/displayconfigurationpolicy.h	1970-01-01 00:00:00 +0000
+++ src/unity-mir/displayconfigurationpolicy.h	2014-02-19 23:28:37 +0000
@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2013,2014 Canonical, Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU Lesser General Public License version 3, as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+ * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef DISPLAYCONFIGURATIONPOLICY_H
+#define DISPLAYCONFIGURATIONPOLICY_H
+
+#include "mir/graphics/display_configuration_policy.h"
+
+#include <memory>
+
+class DisplayConfigurationPolicy : public mir::graphics::DisplayConfigurationPolicy
+{
+public:
+    DisplayConfigurationPolicy(std::shared_ptr<mir::graphics::DisplayConfigurationPolicy> const& base_policy,
+                               bool with_alpha);
+    void apply_to(mir::graphics::DisplayConfiguration& conf);
+
+private:
+    std::shared_ptr<mir::graphics::DisplayConfigurationPolicy> const base_policy;
+    bool const with_alpha;
+};
+
+#endif // DISPLAYCONFIGURATIONPOLICY_H

=== modified file 'src/unity-mir/shellserverconfiguration.cpp'
--- src/unity-mir/shellserverconfiguration.cpp	2014-02-07 16:10:12 +0000
+++ src/unity-mir/shellserverconfiguration.cpp	2014-02-19 23:28:37 +0000
@@ -16,6 +16,7 @@
 
 #include "shellserverconfiguration.h"
 
+#include "displayconfigurationpolicy.h"
 #include "unityprotobufservice.h"
 #include "initialsurfaceplacementstrategy.h"
 #include "serverstatuslistener.h"
@@ -27,6 +28,7 @@
 #include "focussetter.h"
 #include "logging.h"
 
+namespace mg = mir::graphics;
 namespace msh = mir::shell;
 
 ShellServerConfiguration::ShellServerConfiguration(int argc, char const* argv[], QObject* parent)
@@ -41,6 +43,17 @@
 {
 }
 
+std::shared_ptr<mg::DisplayConfigurationPolicy>
+ShellServerConfiguration::the_display_configuration_policy()
+{
+    return display_configuration_policy(
+        [this]
+        {
+            bool alpha = (qgetenv("XDG_SESSION_CLASS") == "greeter");
+            return std::make_shared<DisplayConfigurationPolicy>(DefaultServerConfiguration::the_display_configuration_policy(), alpha);
+        });
+}
+
 std::shared_ptr<msh::PlacementStrategy>
 ShellServerConfiguration::the_shell_placement_strategy()
 {

=== modified file 'src/unity-mir/shellserverconfiguration.h'
--- src/unity-mir/shellserverconfiguration.h	2014-02-06 12:12:21 +0000
+++ src/unity-mir/shellserverconfiguration.h	2014-02-19 23:28:37 +0000
@@ -47,6 +47,7 @@
     ~ShellServerConfiguration();
 
     /* mir specific */
+    std::shared_ptr<mir::graphics::DisplayConfigurationPolicy> the_display_configuration_policy() override;
     std::shared_ptr<mir::shell::PlacementStrategy> the_shell_placement_strategy() override;
     std::shared_ptr<mir::shell::SessionListener> the_shell_session_listener() override;
     std::shared_ptr<mir::shell::SurfaceConfigurator> the_shell_surface_configurator() override;