Changeset 649f59 for src/UIElements

Timestamp:

Apr 25, 2016, 7:05:31 AM (9 years ago)

Author:

Frederik Heber <heber@…>

Branches:

Action_Thermostats, Add_AtomRandomPerturbation, Add_FitFragmentPartialChargesAction, Add_RotateAroundBondAction, Add_SelectAtomByNameAction, Added_ParseSaveFragmentResults, AddingActions_SaveParseParticleParameters, Adding_Graph_to_ChangeBondActions, Adding_MD_integration_tests, Adding_ParticleName_to_Atom, Adding_StructOpt_integration_tests, AtomFragments, Automaking_mpqc_open, AutomationFragmentation_failures, Candidate_v1.5.4, Candidate_v1.6.0, Candidate_v1.6.1, ChangeBugEmailaddress, ChangingTestPorts, ChemicalSpaceEvaluator, CombiningParticlePotentialParsing, Combining_Subpackages, Debian_Package_split, Debian_package_split_molecuildergui_only, Disabling_MemDebug, Docu_Python_wait, EmpiricalPotential_contain_HomologyGraph, EmpiricalPotential_contain_HomologyGraph_documentation, Enable_parallel_make_install, Enhance_userguide, Enhanced_StructuralOptimization, Enhanced_StructuralOptimization_continued, Example_ManyWaysToTranslateAtom, Exclude_Hydrogens_annealWithBondGraph, FitPartialCharges_GlobalError, Fix_BoundInBox_CenterInBox_MoleculeActions, Fix_ChargeSampling_PBC, Fix_ChronosMutex, Fix_FitPartialCharges, Fix_FitPotential_needs_atomicnumbers, Fix_ForceAnnealing, Fix_IndependentFragmentGrids, Fix_ParseParticles, Fix_ParseParticles_split_forward_backward_Actions, Fix_PopActions, Fix_QtFragmentList_sorted_selection, Fix_Restrictedkeyset_FragmentMolecule, Fix_StatusMsg, Fix_StepWorldTime_single_argument, Fix_Verbose_Codepatterns, Fix_fitting_potentials, Fixes, ForceAnnealing_goodresults, ForceAnnealing_oldresults, ForceAnnealing_tocheck, ForceAnnealing_with_BondGraph, ForceAnnealing_with_BondGraph_continued, ForceAnnealing_with_BondGraph_continued_betteresults, ForceAnnealing_with_BondGraph_contraction-expansion, FragmentAction_writes_AtomFragments, FragmentMolecule_checks_bonddegrees, GeometryObjects, Gui_Fixes, Gui_displays_atomic_force_velocity, ImplicitCharges, IndependentFragmentGrids, IndependentFragmentGrids_IndividualZeroInstances, IndependentFragmentGrids_IntegrationTest, IndependentFragmentGrids_Sole_NN_Calculation, JobMarket_RobustOnKillsSegFaults, JobMarket_StableWorkerPool, JobMarket_unresolvable_hostname_fix, MoreRobust_FragmentAutomation, ODR_violation_mpqc_open, PartialCharges_OrthogonalSummation, PdbParser_setsAtomName, PythonUI_with_named_parameters, QtGui_reactivate_TimeChanged_changes, Recreated_GuiChecks, Rewrite_FitPartialCharges, RotateToPrincipalAxisSystem_UndoRedo, SaturateAtoms_findBestMatching, SaturateAtoms_singleDegree, StoppableMakroAction, Subpackage_CodePatterns, Subpackage_JobMarket, Subpackage_LinearAlgebra, Subpackage_levmar, Subpackage_mpqc_open, Subpackage_vmg, Switchable_LogView, ThirdParty_MPQC_rebuilt_buildsystem, TrajectoryDependenant_MaxOrder, TremoloParser_IncreasedPrecision, TremoloParser_MultipleTimesteps, TremoloParser_setsAtomName, Ubuntu_1604_changes, stable

Children:

efeeb7

Parents:

136842

git-author:

Frederik Heber <heber@…> (04/15/16 07:38:55)

git-committer:

Frederik Heber <heber@…> (04/25/16 07:05:31)

Message:

GLWorldScene removes stored observed values only after both parent is NULL and atomRemoved() came in.

• this is to ensure that lifetime of the observed value instance is as long as required.

Location:

src/UIElements/Views/Qt4/Qt3D

