Fixes for filling the unit cell

- Move "fill unit cell" to the main menu for easy discovery - Wrap any atoms at 1.0 fractional to 0.0 for convention - Don't generate "all copies" by default (eventually we should have this as a separate command) Signed-off-by: Geoff Hutchison <[email protected]>
ghutchis · Dec 29, 2024 · 932739b · 932739b · github-actions · Dec 29, 2024
1 parent 8a57117
commit 932739b
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 7 deletions.
diff --git a/avogadro/core/spacegroups.cpp b/avogadro/core/spacegroups.cpp
@@ -301,10 +301,7 @@ void SpaceGroups::fillUnitCell(Molecule& mol, unsigned short hallNumber,
     }
   }
 
-  // if (wrapToCell)
-  //   CrystalTools::wrapAtomsToUnitCell(mol);
-
-  // Now we need to generate any copies on the unit boundary
+  // Now we generate any copies on the unit boundary
   // We need to loop through all the atoms again
   // if a fractional coordinate contains 0.0, we need to generate a copy
   // of the atom at 1.0

diff --git a/avogadro/core/spacegroups.h b/avogadro/core/spacegroups.h
@@ -126,7 +126,7 @@ class AVOGADROCORE_EXPORT SpaceGroups
    */
   static void fillUnitCell(Molecule& mol, unsigned short hallNumber,
                            double cartTol = 1e-5, bool wrapToCell = true,
-                           bool allCopies = true);
+                           bool allCopies = false);
 
   /**
    * Reduce a cell to its asymmetric unit.

diff --git a/avogadro/core/unitcell.h b/avogadro/core/unitcell.h
@@ -291,6 +291,14 @@ inline Vector3 UnitCell::wrapFractional(const Vector3& f) const
     ++result[1];
   if (result[2] < static_cast<Real>(0.0))
     ++result[2];
+  // set anything at 1.0 to 0.0
+  if (result[0] >= static_cast<Real>(0.999999))
+    result[0] = static_cast<Real>(0.0);
+  if (result[1] == static_cast<Real>(0.999999))
+    result[1] = static_cast<Real>(0.0);
+  if (result[2] == static_cast<Real>(0.999999))
+    result[2] = static_cast<Real>(0.0);
+
   return result;
 }
 
@@ -305,6 +313,14 @@ inline void UnitCell::wrapFractional(const Vector3& f, Vector3& wrapped) const
     ++wrapped[1];
   if (wrapped[2] < static_cast<Real>(0.0))
     ++wrapped[2];
+
+  // set anything at 1.0 to 0.0
+  if (wrapped[0] >= static_cast<Real>(0.999999))
+    wrapped[0] = static_cast<Real>(0.0);
+  if (wrapped[1] >= static_cast<Real>(0.999999))
+    wrapped[1] = static_cast<Real>(0.0);
+  if (wrapped[2] >= static_cast<Real>(0.999999))
+    wrapped[2] = static_cast<Real>(0.0);
 }
 
 inline Vector3 UnitCell::wrapCartesian(const Vector3& cart) const

diff --git a/avogadro/qtplugins/spacegroup/spacegroup.cpp b/avogadro/qtplugins/spacegroup/spacegroup.cpp
@@ -73,7 +73,8 @@ SpaceGroup::SpaceGroup(QObject* parent_)
   m_fillUnitCellAction->setText(tr("Fill Unit Cell…"));
   connect(m_fillUnitCellAction, SIGNAL(triggered()), SLOT(fillUnitCell()));
   m_actions.push_back(m_fillUnitCellAction);
-  m_fillUnitCellAction->setProperty("menu priority", 50);
+  // should fall next to the "Wrap Atoms to Unit Cell" action
+  m_fillUnitCellAction->setProperty("menu priority", 185);
 
   m_reduceToAsymmetricUnitAction->setText(tr("Reduce to Asymmetric Unit"));
   connect(m_reduceToAsymmetricUnitAction, SIGNAL(triggered()),
@@ -100,8 +101,11 @@ QList<QAction*> SpaceGroup::actions() const
   return m_actions;
 }
 
-QStringList SpaceGroup::menuPath(QAction*) const
+QStringList SpaceGroup::menuPath(QAction* action) const
 {
+  if (action == m_fillUnitCellAction)
+    return QStringList() << tr("&Crystal");
+
   return QStringList() << tr("&Crystal") << tr("Space Group");
 }