refactor: replace raw object pointers with SmartPointer in ObjectsContext and update dependent codebases

app/gcompose/src/ContextModel.cpp

@@ -38,8 +38,8 @@ void ContextModel::setContext(uLib::ObjectsContext* context) {
         });
 
         // Connect existing objects
-        for (auto* obj : m_rootContext->GetObjects()) {
+        for (const auto& obj : m_rootContext->GetObjects()) {
-            uLib::Object::connect(obj, &uLib::Object::Updated, refresh);
+            uLib::Object::connect(obj.get(), &uLib::Object::Updated, refresh);
         }
     }
     endResetModel();
@@ -229,8 +229,8 @@ bool ContextModel::dropMimeData(const QMimeData* data, Qt::DropAction action, in
         [&findAndRemoveRecursive](uLib::Object* current, uLib::Object* target) {
         if (auto ctx = current->GetChildren()) {
             ctx->RemoveObject(target);
-            for (auto* obj : ctx->GetObjects()) {
+            for (const auto& obj : ctx->GetObjects()) {
-                findAndRemoveRecursive(obj, target);
+                findAndRemoveRecursive(obj.get(), target);
             }
         }
     };
@@ -244,12 +244,12 @@ bool ContextModel::dropMimeData(const QMimeData* data, Qt::DropAction action, in
              // check if targetCtx is descendant of obj
              std::function<bool(uLib::Object*, uLib::Object*)> isDescendant = 
                 [&isDescendant](uLib::Object* root, uLib::Object* target) -> bool {
-                 if (auto ctx = root->GetChildren()) {
+                  if (auto ctx = root->GetChildren()) {
-                     for (auto* child : ctx->GetObjects()) {
+                      for (const auto& child : ctx->GetObjects()) {
-                         if (child == target) return true;
+                          if (child.get() == target) return true;
-                         if (isDescendant(child, target)) return true;
+                          if (isDescendant(child.get(), target)) return true;
-                     }
+                      }
-                 }
+                  }
                  return false;
              };
              if (isDescendant(obj, (uLib::Object*)targetCtx)) invalid = true;

app/gcompose/src/MainPanel.cpp

@@ -182,8 +182,8 @@ void MainPanel::setContext(uLib::ObjectsContext* context) {
 
             // Add any prop3ds that were created during m_mainVtkContext's construction to all panes
             auto panes = this->findChildren<ViewportPane*>();
-            for (auto* obj : context->GetObjects()) {
+            for (const auto& obj : context->GetObjects()) {
-                if (auto* p = m_mainVtkContext->GetProp3D(obj)) {
+                if (auto* p = m_mainVtkContext->GetProp3D(obj.get())) {
                     for (auto* pane : panes) {
                         if (auto* vp = qobject_cast<uLib::Vtk::QViewport*>(pane->currentViewport())) {
                             vp->AddProp3D(*p);

app/gcompose/src/PropertyWidgets.cpp

@@ -426,15 +426,15 @@ void ReferencePropertyWidget::refreshCombo() {
 
     if (m_Context) {
         const auto& objects = m_Context->GetObjects();
-        for (auto* obj : objects) {
+        for (const auto& obj : objects) {
-            if (m_RefProp->IsCompatible(obj)) {
+            if (m_RefProp->IsCompatible(obj.get())) {
                 QString label = QString::fromStdString(obj->GetInstanceName());
                 if (label.isEmpty()) {
                     label = QString::fromStdString(std::string(obj->GetClassName()));
                 }
                 // Add index suffix if name is empty to disambiguate
-                m_Combo->addItem(label, QVariant::fromValue((quintptr)obj));
+                m_Combo->addItem(label, QVariant::fromValue((quintptr)obj.get()));
-                if (obj == currentRef) {
+                if (obj.get() == currentRef) {
                     selectedIdx = m_Combo->count() - 1;
                 }
             }

docs/object_context.md

@@ -2,6 +2,8 @@
 
 In uLib the context is meant to hold a set of objects and their hierarchy. In addition ObjectFactory is used to create objects from a predefined registry. 
 
+Object context can be thouught as a collection of uLib::Object instances. And there exists nested collection of objects if a context is added to another context. A nested context is a Group of elements that appears like a single object in the parent context and a hierarchy of objects inside the tree structure. 
+
 ## Geant Physical Volumes
 
 The Geant library add a further layer of complexity. The physical volumes are created from a what is called LogicalVolume (which holds information about the shape, material and daughter volumes) and represent the actual instances of the volumes in the detector. So in this sense they represent what could be the Prop3D in the uLib Vtk library. The PhysicalVolume is created from the LogicalVolume and is the one that is actually placed in the scene, with its own relative TRS: position and rotation (rotation here is a rotation matrix comprising the scaling). 

src/Core/ObjectsContext.cpp

@@ -8,28 +8,41 @@ ObjectsContext::ObjectsContext() : Object() {}
 ObjectsContext::~ObjectsContext() {}
 
 void ObjectsContext::AddObject(Object* obj) {
-    if (obj && std::find(m_objects.begin(), m_objects.end(), obj) == m_objects.end()) {
+    if (obj) {
-        m_objects.push_back(obj);
+        auto it = std::find_if(m_objects.begin(), m_objects.end(), [obj](const SmartPointer<Object>& sp) {
-        // Connect child's update to context's update to trigger re-renders
+            return sp.get() == obj;
-        Object::connect(obj, &Object::Updated, this, &Object::Updated);
+        });
-        ULIB_SIGNAL_EMIT(ObjectsContext::ObjectAdded, obj);
+        if (it == m_objects.end()) {
-        this->Updated(); // Signal that the context has been updated
+            m_objects.push_back(SmartPointer<Object>(obj));
+            // Connect child's update to context's update to trigger re-renders
+            Object::connect(obj, &Object::Updated, this, &Object::Updated);
+            ULIB_SIGNAL_EMIT(ObjectsContext::ObjectAdded, obj);
+            this->Updated(); // Signal that the context has been updated
+        }
     }
 }
 
 void ObjectsContext::RemoveObject(Object* obj) {
-    auto it = std::find(m_objects.begin(), m_objects.end(), obj);
+    auto it = std::find_if(m_objects.begin(), m_objects.end(), [obj](const SmartPointer<Object>& sp) {
+        return sp.get() == obj;
+    });
     if (it != m_objects.end()) {
-        Object* removedObj = *it;
+        Object* removedObj = it->get();
-        m_objects.erase(it);
+        // Since we are about to erase it from the vector, if it was the last reference
+        // it would be deleted. We might want to emit the signal BEFORE erasing.
         ULIB_SIGNAL_EMIT(ObjectsContext::ObjectRemoved, removedObj);
+        m_objects.erase(it);
         this->Updated(); // Signal that the context has been updated
     }
 }
 
 void ObjectsContext::Clear() {
     if (!m_objects.empty()) {
-        for (auto obj : m_objects) {
+        // Create a copy of the pointers to emit signals since m_objects might be modified or cleared
+        std::vector<Object*> toRemove;
+        for (const auto& sp : m_objects) toRemove.push_back(sp.get());
+
+        for (auto obj : toRemove) {
             ULIB_SIGNAL_EMIT(ObjectsContext::ObjectRemoved, obj);
         }
         m_objects.clear();
@@ -37,7 +50,7 @@ void ObjectsContext::Clear() {
     }
 }
 
-const std::vector<Object*>& ObjectsContext::GetObjects() const {
+const std::vector<SmartPointer<Object>>& ObjectsContext::GetObjects() const {
     return m_objects;
 }
 
@@ -47,7 +60,7 @@ size_t ObjectsContext::GetCount() const {
 
 Object* ObjectsContext::GetObject(size_t index) const {
     if (index < m_objects.size()) {
-        return m_objects[index];
+        return m_objects[index].get();
     }
     return nullptr;
 }

src/Core/ObjectsContext.h

@@ -2,6 +2,7 @@
 #define U_CORE_OBJECTS_CONTEXT_H
 
 #include "Core/Object.h"
+#include "Core/SmartPointer.h"
 #include <vector>
 
 namespace uLib {
@@ -36,9 +37,9 @@ public:
 
     /**
      * @brief Returns the collection of objects.
-     * @return Const reference to the vector of object pointers.
+     * @return Const reference to the vector of SmartPointer<Object>.
      */
-    const std::vector<Object*>& GetObjects() const;
+    const std::vector<SmartPointer<Object>>& GetObjects() const;
     
     signals:
     /** @brief Signal emitted when an object is added. */
@@ -60,7 +61,7 @@ public:
     Object* GetObject(size_t index) const;
 
 private:
-    std::vector<Object*> m_objects;
+    std::vector<SmartPointer<Object>> m_objects;
 };
 
 } // namespace uLib

src/Math/Assembly.cpp

@@ -89,8 +89,8 @@ void Assembly::ComputeBoundingBox() {
     m_BBoxMin = Vector3f(inf, inf, inf);
     m_BBoxMax = Vector3f(-inf, -inf, -inf);
 
-    for (Object *obj : objects) {
+    for (const auto& obj : objects) {
-        if (auto *box = dynamic_cast<ContainerBox *>(obj)) {
+        if (auto *box = dynamic_cast<ContainerBox *>(obj.get())) {
             // ContainerBox: wm is matrix from unit cube [0,1] to local space
             // Since it is parented to 'this', GetMatrix() is sufficient.
             Matrix4f m = box->GetMatrix();
@@ -104,7 +104,7 @@ void Assembly::ComputeBoundingBox() {
                     m_BBoxMax(a) = std::max(m_BBoxMax(a), corner(a));
                 }
             }
-        } else if (auto *cyl = dynamic_cast<Cylinder *>(obj)) {
+        } else if (auto *cyl = dynamic_cast<Cylinder *>(obj.get())) {
             // Cylinder: centered [-1, 1] radial, [-0.5, 0.5] height
             Matrix4f m = cyl->GetMatrix();
             for (int i = 0; i < 8; ++i) {
@@ -117,7 +117,7 @@ void Assembly::ComputeBoundingBox() {
                     m_BBoxMax(a) = std::max(m_BBoxMax(a), corner(a));
                 }
             }
-        } else if (auto *subAsm = dynamic_cast<Assembly *>(obj)) {
+        } else if (auto *subAsm = dynamic_cast<Assembly *>(obj.get())) {
             // Recursive AABB for nested assemblies
             subAsm->ComputeBoundingBox();
             Vector3f subMin, subMax;

src/Vtk/vtkObjectsContext.cpp

@@ -49,8 +49,8 @@ void ObjectsContext::Synchronize() {
   // 1. Identify objects to add and remove
   const auto &objects = m_Context->GetObjects();
   std::map<uLib::Object *, bool> currentObjects;
-  for (auto obj : objects)
+  for (const auto& obj : objects)
-    currentObjects[obj] = true;
+    currentObjects[obj.get()] = true;
 
   // Remove Prop3Ds for objects no longer in context
   for (auto it = m_Prop3Ds.begin(); it != m_Prop3Ds.end();) {
@@ -71,11 +71,11 @@ void ObjectsContext::Synchronize() {
   }
 
   // Add Prop3Ds for new objects
-  for (auto obj : objects) {
+  for (const auto& obj : objects) {
-    if (m_Prop3Ds.find(obj) == m_Prop3Ds.end()) {
+    if (m_Prop3Ds.find(obj.get()) == m_Prop3Ds.end()) {
-      Prop3D *prop3d = this->CreateProp3D(obj);
+      Prop3D *prop3d = this->CreateProp3D(obj.get());
       if (prop3d) {
-        m_Prop3Ds[obj] = prop3d;
+        m_Prop3Ds[obj.get()] = prop3d;
         if (auto *p3d = vtkProp3D::SafeDownCast(prop3d->GetProp()))
           m_Assembly->AddPart(p3d);
         this->Prop3DAdded(prop3d);