Debug|Refactor|UI|Client: Investigating issue when deleting widgets

Added GuiWidgetPrivate<> template to act as the base class for GUI widgets'
private implementations.

However, it appears that the widgets are not being deinitialized properly
under certain circumstances...

doomsday/client/client.pro

@@ -441,7 +441,8 @@ INCLUDEPATH += \
 
 HEADERS += \
     $$DENG_API_HEADERS \
-    $$DENG_HEADERS
+    $$DENG_HEADERS \
+    include/ui/widgets/guiwidgetprivate.h
 
 # Platform-specific sources.
 win32 {

doomsday/client/include/ui/widgets/blurwidget.h

@@ -30,6 +30,7 @@ class BlurWidget : public GuiWidget
 {
 public:
     BlurWidget(de::String const &name = "");
+    ~BlurWidget();
 };
 
 #endif // DENG_CLIENT_BLURWIDGET_H

doomsday/client/include/ui/widgets/consolewidget.h

@@ -78,10 +78,6 @@ public slots:
 protected slots:
     void logContentHeightIncreased(int delta);
 
-protected:
-    void glInit();
-    void glDeinit();
-
 private:
     DENG2_PRIVATE(d)
 };

doomsday/client/include/ui/widgets/guiwidget.h

@@ -26,6 +26,7 @@
 
 #include "../uidefs.h"
 #include "ui/style.h"
+#include "guiwidgetprivate.h"
 
 class GuiRootWidget;
 class BlurWidget;
@@ -68,6 +69,10 @@ class BlurWidget;
  *
  * QObject is a base class for the signals and slots capabilities.
  *
+ * @note The destructor of a derived class is required to first call
+ * GuiWidget::deinitialize(). This will ensure all the GL resources of the
+ * involved widget classes are released as appropriate.
+ *
  * @ingroup gui
  */
 class GuiWidget : public QObject, public de::Widget
@@ -241,6 +246,8 @@ class GuiWidget : public QObject, public de::Widget
 
     bool geometryRequested() const;
 
+    bool isInitialized() const;
+
 protected:
     /**
      * Called by GuiWidget::update() the first time an update is being carried
@@ -252,10 +259,11 @@ class GuiWidget : public QObject, public de::Widget
     virtual void glInit();
 
     /**
-     * Called by GuiWidget before the widget is destroyed. This is the
-     * appropriate place for the widget to release its GL resources. If one
-     * waits until the widget's destructor to do so, it may already have lost
-     * access to some required information (such as the root widget).
+     * Called from deinitialize(). Deinitialization must occur before the
+     * widget is destroyed. This is the appropriate place for the widget to
+     * release its GL resources. If one waits until the widget's destructor to
+     * do so, it may already have lost access to some required information
+     * (such as the root widget, or derived classes' private instances).
      */
     virtual void glDeinit();
 

doomsday/client/include/ui/widgets/guiwidgetprivate.h

