Merge lp:~fkrull/unity-2d/compiz-integration into lp:unity-2d/3.0

[Felix Krull]

For the workspaces and window overviews, trigger Compiz's Expo and Scale plugins if Compiz is running. If Compiz isn't running, said plugins aren't enabled or the D-Bus plugin isn't enabled, fall back to using unity-2d-spread.

See also https://bugs.launchpad.net/unity-2d/+bug/760674

How to test:
Log into Unity 2D. Run `compiz --replace`. Using ccsm, make sure that the Expo, Scale and D-Bus plugins for Compiz are enabled. Trigger the workspace or window overview.

[lp:~fkrull/unity-2d/compiz-integration]

534. By Felix Krull on 2011-04-25

Also invoke Expo plugin on Super+S keypress.

[Florian Boucault (fboucault)]

Although the code looks generally ok, I am unable to test it for a lack of D-Bus service 'org.freedesktop.compiz'. I carefully activated Expo, Scale and D-Bus plugins in ccsm. Any tips?

[Florian Boucault (fboucault)]

Note there is a conflict with trunk.

review: Needs Fixing

[Felix Krull (fkrull)]

I can't really reproduce your problem regarding D-Bus. Having just enabled the plugin, I can see that service just fine (in qdbusviewer). Anyway, how would I resolve that conflict with trunk? I tried to rebase, but couldn't push that -- obviously, really. Or would I just pull and merge from trunk?

[Olivier Tilloy (osomon)]

Felix, you’ll need to merge the latest trunk into your branch and solve the conflict(s) there. Once this is done, commit and push to lp:~fkrull/unity-2d/compiz-integration. Thanks!

[lp:~fkrull/unity-2d/compiz-integration]

535. By Felix Krull on 2011-05-03

Merged trunk (r557).

536. By Felix Krull on 2011-05-14

Merged r560 from trunk.

[Felix Krull (fkrull)]

I've resolved any conflicts, but I noticed another problem with the code: Activating the window overview for an application doesn't work correctly using the Super+<Number> shortcut keys.

a) The overview doesn't appear as soon as you press the number button (and toggles quickly when kept pressed), but only when you release it. Also, one has to keep the keys pressed for a time (half a second, maybe), otherwise nothing happens.
b) It only affects the keyboard shortcuts. Activating the overview by clicking on an icon in the launcher works perfectly. Also, holding Super and then clicking an icon works as well.
c) It only affects the window overview with Compiz's Scale plugin. Switching to the application when it is not focused works, as does activating the unity-2d-spread 2D overview, *as does activating Compiz's Expo plugin*.

I have honestly no idea what this is about. I can only guess that there is some weird quirk or bug with the Scale plugin. I don't think it's a bug in my code, because I can't imagine where that would come from. Anyway, with that in mind, you might or might not want to actually merge this; I think it's OK apart from this one problem.

(Do I need to select resubmit here or anything?)

[Olivier Tilloy (osomon)]

No need for a resubmit or any additional interaction on your part Felix. Thanks for your work, we’ll make sure to look into it soon.

[Florian Boucault (fboucault)]

After resolving some new conflicts that arose with the latest trunk I performed some functional testing and I must say that it works really well! Even the Super+<Number> shortcut keys :)

I will finish checking a bit more the code and then it's good to go. Well done Felix!

review: Approve (functional)

[Florian Boucault (fboucault)]

Merge with trunk was done in branch: lp:~fboucault/unity-2d/compiz-integration

[Florian Boucault (fboucault)]

Felix, I think the code is good. I added one simplification for which I would love to have your feedback on. It's in the last revision of lp:~fboucault/unity-2d/compiz-integration

If that change is ok with you then I suggest we merge the whole thing in trunk.

review: Approve (code)

[Felix Krull (fkrull)]

Yeah, sure, no problem with that ;) I'm not really familiar with Qt, so I just guessed there; your variant is certainly simpler.

