Merge lp:~osomon/unity-2d/one-panel-per-screen into lp:unity-2d/3.0

[Olivier Tilloy]

[panel] Display one panel per screen.
Only the panel on the primary screen contains the home button applet ("circle of friends" that invokes the dash) and the systray.
All panels contain an instance of the applet that displays the active window’s application title and menu bar, as well as the application and system indicators.

Notes to the reviewer:

 - I developed and tested on Maverick, so thorough functional testing on Natty is expected, in particular dynamically adding/removing screens while unity-2d-panel is running.

 - On Maverick, I noticed one tiny problem with the "me" system indicator: when there are several screens, only the instance on the primary screen has an icon and a label, other instances seem empty. However their menus are there and functional, the problem is only with the menu title. I suspect a bug in the indicator itself, but I’m not sure. It may have been fixed in Natty as well, feedback is welcome.

[Aurélien Gâteau (agateau)]

No time to test tonight, but the way you implemented it looks good. Here are a few remarks to keep things moving:

- Please rename PanelsManager to PanelManager

- Can you make the helper functions in panelmanagers static? this makes it more explicit they are not meant to be used from outside (they were not when they were in main.cpp because I assumed it was clear no code from main.cpp would ever be called from somewhere else)

- Adjust the registrar singleton method to something like this:

Registrar* Registrar::instance()
{
    static Registrar singleton;
    return &singleton;
}

It's almost the same, except memory checkers such as valgrind won't complain about not released memory.

- You don't need to mark Registrar constructor explicit. This is only useful for one-parameter constructors.

I may add a few other requests tomorrow morning, but I have to go now.

review: Needs Fixing

[lp:~osomon/unity-2d/one-panel-per-screen]

502. By Olivier Tilloy on 2011-04-04

Renamed PanelsManager to PanelManager.

503. By Olivier Tilloy on 2011-04-04

Made some helper functions static.

504. By Olivier Tilloy on 2011-04-04

Cosmetics to make the code more valgrind-proof.

505. By Olivier Tilloy on 2011-04-04

Removed unneeded explicitness.

[Aurélien Gâteau (agateau)]

Looks good now. Does not support the case where top edges are not aligned but this is tracked by a separate bug report.

review: Approve

=== modified file 'panel/app/CMakeLists.txt'
--- panel/app/CMakeLists.txt	2011-02-02 16:57:00 +0000
+++ panel/app/CMakeLists.txt	2011-04-04 16:49:30 +0000
@@ -1,5 +1,6 @@
 set(panel_SRCS
     main.cpp
+    panelmanager.cpp
     unity2dstyle.cpp
     )
 

=== modified file 'panel/app/main.cpp'
--- panel/app/main.cpp	2011-03-22 10:31:28 +0000
+++ panel/app/main.cpp	2011-04-04 16:49:30 +0000
@@ -21,18 +21,11 @@
 
 // Local
 #include <config.h>
-
-// Applets
-#include <appindicator/appindicatorapplet.h>
-#include <appname/appnameapplet.h>
-#include <homebutton/homebuttonapplet.h>
-#include <indicator/indicatorapplet.h>
-#include <legacytray/legacytrayapplet.h>
+#include <panelmanager.h>
 
 // Unity
 #include <gnomesessionclient.h>
 #include <unity2ddebug.h>
-#include <unity2dpanel.h>
 #include <unity2dapplication.h>
 #include <unity2dstyle.h>
 #include <unity2dtr.h>
@@ -41,9 +34,6 @@
 #include <QAbstractFileEngineHandler>
 #include <QApplication>
 #include <QFSFileEngine>
-#include <QLabel>
-
-using namespace Unity2d;
 
 class ThemeEngineHandler : public QAbstractFileEngineHandler
 {
@@ -59,32 +49,6 @@
     }
 };
 