@@ -0,0 +1,71 @@
+#ifndef DENG_CLIENT_GUIWIDGETPRIVATE_H
+#define DENG_CLIENT_GUIWIDGETPRIVATE_H
+
+#include <de/libdeng2.h>
+#include "guirootwidget.h"
+
+/**
+ * Base class for GuiWidget-derived widgets' private implementation. Provides
+ * easy access to the root widget and shared GL resources. This should be used
+ * as the base class for private implementations if GL resources are being
+ * used (i.e., glInit() and glDeinit() are being called).
+ *
+ * The destructor of classes derived from GuiWidgetPrivate must make the
+ * following call: <pre>self.deserialize()</pre>
+ *
+ * Use DENG_GUI_PIMPL() instead of the DENG2_PIMPL() macro.
+ */
+template <typename PublicType>
+class GuiWidgetPrivate : public de::Private<PublicType>
+{
+public:
+    typedef GuiWidgetPrivate<PublicType> Base; // shadows de::Private<>::Base
+
+public:
+    GuiWidgetPrivate(PublicType &i) : de::Private<PublicType>(i) {}
+
+    GuiWidgetPrivate(PublicType *i) : de::Private<PublicType>(i) {}
+
+    virtual ~GuiWidgetPrivate()
+    {
+        /**
+         * Ensure that the derived's class's glDeinit() method has been
+         * called before the private class instance is destroyed. At least
+         * classes that have GuiWidget as the immediate parent class need to
+         * call deinitialize() in their destructors.
+         */
+        DENG2_ASSERT(!self.isInitialized());
+    }
+
+    bool hasRoot() const
+    {
+        return self.hasRoot();
+    }
+
+    GuiRootWidget &root() const
+    {
+        DENG2_ASSERT(hasRoot());
+        return self.root();
+    }
+
+    de::AtlasTexture &atlas() const
+    {
+        return root().atlas();
+    }
+
+    de::GLUniform &uAtlas() const
+    {
+        return root().uAtlas();
+    }
+
+    de::GLShaderBank &shaders() const
+    {
+        return root().shaders();
+    }
+};
+
+#define DENG_GUI_PIMPL(ClassName) \
+    typedef ClassName Public; \
+    struct ClassName::Instance : public GuiWidgetPrivate<ClassName>
+
+#endif // DENG_CLIENT_GUIWIDGETPRIVATE_H

doomsday/client/src/ui/widgets/blurwidget.cpp

@@ -24,3 +24,8 @@ BlurWidget::BlurWidget(String const &name) : GuiWidget(name)
 {
     set(Background(Vector4f(1, 1, 1, 0), Background::Blurred));
 }
+
+BlurWidget::~BlurWidget()
+{
+    deinitialize();
+}

doomsday/client/src/ui/widgets/busywidget.cpp

@@ -33,7 +33,7 @@
 
 using namespace de;
 
-DENG2_PIMPL(BusyWidget)
+DENG_GUI_PIMPL(BusyWidget)
 {
     typedef DefaultVertexBuf VertexBuf;
 
@@ -56,6 +56,11 @@ DENG2_PIMPL(BusyWidget)
         self.add(progress);
     }
 
+    ~Instance()
+    {
+        self.deinitialize();
+    }
+
     void glInit()
     {
         VertexBuf *buf = new VertexBuf;
@@ -65,7 +70,7 @@ DENG2_PIMPL(BusyWidget)
         buf->setVertices(gl::TriangleStrip, verts, gl::Static);
 
         drawable.addBuffer(buf);
-        self.root().shaders().build(drawable.program(), "generic.textured.color")
+        shaders().build(drawable.program(), "generic.textured.color")
                 << uMvpMatrix << uTex;
     }
 

doomsday/client/src/ui/widgets/buttonwidget.cpp

@@ -24,7 +24,7 @@
 
 using namespace de;
 
-DENG2_PIMPL(ButtonWidget),
+DENG_GUI_PIMPL(ButtonWidget),
 DENG2_OBSERVES(Action, Triggered)
 {
     State state;
@@ -42,6 +42,11 @@ DENG2_OBSERVES(Action, Triggered)
         setDefaultBackground();
     }
 
+    ~Instance()
+    {
+        self.deinitialize();
+    }
+
     void setState(State st)
     {
         if(state == st) return;

doomsday/client/src/ui/widgets/choicewidget.cpp

@@ -23,7 +23,7 @@
 using namespace de;
 using namespace ui;
 
-DENG2_PIMPL(ChoiceWidget),
+DENG_GUI_PIMPL(ChoiceWidget),
 DENG2_OBSERVES(Context, Addition),
 DENG2_OBSERVES(Context, Removal),
 DENG2_OBSERVES(ContextWidgetOrganizer, WidgetCreation)
@@ -75,6 +75,11 @@ DENG2_OBSERVES(ContextWidgetOrganizer, WidgetCreation)
         updateStyle();
     }
 
