ctkLayoutManager was not properly cleaning the layout

clearLayout() was not deleting the layout elements (e.g. Splitters, QTabWidget)
created previously by setupLayout().
It was a problem when reparenting the views when applying the new layout in the viewport in setupLayout.
The reparenting of a QTabWidget child was setting the WA_WState_ExplicitShowHide flag on the widgets which
was preventing _q_showIfNotHidden() from setting the visibility of the child back at the next update.
Moreover the QTabWidgets and QSplitters were not destroyed until ctkLayoutManager was destroyed.
When clearing layout, it is enforced that the widgets don't redraw (QWidget::setUpdatesEnabled(false)) and the layouts don't resize (QLayout::setEnabled(false))to prevent flickering.

Libs/Widgets/Testing/Cpp/ctkLayoutManagerTest1.cpp

@@ -20,10 +20,12 @@
 
 // Qt includse
 #include <QApplication>
+#include <QDebug>
 #include <QPushButton>
 #include <QTimer>
 
 // CTK includes
+#include "ctkSliderWidget.h"
 #include "ctkSimpleLayoutManager.h"
 
 // STD includes
@@ -34,7 +36,7 @@ QString vboxLayout("<layout type=\"vertical\"><item><view/></item><item><view/><
 QString gridLayout(
 "<layout type=\"grid\">"
 " <item><view/></item>"
-" <item col=\"1\"><view/></item>"
+" <item column=\"1\"><view/></item>"
 " <item row=\"1\"><view/></item>"
 " <item row=\"1\" column=\"1\"><view/></item>"
 " <item row=\"2\" colspan=\"2\"><view/></item>"
@@ -70,11 +72,25 @@ QString nestedLayout(
 " <item><view name=\"tab3\"/></item>"
 "</layout>");
 
-class ctkPushButton: public QPushButton
+/// \ingroup Widgets
+struct ctkCachedInstanciator
+  : public ctkWidgetInstanciator
 {
-  Q_OBJECT
-public:
-  Q_INVOKABLE ctkPushButton(): QPushButton(0){}
+  int CreateWidgetCount;
+  QWidgetList CachedWidgets;
+  virtual void beginSetupLayout()
+    {
+    this->CreateWidgetCount = 0;
+    }
+  virtual QWidget* createWidget()
+    {
+    if (this->CreateWidgetCount >= this->CachedWidgets.size())
+      {
+      QWidget* widget = new ctkSliderWidget;
+      this->CachedWidgets.push_back(widget);
+      }
+    return this->CachedWidgets[this->CreateWidgetCount++];
+    }
 };
 
 //-----------------------------------------------------------------------------
@@ -94,7 +110,6 @@ int ctkLayoutManagerTest1(int argc, char * argv [] )
     return EXIT_FAILURE;
     }
 
-  //layoutManager.setViewMetaObject(QPushButton::staticMetaObject);
   ctkTemplateInstanciator<QPushButton> pButtonInstanciator;
   layoutManager.setViewInstanciator(&pButtonInstanciator);
 
@@ -123,7 +138,6 @@ int ctkLayoutManagerTest1(int argc, char * argv [] )
     }
   viewport.show();
 
-
   QWidget vbox;
   vbox.setWindowTitle("Vertical Box Layout");
   ctkSimpleLayoutManager vboxLayoutManager;
@@ -141,7 +155,7 @@ int ctkLayoutManagerTest1(int argc, char * argv [] )
   grid.show();
 
   QWidget tab;
-  grid.setWindowTitle("Tab Layout");
+  tab.setWindowTitle("Tab Layout");
   ctkSimpleLayoutManager tabLayoutManager;
   tabLayoutManager.setViewInstanciator(&pButtonInstanciator);
   tabLayoutManager.setLayout(tabLayoutDoc);
@@ -149,22 +163,109 @@ int ctkLayoutManagerTest1(int argc, char * argv [] )
   tab.show();
 
   QWidget nested;
-  grid.setWindowTitle("Nested Layout");
+  nested.setWindowTitle("Nested Layout");
   ctkSimpleLayoutManager nestedLayoutManager;
   nestedLayoutManager.setViewInstanciator(&pButtonInstanciator);
   nestedLayoutManager.setLayout(nestedLayoutDoc);
   nestedLayoutManager.setViewport(&nested);
   nested.show();
 
