Merge lp:~aacid/unity-mir/improve_input_area_handling into lp:unity-mir

[Albert Astals Cid]

Improve InputArea<->MirSurface<->Mir::Surface mapping

This gets the lower row of buttons in http://bazaar.launchpad.net/~gerboland/+junk/qml-demo-shell/ to work

Rationale is, before this patch it's input area had it's own MirSurface but some of the MirSurfaces shared the same mir::surface, so they ended up overwriting eachother input_regions. Now there's just a MirSurface per surface that is shared with the various InputAreas and thus the input_regions nicely cooperate

[lp:~aacid/unity-mir/improve_input_area_handling]

26. By Albert Astals Cid on 2013-07-30

Take into account m_surface may still not be there when setting enabled

[Gerry Boland (gerboland)]

Nice fixing, thank you!

review: Approve

=== modified file 'src/modules/Unity/SurfaceManager/inputarea.cpp'
--- src/modules/Unity/SurfaceManager/inputarea.cpp	2013-07-19 17:29:44 +0000
+++ src/modules/Unity/SurfaceManager/inputarea.cpp	2013-07-30 15:59:58 +0000
@@ -7,6 +7,7 @@
 // local
 #include "inputarea.h"
 #include "mirsurface.h"
+#include "mirsurfacemanager.h"
 #include "qmirserverapplication.h"
 #include "shellserverconfiguration.h"
 #include "surfacesource.h"
@@ -32,8 +33,10 @@
 
 void InputArea::surfaceReady(std::shared_ptr<mir::shell::Surface> const& surface)
 {
-    m_surface = new MirSurface(surface);
-    m_surface->installInputArea(this);
+    m_surface = MirSurfaceManager::singleton()->surfaceFor(surface);
+    if (m_enabled) {
+        m_surface->installInputArea(this);
+    }
 }
 
 bool InputArea::enabled() const
@@ -45,7 +48,9 @@
 {
     if (enabled != m_enabled) {
         m_enabled = enabled;
-        m_surface->enableInputArea(this, enabled);
+        if (m_surface) {
+            m_surface->enableInputArea(this, enabled);
+        }
         Q_EMIT enabledChanged();
     }
 }
@@ -125,7 +130,6 @@
     m_mirInputArea.size.height = Height{rect.height()};
 
     if (m_surface) {
-        m_surface->removeInputArea(this);
         m_surface->installInputArea(this);
     }
 }

=== modified file 'src/modules/Unity/SurfaceManager/mirsurface.cpp'
--- src/modules/Unity/SurfaceManager/mirsurface.cpp	2013-07-24 15:13:01 +0000
+++ src/modules/Unity/SurfaceManager/mirsurface.cpp	2013-07-30 15:59:58 +0000
@@ -16,7 +16,6 @@
 
 MirSurface::~MirSurface()
 {
-    m_mirInputAreas.clear();
 //    Q_FOREACH(auto inputArea, m_inputAreas) {
 //        delete inputArea;
 //    }
@@ -138,46 +137,38 @@
         LOG("MirSurface::installInputArea - surface does not support input");
     }
 
