[WIP] Merge dev branch into main (#2476)

* --[Bugfix] - Scene Reset Redundancy (#2470)
* --modify reset() to not perform redundant object re-placement
After successful scene creation, reset is called, but the objects have all already been placed in their initial positions.
* --fix bindings
Don't expose the boolean in reset to python, should only be consumed from Simulator::reconfigure
* --[BE] - Add ability to filter Configuration subconfigs (#2471)
* --add subconfig filtering process.
This function will remove any values and subconfigs from a Configuration that match those found within a passed Configuration.
* --minor naming clarification
* --expand Configuration tests to test filtering; rename test
Test only tests Configurations, so name appropriately
* --[Bugfix] - Address ObjectInstanceAttributes save issues (#2472)
* --access init instance attributes for an rigid/AO via const ref or copy
* --access sim directly to query for defaultCOMCorrection state
So that objects added to an existing scene will use the correct defaultCOMCorrection state.
* --verify requested type is appropriate
* --add queries for current state of Configuration fields
Query whether a field is a default value, a hidden value or backed by an enum (i.e. 'translated')
* --properly set enum-backed/'translated' string fields in instance attrs
* --add is_visible boolean for AOs
For instance consistency - not yet supported.
* --only set instance vals if not default config vals; clean up JSON write
Configuration now supports not writing initialization/default fields to JSON.
* --Add access to user_defined view and setter
* --better names for object init attrs and object instance  init attrs
* --use base obj's current state to build ObjectInstanceAttr and obj copy
* --only change fields if they are different than defaults.
* --[BE] - Baselink ID init via constant (#2465)
* --system-wide constant for baseLink ID
* --use BASELINK_ID instead of magic number -1 to denote baseLink
* --Use ID_UNDEFINED for rigid object link ids.
* --more fixes
* --[BE] - ManagedContainer updates and cleanups (#2475)
* --clean up handle query
* --move object library map settings to base class.
* --make managed container map member variables private.
* --verify templated accessor results are cast to legitimate type.
* --simplify caller-message arguments
* --clarify naming of existing dirty flag, in preparation of file-save-based flag.
* --move filepath check flag to base attributes to make available
* --treat pbr/ibl helper key as hidden field
* --provide rudiments of file-status dirty flags.
The purpose of this flag is to specify that a particular Managed File-based object has been registered in its Manager in a state that is different than that of its disk-based counterpart.
* --rename constructor-building method for clarity
* --[Bugfix] - Compare Configuration scalar doubles properly (#2478)
* --remove unnecessary magnum decorator
* --add and test support for scalar ConfigValues requiring fuzzy compare
This uses the same mechanism that Magnum constructs use, so results are consistent
* --separate Configuration fuzzy compare tests; minor updates
* --make sure comparisons only involve the number of non-hidden values
There is a possibility, albeit remote, that 2 otherwise identical configurations might have a different number of internal-use/hidden values and would appear to be different when they were, in fact, the same. Not anymore. Note, these are only accessible internally (in c++ source) and so the likelihood of this happening is vanishingly small.
* add a check, error meesage, and correction for 0 scale in box shapes (#2464)
Co-authored-by: aclegg3 <[email protected]>
---------
Co-authored-by: John Turner <[email protected]>

examples/tutorials/nb_python/ECCV_2020_Advanced_Features.py

@@ -171,7 +171,11 @@ def build_dict_of_PhyObj_attrs(phys_obj_template):
         False,
         "boolean",
     )
-    res_dict["is_dirty"] = (phys_obj_template.is_dirty, False, "boolean")
+    res_dict["filenames_are_dirty"] = (
+        phys_obj_template.filenames_are_dirty,
+        False,
+        "boolean",
+    )
     return res_dict
 
 

examples/tutorials/nb_python/asset_viewer.py

@@ -331,7 +331,11 @@ def build_dict_of_PhyObj_attrs(phys_obj_template):
         "boolean",
     )
     res_dict["is_collidable"] = (phys_obj_template.is_collidable, True, "boolean")
-    res_dict["is_dirty"] = (phys_obj_template.is_dirty, False, "boolean")
+    res_dict["filenames_are_dirty"] = (
+        phys_obj_template.filenames_are_dirty,
+        False,
+        "boolean",
+    )
     return res_dict
 
 

examples/tutorials/notebooks/ECCV_2020_Advanced_Features.ipynb

@@ -163,7 +163,11 @@
     "        False,\n",
     "        \"boolean\",\n",
     "    )\n",
-    "    res_dict[\"is_dirty\"] = (phys_obj_template.is_dirty, False, \"boolean\")\n",
+    "    res_dict[\"filenames_are_dirty\"] = (\n",
+    "        phys_obj_template.filenames_are_dirty,\n",
+    "        False,\n",
+    "        \"boolean\",\n",
+    "    )\n",
     "    return res_dict\n",
     "\n",
     "\n",

examples/tutorials/notebooks/asset_viewer.ipynb

@@ -329,7 +329,11 @@
     "        \"boolean\",\n",
     "    )\n",
     "    res_dict[\"is_collidable\"] = (phys_obj_template.is_collidable, True, \"boolean\")\n",
-    "    res_dict[\"is_dirty\"] = (phys_obj_template.is_dirty, False, \"boolean\")\n",
+    "    res_dict[\"filenames_are_dirty\"] = (\n",
+    "        phys_obj_template.filenames_are_dirty,\n",
+    "        False,\n",
+    "        \"boolean\",\n",
+    "    )\n",
     "    return res_dict\n",
     "\n",
     "\n",

src/esp/assets/ResourceManager.cpp

@@ -2885,7 +2885,7 @@ bool ResourceManager::instantiateAssetsOnDemand(
   // object has acquired a copy of its parent attributes.  No object should
   // ever have a copy of attributes with isDirty == true - any editing of
   // attributes for objects requires object rebuilding.
-  if (objectAttributes->getIsDirty()) {
+  if (objectAttributes->getFilePathsAreDirty()) {
     CORRADE_ASSERT(
         (getObjectAttributesManager()->registerObject(
              objectAttributes, objectTemplateHandle) != ID_UNDEFINED),

src/esp/bindings/AttributesBindings.cpp

@@ -453,7 +453,11 @@ void initAttributesBindings(py::module& m) {
                              R"(Class name of Attributes template.)")
       .def_property_readonly(
           "csv_info", &AbstractAttributes::getObjectInfo,
-          R"(Comma-separated informational string describing this Attributes template)");
+          R"(Comma-separated informational string describing this Attributes template)")
+      .def_property_readonly(
+          "filenames_are_dirty", &AbstractAttributes::getFilePathsAreDirty,
+          R"(Whether filenames or paths in this attributes have been changed requiring
+          re-registration before they can be used to create an object. )");
 
   // Attributes should only use named properties or subconfigurations to set
   // specific values, to guarantee essential value type integrity. This will
@@ -680,11 +684,7 @@ void initAttributesBindings(py::module& m) {
       .def_property(
           "is_collidable", &ObjectAttributes::getIsCollidable,
           &ObjectAttributes::setIsCollidable,
-          R"(Whether constructions built from this template are collidable upon initialization.)")
-      .def_property_readonly(
-          "is_dirty", &AbstractObjectAttributes::getIsDirty,
-          R"(Whether values in this attributes have been changed requiring
-          re-registration before they can be used to create an object. )");
+          R"(Whether constructions built from this template are collidable upon initialization.)");
 
   // ==== ObjectAttributes ====
   py::class_<ObjectAttributes, AbstractObjectAttributes, ObjectAttributes::ptr>(

src/esp/bindings/ConfigBindings.cpp

@@ -152,7 +152,7 @@ void initConfigBindings(py::module& m) {
           R"(Returns whether or not this Configuration has the passed key. Does not check subconfigurations.)",
           "key"_a)
       .def(
-          "has_key_to_type", &Configuration::hasKeyOfType,
+          "has_key_to_type", &Configuration::hasKeyToValOfType,
           R"(Returns whether passed key points to a value of specified ConfigValType)",
           "key"_a, "value_type"_a)
       .def(

src/esp/bindings/SimBindings.cpp

@@ -164,7 +164,7 @@ void initSimBindings(py::module& m) {
           R"(Use gfx_replay_manager for replay recording and playback.)")
       .def("seed", &Simulator::seed, "new_seed"_a)
       .def("reconfigure", &Simulator::reconfigure, "configuration"_a)
-      .def("reset", &Simulator::reset)
+      .def("reset", [](Simulator& self) { self.reset(false); })
       .def(
           "close", &Simulator::close, "destroy"_a = true,
           R"(Free all loaded assets and GPU contexts. Use destroy=true except where noted in tutorials/async_rendering.py.)")

src/esp/core/Configuration.cpp

@@ -207,18 +207,35 @@ bool operator==(const ConfigValue& a, const ConfigValue& b) {
   if (a._typeAndFlags != b._typeAndFlags) {
     return false;
   }
+  const auto dataType = a.getType();
   // Pointer-backed data types need to have _data dereffed
-  if (isConfigValTypePointerBased(a.getType())) {
-    return pointerBasedConfigTypeHandlerFor(a.getType())
-        .comparator(a._data, b._data);
+  if (isConfigValTypePointerBased(dataType)) {
+    return pointerBasedConfigTypeHandlerFor(dataType).comparator(a._data,
+                                                                 b._data);
   }
 
-  // Trivial type : a._data holds the actual value
-  // _data array will always hold only legal data, since a ConfigValue should
-  // never change type.
+  // By here we know the type is a trivial type and that the types for both
+  // values are equal
+  if (a.reqsFuzzyCompare()) {
+    // Type is specified to require fuzzy comparison
+    switch (dataType) {
+      case ConfigValType::Double: {
+        return Mn::Math::equal(a.get<double>(), b.get<double>());
+      }
+      default: {
+        CORRADE_ASSERT_UNREACHABLE(
+            "Unknown/unsupported Type in ConfigValue::operator==()", "");
+      }
+    }
+  }
+
+  // Trivial non-fuzzy-comparison-requiring type : a._data holds the actual
+  // value _data array will always hold only legal data, since a ConfigValue
+  // should never change type.
   return std::equal(std::begin(a._data), std::end(a._data),
                     std::begin(b._data));
-}
+
+}  // ConfigValue::operator==
 
 bool operator!=(const ConfigValue& a, const ConfigValue& b) {
   return !(a == b);
@@ -236,7 +253,7 @@ std::string ConfigValue::getAsString() const {
       return std::to_string(get<int>());
     }
     case ConfigValType::Double: {
-      return std::to_string(get<double>());
+      return Cr::Utility::formatString("{}", get<double>());
     }
     case ConfigValType::String: {
       return get<std::string>();
@@ -295,7 +312,8 @@ std::string ConfigValue::getAsString() const {
 
 io::JsonGenericValue ConfigValue::writeToJsonObject(
     io::JsonAllocator& allocator) const {
-  // unknown is checked before this function is called, so does not need support
+  // unknown is checked before this function is called, so does not need
+  // support
   switch (getType()) {
     case ConfigValType::Boolean: {
       return io::toJsonValue(get<bool>(), allocator);
@@ -487,17 +505,17 @@ int Configuration::loadOneConfigFromJson(int numConfigSettings,
       } else {
         // The array does not match any currently supported magnum
         // objects, so place in indexed subconfig of values.
-        // decrement count by 1 - the recursive subgroup load will count all the
-        // values.
+        // decrement count by 1 - the recursive subgroup load will count all
+        // the values.
         --numConfigSettings;
         // create a new subgroup
         std::shared_ptr<core::config::Configuration> subGroupPtr =
             editSubconfig<core::config::Configuration>(key);
         // load array into subconfig
         numConfigSettings += subGroupPtr->loadFromJsonArray(jsonObj);
       }
-      // value in array is a number of specified length, else it is a string, an
-      // object or a nested array
+      // value in array is a number of specified length, else it is a string,
+      // an object or a nested array
     } else {
       // decrement count by 1 - the recursive subgroup load will count all the
       // values.
@@ -584,8 +602,8 @@ void Configuration::writeValuesToJson(io::JsonGenericValue& jsonObj,
           << "`, so nothing will be written to JSON for this key.";
 
     } else if (valIter->second.shouldWriteToFile()) {
-      // Create Generic value for key, using allocator, to make sure its a copy
-      // and lives long enough
+      // Create Generic value for key, using allocator, to make sure its a
+      // copy and lives long enough
       writeValueToJsonInternal(valIter->second, valIter->first.c_str(), jsonObj,
                                allocator);
     } else {
@@ -602,8 +620,8 @@ void Configuration::writeSubconfigsToJson(io::JsonGenericValue& jsonObj,
        ++cfgIter) {
     // 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
+      // 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};
       io::JsonGenericValue subObj =
           cfgIter->second->writeToJsonObject(allocator);
@@ -663,9 +681,9 @@ void Configuration::setSubconfigValsOfTypeInVector(
 /**
  * @brief Retrieves a shared pointer to a copy of the subConfig @ref
  * esp::core::config::Configuration that has the passed @p name . This will
- * create a pointer to a new sub-configuration if none exists already with that
- * name, but will not add this configuration to this Configuration's internal
- * storage.
+ * create a pointer to a new sub-configuration if none exists already with
+ * that name, but will not add this configuration to this Configuration's
+ * internal storage.
  *
  * @param name The name of the configuration to retrieve.
  * @return A pointer to a copy of the configuration having the requested
@@ -783,6 +801,52 @@ std::vector<std::string> Configuration::findValue(
   return breadcrumbs;
 }
 
+void Configuration::overwriteWithConfig(
+    const std::shared_ptr<const Configuration>& src) {
+  if (src->getNumEntries() == 0) {
+    return;
+  }
+  // copy every element over from src
+  for (const auto& elem : src->valueMap_) {
+    valueMap_[elem.first] = elem.second;
+  }
+  // merge subconfigs
+  for (const auto& subConfig : src->configMap_) {
+    const auto name = subConfig.first;
+    // make if DNE and merge src subconfig
+    addOrEditSubgroup<Configuration>(name).first->second->overwriteWithConfig(
+        subConfig.second);
+  }
+}  // Configuration::overwriteWithConfig
+
+void Configuration::filterFromConfig(
+    const std::shared_ptr<const Configuration>& src) {
+  if (src->getNumEntries() == 0) {
+    return;
+  }
+  // filter out every element that is present with the same value in both src
+  // and this.
+  for (const auto& elem : src->valueMap_) {
+    ValueMapType::const_iterator mapIter = valueMap_.find(elem.first);
+    // if present and has the same data, erase this configuration's data
+    if ((mapIter != valueMap_.end()) && (mapIter->second == elem.second)) {
+      valueMap_.erase(mapIter);
+    }
+  }
+  // repeat process on all subconfigs of src that are present in this.
+  for (const auto& subConfig : src->configMap_) {
+    // find if this has subconfig of same name
+    ConfigMapType::iterator mapIter = configMap_.find(subConfig.first);
+    if (mapIter != configMap_.end()) {
+      mapIter->second->filterFromConfig(subConfig.second);
+      // remove the subconfig if it has no entries after filtering
+      if (mapIter->second->getNumEntries() == 0) {
+        configMap_.erase(mapIter);
+      }
+    }
+  }
+}  // Configuration::filterFromConfig
+
 Configuration& Configuration::operator=(const Configuration& otr) {
   if (this != &otr) {
     configMap_.clear();
@@ -820,7 +884,7 @@ Mn::Debug& operator<<(Mn::Debug& debug, const Configuration& cfg) {
 
 bool operator==(const Configuration& a, const Configuration& b) {
   if ((a.getNumSubconfigs() != b.getNumSubconfigs()) ||
-      (a.getNumValues() != b.getNumValues())) {
+      (a.getNumVisibleValues() != b.getNumVisibleValues())) {
     return false;
   }
   for (const auto& entry : a.configMap_) {