From cdd8531290928ddf6891cf2deae792cfdf00baab Mon Sep 17 00:00:00 2001 From: Chengrui Zhu Date: Mon, 22 Jul 2024 13:45:15 +0800 Subject: [PATCH 01/10] Fix typos and indents. --- src/user/user_mesh.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/user/user_mesh.cc b/src/user/user_mesh.cc index 1b2d1022..c8a9195b 100644 --- a/src/user/user_mesh.cc +++ b/src/user/user_mesh.cc @@ -2152,7 +2152,7 @@ void mjCSkin::Compile(const mjVFS* vfs) { if (pmat) { matid = pmat->id; } else if (!material_.empty()) { - throw mjCError(this, "unkown material '%s' in skin", material_.c_str()); + throw mjCError(this, "unknown material '%s' in skin", material_.c_str()); } // set total vertex weights to 0 @@ -2452,7 +2452,7 @@ void mjCFlex::ResolveReferences(const mjCModel* m) { if (pbody) { vertbodyid.push_back(pbody->id); } else { - throw mjCError(this, "unkown body '%s' in flex", vertbody.c_str()); + throw mjCError(this, "unknown body '%s' in flex", vertbody.c_str()); } } } @@ -2515,7 +2515,7 @@ void mjCFlex::Compile(const mjVFS* vfs) { if (pmat) { matid = pmat->id; } else if (!material_.empty()) { - throw mjCError(this, "unkown material '%s' in flex", material_.c_str()); + throw mjCError(this, "unknown material '%s' in flex", material_.c_str()); } // resolve body ids From 2746cb8559c08d57aae6239a01144a3ef7866b58 Mon Sep 17 00:00:00 2001 From: Kyle Bayes Date: Tue, 23 Jul 2024 08:35:56 -0700 Subject: [PATCH 02/10] Support FilePaths instead of raw strings in XML code. PiperOrigin-RevId: 655177026 Change-Id: I55b055551384b01ba7a4baa5d75fbe3329b7d25d --- src/user/user_util.cc | 2 +- src/xml/xml.cc | 143 +++++++++++++---------------- src/xml/xml.h | 13 +-- src/xml/xml_api.cc | 14 ++- src/xml/xml_native_reader.cc | 57 +++++++----- src/xml/xml_native_reader.h | 29 +++--- src/xml/xml_util.cc | 68 +++++++------- src/xml/xml_util.h | 19 ++-- test/user/user_util_test.cc | 5 + test/xml/xml_native_reader_test.cc | 24 +++-- 10 files changed, 195 insertions(+), 179 deletions(-) diff --git a/src/user/user_util.cc b/src/user/user_util.cc index 842229e8..803c22d4 100644 --- a/src/user/user_util.cc +++ b/src/user/user_util.cc @@ -996,7 +996,7 @@ std::string FilePath::PathReduce(const std::string& str) { if (IsSeperator(str[i])) { std::string temp = str.substr(j, i - j); j = i + 1; - if (temp == ".." && !dirs.empty()) { + if (temp == ".." && !dirs.empty() && dirs.back() != "..") { dirs.pop_back(); } else if (temp != ".") { dirs.push_back(std::move(temp)); diff --git a/src/xml/xml.cc b/src/xml/xml.cc index 9c64ce2d..8459902f 100644 --- a/src/xml/xml.cc +++ b/src/xml/xml.cc @@ -48,9 +48,11 @@ namespace { using tinyxml2::XMLDocument; using tinyxml2::XMLElement; using tinyxml2::XMLNode; +using mujoco::user::FilePath; namespace mju = ::mujoco::util; + // We are using "locale-sensitive" sprintf to read and write XML. // When MuJoCo is being used as a plug-in for an application that respects the system locale // (e.g. Unity), the user's locale setting can affect the formatting of numbers into strings. @@ -97,29 +99,30 @@ class LocaleOverride { }; #endif -} // namespace - -// Main writer function - calls mjXWrite -std::string mjWriteXML(const mjSpec* spec, char* error, int error_sz) { - LocaleOverride locale_override; - - // check for empty model - if (!spec) { - mjCopyError(error, "Cannot write empty model", error_sz); - return ""; +void RegisterResourceProvider() { + // register string resource provider if not registered before + if (mjp_getResourceProvider("LoadModelFromString:") == nullptr) { + mjpResourceProvider resourceProvider; + mjp_defaultResourceProvider(&resourceProvider); + resourceProvider.prefix = "LoadModelFromString"; + resourceProvider.open = +[](mjResource* resource) { + resource->data = &(resource->name[strlen("LoadModelFromString:")]); + return 1; + }; + resourceProvider.read = + +[](mjResource* resource, const void** buffer) { + *buffer = resource->data; + return (int) strlen((const char*) resource->data); + }; + resourceProvider.close = +[](mjResource* resource) {}; + mjp_registerResourceProvider(&resourceProvider); } - - mjXWriter writer; - writer.SetModel(spec); - return writer.Write(error, error_sz); } - - // find include elements recursively, replace them with subtree from xml file -static void mjIncludeXML(mjXReader& reader, XMLElement* elem, - std::string_view dir, const mjVFS* vfs, - std::unordered_set& included) { +void IncludeXML(mjXReader& reader, XMLElement* elem, + const FilePath& dir, const mjVFS* vfs, + std::unordered_set& included) { // capture directory defaults on first pass of XML tree if (!strcasecmp(elem->Value(), "compiler")) { auto assetdir_attr = mjXUtil::ReadAttrStr(elem, "assetdir"); @@ -142,7 +145,7 @@ static void mjIncludeXML(mjXReader& reader, XMLElement* elem, if (strcasecmp(elem->Value(), "include")) { XMLElement* child = elem->FirstChildElement(); for (; child; child = child->NextSiblingElement()) { - mjIncludeXML(reader, child, dir, vfs, included); + IncludeXML(reader, child, dir, vfs, included); } return; } @@ -153,26 +156,22 @@ static void mjIncludeXML(mjXReader& reader, XMLElement* elem, } // get filename - auto file_attr = mjXUtil::ReadAttrStr(elem, "file", true); + auto file_attr = mjXUtil::ReadAttrFile(elem, "file", reader.ModelFileDir(), + true); if (!file_attr.has_value()) { throw mjXError(elem, "Include element missing file attribute"); } - std::string filename = file_attr.value(); + FilePath filename = file_attr.value(); // block repeated include files - if (included.find(filename) != included.end()) { + if (included.find(filename.Str()) != included.end()) { throw mjXError(elem, "File '%s' already included", filename.c_str()); } // TODO: b/325905702 - We have a messy wrapper here to remain backwards // compatible, which will be removed in the near future. - std::string fullname; - if (!mjuu_isabspath(filename)) { - fullname = reader.ModelFileDir() + filename; - } else { - fullname = filename; - } + FilePath fullname = reader.ModelFileDir() + filename; // legacy behavior: try to load in top level directory std::array error; @@ -180,8 +179,8 @@ static void mjIncludeXML(mjXReader& reader, XMLElement* elem, error.data(), error.size()); if (resource == nullptr) { // new behavior: try to load in relative directory - if (!mjuu_isabspath(filename)) { - fullname = std::string(dir) + filename; + if (!filename.IsAbs()) { + fullname = dir + filename; resource = mju_openResource(fullname.c_str(), vfs, error.data(), error.size()); } } @@ -190,18 +189,14 @@ static void mjIncludeXML(mjXReader& reader, XMLElement* elem, throw mjXError(elem, "%s", error.data()); } - if (!mjuu_isabspath(filename)) { - filename = std::string(dir) + filename; - } + filename = dir + filename; const char* include_dir = nullptr; int ninclude_dir = 0; mju_getResourceDir(resource, &include_dir, &ninclude_dir); - std::string next_dir = std::string(include_dir, ninclude_dir); - if (!mjuu_isabspath(filename)) { - next_dir = std::string(dir) + next_dir; - } - elem->SetAttribute("dir", next_dir.data()); + FilePath next_dir = FilePath(std::string(include_dir, ninclude_dir)); + next_dir = dir + next_dir; + elem->SetAttribute("dir", next_dir.c_str()); const char* xmlstring = nullptr; int buffer_size = mju_readResource(resource, (const void**) &xmlstring); @@ -228,7 +223,7 @@ static void mjIncludeXML(mjXReader& reader, XMLElement* elem, } // remember that file was included - included.insert(filename); + included.insert(filename.Str()); // get and check root element XMLElement* docroot = doc.RootElement(); @@ -262,21 +257,21 @@ static void mjIncludeXML(mjXReader& reader, XMLElement* elem, // recursively run include child = include->FirstChildElement(); for (; child; child = child->NextSiblingElement()) { - mjIncludeXML(reader, child, next_dir, vfs, included); + IncludeXML(reader, child, next_dir, vfs, included); } } - +} // namespace // Main parser function -mjSpec* mjParseXML(const char* filename, const mjVFS* vfs, - char* error, int error_sz) { +mjSpec* ParseXML(const char* filename, const mjVFS* vfs, + char* error, int nerror) { LocaleOverride locale_override; // check arguments if (!filename) { if (error) { - std::snprintf(error, error_sz, "mjParseXML: filename argument required\n"); + std::snprintf(error, nerror, "ParseXML: filename argument required\n"); } return nullptr; } @@ -293,21 +288,21 @@ mjSpec* mjParseXML(const char* filename, const mjVFS* vfs, mjResource* resource = mju_openResource(filename, vfs, rerror.data(), rerror.size()); if (resource == nullptr) { - std::snprintf(error, error_sz, "mjParseXML: %s", rerror.data()); + std::snprintf(error, nerror, "ParseXML: %s", rerror.data()); return nullptr; } int buffer_size = mju_readResource(resource, (const void**) &xmlstring); if (buffer_size < 0) { if (error) { - std::snprintf(error, error_sz, - "mjParseXML: error reading file '%s'", filename); + std::snprintf(error, nerror, + "ParseXML: error reading file '%s'", filename); } mju_closeResource(resource); return nullptr; } else if (!buffer_size) { if (error) { - std::snprintf(error, error_sz, "mjParseXML: empty file '%s'", filename); + std::snprintf(error, nerror, "ParseXML: empty file '%s'", filename); } mju_closeResource(resource); return nullptr; @@ -321,7 +316,7 @@ mjSpec* mjParseXML(const char* filename, const mjVFS* vfs, // error checking if (doc.Error()) { if (error) { - snprintf(error, error_sz, "XML parse error %d:\n%s\n", + snprintf(error, nerror, "XML parse error %d:\n%s\n", doc.ErrorID(), doc.ErrorStr()); } mju_closeResource(resource); @@ -332,7 +327,7 @@ mjSpec* mjParseXML(const char* filename, const mjVFS* vfs, XMLElement* root = doc.RootElement(); if (!root) { mju_closeResource(resource); - mjCopyError(error, "XML root element not found", error_sz); + mjCopyError(error, "XML root element not found", nerror); return nullptr; } @@ -357,7 +352,7 @@ mjSpec* mjParseXML(const char* filename, const mjVFS* vfs, std::unordered_set included = {filename}; mjXReader parser; parser.SetModelFileDir(mjs_getString(spec->modelfiledir)); - mjIncludeXML(parser, root, mjs_getString(spec->modelfiledir), vfs, included); + IncludeXML(parser, root, parser.ModelFileDir(), vfs, included); // parse MuJoCo model parser.SetModel(spec); @@ -385,7 +380,7 @@ mjSpec* mjParseXML(const char* filename, const mjVFS* vfs, // catch known errors catch (mjXError err) { - mjCopyError(error, err.message, error_sz); + mjCopyError(error, err.message, nerror); mj_deleteSpec(spec); return nullptr; } @@ -393,32 +388,24 @@ mjSpec* mjParseXML(const char* filename, const mjVFS* vfs, return spec; } - -static void RegisterResourceProvider() { - // register string resource provider if not registered before - if (mjp_getResourceProvider("LoadModelFromString:") == nullptr) { - mjpResourceProvider resourceProvider; - mjp_defaultResourceProvider(&resourceProvider); - resourceProvider.prefix = "LoadModelFromString"; - resourceProvider.open = +[](mjResource* resource) { - resource->data = &(resource->name[strlen("LoadModelFromString:")]); - return 1; - }; - resourceProvider.read = - +[](mjResource* resource, const void** buffer) { - *buffer = resource->data; - return (int) strlen((const char*) resource->data); - }; - resourceProvider.close = +[](mjResource* resource) {}; - mjp_registerResourceProvider(&resourceProvider); - } -} - - -mjSpec* ParseSpecFromString(std::string_view xml, char* error, - int error_size) { +mjSpec* ParseSpecFromString(std::string_view xml, char* error, int nerror) { RegisterResourceProvider(); std::string xml2 = {xml.begin(), xml.end()}; std::string str = "LoadModelFromString:" + xml2; - return mjParseXML(str.c_str(), nullptr, error, error_size); + return ParseXML(str.c_str(), nullptr, error, nerror); +} + +// Main writer function - calls mjXWrite +std::string WriteXML(const mjSpec* spec, char* error, int nerror) { + LocaleOverride locale_override; + + // check for empty model + if (!spec) { + mjCopyError(error, "Cannot write empty model", nerror); + return ""; + } + + mjXWriter writer; + writer.SetModel(spec); + return writer.Write(error, nerror); } diff --git a/src/xml/xml.h b/src/xml/xml.h index dadc2451..d83f026a 100644 --- a/src/xml/xml.h +++ b/src/xml/xml.h @@ -16,22 +16,19 @@ #define MUJOCO_SRC_XML_XML_H_ #include +#include -#include #include #include -// Top level API - -// Main writer function -std::string mjWriteXML(const mjSpec* spec, char* error, int error_sz); - // Main parser function -mjSpec* mjParseXML(const char* filename, const mjVFS* vfs, char* error, int error_sz); +mjSpec* ParseXML(const char* filename, const mjVFS* vfs, char* error, int nerror); // Returns a newly-allocated mjSpec, loaded from the contents of xml. // On failure returns nullptr and populates the error array if present. -mjSpec* ParseSpecFromString(std::string_view xml, char* error = nullptr, int error_size = 0); +mjSpec* ParseSpecFromString(std::string_view xml, char* error = nullptr, int nerror = 0); +// Main writer function +std::string WriteXML(const mjSpec* spec, char* error, int nerror); #endif // MUJOCO_SRC_XML_XML_H_ diff --git a/src/xml/xml_api.cc b/src/xml/xml_api.cc index 623466ed..2107a80e 100644 --- a/src/xml/xml_api.cc +++ b/src/xml/xml_api.cc @@ -37,6 +37,8 @@ //---------------------------------- Globals ------------------------------------------------------- +namespace { + // global user model class class GlobalModel { public: @@ -61,7 +63,7 @@ std::optional GlobalModel::ToXML(const mjModel* m, char* error, return std::nullopt; } mj_copyBack(spec_, m); - std::string result = mjWriteXML(spec_, error, error_sz); + std::string result = WriteXML(spec_, error, error_sz); if (result.empty()) { return std::nullopt; } @@ -86,6 +88,8 @@ GlobalModel& GetGlobalModel() { return global_model; } +} // namespace + //---------------------------------- Functions ----------------------------------------------------- // parse XML file in MJCF or URDF format, compile it, return low-level model @@ -96,7 +100,7 @@ mjModel* mj_loadXML(const char* filename, const mjVFS* vfs, // parse new model std::unique_ptr> spec( - mjParseXML(filename, vfs, error, error_sz), + ParseXML(filename, vfs, error, error_sz), [](mjSpec* s) { mj_deleteSpec(s); }); if (!spec) { return nullptr; @@ -211,7 +215,7 @@ mjModel* mj_loadModel(const char* filename, const mjVFS* vfs) { // parse spec from file mjSpec* mj_parseXML(const char* filename, const mjVFS* vfs, char* error, int error_sz) { - return mjParseXML(filename, vfs, error, error_sz); + return ParseXML(filename, vfs, error, error_sz); } @@ -225,7 +229,7 @@ mjSpec* mj_parseXMLString(const char* xml, const mjVFS* vfs, char* error, int er // save spec to XML file, return 1 on success, 0 otherwise int mj_saveXML(const mjSpec* s, const char* filename, char* error, int error_sz) { - std::string result = mjWriteXML(s, error, error_sz); + std::string result = WriteXML(s, error, error_sz); if (result.empty()) { return 0; } @@ -241,7 +245,7 @@ int mj_saveXML(const mjSpec* s, const char* filename, char* error, int error_sz) // save spec to string, return 1 on success, 0 otherwise int mj_saveXMLString(const mjSpec* s, char* xml, int xml_sz, char* error, int error_sz) { - std::string result = mjWriteXML(s, error, error_sz); + std::string result = WriteXML(s, error, error_sz); if (result.size() >= xml_sz) { std::string error_msg = "Output string too short, should be at least " + std::to_string(result.size()+1); diff --git a/src/xml/xml_native_reader.cc b/src/xml/xml_native_reader.cc index 6ec5a6a0..3a87a939 100644 --- a/src/xml/xml_native_reader.cc +++ b/src/xml/xml_native_reader.cc @@ -49,6 +49,7 @@ namespace { using std::string; using std::vector; +using mujoco::user::FilePath; using tinyxml2::XMLElement; void ReadPluginConfigs(tinyxml2::XMLElement* elem, mjsPlugin* p) { @@ -2553,7 +2554,12 @@ void mjXReader::OneFlexcomp(XMLElement* elem, mjsBody* pbody) { ReadAttr(elem, "scale", 3, fcomp.scale, text); ReadAttr(elem, "mass", 1, &fcomp.mass, text); ReadAttr(elem, "inertiabox", 1, &fcomp.inertiabox, text); - fcomp.file = ReadAttrFile(elem, "file", modelfiledir_).value_or(""); + auto maybe_file = ReadAttrFile(elem, "file", modelfiledir_); + if (maybe_file.has_value()) { + fcomp.file = std::move(maybe_file.value().Str()); + } else { + fcomp.file = ""; + } if (ReadAttrTxt(elem, "material", material)) { mjs_setString(dflex.material, material.c_str()); } @@ -3171,14 +3177,18 @@ void mjXReader::Asset(XMLElement* section, const mjVFS* vfs) { } // separate files - std::vector cubefiles(6); - cubefiles[0] = ReadAttrFile(elem, "fileright", TextureDir()).value_or(""); - cubefiles[1] = ReadAttrFile(elem, "fileleft", TextureDir()).value_or(""); - cubefiles[2] = ReadAttrFile(elem, "fileup", TextureDir()).value_or(""); - cubefiles[3] = ReadAttrFile(elem, "filedown", TextureDir()).value_or(""); - cubefiles[4] = ReadAttrFile(elem, "filefront", TextureDir()).value_or(""); - cubefiles[5] = ReadAttrFile(elem, "fileback", TextureDir()).value_or(""); + std::vector cubefiles(6); + std::vector cubefile_names = {"fileright", "fileleft", + "fileup", "filedown", + "filefront", "fileback"}; for (int i = 0; i < cubefiles.size(); i++) { + auto maybe_file = ReadAttrFile(elem, cubefile_names[i].c_str(), + TextureDir()); + if (maybe_file.has_value()) { + cubefiles[i] = maybe_file.value().Str(); + } else { + cubefiles[i] = ""; + } mjs_setInStringVec(ptex->cubefiles, i, cubefiles[i].c_str()); } } @@ -3264,7 +3274,7 @@ void mjXReader::Asset(XMLElement* section, const mjVFS* vfs) { // model sub-element else if (name=="model") { - auto filename = modelfiledir_ + ReadAttrFile(elem, "file", "").value(); + auto filename = modelfiledir_ + ReadAttrFile(elem, "file").value(); // parse the child std::array error; @@ -4227,36 +4237,35 @@ mjsDefault* mjXReader::GetClass(XMLElement* section) { return def; } -void mjXReader::SetModelFileDir(std::string modelfiledir) { - modelfiledir_ = modelfiledir; +void mjXReader::SetModelFileDir(const std::string& modelfiledir) { + modelfiledir_ = FilePath(modelfiledir); } -void mjXReader::SetAssetDir(std::string assetdir) { - assetdir_ = assetdir; +void mjXReader::SetAssetDir(const std::string& assetdir) { + assetdir_ = FilePath(assetdir); } -void mjXReader::SetMeshDir(std::string meshdir) { - meshdir_ = meshdir; +void mjXReader::SetMeshDir(const std::string& meshdir) { + meshdir_ = FilePath(meshdir); } -void mjXReader::SetTextureDir(std::string texturedir) { - texturedir_ = texturedir; +void mjXReader::SetTextureDir(const std::string& texturedir) { + texturedir_ = FilePath(texturedir); } -std::string mjXReader::AssetDir() const { - return mjuu_combinePaths(modelfiledir_, assetdir_); +FilePath mjXReader::AssetDir() const { + return modelfiledir_ + assetdir_; } -std::string mjXReader::MeshDir() const { +FilePath mjXReader::MeshDir() const { if (meshdir_.empty()) { return AssetDir(); } - return mjuu_combinePaths(modelfiledir_, meshdir_); + return modelfiledir_ + meshdir_; } - -std::string mjXReader::TextureDir() const { +FilePath mjXReader::TextureDir() const { if (texturedir_.empty()) { return AssetDir(); } - return mjuu_combinePaths(modelfiledir_, texturedir_); + return modelfiledir_ + texturedir_; } diff --git a/src/xml/xml_native_reader.h b/src/xml/xml_native_reader.h index 8e4e8cf4..4d622f5a 100644 --- a/src/xml/xml_native_reader.h +++ b/src/xml/xml_native_reader.h @@ -22,6 +22,7 @@ #include #include +#include "user/user_util.h" #include "xml/xml_base.h" #include "xml/xml_util.h" @@ -33,13 +34,13 @@ class mjXReader : public mjXBase { void Parse(tinyxml2::XMLElement* root, const mjVFS* vfs = nullptr); // parse XML document void PrintSchema(std::stringstream& str, bool html, bool pad); // print text or HTML schema - void SetModelFileDir(std::string modelfiledir); - const std::string& ModelFileDir() const { return modelfiledir_; } + void SetModelFileDir(const std::string& modelfiledir); + const mujoco::user::FilePath& ModelFileDir() const { return modelfiledir_; } // setters for directory defaults - void SetAssetDir(std::string assetdir); - void SetMeshDir(std::string meshdir); - void SetTextureDir(std::string texturedir); + void SetAssetDir(const std::string& assetdir); + void SetMeshDir(const std::string& meshdir); + void SetTextureDir(const std::string& texturedir); // XML sections embedded in all formats static void Compiler(tinyxml2::XMLElement* section, mjSpec* spec); // compiler section @@ -82,20 +83,20 @@ class mjXReader : public mjXBase { void OneFlexcomp(tinyxml2::XMLElement* elem, mjsBody* pbody); void OnePlugin(tinyxml2::XMLElement* elem, mjsPlugin* plugin); - mjXSchema schema; // schema used for validation - mjsDefault* GetClass(tinyxml2::XMLElement* section); // get default class name + mjXSchema schema; // schema used for validation + mjsDefault* GetClass(tinyxml2::XMLElement* section); // get default class name bool readingdefaults; // true while reading defaults // accessors for directory defaults - std::string AssetDir() const; - std::string MeshDir() const; - std::string TextureDir() const; + mujoco::user::FilePath AssetDir() const; + mujoco::user::FilePath MeshDir() const; + mujoco::user::FilePath TextureDir() const; - std::string modelfiledir_; - std::string assetdir_; - std::string meshdir_; - std::string texturedir_; + mujoco::user::FilePath modelfiledir_; + mujoco::user::FilePath assetdir_; + mujoco::user::FilePath meshdir_; + mujoco::user::FilePath texturedir_; }; // MJCF schema diff --git a/src/xml/xml_util.cc b/src/xml/xml_util.cc index a0740cf0..48ad62de 100644 --- a/src/xml/xml_util.cc +++ b/src/xml/xml_util.cc @@ -45,6 +45,7 @@ namespace { using tinyxml2::XMLAttribute; using tinyxml2::XMLElement; +using mujoco::user::FilePath; namespace mju = ::mujoco::util; @@ -74,6 +75,36 @@ static std::optional ParseInfOrNan(const std::string& s) { return std::nullopt; } +FilePath ResolveFilePath(XMLElement* e, const FilePath& filename, + const FilePath& dir) { + std::string path = ""; + if (filename.IsAbs()) { + return filename; + } + + // TODO(kylebayes): We first look in the base model directory for files to + // remain backwards compatible. + FilePath fullname = dir + filename; + mjResource *resource = mju_openResource(fullname.c_str(), nullptr, + nullptr, 0); + if (resource != nullptr) { + mju_closeResource(resource); + return filename; + } + + XMLElement* parent = e->Parent()->ToElement(); + for (; parent; parent = parent->Parent()->ToElement()) { + if (!std::strcmp(parent->Value(), "include")) { + auto file_attr = mjXUtil::ReadAttrStr(parent, "dir", false); + if (file_attr.has_value()) { + path = file_attr.value(); + } + break; + } + } + return FilePath(path) + filename; +} + } // namespace @@ -155,36 +186,6 @@ XMLElement* NextSiblingElement(XMLElement* e, const char* name) { return nullptr; } -static std::string ResolveFilePath(XMLElement* e, std::string filename, - const std::string& dir) { - std::string path = ""; - if (mjuu_isabspath(filename)) { - return filename; - } - - // TODO(kylebayes): We first look in the base model directory for files to - // remain backwards compatible. - std::string full_filename = mjuu_combinePaths(dir, filename); - mjResource *resource = mju_openResource(full_filename.c_str(), nullptr, - nullptr, 0); - if (resource != nullptr) { - mju_closeResource(resource); - return filename; - } - - XMLElement* parent = e->Parent()->ToElement(); - for (; parent; parent = parent->Parent()->ToElement()) { - if (!std::strcmp(parent->Value(), "include")) { - auto file_attr = mjXUtil::ReadAttrStr(parent, "dir", false); - if (file_attr.has_value()) { - path = file_attr.value(); - } - break; - } - } - return mjuu_combinePaths(path, filename); -} - // constructor mjXSchema::mjXSchema(const char* schema[][mjXATTRNUM], unsigned nrow) { // set name and type @@ -621,14 +622,15 @@ mjXUtil::ReadAttrStr(XMLElement* elem, const char* attr, bool required) { } // if attribute is present, return attribute as a filename -std::optional +std::optional mjXUtil::ReadAttrFile(XMLElement* elem, const char* attr, - const std::string& dir, bool required) { + const FilePath& dir, bool required) { auto maybe_str = ReadAttrStr(elem, attr, required); if (!maybe_str.has_value()) { return std::nullopt; } - return ResolveFilePath(elem, maybe_str.value(), dir); + FilePath filename(maybe_str.value()); + return ResolveFilePath(elem, filename, dir); } // if attribute is present, return numerical value of attribute diff --git a/src/xml/xml_util.h b/src/xml/xml_util.h index f1799cb4..7ef5913e 100644 --- a/src/xml/xml_util.h +++ b/src/xml/xml_util.h @@ -26,6 +26,7 @@ #include "tinyxml2.h" +#include "user/user_util.h" // error string copy void mjCopyError(char* dst, const char* src, int maxlen); @@ -101,26 +102,32 @@ class mjXUtil { // if attribute is present, return vector of numerical data template - static std::optional> ReadAttrVec(tinyxml2::XMLElement* elem, const char* attr, + static std::optional> ReadAttrVec(tinyxml2::XMLElement* elem, + const char* attr, bool required = false); // if attribute is present, return attribute as a string - static std::optional ReadAttrStr(tinyxml2::XMLElement* elem, const char* attr, + static std::optional ReadAttrStr(tinyxml2::XMLElement* elem, + const char* attr, bool required = false); // if attribute is present, return attribute as a filename - static std::optional ReadAttrFile(tinyxml2::XMLElement* elem, const char* attr, - const std::string& dir = "", bool required = false); + static std::optional + ReadAttrFile(tinyxml2::XMLElement* elem, const char* attr, + const mujoco::user::FilePath& dir = mujoco::user::FilePath(), + bool required = false); // if attribute is present, return numerical value of attribute template - static std::optional ReadAttrNum(tinyxml2::XMLElement* elem, const char* attr, + static std::optional ReadAttrNum(tinyxml2::XMLElement* elem, + const char* attr, bool required = false); // if attribute is present, return array of numerical data // N should be small as data is allocated on the stack template - static std::optional> ReadAttrArr(tinyxml2::XMLElement* elem, const char* attr, + static std::optional> ReadAttrArr(tinyxml2::XMLElement* elem, + const char* attr, bool required = false) { std::array arr; int n = 0; diff --git a/test/user/user_util_test.cc b/test/user/user_util_test.cc index 02b30c08..9873c74b 100644 --- a/test/user/user_util_test.cc +++ b/test/user/user_util_test.cc @@ -46,6 +46,11 @@ TEST_F(UserUtilTest, PathReduce2) { EXPECT_EQ(path.Str(), "../hello/world/"); } +TEST_F(UserUtilTest, PathReduce3) { + FilePath path = FilePath("../../hello/world.txt"); + EXPECT_EQ(path.Str(), "../../hello/world.txt"); +} + TEST_F(UserUtilTest, PathReduceWin) { FilePath path = FilePath("C:\\hello\\..\\world"); EXPECT_EQ(path.Str(), "C:\\world"); diff --git a/test/xml/xml_native_reader_test.cc b/test/xml/xml_native_reader_test.cc index 817ab79b..22fc75b8 100644 --- a/test/xml/xml_native_reader_test.cc +++ b/test/xml/xml_native_reader_test.cc @@ -649,7 +649,7 @@ TEST_F(XMLReaderTest, IncludeTest) { std::array error; mjModel* model = LoadModelFromString(xml, error.data(), error.size(), vfs.get()); - ASSERT_THAT(model, NotNull()); + ASSERT_THAT(model, NotNull()) << error.data(); EXPECT_EQ(mj_name2id(model, mjOBJ_GEOM, "ball"), 2); EXPECT_EQ(mj_name2id(model, mjOBJ_GEOM, "another_box"), 3); mj_deleteModel(model); @@ -733,12 +733,14 @@ TEST_F(XMLReaderTest, IncludePathTest) { fs.ChangeDirectory("submodels/"); fs.AddFile("model1.xml", (const unsigned char*) xml1, sizeof(xml1)); fs.AddFile("model2.xml", (const unsigned char*) xml2, sizeof(xml2)); - fs.AddFile("subsubmodels/model3.xml", (const unsigned char*) xml3, sizeof(xml3)); + fs.AddFile("subsubmodels/model3.xml", (const unsigned char*) xml3, + sizeof(xml3)); fs.ChangeDirectory("/"); - mjModel* model = mj_loadXML(modelpath.c_str(), nullptr, - nullptr, 0); - ASSERT_THAT(model, NotNull()); + std::array error; + mjModel* model = mj_loadXML(modelpath.c_str(), nullptr, error.data(), + error.size()); + ASSERT_THAT(model, NotNull()) << error.data(); EXPECT_EQ(mj_name2id(model, mjOBJ_GEOM, "ball"), 2); EXPECT_EQ(mj_name2id(model, mjOBJ_GEOM, "another_box"), 3); mj_deleteModel(model); @@ -920,9 +922,11 @@ TEST_F(XMLReaderTest, IncludeAssetsTest) { std::string modelpath = fs.FullPath("model.xml"); // loading the file should be successful - mjModel* model = mj_loadXML(modelpath.c_str(), nullptr, nullptr, 0); + std::array error; + mjModel* model = mj_loadXML(modelpath.c_str(), nullptr, error.data(), + error.size()); - EXPECT_THAT(model, NotNull()); + ASSERT_THAT(model, NotNull()) << error.data(); mj_deleteModel(model); } @@ -969,7 +973,7 @@ TEST_F(XMLReaderTest, FallbackIncludeAssetsTest) { std::array error; mjModel* model = mj_loadXML(modelpath.c_str(), nullptr, error.data(), error.size()); - EXPECT_THAT(model, NotNull()); + ASSERT_THAT(model, NotNull()) << error.data(); mj_deleteModel(model); } @@ -1013,7 +1017,7 @@ TEST_F(XMLReaderTest, IncludeAbsoluteTest) { // loading the file should be successful mjModel* model = mj_loadXML(modelpath.c_str(), nullptr, error.data(), error.size()); - EXPECT_THAT(model, NotNull()); + ASSERT_THAT(model, NotNull()) << error.data(); mj_deleteModel(model); } @@ -1054,7 +1058,7 @@ TEST_F(XMLReaderTest, IncludeAbsoluteMeshDirTest) { // loading the file should be successful mjModel* model = mj_loadXML(modelpath.c_str(), nullptr, error.data(), error.size()); - ASSERT_THAT(model, NotNull()) << "Failed to load model: " << error.data(); + ASSERT_THAT(model, NotNull()) << error.data(); mj_deleteModel(model); } From 5ac5cfb618c11ff4aa0199b25c3856da3d194703 Mon Sep 17 00:00:00 2001 From: Kyle Bayes Date: Tue, 23 Jul 2024 13:38:28 -0700 Subject: [PATCH 03/10] Support directories in VFS via mj_addBufferVFS. PiperOrigin-RevId: 655286683 Change-Id: I846376a0571d8df28d979beb5981a0cbccd7a04f --- python/mujoco/structs.cc | 21 ++++- src/user/user_flexcomp.cc | 13 ++- src/user/user_mesh.cc | 8 +- src/user/user_objects.cc | 16 ++-- src/user/user_objects.h | 3 +- src/user/user_resource.cc | 21 +++-- src/user/user_resource.h | 6 +- src/user/user_vfs.cc | 19 +++-- src/xml/xml.cc | 19 +++-- src/xml/xml_api.cc | 2 +- src/xml/xml_native_reader.cc | 51 ++++++------ src/xml/xml_native_reader.h | 13 +-- src/xml/xml_util.cc | 8 +- src/xml/xml_util.h | 2 + test/user/user_cache_test.cc | 4 +- test/user/user_resource_test.cc | 12 +-- test/user/user_vfs_test.cc | 9 +-- test/xml/xml_native_reader_test.cc | 126 +++++++++++++++-------------- 18 files changed, 197 insertions(+), 156 deletions(-) diff --git a/python/mujoco/structs.cc b/python/mujoco/structs.cc index 373593c7..66566fe7 100644 --- a/python/mujoco/structs.cc +++ b/python/mujoco/structs.cc @@ -16,7 +16,9 @@ #include +#include #include +#include #include #include #include @@ -84,6 +86,22 @@ constexpr auto XArrayShapeImpl(const std::string_view dim1_str) { inline std::size_t NConMax(const mjData* d) { return d->narena / sizeof(mjContact); } + +// strip path prefix from filename and make lowercase +std::string StripPath(const char* name) { + std::string filename(name); + size_t start = filename.find_last_of("/\\"); + + // get name without path + if (start != std::string::npos) { + filename = filename.substr(start + 1, filename.size() - start - 1); + } + + // make lowercase + std::transform(filename.begin(), filename.end(), filename.begin(), + [](unsigned char c) { return std::tolower(c); }); + return filename; +} } // namespace // ==================== MJOPTION =============================================== @@ -323,8 +341,9 @@ static raw::MjModel* LoadModelFileImpl( mj_defaultVFS(&vfs); vfs_ptr = &vfs; for (const auto& asset : assets) { + std::string buffer_name = StripPath(asset.name); const int vfs_error = InterceptMjErrors(mj_addBufferVFS)( - vfs_ptr, asset.name, asset.content, asset.content_size); + vfs_ptr, buffer_name.c_str(), asset.content, asset.content_size); if (vfs_error) { throw py::value_error("assets dict is too big"); } diff --git a/src/user/user_flexcomp.cc b/src/user/user_flexcomp.cc index 1662fbd8..5dfc6943 100644 --- a/src/user/user_flexcomp.cc +++ b/src/user/user_flexcomp.cc @@ -900,12 +900,12 @@ bool mjCFlexcomp::MakeMesh(mjCModel* model, char* error, int error_sz) { } // load resource - std::string filename = mjuu_combinePaths(mjs_getString(model->spec.modelfiledir), - mjs_getString(model->spec.meshdir), file); + std::string filename = mjuu_combinePaths(mjs_getString(model->spec.meshdir), file); mjResource* resource = nullptr; try { - resource = mjCBase::LoadResource(filename, 0); + resource = mjCBase::LoadResource(mjs_getString(model->spec.modelfiledir), + filename, 0); } catch (mjCError err) { return comperr(error, err.message, error_sz); } @@ -999,12 +999,11 @@ bool mjCFlexcomp::MakeGMSH(mjCModel* model, char* error, int error_sz) { } // open resource - std::string filename = mjuu_combinePaths(mjs_getString(model->spec.modelfiledir), - mjs_getString(model->spec.meshdir), file); mjResource* resource = nullptr; - try { - resource = mjCBase::LoadResource(filename, 0); + std::string filename = mjuu_combinePaths(mjs_getString(model->spec.meshdir), file); + resource = mjCBase::LoadResource(mjs_getString(model->spec.modelfiledir), + filename, 0); } catch (mjCError err) { return comperr(error, err.message, error_sz); } diff --git a/src/user/user_mesh.cc b/src/user/user_mesh.cc index 1b2d1022..0d455eb9 100644 --- a/src/user/user_mesh.cc +++ b/src/user/user_mesh.cc @@ -402,8 +402,8 @@ void mjCMesh::Compile(const mjVFS* vfs) { throw mjCError(this, "unsupported content type: '%s'", asset_type.c_str()); } - std::string filename = mjuu_combinePaths(model->modelfiledir_, model->meshdir_, file_); - mjResource* resource = LoadResource(filename, vfs); + std::string filename = mjuu_combinePaths(model->meshdir_, file_); + mjResource* resource = LoadResource(model->modelfiledir_, filename, vfs); try { if (asset_type == "model/stl") { @@ -2095,8 +2095,8 @@ void mjCSkin::Compile(const mjVFS* vfs) { throw mjCError(this, "Unknown skin file type: %s", file_.c_str()); } - std::string filename = mjuu_combinePaths(model->modelfiledir_, model->meshdir_, file_); - mjResource* resource = LoadResource(filename, vfs); + std::string filename = mjuu_combinePaths(model->meshdir_, file_); + mjResource* resource = LoadResource(model->modelfiledir_, filename, vfs); try { LoadSKN(resource); diff --git a/src/user/user_objects.cc b/src/user/user_objects.cc index c647f4b3..9ff645b2 100644 --- a/src/user/user_objects.cc +++ b/src/user/user_objects.cc @@ -701,10 +701,12 @@ void mjCBase::NameSpace(const mjCModel* m) { // load resource if found (fallback to OS filesystem) -mjResource* mjCBase::LoadResource(std::string filename, const mjVFS* vfs) { +mjResource* mjCBase::LoadResource(const std::string& modelfiledir, + const std::string& filename, + const mjVFS* vfs) { // try reading from provided VFS or fallback to OS filesystem std::array error; - mjResource* resource = mju_openResource(filename.c_str(), vfs, + mjResource* resource = mju_openResource(modelfiledir.c_str(), filename.c_str(), vfs, error.data(), error.size()); if (!resource) { throw mjCError(nullptr, "%s", error.data()); @@ -3159,8 +3161,8 @@ void mjCHField::Compile(const mjVFS* vfs) { throw mjCError(this, "unsupported content type: '%s'", asset_type.c_str()); } - std::string filename = mjuu_combinePaths(model->modelfiledir_, model->meshdir_, file_); - mjResource* resource = LoadResource(filename, vfs); + std::string filename = mjuu_combinePaths(model->meshdir_, file_); + mjResource* resource = LoadResource(model->modelfiledir_, filename, vfs); try { if (asset_type == "image/png") { @@ -3594,7 +3596,7 @@ void mjCTexture::LoadFlip(std::string filename, const mjVFS* vfs, throw mjCError(this, "unsupported content type: '%s'", asset_type.c_str()); } - mjResource* resource = LoadResource(filename, vfs); + mjResource* resource = LoadResource(model->modelfiledir_, filename, vfs); try { if (asset_type == "image/png") { @@ -3797,7 +3799,7 @@ void mjCTexture::LoadCubeSeparate(const mjVFS* vfs) { } // make filename - std::string filename = mjuu_combinePaths(model->modelfiledir_, model->texturedir_, cubefiles_[i]); + std::string filename = mjuu_combinePaths(model->texturedir_, cubefiles_[i]); // load PNG or custom unsigned int w, h; @@ -3895,7 +3897,7 @@ void mjCTexture::Compile(const mjVFS* vfs) { } // make filename - std::string filename = mjuu_combinePaths(model->modelfiledir_, model->texturedir_, file_); + std::string filename = mjuu_combinePaths(model->texturedir_, file_); // dispatch if (type==mjTEXTURE_2D) { diff --git a/src/user/user_objects.h b/src/user/user_objects.h index 4e508b04..18d9b886 100644 --- a/src/user/user_objects.h +++ b/src/user/user_objects.h @@ -181,7 +181,8 @@ class mjCBase : public mjCBase_ { public: // load resource if found (fallback to OS filesystem) - static mjResource* LoadResource(std::string filename, const mjVFS* vfs); + static mjResource* LoadResource(const std::string& modelfiledir, + const std::string& filename, const mjVFS* vfs); // Get and sanitize content type from raw_text if not empty, otherwise parse // content type from resource_name; throw on failure diff --git a/src/user/user_resource.cc b/src/user/user_resource.cc index 805a3156..980d7ee9 100644 --- a/src/user/user_resource.cc +++ b/src/user/user_resource.cc @@ -23,6 +23,7 @@ #include #include #include +#include #include #if defined (__unix__) || (defined (__APPLE__) && defined (__MACH__)) @@ -117,8 +118,8 @@ int FileModified(const mjResource* resource, const char*timestamp) { // open the given resource; if the name doesn't have a prefix matching with a // resource provider, then the OS filesystem is used -mjResource* mju_openResource(const char* name, const mjVFS* vfs, - char* error, size_t nerror) { +mjResource* mju_openResource(const char* dir, const char* name, + const mjVFS* vfs, char* error, size_t nerror) { // no error so far if (error) { error[0] = '\0'; @@ -136,8 +137,10 @@ mjResource* mju_openResource(const char* name, const mjVFS* vfs, // clear out resource memset(resource, 0, sizeof(mjResource)); - // copy name - resource->name = (char*) mju_malloc(sizeof(char) * (strlen(name) + 1)); + // make space for filename + std::string fullname = mjuu_combinePaths(dir, name); + std::size_t n = fullname.size(); + resource->name = (char*) mju_malloc(sizeof(char) * (n + 1)); if (resource->name == nullptr) { if (error) { strncpy(error, "could not allocate memory", nerror); @@ -146,10 +149,11 @@ mjResource* mju_openResource(const char* name, const mjVFS* vfs, mju_closeResource(resource); return nullptr; } - memcpy(resource->name, name, sizeof(char) * (strlen(name) + 1)); // first priority is to check the VFS if (vfs != nullptr) { + memcpy(resource->name, name, + sizeof(char) * (std::strlen(name) + 1)); const mjpResourceProvider* provider = GetVfsResourceProvider(); resource->data = (void*) vfs; resource->provider = provider; @@ -158,8 +162,11 @@ mjResource* mju_openResource(const char* name, const mjVFS* vfs, } } + // copy full path over + memcpy(resource->name, fullname.c_str(), sizeof(char) * (n + 1)); + // find provider based off prefix of name - const mjpResourceProvider* provider = mjp_getResourceProvider(name); + const mjpResourceProvider* provider = mjp_getResourceProvider(resource->name); if (provider != nullptr) { resource->provider = provider; resource->data = nullptr; @@ -170,7 +177,7 @@ mjResource* mju_openResource(const char* name, const mjVFS* vfs, if (error) { snprintf(error, nerror, "could not open '%s'" "using a resource provider matching prefix '%s'", - name, provider->prefix); + resource->name, provider->prefix); } mju_closeResource(resource); diff --git a/src/user/user_resource.h b/src/user/user_resource.h index e64874cb..82a70ed6 100644 --- a/src/user/user_resource.h +++ b/src/user/user_resource.h @@ -18,8 +18,6 @@ #define MUJOCO_SRC_ENGINE_ENGINE_RESOURCE_H_ #include -#include -#include #include #include @@ -30,8 +28,8 @@ extern "C" { // open the given resource; if the name doesn't have a prefix matching with a // resource provider, then the OS filesystem is used -MJAPI mjResource* mju_openResource(const char* name, const mjVFS* vfs, - char* error, std::size_t nerror); +MJAPI mjResource* mju_openResource(const char* dir, const char* name, + const mjVFS* vfs, char* error, size_t nerror); // close the given resource; no-op if resource is NULL MJAPI void mju_closeResource(mjResource* resource); diff --git a/src/user/user_vfs.cc b/src/user/user_vfs.cc index 8bc484a2..02c8829d 100644 --- a/src/user/user_vfs.cc +++ b/src/user/user_vfs.cc @@ -142,9 +142,13 @@ int Open(mjResource* resource) { const VFS* cvfs = GetVFSImpl(vfs); const VFSFile* file = cvfs->GetFile(StripPath(resource->name)); if (file == nullptr) { - return 0; + file = cvfs->GetFile(FilePath(resource->name)); + if (file == nullptr) { + return 0; + } } + resource->data = (void*) file; resource->timestamp[0] = '\0'; if (file->filestamp) { mju_encodeBase64(resource->timestamp, (uint8_t*) &file->filestamp, @@ -160,8 +164,7 @@ int Read(mjResource* resource, const void** buffer) { return -1; } - const VFS* vfs = GetVFSImpl(static_cast(resource->data)); - const VFSFile* file = vfs->GetFile(StripPath(resource->name)); + const VFSFile* file = static_cast(resource->data); if (file == nullptr) { *buffer = nullptr; return -1; @@ -193,8 +196,7 @@ int Modified(const mjResource* resource, const char* timestamp) { if (!filestamp) return 3; // no hash (assume modified) if (resource) { - const VFS* cvfs = GetVFSImpl(static_cast(resource->data)); - const VFSFile* file = cvfs->GetFile(StripPath(resource->name)); + const VFSFile* file = static_cast(resource->data); if (file == nullptr) return 4; // missing file (assume modified) if (!file->filestamp) return 5; // missing filestamp (assume modified) @@ -245,7 +247,7 @@ int mj_addBufferVFS(mjVFS* vfs, const char* name, const void* buffer, std::vector inbuffer; VFS* cvfs = GetVFSImpl(vfs); VFSFile* file; - if (!(file = cvfs->AddFile(StripPath(name), std::move(inbuffer), 0))) { + if (!(file = cvfs->AddFile(FilePath(name), std::move(inbuffer), 0))) { return 2; // AddFile failed, repeated name } file->filedata.reserve(nbuffer); @@ -256,7 +258,10 @@ int mj_addBufferVFS(mjVFS* vfs, const char* name, const void* buffer, // delete file from VFS, return 0: success, -1: not found in VFS int mj_deleteFileVFS(mjVFS* vfs, const char* filename) { VFS* cvfs = GetVFSImpl(vfs); - return cvfs->DeleteFile(StripPath(filename)); + if (cvfs->DeleteFile(StripPath(filename))) { + return cvfs->DeleteFile(FilePath(filename)); + } + return 0; } // delete all files from VFS diff --git a/src/xml/xml.cc b/src/xml/xml.cc index 8459902f..e299cea2 100644 --- a/src/xml/xml.cc +++ b/src/xml/xml.cc @@ -156,8 +156,8 @@ void IncludeXML(mjXReader& reader, XMLElement* elem, } // get filename - auto file_attr = mjXUtil::ReadAttrFile(elem, "file", reader.ModelFileDir(), - true); + auto file_attr = mjXUtil::ReadAttrFile(elem, "file", vfs, + reader.ModelFileDir(), true); if (!file_attr.has_value()) { throw mjXError(elem, "Include element missing file attribute"); } @@ -171,17 +171,17 @@ void IncludeXML(mjXReader& reader, XMLElement* elem, // TODO: b/325905702 - We have a messy wrapper here to remain backwards // compatible, which will be removed in the near future. - FilePath fullname = reader.ModelFileDir() + filename; - // legacy behavior: try to load in top level directory std::array error; - mjResource *resource = mju_openResource(fullname.c_str(), vfs, + mjResource *resource = mju_openResource(reader.ModelFileDir().c_str(), + filename.c_str(), vfs, error.data(), error.size()); if (resource == nullptr) { // new behavior: try to load in relative directory if (!filename.IsAbs()) { - fullname = dir + filename; - resource = mju_openResource(fullname.c_str(), vfs, error.data(), error.size()); + FilePath fullname = dir + filename; + resource = mju_openResource(reader.ModelFileDir().c_str(), + fullname.c_str(), vfs, error.data(), error.size()); } } @@ -195,7 +195,6 @@ void IncludeXML(mjXReader& reader, XMLElement* elem, int ninclude_dir = 0; mju_getResourceDir(resource, &include_dir, &ninclude_dir); FilePath next_dir = FilePath(std::string(include_dir, ninclude_dir)); - next_dir = dir + next_dir; elem->SetAttribute("dir", next_dir.c_str()); const char* xmlstring = nullptr; @@ -285,7 +284,7 @@ mjSpec* ParseXML(const char* filename, const mjVFS* vfs, // get data source const char* xmlstring = nullptr; std::array rerror; - mjResource* resource = mju_openResource(filename, vfs, + mjResource* resource = mju_openResource("", filename, vfs, rerror.data(), rerror.size()); if (resource == nullptr) { std::snprintf(error, nerror, "ParseXML: %s", rerror.data()); @@ -352,7 +351,7 @@ mjSpec* ParseXML(const char* filename, const mjVFS* vfs, std::unordered_set included = {filename}; mjXReader parser; parser.SetModelFileDir(mjs_getString(spec->modelfiledir)); - IncludeXML(parser, root, parser.ModelFileDir(), vfs, included); + IncludeXML(parser, root, FilePath(), vfs, included); // parse MuJoCo model parser.SetModel(spec); diff --git a/src/xml/xml_api.cc b/src/xml/xml_api.cc index 2107a80e..31deeec8 100644 --- a/src/xml/xml_api.cc +++ b/src/xml/xml_api.cc @@ -192,7 +192,7 @@ int mj_printSchema(const char* filename, char* buffer, int buffer_sz, int flg_ht // load model from binary MJB resource mjModel* mj_loadModel(const char* filename, const mjVFS* vfs) { std::array error; - mjResource* resource = mju_openResource(filename, vfs, + mjResource* resource = mju_openResource("", filename, vfs, error.data(), error.size()); if (resource == nullptr) { mju_warning("%s", error.data()); diff --git a/src/xml/xml_native_reader.cc b/src/xml/xml_native_reader.cc index 3a87a939..2dc7138b 100644 --- a/src/xml/xml_native_reader.cc +++ b/src/xml/xml_native_reader.cc @@ -893,7 +893,7 @@ void mjXReader::Parse(XMLElement* root, const mjVFS* vfs) { readingdefaults = true; for (XMLElement* section = FirstChildElement(root, "default"); section; section = NextSiblingElement(section, "default")) { - Default(section, nullptr); + Default(section, nullptr, vfs); } readingdefaults = false; @@ -919,7 +919,7 @@ void mjXReader::Parse(XMLElement* root, const mjVFS* vfs) { for (XMLElement* section = FirstChildElement(root, "deformable"); section; section = NextSiblingElement(section, "deformable")) { - Deformable(section); + Deformable(section, vfs); } for (XMLElement* section = FirstChildElement(root, "equality"); section; @@ -949,7 +949,7 @@ void mjXReader::Parse(XMLElement* root, const mjVFS* vfs) { for (XMLElement* section = FirstChildElement(root, "worldbody"); section; section = NextSiblingElement(section, "worldbody")) { - Body(section, mjs_findBody(spec, "world"), nullptr); + Body(section, mjs_findBody(spec, "world"), nullptr, vfs); } } @@ -1389,7 +1389,7 @@ void mjXReader::OneFlex(XMLElement* elem, mjsFlex* pflex) { // mesh element parser -void mjXReader::OneMesh(XMLElement* elem, mjsMesh* pmesh) { +void mjXReader::OneMesh(XMLElement* elem, mjsMesh* pmesh, const mjVFS* vfs) { int n; string text, name, content_type; @@ -1400,7 +1400,7 @@ void mjXReader::OneMesh(XMLElement* elem, mjsMesh* pmesh) { if (ReadAttrTxt(elem, "content_type", content_type)) { *pmesh->content_type = content_type; } - auto file = ReadAttrFile(elem, "file", MeshDir()); + auto file = ReadAttrFile(elem, "file", vfs, MeshDir()); if (file) { mjs_setString(pmesh->file, file->c_str()); } @@ -1461,7 +1461,7 @@ void mjXReader::OneMesh(XMLElement* elem, mjsMesh* pmesh) { // skin element parser -void mjXReader::OneSkin(XMLElement* elem, mjsSkin* pskin) { +void mjXReader::OneSkin(XMLElement* elem, mjsSkin* pskin, const mjVFS* vfs) { string text, name, material; float data[4]; @@ -1469,7 +1469,7 @@ void mjXReader::OneSkin(XMLElement* elem, mjsSkin* pskin) { if (ReadAttrTxt(elem, "name", name)) { mjs_setString(pskin->name, name.c_str()); } - auto file = ReadAttrFile(elem, "file", AssetDir()); + auto file = ReadAttrFile(elem, "file", vfs, AssetDir()); if (file.has_value()) { mjs_setString(pskin->file, file->c_str()); } @@ -2536,7 +2536,7 @@ void mjXReader::OneComposite(XMLElement* elem, mjsBody* pbody, mjsDefault* def) // make flexcomp -void mjXReader::OneFlexcomp(XMLElement* elem, mjsBody* pbody) { +void mjXReader::OneFlexcomp(XMLElement* elem, mjsBody* pbody, const mjVFS* vfs) { string text, material; int n; @@ -2554,7 +2554,7 @@ void mjXReader::OneFlexcomp(XMLElement* elem, mjsBody* pbody) { ReadAttr(elem, "scale", 3, fcomp.scale, text); ReadAttr(elem, "mass", 1, &fcomp.mass, text); ReadAttr(elem, "inertiabox", 1, &fcomp.inertiabox, text); - auto maybe_file = ReadAttrFile(elem, "file", modelfiledir_); + auto maybe_file = ReadAttrFile(elem, "file", vfs, modelfiledir_); if (maybe_file.has_value()) { fcomp.file = std::move(maybe_file.value().Str()); } else { @@ -2690,7 +2690,7 @@ void mjXReader::OnePlugin(XMLElement* elem, mjsPlugin* plugin) { //------------------ MJCF-specific sections -------------------------------------------------------- // default section parser -void mjXReader::Default(XMLElement* section, const mjsDefault* def) { +void mjXReader::Default(XMLElement* section, const mjsDefault* def, const mjVFS* vfs) { XMLElement* elem; string text, name; @@ -2721,7 +2721,7 @@ void mjXReader::Default(XMLElement* section, const mjsDefault* def) { name = elem->Value(); // read mesh - if (name=="mesh") OneMesh(elem, def->mesh); + if (name=="mesh") OneMesh(elem, def->mesh, vfs); // read material else if (name=="material") OneMaterial(elem, def->material); @@ -2775,7 +2775,7 @@ void mjXReader::Default(XMLElement* section, const mjsDefault* def) { // read default if (name=="default") { - Default(elem, def); + Default(elem, def, vfs); } // advance @@ -3136,7 +3136,7 @@ void mjXReader::Asset(XMLElement* section, const mjVFS* vfs) { if (ReadAttrTxt(elem, "content_type", content_type)) { mjs_setString(ptex->content_type, content_type.c_str()); } - auto file = ReadAttrFile(elem, "file", TextureDir()); + auto file = ReadAttrFile(elem, "file", vfs, TextureDir()); if (file.has_value()) { mjs_setString(ptex->file, file->c_str()); } @@ -3182,7 +3182,7 @@ void mjXReader::Asset(XMLElement* section, const mjVFS* vfs) { "fileup", "filedown", "filefront", "fileback"}; for (int i = 0; i < cubefiles.size(); i++) { - auto maybe_file = ReadAttrFile(elem, cubefile_names[i].c_str(), + auto maybe_file = ReadAttrFile(elem, cubefile_names[i].c_str(), vfs, TextureDir()); if (maybe_file.has_value()) { cubefiles[i] = maybe_file.value().Str(); @@ -3204,14 +3204,14 @@ void mjXReader::Asset(XMLElement* section, const mjVFS* vfs) { else if (name=="mesh") { // create mesh and parse mjsMesh* pmesh = mjs_addMesh(spec, def); - OneMesh(elem, pmesh); + OneMesh(elem, pmesh, vfs); } // skin sub-element... deprecate ??? else if (name=="skin") { // create skin and parse mjsSkin* pskin = mjs_addSkin(spec); - OneSkin(elem, pskin); + OneSkin(elem, pskin, vfs); } // hfield sub-element @@ -3230,7 +3230,7 @@ void mjXReader::Asset(XMLElement* section, const mjVFS* vfs) { if (ReadAttrTxt(elem, "content_type", content_type)) { mjs_setString(phf->content_type, content_type.c_str()); } - auto file = ReadAttrFile(elem, "file", AssetDir()); + auto file = ReadAttrFile(elem, "file", vfs, AssetDir()); if (file.has_value()) { mjs_setString(phf->file, file->c_str()); } @@ -3274,7 +3274,7 @@ void mjXReader::Asset(XMLElement* section, const mjVFS* vfs) { // model sub-element else if (name=="model") { - auto filename = modelfiledir_ + ReadAttrFile(elem, "file").value(); + auto filename = modelfiledir_ + ReadAttrFile(elem, "file", vfs).value(); // parse the child std::array error; @@ -3301,7 +3301,8 @@ void mjXReader::Asset(XMLElement* section, const mjVFS* vfs) { // body/world section parser; recursive -void mjXReader::Body(XMLElement* section, mjsBody* pbody, mjsFrame* frame) { +void mjXReader::Body(XMLElement* section, mjsBody* pbody, mjsFrame* frame, + const mjVFS* vfs) { string text, name; XMLElement* elem; int n; @@ -3427,7 +3428,7 @@ void mjXReader::Body(XMLElement* section, mjsBody* pbody, mjsFrame* frame) { // flexcomp sub-element else if (name=="flexcomp") { // parse flexcomp - OneFlexcomp(elem, pbody); + OneFlexcomp(elem, pbody, vfs); } // frame sub-element @@ -3459,7 +3460,7 @@ void mjXReader::Body(XMLElement* section, mjsBody* pbody, mjsFrame* frame) { ReadQuat(elem, "quat", pframe->quat, text); ReadAlternative(elem, pframe->alt); - Body(elem, pbody, pframe); + Body(elem, pbody, pframe, vfs); } // replicate sub-element @@ -3518,7 +3519,7 @@ void mjXReader::Body(XMLElement* section, mjsBody* pbody, mjsFrame* frame) { UpdateString(suffix, count, i); // process subtree - Body(elem, subtree, pframe); + Body(elem, subtree, pframe, vfs); // attach to parent if (mjs_attachFrame(pbody, pframe, /*prefix=*/"", suffix.c_str()) != 0) { @@ -3577,7 +3578,7 @@ void mjXReader::Body(XMLElement* section, mjsBody* pbody, mjsFrame* frame) { mjs_setFrame(pchild->element, frame); // make recursive call - Body(elem, pchild, nullptr); + Body(elem, pchild, nullptr, vfs); } // attachment @@ -3695,7 +3696,7 @@ void mjXReader::Equality(XMLElement* section) { // deformable section parser -void mjXReader::Deformable(XMLElement* section) { +void mjXReader::Deformable(XMLElement* section, const mjVFS* vfs) { string name; XMLElement* elem; @@ -3722,7 +3723,7 @@ void mjXReader::Deformable(XMLElement* section) { else if (name=="skin") { // create skin and parse mjsSkin* pskin = mjs_addSkin(spec); - OneSkin(elem, pskin); + OneSkin(elem, pskin, vfs); } // advance to next element diff --git a/src/xml/xml_native_reader.h b/src/xml/xml_native_reader.h index 4d622f5a..cfa87961 100644 --- a/src/xml/xml_native_reader.h +++ b/src/xml/xml_native_reader.h @@ -49,16 +49,17 @@ class mjXReader : public mjXBase { private: // XML section specific to MJCF - void Default(tinyxml2::XMLElement* section, const mjsDefault* def); // default section + void Default(tinyxml2::XMLElement* section, const mjsDefault* def, + const mjVFS* vfs); // default section void Extension(tinyxml2::XMLElement* section); // extension section void Custom(tinyxml2::XMLElement* section); // custom section void Visual(tinyxml2::XMLElement* section); // visual section void Statistic(tinyxml2::XMLElement* section); // statistic section void Asset(tinyxml2::XMLElement* section, const mjVFS* vfs); // asset section void Body(tinyxml2::XMLElement* section, mjsBody* pbody, - mjsFrame* pframe); // body/world section + mjsFrame* pframe, const mjVFS* vfs); // body/world section void Contact(tinyxml2::XMLElement* section); // contact section - void Deformable(tinyxml2::XMLElement* section); // deformable section + void Deformable(tinyxml2::XMLElement* section, const mjVFS* vfs); // deformable section void Equality(tinyxml2::XMLElement* section); // equality section void Tendon(tinyxml2::XMLElement* section); // tendon section void Actuator(tinyxml2::XMLElement* section); // actuator section @@ -67,8 +68,8 @@ class mjXReader : public mjXBase { // single element parsers, used in defaults and main body void OneFlex(tinyxml2::XMLElement* elem, mjsFlex* pflex); - void OneMesh(tinyxml2::XMLElement* elem, mjsMesh* pmesh); - void OneSkin(tinyxml2::XMLElement* elem, mjsSkin* pskin); + void OneMesh(tinyxml2::XMLElement* elem, mjsMesh* pmesh, const mjVFS* vfs); + void OneSkin(tinyxml2::XMLElement* elem, mjsSkin* pskin, const mjVFS* vfs); void OneMaterial(tinyxml2::XMLElement* elem, mjsMaterial* pmaterial); void OneJoint(tinyxml2::XMLElement* elem, mjsJoint* pjoint); void OneGeom(tinyxml2::XMLElement* elem, mjsGeom* pgeom); @@ -80,7 +81,7 @@ class mjXReader : public mjXBase { void OneTendon(tinyxml2::XMLElement* elem, mjsTendon* ptendon); void OneActuator(tinyxml2::XMLElement* elem, mjsActuator* pactuator); void OneComposite(tinyxml2::XMLElement* elem, mjsBody* pbody, mjsDefault* def); - void OneFlexcomp(tinyxml2::XMLElement* elem, mjsBody* pbody); + void OneFlexcomp(tinyxml2::XMLElement* elem, mjsBody* pbody, const mjVFS* vfs); void OnePlugin(tinyxml2::XMLElement* elem, mjsPlugin* plugin); mjXSchema schema; // schema used for validation diff --git a/src/xml/xml_util.cc b/src/xml/xml_util.cc index 48ad62de..cd2fe3ed 100644 --- a/src/xml/xml_util.cc +++ b/src/xml/xml_util.cc @@ -76,7 +76,7 @@ static std::optional ParseInfOrNan(const std::string& s) { } FilePath ResolveFilePath(XMLElement* e, const FilePath& filename, - const FilePath& dir) { + const FilePath& dir, const mjVFS* vfs) { std::string path = ""; if (filename.IsAbs()) { return filename; @@ -85,7 +85,7 @@ FilePath ResolveFilePath(XMLElement* e, const FilePath& filename, // TODO(kylebayes): We first look in the base model directory for files to // remain backwards compatible. FilePath fullname = dir + filename; - mjResource *resource = mju_openResource(fullname.c_str(), nullptr, + mjResource *resource = mju_openResource("", fullname.c_str(), vfs, nullptr, 0); if (resource != nullptr) { mju_closeResource(resource); @@ -623,14 +623,14 @@ mjXUtil::ReadAttrStr(XMLElement* elem, const char* attr, bool required) { // if attribute is present, return attribute as a filename std::optional -mjXUtil::ReadAttrFile(XMLElement* elem, const char* attr, +mjXUtil::ReadAttrFile(XMLElement* elem, const char* attr, const mjVFS* vfs, const FilePath& dir, bool required) { auto maybe_str = ReadAttrStr(elem, attr, required); if (!maybe_str.has_value()) { return std::nullopt; } FilePath filename(maybe_str.value()); - return ResolveFilePath(elem, filename, dir); + return ResolveFilePath(elem, filename, dir, vfs); } // if attribute is present, return numerical value of attribute diff --git a/src/xml/xml_util.h b/src/xml/xml_util.h index 7ef5913e..86d61948 100644 --- a/src/xml/xml_util.h +++ b/src/xml/xml_util.h @@ -26,6 +26,7 @@ #include "tinyxml2.h" +#include #include "user/user_util.h" // error string copy @@ -114,6 +115,7 @@ class mjXUtil { // if attribute is present, return attribute as a filename static std::optional ReadAttrFile(tinyxml2::XMLElement* elem, const char* attr, + const mjVFS* vfs, const mujoco::user::FilePath& dir = mujoco::user::FilePath(), bool required = false); diff --git a/test/user/user_cache_test.cc b/test/user/user_cache_test.cc index 5b353eac..f226827b 100644 --- a/test/user/user_cache_test.cc +++ b/test/user/user_cache_test.cc @@ -47,7 +47,7 @@ void CacheText(mjCCache& cache, const std::string& model, mjVFS vfs; mj_defaultVFS(&vfs); mj_addBufferVFS(&vfs, name.c_str(), text.data(), text.size()); - mjResource* resource = mju_openResource(name.c_str(), &vfs, nullptr, 0); + mjResource* resource = mju_openResource("", name.c_str(), &vfs, nullptr, 0); std::shared_ptr data(&text, +[](const void* data) {}); cache.Insert(model, resource, data, text.size()); mju_closeResource(resource); @@ -61,7 +61,7 @@ GetCachedText(mjCCache& cache, const std::string& model, mjVFS vfs; mj_defaultVFS(&vfs); mj_addBufferVFS(&vfs, name.c_str(), text.data(), std::strlen(text.c_str())); - mjResource* resource = mju_openResource(name.c_str(), &vfs, nullptr, 0); + mjResource* resource = mju_openResource("", name.c_str(), &vfs, nullptr, 0); bool inserted = cache.PopulateData(resource, [&cached_text](const void* data) { cached_text = *(static_cast(data)); diff --git a/test/user/user_resource_test.cc b/test/user/user_resource_test.cc index 3f243641..357e75e4 100644 --- a/test/user/user_resource_test.cc +++ b/test/user/user_resource_test.cc @@ -233,7 +233,7 @@ TEST_F(ResourceTest, GeneralTest) { EXPECT_GT(i, 0); // open resource - mjResource* resource = mju_openResource("str:file", nullptr, nullptr, 0); + mjResource* resource = mju_openResource("", "str:file", nullptr, nullptr, 0); ASSERT_THAT(resource, NotNull()); const char* buffer = NULL; @@ -256,7 +256,7 @@ TEST_F(ResourceTest, GeneralFailureTest) { static std::array error; // open resource - mjResource* resource = mju_openResource("str:notfound", nullptr, + mjResource* resource = mju_openResource("", "str:notfound", nullptr, error.data(), error.size()); ASSERT_THAT(resource, IsNull()); @@ -281,7 +281,7 @@ TEST_F(ResourceTest, NameWithValidPrefix) { }; // open resource - mjResource* resource = mju_openResource("nop:found", nullptr, nullptr, 0); + mjResource* resource = mju_openResource("", "nop:found", nullptr, nullptr, 0); ASSERT_THAT(resource, NotNull()); mju_closeResource(resource); } @@ -304,7 +304,7 @@ TEST_F(ResourceTest, NameWithUpperCasePrefix) { }; // open resource - mjResource* resource = mju_openResource("NOP:found", nullptr, nullptr, 0); + mjResource* resource = mju_openResource("", "NOP:found", nullptr, nullptr, 0); ASSERT_THAT(resource, NotNull()); mju_closeResource(resource); } @@ -327,7 +327,7 @@ TEST_F(ResourceTest, NameWithInvalidPrefix) { }; // open resource - mjResource* resource = mju_openResource("nopfound", nullptr, nullptr, 0); + mjResource* resource = mju_openResource("", "nopfound", nullptr, nullptr, 0); ASSERT_THAT(resource, IsNull()); } @@ -338,7 +338,7 @@ TEST_F(ResourceTest, OSFilesystemTimestamps) { const char* const file = "engine/testdata/collision_box/boxbox_deep.xml"; const std::string xml_path = GetTestDataFilePath(file); - mjResource* resource = mju_openResource(xml_path.c_str(), nullptr, + mjResource* resource = mju_openResource("", xml_path.c_str(), nullptr, nullptr, 0); mju_decodeBase64((uint8_t*) &t, resource->timestamp); diff --git a/test/user/user_vfs_test.cc b/test/user/user_vfs_test.cc index 36378130..2210c8d6 100644 --- a/test/user/user_vfs_test.cc +++ b/test/user/user_vfs_test.cc @@ -30,7 +30,7 @@ using ::testing::NotNull; using UserVfsTest = MujocoTest; static bool HasFile(const mjVFS* vfs, const std::string& filename) { - mjResource* resource = mju_openResource(filename.c_str(), vfs, nullptr, 0); + mjResource* resource = mju_openResource("", filename.c_str(), vfs, nullptr, 0); bool result = resource != nullptr; mju_closeResource(resource); return result; @@ -196,14 +196,13 @@ TEST_F(UserVfsTest, AddBufferRepeat) { mj_deleteVFS(&vfs); } -TEST_F(UserVfsTest, BufferStripPath) { +TEST_F(UserVfsTest, BufferPath) { mjVFS vfs; mj_defaultVFS(&vfs); std::string buffer = ""; const void* ptr = static_cast(buffer.c_str()); mj_addBufferVFS(&vfs, "dir/model", ptr, buffer.size()); - EXPECT_TRUE(HasFile(&vfs, "MODEL")); - EXPECT_TRUE(HasFile(&vfs, "dir\\model")); + EXPECT_TRUE(HasFile(&vfs, "files/../dir/model")); mj_deleteVFS(&vfs); } @@ -222,7 +221,7 @@ TEST_F(UserVfsTest, Timestamps) { mj_defaultVFS(&vfs); mj_addBufferVFS(&vfs, "cube.obj", cube, sizeof(cube)); - mjResource* resource = mju_openResource("cube.obj", &vfs, nullptr, 0); + mjResource* resource = mju_openResource("", "cube.obj", &vfs, nullptr, 0); // same timestamps EXPECT_EQ(mju_isModifiedResource(resource, resource->timestamp), 0); diff --git a/test/xml/xml_native_reader_test.cc b/test/xml/xml_native_reader_test.cc index 22fc75b8..674f3578 100644 --- a/test/xml/xml_native_reader_test.cc +++ b/test/xml/xml_native_reader_test.cc @@ -15,7 +15,6 @@ // Tests for xml/xml_native_reader.cc. #include -#include #include #include #include @@ -613,6 +612,18 @@ static constexpr unsigned char kTinyPng[] = { 0x82 }; +// mesh OBJ file of a cube +static constexpr char kTinyObj[] = R"( + v -1 -1 1 + v 1 -1 1 + v -1 1 1 + v 1 1 1 + v -1 1 -1 + v 1 1 -1 + v -1 -1 -1 + v 1 -1 -1)"; + + TEST_F(XMLReaderTest, IncludeTest) { static constexpr char xml[] = R"( @@ -726,24 +737,22 @@ TEST_F(XMLReaderTest, IncludePathTest) { )"; - MockFilesystem fs("IncludePathTest"); - fs.AddFile("model.xml", (const unsigned char*) xml, sizeof(xml)); - std::string modelpath = fs.FullPath("model.xml"); - - fs.ChangeDirectory("submodels/"); - fs.AddFile("model1.xml", (const unsigned char*) xml1, sizeof(xml1)); - fs.AddFile("model2.xml", (const unsigned char*) xml2, sizeof(xml2)); - fs.AddFile("subsubmodels/model3.xml", (const unsigned char*) xml3, - sizeof(xml3)); - fs.ChangeDirectory("/"); + mjVFS vfs; + mj_defaultVFS(&vfs); + mj_addBufferVFS(&vfs, "model.xml", xml, sizeof(xml)); + mj_addBufferVFS(&vfs, "submodels/model1.xml", xml1, sizeof(xml1)); + mj_addBufferVFS(&vfs, "submodels/model2.xml", xml2, sizeof(xml2)); + mj_addBufferVFS(&vfs, "submodels/subsubmodels/model3.xml", xml3, + sizeof(xml3)); std::array error; - mjModel* model = mj_loadXML(modelpath.c_str(), nullptr, error.data(), + mjModel* model = mj_loadXML("model.xml", &vfs, error.data(), error.size()); ASSERT_THAT(model, NotNull()) << error.data(); EXPECT_EQ(mj_name2id(model, mjOBJ_GEOM, "ball"), 2); EXPECT_EQ(mj_name2id(model, mjOBJ_GEOM, "another_box"), 3); mj_deleteModel(model); + mj_deleteVFS(&vfs); } TEST_F(XMLReaderTest, FallbackIncludePathTest) { @@ -772,22 +781,21 @@ TEST_F(XMLReaderTest, FallbackIncludePathTest) { )"; - MockFilesystem fs("FallbackIncludePathTest"); - fs.AddFile("model.xml", (const unsigned char*) xml, sizeof(xml)); - std::string modelpath = fs.FullPath("model.xml"); - - fs.AddFile("model1.xml", (const unsigned char*) xml1, sizeof(xml1)); - fs.AddFile("submodels/model2.xml", (const unsigned char*) xml2, sizeof(xml2)); - fs.AddFile("subsubmodels/model3.xml", (const unsigned char*) xml3, - sizeof(xml3)); + mjVFS vfs; + mj_defaultVFS(&vfs); + mj_addBufferVFS(&vfs, "model.xml", xml, sizeof(xml)); + mj_addBufferVFS(&vfs, "model1.xml", xml1, sizeof(xml1)); + mj_addBufferVFS(&vfs, "submodels/model2.xml", xml2, sizeof(xml2)); + mj_addBufferVFS(&vfs, "subsubmodels/model3.xml", xml3, sizeof(xml3)); std::array error; - mjModel* model = mj_loadXML(modelpath.c_str(), nullptr, + mjModel* model = mj_loadXML("model.xml", &vfs, error.data(), error.size()); ASSERT_THAT(model, NotNull()) << error.data(); EXPECT_EQ(mj_name2id(model, mjOBJ_GEOM, "ball"), 2); EXPECT_EQ(mj_name2id(model, mjOBJ_GEOM, "another_box"), 3); mj_deleteModel(model); + mj_deleteVFS(&vfs); } TEST_F(XMLReaderTest, MaterialTextureTest) { @@ -809,14 +817,14 @@ TEST_F(XMLReaderTest, MaterialTextureTest) { )"; - MockFilesystem fs("MaterialTextureTest"); - fs.AddFile("tiny0.png", kTinyPng, sizeof(kTinyPng)); - fs.AddFile("tiny1.png", kTinyPng, sizeof(kTinyPng)); - fs.AddFile("model.xml", (const unsigned char*) xml, sizeof(xml)); - std::string modelpath = fs.FullPath("model.xml"); + mjVFS vfs; + mj_defaultVFS(&vfs); + mj_addBufferVFS(&vfs, "tiny0.png", kTinyPng, sizeof(kTinyPng)); + mj_addBufferVFS(&vfs, "tiny1.png", kTinyPng, sizeof(kTinyPng)); + mj_addBufferVFS(&vfs, "model.xml", xml, sizeof(xml)); char error[1024]; - mjModel* model = mj_loadXML(modelpath.c_str(), nullptr, error, 1024); + mjModel* model = mj_loadXML("model.xml", &vfs, error, 1024); EXPECT_THAT(model, NotNull()) << error; EXPECT_EQ(model->mat_texid[mjTEXROLE_RGB], 1); @@ -825,6 +833,7 @@ TEST_F(XMLReaderTest, MaterialTextureTest) { EXPECT_EQ(model->mat_texid[mjTEXROLE_OCCLUSION], 0); mj_deleteModel(model); + mj_deleteVFS(&vfs); } TEST_F(XMLReaderTest, LegacyMaterialTextureTest) { @@ -841,19 +850,20 @@ TEST_F(XMLReaderTest, LegacyMaterialTextureTest) { )"; - MockFilesystem fs("LegacyMaterialTextureTest"); - fs.AddFile("tiny0.png", kTinyPng, sizeof(kTinyPng)); - fs.AddFile("tiny1.png", kTinyPng, sizeof(kTinyPng)); - fs.AddFile("model.xml", (const unsigned char*) xml, sizeof(xml)); - std::string modelpath = fs.FullPath("model.xml"); + mjVFS vfs; + mj_defaultVFS(&vfs); + mj_addBufferVFS(&vfs, "tiny0.png", kTinyPng, sizeof(kTinyPng)); + mj_addBufferVFS(&vfs, "tiny1.png", kTinyPng, sizeof(kTinyPng)); + mj_addBufferVFS(&vfs, "model.xml", xml, sizeof(xml)); char error[1024]; - mjModel* model = mj_loadXML(modelpath.c_str(), nullptr, error, 1024); + mjModel* model = mj_loadXML("model.xml", &vfs, error, 1024); EXPECT_THAT(model, NotNull()) << error; EXPECT_EQ(model->mat_texid[mjTEXROLE_RGB], 1); mj_deleteModel(model); + mj_deleteVFS(&vfs); } TEST_F(XMLReaderTest, MaterialTextureFailTest) { @@ -873,12 +883,6 @@ TEST_F(XMLReaderTest, MaterialTextureFailTest) { )"; - MockFilesystem fs("MaterialTextureFailTest"); - fs.AddFile("tiny0.png", kTinyPng, sizeof(kTinyPng)); - fs.AddFile("tiny1.png", kTinyPng, sizeof(kTinyPng)); - fs.AddFile("model.xml", (const unsigned char*) xml, sizeof(xml)); - std::string modelpath = fs.FullPath("model.xml"); - std::array error; mjModel* m = LoadModelFromString(xml, error.data(), error.size()); EXPECT_THAT(m, IsNull()); @@ -907,28 +911,32 @@ TEST_F(XMLReaderTest, IncludeAssetsTest) { static constexpr char subassets[] = R"( + )"; - MockFilesystem fs("IncludeAssetsTest"); - fs.AddFile("assets/tiny.png", kTinyPng, sizeof(kTinyPng)); - fs.AddFile("assets/subassets/subtiny.png", kTinyPng, sizeof(kTinyPng)); - fs.AddFile("assets/assets.xml", (const unsigned char*) assets, - sizeof(assets)); - fs.AddFile("assets/subassets/assets.xml", (const unsigned char*) subassets, - sizeof(subassets)); - fs.AddFile("model.xml", (const unsigned char*) xml, sizeof(xml)); - std::string modelpath = fs.FullPath("model.xml"); + mjVFS vfs; + mj_defaultVFS(&vfs); + mj_addBufferVFS(&vfs, "assets/tiny.png", kTinyPng, sizeof(kTinyPng)); + mj_addBufferVFS(&vfs, "assets/subassets/subtiny.png", kTinyPng, + sizeof(kTinyPng)); + mj_addBufferVFS(&vfs, "assets/subassets/cube.obj", kTinyObj, + sizeof(kTinyObj)); + mj_addBufferVFS(&vfs, "assets/assets.xml", assets, sizeof(assets)); + mj_addBufferVFS(&vfs, "assets/subassets/assets.xml", subassets, + sizeof(subassets)); + mj_addBufferVFS(&vfs, "model.xml", xml, sizeof(xml)); // loading the file should be successful std::array error; - mjModel* model = mj_loadXML(modelpath.c_str(), nullptr, error.data(), + mjModel* model = mj_loadXML("model.xml", &vfs, error.data(), error.size()); ASSERT_THAT(model, NotNull()) << error.data(); mj_deleteModel(model); + mj_deleteVFS(&vfs); } TEST_F(XMLReaderTest, FallbackIncludeAssetsTest) { @@ -956,26 +964,26 @@ TEST_F(XMLReaderTest, FallbackIncludeAssetsTest) { )"; - MockFilesystem fs("FallbackIncludeAssetsTest"); - fs.AddFile("assets/tiny.png", kTinyPng, sizeof(kTinyPng)); + mjVFS vfs; + mj_defaultVFS(&vfs); + mj_addBufferVFS(&vfs, "assets/tiny.png", kTinyPng, sizeof(kTinyPng)); // need to fallback for backwards compatibility - fs.AddFile("subtiny.png", kTinyPng, sizeof(kTinyPng)); + mj_addBufferVFS(&vfs, "subtiny.png", kTinyPng, sizeof(kTinyPng)); - fs.AddFile("assets/assets.xml", (const unsigned char*) assets, - sizeof(assets)); - fs.AddFile("assets/subassets/assets.xml", (const unsigned char*) subassets, - sizeof(subassets)); - fs.AddFile("model.xml", (const unsigned char*) xml, sizeof(xml)); - std::string modelpath = fs.FullPath("model.xml"); + mj_addBufferVFS(&vfs, "assets/assets.xml", assets, sizeof(assets)); + mj_addBufferVFS(&vfs, "assets/subassets/assets.xml", subassets, + sizeof(subassets)); + mj_addBufferVFS(&vfs, "model.xml", xml, sizeof(xml)); // loading the file should be successful std::array error; - mjModel* model = mj_loadXML(modelpath.c_str(), nullptr, + mjModel* model = mj_loadXML("model.xml", &vfs, error.data(), error.size()); ASSERT_THAT(model, NotNull()) << error.data(); mj_deleteModel(model); + mj_deleteVFS(&vfs); } TEST_F(XMLReaderTest, IncludeAbsoluteTest) { From 70d70bc3b7c8fa3a21dcd993901989827c4a4ba4 Mon Sep 17 00:00:00 2001 From: Google DeepMind Date: Thu, 25 Jul 2024 13:30:10 -0700 Subject: [PATCH 04/10] Fix typos in MuJoCo documentation. PiperOrigin-RevId: 656068822 Change-Id: I5ebbb3701148ea4d978a6a1ad3f762ffcc7e57cb --- doc/APIreference/functions.rst | 8 ++++---- doc/APIreference/functions_override.rst | 8 ++++---- doc/computation/index.rst | 2 +- doc/modeling.rst | 4 ++-- doc/overview.rst | 2 +- doc/programming/extension.rst | 2 +- doc/programming/modeledit.rst | 4 ++-- doc/programming/simulation.rst | 2 +- doc/unity.rst | 2 +- model/balloons/balloons.xml | 2 +- plugin/actuator/README.md | 4 ++-- 11 files changed, 20 insertions(+), 20 deletions(-) diff --git a/doc/APIreference/functions.rst b/doc/APIreference/functions.rst index 0b85227a..cbe5bb41 100644 --- a/doc/APIreference/functions.rst +++ b/doc/APIreference/functions.rst @@ -323,7 +323,7 @@ frame (``point``) treated as attached to the body, the Jacobian has both transla (``jacr``) components. Passing ``NULL`` for either pointer will skip that part of the computation. Each component is a 3-by-nv matrix. Each row of this matrix is the gradient of the corresponding coordinate of the specified point with respect to the degrees-of-freedom. The :ref:`pipeline stages` required for Jacobian computations to be -consistenst with the current generalized positions ``mjData.qpos`` are :ref:`mj_kinematics` and :ref:`mj_comPos`. +consistent with the current generalized positions ``mjData.qpos`` are :ref:`mj_kinematics` and :ref:`mj_comPos`. .. _mj_jacBody: @@ -1882,7 +1882,7 @@ Twice continuously differentiable sigmoid function using a quintic polynomial: Interaction ^^^^^^^^^^^ -These function implement abstract mouse interactions, allowing control over cameras and perturbations. Their use is well +These functions implement abstract mouse interactions, allowing control over cameras and perturbations. Their use is well illustrated in :ref:`simulate`. .. _mjv_defaultCamera: @@ -2066,8 +2066,8 @@ an illustration. Visualization ^^^^^^^^^^^^^ -The functions in this section implement abstract visualization. The results are used by the OpenGL rendered, and can -also be used by users wishing to implement their own rendered, or hook up MuJoCo to advanced rendering tools such as +The functions in this section implement abstract visualization. The results are used by the OpenGL renderer, and can +also be used by users wishing to implement their own renderer, or hook up MuJoCo to advanced rendering tools such as Unity or Unreal Engine. See :ref:`simulate` for illustration of how to use these functions. .. _mjv_defaultOption: diff --git a/doc/APIreference/functions_override.rst b/doc/APIreference/functions_override.rst index a63bf214..b7e62d78 100644 --- a/doc/APIreference/functions_override.rst +++ b/doc/APIreference/functions_override.rst @@ -180,7 +180,7 @@ frame (``point``) treated as attached to the body, the Jacobian has both transla (``jacr``) components. Passing ``NULL`` for either pointer will skip that part of the computation. Each component is a 3-by-nv matrix. Each row of this matrix is the gradient of the corresponding coordinate of the specified point with respect to the degrees-of-freedom. The :ref:`pipeline stages` required for Jacobian computations to be -consistenst with the current generalized positions ``mjData.qpos`` are :ref:`mj_kinematics` and :ref:`mj_comPos`. +consistent with the current generalized positions ``mjData.qpos`` are :ref:`mj_kinematics` and :ref:`mj_comPos`. .. _mj_jacBody: @@ -270,7 +270,7 @@ bodyexclude=-1 can be used to indicate that all bodies are included. .. _Interaction: -These function implement abstract mouse interactions, allowing control over cameras and perturbations. Their use is well +These functions implement abstract mouse interactions, allowing control over cameras and perturbations. Their use is well illustrated in :ref:`simulate`. .. _mjv_select: @@ -283,8 +283,8 @@ an illustration. .. _Visualization-api: -The functions in this section implement abstract visualization. The results are used by the OpenGL rendered, and can -also be used by users wishing to implement their own rendered, or hook up MuJoCo to advanced rendering tools such as +The functions in this section implement abstract visualization. The results are used by the OpenGL renderer, and can +also be used by users wishing to implement their own renderer, or hook up MuJoCo to advanced rendering tools such as Unity or Unreal Engine. See :ref:`simulate` for illustration of how to use these functions. .. _OpenGLrendering: diff --git a/doc/computation/index.rst b/doc/computation/index.rst index eb1bf285..06e0da68 100644 --- a/doc/computation/index.rst +++ b/doc/computation/index.rst @@ -1328,7 +1328,7 @@ representations of the constraint Jacobian and related matrices. **PGS** : Projected Gauss-Seidel method This is the most common algorithm used in physics simulators, and used to be the default in MuJoCo, until we developed the Newton method which appears to be better in every way. PGS uses the dual formulation. Unlike - gradient-based method which improve the solution along oblique directions, Gauss-Seidel works on one scalar component + gradient-based methods which improve the solution along oblique directions, Gauss-Seidel works on one scalar component at a time, and sets it to its optimal value given the current values of all other components. One sweep of PGS has the computational complexity of one matrix-vector multiplication (although the constants are larger). It has first-order convergence but nevertheless makes rapid progress in a few iterations. diff --git a/doc/modeling.rst b/doc/modeling.rst index ed12659e..325317f1 100644 --- a/doc/modeling.rst +++ b/doc/modeling.rst @@ -1407,7 +1407,7 @@ Using the :ref:`flexcomp` element, we can create flexes from mesh automatically generate all the bodies/vertices and connect them with suitable elements. We can also create grids and other topologies automatically. This machinery makes it easy to create very large flexes, involving thousands or even tens of thousands of bodies, elements and edges. Obviously such simulations will not be fast. Even for medium-sized -flexes, pruning of collision pairs and essential. This is why we have developed elaborate methods for pruning +flexes, pruning of collision pairs is essential. This is why we have developed elaborate methods for pruning self-collisions; see XML reference. In case of 3D flexes made of tetrahedra, it may be useful to examine how the flex is "triangulated" internally. We have @@ -1724,7 +1724,7 @@ better visualize and understand the contact configuration and resulting forces. model by design, since without it the inverse dynamics are not defined. This is discussed in detail in the :ref:`softness and slip` clarification. This type of slippage can be addressed in two ways. - a. Increase the :ref:`impration` parameter. This will reduce (but not entirely prevent) slow + a. Increase the :ref:`impratio` parameter. This will reduce (but not entirely prevent) slow slippage. Note that high impratio values work well only with :ref:`elliptic cones`. b. Enable the noslip solver by increasing :ref:`noslip_iterations` to a positive integer. A small number (1, 2 or 3) is usually sufficient. The noslip post-processing solver will entirely prevent slip, diff --git a/doc/overview.rst b/doc/overview.rst index 337023a3..a88fb7bc 100644 --- a/doc/overview.rst +++ b/doc/overview.rst @@ -9,7 +9,7 @@ aims to facilitate research and development in robotics, biomechanics, graphics other areas that demand fast and accurate simulation of articulated structures interacting with their environment. Initially developed by Roboti LLC, it was acquired and made `freely available `__ by DeepMind in October 2021, and open sourced in May -2022. The MuJoCo codebase is available at the `deepmind/mujoco `__ repository +2022. The MuJoCo codebase is available at the `google-deepmind/mujoco `__ repository on GitHub. MuJoCo is a C/C++ library with a C API, intended for researchers and developers. The runtime simulation module is tuned diff --git a/doc/programming/extension.rst b/doc/programming/extension.rst index 33e1002b..d61139fa 100644 --- a/doc/programming/extension.rst +++ b/doc/programming/extension.rst @@ -185,7 +185,7 @@ faithfully restored. Plugins must declare the number of floating point values required for each instance via the ``nstate`` callback of its :ref:`mjpPlugin` struct. Note that this number can depend on the exact configuration of the instance. During -:ref:`mj_makeData`, MuJoCo allocate the requisite number of slots in the ``plugin_state`` field of :ref:`mjData` for +:ref:`mj_makeData`, MuJoCo allocates the requisite number of slots in the ``plugin_state`` field of :ref:`mjData` for each plugin instance. The ``plugin_stateadr`` field in :ref:`mjModel` indicates the position within the overall ``plugin_state`` array at which each plugin instance can find its state values. diff --git a/doc/programming/modeledit.rst b/doc/programming/modeledit.rst index 24aa64ae..8968d4cc 100644 --- a/doc/programming/modeledit.rst +++ b/doc/programming/modeledit.rst @@ -20,7 +20,7 @@ Overview The new API augments the traditional workflow of creating and editing models using XML files, breaking up the *parse* and *compile* steps. As summarized in the the :ref:`Overview chapter`, the traditional workflow is: - 1. Create an XML model description file (MJCF or URDF) and ascociated assets. |br| + 1. Create an XML model description file (MJCF or URDF) and associated assets. |br| 2. Call :ref:`mj_loadXML`, obtain an :ref:`mjModel` instance. The new workflow is: @@ -107,7 +107,7 @@ Known issues to `user_api_test.cc `__ and the MJCF parser in `xml_native_reader.cc `__, which is already using this API. -- One of the central design consideration of the new API is incremental compilation, meaning that after making small +- One of the central design considerations of the new API is incremental compilation, meaning that after making small changes to a spec that has already been compiled, subsequent re-compilation will be very fast. While the code is written to support incremental compilation, this functionality is not fully implemented and will be added in the future, resulting in faster re-compilation times. diff --git a/doc/programming/simulation.rst b/doc/programming/simulation.rst index 9233f6df..ca1d0b60 100644 --- a/doc/programming/simulation.rst +++ b/doc/programming/simulation.rst @@ -945,7 +945,7 @@ this body quaternion, the quaternions of all other objects attached to the body multiplication. The function :ref:`mj_local2Global` converts from local body coordinates to global Cartesian coordinates. -:ref:`mju_negPose` and :ref:`mju_trnVecPose`. A pose is a grouping of a 3D position and a unit quaternion orientation. +A pose is a grouping of a 3D position and a unit quaternion orientation. There is no separate data structure; the grouping is in terms of logic. This represents a position and orientation in space, or in other words a spatial frame. Note that OpenGL uses 4-by-4 matrices to represent the same information, except here we use a quaternion for orientation. The function mju_mulPose multiplies two poses, meaning that it diff --git a/doc/unity.rst b/doc/unity.rst index 80ba500c..6177ef02 100644 --- a/doc/unity.rst +++ b/doc/unity.rst @@ -328,7 +328,7 @@ ___________________________________ Roboti’s `MuJoCo plug-in for Unity `_ steps the simulation in an external Python process, and uses Unity only for rendering. In contrast, our plug-in relies on Unity to step the simulation. It should -be possible to use our plug-in while an external process "drives" the simulation, for example by seting ``qpos``, +be possible to use our plug-in while an external process "drives" the simulation, for example by setting ``qpos``, calling ``mj_kinematics``, synchronizing the transforms, and then using Unity to render or compute game logic. In order to establish communication with an external process, you can use Unity's `ML-Agents `_ package. diff --git a/model/balloons/balloons.xml b/model/balloons/balloons.xml index 31659151..4aab660f 100644 --- a/model/balloons/balloons.xml +++ b/model/balloons/balloons.xml @@ -57,7 +57,7 @@ - + diff --git a/plugin/actuator/README.md b/plugin/actuator/README.md index 9139aed1..a6b1e862 100644 --- a/plugin/actuator/README.md +++ b/plugin/actuator/README.md @@ -4,7 +4,7 @@ The `mujoco.pid` actuator plugin implements a configurable [PID controller](https://en.wikipedia.org/wiki/Proportional%E2%80%93integral%E2%80%93derivative_controller): -$$f(t) = K_\text{p} e(t) + K_\text{i} \int_0^t e(\tau) \,\mathrm{d}\tau + K_\text{d} \frac{\mathrm{d}e(t)}{\mathrm{d}t},$$ +$$f(t) = K_\text{p} e(t) + K_\text{i} \int_0^t e(\tau) \mathrm{d}\tau + K_\text{d} \frac{\mathrm{d}e(t)}{\mathrm{d}t},$$ where $e(t) = u(t) - \ell(t)$ is the difference between the control $u$ and the actuator length $\ell$. You can use it like: @@ -43,5 +43,5 @@ The available options are: |`kp` | 0 | **P** gain for the controller. | |`ki` | 0 | **I** gain for the controller.

If nonzero, one activation variable will be added to `mjData.act`, containing the current I term (in units of force). | |`kd` | 0 | **D** gain for the controller. | -|`imax` | Optional | If specified, the force produced by the I term will be clipped to the range `[-imax, -imax]`. | +|`imax` | Optional | If specified, the force produced by the I term will be clipped to the range `[-imax, imax]`. | |`slewmax` | Optional | The maximum rate at which the setpoint for the PID controller can change.

If a bigger change is requested between two timesteps, it will be clipped to the range `[ctrl - slewmax * dt, ctrl + slewmax * dt]`

If specified, one activation variable will be added to `mjData.act` containing the previous value of `ctrl`. | From 3f0749a0f776486bcd02b612a644420e331e6dc7 Mon Sep 17 00:00:00 2001 From: Alessio Quaglino Date: Mon, 29 Jul 2024 07:18:02 -0700 Subject: [PATCH 05/10] Do not remove elements that contain user errors when detaching a subtree. PiperOrigin-RevId: 657190780 Change-Id: I073a437013f80596a341f7b80bed991536efeb43 --- src/user/user_model.cc | 23 +++++++++++++++-------- src/user/user_model.h | 2 +- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/user/user_model.cc b/src/user/user_model.cc index b6ec8086..fbd94a6b 100644 --- a/src/user/user_model.cc +++ b/src/user/user_model.cc @@ -250,15 +250,22 @@ mjCModel& mjCModel::operator+=(const mjCModel& other) { template -void mjCModel::RemoveFromList(std::vector& list) { +void mjCModel::RemoveFromList(std::vector& list, const mjCModel& other) { int nlist = (int)list.size(); int removed = 0; for (int i = 0; i < nlist; i++) { T* element = list[i]; element->id -= removed; + try { + // check if the element contains an error + element->NameSpace(&other); + element->CopyFromSpec(); + element->ResolveReferences(&other); + } catch (mjCError err) { + continue; + } try { // check if the element references something that was removed - // TODO: do not remove elements that contain user errors element->NameSpace(this); element->CopyFromSpec(); element->ResolveReferences(this); @@ -296,12 +303,12 @@ mjCModel& mjCModel::operator-=(const mjCBody& subtree) { ProcessLists(/*checkrepeat=*/false); // check if we have to remove anything else - RemoveFromList(pairs_); - RemoveFromList(excludes_); - RemoveFromList(tendons_); - RemoveFromList(equalities_); - RemoveFromList(actuators_); - RemoveFromList(sensors_); + RemoveFromList(pairs_, oldmodel); + RemoveFromList(excludes_, oldmodel); + RemoveFromList(tendons_, oldmodel); + RemoveFromList(equalities_, oldmodel); + RemoveFromList(actuators_, oldmodel); + RemoveFromList(sensors_, oldmodel); // restore to the original state if (!compiled) { diff --git a/src/user/user_model.h b/src/user/user_model.h index f6c1c782..52bef9c1 100644 --- a/src/user/user_model.h +++ b/src/user/user_model.h @@ -345,7 +345,7 @@ class mjCModel : public mjCModel_, private mjSpec { const std::vector& sources); // delete from list the elements that cause an error - template void RemoveFromList(std::vector& list); + template void RemoveFromList(std::vector& list, const mjCModel& other); // create mjCBase lists from children lists void CreateObjectLists(); From a4bd2bec0a0f9df72fe9feab4e73e3138d5091da Mon Sep 17 00:00:00 2001 From: Yuval Tassa Date: Mon, 29 Jul 2024 07:30:54 -0700 Subject: [PATCH 06/10] Skip disabled actuators in mjd_actuator_vel. Fixes a bug where the derivative of the actuator force with respect to the generalized velocities (used in implicit and implicitfast integrators) was failing to take into account disabled actuators. Fixes #1838 PiperOrigin-RevId: 657193960 Change-Id: Id8c0ab863a39e2a01cd2703774f460e9731b4807 --- doc/XMLreference.rst | 18 ++++---- doc/changelog.rst | 10 +++-- src/engine/engine_derivative.c | 5 +++ test/engine/engine_derivative_test.cc | 64 +++++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 13 deletions(-) diff --git a/doc/XMLreference.rst b/doc/XMLreference.rst index ba447d4b..ec0fea5c 100644 --- a/doc/XMLreference.rst +++ b/doc/XMLreference.rst @@ -1751,7 +1751,7 @@ properties are grouped together. .. _material-rgb: :el-prefix:`material/` |-| **rgb** (?) -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This element references a texture asset used to specify base color / albedo values. @@ -1763,7 +1763,7 @@ This element references a texture asset used to specify base color / albedo valu .. _material-normal: :el-prefix:`material/` |-| **normal** (?) -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This element references a texture asset used to specify the bump map (surface normals). @@ -1775,7 +1775,7 @@ This element references a texture asset used to specify the bump map (surface no .. _material-occlusion: :el-prefix:`material/` |-| **occlusion** (?) -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This element references a texture asset used to specify ambient occlusion. @@ -1787,7 +1787,7 @@ This element references a texture asset used to specify ambient occlusion. .. _material-roughness: :el-prefix:`material/` |-| **roughness** (?) -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This element references a texture asset used to specify the roughness map. @@ -1799,7 +1799,7 @@ This element references a texture asset used to specify the roughness map. .. _material-metallic: :el-prefix:`material/` |-| **metallic** (?) -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This element references a texture asset used to specify the metallic map. @@ -1811,7 +1811,7 @@ This element references a texture asset used to specify the metallic map. .. _material-opacity: :el-prefix:`material/` |-| **opacity** (?) -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This element references a texture asset used to specify the opacity map (alpha channel, transparency). @@ -1823,7 +1823,7 @@ This element references a texture asset used to specify the opacity map (alpha c .. _material-emissive: :el-prefix:`material/` |-| **emissive** (?) -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This element references a texture asset used to specify light emission. @@ -1835,7 +1835,7 @@ This element references a texture asset used to specify light emission. .. _material-orm: :el-prefix:`material/` |-| **orm** (?) -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This element references a texture asset used to specify a packed ORM map, where occlusion, roughness, and metallic are joined into the corresponding RGB values of a single texture. @@ -1848,7 +1848,7 @@ are joined into the corresponding RGB values of a single texture. .. _material-rgba: :el-prefix:`material/` |-| **rgba** (?) -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This element references a texture asset used to specify a packed map where albedo and opacity are joined into the same 4-channel texture. diff --git a/doc/changelog.rst b/doc/changelog.rst index b19bcc28..d22ccc3b 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -7,16 +7,14 @@ Upcoming version (not yet released) General ^^^^^^^ - 1. Renamed ``mjModel.tex_rbg`` to ``mjModel.tex_data``. 2. Added a new disable flag ``mjDSBL_AUTORESETNAN`` to disable automatic reset when NaNs or infinities are produced. 3. Added sub-elements to the MJCF :ref:`material` element, to allow specification of multiple textures -for rendering (e.g., :ref:`occlusion-roughness-metallic`). Note that the MuJoCo renderer doesn't support -these new features, and they are made available for use with external renderers. + for rendering (e.g., :ref:`occlusion-roughness-metallic`). Note that the MuJoCo renderer doesn't + support these new features, and they are made available for use with external renderers. MJX ^^^ - 4. Added more fields to ``mjx.Model`` and ``mjx.Data`` for further compatibility with the corresponding MuJoCo structs. 5. Added support for :ref:`fixed tendons `. 6. Added support for tendon length limits (``mjCNSTR_LIMIT_TENDON`` in :ref:`mjtConstraint`). @@ -28,6 +26,10 @@ Python bindings 9. Added support for asset dictionary argument in ``mujoco.spec.from_file``, ``mujoco.spec.from_string`` and ``mujoco.spec.compile``. +Bug fixes +^^^^^^^^^ +10. Fixed a bug where implicit integrators did not take into account disabled actuators (:github:issue:`1838`). + Version 3.2.0 (Jul 15, 2024) ---------------------------- diff --git a/src/engine/engine_derivative.c b/src/engine/engine_derivative.c index 73c64b0f..dec37eb8 100644 --- a/src/engine/engine_derivative.c +++ b/src/engine/engine_derivative.c @@ -834,6 +834,11 @@ void mjd_actuator_vel(const mjModel* m, mjData* d) { // process actuators for (int i=0; i < m->nu; i++) { + // skip if disabled + if (mj_actuatorDisabled(m, i)) { + continue; + } + mjtNum bias_vel = 0, gain_vel = 0; // affine bias diff --git a/test/engine/engine_derivative_test.cc b/test/engine/engine_derivative_test.cc index 85ecf60a..9a7a776e 100644 --- a/test/engine/engine_derivative_test.cc +++ b/test/engine/engine_derivative_test.cc @@ -159,6 +159,70 @@ TEST_F(DerivativeTest, SmoothDvel) { } } +// disabled actuators do not contribute to d_qfrc_actuator/d_qvel +TEST_F(DerivativeTest, DisabledActuators) { + // model with only a position actuator + static constexpr char xml1[] = R"( + + + )"; + + mjModel* m1 = LoadModelFromString(xml1); + mjData* d1 = mj_makeData(m1); + + d1->ctrl[0] = 6; + while (d1->time < 1) + mj_step(m1, d1); + + // model with a position actuator and an intvelocity actuator + static constexpr char xml2[] = R"( + + + )"; + + mjModel* m2 = LoadModelFromString(xml2); + mjData* d2 = mj_makeData(m2); + + d2->ctrl[0] = 6; + d2->ctrl[1] = 6; + + while (d2->time < 1) + mj_step(m2, d2); + + // expect same qvel in both models + EXPECT_EQ(d1->qvel[0], d2->qvel[0]); + + mj_deleteData(d2); + mj_deleteModel(m2); + mj_deleteData(d1); + mj_deleteModel(m1); +} + // compare analytic and fin-diff d_qfrc_passive/d_qvel TEST_F(DerivativeTest, PassiveDvel) { for (const char* local_path : {kTumblingThinObjectPath, From a29f67c850ba5121e05e9740dd6a48220fa19a9e Mon Sep 17 00:00:00 2001 From: Yuval Tassa Date: Mon, 29 Jul 2024 08:23:41 -0700 Subject: [PATCH 07/10] Add test models for tendon wrapping. PiperOrigin-RevId: 657208035 Change-Id: Ib82bd2b31a15d1386f8096b280def9e7325c8e46 --- .../core_smooth/tendon_wrap_cylinder.xml | 56 +++++++++++++++++++ .../core_smooth/tendon_wrap_sphere.xml | 56 +++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 test/engine/testdata/core_smooth/tendon_wrap_cylinder.xml create mode 100644 test/engine/testdata/core_smooth/tendon_wrap_sphere.xml diff --git a/test/engine/testdata/core_smooth/tendon_wrap_cylinder.xml b/test/engine/testdata/core_smooth/tendon_wrap_cylinder.xml new file mode 100644 index 00000000..fa70af44 --- /dev/null +++ b/test/engine/testdata/core_smooth/tendon_wrap_cylinder.xml @@ -0,0 +1,56 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/engine/testdata/core_smooth/tendon_wrap_sphere.xml b/test/engine/testdata/core_smooth/tendon_wrap_sphere.xml new file mode 100644 index 00000000..2ddba3bb --- /dev/null +++ b/test/engine/testdata/core_smooth/tendon_wrap_sphere.xml @@ -0,0 +1,56 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 6928e09f362aa4ba1d335e6f060dda72b1476941 Mon Sep 17 00:00:00 2001 From: Alessio Quaglino Date: Mon, 29 Jul 2024 09:31:54 -0700 Subject: [PATCH 08/10] Small performance improvement: Parse body once during replicate. PiperOrigin-RevId: 657227162 Change-Id: I5b60c25ab4634a44ed0474fdbf22e676bcb8f6d7 --- src/xml/xml_native_reader.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/xml/xml_native_reader.cc b/src/xml/xml_native_reader.cc index 2dc7138b..4e16d79b 100644 --- a/src/xml/xml_native_reader.cc +++ b/src/xml/xml_native_reader.cc @@ -3497,12 +3497,16 @@ void mjXReader::Body(XMLElement* section, mjsBody* pbody, mjsFrame* frame, double pos[3] = {0, 0, 0}; double quat[4] = {1, 0, 0, 0}; - for (int i = 0; i < count; i++) { - // create parent frame - mjsFrame* pframe = mjs_addFrame(subtree, frame); - mjs_setString(pframe->info, ("line = " + std::to_string(elem->GetLineNum())).c_str()); - mjs_setDefault(pframe->element, childdef ? childdef : def); + // parent frame that will be used to attach the subtree + mjsFrame* pframe = mjs_addFrame(subtree, frame); + mjs_setDefault(pframe->element, childdef ? childdef : def); + mjs_setString(pframe->info, ("line = " + std::to_string(elem->GetLineNum())).c_str()); + // parse subtree + Body(elem, subtree, pframe, vfs); + + // update pframe and attach + for (int i = 0; i < count; i++) { // accumulate rotation mjuu_setvec(pframe->pos, pos[0], pos[1], pos[2]); mjuu_frameaccum(pos, quat, offset, rotation); @@ -3518,9 +3522,6 @@ void mjXReader::Body(XMLElement* section, mjsBody* pbody, mjsFrame* frame, std::string suffix = separator; UpdateString(suffix, count, i); - // process subtree - Body(elem, subtree, pframe, vfs); - // attach to parent if (mjs_attachFrame(pbody, pframe, /*prefix=*/"", suffix.c_str()) != 0) { throw mjXError(elem, mjs_getError(spec)); From 24a555062d2e167f81e0e25693690d5b084bd44e Mon Sep 17 00:00:00 2001 From: Yuval Tassa Date: Tue, 30 Jul 2024 08:40:55 -0700 Subject: [PATCH 09/10] Rename mjDSBL_AUTORESETNAN to mjDSBL_AUTORESET. - Expose in MJCF. - Add documentation. - Improve handling in simulate. PiperOrigin-RevId: 657606140 Change-Id: I6a27c2f4842d7d4ae32642f2a6255e5f8f1f605a --- doc/XMLreference.rst | 5 +++++ doc/XMLschema.rst | 6 +++--- doc/changelog.rst | 3 ++- doc/includes/references.h | 2 +- include/mujoco/mjmodel.h | 2 +- introspect/enums.py | 2 +- simulate/main.cc | 26 ++++++++++++++++++++++++-- simulate/simulate.cc | 7 +------ src/engine/engine_forward.c | 8 ++++---- src/engine/engine_support.c | 2 +- src/xml/xml_native_reader.cc | 5 +++-- src/xml/xml_native_writer.cc | 1 + unity/Runtime/Bindings/MjBindings.cs | 2 +- 13 files changed, 48 insertions(+), 23 deletions(-) diff --git a/doc/XMLreference.rst b/doc/XMLreference.rst index ec0fea5c..56c026a4 100644 --- a/doc/XMLreference.rst +++ b/doc/XMLreference.rst @@ -577,6 +577,11 @@ from its default. This flag disables implicit integration with respect to joint damping in the Euler integrator. See the :ref:`Numerical Integration` section for more details. +.. _option-flag-autoreset: + +:at:`autoreset`: :at-val:`[disable, enable], "enable"` + This flag disables the automatic reseting of the simulation state when numerical issues are detected. + .. _option-flag-override: :at:`override`: :at-val:`[disable, enable], "disable"` diff --git a/doc/XMLschema.rst b/doc/XMLschema.rst index 9fdd9dcd..ae9f43c7 100644 --- a/doc/XMLschema.rst +++ b/doc/XMLschema.rst @@ -35,11 +35,11 @@ | | | +-----------------------------------------------------------------+-----------------------------------------------------------------+-----------------------------------------------------------------+-----------------------------------------------------------------+ | | | | | :ref:`warmstart` | :ref:`filterparent` | :ref:`actuation` | :ref:`refsafe` | | | | | +-----------------------------------------------------------------+-----------------------------------------------------------------+-----------------------------------------------------------------+-----------------------------------------------------------------+ | -| | | | :ref:`sensor` | :ref:`midphase` | :ref:`eulerdamp` | :ref:`override` | | +| | | | :ref:`sensor` | :ref:`midphase` | :ref:`eulerdamp` | :ref:`autoreset` | | | | | +-----------------------------------------------------------------+-----------------------------------------------------------------+-----------------------------------------------------------------+-----------------------------------------------------------------+ | -| | | | :ref:`energy` | :ref:`fwdinv` | :ref:`invdiscrete` | :ref:`multiccd` | | +| | | | :ref:`override` | :ref:`energy` | :ref:`fwdinv` | :ref:`invdiscrete` | | | | | +-----------------------------------------------------------------+-----------------------------------------------------------------+-----------------------------------------------------------------+-----------------------------------------------------------------+ | -| | | | :ref:`island` | | | | | +| | | | :ref:`multiccd` | :ref:`island` | | | | | | | +-----------------------------------------------------------------+-----------------------------------------------------------------+-----------------------------------------------------------------+-----------------------------------------------------------------+ | +------------------------------------+----+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ | mujoco |br| |L| | | .. table:: | diff --git a/doc/changelog.rst b/doc/changelog.rst index d22ccc3b..a574c451 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -8,7 +8,8 @@ Upcoming version (not yet released) General ^^^^^^^ 1. Renamed ``mjModel.tex_rbg`` to ``mjModel.tex_data``. -2. Added a new disable flag ``mjDSBL_AUTORESETNAN`` to disable automatic reset when NaNs or infinities are produced. +2. Added a new :ref:`autoreset` flag to disable automatic reset when NaNs or infinities are + detected. 3. Added sub-elements to the MJCF :ref:`material` element, to allow specification of multiple textures for rendering (e.g., :ref:`occlusion-roughness-metallic`). Note that the MuJoCo renderer doesn't support these new features, and they are made available for use with external renderers. diff --git a/doc/includes/references.h b/doc/includes/references.h index 871069f2..d301a2c2 100644 --- a/doc/includes/references.h +++ b/doc/includes/references.h @@ -418,7 +418,7 @@ typedef enum mjtDisableBit_ { // disable default feature bitflags mjDSBL_SENSOR = 1<<12, // sensors mjDSBL_MIDPHASE = 1<<13, // mid-phase collision filtering mjDSBL_EULERDAMP = 1<<14, // implicit integration of joint damping in Euler integrator - mjDSBL_AUTORESETNAN = 1<<15, // automatic reset when numerical issues are detected + mjDSBL_AUTORESET = 1<<15, // automatic reset when numerical issues are detected mjNDISABLE = 16 // number of disable flags } mjtDisableBit; diff --git a/include/mujoco/mjmodel.h b/include/mujoco/mjmodel.h index 73697a66..5f3fe4bd 100644 --- a/include/mujoco/mjmodel.h +++ b/include/mujoco/mjmodel.h @@ -61,7 +61,7 @@ typedef enum mjtDisableBit_ { // disable default feature bitflags mjDSBL_SENSOR = 1<<12, // sensors mjDSBL_MIDPHASE = 1<<13, // mid-phase collision filtering mjDSBL_EULERDAMP = 1<<14, // implicit integration of joint damping in Euler integrator - mjDSBL_AUTORESETNAN = 1<<15, // automatic reset when numerical issues are detected + mjDSBL_AUTORESET = 1<<15, // automatic reset when numerical issues are detected mjNDISABLE = 16 // number of disable flags } mjtDisableBit; diff --git a/introspect/enums.py b/introspect/enums.py index 52bf4c6a..f040505c 100644 --- a/introspect/enums.py +++ b/introspect/enums.py @@ -42,7 +42,7 @@ ENUMS: Mapping[str, EnumDecl] = dict([ ('mjDSBL_SENSOR', 4096), ('mjDSBL_MIDPHASE', 8192), ('mjDSBL_EULERDAMP', 16384), - ('mjDSBL_AUTORESETNAN', 32768), + ('mjDSBL_AUTORESET', 32768), ('mjNDISABLE', 16), ]), )), diff --git a/simulate/main.cc b/simulate/main.cc index 17c3ff5e..15316383 100644 --- a/simulate/main.cc +++ b/simulate/main.cc @@ -199,6 +199,16 @@ void scanPluginLibraries() { //------------------------------------------- simulation ------------------------------------------- +const char* Diverged(int disableflags, const mjData* d) { + if (disableflags & mjDSBL_AUTORESET) { + for (mjtWarning w : {mjWARN_BADQACC, mjWARN_BADQVEL, mjWARN_BADQPOS}) { + if (d->warning[w].number > 0) { + return mju_warningText(w, d->warning[w].lastinfo); + } + } + } + return nullptr; +} mjModel* LoadModel(const char* file, mj::Simulate& sim) { // this copy is needed so that the mju::strlen call below compiles @@ -356,7 +366,13 @@ void PhysicsLoop(mj::Simulate& sim) { // run single step, let next iteration deal with timing mj_step(m, d); - stepped = true; + const char* message = Diverged(m->opt.disableflags, d); + if (message) { + sim.run = 0; + mju::strcpy_arr(sim.load_error, message); + } else { + stepped = true; + } } // in-sync: step until ahead of cpu @@ -381,7 +397,13 @@ void PhysicsLoop(mj::Simulate& sim) { // call mj_step mj_step(m, d); - stepped = true; + const char* message = Diverged(m->opt.disableflags, d); + if (message) { + sim.run = 0; + mju::strcpy_arr(sim.load_error, message); + } else { + stepped = true; + } // break if reset if (d->time < prevSim) { diff --git a/simulate/simulate.cc b/simulate/simulate.cc index 9fe5ae7c..d676129b 100644 --- a/simulate/simulate.cc +++ b/simulate/simulate.cc @@ -739,18 +739,12 @@ void MakePhysicsSection(mj::Simulate* sim) { for (int i=0; idisable + i; - if ((1 << i) == mjDSBL_AUTORESETNAN) { - defFlag[0].state = 0; - } else { - defFlag[0].state = 2; - } mjui_add(&sim->ui0, defFlag); } mjui_add(&sim->ui0, defEnableFlags); for (int i=0; ienable + i; - defFlag[0].state = 2; mjui_add(&sim->ui0, defFlag); } // add contact override @@ -1956,6 +1950,7 @@ void Simulate::Sync() { if (pending_.reset) { mj_resetData(m_, d_); mj_forward(m_, d_); + load_error[0] = '\0'; update_profiler = true; update_sensor = true; scrub_index = 0; diff --git a/src/engine/engine_forward.c b/src/engine/engine_forward.c index 6c78663e..cb326594 100644 --- a/src/engine/engine_forward.c +++ b/src/engine/engine_forward.c @@ -52,7 +52,7 @@ void mj_checkPos(const mjModel* m, mjData* d) { for (int i=0; i < m->nq; i++) { if (mju_isBad(d->qpos[i])) { mj_warning(d, mjWARN_BADQPOS, i); - if (!(m->opt.disableflags & mjDSBL_AUTORESETNAN)) { + if (!mjDISABLED(mjDSBL_AUTORESET)) { mj_resetData(m, d); } d->warning[mjWARN_BADQPOS].number++; @@ -69,7 +69,7 @@ void mj_checkVel(const mjModel* m, mjData* d) { for (int i=0; i < m->nv; i++) { if (mju_isBad(d->qvel[i])) { mj_warning(d, mjWARN_BADQVEL, i); - if (!(m->opt.disableflags & mjDSBL_AUTORESETNAN)) { + if (!mjDISABLED(mjDSBL_AUTORESET)) { mj_resetData(m, d); } d->warning[mjWARN_BADQVEL].number++; @@ -86,12 +86,12 @@ void mj_checkAcc(const mjModel* m, mjData* d) { for (int i=0; i < m->nv; i++) { if (mju_isBad(d->qacc[i])) { mj_warning(d, mjWARN_BADQACC, i); - if (!(m->opt.disableflags & mjDSBL_AUTORESETNAN)) { + if (!mjDISABLED(mjDSBL_AUTORESET)) { mj_resetData(m, d); } d->warning[mjWARN_BADQACC].number++; d->warning[mjWARN_BADQACC].lastinfo = i; - if (!(m->opt.disableflags & mjDSBL_AUTORESETNAN)) { + if (!mjDISABLED(mjDSBL_AUTORESET)) { mj_forward(m, d); } return; diff --git a/src/engine/engine_support.c b/src/engine/engine_support.c index b0e86887..a3f269c0 100644 --- a/src/engine/engine_support.c +++ b/src/engine/engine_support.c @@ -58,7 +58,7 @@ const char* mjDISABLESTRING[mjNDISABLE] = { "Sensor", "Midphase", "Eulerdamp", - "AutoResetNaN" + "AutoReset" }; diff --git a/src/xml/xml_native_reader.cc b/src/xml/xml_native_reader.cc index 4e16d79b..46b13c4f 100644 --- a/src/xml/xml_native_reader.cc +++ b/src/xml/xml_native_reader.cc @@ -115,9 +115,9 @@ const char* MJCF[nMJCF][mjXATTRNUM] = { "solver", "iterations", "ls_iterations", "noslip_iterations", "mpr_iterations", "sdf_iterations", "sdf_initpoints", "actuatorgroupdisable"}, {"<"}, - {"flag", "?", "21", "constraint", "equality", "frictionloss", "limit", "contact", + {"flag", "?", "22", "constraint", "equality", "frictionloss", "limit", "contact", "passive", "gravity", "clampctrl", "warmstart", - "filterparent", "actuation", "refsafe", "sensor", "midphase", "eulerdamp", + "filterparent", "actuation", "refsafe", "sensor", "midphase", "eulerdamp", "autoreset", "override", "energy", "fwdinv", "invdiscrete", "multiccd", "island"}, {">"}, @@ -1122,6 +1122,7 @@ void mjXReader::Option(XMLElement* section, mjOption* opt) { READDSBL("sensor", mjDSBL_SENSOR) READDSBL("midphase", mjDSBL_MIDPHASE) READDSBL("eulerdamp", mjDSBL_EULERDAMP) + READDSBL("autoreset", mjDSBL_AUTORESET) #undef READDSBL #define READENBL(NAME, MASK) \ diff --git a/src/xml/xml_native_writer.cc b/src/xml/xml_native_writer.cc index 5798ef7e..ceb5d836 100644 --- a/src/xml/xml_native_writer.cc +++ b/src/xml/xml_native_writer.cc @@ -1001,6 +1001,7 @@ void mjXWriter::Option(XMLElement* root) { WRITEDSBL("sensor", mjDSBL_SENSOR) WRITEDSBL("midphase", mjDSBL_MIDPHASE) WRITEDSBL("eulerdamp", mjDSBL_EULERDAMP) + WRITEDSBL("autoreset", mjDSBL_AUTORESET) #undef WRITEDSBL #define WRITEENBL(NAME, MASK) \ diff --git a/unity/Runtime/Bindings/MjBindings.cs b/unity/Runtime/Bindings/MjBindings.cs index 46f89ab6..b5563b2e 100644 --- a/unity/Runtime/Bindings/MjBindings.cs +++ b/unity/Runtime/Bindings/MjBindings.cs @@ -157,7 +157,7 @@ public enum mjtDisableBit : int{ mjDSBL_SENSOR = 4096, mjDSBL_MIDPHASE = 8192, mjDSBL_EULERDAMP = 16384, - mjDSBL_AUTORESETNAN = 32768, + mjDSBL_AUTORESET = 32768, mjNDISABLE = 16, } public enum mjtEnableBit : int{ From b341bd391de78fcb2387ba6dbb1199e4eba5017c Mon Sep 17 00:00:00 2001 From: Alessio Quaglino Date: Tue, 30 Jul 2024 09:12:57 -0700 Subject: [PATCH 10/10] Fix a bug with `mjs_delete` functions. PiperOrigin-RevId: 657615906 Change-Id: Ic70eaa9a5c20337db3897cf882a60e9d94f2869b --- python/mujoco/specs.cc | 1 - python/mujoco/specs_test.py | 52 +++++++++++++++++++++++++++++++ src/user/user_api.cc | 4 +-- src/user/user_model.cc | 62 +++++++++++++++++++++++++++++++++++++ src/user/user_model.h | 3 ++ 5 files changed, 119 insertions(+), 3 deletions(-) diff --git a/python/mujoco/specs.cc b/python/mujoco/specs.cc index 70bbc18f..c280911e 100644 --- a/python/mujoco/specs.cc +++ b/python/mujoco/specs.cc @@ -568,7 +568,6 @@ PYBIND11_MODULE(_specs, m) { // ============================= MJSBODY ===================================== mjsBody.def_property_readonly( "id", [](raw::MjsBody& self) -> int { return mjs_getId(self.element); }); - mjsBody.def("delete", [](raw::MjsBody& self) { mjs_delete(self.element); }); mjsBody.def( "add_body", [](raw::MjsBody& self, raw::MjsDefault* default_) -> raw::MjsBody* { diff --git a/python/mujoco/specs_test.py b/python/mujoco/specs_test.py index 0459b013..ad068f91 100644 --- a/python/mujoco/specs_test.py +++ b/python/mujoco/specs_test.py @@ -98,6 +98,34 @@ class SpecsTest(absltest.TestCase): """),) + def test_load_xml(self): + filename = '../../test/testdata/model.xml' + state_type = mujoco.mjtState.mjSTATE_INTEGRATION + + # Load from file. + spec1 = mujoco.MjSpec() + spec1.from_file(filename) + model1 = spec1.compile() + data1 = mujoco.MjData(model1) + mujoco.mj_step(model1, data1) + size1 = mujoco.mj_stateSize(model1, state_type) + state1 = np.empty(size1, np.float64) + mujoco.mj_getState(model1, data1, state1, state_type) + + # Load from string. + spec2 = mujoco.MjSpec() + with open(filename, 'r') as file: + spec2.from_string(file.read().rstrip()) + model2 = spec2.compile() + data2 = mujoco.MjData(model2) + mujoco.mj_step(model2, data2) + size2 = mujoco.mj_stateSize(model2, state_type) + state2 = np.empty(size2, np.float64) + mujoco.mj_getState(model2, data2, state2, state_type) + + # Check that the state is the same. + np.testing.assert_array_equal(state1, state2) + def test_compile_errors_with_line_info(self): spec = mujoco.MjSpec() @@ -269,6 +297,30 @@ class SpecsTest(absltest.TestCase): model = spec.compile({'cube.obj': cube}) self.assertEqual(model.nmeshvert, 8) + def test_delete(self): + filename = '../../test/testdata/model.xml' + + spec = mujoco.MjSpec() + spec.from_file(filename) + + model = spec.compile() + self.assertIsNotNone(model) + self.assertEqual(model.nsite, 11) + self.assertEqual(model.nsensor, 11) + + head = spec.find_body('head') + self.assertIsNotNone(head) + site = head.first_site() + self.assertIsNotNone(site) + + site.delete() + spec.sensors[-1].delete() + spec.sensors[-1].delete() + + model = spec.compile() + self.assertIsNotNone(model) + self.assertEqual(model.nsite, 10) + self.assertEqual(model.nsensor, 9) if __name__ == '__main__': absltest.main() diff --git a/src/user/user_api.cc b/src/user/user_api.cc index 2c3b5aee..ba5890fe 100644 --- a/src/user/user_api.cc +++ b/src/user/user_api.cc @@ -148,7 +148,7 @@ int mjs_detachBody(mjSpec* s, mjsBody* b) { mjCModel* model = static_cast(s->element); mjCBody* body = static_cast(b->element); *model -= *body; - mjs_delete(b->element); + delete body; return 0; } @@ -181,7 +181,7 @@ void mjs_addSpec(mjSpec* s, mjSpec* child) { // delete object, it will call the appropriate destructor since ~mjCBase is virtual void mjs_delete(mjsElement* element) { mjCBase* object = static_cast(element); - delete object; + object->model->DeleteElement(element); } diff --git a/src/user/user_model.cc b/src/user/user_model.cc index fbd94a6b..c7fd5ccb 100644 --- a/src/user/user_model.cc +++ b/src/user/user_model.cc @@ -340,6 +340,68 @@ mjCModel_& mjCModel::operator+=(mjCDef& subtree) { +template +void deletefromlist(std::vector* list, mjsElement* element) { + if (!list) { + return; + } + for (int j = 0; j < list->size(); ++j) { + list->at(j)->id = -1; + if (list->at(j) == element) { + delete list->at(j); + list->erase(list->begin() + j); + j--; + } + } +} + + + +// discard all invalid elements from all lists +void mjCModel::DeleteElement(mjsElement* el) { + mjCBody *world = bodies_[0]; + if (compiled) { + ResetTreeLists(); + } + + switch (el->elemtype) { + case mjOBJ_BODY: + throw mjCError(NULL, "bodies cannot be deleted, use detach instead"); + break; + + case mjOBJ_GEOM: + deletefromlist(&(static_cast(el)->body->geoms), el); + break; + + case mjOBJ_SITE: + deletefromlist(&(static_cast(el)->body->sites), el); + break; + + case mjOBJ_JOINT: + deletefromlist(&(static_cast(el)->body->joints), el); + break; + + case mjOBJ_LIGHT: + deletefromlist(&(static_cast(el)->body->lights), el); + break; + + case mjOBJ_CAMERA: + deletefromlist(&(static_cast(el)->body->cameras), el); + break; + + default: + deletefromlist(object_lists_[el->elemtype], el); + break; + } + + if (compiled) { + MakeLists(world); + ProcessLists(/*checkrepeat=*/false); + } +} + + + // TODO: we should not use C-type casting with multiple C++ inheritance void mjCModel::CreateObjectLists() { for (int i = 0; i < mjNOBJECT; ++i) { diff --git a/src/user/user_model.h b/src/user/user_model.h index 52bef9c1..9f88bb5b 100644 --- a/src/user/user_model.h +++ b/src/user/user_model.h @@ -202,6 +202,9 @@ class mjCModel : public mjCModel_, private mjSpec { // delete all elements template void DeleteAll(std::vector& elements); + // delete object from the corresponding list + void DeleteElement(mjsElement* el); + // API for access to model elements (outside tree) int NumObjects(mjtObj type); // number of objects in specified list mjCBase* GetObject(mjtObj type, int id); // pointer to specified object