-    m_inputAreas.push_back(area);
-    m_mirInputAreas.push_back(area->m_mirInputArea);
-
+    m_inputAreas.insert(area);
+    updateMirInputRegion();
+}
+
+bool MirSurface::removeInputArea(const InputArea* area)
+{
+    const bool res = m_inputAreas.remove(area);
+    updateMirInputRegion();
+    return res;
+}
+
+bool MirSurface::enableInputArea(const InputArea* area, bool enable)
+{
+    bool res;
+    if (enable) {
+        m_inputAreas.insert(area);
+        res = true;
+    } else {
+        res = m_inputAreas.remove(area);
+    }
+    updateMirInputRegion();
+    return res;
+}
+
+void MirSurface::updateMirInputRegion()
+{
     /* WARNING: by default, a surface has an input region covering the whole surface.
        Once the surface input region is set, the default will *not* be restored when
        all input regions are removed/disabled */
-    m_surface->set_input_region(m_mirInputAreas);
-}
-
-bool MirSurface::removeInputArea(const InputArea* area)
-{
-    int index = m_inputAreas.indexOf(area);
-    if (index < 0) return false;
-
-    disableMirInputArea(area->m_mirInputArea);
-
-    m_inputAreas.remove(index);
-    return true;
-}
-
-bool MirSurface::enableInputArea(const InputArea* area, bool enable)
-{
-    if (m_inputAreas.contains(area)) return false;
-
-    if (enable) {
-        m_mirInputAreas.push_back(area->m_mirInputArea);
-        return true;
-    } else {
-        return disableMirInputArea(area->m_mirInputArea);
-    }
-}
-
-bool MirSurface::disableMirInputArea(const mir::geometry::Rectangle& rect)
-{
-    for (auto it = m_mirInputAreas.begin(); it != m_mirInputAreas.end(); it++) {
-        if (it->size == rect.size && it->top_left == rect.top_left) {
-            m_mirInputAreas.erase(it);
-            return true;
-        }
-    }
-    return false;
-}
-
+    std::vector<mir::geometry::Rectangle> mirInputAreas;
+    for (auto const& area : m_inputAreas) {
+        mirInputAreas.push_back(area->m_mirInputArea);
+    }
+    m_surface->set_input_region(mirInputAreas);
+}

=== modified file 'src/modules/Unity/SurfaceManager/mirsurface.h'
--- src/modules/Unity/SurfaceManager/mirsurface.h	2013-07-25 13:51:47 +0000
+++ src/modules/Unity/SurfaceManager/mirsurface.h	2013-07-30 15:59:58 +0000
@@ -3,7 +3,7 @@
 
 // Qt
 #include <QQuickItem>
-#include <QVector>
+#include <QSet>
 
 // mir
 #include <mir/shell/surface.h>
@@ -85,16 +85,17 @@
     std::shared_ptr<mir::shell::Surface> m_surface;
 
 private:
-    // start of methods used by InputFilter
+    // start of methods used by InputArea
     void installInputArea(const InputArea* area);
     bool removeInputArea(const InputArea* area);
     bool enableInputArea(const InputArea* area, bool enable = true);
-    // end of methods used by InputFilter
+    // end of methods used by InputArea
+
+    void updateMirInputRegion();
 
     bool disableMirInputArea(const mir::geometry::Rectangle& rect);
 
-    QVector<const InputArea*> m_inputAreas;
-    std::vector<mir::geometry::Rectangle> m_mirInputAreas;
+    QSet<const InputArea*> m_inputAreas;
 
     bool m_visible:true; //FIXME(greyback) state should be in Mir::Shell::Surface, not here
 

=== modified file 'src/modules/Unity/SurfaceManager/mirsurfacemanager.cpp'
--- src/modules/Unity/SurfaceManager/mirsurfacemanager.cpp	2013-07-24 17:22:08 +0000
+++ src/modules/Unity/SurfaceManager/mirsurfacemanager.cpp	2013-07-30 15:59:58 +0000
@@ -14,8 +14,19 @@
 
 namespace msh = mir::shell;
 
+MirSurfaceManager *MirSurfaceManager::the_surface_manager = nullptr;
+
+MirSurfaceManager* MirSurfaceManager::singleton()
+{
+    if (!the_surface_manager) {
+        the_surface_manager = new MirSurfaceManager();
+    }
+    return the_surface_manager;
+}
+
 MirSurfaceManager::MirSurfaceManager(QObject *parent)
     : QObject(parent)
+    , m_shellSurface(nullptr)
 {
     DLOG("MirSurfaceManager::MirSurfaceManager (this=%p)", this);
 
@@ -42,12 +53,29 @@
     m_surfaces.clear();
 }
 
