diff --git a/src/user/user_model.cc b/src/user/user_model.cc index 09da35e3..7032ea72 100644 --- a/src/user/user_model.cc +++ b/src/user/user_model.cc @@ -1431,7 +1431,6 @@ void mjCModel::IndexAssets(bool discard) { template void mjCModel::SetDefaultNames(std::vector& assets) { string stripped; - std::map> names; // use filename if name is missing for (int i=0; i& assets) { if (assets[i]->name.empty()) { stripped = mjuu_strippath(assets[i]->File()); assets[i]->name = mjuu_stripext(stripped); - names[assets[i]->name].push_back(i); - } - } - - // add suffix if duplicates - for (auto const& [name, indices] : names) { - if (indices.size() > 1) { - for (int i=0; iname += "_" + std::to_string(i); - } } } } diff --git a/src/xml/xml_urdf.cc b/src/xml/xml_urdf.cc index 2c2875c9..a0fc21d6 100644 --- a/src/xml/xml_urdf.cc +++ b/src/xml/xml_urdf.cc @@ -575,6 +575,9 @@ mjsGeom* mjXURDF::Geom(XMLElement* geom_elem, mjsBody* pbody, bool collision) { // mesh else if ((temp = FindSubElem(elem, "mesh"))) { + mjsMesh* pmesh = 0; + bool newmesh = false; + // set geom type and read mesh attributes pgeom->type = mjGEOM_MESH; meshfile = ReadAttrStr(temp, "filename", true).value(); @@ -591,35 +594,43 @@ mjsGeom* mjXURDF::Geom(XMLElement* geom_elem, mjsBody* pbody, bool collision) { std::string meshname = mjuu_strippath(meshfile); meshname = mjuu_stripext(meshname); - // look for existing mesh - mjsMesh* mesh = mjs_asMesh(mjs_findElement(spec, mjOBJ_MESH, meshname.c_str())); - mjsMesh* pmesh = 0; - - // does not exist: create - if (!mesh) { + if (meshes.find(meshname) == meshes.end()) { + // does not exist: create pmesh = mjs_addMesh(spec, 0); - } + meshes[meshname].push_back(pmesh); + newmesh = true; + } else { + int i = 0; - // exists with different scale: append name with '1', create - else if (mesh->scale[0]!=meshscale[0] || - mesh->scale[1]!=meshscale[1] || - mesh->scale[2]!=meshscale[2]) { - pmesh = mjs_addMesh(spec, 0); - meshname = meshname + "1"; - } + // find if it exists with the same scale + for (mjsMesh* mesh : meshes[meshname]) { + if (mesh->scale[0] == meshscale[0] && + mesh->scale[1] == meshscale[1] && + mesh->scale[2] == meshscale[2]) { + pmesh = mesh; + break; + } + i++; + } - // point to already existing spec - else { - pmesh = mesh; + // add a new spec making an incremental new name + if (i == meshes[meshname].size()) { + pmesh = mjs_addMesh(spec, 0); + meshes[meshname].push_back(pmesh); + meshname = meshname + std::to_string(i); + newmesh = true; + } } // set fields - mjs_setString(pmesh->file, meshfile.c_str()); - mjs_setString(pmesh->name, meshname.c_str()); + if (newmesh) { + mjs_setString(pmesh->file, meshfile.c_str()); + mjs_setString(pmesh->name, meshname.c_str()); + pmesh->scale[0] = meshscale[0]; + pmesh->scale[1] = meshscale[1]; + pmesh->scale[2] = meshscale[2]; + } mjs_setString(pgeom->meshname, meshname.c_str()); - pmesh->scale[0] = meshscale[0]; - pmesh->scale[1] = meshscale[1]; - pmesh->scale[2] = meshscale[2]; } else { diff --git a/src/xml/xml_urdf.h b/src/xml/xml_urdf.h index 2a3f682f..eeda989b 100644 --- a/src/xml/xml_urdf.h +++ b/src/xml/xml_urdf.h @@ -15,6 +15,7 @@ #ifndef THIRD_PARTY_MUJOCO_SRC_XML_XML_URDF_ #define THIRD_PARTY_MUJOCO_SRC_XML_XML_URDF_ +#include #include #include #include @@ -66,6 +67,7 @@ class mjXURDF : public mjXBase { std::vector urMat; // material name std::vector urRGBA; // material RBG value std::unordered_set urGeomNames; // geom name + std::map> meshes; // map from name to mjsMesh std::string urPrefix; // prefix to apply to all names }; diff --git a/test/user/testdata/load_twice.xml b/test/user/testdata/malformed_duplicated.xml similarity index 100% rename from test/user/testdata/load_twice.xml rename to test/user/testdata/malformed_duplicated.xml diff --git a/test/user/user_model_test.cc b/test/user/user_model_test.cc index 03b06985..74a14f77 100644 --- a/test/user/user_model_test.cc +++ b/test/user/user_model_test.cc @@ -280,22 +280,14 @@ TEST_F(UserDataTest, AutoNUserSensor) { // ------------- test duplicate names ------------------------------------------ TEST_F(UserDataTest, DuplicateNames) { - static const char* const kFilePath = "user/testdata/load_twice.xml"; + static const char* const kFilePath = "user/testdata/malformed_duplicated.xml"; const std::string xml_path = GetTestDataFilePath(kFilePath); std::array error; mjModel* m = mj_loadXML(xml_path.c_str(), 0, error.data(), error.size()); - EXPECT_THAT(m, NotNull()) << error.data(); - EXPECT_THAT(m->nmesh, 2); - - for (int i = 0; i < m->nmesh; i++) { - char mesh_name[mjMAXUINAME] = ""; - util::strcat_arr(mesh_name, m->names + m->name_meshadr[i]); - EXPECT_THAT(std::string(mesh_name), "cube_" + std::to_string(i)); - } - - mj_deleteModel(m); + EXPECT_THAT(m, IsNull()); + EXPECT_STREQ(error.data(), "Error: repeated name 'cube' in mesh"); } // ------------- test fusestatic ----------------------------------------------- diff --git a/test/xml/xml_urdf_test.cc b/test/xml/xml_urdf_test.cc index 50a45da0..a7524280 100644 --- a/test/xml/xml_urdf_test.cc +++ b/test/xml/xml_urdf_test.cc @@ -15,8 +15,6 @@ // Tests for xml/xml_api.cc. #include -#include -#include #include #include @@ -28,6 +26,7 @@ namespace mujoco { namespace { +using ::testing::IsNull; using ::testing::NotNull; // ---------------------------- test capsule -------------------------------- @@ -252,5 +251,69 @@ TEST_F(MujocoTest, ReadsJointTypes) { mj_deleteModel(model); } +TEST_F(MujocoTest, RepeatedMeshName) { + static constexpr char urdf[] = R"( + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + )"; + + std::array error; + mjSpec* spec = mj_parseXMLString(urdf, 0, error.data(), error.size()); + EXPECT_THAT(spec, NotNull()) << error.data(); + + mjsMesh* mesh = mjs_asMesh(mjs_findElement(spec, mjOBJ_MESH, "mesh")); + mjsMesh* mesh1 = mjs_asMesh(mjs_findElement(spec, mjOBJ_MESH, "mesh1")); + mjsMesh* mesh2 = mjs_asMesh(mjs_findElement(spec, mjOBJ_MESH, "mesh2")); + mjsMesh* mesh3 = mjs_asMesh(mjs_findElement(spec, mjOBJ_MESH, "mesh3")); + EXPECT_THAT(mesh, NotNull()); + EXPECT_THAT(mesh1, NotNull()); + EXPECT_THAT(mesh2, NotNull()); + EXPECT_THAT(mesh3, IsNull()); + EXPECT_STREQ(mjs_getString(mesh->name), "mesh"); + EXPECT_STREQ(mjs_getString(mesh1->name), "mesh1"); + EXPECT_STREQ(mjs_getString(mesh2->name), "mesh2"); + + mj_deleteSpec(spec); +} + } // namespace } // namespace mujoco