--revert markerset subconfig removals; Don't write empty configs to f…

…ile (#2435) Removing the markerset subconfigs when they were empty caused memory corruption in certain rare cases, so this is change is being reverted; Instead, a subconfig with no values in its entire subconfig tree (instead of just in its own primary map) will not be written to JSON.
facebookresearch · Jul 25, 2024 · 5932ce6 · 5932ce6
1 parent 0b58d04
commit 5932ce6
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 35 deletions.
diff --git a/src/esp/core/Configuration.cpp b/src/esp/core/Configuration.cpp
@@ -606,8 +606,8 @@ void Configuration::writeSubconfigsToJson(io::JsonGenericValue& jsonObj,
   auto cfgIterPair = getSubconfigIterator();
   for (auto& cfgIter = cfgIterPair.first; cfgIter != cfgIterPair.second;
        ++cfgIter) {
-    // only save if subconfig has entries
-    if (cfgIter->second->getNumEntries() > 0) {
+    // only save if subconfig tree has value entries
+    if (cfgIter->second->getConfigTreeNumValues() > 0) {
       // Create Generic value for key, using allocator, to make sure its a copy
       // and lives long enough
       io::JsonGenericValue name{cfgIter->first.c_str(), allocator};

diff --git a/src/esp/metadata/attributes/MarkerSets.h b/src/esp/metadata/attributes/MarkerSets.h
@@ -145,11 +145,7 @@ class LinkSet : public esp::core::config::Configuration {
    */
   void setMarkerSetPoints(const std::string& markerSetName,
                           const std::vector<Mn::Vector3>& markerList) {
-    if (markerList.size() == 0) {
-      removeMarkerSet(markerSetName);
-    } else {
-      editMarkerSet(markerSetName)->setAllPoints(markerList);
-    }
+    editMarkerSet(markerSetName)->setAllPoints(markerList);
   }
 
   /**
@@ -325,12 +321,7 @@ class TaskSet : public esp::core::config::Configuration {
   void setLinkMarkerSetPoints(const std::string& linkSetName,
                               const std::string& markerSetName,
                               const std::vector<Mn::Vector3>& markerList) {
-    auto linkSet = editLinkSet(linkSetName);
-    linkSet->setMarkerSetPoints(markerSetName, markerList);
-    // The above may result in the link set being empty. If so remove it.
-    if (linkSet->getNumMarkerSets() == 0) {
-      removeLinkSet(linkSetName);
-    }
+    editLinkSet(linkSetName)->setMarkerSetPoints(markerSetName, markerList);
   }
 
   /**
@@ -345,12 +336,7 @@ class TaskSet : public esp::core::config::Configuration {
       const std::string& linkSetName,
       const std::unordered_map<std::string, std::vector<Mn::Vector3>>&
           markers) {
-    auto linkSet = editLinkSet(linkSetName);
-    linkSet->setAllMarkerPoints(markers);
-    // The above may result in the link set being empty. If so remove it.
-    if (linkSet->getNumMarkerSets() == 0) {
-      removeLinkSet(linkSetName);
-    }
+    editLinkSet(linkSetName)->setAllMarkerPoints(markers);
   }
 
   /**
@@ -585,12 +571,8 @@ class MarkerSets : public esp::core::config::Configuration {
                                   const std::string& linkSetName,
                                   const std::string& markerSetName,
                                   const std::vector<Mn::Vector3>& markerList) {
-    auto taskSetPtr = editTaskSet(taskSetName);
-    taskSetPtr->setLinkMarkerSetPoints(linkSetName, markerSetName, markerList);
-    // After this process, the taskset might be empty, if so, delete it.
-    if (taskSetPtr->getNumLinkSets() == 0) {
-      removeTaskSet(taskSetName);
-    }
+    editTaskSet(taskSetName)
+        ->setLinkMarkerSetPoints(linkSetName, markerSetName, markerList);
   }
 
   /**
@@ -607,12 +589,7 @@ class MarkerSets : public esp::core::config::Configuration {
       const std::string& linkSetName,
       const std::unordered_map<std::string, std::vector<Mn::Vector3>>&
           markerMap) {
-    auto taskSetPtr = editTaskSet(taskSetName);
-    taskSetPtr->setLinkSetPoints(linkSetName, markerMap);
-    // After this process, the taskset might be empty, if so, delete it.
-    if (taskSetPtr->getNumLinkSets() == 0) {
-      removeTaskSet(taskSetName);
-    }
+    editTaskSet(taskSetName)->setLinkSetPoints(linkSetName, markerMap);
   }
 
   /**
@@ -632,10 +609,6 @@ class MarkerSets : public esp::core::config::Configuration {
     for (const auto& markers : markerMap) {
       taskSetPtr->setLinkSetPoints(markers.first, markers.second);
     }
-    // After this process, the taskset might be empty, if so, delete it.
-    if (taskSetPtr->getNumLinkSets() == 0) {
-      removeTaskSet(taskSetName);
-    }
   }
 
   /**