OP-1797 avoid unparenting gadgets when splitting/unsplitting/closing a gadget (see Jira for details)

ground/openpilotgcs/src/plugins/coreplugin/uavgadgetmanager/splitterorview.cpp

@@ -34,7 +34,6 @@
 
 #include <QtCore/QDebug>
 
-
 #ifdef Q_WS_MAC
 #include <qmacstyle_mac.h>
 #endif
@@ -47,20 +46,28 @@ SplitterOrView::SplitterOrView(Core::UAVGadgetManager *uavGadgetManager, Core::I
     m_splitter(0)
 {
     m_view   = new UAVGadgetView(m_uavGadgetManager, uavGadget, this);
-    m_layout = new QStackedLayout(this);
-    m_layout->addWidget(m_view);
+    setLayout(new QStackedLayout());
+    layout()->addWidget(m_view);
+}
+
+SplitterOrView::SplitterOrView(SplitterOrView &splitterOrView, QWidget *parent) :
+    QWidget(parent),
+    m_uavGadgetManager(splitterOrView.m_uavGadgetManager),
+    m_view(splitterOrView.m_view),
+    m_splitter(splitterOrView.m_splitter)
+{
+    Q_ASSERT((m_view || m_splitter) && !(m_view && m_splitter));
+    setLayout(new QStackedLayout());
+    if (m_view) {
+        layout()->addWidget(m_view);
+    }
+    else if (m_splitter) {
+        layout()->addWidget(m_splitter);
+    }
 }
 
 SplitterOrView::~SplitterOrView()
 {
-    if (m_view) {
-        delete m_view;
-        m_view = 0;
-    }
-    if (m_splitter) {
-        delete m_splitter;
-        m_splitter = 0;
-    }
 }
 
 void SplitterOrView::mousePressEvent(QMouseEvent *e)
@@ -221,7 +228,7 @@ QSplitter *SplitterOrView::takeSplitter()
     QSplitter *oldSplitter = m_splitter;
 
     if (m_splitter) {
-        m_layout->removeWidget(m_splitter);
+        layout()->removeWidget(m_splitter);
     }
     m_splitter = 0;
     return oldSplitter;
@@ -232,7 +239,7 @@ UAVGadgetView *SplitterOrView::takeView()
     UAVGadgetView *oldView = m_view;
 
     if (m_view) {
-        m_layout->removeWidget(m_view);
+        layout()->removeWidget(m_splitter);
     }
     m_view = 0;
     return oldView;
@@ -255,35 +262,6 @@ QList<IUAVGadget *> SplitterOrView::gadgets()
     return g;
 }
 
-void SplitterOrView::split(Qt::Orientation orientation)
-{
-    Q_ASSERT(m_view);
-    Q_ASSERT(!m_splitter);
-    m_splitter = new MiniSplitter(this);
-    m_splitter->setOrientation(orientation);
-    connect(m_splitter, SIGNAL(splitterMoved(int, int)), this, SLOT(onSplitterMoved(int, int)));
-    m_layout->addWidget(m_splitter);
-    Core::IUAVGadget *ourGadget = m_view->gadget();
-
-    if (ourGadget) {
-        // Give our gadget to the new left or top SplitterOrView.
-        m_view->removeGadget();
-        m_splitter->addWidget(new SplitterOrView(m_uavGadgetManager, ourGadget));
-        m_splitter->addWidget(new SplitterOrView(m_uavGadgetManager));
-    } else {
-        m_splitter->addWidget(new SplitterOrView(m_uavGadgetManager));
-        m_splitter->addWidget(new SplitterOrView(m_uavGadgetManager));
-    }
-
-    m_layout->setCurrentWidget(m_splitter);
-
-    if (m_view) {
-        m_uavGadgetManager->emptyView(m_view);
-        delete m_view;
-        m_view = 0;
-    }
-}
-
 void SplitterOrView::onSplitterMoved(int pos, int index)
 {
     Q_UNUSED(pos);
@@ -292,66 +270,108 @@ void SplitterOrView::onSplitterMoved(int pos, int index)
     m_sizes = m_splitter->sizes();
 }
 
