Support FilePaths instead of raw strings in XML code.

PiperOrigin-RevId: 655177026
Change-Id: I55b055551384b01ba7a4baa5d75fbe3329b7d25d

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));

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<std::string>& included) {
+void IncludeXML(mjXReader& reader, XMLElement* elem,
+                const FilePath& dir, const mjVFS* vfs,
+                std::unordered_set<std::string>& 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<char, 1024> 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<std::string> 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);
 }

src/xml/xml.h

@@ -16,22 +16,19 @@
 #define MUJOCO_SRC_XML_XML_H_
 
 #include <string>
+#include <string_view>
 
-#include <mujoco/mjexport.h>
 #include <mujoco/mjmodel.h>
 #include <mujoco/mjspec.h>
 
-// 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_

src/xml/xml_api.cc

@@ -37,6 +37,8 @@
 
 //---------------------------------- Globals -------------------------------------------------------
 
+namespace {
+
 // global user model class
 class GlobalModel {
  public:
@@ -61,7 +63,7 @@ std::optional<std::string> 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<mjSpec, std::function<void(mjSpec*)>> 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);

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<string> 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<std::string> cubefiles(6);
+      std::vector<std::string> 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<char, 1024> 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_;
 }

src/xml/xml_native_reader.h

@@ -22,6 +22,7 @@
 
 #include <mujoco/mujoco.h>
 #include <mujoco/mjspec.h>
+#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

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<T> 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::string>
+std::optional<FilePath>
 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

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<typename T>
-  static std::optional<std::vector<T>> ReadAttrVec(tinyxml2::XMLElement* elem, const char* attr,
+  static std::optional<std::vector<T>> ReadAttrVec(tinyxml2::XMLElement* elem,
+                                                   const char* attr,
                                                    bool required = false);
 
   // if attribute is present, return attribute as a string
-  static std::optional<std::string> ReadAttrStr(tinyxml2::XMLElement* elem, const char* attr,
+  static std::optional<std::string> ReadAttrStr(tinyxml2::XMLElement* elem,
+                                                const char* attr,
                                                 bool required = false);
 
   // if attribute is present, return attribute as a filename
-  static std::optional<std::string> ReadAttrFile(tinyxml2::XMLElement* elem, const char* attr,
-                                                 const std::string& dir = "", bool required = false);
+  static std::optional<mujoco::user::FilePath>
+      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<typename T>
-  static std::optional<T> ReadAttrNum(tinyxml2::XMLElement* elem, const char* attr,
+  static std::optional<T> 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<typename T, int N>
-  static std::optional<std::array<T, N>> ReadAttrArr(tinyxml2::XMLElement* elem, const char* attr,
+  static std::optional<std::array<T, N>> ReadAttrArr(tinyxml2::XMLElement* elem,
+                                                     const char* attr,
                                                      bool required = false) {
     std::array<T, N> arr;
     int n = 0;

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");

test/xml/xml_native_reader_test.cc

@@ -649,7 +649,7 @@ TEST_F(XMLReaderTest, IncludeTest) {
   std::array<char, 1024> 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<char, 1024> 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<char, 1024> 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<char, 1024> 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);
 }