+  // TabToGrid
+  QWidget tabToGrid;
+  tabToGrid.setWindowTitle("Tab to Grid Layout");
+  ctkCachedInstanciator tabToGridInstanciator;
+  ctkSimpleLayoutManager tabToGridLayoutManager;
+  tabToGridLayoutManager.setViewInstanciator(&tabToGridInstanciator);
+  tabToGridLayoutManager.setLayout(tabLayoutDoc);
+  tabToGridLayoutManager.setViewport(&tabToGrid);
+  tabToGrid.show();
+
+  QTimer::singleShot(200, &app, SLOT(quit()));
+  app.exec();
+
+  tabToGridLayoutManager.setLayout(gridLayoutDoc);
+
+  QTimer::singleShot(200, &app, SLOT(quit()));
+  app.exec();
+
+  if (tabToGridInstanciator.CachedWidgets[0]->isHidden() ||
+      tabToGridInstanciator.CachedWidgets[1]->isHidden() ||
+      tabToGridInstanciator.CachedWidgets[2]->isHidden() ||
+      tabToGridInstanciator.CachedWidgets[3]->isHidden())
+    {
+    std::cout << __LINE__ << " TabToGrid: "
+              << "ctkLayoutManager::setupLayout() failed to show/hide widgets"
+              << tabToGridInstanciator.CachedWidgets[0]->isHidden() << " "
+              << tabToGridInstanciator.CachedWidgets[1]->isHidden() << " "
+              << tabToGridInstanciator.CachedWidgets[2]->isHidden() << " "
+              << tabToGridInstanciator.CachedWidgets[3]->isHidden() << std::endl;
+    return EXIT_FAILURE;
+    }
+
+  // TabToSimple
   QWidget tabToSimple;
   tabToSimple.setWindowTitle("Tab to Simple Layout");
+  ctkCachedInstanciator tabToSimpleInstanciator;
   ctkSimpleLayoutManager tabToSimpleLayoutManager;
-  tabToSimpleLayoutManager.setViewInstanciator(&pButtonInstanciator);
+  tabToSimpleLayoutManager.setViewInstanciator(&tabToSimpleInstanciator);
+  //tabToSimpleLayoutManager.setLayout(gridLayoutDoc);
   tabToSimpleLayoutManager.setLayout(tabLayoutDoc);
   tabToSimpleLayoutManager.setViewport(&tabToSimple);
   tabToSimple.show();
+
+  QTimer::singleShot(200, &app, SLOT(quit()));
+  app.exec();
+
   tabToSimpleLayoutManager.setLayout(simpleLayoutDoc);
 
+  QTimer::singleShot(200, &app, SLOT(quit()));
+  app.exec();
+
+  if (tabToSimpleInstanciator.CachedWidgets[0]->isHidden() ||
+      tabToSimpleInstanciator.CachedWidgets[1]->isVisible() ||
+      tabToSimpleInstanciator.CachedWidgets[2]->isVisible())
+    {
+    std::cout << __LINE__ << " TabToSimple: "
+              << "ctkLayoutManager::setupLayout() failed to show/hide widgets"
+              << tabToSimpleInstanciator.CachedWidgets[0]->isHidden() << " "
+              << tabToSimpleInstanciator.CachedWidgets[1]->isVisible() << " "
+              << tabToSimpleInstanciator.CachedWidgets[2]->isVisible() << std::endl;
+    return EXIT_FAILURE;
+    }
+
+  // NestedToTab
+  QWidget nestedToTab;
+  nestedToTab.setWindowTitle("Nested to Tab Layout");
+  ctkCachedInstanciator nestedToTabInstanciator;
+  ctkSimpleLayoutManager nestedToTabLayoutManager;
+  nestedToTabLayoutManager.setViewInstanciator(&nestedToTabInstanciator);
+  nestedToTabLayoutManager.setLayout(nestedLayoutDoc);
+  nestedToTabLayoutManager.setViewport(&nestedToTab);
+  nestedToTab.show();
+
+  QTimer::singleShot(200, &app, SLOT(quit()));
+  app.exec();
+
+  nestedToTabLayoutManager.setLayout(tabLayoutDoc);
+
+  QTimer::singleShot(200, &app, SLOT(quit()));
+  app.exec();
+
+  if (nestedToTabInstanciator.CachedWidgets[0]->isHidden() ||
+      nestedToTabInstanciator.CachedWidgets[1]->isVisible() ||
+      nestedToTabInstanciator.CachedWidgets[2]->isVisible() ||
+      nestedToTabInstanciator.CachedWidgets[3]->isVisible())
+    {
+    std::cout << __LINE__ << " NestedToTab: "
+              << "ctkLayoutManager::setupLayout() failed to show/hide widgets"
+              << nestedToTabInstanciator.CachedWidgets[0]->isHidden() << " "
+              << nestedToTabInstanciator.CachedWidgets[1]->isVisible() << " "
+              << nestedToTabInstanciator.CachedWidgets[2]->isVisible() << " "
+              << nestedToTabInstanciator.CachedWidgets[3]->isVisible() << std::endl;
+    return EXIT_FAILURE;
+    }
+
+
   if (argc < 2 || QString(argv[1]) != "-I" )
     {
     QTimer::singleShot(200, &app, SLOT(quit()));

Libs/Widgets/ctkLayoutManager.cpp

@@ -52,23 +52,78 @@ void ctkLayoutManagerPrivate::init()
 }
 
 //-----------------------------------------------------------------------------