-void SplitterOrView::unsplitAll(IUAVGadget *currentGadget)
+void SplitterOrView::split(Qt::Orientation orientation)
 {
-    Q_ASSERT(m_splitter);
-    Q_ASSERT(!m_view);
-    m_splitter->hide();
-    m_layout->removeWidget(m_splitter); // workaround Qt bug
-    unsplitAll_helper();
-    delete m_splitter;
-    m_splitter = 0;
+    Q_ASSERT(m_view);
+    Q_ASSERT(!m_splitter);
 
-    m_view     = new UAVGadgetView(m_uavGadgetManager, currentGadget, this);
-    m_layout->addWidget(m_view);
+    MiniSplitter *splitter = new MiniSplitter(this);
+    splitter->setOrientation(orientation);
+    layout()->addWidget(splitter);
+
+    // [OP-1586] make sure that the view never becomes parent less otherwise a rendering bug happens
+    // in osgearth QML views (not all kind of scenes are affected but those containing terrain are)
+    // Making the view parent less will destroy the OpenGL context used by the QQuickFramebufferObject used OSGViewport
+    // A new OpenGL context will be created but for some reason, osgearth does not switch to it gracefully.
+    // Enabling the stats overlay (by pressing the 's' key in the view) will restore proper rendering (?).
+    // Note : avoiding to make the view parent less is a workaround... the real cause of the rendering bug needs to be
+    // understood and fixed (the same workaround is also need in unsplit and unsplitAll)
+    // Important : the changes also apparently make splitting and un-splitting more reactive and less jumpy!
+
+    // Give our view to the new left or top SplitterOrView.
+    splitter->addWidget(new SplitterOrView(*this, splitter));
+    splitter->addWidget(new SplitterOrView(m_uavGadgetManager));
+
+    m_view = 0;
+    m_splitter = splitter;
+
+    connect(m_splitter, SIGNAL(splitterMoved(int, int)), this, SLOT(onSplitterMoved(int, int)));
 }
 
-void SplitterOrView::unsplitAll_helper()
-{
-    if (m_view) {
-        m_uavGadgetManager->emptyView(m_view);
-    }
-    if (m_splitter) {
-        for (int i = 0; i < m_splitter->count(); ++i) {
-            if (SplitterOrView * splitterOrView = qobject_cast<SplitterOrView *>(m_splitter->widget(i))) {
-                splitterOrView->unsplitAll_helper();
-            }
-        }
-    }
-}
-
-void SplitterOrView::unsplit()
+void SplitterOrView::unsplit(IUAVGadget *gadget)
 {
     if (!m_splitter) {
         return;
     }
-    Q_ASSERT(m_splitter->count() == 1);
-    SplitterOrView *childSplitterOrView = qobject_cast<SplitterOrView *>(m_splitter->widget(0));
-    QSplitter *oldSplitter = m_splitter;
-    m_splitter = 0;
-
-    if (childSplitterOrView->isSplitter()) {
-        Q_ASSERT(childSplitterOrView->view() == 0);
-        m_splitter = childSplitterOrView->takeSplitter();
-        m_layout->addWidget(m_splitter);
-        m_layout->setCurrentWidget(m_splitter);
-    } else {
-        UAVGadgetView *childView = childSplitterOrView->view();
-        Q_ASSERT(childView);
-        if (m_view) {
-            if (IUAVGadget * e = childView->gadget()) {
-                childView->removeGadget();
-                m_view->setGadget(e);
-            }
-            m_uavGadgetManager->emptyView(childView);
-        } else {
-            m_view = childSplitterOrView->takeView();
-            m_layout->addWidget(m_view);
-        }
-        m_layout->setCurrentWidget(m_view);
+    SplitterOrView *view = findView(gadget);
+    if (!view || view == this) {
+        return;
+    }
+
+    // find the other gadgets
+    // TODO handle case where m_splitter->count() > 2
+    SplitterOrView *splitterOrView = NULL;
+    for (int i = 0; i < m_splitter->count(); ++i) {
+        splitterOrView = qobject_cast<SplitterOrView *>(m_splitter->widget(i));
+        if (splitterOrView && (splitterOrView != view)) {
+            break;
+        }
+    }
+    if (splitterOrView) {
+        if (splitterOrView->isView()) {
+            layout()->addWidget(splitterOrView->m_view);
+        }
+        else {
+            layout()->addWidget(splitterOrView->m_splitter);
+        }
+        layout()->removeWidget(m_splitter);
+
+        m_uavGadgetManager->emptyView(view->m_view);
+        delete view;
+        delete m_splitter;
+
+        m_view = splitterOrView->m_view;
+        m_splitter = splitterOrView->m_splitter;
+    }
+}
+
+void SplitterOrView::unsplitAll(Core::IUAVGadget *gadget)
+{
+    Q_ASSERT(m_splitter);
+    Q_ASSERT(!m_view);
+
+    SplitterOrView *splitterOrView = findView(gadget);
+    if (!splitterOrView || splitterOrView == this) {
+        return;
+    }
+
+    // first re-parent the gadget (see split for an explanation)
+    m_view = splitterOrView->m_view;
+    layout()->addWidget(m_view);
+    layout()->removeWidget(m_splitter);
+    // make sure the old m_view is not emptied...
+    splitterOrView->m_view = NULL;
+
+    // cleanup
+    unsplitAll_helper(m_uavGadgetManager, m_splitter);
+
+    delete m_splitter;
+    m_splitter = 0;
+}
+
+void SplitterOrView::unsplitAll_helper(UAVGadgetManager *uavGadgetManager, QSplitter *splitter)
+{
+    for (int i = 0; i < splitter->count(); ++i) {
+        if (SplitterOrView * splitterOrView = qobject_cast<SplitterOrView *>(splitter->widget(i))) {
+            if (splitterOrView->m_view) {
+                uavGadgetManager->emptyView(splitterOrView->m_view);
+            }
+            if (splitterOrView->m_splitter) {
+                unsplitAll_helper(uavGadgetManager,splitterOrView->m_splitter);
+            }
+            delete splitterOrView;
+        }
     }
-    delete oldSplitter;
-    m_uavGadgetManager->setCurrentGadget(findFirstView()->gadget());
 }
 
 void SplitterOrView::saveState(QSettings *qSettings) const

ground/openpilotgcs/src/plugins/coreplugin/uavgadgetmanager/splitterorview.h

@@ -43,7 +43,11 @@ public:
     ~SplitterOrView();
 
     void split(Qt::Orientation orientation);
-    void unsplit();
+
+    void unsplit(IUAVGadget *gadget);
+
+    // un-split all and keep only the specified gadget
+    void unsplitAll(IUAVGadget *gadget);
 
     inline bool isView() const
     {
@@ -95,10 +99,7 @@ public:
     }
     QSize minimumSizeHint() const;
 
-    void unsplitAll(IUAVGadget *currentGadget);
-
 protected:
-// void paintEvent(QPaintEvent *);
     void mousePressEvent(QMouseEvent *e);
 
 private slots:
@@ -106,19 +107,19 @@ private slots:
     void onSplitterMoved(int pos, int index);
 
 private:
-    void unsplitAll_helper();
+    // private "copy" constructor
+    SplitterOrView(SplitterOrView &splitterOrView, QWidget *parent);
+
+    static void unsplitAll_helper(UAVGadgetManager *uavGadgetManager, QSplitter *splitter);
     SplitterOrView *findNextView_helper(SplitterOrView *view, bool *found);
 
     // The gadget manager that controls us.
     QPointer<UAVGadgetManager> m_uavGadgetManager;
 
-    // Our layout, we use stacked so we can change stuff without visual artifacts (I think...)
-    QPointer<QStackedLayout> m_layout;
-
     // Our view, if we are a view (showing 1 gadget) and not a splitter.
     QPointer<UAVGadgetView> m_view;
 
-    // Out splitter, if we are a splitter.
+    // Our splitter, if we are a splitter.
     QPointer<QSplitter> m_splitter;
 
     // The splitter sizes. We keep our own copy of these, since after loading they can't realiably be retrieved.

ground/openpilotgcs/src/plugins/coreplugin/uavgadgetmanager/uavgadgetmanager.cpp

@@ -108,7 +108,7 @@ UAVGadgetManager::UAVGadgetManager(ICore *core, QString name, QIcon icon, int pr
             this, SLOT(modeChanged(Core::IMode *)));
 
     // other setup
-    m_splitterOrView = new SplitterOrView(this, 0);
+    m_splitterOrView = new SplitterOrView(this);
 
     // SplitterOrView with 0 as gadget calls our setCurrentGadget, which relies on currentSplitterOrView(),
     // which needs our m_splitterorView to be set, which isn't set yet at that time.
@@ -209,10 +209,10 @@ void UAVGadgetManager::emptyView(Core::Internal::UAVGadgetView *view)
     }
 
     IUAVGadget *uavGadget = view->gadget();