The number shortcuts consistently don't work as I described both on my main system and in a clean Natty VM. If it works for you, did you do anything weird or do you have a different Compiz or something?

[Florian Boucault (fboucault)]

Thanks Felix, it's now merged in trunk and will be part of tomorrow's daily build.

I don't think I have any strange Compiz version or configuration. In case that was not clear, I am using my laptop's keyboard with no external keyboard connected and pressing the number keys that are above the letters.

=== modified file 'launcher/UnityApplications/launcherapplication.cpp'
--- launcher/UnityApplications/launcherapplication.cpp	2011-04-28 13:09:25 +0000
+++ launcher/UnityApplications/launcherapplication.cpp	2011-05-14 15:59:28 +0000
@@ -50,6 +50,7 @@
 #include <debug_p.h>
 
 // Qt
+#include <Qt>
 #include <QDebug>
 #include <QAction>
 #include <QDBusInterface>
@@ -57,6 +58,7 @@
 #include <QFile>
 #include <QFileSystemWatcher>
 #include <QScopedPointer>
+#include <QX11Info>
 
 extern "C" {
 #include <libsn/sn.h>
@@ -767,21 +769,41 @@
 void
 LauncherApplication::spread(bool showAllWorkspaces)
 {
-    QDBusInterface iface("com.canonical.Unity2d.Spread", "/Spread",
-                         "com.canonical.Unity2d.Spread");
-    QDBusReply<bool> isShown = iface.call("IsShown");
-    if (isShown.isValid()) {
-        if (isShown.value() == true) {
-            iface.asyncCall("FilterByApplication", m_application->desktop_file());
-        } else {
-            if (showAllWorkspaces) {
-                iface.asyncCall("ShowAllWorkspaces", m_application->desktop_file());
+    QDBusInterface compiz("org.freedesktop.compiz",
+                          "/org/freedesktop/compiz/scale/screen0/initiate_all_key",
+                          "org.freedesktop.compiz");
+
+    if (compiz.isValid()) {
+        Qt::HANDLE root = QX11Info::appRootWindow();
+        BamfUintList* xids = m_application->xids();
+        QString match = "";
+        for (int i = 0; i < xids->size(); i++) {
+            uint xid = xids->at(i);
+            match.append("xid=");
+            QVariant tmp(xid);
+            match.append(tmp.toString());
+            if (i < xids->size() - 1) {
+                match.append(" | ");
+            }
+        }
+        compiz.asyncCall("activate", "root", static_cast<int>(root), "match", match);
+    } else {
+        QDBusInterface spread("com.canonical.Unity2d.Spread", "/Spread",
+                              "com.canonical.Unity2d.Spread");
+        QDBusReply<bool> isShown = spread.call("IsShown");
+        if (isShown.isValid()) {
+            if (isShown.value() == true) {
+                spread.asyncCall("FilterByApplication", m_application->desktop_file());
             } else {
-                iface.asyncCall("ShowCurrentWorkspace", m_application->desktop_file());
+                if (showAllWorkspaces) {
+                    spread.asyncCall("ShowAllWorkspaces", m_application->desktop_file());
+                } else {
+                    spread.asyncCall("ShowCurrentWorkspace", m_application->desktop_file());
+                }
             }
+        } else {
+            UQ_WARNING << "Failed to get property IsShown on com.canonical.Unity2d.Spread";
         }
-    } else {
-        UQ_WARNING << "Failed to get property IsShown on com.canonical.Unity2d.Spread";
     }
 }
 
 
=== modified file 'launcher/UnityApplications/workspaces.cpp'
--- launcher/UnityApplications/workspaces.cpp	2011-04-28 13:09:25 +0000
+++ launcher/UnityApplications/workspaces.cpp	2011-05-14 15:59:28 +0000
@@ -23,6 +23,8 @@
 
 #include <QDBusInterface>
 #include <QDBusReply>
+#include <Qt>
+#include <QX11Info>
 
 // libunity-2d
 #include <unity2dtr.h>
@@ -85,17 +87,26 @@
 void
 Workspaces::activate()
 {
-    QDBusInterface iface("com.canonical.Unity2d.Spread", "/Spread",
-                         "com.canonical.Unity2d.Spread");
-    QDBusReply<bool> isShown = iface.call("IsShown");
-    if (isShown.isValid()) {
-        if (isShown.value() == true) {
-            iface.asyncCall("FilterByApplication", QString());
+    QDBusInterface compiz("org.freedesktop.compiz",
+                          "/org/freedesktop/compiz/expo/screen0/expo_key",
+                          "org.freedesktop.compiz");
+
+    if (compiz.isValid()) {
+        Qt::HANDLE root = QX11Info::appRootWindow();
+        compiz.asyncCall("activate", "root", static_cast<int>(root));
+    } else {
+        QDBusInterface spread("com.canonical.Unity2d.Spread", "/Spread",
+                              "com.canonical.Unity2d.Spread");
+        QDBusReply<bool> isShown = spread.call("IsShown");
+        if (isShown.isValid()) {
+            if (isShown.value() == true) {
+                spread.asyncCall("FilterByApplication", QString());
+            } else {
+                spread.asyncCall("ShowAllWorkspaces", QString());
+            }
         } else {
-            iface.asyncCall("ShowAllWorkspaces", QString());
+            UQ_WARNING << "Failed to get property IsShown on com.canonical.Unity2d.Spread";
         }
-    } else {
-        UQ_WARNING << "Failed to get property IsShown on com.canonical.Unity2d.Spread";
     }
 }
 
 
=== modified file 'launcher/app/launcherview.cpp'
--- launcher/app/launcherview.cpp	2011-05-03 18:34:05 +0000
+++ launcher/app/launcherview.cpp	2011-05-14 15:59:28 +0000
@@ -344,19 +344,28 @@
 void
 LauncherView::showWorkspaceSwitcher()
 {
-    QDBusInterface spreadInterface(SPREAD_DBUS_SERVICE, SPREAD_DBUS_PATH, SPREAD_DBUS_INTERFACE);
-
-    /* Here we only show the spread, if it's hidden.
-       However on Super+s the spread should exit if it's already running.
-       That is done directly in spread/Workspaces.qml because the spread
-       fully grabs the keyboard, so it's the only place where Super+s can
-       be handled while the spread is active */
-    QDBusReply<bool> isShown = spreadInterface.call("IsShown");
-    if (isShown.isValid()) {
-        if (isShown.value() == false) {
-            spreadInterface.asyncCall("ShowAllWorkspaces", QString());
+    QDBusInterface compiz("org.freedesktop.compiz",
+                          "/org/freedesktop/compiz/expo/screen0/expo_key",
+                          "org.freedesktop.compiz");
+
+    if (compiz.isValid()) {
+        Qt::HANDLE root = QX11Info::appRootWindow();
+        compiz.asyncCall("activate", "root", static_cast<int>(root));
+    } else {
+        QDBusInterface spreadInterface(SPREAD_DBUS_SERVICE, SPREAD_DBUS_PATH, SPREAD_DBUS_INTERFACE);
+
+        /* Here we only show the spread, if it's hidden.
+           However on Super+s the spread should exit if it's already running.
+           That is done directly in spread/Workspaces.qml because the spread
+           fully grabs the keyboard, so it's the only place where Super+s can
+           be handled while the spread is active */
+        QDBusReply<bool> isShown = spreadInterface.call("IsShown");
+        if (isShown.isValid()) {
+            if (isShown.value() == false) {
+                spreadInterface.asyncCall("ShowAllWorkspaces", QString());
+            }
+        } else {
+            UQ_WARNING << "Failed to get property IsShown on" << SPREAD_DBUS_SERVICE;
         }
-    } else {
-        UQ_WARNING << "Failed to get property IsShown on" << SPREAD_DBUS_SERVICE;
     }
 }