+void ctkLayoutManagerPrivate::clearWidget(QWidget* widget, QLayout* parentLayout)
+{
+  if (!this->LayoutWidgets.contains(widget))
+    {
+    widget->setVisible(false);
+    if (parentLayout)
+      {
+      parentLayout->removeWidget(widget);
+      }
+    widget->setParent(this->Viewport);
+    }
+  else
+    {
+    if (widget->layout())
+      {
+      this->clearLayout(widget->layout());
+      }
+    else if (qobject_cast<QTabWidget*>(widget))
+      {
+      QTabWidget* tabWidget = qobject_cast<QTabWidget*>(widget);
+      while (tabWidget->count())
+        {
+        QWidget* page = tabWidget->widget(0);
+        this->clearWidget(page);
+        }
+      }
+    else if (qobject_cast<QSplitter*>(widget))
+      {
+      QSplitter* splitterWidget = qobject_cast<QSplitter*>(widget);
+      // Hide the splitter before removing pages. Removing pages
+      // would resize the other children if the splitter is visible.
+      // We don't want to resize the children as we are trying to clear the
+      // layout, it would set intermediate sizes to widgets.
+      // See QSplitter::childEvent
+      splitterWidget->setVisible(false);
+      while (splitterWidget->count())
+        {
+        QWidget* page = splitterWidget->widget(0);
+        this->clearWidget(page);
+        }
+      }
+    this->LayoutWidgets.remove(widget);
+    if (parentLayout)
+      {
+      parentLayout->removeWidget(widget);
+      }
+    delete widget;
+    }
+}
+
+//-----------------------------------------------------------------------------
 void ctkLayoutManagerPrivate::clearLayout(QLayout* layout)
 {
   if (!layout)
     {
     return;
     }
+  layout->setEnabled(false);
   QLayoutItem * layoutItem = 0;
-  while ((layoutItem = layout->takeAt(0)) != 0)
+  while ((layoutItem = layout->itemAt(0)) != 0)
     {
     if (layoutItem->widget())
       {
-      layoutItem->widget()->setVisible(false);
-      layout->removeWidget(layoutItem->widget());
+      this->clearWidget(layoutItem->widget(), layout);
       }
     else if (layoutItem->layout())
       {
+      /// Warning, this might delete the layouts of "custom" widgets, not just
+      /// the layouts generated by ctkLayoutManager
       this->clearLayout(layoutItem->layout());
+      layout->removeItem(layoutItem);
+      delete layoutItem;
       }
     }
   if (layout->parentWidget() && layout->parentWidget()->layout() == layout)
@@ -156,6 +211,7 @@ void ctkLayoutManager::clearLayout()
     }
   // TODO: post an event on the event queue
   d->clearLayout(d->Viewport->layout());
+  Q_ASSERT(d->LayoutWidgets.size() == 0);
 }
 
 //-----------------------------------------------------------------------------
@@ -168,6 +224,7 @@ void ctkLayoutManager::setupLayout()
     return;
     }
   d->Views.clear();
+  d->LayoutWidgets.clear();
   Q_ASSERT(!d->Viewport->layout());
   QLayoutItem* layoutItem = this->processElement(
     d->Layout.documentElement());
@@ -180,18 +237,7 @@ void ctkLayoutManager::setupLayout()
     hboxLayout->addItem(layoutItem);
     layout = hboxLayout;
     }
