Changeset 70db8f

Timestamp:

Apr 17, 2013, 6:56:52 PM (12 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:

c8bbf2

Parents:

411202d

git-author:

Frederik Heber <heber@…> (03/18/13 18:43:08)

git-committer:

Frederik Heber <heber@…> (04/17/13 18:56:52)

Message:

FIX: Significantly simplified observer use in GLMoleculeObject_bond.

• stored bond is now just reference. As if we store the bond::ptr, we prevent the bond from being free'd when removed.
• we additionally store leftatom and rightatom as Observable to know when their notifications have arrived. We also store both ids for quick and safe BondRemoved signalling.
• dstor now always calls signOff()s, elsewhere we just delete ourselves.

Location:

src/UIElements/Views/Qt4/Qt3D

Files:

• GLMoleculeObject_bond.cpp (modified) (5 diffs)
• GLMoleculeObject_bond.hpp (modified) (2 diffs)

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

@@ -60 +60 @@
       +std::string("-")
       +toString(bondref->rightatom->getId())),
-  _bond(bondref),
+  _bond(*bondref),
+  leftatom(bondref->leftatom),
+  rightatom(bondref->rightatom),
+  leftatomId(bondref->leftatom->getId()),
+  rightatomId(bondref->rightatom->getId()),
   BondSide(side)
 {
   // sign on as observer (obtain non-const instance before)
-  _bond->signOn(this, BondObservable::BondRemoved);
-  _bond->leftatom->signOn(this, AtomObservable::PositionChanged);
-  _bond->rightatom->signOn(this, AtomObservable::PositionChanged);
+  _bond.signOn(this, BondObservable::BondRemoved);
+  leftatom->signOn(this, AtomObservable::PositionChanged);
+  leftatom->signOn(this, AtomObservable::ElementChanged);
+  rightatom->signOn(this, AtomObservable::PositionChanged);
+  rightatom->signOn(this, AtomObservable::ElementChanged);
 
   size_t elementno = 0;
   switch (BondSide) {
     case left:
-      if (_bond->rightatom->getType() != NULL) {
-        elementno = _bond->rightatom->getType()->getAtomicNumber();
+      if (_bond.rightatom->getType() != NULL) {
+        elementno = _bond.rightatom->getType()->getAtomicNumber();
       } else { // if not element yet set, set to hydrogen
         elementno = 1;
@@ -78 +84 @@
       break;
     case right:
-      if (_bond->leftatom->getType() != NULL) {
-        elementno = _bond->leftatom->getType()->getAtomicNumber();
+      if (_bond.leftatom->getType() != NULL) {
+        elementno = _bond.leftatom->getType()->getAtomicNumber();
       } else { // if not element yet set, set to hydrogen
         elementno = 1;
@@ -99 +105 @@
 GLMoleculeObject_bond::~GLMoleculeObject_bond()
 {
-  // sign on as observer (obtain non-const instance before)
-  if (_bond){
-    _bond->signOff(this, BondObservable::BondRemoved);
-    _bond->leftatom->signOff(this, AtomObservable::PositionChanged);
-    _bond->rightatom->signOff(this, AtomObservable::PositionChanged);
-    LOG(2, "INFO: Destroying  GLMoleculeObject_bond to bond " << *_bond << " and side " << BondSide << ".");
-  }else{
-    LOG(2, "INFO: Destroying  GLMoleculeObject_bond (bond already killed).");
-  }
-}
-
-void GLMoleculeObject_bond::update(Observable *publisher)
-{
-#ifdef LOG_OBSERVER
-  observerLog().addMessage() << "++ Update of Observer " << observerLog().getName(this) << " from bond " << *_bond << ".";
-#endif
-}
-
-void GLMoleculeObject_bond::subjectKilled(Observable *publisher)
-{
-  LOG(2, "INFO: Received subjectKilled from " << *_bond << ".");
+  // sign off
   switch (BondSide) {
     case left:
-      emit BondRemoved(_bond->leftatom->getId(), _bond->rightatom->getId());
+      emit BondRemoved(leftatomId, rightatomId);
       break;
     case right:
-      emit BondRemoved(_bond->rightatom->getId(), _bond->leftatom->getId());
+      emit BondRemoved(rightatomId, leftatomId);
       break;
     default:
       ASSERT(0,
-          "GLMoleculeObject_bond::subjectKilled() - side is not a valid argument: "+toString(BondSide)+".");
-      break;
-  }
-
-  // Don't let the destructor automatically sign off from a dead bond!
-  _bond->leftatom->signOff(this, AtomObservable::PositionChanged);
-  _bond->rightatom->signOff(this, AtomObservable::PositionChanged);
-  _bond.reset();
+          "GLMoleculeObject_bond::subjectKilled() - side is not a valid argument: "
+          +toString(BondSide)+".");
+      break;
+  }
+  _bond.signOff(this, BondObservable::BondRemoved);
+  leftatom->signOff(this, AtomObservable::PositionChanged);
+  leftatom->signOff(this, AtomObservable::ElementChanged);
+  rightatom->signOff(this, AtomObservable::PositionChanged);
+  rightatom->signOff(this, AtomObservable::ElementChanged);
+  LOG(3, "DEBUG: Destroying  GLMoleculeObject_bond to bond " << &_bond << " and side " << BondSide << ".");
+}
+
+void GLMoleculeObject_bond::update(Observable *publisher)
+{
+#ifdef LOG_OBSERVER
+  if (publisher == static_cast<const Observable *>(&_bond)) {
+    observerLog().addMessage() << "++ Update of Observer "
+        << observerLog().getName(static_cast<Observer*>(this))
+        << " from bond.";
+  } else if (publisher == leftatom) {
+    observerLog().addMessage() << "++ Update of Observer "
+        << observerLog().getName(static_cast<Observer*>(this))
+        << " from leftatom " << _bond.leftatom->getId() << ".";
+  } else if (publisher == rightatom) {
+    observerLog().addMessage() << "++ Update of Observer " <<
+        observerLog().getName(static_cast<Observer*>(this))
+        << " from rightatom " << _bond.rightatom->getId() << ".";
+  } else
+    observerLog().addMessage() << "++ Update of Observer " <<
+    observerLog().getName(static_cast<Observer*>(this)) << " from unknown source.";
+#endif
+}
+
+void GLMoleculeObject_bond::subjectKilled(Observable *publisher)
+{
   delete this;
 }
@@ -143 +156 @@
 {
 #ifdef LOG_OBSERVER
-  observerLog().addMessage() << "++ Update of Observer "<< observerLog().getName(this)
-      << " received notification from bond " << *_bond << " for channel "
-      << notification->getChannelNo() << ".";
+  if (publisher == static_cast<const Observable *>(&_bond)) {
+    observerLog().addMessage() << "++ Update of Observer "
+        << observerLog().getName(static_cast<Observer*>(this))
+        << " received notification from bond for channel "
+        << notification->getChannelNo() << ".";
+  } else if (publisher == leftatom) {
+    observerLog().addMessage() << "++ Update of Observer "
+        << observerLog().getName(static_cast<Observer*>(this))
+        << " received notification from leftatom " << _bond.leftatom->getId() << " for channel "
+        << notification->getChannelNo() << ".";
+  } else if (publisher == rightatom) {
+    observerLog().addMessage() << "++ Update of Observer "
+        << observerLog().getName(static_cast<Observer*>(this))
+        << " received notification from rightatom " << _bond.rightatom->getId() << " for channel "
+        << notification->getChannelNo() << ".";
+  } else
+    observerLog().addMessage() << "++ Update of Observer "
+        << observerLog().getName(static_cast<Observer*>(this))
+        << " received notification from unknown source.";
 #endif
-  if (publisher == dynamic_cast<const Observable *>(_bond.get())){
-    // from the bond
-    switch (notification->getChannelNo()) {
-      case BondObservable::BondRemoved:
-        LOG(2, "INFO: Received notification of BondRemoved from " << *_bond << ".");
-        switch (BondSide) {
-          case left:
-            emit BondRemoved(_bond->leftatom->getId(), _bond->rightatom->getId());
-            break;
-          case right:
-            emit BondRemoved(_bond->rightatom->getId(), _bond->leftatom->getId());
-            break;
-          default:
-            ASSERT(0,
-                "GLMoleculeObject_bond::recieveNotification() - side is not a valid argument: "+toString(BondSide)+".");
-            break;
-        }
-
-        // Don't let the destructor automatically sign off from a dead bond!
-        _bond->leftatom->signOff(this, AtomObservable::PositionChanged);
-        _bond->rightatom->signOff(this, AtomObservable::PositionChanged);
-        _bond.reset();
-        delete this;
-        break;
-          default:
-            break;
-    }
-  }else{
+  if (publisher == static_cast<const Observable *>(&_bond)){
+    delete this;
+  } else {
+    bool DoResetPosition = false;
     // from an atom
     switch (notification->getChannelNo()) {
-      case AtomObservable::PositionChanged:
-        LOG(2, "INFO: Received notification of PositionChanged.");
+    case AtomObservable::PositionChanged:
+      LOG(2, "INFO: Received notification of PositionChanged.");
+      DoResetPosition = true;
+      break;
+    case AtomObservable::ElementChanged:
+      LOG(2, "INFO: Received notification of ElementChanged.");
+      DoResetPosition = true;
+      break;
+    default:
+      break;
+    }
+    if (DoResetPosition) {
         resetPosition();
         emit changed();
@@ -191 +206 @@
   switch (BondSide) {
     case left:
-      Position = _bond->leftatom->getPosition();
-      OtherPosition = _bond->rightatom->getPosition();
+      Position = _bond.leftatom->getPosition();
+      OtherPosition = _bond.rightatom->getPosition();
       break;
     case right:
-      Position = _bond->rightatom->getPosition();
-      OtherPosition = _bond->leftatom->getPosition();
+      Position = _bond.rightatom->getPosition();
+      OtherPosition = _bond.leftatom->getPosition();
       break;
     default:

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

@@ -21 +21 @@
 #include "types.hpp"
 
+class atom;
 
 class GLMoleculeObject_bond : public GLMoleculeObject, public Observer
@@ -41 +42 @@
 private:
   void resetPosition();
-  bond::ptr _bond;
+  const bond& _bond;
+  /** Observable instance inside atom_observable for left bond partner.
+   * We require this knowledge as subjectKilled is called by Observable, not
+   * by AtomObservable which has already been destroyed at this time.
+   *
+   */
+  const Observable *leftatom;
+  /** Observable instance inside atom_observable for right bond partner.
+   * We require this knowledge as subjectKilled is called by Observable, not
+   * by AtomObservable which has already been destroyed at this time.
+   *
+   */
+  const Observable *rightatom;
+  //!> id of left bond partner for safely emitting BondRemoved sigbal
+  const atomId_t leftatomId;
+  //!> id of right bond partner for safely emitting BondRemoved sigbal
+  const atomId_t rightatomId;
   const enum SideOfBond BondSide;
 };