-QPalette getPalette()
-{
-    QPalette palette;
-
-    /* Should use the panel's background provided by Unity but it turns
-       out not to be good. It would look like:
-
-         QBrush bg(QPixmap("theme:/panel_background.png"));
-    */
-    QBrush bg(QPixmap(unity2dDirectory() + "/panel/artwork/background.png"));
-    palette.setBrush(QPalette::Window, bg);
-    palette.setBrush(QPalette::Button, bg);
-    palette.setColor(QPalette::WindowText, Qt::white);
-    palette.setColor(QPalette::ButtonText, Qt::white);
-    return palette;
-}
-
-QLabel* createSeparator()
-{
-    QLabel* label = new QLabel;
-    QPixmap pix(unity2dDirectory() + "/panel/artwork/divider.png");
-    label->setPixmap(pix);
-    label->setFixedSize(pix.size());
-    return label;
-}
-
 int main(int argc, char** argv)
 {
     ThemeEngineHandler handler;
@@ -109,16 +73,7 @@
     /* Configure translations */
     Unity2dTr::init("unity-2d", INSTALL_PREFIX "/share/locale");
 
-    Unity2dPanel panel;
-    panel.setEdge(Unity2dPanel::TopEdge);
-    panel.setPalette(getPalette());
-    panel.setFixedHeight(24);
+    PanelManager panels;
 
-    panel.addWidget(new HomeButtonApplet);
-    panel.addWidget(createSeparator());
-    panel.addWidget(new AppNameApplet);
-    panel.addWidget(new LegacyTrayApplet);
-    panel.addWidget(new IndicatorApplet);
-    panel.show();
     return app.exec();
 }

=== added file 'panel/app/panelmanager.cpp'
--- panel/app/panelmanager.cpp	1970-01-01 00:00:00 +0000
+++ panel/app/panelmanager.cpp	2011-04-04 16:49:30 +0000
@@ -0,0 +1,135 @@
+/*
+ * This file is part of unity-2d
+ *
+ * Copyright 2010 Canonical Ltd.
+ *
+ * Authors:
+ * - Olivier Tilloy <[e-mail]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ */
+
+// Self
+#include "panelmanager.h"
+
+// Local
+#include <config.h>
+
+// Applets
+#include <appindicator/appindicatorapplet.h>
+#include <appname/appnameapplet.h>
+#include <homebutton/homebuttonapplet.h>
+#include <indicator/indicatorapplet.h>
+#include <legacytray/legacytrayapplet.h>
+
+// Unity
+#include <unity2dpanel.h>
+
+// Qt
+#include <QApplication>
+#include <QDesktopWidget>
+#include <QLabel>
+
+using namespace Unity2d;
+
+static QPalette getPalette()
+{
+    QPalette palette;
+
+    /* Should use the panel's background provided by Unity but it turns
+       out not to be good. It would look like:
+
+         QBrush bg(QPixmap("theme:/panel_background.png"));
+    */
+    QBrush bg(QPixmap(unity2dDirectory() + "/panel/artwork/background.png"));
+    palette.setBrush(QPalette::Window, bg);
+    palette.setBrush(QPalette::Button, bg);
+    palette.setColor(QPalette::WindowText, Qt::white);
+    palette.setColor(QPalette::ButtonText, Qt::white);
+    return palette;
+}
+
+static QLabel* createSeparator()
+{
+    QLabel* label = new QLabel;
+    QPixmap pix(unity2dDirectory() + "/panel/artwork/divider.png");
+    label->setPixmap(pix);
+    label->setFixedSize(pix.size());
+    return label;
+}
+
+static Unity2dPanel* instantiatePanel(int screen)
+{
+    Unity2dPanel* panel = new Unity2dPanel;
+    panel->setEdge(Unity2dPanel::TopEdge);
+    panel->setPalette(getPalette());
+    panel->setFixedHeight(24);
+
+    int primary = QApplication::desktop()->primaryScreen();
+    if (screen == primary) {
+        panel->addWidget(new HomeButtonApplet);
+        panel->addWidget(createSeparator());
+    }
+    panel->addWidget(new AppNameApplet);
+    if (screen == primary) {
+        /* It doesn’t make sense to have more than one instance of the systray,
+           XEmbed’ed windows can be displayed only once anyway. */
+        panel->addWidget(new LegacyTrayApplet);
+    }
+    panel->addWidget(new IndicatorApplet);
+    return panel;
+}
+
+PanelManager::PanelManager(QObject* parent)
+    : QObject(parent)
+{
+    QDesktopWidget* desktop = QApplication::desktop();
+    for(int i = 0; i < desktop->screenCount(); ++i) {
+        Unity2dPanel* panel = instantiatePanel(i);
+        m_panels.append(panel);
+        panel->show();
+        panel->move(desktop->screenGeometry(i).topLeft());
+    }
+    connect(desktop, SIGNAL(screenCountChanged(int)), SLOT(onScreenCountChanged(int)));
+}
+
+PanelManager::~PanelManager()
+{
+    qDeleteAll(m_panels);
+}
+
+void
+PanelManager::onScreenCountChanged(int newCount)
+{
+    QDesktopWidget* desktop = QApplication::desktop();
+    int size = m_panels.size();
+    /* Update the position of existing panels, and instantiate new panels. */
+    for (int i = 0; i < newCount; ++i) {
+        Unity2dPanel* panel;
+        if (i < size) {
+            panel = m_panels[i];
+        } else {
+            panel = instantiatePanel(i);
+            m_panels.append(panel);
+        }
+        panel->show();
+        panel->move(desktop->screenGeometry(i).topLeft());
+    }
+    /* Remove extra panels if any. */
+    while (m_panels.size() > newCount) {
+        delete m_panels.takeLast();
+    }
+}
+
+#include "panelmanager.moc"
+

