Merge lp:~alan-griffiths/miral/tidy-SurfaceObserver-part1 into lp:miral

[Alan Griffiths]

[miral-qt] Tidy up dead and purposeless code in SurfaceObserver

[lp:~alan-griffiths/miral/tidy-SurfaceObserver-part1]

422. By Alan Griffiths on 2016-10-27

Delete pointless code

[Gerry Boland (gerboland)]

-void SurfaceObserver::notifySurfaceModifications(const mir::shell::SurfaceSpecification &modifications)
I'm only happy to proceed with this removal when we have the miral-based equivalent to this ready to land. Otherwise it will upset Daniels work integrating miral-qt with unity8.

[Alan Griffiths (alan-griffiths)]

> -void SurfaceObserver::notifySurfaceModifications(const
> mir::shell::SurfaceSpecification &modifications)
> I'm only happy to proceed with this removal when we have the miral-based
> equivalent to this ready to land. Otherwise it will upset Daniels work
> integrating miral-qt with unity8.

It is called by Daniel's code? Where's that?

[Daniel d'Andrada (dandrader)]

In qtmir, SurfaceObserver::notifySurfaceModifications is called by src/platforms/mirserver/mirwindowmanager.cpp

There's no mirwindowmanager.cpp in miral-qt anymore, so this code is already broken here.

Question now is when did it get removed and what replaces it.

review: Needs Information

[Alan Griffiths (alan-griffiths)]

> In qtmir, SurfaceObserver::notifySurfaceModifications is called by
> src/platforms/mirserver/mirwindowmanager.cpp
>
> There's no mirwindowmanager.cpp in miral-qt anymore, so this code is already
> broken here.
>
> Question now is when did it get removed and what replaces it.

Not sure when it got removed, but it looks like these notifications should be generated from WindowManagementPolicy::handle_modify_window() invoking WindowModelNotifier.

In any case, I think the problem is pre-existing and shouldn't block this MP. I'll take the action of creating new wiring to Q_EMIT the signals this generated.

[Daniel d'Andrada (dandrader)]

On 31/10/2016 08:07, Alan Griffiths wrote:
>> In qtmir, SurfaceObserver::notifySurfaceModifications is called by
>> src/platforms/mirserver/mirwindowmanager.cpp
>>
>> There's no mirwindowmanager.cpp in miral-qt anymore, so this code is already
>> broken here.
>>
>> Question now is when did it get removed and what replaces it.
> Not sure when it got removed, but it looks like these notifications should be generated from WindowManagementPolicy::handle_modify_window() invoking WindowModelNotifier.
>
> In any case, I think the problem is pre-existing and shouldn't block this MP. I'll take the action of creating new wiring to Q_EMIT the signals this generated.

Indeed. Thanks!

Unmerged revisions

422. By Alan Griffiths on 2016-10-27

Delete pointless code

421. By Alan Griffiths on 2016-10-27

Delete dead code

=== modified file 'miral-qt/src/modules/Unity/Application/mirsurface.cpp'
--- miral-qt/src/modules/Unity/Application/mirsurface.cpp	2016-10-12 15:49:08 +0000
+++ miral-qt/src/modules/Unity/Application/mirsurface.cpp	2016-10-27 16:26:37 +0000
@@ -89,7 +89,6 @@
 {
     DEBUG_MSG << "()";
 
-    SurfaceObserver::registerObserverForSurface(m_surfaceObserver.get(), m_surface.get());
     m_surface->add_observer(m_surfaceObserver);
 
     //m_shellChrome = creationHints.shellChrome; TODO - where will this come from now?

=== modified file 'miral-qt/src/platforms/mirserver/surfaceobserver.cpp'
--- miral-qt/src/platforms/mirserver/surfaceobserver.cpp	2016-09-23 15:56:53 +0000
+++ miral-qt/src/platforms/mirserver/surfaceobserver.cpp	2016-10-27 16:26:37 +0000
@@ -29,25 +29,6 @@
 #include <mir/shell/surface_specification.h>
 
 
-namespace {
-
-QRect calculateBoundingRect(const std::vector<mir::geometry::Rectangle> &rectVector)
-{
-    QRect boundingRect;
-    for (auto mirRect : rectVector) {
-        boundingRect |= QRect(mirRect.top_left.x.as_int(),
-                mirRect.top_left.y.as_int(),
-                mirRect.size.width.as_int(),
-                mirRect.size.height.as_int());
-    }
-    return boundingRect;
-}
-
-} // anonymous namespace
-
-QHash<const mir::scene::Surface*, SurfaceObserver*> SurfaceObserver::m_surfaceToObserverMap;
-QMutex SurfaceObserver::mutex;
-
 SurfaceObserver::SurfaceObserver()
     : m_listener(nullptr)
     , m_framesPosted(false)
@@ -99,15 +80,6 @@
 
 SurfaceObserver::~SurfaceObserver()
 {
-    QMutexLocker locker(&mutex);
-    QMutableHashIterator<const mir::scene::Surface*, SurfaceObserver*> i(m_surfaceToObserverMap);
-    while (i.hasNext()) {
-        i.next();
-        if (i.value() == this) {
-            i.remove();
-            return;
-        }
-    }
 }
 
 void SurfaceObserver::setListener(QObject *listener)
@@ -154,53 +126,6 @@
     Q_EMIT resized(QSize(size.width.as_int(), size.height.as_int()));
 }
 
-void SurfaceObserver::notifySurfaceModifications(const mir::shell::SurfaceSpecification &modifications)
-{
-    if (modifications.min_width.is_set()) {
-        Q_EMIT minimumWidthChanged(modifications.min_width.value().as_int());
-    }
-    if (modifications.min_height.is_set()) {
-        Q_EMIT minimumHeightChanged(modifications.min_height.value().as_int());
-    }
-    if (modifications.max_width.is_set()) {
-        Q_EMIT maximumWidthChanged(modifications.max_width.value().as_int());
-    }
-    if (modifications.max_height.is_set()) {
-        Q_EMIT maximumHeightChanged(modifications.max_height.value().as_int());
-    }
-    if (modifications.width_inc.is_set()) {
-        Q_EMIT widthIncrementChanged(modifications.width_inc.value().as_int());
-    }
-    if (modifications.height_inc.is_set()) {
-        Q_EMIT heightIncrementChanged(modifications.height_inc.value().as_int());
-    }
-    if (modifications.shell_chrome.is_set()) {
-        Q_EMIT shellChromeChanged(modifications.shell_chrome.value());
-    }
-    if (modifications.input_shape.is_set()) {
-        QRect qRect = calculateBoundingRect(modifications.input_shape.value());
-        Q_EMIT inputBoundsChanged(qRect);
-    }
-    if (modifications.confine_pointer.is_set()) {
-        Q_EMIT confinesMousePointerChanged(modifications.confine_pointer.value() == mir_pointer_confined_to_surface);
-    }
-}
-
-SurfaceObserver *SurfaceObserver::observerForSurface(const mir::scene::Surface *surface)
-{
-    if (m_surfaceToObserverMap.contains(surface)) {
-        return m_surfaceToObserverMap.value(surface);
-    } else {
-        return nullptr;
-    }
-}
-
-void SurfaceObserver::registerObserverForSurface(SurfaceObserver *observer, const mir::scene::Surface *surface)
-{
-    QMutexLocker locker(&mutex);
-    m_surfaceToObserverMap[surface] = observer;
-}
-
 void SurfaceObserver::cursor_image_set_to(const mir::graphics::CursorImage &cursorImage)
 {
     QCursor qcursor = createQCursorFromMirCursorImage(cursorImage);

=== modified file 'miral-qt/src/platforms/mirserver/surfaceobserver.h'
--- miral-qt/src/platforms/mirserver/surfaceobserver.h	2016-09-23 15:56:53 +0000
+++ miral-qt/src/platforms/mirserver/surfaceobserver.h	2016-10-27 16:26:37 +0000
@@ -27,15 +27,6 @@
 #include <mir/scene/surface_observer.h>
 #include <mir/version.h>
 
-namespace mir {
-    namespace scene {
-        class Surface;
-    }
-    namespace shell {
-        struct SurfaceSpecification;
-    }
-}
-
 class SurfaceObserver : public QObject, public mir::scene::SurfaceObserver
 {
     Q_OBJECT
@@ -69,12 +60,6 @@
     void placed_relative(mir::geometry::Rectangle const& placement) override;
 #endif
 
-    void notifySurfaceModifications(const mir::shell::SurfaceSpecification&);
-
-    static SurfaceObserver *observerForSurface(const mir::scene::Surface *surface);
-    static void registerObserverForSurface(SurfaceObserver *observer, const mir::scene::Surface *surface);
-    static QMutex mutex;
-
 Q_SIGNALS:
     void attributeChanged(const MirSurfaceAttrib attribute, const int value);
     void framesPosted();
@@ -98,7 +83,6 @@
     QObject *m_listener;
     bool m_framesPosted;
     QMap<QByteArray, Qt::CursorShape> m_cursorNameToShape;
-    static QHash<const mir::scene::Surface*, SurfaceObserver*> m_surfaceToObserverMap;
 };
 
 #endif