Fix crashing with dipole moment

avogadro/calc/chargemanager.cpp

@@ -164,6 +164,10 @@ Vector3 ChargeManager::dipoleMoment(const std::string& identifier,
     return Vector3(0.0, 0.0, 0.0);
   }
 
+  if (molecule.atomCount() < 2) {
+    return Vector3(0.0, 0.0, 0.0);
+  }
+
   const auto id = m_identifiers[lowerId];
   const ChargeModel* model = m_models[id];
   return model->dipoleMoment(molecule);

avogadro/calc/chargemodel.cpp

@@ -23,6 +23,9 @@ constexpr double M_PI = 3.14159265358979323846;
 
 Vector3 ChargeModel::dipoleMoment(const Molecule& mol) const
 {
+  if (mol.atomCount() < 2)
+    return Vector3(0.0, 0.0, 0.0);
+
   // default is to get the set of partial atomic charges
   // (some models might do something more sophisticated)
   const MatrixX charges = partialCharges(mol);

avogadro/qtopengl/glwidget.cpp

@@ -30,10 +30,10 @@ GLWidget::GLWidget(QWidget* p)
     m_renderTimer(nullptr)
 {
   setFocusPolicy(Qt::ClickFocus);
-  connect(&m_scenePlugins,
-          SIGNAL(pluginStateChanged(Avogadro::QtGui::ScenePlugin*)),
-          SLOT(updateScene()));
-  connect(&m_scenePlugins, SIGNAL(pluginConfigChanged()), SLOT(updateScene()));
+  connect(&m_scenePlugins, &QtGui::ScenePluginModel::pluginStateChanged, this,
+          &GLWidget::updateScene);
+  connect(&m_scenePlugins, &QtGui::ScenePluginModel::pluginConfigChanged, this,
+          &GLWidget::updateScene);
   m_renderer.setTextRenderStrategy(new QtTextRenderStrategy);
 }
 
@@ -47,7 +47,13 @@ void GLWidget::setMolecule(QtGui::Molecule* mol)
   m_molecule = mol;
   foreach (QtGui::ToolPlugin* tool, m_tools)
     tool->setMolecule(m_molecule);
-  connect(m_molecule, SIGNAL(changed(unsigned int)), SLOT(updateScene()));
+
+  if (m_molecule != nullptr) {
+    // update properties like dipole rendering
+    QTimer::singleShot(500, m_molecule, &QtGui::Molecule::update);
+  }
+
+  connect(m_molecule, &QtGui::Molecule::changed, this, &GLWidget::updateScene);
 }
 
 QtGui::Molecule* GLWidget::molecule()
@@ -60,6 +66,14 @@ const QtGui::Molecule* GLWidget::molecule() const
   return m_molecule;
 }
 
+void GLWidget::updateMolecule()
+{
+  if (m_molecule != nullptr) {
+    // update properties like dipole rendering
+    QTimer::singleShot(500, m_molecule, &QtGui::Molecule::update);
+  }
+}
+
 void GLWidget::updateScene()
 {
   // Build up the scene with the scene plugins, creating the appropriate nodes.
@@ -124,7 +138,8 @@ void GLWidget::addTool(QtGui::ToolPlugin* tool)
   if (m_tools.contains(tool))
     return;
 
-  connect(tool, SIGNAL(updateRequested()), SLOT(requestUpdate()));
+  connect(tool, &QtGui::ToolPlugin::updateRequested, this,
+          &GLWidget::requestUpdate);
   tool->setParent(this);
   tool->setGLWidget(this);
   tool->setActiveWidget(this);
@@ -151,17 +166,17 @@ void GLWidget::setActiveTool(QtGui::ToolPlugin* tool)
     return;
 
   if (m_activeTool && m_activeTool != m_defaultTool) {
-    disconnect(m_activeTool, SIGNAL(drawablesChanged()), this,
-               SLOT(updateScene()));
+    disconnect(m_activeTool, &QtGui::ToolPlugin::drawablesChanged, this,
+               &GLWidget::updateScene);
   }
 
   if (tool)
     addTool(tool);
   m_activeTool = tool;
 
   if (m_activeTool && m_activeTool != m_defaultTool) {
-    connect(m_activeTool, SIGNAL(drawablesChanged()), this,
-            SLOT(updateScene()));
+    connect(m_activeTool, &QtGui::ToolPlugin::drawablesChanged, this,
+            &GLWidget::updateScene);
   }
 }
 