=== added file 'panel/app/panelmanager.h'
--- panel/app/panelmanager.h	1970-01-01 00:00:00 +0000
+++ panel/app/panelmanager.h	2011-04-04 16:49:30 +0000
@@ -0,0 +1,48 @@
+/*
+ * This file is part of unity-2d
+ *
+ * Copyright 2010 Canonical Ltd.
+ *
+ * Authors:
+ * - Olivier Tilloy <[e-mail]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ */
+
+#ifndef PanelManager_H
+#define PanelManager_H
+
+// Qt
+#include <QObject>
+#include <QList>
+
+class Unity2dPanel;
+
+class PanelManager : public QObject
+{
+Q_OBJECT
+
+public:
+    PanelManager(QObject* parent = 0);
+    ~PanelManager();
+
+private:
+    Q_DISABLE_COPY(PanelManager)
+    QList<Unity2dPanel*> m_panels;
+
+private Q_SLOTS:
+    void onScreenCountChanged(int newCount);
+};
+
+#endif // PanelManager_H
+

=== modified file 'panel/applets/appname/appnameapplet.cpp'
--- panel/applets/appname/appnameapplet.cpp	2011-03-14 11:23:53 +0000
+++ panel/applets/appname/appnameapplet.cpp	2011-04-04 16:49:30 +0000
@@ -41,6 +41,8 @@
 #include <QLabel>
 #include <QMenuBar>
 #include <QPainter>
+#include <QApplication>
+#include <QDesktopWidget>
 
 static const char* METACITY_DIR = "/usr/share/themes/Ambiance/metacity-1";
 
@@ -217,12 +219,13 @@
 
     bool isMaximized = d->m_windowHelper->isMaximized();
     bool isUserVisibleApp = app ? app->user_visible() : false;
-    bool showMenu = (!d->m_menuBarWidget->isEmpty() && isUserVisibleApp)
+    bool isOnSameScreen = d->m_windowHelper->isMostlyOnScreen(QApplication::desktop()->screenNumber(this));
+    bool showMenu = (!d->m_menuBarWidget->isEmpty() && isUserVisibleApp && isOnSameScreen)
         && (window()->geometry().contains(QCursor::pos())
         || KeyboardModifiersMonitor::instance()->keyboardModifiers() == Qt::AltModifier
         || d->m_menuBarWidget->isOpened()
         );
-    bool showLabel = !(isMaximized && showMenu) && isUserVisibleApp;
+    bool showLabel = !(isMaximized && showMenu) && isUserVisibleApp && isOnSameScreen;
 
     d->m_windowButtonWidget->setVisible(isMaximized);
 