-  // setting the layout to the widget will reparent all the 1 level widgets.
-  // Unfortunately, it has the side effect of hiding
-  // (testAttribute(Qt::WA_WState_Hidden)) the widgets that were already having
-  // a parent (read doc for QWidget::isHidden()).
-  // we then need to manually display the widgets again. Views is not probably
-  // not the best list to use to retrieve the widgets to remove the hidden flag
-  // it seems to fit the bill for the moment so we can keep using it.
   d->Viewport->setLayout(layout);
-  foreach(QWidget* view, d->Views)
-    {
-    view->setHidden(false);
-    }
 }
 
 //-----------------------------------------------------------------------------
@@ -272,12 +318,19 @@ QLayoutItem* ctkLayoutManager::processLayoutElement(QDomElement layoutElement)
 
   QLayoutItem* layoutItem = this->layoutFromXML(layoutElement);
   QLayout* layout = layoutItem->layout();
+  QWidget* widget = layoutItem->widget();
 
   if (layout)
     {
     layout->setContentsMargins(0,0,0,0);
     layout->setSpacing(d->Spacing);
     }
+  else if (widget)
+    {
+    // mark the widget as ctkLayoutManager own widget so that it can be
+    // deleted when the layout is cleared.
+    d->LayoutWidgets << widget;
+    }
   for(QDomNode child = layoutElement.firstChild();
       !child.isNull();
       child = child.nextSibling())
@@ -349,6 +402,7 @@ void ctkLayoutManager::processItemElement(QDomElement itemElement, QLayoutItem*
 //-----------------------------------------------------------------------------
 void ctkLayoutManager::addChildItemToLayout(QDomElement itemElement, QLayoutItem* childItem, QLayoutItem* layoutItem)
 {
+  Q_D(ctkLayoutManager);
   Q_ASSERT(childItem);
   QString itemName = itemElement.attribute("name");
   if (itemName.isEmpty() && childItem->widget())
@@ -378,6 +432,7 @@ void ctkLayoutManager::addChildItemToLayout(QDomElement itemElement, QLayoutItem
     if (!childWidget)
       {
       childWidget = new QWidget();
+      d->LayoutWidgets << childWidget;
       childWidget->setLayout(childItem->layout());
       }
     if (tabWidget)

Libs/Widgets/ctkLayoutManager.h

@@ -65,7 +65,7 @@ protected:
 
   virtual void onViewportChanged();
   void clearLayout();
-  void setupLayout();
+  virtual void setupLayout();
 
   virtual void setLayout(const QDomDocument& newLayout);
   const QDomDocument layout()const;

Libs/Widgets/ctkLayoutManager_p.h

@@ -47,10 +47,12 @@ public:
 
   virtual void init();
   void clearLayout(QLayout* layout);
+  void clearWidget(QWidget* widget, QLayout* parentLayout = 0);
 
   QWidget*       Viewport;
   QDomDocument   Layout;
   QSet<QWidget*> Views;
+  QSet<QWidget*> LayoutWidgets;
   int            Spacing;
 };
 

Libs/Widgets/ctkSimpleLayoutManager.cpp

@@ -108,3 +108,18 @@ QWidget* ctkSimpleLayoutManager::viewFromXML(QDomElement viewElement)
     }
   return d->ViewInstanciator->createWidget();
 }
+
+//-----------------------------------------------------------------------------
+void ctkSimpleLayoutManager::setupLayout()
+{
+  Q_D(ctkSimpleLayoutManager);
+  if (d->ViewInstanciator)
+    {
+    d->ViewInstanciator->beginSetupLayout();
+    }
+  this->ctkLayoutManager::setupLayout();
+  if (d->ViewInstanciator)
+    {
+    d->ViewInstanciator->endSetupLayout();
+    }
+}

Libs/Widgets/ctkSimpleLayoutManager.h

@@ -31,6 +31,8 @@ class ctkSimpleLayoutManagerPrivate;
 /// \ingroup Widgets
 struct ctkWidgetInstanciator
 {
+  virtual void beginSetupLayout(){}
+  virtual void endSetupLayout(){}
   virtual QWidget* createWidget() = 0;
 };
 
@@ -62,8 +64,8 @@ public:
   ctkWidgetInstanciator* viewInstanciator()const;
 
 protected:
- virtual QWidget* viewFromXML(QDomElement viewElement);
-
+  virtual QWidget* viewFromXML(QDomElement viewElement);
+  virtual void setupLayout();
 private:
   Q_DECLARE_PRIVATE(ctkSimpleLayoutManager);
   Q_DISABLE_COPY(ctkSimpleLayoutManager);