@@ -184,26 +199,26 @@ void GLWidget::setDefaultTool(QtGui::ToolPlugin* tool)
     return;
 
   if (m_defaultTool && m_activeTool != m_defaultTool) {
-    disconnect(m_defaultTool, SIGNAL(drawablesChanged()), this,
-               SLOT(updateScene()));
+    disconnect(m_defaultTool, &QtGui::ToolPlugin::drawablesChanged, this,
+               &GLWidget::updateScene);
   }
 
   if (tool)
     addTool(tool);
   m_defaultTool = tool;
 
   if (m_defaultTool && m_activeTool != m_defaultTool) {
-    connect(m_defaultTool, SIGNAL(drawablesChanged()), this,
-            SLOT(updateScene()));
+    connect(m_defaultTool, &QtGui::ToolPlugin::drawablesChanged, this,
+            &GLWidget::updateScene);
   }
 }
 
 void GLWidget::requestUpdate()
 {
   if (!m_renderTimer) {
     m_renderTimer = new QTimer(this);
-    connect(m_renderTimer, SIGNAL(timeout()), SLOT(updateTimeout()));
-    m_renderTimer->setSingleShot(1000 / 30);
+    connect(m_renderTimer, &QTimer::timeout, this, &GLWidget::updateTimeout);
+    m_renderTimer->setSingleShot(1000 / 30); // 30 fps
     m_renderTimer->start();
   }
 }

avogadro/qtopengl/glwidget.h

@@ -116,6 +116,11 @@ public slots:
    */
   void updateScene();
 
+  /**
+   * Request update of molecule properties (e.g., dipole moment)
+   */
+  void updateMolecule();
+
   /**
    * Clear the contents of the scene.
    */
@@ -201,7 +206,7 @@ protected slots:
   QTimer* m_renderTimer;
 };
 
-} // End QtOpenGL namespace
-} // End Avogadro namespace
+} // namespace QtOpenGL
+} // namespace Avogadro
 
 #endif // AVOGADRO_QTOPENGL_GLWIDGET_H

avogadro/qtplugins/dipole/dipole.cpp

@@ -34,6 +34,26 @@ Dipole::~Dipole() {}
 void Dipole::process(const QtGui::Molecule& molecule,
                      Rendering::GroupNode& node)
 {
+  // check if the molecule is empty
+  // (single atoms don't have a dipole moment)
+  if (molecule.atomCount() < 0)
+    return;
+
+  // check if the molecule has the dipole set
+  if (!m_customDipole) {
+    if (molecule.hasData("dipoleMoment")) {
+      m_dipoleVector = molecule.data("dipoleMoment").toVector3();
+    } else {
+      // connect to molecule changes
+      connect(&molecule, &QtGui::Molecule::update, this, &Dipole::updateDipole);
+      connect(&molecule, SIGNAL(changed(unsigned int)), SLOT(updateDipole()));
+    }
+  } else {
+    // custom dipole moment set
+    m_dipoleVector = m_customDipoleVector;
+  }
+
+  // okay if we have all that, set up the arrow
   auto* geometry = new GeometryNode;
   node.addChild(geometry);
 
@@ -44,20 +64,33 @@ void Dipole::process(const QtGui::Molecule& molecule,
   geometry->addDrawable(arrow);
 
   Vector3f origin = Vector3f::Zero();
+  arrow->addSingleArrow(m_dipoleVector.cast<float>(), origin);
+}
 
-  // check if the molecule has the dipole set
-  if (!m_customDipole) {
-    if (molecule.hasData("dipoleMoment")) {
-      m_dipoleVector = molecule.data("dipoleMoment").toVector3();
-    } else {
-      if (!molecule.isInteractive()) {
-        m_dipoleVector =
-          Calc::ChargeManager::instance().dipoleMoment(m_type, molecule);
-      }
-    }
-  }
+void Dipole::updateFinished()
+{
+  m_updateNeeded = true;
+  emit drawablesChanged();
+}
 
-  arrow->addSingleArrow(m_dipoleVector.cast<float>(), origin);
+void Dipole::updateDipole()
+{
+  QtGui::Molecule* molecule = qobject_cast<QtGui::Molecule*>(sender());
+  if (molecule == nullptr || molecule->isInteractive())
+    return;
+
+  // if the molecule has a dipole moment set, use it
+  if (molecule->hasData("dipoleMoment"))
+    return;
+
+  // otherwise, calculate it
+  if (m_updateNeeded) {
+    m_updateNeeded = false;
+    m_dipoleVector =
+      Calc::ChargeManager::instance().dipoleMoment(m_type, *molecule);
+    // single-shot
+    QTimer::singleShot(0, this, SLOT(updateFinished()));
+  }
 }
 
 QWidget* Dipole::setupWidget()

avogadro/qtplugins/dipole/dipole.h

@@ -41,12 +41,18 @@ class Dipole : public QtGui::ScenePlugin
   QWidget* setupWidget() override;
   bool hasSetupWidget() const override { return false; }
 
+public slots:
+  void updateDipole();
+  void updateFinished();
+
 private:
   std::string m_name = "Dipole Moment";
   std::string m_type = "MMFF94";
   std::vector<std::string> m_types;
   Vector3 m_dipoleVector;
+  Vector3 m_customDipoleVector;
   bool m_customDipole = false; // Custom dipole moment set
+  bool m_updateNeeded = true;
 };
 
 } // end namespace QtPlugins