+MirSurface *MirSurfaceManager::surfaceFor(std::shared_ptr<mir::shell::Surface> surface, QQuickItem *parent)
+{
+    auto it = m_surfaces.find(surface.get());
+    if (it != m_surfaces.end()) {
+        return *it;
+    }
+    if (!m_shellSurface) {
+        // If sessionCreatedSurface was not called assume it is the shell surface
+        m_shellSurface = new MirSurface(surface, parent);
+        m_surfaces.insert(surface.get(), m_shellSurface);
+        return m_shellSurface;
+    } else {
+        DLOG("MirSurfaceManager::surfaceFor (this=%p) with surface name '%s' asking for a surface that was not created and is not the shell surface", this, surface->name().c_str());
+        return nullptr;
+    }
+}
+
 void MirSurfaceManager::sessionCreatedSurface(mir::shell::ApplicationSession const*, std::shared_ptr<mir::shell::Surface> const& surface)
 {
     DLOG("MirSurfaceManager::sessionCreatedSurface (this=%p) with surface name '%s'", this, surface->name().c_str());
     auto qmlSurface = new MirSurface(surface);
 
-    m_surfaces.prepend(qmlSurface);
+    m_surfaces.insert(surface.get(), qmlSurface);
     Q_EMIT surfaceCreated(qmlSurface);
 }
 
@@ -55,12 +83,11 @@
 {
     DLOG("MirSurfaceManager::sessionDestroyingSurface (this=%p) with surface name '%s'", this, surface->name().c_str());
 
-    Q_FOREACH(MirSurface* s, m_surfaces) {
-        if (s->m_surface == surface) {
-            m_surfaces.removeOne(s);
-            Q_EMIT surfaceDestroyed(s);
-            return;
-        }
+    auto it = m_surfaces.find(surface.get());
+    if (it != m_surfaces.end()) {
+        m_surfaces.erase(it);
+        Q_EMIT surfaceDestroyed(*it);
+        delete *it;
     }
 
     DLOG("MirSurfaceManager::sessionDestroyingSurface: unable to find MirSurface corresponding to surface '%s'", surface->name().c_str());

=== modified file 'src/modules/Unity/SurfaceManager/mirsurfacemanager.h'
--- src/modules/Unity/SurfaceManager/mirsurfacemanager.h	2013-07-24 17:22:08 +0000
+++ src/modules/Unity/SurfaceManager/mirsurfacemanager.h	2013-07-30 15:59:58 +0000
@@ -17,11 +17,15 @@
     Q_OBJECT
 
 public:
+    static MirSurfaceManager* singleton();
+
     MirSurfaceManager(QObject *parent = 0);
     ~MirSurfaceManager();
 
     MirSurface* shellSurface() const;
 
+    MirSurface *surfaceFor(std::shared_ptr<mir::shell::Surface> surface, QQuickItem *parent = 0);
+
 Q_SIGNALS:
     void surfaceCreated(MirSurface* surface);
     void surfaceDestroyed(MirSurface* surface);
@@ -33,8 +37,10 @@
     void sessionDestroyingSurface(mir::shell::ApplicationSession const*, std::shared_ptr<mir::shell::Surface> const&);
 
 private:
-    QList<MirSurface*> m_surfaces;
+    QHash<mir::shell::Surface *, MirSurface *> m_surfaces;
+    MirSurface* m_shellSurface;
     ShellServerConfiguration* m_mirServer;
+    static MirSurfaceManager *the_surface_manager;
 };
 
 #endif // MIR_SURFACE_MANAGER_H

=== modified file 'src/modules/Unity/SurfaceManager/plugin.cpp'
--- src/modules/Unity/SurfaceManager/plugin.cpp	2013-07-19 17:29:44 +0000
+++ src/modules/Unity/SurfaceManager/plugin.cpp	2013-07-30 15:59:58 +0000
@@ -8,7 +8,7 @@
   Q_UNUSED(engine);
   Q_UNUSED(scriptEngine);
   DLOG("surfaceManagerSingleton (engine=%p, scriptEngine=%p)", engine, scriptEngine);
-  return new MirSurfaceManager();
+  return MirSurfaceManager::singleton();
 }
 
 class UbuntuApplicationPlugin : public QQmlExtensionPlugin {