Add support for vfs assets in mj_compile function to MuJoCo WASM.
Regarding mj_compile VFS argument: I was encountering a memory error when using `std::optional<MjVFS>` with `mj_compile` (because vfs should be an optional argument of the function). This is likely because `MjVFS` doesn't have copy/move constructors, which are required for types used with `std::optional`, causing issues with memory management when std::optional copies the MjVFS object. To solve this and to make vfs an optional argument to `mj_compile`, I replaced `mj_compile_wrapper` with two overloaded functions. This avoids using `std::optional<MjVFS>` and resolves the memory issue, while still allowing mj_compile to be called with or without the vfs argument from JavaScript. But what if we instead add those copy/move constructors so it works with `std::optional`?: While making `MjVFS` copyable or movable would theoretically allow it to work with std::optional, it's not a feasible approach in this case. The MjVFS struct manages file data in memory, allocating buffers when files are added and freeing them when `mj_deleteVFS` is called in its destructor. A copy would require a deep copy of the entire virtual filesystem, but MuJoCo does not provide a public API function (like a "mj_copyVFS") to do this. Implementing a deep copy manually would be complex and fragile. A shallow copy would result in multiple MjVFS objects pointing to the same memory, leading to double-free errors when their destructors run. Given these constraints, MjVFS cannot safely be made copyable. The approach of providing two overloaded C++ functions is better. PiperOrigin-RevId: 855740541 Change-Id: Ia08b6db8868ad245f07145db3fb0322cb9f737cb
This commit is contained in:
committed by
Copybara-Service
parent
7e21f18fd3
commit
37762e3f70
@@ -17,7 +17,7 @@ import 'jasmine';
|
||||
import {MainModule, MjContact, MjContactVec, MjData, MjLROpt, MjModel,
|
||||
MjOption, MjsGeom, MjSolverStat, MjSpec, MjStatistic, MjTimerStat, MjvCamera,
|
||||
MjvFigure, MjvGeom, MjvGLCamera, MjvLight, MjvOption, MjvPerturb, MjvScene,
|
||||
MjWarningStat} from '../dist/mujoco_wasm.js';
|
||||
MjWarningStat, MjVFS} from '../dist/mujoco_wasm.js';
|
||||
|
||||
import loadMujoco from '../dist/mujoco_wasm.js'
|
||||
|
||||
@@ -1803,7 +1803,7 @@ describe('MuJoCo WASM Bindings', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('should compile a spec from XML string', () => {
|
||||
it('should compile a spec from XML string with no assets', () => {
|
||||
let spec = null;
|
||||
let model = null;
|
||||
try {
|
||||
@@ -1826,4 +1826,50 @@ describe('MuJoCo WASM Bindings', () => {
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it('should compile a spec from XML with .obj asset', () => {
|
||||
const xml = `
|
||||
<mujoco>
|
||||
<asset>
|
||||
<mesh file="cube.obj"/>
|
||||
</asset>
|
||||
<worldbody>
|
||||
<geom type="mesh" mesh="cube"/>
|
||||
</worldbody>
|
||||
</mujoco>`;
|
||||
|
||||
const cube1 = `
|
||||
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`;
|
||||
|
||||
let spec: MjSpec|null = null;
|
||||
let model: MjModel|null = null;
|
||||
let vfs: MjVFS|null = null;
|
||||
try {
|
||||
spec = mujoco.parseXMLString(xml);
|
||||
assertExists(spec);
|
||||
|
||||
vfs = new mujoco.MjVFS();
|
||||
vfs.addBuffer('cube.obj', new TextEncoder().encode(cube1));
|
||||
assertExists(vfs);
|
||||
|
||||
model = mujoco.mj_compile(spec, vfs);
|
||||
assertExists(model);
|
||||
expect(model.nmesh).toBe(1);
|
||||
|
||||
const meshId =
|
||||
mujoco.mj_name2id(model, mujoco.mjtObj.mjOBJ_MESH.value, 'cube');
|
||||
expect(meshId).toBeGreaterThanOrEqual(0);
|
||||
} finally {
|
||||
spec?.delete();
|
||||
vfs?.delete();
|
||||
model?.delete();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user