Files:

• GLWorldScene.cpp (modified) (10 diffs)
• GLWorldScene.hpp (modified) (4 diffs)

src/UIElements/Views/Qt4/Qt3D/GLWorldScene.cpp

@@ -163 +163 @@
 
   // store the object, as we need it on reparenting
+  {
 #ifndef NDEBUG
-  std::pair< ObservedAtoms_t::iterator, bool > inserter =
+    std::pair< ObservedAtoms_t::iterator, bool > inserter =
 #endif
-      ObservedAtoms.insert( std::make_pair(_atom->getIndex(), _atom) );
-  ASSERT( inserter.second,
-      "GLWorldScene::connectAtom() - observed atom "+toString(_atom)+" already stored?");
-
+        ObservedAtoms.insert( std::make_pair(_atom.get(), _atom) );
+    ASSERT( inserter.second,
+        "GLWorldScene::connectAtom() - observed atom "+toString(_atom)+" already stored?");
+#ifndef NDEBUG
+    std::pair< ObservedValueIndexLookup_t::iterator, bool > indexinserter =
+#endif
+        ObservedValueIndexLookup.insert( std::make_pair(_atom->getIndex(), _atom.get()) );
+    ASSERT( indexinserter.second,
+        "GLWorldScene::connectAtom() - observed atom's index "
+        +toString(_atom->getIndex())+" already stored?");
+  }
   {
     const ObservedValue_Index_t atomid = _atom->getIndex();
@@ -226 +234 @@
 }
 
