Merge pull request #1408 from ghutchis/fix-selectionwidget-crash

OpenChemistry · Oct 26, 2023 · de3915d · de3915d
2 parents 7fadf98 + ab93b94
commit de3915d
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 15 deletions.
diff --git a/avogadro/qtplugins/selectiontool/selectiontool.cpp b/avogadro/qtplugins/selectiontool/selectiontool.cpp
@@ -22,10 +22,10 @@
 #include <avogadro/qtgui/rwlayermanager.h>
 #include <avogadro/qtgui/rwmolecule.h>
 
+#include <QAction>
 #include <QtCore/QDebug>
 #include <QtGui/QIcon>
 #include <QtGui/QMouseEvent>
-#include <QAction>
 
 #include <queue>
 #include <set>
@@ -43,8 +43,7 @@ namespace Avogadro::QtPlugins {
 
 SelectionTool::SelectionTool(QObject* parent_)
   : QtGui::ToolPlugin(parent_), m_activateAction(new QAction(this)),
-    m_molecule(nullptr), m_renderer(nullptr),
-    m_toolWidget(new SelectionToolWidget(qobject_cast<QWidget*>(parent_))),
+    m_molecule(nullptr), m_renderer(nullptr), m_toolWidget(nullptr),
     m_drawSelectionBox(false), m_doubleClick(false), m_initSelectionBox(false),
     m_layerManager("Selection Tool")
 {
@@ -57,16 +56,19 @@ SelectionTool::SelectionTool(QObject* parent_)
        "Right Mouse: \tClick outside the molecule to clear selection\n"
        "Use Ctrl to toggle the selection and shift to add to the selection.\n"
        "Double-Click: \tSelect an entire fragment."));
-
-  connect(m_toolWidget, SIGNAL(colorApplied(Vector3ub)), this,
-          SLOT(applyColor(Vector3ub)));
-  connect(m_toolWidget, SIGNAL(changeLayer(int)), this, SLOT(applyLayer(int)));
 }
 
 SelectionTool::~SelectionTool() {}
 
 QWidget* SelectionTool::toolWidget() const
 {
+  if (m_toolWidget == nullptr) {
+    m_toolWidget = new SelectionToolWidget(qobject_cast<QWidget*>(parent()));
+    connect(m_toolWidget, SIGNAL(colorApplied(Vector3ub)), this,
+            SLOT(applyColor(Vector3ub)));
+    connect(m_toolWidget, SIGNAL(changeLayer(int)), this,
+            SLOT(applyLayer(int)));
+  }
   return m_toolWidget;
 }
 
@@ -132,7 +134,7 @@ QUndoCommand* SelectionTool::mouseReleaseEvent(QMouseEvent* e)
       }
     }
   }
-  if (anySelect) {
+  if (anySelect && m_toolWidget != nullptr) {
     m_toolWidget->setDropDown(m_layerManager.getLayerID(selectedIndex),
                               m_layerManager.layerCount());
   }
@@ -262,7 +264,8 @@ void SelectionTool::applyLayer(int layer)
     layer = layerInfo.maxLayer();
 
     // update the menu too
-    m_toolWidget->setDropDown(layer, m_layerManager.layerCount());
+    if (m_toolWidget != nullptr)
+      m_toolWidget->setDropDown(layer, m_layerManager.layerCount());
     changes |= Molecule::Layers | Molecule::Added;
   }
 
@@ -360,7 +363,8 @@ void SelectionTool::setMolecule(QtGui::Molecule* mol)
     maxLayers = m_layerManager.layerCount();
   }
 
-  m_toolWidget->setDropDown(currentLayer, maxLayers);
+  if (m_toolWidget != nullptr)
+    m_toolWidget->setDropDown(currentLayer, maxLayers);
 }
 
-} // namespace Avogadro
+} // namespace Avogadro::QtPlugins
diff --git a/avogadro/qtplugins/selectiontool/selectiontool.h b/avogadro/qtplugins/selectiontool/selectiontool.h
@@ -63,7 +63,7 @@ private slots:
   QAction* m_activateAction;
   QtGui::Molecule* m_molecule;
   Rendering::GLRenderer* m_renderer;
-  SelectionToolWidget* m_toolWidget;
+  mutable SelectionToolWidget* m_toolWidget;
   bool m_drawSelectionBox;
   bool m_doubleClick;
   bool m_initSelectionBox;

diff --git a/avogadro/qtplugins/selectiontool/selectiontoolwidget.cpp b/avogadro/qtplugins/selectiontool/selectiontoolwidget.cpp
@@ -17,8 +17,6 @@ SelectionToolWidget::SelectionToolWidget(QWidget* parent)
   setDropDown(0, 1);
   connect(m_ui->applyColorButton, SIGNAL(clicked()), this,
           SLOT(userClickedColor()));
-  connect(m_ui->changeLayerDropDown, SIGNAL(currentIndexChanged(int)), this,
-          SIGNAL(changeLayer(int)));
 }
 
 SelectionToolWidget::~SelectionToolWidget()
@@ -28,13 +26,19 @@ SelectionToolWidget::~SelectionToolWidget()
 
 void SelectionToolWidget::setDropDown(size_t current, size_t max)
 {
+  // disconnect the signal so we don't send it accidentally
+  disconnect(m_ui->changeLayerDropDown, nullptr, nullptr, nullptr);
   m_ui->changeLayerDropDown->clear();
   for (size_t i = 0; i < max; ++i) {
     m_ui->changeLayerDropDown->addItem(QString::number(i + 1));
   }
   m_ui->changeLayerDropDown->addItem(tr("New Layer"));
   if (current != m_ui->changeLayerDropDown->currentIndex())
     m_ui->changeLayerDropDown->setCurrentIndex(current);
+
+  // reconnect the signal
+  connect(m_ui->changeLayerDropDown, SIGNAL(currentIndexChanged(int)), this,
+          SIGNAL(changeLayer(int)));
 }
 
 void SelectionToolWidget::userClickedColor()
@@ -59,4 +63,4 @@ void SelectionToolWidget::userClickedColor()
   }
 }
 
-} // namespace Avogadro
+} // namespace Avogadro::QtPlugins