diff --git a/doc/changelog.rst b/doc/changelog.rst index 03b768f7..ff3091c8 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -23,6 +23,13 @@ General **Migration:** The flag :ref:`multiccd` must be explicitly disabled. +Bug fixes +^^^^^^^^^ + +- Asset paths in attached child specs are now resolved relative to the model file directory of the child spec, rather + than the parent spec. This prevents the origin of the parent spec to affect the resolution of asset paths in the child + spec. + Version 3.7.0 (April 14, 2026) ------------------------------ diff --git a/src/user/user_mesh.cc b/src/user/user_mesh.cc index ae54b634..a7b7dc04 100644 --- a/src/user/user_mesh.cc +++ b/src/user/user_mesh.cc @@ -300,9 +300,6 @@ void mjCMesh::NameSpace(const mjCModel* m) { name = mjuu_stripext(stripped); } mjCBase::NameSpace(m); - if (modelfiledir_.empty()) { - modelfiledir_ = FilePath(m->spec_modelfiledir_); - } if (!plugin_instance_name.empty()) { plugin_instance_name = m->prefix + plugin_instance_name + m->suffix; } @@ -712,17 +709,14 @@ void mjCMesh::TryCompile(const mjVFS* vfs) { mujoco::user::FilePath meshdir_; meshdir_ = FilePath(mjs_getString(compiler->meshdir)); - if (modelfiledir_.empty()) { - modelfiledir_ = FilePath(model->modelfiledir_); - } - // remove path from file if necessary if (model->strippath) { file_ = mjuu_strippath(file_); } + mjSpec* owning_spec = model->FindSpec(compiler); FilePath filename = meshdir_ + FilePath(file_); - resource_ = LoadResource(modelfiledir_.Str(), filename.Str(), vfs); + resource_ = LoadResource(owning_spec->modelfiledir->c_str(), filename.Str(), vfs); // try loading from cache if (cache != nullptr && LoadCachedMesh(cache, resource_)) { @@ -2957,9 +2951,6 @@ void mjCSkin::NameSpace(const mjCModel* m) { for (auto& name : spec_bodyname_) { name = m->prefix + name + m->suffix; } - if (modelfiledir_.empty()) { - modelfiledir_ = FilePath(m->spec_modelfiledir_); - } } @@ -3046,15 +3037,12 @@ void mjCSkin::Compile(const mjVFS* vfs) { throw mjCError(this, "Unknown skin file type: %s", file_.c_str()); } - // copy paths from model if not already defined - if (modelfiledir_.empty()) { - modelfiledir_ = FilePath(model->modelfiledir_); - } mujoco::user::FilePath meshdir_; meshdir_ = FilePath(mjs_getString(compiler->meshdir)); FilePath filename = meshdir_ + FilePath(file_); - mjResource* resource = LoadResource(modelfiledir_.Str(), filename.Str(), vfs); + mjSpec* owning_spec = model->FindSpec(compiler); + mjResource* resource = LoadResource(owning_spec->modelfiledir->c_str(), filename.Str(), vfs); try { LoadSKN(resource); diff --git a/src/user/user_objects.cc b/src/user/user_objects.cc index f83bd387..75a7e679 100644 --- a/src/user/user_objects.cc +++ b/src/user/user_objects.cc @@ -4668,9 +4668,6 @@ void mjCHField::NameSpace(const mjCModel* m) { name = mjuu_stripext(stripped); } mjCBase::NameSpace(m); - if (modelfiledir_.empty()) { - modelfiledir_ = FilePath(m->spec_modelfiledir_); - } } @@ -4798,15 +4795,12 @@ void mjCHField::Compile(const mjVFS* vfs) { throw mjCError(this, "unsupported content type: '%s'", asset_type.c_str()); } - // copy paths from model if not already defined - if (modelfiledir_.empty()) { - modelfiledir_ = FilePath(model->modelfiledir_); - } mujoco::user::FilePath meshdir_; meshdir_ = FilePath(mjs_getString(compiler->meshdir)); FilePath filename = meshdir_ + FilePath(file_); - mjResource* resource = LoadResource(modelfiledir_.Str(), filename.Str(), vfs); + mjSpec* owning_spec = model->FindSpec(compiler); + mjResource* resource = LoadResource(owning_spec->modelfiledir->c_str(), filename.Str(), vfs); struct CachedHField { int nrow, ncol; @@ -4965,9 +4959,6 @@ void mjCTexture::NameSpace(const mjCModel* m) { name = mjuu_stripext(stripped); } mjCBase::NameSpace(m); - if (modelfiledir_.empty()) { - modelfiledir_ = FilePath(m->spec_modelfiledir_); - } } @@ -5388,7 +5379,8 @@ void mjCTexture::LoadFlip(std::string filename, const mjVFS* vfs, } // try loading from cache - mjResource* resource = LoadResource(modelfiledir_.Str(), filename, vfs); + mjSpec* owning_spec = model->FindSpec(compiler); + mjResource* resource = LoadResource(owning_spec->modelfiledir->c_str(), filename, vfs); if (cache && cache->PopulateData(GetCacheId(resource, asset_type), resource, callback)) { mju_closeResource(resource); return; @@ -5640,10 +5632,6 @@ void mjCTexture::LoadCubeSeparate(const mjVFS* vfs) { void mjCTexture::Compile(const mjVFS* vfs) { CopyFromSpec(); - // copy paths from model if not already defined - if (modelfiledir_.empty()) { - modelfiledir_ = FilePath(model->modelfiledir_); - } mujoco::user::FilePath texturedir_; texturedir_ = FilePath(mjs_getString(compiler->texturedir)); diff --git a/src/user/user_objects.h b/src/user/user_objects.h index 5c81e40c..4f4da58d 100644 --- a/src/user/user_objects.h +++ b/src/user/user_objects.h @@ -1122,9 +1122,6 @@ class mjCMesh_ : public mjCBase { // octree mjCOctree octree_; // octree of the mesh - - // paths stored during model attachment - mujoco::user::FilePath modelfiledir_; }; class mjCMesh: public mjCMesh_, private mjsMesh { @@ -1336,9 +1333,6 @@ class mjCSkin_ : public mjCBase { int matid; // material id std::vector bodyid; // body ids - - // paths stored during model attachment - mujoco::user::FilePath modelfiledir_; }; class mjCSkin: public mjCSkin_, private mjsSkin { @@ -1391,9 +1385,6 @@ class mjCHField_ : public mjCBase { std::string spec_file_; std::string spec_content_type_; std::vector spec_userdata_; - - // paths stored during model attachment - mujoco::user::FilePath modelfiledir_; }; class mjCHField : public mjCHField_, private mjsHField { @@ -1442,9 +1433,6 @@ class mjCTexture_ : public mjCBase { std::string spec_file_; std::string spec_content_type_; std::vector spec_cubefiles_; - - // paths stored during model attachment - mujoco::user::FilePath modelfiledir_; }; class mjCTexture : public mjCTexture_, private mjsTexture { diff --git a/test/user/user_api_test.cc b/test/user/user_api_test.cc index 3c7af426..57e889bb 100644 --- a/test/user/user_api_test.cc +++ b/test/user/user_api_test.cc @@ -19,6 +19,7 @@ #include #include #include +#include #include // NOLINT #include #include @@ -32,6 +33,7 @@ #include "src/cc/array_safety.h" #include #include +#include #include "src/xml/xml_api.h" #include "src/xml/xml_numeric_format.h" #include "test/fixture.h" @@ -203,6 +205,103 @@ TEST_F(MujocoTest, AttachAndChildDeletion) { mj_deleteSpec(parent_spec); } +int open_mock(mjResource* resource) { + static const char parent_xml[] = R"( + + + + + + )"; + resource->data = mju_malloc(sizeof(parent_xml)); + std::strcpy((char*)resource->data, parent_xml); + return 1; +} + +int read_mock(mjResource* resource, const void** buffer) { + *buffer = resource->data; + return std::strlen((const char*)resource->data); +} + +void close_mock(mjResource* resource) { + mju_free(resource->data); + resource->data = nullptr; +} + +TEST_F(MujocoTest, AttachedSpecDoesNotInheritURI) { + // This test checks that when we attach a child spec to a parent spec that was + // loaded from a resource provider, the child spec does not inherit the + // resource URI from the parent. This allows the child spec to specify assets + // relative to its model file or in the VFS. + mjpResourceProvider provider = { + .prefix = "fakeprovider", + .open = open_mock, + .read = read_mock, + .close = close_mock, + }; + + mjp_registerResourceProvider(&provider); + + std::array err; + mjSpec* parent_spec = + mj_parseXML("fakeprovider:parent.xml", nullptr, err.data(), err.size()); + mjs_setString(parent_spec->modelname, "parent"); + ASSERT_THAT(parent_spec, NotNull()) << err.data(); + + // Create child spec + static constexpr char child_xml[] = R"( + + + + + + + + + + + )"; + + // Setup VFS with asset + mjVFS vfs; + mj_defaultVFS(&vfs); + static constexpr char asset_data[] = R"( + v 0 0 0 + v 1 0 0 + v 0 1 0 + v 0 0 1 + f 1 2 3 + f 1 2 4 + f 2 3 4 + f 3 1 4 + )"; + mj_addBufferVFS(&vfs, "asset.obj", asset_data, sizeof(asset_data)); + + mjSpec* child_spec = + mj_parseXMLString(child_xml, &vfs, err.data(), err.size()); + mjs_setString(child_spec->modelname, "child"); + ASSERT_THAT(child_spec, NotNull()) << err.data(); + + // Attach child spec to parent spec's world body + mjsBody* world = mjs_findBody(parent_spec, "world"); + ASSERT_THAT(world, NotNull()); + + mjsElement* attached = + mjs_attach(world->element, child_spec->element, "", ""); + ASSERT_THAT(attached, NotNull()); + + mjModel* model = mj_compile(parent_spec, &vfs); + mj_deleteVFS(&vfs); + + EXPECT_THAT(model, NotNull()) << mjs_getError(parent_spec); + + if (model) { + mj_deleteModel(model); + } + mj_deleteSpec(parent_spec); + mj_deleteSpec(child_spec); +} + TEST_F(MujocoTest, ActivatePlugin) { mjSpec* spec = mj_makeSpec(); mjs_activatePlugin(spec, "mujoco.elasticity.cable");