+template <class T>
+void removeStoredObservedValue(
+    GLWorldScene::ObservedValueIndexLookup_t &_ObservedValueIndexLookup,
+    T &_ObservedValues,
+    ObservedValue_Index_t _id)
+{
+  // get QObject* from lookup
+  const GLWorldScene::ObservedValueIndexLookup_t::iterator iter =
+      _ObservedValueIndexLookup.find(_id);
+  ASSERT( iter != _ObservedValueIndexLookup.end(),
+      "GLWorldScene::removeAtom() - could not find index "+toString(_id)
+      +" to stored observed value.");
+  QObject * sender = iter->second;
+  const size_t erased = _ObservedValues.erase(sender);
+  ASSERT( erased == 1,
+      "GLWorldScene::removeStoredObservedValue() - could not erase stored observed value "
+      +toString(_id));
+  _ObservedValueIndexLookup.erase(iter);
+}
+
 /** Removes an atom.
  *
@@ -246 +274 @@
   {
     boost::recursive_mutex::scoped_lock lock(AtomNodeParentMap_mutex);
-    AtomNodeParentMap.left.erase(_atomid);
-  }
-  {
-    const size_t erased = ObservedAtoms.erase(_atomid);
-    ASSERT( erased == 1,
-        "GLWorldScene::removeAtom() - could not erase stored observed value "+toString(_atomid));
-  }
+    AtomNodeParentMap_t::left_iterator iter = AtomNodeParentMap.left.find(_atomid);
+    ASSERT( iter != AtomNodeParentMap.left.end(),
+        "GLWorldScene::removeAtom() - missing parent entry for "+toString(_atomid));
+    if (iter->second == (ObservedValue_Index_t)NULL) {
+      LOG(4, "DEBUG: Found parent of atom as GLWorldScene, removing.");
+      removeStoredObservedValue(ObservedValueIndexLookup, ObservedAtoms, _atomid);
+    }
+    AtomNodeParentMap.left.erase(iter);
+  }
+
   emit changed();
   emit changeOccured();
@@ -270 +301 @@
 
   // store the object, as we need it on reparenting
+  {
 #ifndef NDEBUG
-  std::pair< ObservedBonds_t::iterator, bool > inserter =
+    std::pair< ObservedBonds_t::iterator, bool > inserter =
 #endif
-      ObservedBonds.insert( std::make_pair(_bond->getIndex(), _bond) );
-  ASSERT( inserter.second,
-      "GLWorldScene::connectBond() - observed bond "+toString(_bond)+" already stored?");
-
+        ObservedBonds.insert( std::make_pair(_bond.get(), _bond) );
+    ASSERT( inserter.second,
+        "GLWorldScene::connectBond() - observed bond "+toString(_bond)+" already stored?");
+#ifndef NDEBUG
+    std::pair< ObservedValueIndexLookup_t::iterator, bool > indexinserter =
+#endif
+        ObservedValueIndexLookup.insert( std::make_pair(_bond->getIndex(), _bond.get()) );
+    ASSERT( indexinserter.second,
+        "GLWorldScene::connectBond() - observed bond's index "
+        +toString(_bond->getIndex())+" already stored?");
+  }
   {
     const ObservedValue_Index_t bondid = _bond->getIndex();
@@ -365 +404 @@
   {
     boost::recursive_mutex::scoped_lock lock(BondNodeParentMap_mutex);
-    BondNodeParentMaps[0].left.erase(_bondid);
-    BondNodeParentMaps[1].left.erase(_bondid);
-  }
-  {
-    const size_t erased = ObservedAtoms.erase(_bondid);
-    ASSERT( erased == 1,
-        "GLWorldScene::removeBond() - could not erase stored observed value "+toString(_bondid));
+    for (size_t i=0;i<2;++i) {
+      BondNodeParentMap_t::left_iterator iter = BondNodeParentMaps[i].left.find(_bondid);
+      ASSERT( iter != BondNodeParentMaps[i].left.end(),
+          "GLWorldScene::removeBond() - missing parent entry for "+toString(_bondid));
+      if (iter->second == (ObservedValue_Index_t)NULL) {
+        LOG(4, "DEBUG: Found parent of bond as GLWorldScene, removing.");
+        removeStoredObservedValue(ObservedValueIndexLookup, ObservedBonds, _bondid);
+      }
+      BondNodeParentMaps[i].left.erase(iter);
+    }
   }
 
@@ -556 +598 @@
 void GLWorldScene::reparentAtom()
 {
-  QtObservedAtom *walker = dynamic_cast<QtObservedAtom *>(sender());
   boost::recursive_mutex::scoped_lock lock(AtomNodeParentMap_mutex);
+  QObject * origin = sender();
+  ObservedAtoms_t::const_iterator iter = ObservedAtoms.find(origin);
+  ASSERT( iter != ObservedAtoms.end(),
+      "GLWorldScene::reparentAtom() - atom's "+toString(origin)+" is no longer stored?");
+  QtObservedAtom::ptr walker = iter->second;
   const ObservedValue_Index_t walkerid = walker->getIndex();
   LOG(4, "DEBUG: GLWorldScene: Received signal moleculeChanged for atom "+toString(walkerid)+".");
@@ -576 +622 @@
     resetParent(atomiter->second, getMoleculeObject(parentiter->second));
   // else atom does not yet exist
+
+  // check whether node is still shown, otherwise remove from lists completely
+  {
+    AtomNodeMap::iterator iter = AtomsinSceneMap.find(walkerid);
+    if (iter == AtomsinSceneMap.end()) {
+      LOG(3, "DEBUG: Reparented atom that is no longer on display, removing.");
+      removeStoredObservedValue(ObservedValueIndexLookup, ObservedAtoms, walkerid);
+    }
+  }
 }
 
@@ -584 +639 @@
 {
   boost::recursive_mutex::scoped_lock lock(BondNodeParentMap_mutex);
-  QtObservedBond *bond = dynamic_cast<QtObservedBond *>(sender());
+  QObject * origin = sender();
+  ObservedBonds_t::const_iterator iter = ObservedBonds.find(origin);
+  ASSERT( iter != ObservedBonds.end(),
+      "GLWorldScene::reparentBondLeft() - bond "+toString(origin)+" is no longer stored?");
+  QtObservedBond::ptr bond = iter->second;
   reparentBond(bond, bond->getLeftAtom(), GLMoleculeObject_bond::left);
+
+  // check whether node is still shown, otherwise remove from lists completely
+  {
+    const ObservedValue_Index_t bondid = bond->getIndex();
+    BondNodeMap::iterator iter = BondsinSceneMap.find(bondid);
+    if (iter == BondsinSceneMap.end()) {
+      // check whether both parents are now NULL
+      BondNodeParentMap_t::left_iterator leftiter = BondNodeParentMaps[0].left.find(bondid);
+      BondNodeParentMap_t::left_iterator rightiter = BondNodeParentMaps[1].left.find(bondid);
+      ASSERT( leftiter != BondNodeParentMaps[0].left.end(),
+          "GLWorldScene::reparentBondRight() - left parent to index "+toString(bondid)+" missing?");
+      ASSERT( rightiter != BondNodeParentMaps[1].left.end(),
+          "GLWorldScene::reparentBondRight() - right parent to index "+toString(bondid)+" missing?");
+      if ((leftiter->second == (ObservedValue_Index_t)NULL)
+          && (rightiter->second == (ObservedValue_Index_t)NULL)) {
+        LOG(3, "DEBUG: Reparented both left and right bond that is also no longer on display, removing.");
+        removeStoredObservedValue(ObservedValueIndexLookup, ObservedBonds, bondid);
+      }
+    }
+  }
 }
 
@@ -594 +673 @@
 {
   boost::recursive_mutex::scoped_lock lock(BondNodeParentMap_mutex);
-  QtObservedBond *bond = dynamic_cast<QtObservedBond *>(sender());
+  QObject * origin = sender();
+  ObservedBonds_t::const_iterator iter = ObservedBonds.find(origin);
+  ASSERT( iter != ObservedBonds.end(),
+      "GLWorldScene::reparentBondRight() - bond "+toString(origin)+" is no longer stored?");
+  QtObservedBond::ptr bond = iter->second;
   reparentBond(bond, bond->getRightAtom(), GLMoleculeObject_bond::right);
+
+  // check whether node is still shown, otherwise remove from lists completely
+  {
+    const ObservedValue_Index_t bondid = bond->getIndex();
+    BondNodeMap::iterator iter = BondsinSceneMap.find(bondid);
+    if (iter == BondsinSceneMap.end()) {
+      // check whether both parents are now NULL
+      BondNodeParentMap_t::left_iterator leftiter = BondNodeParentMaps[0].left.find(bondid);
+      BondNodeParentMap_t::left_iterator rightiter = BondNodeParentMaps[1].left.find(bondid);
+      ASSERT( leftiter != BondNodeParentMaps[0].left.end(),
+          "GLWorldScene::reparentBondRight() - left parent to index "+toString(bondid)+" missing?");
+      ASSERT( rightiter != BondNodeParentMaps[1].left.end(),
+          "GLWorldScene::reparentBondRight() - right parent to index "+toString(bondid)+" missing?");
+      if ((leftiter->second == (ObservedValue_Index_t)NULL)
+          && (rightiter->second == (ObservedValue_Index_t)NULL)) {
+        LOG(3, "DEBUG: Reparented both left and right bond that is also no longer on display, removing.");
+        removeStoredObservedValue(ObservedValueIndexLookup, ObservedBonds, bondid);
+      }
+    }
+  }
 }
 
@@ -621 +724 @@
  */
 void GLWorldScene::reparentBond(
-    QtObservedBond *_bond,
+    const QtObservedBond::ptr _bond,
     const QtObservedAtom::ptr _atom,
     const GLMoleculeObject_bond::SideOfBond _side)