=== modified file 'panel/applets/appname/menubarwidget.cpp'
--- panel/applets/appname/menubarwidget.cpp	2011-01-15 01:41:03 +0000
+++ panel/applets/appname/menubarwidget.cpp	2011-04-04 16:49:30 +0000
@@ -74,7 +74,7 @@
 
 void MenuBarWidget::setupRegistrar()
 {
-    m_registrar = new Registrar(this);
+    m_registrar = Registrar::instance();
     if (!m_registrar->connectToBus()) {
         UQ_WARNING << "could not connect registrar to DBus";
     }

=== modified file 'panel/applets/appname/registrar.cpp'
--- panel/applets/appname/registrar.cpp	2011-02-10 01:10:19 +0000
+++ panel/applets/appname/registrar.cpp	2011-04-04 16:49:30 +0000
@@ -51,8 +51,8 @@
     return argument;
 }
 
-Registrar::Registrar(QObject* parent)
-: QObject(parent)
+Registrar::Registrar()
+: QObject()
 , mServiceWatcher(new QDBusServiceWatcher(this))
 {
     qDBusRegisterMetaType<MenuInfo>();
@@ -67,6 +67,12 @@
     QDBusConnection::sessionBus().unregisterService(mService);
 }
 
+Registrar* Registrar::instance()
+{
+    static Registrar singleton;
+    return &singleton;
+}
+
 bool Registrar::connectToBus(const QString& _service, const QString& _path)
 {
     mService = _service.isEmpty() ? DBUS_SERVICE : _service;

=== modified file 'panel/applets/appname/registrar.h'
--- panel/applets/appname/registrar.h	2011-01-15 01:41:03 +0000
+++ panel/applets/appname/registrar.h	2011-04-04 16:49:30 +0000
@@ -53,8 +53,8 @@
     Q_OBJECT
 
 public:
-    Registrar(QObject*);
-    ~Registrar();
+    /* The registrar is a singleton shared between all instances of MenuBarWidget. */
+    static Registrar* instance();
 
     bool connectToBus(const QString& service = QString(), const QString& objectPath = QString());
 
@@ -72,6 +72,10 @@
     void slotServiceUnregistered(const QString& service);
 
 private:
+    Registrar();
+    Q_DISABLE_COPY(Registrar)
+    ~Registrar();
+
     QDBusServiceWatcher* mServiceWatcher;
     typedef QHash<WId, MenuInfo> MenuInfoDb;
     MenuInfoDb mDb;

=== modified file 'panel/applets/appname/windowhelper.cpp'
--- panel/applets/appname/windowhelper.cpp	2011-02-07 15:15:55 +0000
+++ panel/applets/appname/windowhelper.cpp	2011-04-04 16:49:30 +0000
@@ -40,6 +40,8 @@
 
 // Qt
 #include <QDateTime>
+#include <QApplication>
+#include <QDesktopWidget>
 
 struct WindowHelperPrivate
 {
@@ -111,6 +113,29 @@
     return wnck_window_is_maximized(d->m_window);
 }
 
+bool WindowHelper::isMostlyOnScreen(int screen) const
+{
+    if (!d->m_window) {
+        return false;
+    }
+    int x, y, width, height;
+    wnck_window_get_geometry(d->m_window, &x, &y, &width, &height);
+    const QRect windowGeometry(x, y, width, height);
+    QDesktopWidget* desktop = QApplication::desktop();
+    const QRect screenGeometry = desktop->screenGeometry(screen);
+    QRect onScreen = screenGeometry.intersected(windowGeometry);
+    int intersected = onScreen.width() * onScreen.height();
+    for (int i = 0; i < desktop->screenCount(); ++i) {
+        if (i != screen) {
+            onScreen = desktop->screenGeometry(i).intersected(windowGeometry);
+            if (onScreen.width() * onScreen.height() > intersected) {
+                return false;
+            }
+        }
+    }
+    return true;
+}
+
 void WindowHelper::close()
 {
     guint32 timestamp = QDateTime::currentDateTime().toTime_t();

=== modified file 'panel/applets/appname/windowhelper.h'
--- panel/applets/appname/windowhelper.h	2011-02-07 15:10:08 +0000
+++ panel/applets/appname/windowhelper.h	2011-04-04 16:49:30 +0000
@@ -38,6 +38,7 @@
     void setXid(uint);
 
     bool isMaximized() const;
+    bool isMostlyOnScreen(int screen) const;
 
 public Q_SLOTS:
     void close();