+    ~Instance()
+    {
+        self.deinitialize();
+    }
+
     void updateStyle()
     {
         // Popup background color.

doomsday/client/src/ui/widgets/consolecommandwidget.cpp

@@ -30,7 +30,7 @@
 
 using namespace de;
 
-DENG2_PIMPL(ConsoleCommandWidget),
+DENG_GUI_PIMPL(ConsoleCommandWidget),
 DENG2_OBSERVES(App, StartupComplete),
 public IGameChangeObserver
 {
@@ -70,6 +70,7 @@ public IGameChangeObserver
 
     ~Instance()
     {
+        self.deinitialize();
         App::app().audienceForStartupComplete -= this;
         audienceForGameChange -= this;
     }

doomsday/client/src/ui/widgets/consolewidget.cpp

@@ -81,14 +81,7 @@ DENG2_PIMPL(ConsoleWidget)
         releaseRef(horizShift);
         releaseRef(width);
         releaseRef(height);
-    }
-
-    void glInit()
-    {
-    }
-
-    void glDeinit()
-    {
+        self.deinitialize();
     }
 
     void expandLog(int delta, bool useOffsetAnimation)
@@ -292,17 +285,6 @@ void ConsoleWidget::update()
     }
 }
 
-void ConsoleWidget::glInit()
-{
-    LOG_AS("ConsoleWidget");
-    d->glInit();
-}
-
-void ConsoleWidget::glDeinit()
-{
-    d->glDeinit();
-}
-
 bool ConsoleWidget::handleEvent(Event const &event)
 {
     // Hovering over the right edge shows the <-> cursor.

doomsday/client/src/ui/widgets/contextwidgetorganizer.cpp

@@ -86,6 +86,9 @@ DENG2_OBSERVES(ui::Item,    Change     )
 
         ui::Item const &item = context->at(pos);
         GuiWidget *w = factory->makeItemWidget(item, container);
+
+        w->setName(item.label());
+
         if(!w) return; // Unpresentable.
 
         // Others may alter the widget in some way.

doomsday/client/src/ui/widgets/dialogwidget.cpp

@@ -31,6 +31,15 @@ using namespace de;
 
 static TimeDelta const FLASH_ANIM_SPAN = 0.75;
 
+/**
+ * Compares dialog button items to determine the order in which they
+ * should appear in the UI.
+ *
+ * @param a  Dialog button item.
+ * @param b  Dialog button item.
+ *
+ * @return @c true, if a < b.
+ */
 static bool dialogButtonOrder(ui::Item const &a, ui::Item const &b)
 {
     DialogButtonItem const &left  = a.as<DialogButtonItem>();
@@ -66,7 +75,7 @@ static bool dialogButtonOrder(ui::Item const &a, ui::Item const &b)
     return false;
 }
 
-DENG2_PIMPL(DialogWidget),
+DENG_GUI_PIMPL(DialogWidget),
 DENG2_OBSERVES(ContextWidgetOrganizer, WidgetCreation),
 DENG2_OBSERVES(ContextWidgetOrganizer, WidgetUpdate),
 DENG2_OBSERVES(Widget, ChildAddition), // for styling the contents
@@ -125,14 +134,18 @@ DENG2_OBSERVES(ui::Context, Removal)
         self.setContent(container);
     }
 
+    ~Instance()
+    {
+        self.deinitialize();
+    }
+
     void updateContentHeight()
     {
         // The container's height is limited by the height of the view. Normally
         // the dialog tries to show the full height of the content area.
 
-        DENG2_ASSERT(self.hasRoot());
         self.content().rule().setInput(Rule::Height,
-                                       OperatorRule::minimum(self.root().viewHeight(),
+                                       OperatorRule::minimum(root().viewHeight(),
                                                              area->contentRule().height() +
                                                              area->margin() +
                                                              buttons->rule().height()));
@@ -151,7 +164,7 @@ DENG2_OBSERVES(ui::Context, Removal)
     void updateButtonLayout()
     {
         buttons->items().sort(dialogButtonOrder);
-        buttons->updateLayout();
+        //buttons->updateLayout();
 
         needButtonUpdate = false;
     }