Remove modelfiledir_ from compiled assets, use owning spec modelfiledir instead.
Previously specs that were attached to some parent spec would resolve its asset paths relative to the modelfiledir of the parent spec. This means path resolution would change depending on the source of the parent spec. Instead, this change makes asset file path resolution relative to the "owning spec" i.e. the spec where the asset was created. This enables workflows such as loading a parent spec via resource provider, then loading a child spec via `from_zip` or in memory providing `spec.assets` and resolution will work as intended. PiperOrigin-RevId: 903207910 Change-Id: Ia58020ab372a3ceadf31e804d145e2ae53d8e5f9
This commit is contained in:
committed by
Copybara-Service
parent
3325971840
commit
da01bd37a2
@@ -23,6 +23,13 @@ General
|
||||
|
||||
**Migration:** The flag :ref:`multiccd<option-flag-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)
|
||||
------------------------------
|
||||
|
||||
|
||||
+4
-16
@@ -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);
|
||||
|
||||
@@ -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));
|
||||
|
||||
|
||||
@@ -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<int> 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<float> 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<std::string> spec_cubefiles_;
|
||||
|
||||
// paths stored during model attachment
|
||||
mujoco::user::FilePath modelfiledir_;
|
||||
};
|
||||
|
||||
class mjCTexture : public mjCTexture_, private mjsTexture {
|
||||
|
||||
@@ -19,6 +19,7 @@
|
||||
#include <cctype>
|
||||
#include <cstddef>
|
||||
#include <cstdint>
|
||||
#include <cstring>
|
||||
#include <filesystem> // NOLINT
|
||||
#include <functional>
|
||||
#include <map>
|
||||
@@ -32,6 +33,7 @@
|
||||
#include "src/cc/array_safety.h"
|
||||
#include <mujoco/mujoco.h>
|
||||
#include <mujoco/mjspec.h>
|
||||
#include <mujoco/mjplugin.h>
|
||||
#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"(
|
||||
<mujoco>
|
||||
<worldbody>
|
||||
<body name="parent_body"/>
|
||||
</worldbody>
|
||||
</mujoco>
|
||||
)";
|
||||
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<char, 1024> 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"(
|
||||
<mujoco>
|
||||
<worldbody>
|
||||
<body name="child_body">
|
||||
<geom type="mesh" mesh="asset"/>
|
||||
</body>
|
||||
</worldbody>
|
||||
<asset>
|
||||
<mesh name="asset" file="asset.obj"/>
|
||||
</asset>
|
||||
</mujoco>
|
||||
)";
|
||||
|
||||
// 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");
|
||||
|
||||
Reference in New Issue
Block a user