-// emit uavGadgetAboutToClose(uavGadget);
+    //emit uavGadgetAboutToClose(uavGadget);
     removeGadget(uavGadget);
     view->removeGadget();
-// emit uavGadgetsClosed(uavGadgets);
+    //emit uavGadgetsClosed(uavGadgets);
 }
 
 
@@ -221,26 +221,21 @@ void UAVGadgetManager::closeView(Core::Internal::UAVGadgetView *view)
     if (!view) {
         return;
     }
-    SplitterOrView *splitterOrView = m_splitterOrView->findView(view);
-    Q_ASSERT(splitterOrView);
-    Q_ASSERT(splitterOrView->view() == view);
-    if (splitterOrView == m_splitterOrView) {
-        return;
-    }
 
     IUAVGadget *gadget = view->gadget();
-    emptyView(view);
+
+    // find SplitterOrView splitter that contains the view to delete
+    SplitterOrView *splitter     = m_splitterOrView->findSplitter(gadget);
+    if (!splitter) {
+        return;
+    }
+    Q_ASSERT(splitter->isSplitter() == true);
+
+    splitter->unsplit(gadget);
+
     UAVGadgetInstanceManager *im = ICore::instance()->uavGadgetInstanceManager();
     im->removeGadget(gadget);
 
-    SplitterOrView *splitter     = m_splitterOrView->findSplitter(splitterOrView);
-    Q_ASSERT(splitterOrView->hasGadget() == false);
-    Q_ASSERT(splitter->isSplitter() == true);
-    splitterOrView->hide();
-    delete splitterOrView;
-
-    splitter->unsplit();
-
     SplitterOrView *newCurrent = splitter->findFirstView();
     Q_ASSERT(newCurrent);
     if (newCurrent) {
@@ -254,8 +249,7 @@ void UAVGadgetManager::addGadgetToContext(IUAVGadget *gadget)
         return;
     }
     m_core->addContextObject(gadget);
-
-// emit uavGadgetOpened(uavGadget);
+    //emit uavGadgetOpened(uavGadget);
 }
 
 void UAVGadgetManager::removeGadget(IUAVGadget *gadget)