src/UIElements/Views/Qt4/Qt3D/GLWorldScene.hpp

@@ -127 +127 @@
 
   void reparentBond(
-      QtObservedBond *_bond,
+      const QtObservedBond::ptr _bond,
       const QtObservedAtom::ptr _atom,
       const GLMoleculeObject_bond::SideOfBond _side);
@@ -137 +137 @@
 public:
   void updateSelectedShapes();
+
+  typedef std::map< QObject*, QtObservedAtom::ptr > ObservedAtoms_t;
+  typedef std::map< QObject*, QtObservedBond::ptr > ObservedBonds_t;
+
+  typedef std::map< ObservedValue_Index_t, QObject* > ObservedValueIndexLookup_t;
 
 private:
@@ -159 +164 @@
   > BondNodeParentMap_t;
 
-  typedef std::map< ObservedValue_Index_t, QtObservedAtom::ptr > ObservedAtoms_t;
-  typedef std::map< ObservedValue_Index_t, QtObservedBond::ptr > ObservedBonds_t;
-
   //!> mutex to ascertain atomic access to NodeParentMaps
   boost::recursive_mutex AtomNodeParentMap_mutex;
@@ -177 +179 @@
   //!> stored observed bond such that its signals may always be safely evaluated
   ObservedBonds_t ObservedBonds;
+  //!> in moleculeChanged() we only have QObject*, in reparent..() we have ObservedValue_Index_t, need lookup
+  ObservedValueIndexLookup_t ObservedValueIndexLookup;
 
   SelectionModeType selectionMode;