From 57e6760ec9bba5284ca17ca1b007dc4382dfa353 Mon Sep 17 00:00:00 2001 From: Kyle Bayes Date: Thu, 4 Jul 2024 09:21:18 -0700 Subject: [PATCH] Re-implement VFS internals in C++, removing the constraints of the previous implementation. PiperOrigin-RevId: 649433087 Change-Id: Icaa3e6b7f2ef14f56b04fef2f4360b46d8b5f022 --- doc/APIreference/APIglobals.rst | 6 - doc/changelog.rst | 41 +- doc/includes/references.h | 6 +- include/mujoco/mjmodel.h | 8 +- introspect/structs.py | 40 +- python/mujoco/bindings_test.py | 16 - python/mujoco/constants.cc | 2 - python/mujoco/structs.cc | 22 +- src/engine/engine_util_misc.c | 45 -- src/engine/engine_util_misc.h | 4 - src/user/CMakeLists.txt | 2 +- src/user/user_vfs.c | 420 ------------------- src/user/user_vfs.cc | 322 ++++++++++++++ src/user/user_vfs.h | 4 +- test/engine/engine_util_misc_test.cc | 86 ---- test/user/user_vfs_test.cc | 191 +++++++-- unity/Runtime/Bindings/MjBindings.cs | 8 +- unity/Runtime/Tools/MjVfs.cs | 5 - unity/Tests/Editor/Tools/MjVFSEditorTests.cs | 2 +- 19 files changed, 513 insertions(+), 717 deletions(-) delete mode 100644 src/user/user_vfs.c create mode 100644 src/user/user_vfs.cc diff --git a/doc/APIreference/APIglobals.rst b/doc/APIreference/APIglobals.rst index 2d08ce08..3fef402d 100644 --- a/doc/APIreference/APIglobals.rst +++ b/doc/APIreference/APIglobals.rst @@ -414,12 +414,6 @@ shown in the table below. Their names are in the format ``mjKEY_XXX``. They corr - 50 - The maximum depth of each body and mesh bounding volume hierarchy. If this large limit is exceeded, a warning is raised and ray casting may not be possible. For a balanced hierarchy, this implies 1E15 bounding volumes. - * - ``mjMAXVFS`` - - 200 - - The maximal number of characters in the name of each file in the virtual file system. - * - ``mjMAXVFSNAME`` - - 100 - - The maximal number of characters in the name of each file in the virtual file system. * - ``mjNEQDATA`` - 11 - The maximal number of real-valued parameters used to define each equality constraint. Determines the size of diff --git a/doc/changelog.rst b/doc/changelog.rst index 541c3c22..66a41c3c 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -10,11 +10,12 @@ General .. admonition:: Breaking API changes :class: attention - 1. Removed deprecated ``mj_makeEmptyFileVFS`` and ``mj_findFileVFS`` functions. + 1. Removed deprecated ``mj_makeEmptyFileVFS`` and ``mj_findFileVFS`` functions. The constants ``mjMAXVFS`` and + ``mjMAXVFSNAME`` are also removed as they are no longer needed. **Migration:** Use :ref:`mj_addBufferVFS` to copy a buffer into a VFS file directly. - 2. Calls to:ref:`mj_defaultVFS` may allocate memory inside VFS, and the corresponding + 2. Calls to :ref:`mj_defaultVFS` may allocate memory inside VFS, and the corresponding :ref:`mj_deleteVFS` must be called to deallocate any internal allocated memory. 3. Deprecated :ref:`mju_rotVecMat` and :ref:`mju_rotVecMatT` in favor of :ref:`mju_mulMatVec3` and @@ -22,7 +23,9 @@ General The older functions have been removed from the Python bindings and will be removed from the C API in the next release. -4. Added a new API for :doc:`procedural model manipulation`. Fixes :github:issue:`364`. +4. The :ref:`VFS` implementation has been rewritten in C++ and is now considerably more efficient in + speed and memory footprint. +5. Added a new API for :doc:`procedural model manipulation`. Fixes :github:issue:`364`. Still missing: - Detailed documentation. @@ -31,27 +34,27 @@ General :align: right :width: 240px -5. Added support for orthographic cameras. This is available for both fixed cameras and the free camera, using the +6. Added support for orthographic cameras. This is available for both fixed cameras and the free camera, using the :ref:`camera/orthographic` and :ref:`global/orthographic` attributes, respectively. -6. Added :ref:`maxhullvert`, the maximum number of vertices in a mesh's convex hull. -7. Added :ref:`mj_setKeyframe` for saving the current state into a model keyframe. -8. Added support for ``ball`` joints in the URDF parser ("spherical" in URDF). -9. Replaced ``mjUSEDOUBLE`` which was previously hard-coded in - `mjtnum.h `__ - with the build-time flag ``mjUSESINGLE``. If this symbol is not defined, MuJoCo will use double-precision floating - point, as usual. If ``mjUSESINGLE`` is defined, MuJoCo will use single-precision floating point. See :ref:`mjtNum`. +7. Added :ref:`maxhullvert`, the maximum number of vertices in a mesh's convex hull. +8. Added :ref:`mj_setKeyframe` for saving the current state into a model keyframe. +9. Added support for ``ball`` joints in the URDF parser ("spherical" in URDF). +10. Replaced ``mjUSEDOUBLE`` which was previously hard-coded in + `mjtnum.h `__ + with the build-time flag ``mjUSESINGLE``. If this symbol is not defined, MuJoCo will use double-precision floating + point, as usual. If ``mjUSESINGLE`` is defined, MuJoCo will use single-precision floating point. See :ref:`mjtNum`. - Relatedly, fixed various type errors that prevented building with single-precision. -10. Quaternions in ``mjData->qpos`` and ``mjData->mocap_quat`` are no longer normalized in-place by + Relatedly, fixed various type errors that prevented building with single-precision. +11. Quaternions in ``mjData->qpos`` and ``mjData->mocap_quat`` are no longer normalized in-place by :ref:`mj_kinematics`. Instead they are normalized when they are used. After the first step, quaternions in ``mjData->qpos`` will be normalized. MJX ~~~ -10. Added support for :ref:`elliptic friction cones`. -11. Fixed a bug that resulted in less-optimal linesearch solutions for some difficult constraint settings. -12. Fixed a bug in the Newton solver that sometimes resulted in less-optimal gradients. +12. Added support for :ref:`elliptic friction cones`. +13. Fixed a bug that resulted in less-optimal linesearch solutions for some difficult constraint settings. +14. Fixed a bug in the Newton solver that sometimes resulted in less-optimal gradients. .. youtube:: P83tKA1iz2Y @@ -60,14 +63,14 @@ MJX Simulate ^^^^^^^^ -13. Added improved tutorial video. -14. Improved the Brownian noise generator. +15. Added improved tutorial video. +16. Improved the Brownian noise generator. |br| |br| |br| |br| Python bindings ^^^^^^^^^^^^^^^ -15. Fixed a memory leak when using ``copy.deepcopy()`` on a ``mujoco.MjData`` instance (:github:issue:`1572`). +17. Fixed a memory leak when using ``copy.deepcopy()`` on a ``mujoco.MjData`` instance (:github:issue:`1572`). Version 3.1.6 (Jun 3, 2024) --------------------------- diff --git a/doc/includes/references.h b/doc/includes/references.h index 2e3fe36b..257bd64f 100644 --- a/doc/includes/references.h +++ b/doc/includes/references.h @@ -699,11 +699,7 @@ struct mjLROpt_ { // options for mj_setLengthRange() }; typedef struct mjLROpt_ mjLROpt; struct mjVFS_ { // virtual file system for loading from memory - int nfile; // number of files present - char filename[mjMAXVFS][mjMAXVFSNAME]; // file name without path - size_t filesize[mjMAXVFS]; // file size in bytes - void* filedata[mjMAXVFS]; // buffer with file data - uint64_t filestamp[mjMAXVFS]; // checksum of the file data + void* impl_; // internal pointer to VFS memory }; typedef struct mjVFS_ mjVFS; struct mjOption_ { // physics options diff --git a/include/mujoco/mjmodel.h b/include/mujoco/mjmodel.h index 9da23e75..2bafdd13 100644 --- a/include/mujoco/mjmodel.h +++ b/include/mujoco/mjmodel.h @@ -29,8 +29,6 @@ #define mjMAXIMP 0.9999 // maximum constraint impedance #define mjMAXCONPAIR 50 // maximum number of contacts per geom pair #define mjMAXTREEDEPTH 50 // maximum bounding volume hierarchy depth -#define mjMAXVFS 2000 // maximum number of files in virtual file system -#define mjMAXVFSNAME 1000 // maximum filename size in virtual file system //---------------------------------- sizes --------------------------------------------------------- @@ -399,11 +397,7 @@ typedef struct mjLROpt_ mjLROpt; //---------------------------------- mjVFS --------------------------------------------------------- struct mjVFS_ { // virtual file system for loading from memory - int nfile; // number of files present - char filename[mjMAXVFS][mjMAXVFSNAME]; // file name without path - size_t filesize[mjMAXVFS]; // file size in bytes - void* filedata[mjMAXVFS]; // buffer with file data - uint64_t filestamp[mjMAXVFS]; // checksum of the file data + void* impl_; // internal pointer to VFS memory }; typedef struct mjVFS_ mjVFS; diff --git a/introspect/structs.py b/introspect/structs.py index 180ed135..d5ca63a5 100644 --- a/introspect/structs.py +++ b/introspect/structs.py @@ -91,43 +91,11 @@ STRUCTS: Mapping[str, StructDecl] = dict([ declname='struct mjVFS_', fields=( StructFieldDecl( - name='nfile', - type=ValueType(name='int'), - doc='number of files present', - ), - StructFieldDecl( - name='filename', - type=ArrayType( - inner_type=ValueType(name='char'), - extents=(2000, 1000), + name='impl_', + type=PointerType( + inner_type=ValueType(name='void'), ), - doc='file name without path', - ), - StructFieldDecl( - name='filesize', - type=ArrayType( - inner_type=ValueType(name='size_t'), - extents=(2000,), - ), - doc='file size in bytes', - ), - StructFieldDecl( - name='filedata', - type=ArrayType( - inner_type=PointerType( - inner_type=ValueType(name='void'), - ), - extents=(2000,), - ), - doc='buffer with file data', - ), - StructFieldDecl( - name='filestamp', - type=ArrayType( - inner_type=ValueType(name='uint64_t'), - extents=(2000,), - ), - doc='checksum of the file data', + doc='internal pointer to VFS memory', ), ), )), diff --git a/python/mujoco/bindings_test.py b/python/mujoco/bindings_test.py index 4ea6b3a9..f6201776 100644 --- a/python/mujoco/bindings_test.py +++ b/python/mujoco/bindings_test.py @@ -409,19 +409,6 @@ class MuJoCoBindingsTest(parameterized.TestCase): np.testing.assert_array_equal(self.model.geom_size[1], [0.5, 0.5, 0.5]) np.testing.assert_array_equal(model_copy.geom_size[1], [0.1, 0.1, 0.1]) - def test_assets_array_filename_too_long(self): - # Longest allowed filename (excluding null byte) - limit = mujoco.mjMAXVFSNAME - 1 - contents = b'' - valid_filename = 'a' * limit - mujoco.MjModel.from_xml_path(valid_filename, {valid_filename: contents}) - invalid_filename = 'a' * (limit + 1) - expected_message = ( - f'Filename length 1000 exceeds 999 character limit: {invalid_filename}') - with self.assertRaisesWithLiteralMatch(ValueError, expected_message): - mujoco.MjModel.from_xml_path(invalid_filename, - {invalid_filename: contents}) - def test_mjdata_can_copy(self): self.data.qpos = [0, 0, 0.1*np.sqrt(2) - 0.001, np.cos(np.pi/8), np.sin(np.pi/8), 0, 0, 0, @@ -856,9 +843,6 @@ Return the current version of MuJoCo as a null-terminated string. Euler integrator, semi-implicit in velocity. """) - def test_int_constant(self): - self.assertEqual(mujoco.mjMAXVFSNAME, 1000) - def test_float_constant(self): self.assertEqual(mujoco.mjMAXVAL, 1e10) self.assertEqual(mujoco.mjMINVAL, 1e-15) diff --git a/python/mujoco/constants.cc b/python/mujoco/constants.cc index b8327c75..0f0f9ff6 100644 --- a/python/mujoco/constants.cc +++ b/python/mujoco/constants.cc @@ -57,8 +57,6 @@ PYBIND11_MODULE(_constants, pymodule) { X(mjMINIMP); X(mjMAXIMP); X(mjMAXCONPAIR); - X(mjMAXVFS); - X(mjMAXVFSNAME); X(mjNEQDATA); X(mjNDYN); X(mjNGAIN); diff --git a/python/mujoco/structs.cc b/python/mujoco/structs.cc index a0a81a11..c70e28cc 100644 --- a/python/mujoco/structs.cc +++ b/python/mujoco/structs.cc @@ -317,24 +317,22 @@ static raw::MjModel* LoadModelFileImpl( const std::string& filename, const std::vector& assets, LoadFunc&& loadfunc) { - std::unique_ptr vfs(nullptr, [](mjVFS*){}); + mjVFS vfs; + mjVFS* vfs_ptr = nullptr; if (!assets.empty()) { - // mjVFS should be allocated on the heap, because it's ~2MB - vfs = decltype(vfs)(new mjVFS, [](mjVFS* vfs) { - mj_deleteVFS(vfs); - delete vfs; - }); - mj_defaultVFS(vfs.get()); + mj_defaultVFS(&vfs); + vfs_ptr = &vfs; for (const auto& asset : assets) { const int vfs_error = InterceptMjErrors(mj_addBufferVFS)( - vfs.get(), asset.name, asset.content, asset.content_size); + vfs_ptr, asset.name, asset.content, asset.content_size); if (vfs_error) { throw py::value_error("assets dict is too big"); } } } - raw::MjModel* model = loadfunc(filename.c_str(), vfs.get()); + raw::MjModel* model = loadfunc(filename.c_str(), vfs_ptr); + mj_deleteVFS(vfs_ptr); if (model && !model->buffer) { mj_deleteModel(model); model = nullptr; @@ -351,12 +349,6 @@ ConvertAssetsDict( std::vector out; if (assets.has_value()) { for (const auto& [name, content] : *assets) { - if (name.length() >= mjMAXVFSNAME) { - std::ostringstream error; - error << "Filename length " << name.length() << " exceeds " - << mjMAXVFSNAME - 1 << " character limit: " << name; - throw py::value_error(error.str()); - } out.emplace_back(name.c_str(), PYBIND11_BYTES_AS_STRING(content.ptr()), py::len(content)); } diff --git a/src/engine/engine_util_misc.c b/src/engine/engine_util_misc.c index 3564e9b1..7f716b39 100644 --- a/src/engine/engine_util_misc.c +++ b/src/engine/engine_util_misc.c @@ -1407,51 +1407,6 @@ char* mju_strncpy(char *dst, const char *src, int n) { -// assemble full filename from directory and filename, return 0 on success -int mju_makefullname(char* full, size_t nfull, const char* dir, const char* file) { - int dirlen = (!dir) ? 0 : strlen(dir); - int filelen = (!file) ? 0 : strlen(file); - char* filepos = full + dirlen; - - // missing filename - if (!filelen) { - return -1; - } - - // no directory then just copy filename over - if (!dirlen) { - // make sure full has space - if (filelen >= nfull) { - return -1; - } - strcpy(full, file); - return 0; - } - - // make sure full has space - if (dirlen + filelen >= nfull) { - return -1; - } - - // dir doesn't end with a slash - if (dir[dirlen - 1] != '\\' && dir[dirlen - 1] != '/') { - // need extra space for forward slash - if ((dirlen + filelen + 1) >= nfull) { - return -1; - } - - // add forward slash - *filepos++ = '/'; - } - - // copy directory and file over - memcpy(full, dir, sizeof(char) * dirlen); - strcpy(filepos, file); - return 0; -} - - - // sigmoid function over 0<=x<=1 using quintic polynomial mjtNum mju_sigmoid(mjtNum x) { // fast return diff --git a/src/engine/engine_util_misc.h b/src/engine/engine_util_misc.h index 21bc3f83..117d2325 100644 --- a/src/engine/engine_util_misc.h +++ b/src/engine/engine_util_misc.h @@ -164,10 +164,6 @@ MJAPI mjtNum mju_Halton(int index, int base); // call strncpy, then set dst[n-1] = 0 MJAPI char* mju_strncpy(char *dst, const char *src, int n); -// assemble full filename from directory and filename, return 0 on success -MJAPI int mju_makefullname(char* full, size_t nfull, - const char* dir, const char* file); - // sigmoid function over 0<=x<=1 using quintic polynomial MJAPI mjtNum mju_sigmoid(mjtNum x); diff --git a/src/user/CMakeLists.txt b/src/user/CMakeLists.txt index c5d02994..c73b1133 100644 --- a/src/user/CMakeLists.txt +++ b/src/user/CMakeLists.txt @@ -29,7 +29,7 @@ set(MUJOCO_USER_SRCS user_objects.h user_util.cc user_util.h - user_vfs.c + user_vfs.cc user_vfs.h ) diff --git a/src/user/user_vfs.c b/src/user/user_vfs.c deleted file mode 100644 index c4448bf4..00000000 --- a/src/user/user_vfs.c +++ /dev/null @@ -1,420 +0,0 @@ -// Copyright 2021 DeepMind Technologies Limited -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "user/user_vfs.h" - -#include -#include -#include -#include - -#include "engine/engine_array_safety.h" -#include "engine/engine_resource.h" -#include "engine/engine_util_errmem.h" -#include "engine/engine_util_misc.h" - -// strip path prefix from filename -static void vfs_strippath(char* newname, const char* oldname) { - int sz = strlen(oldname); - - // find last delimiter - int i; - for (i=sz-1; i >= 0; i--) { - if (oldname[i] == '\\' || oldname[i] == '/') { - break; - } - } - - // check resulting length - if (sz-(i+1) >= mjMAXVFSNAME) { - mjERROR("filename too long"); - } - if (sz-(i+1) <= 0) { - mjERROR("empty filename"); - } - - // copy - mju_strncpy(newname, oldname+i+1, mjMAXVFSNAME); - - // make lowercase - for (int j=strlen(newname)-1; j >= 0; j--) { - if (newname[j] >= 'A' && newname[j] <= 'Z') { - newname[j] = (char)(((int)newname[j]) +'a' - 'A'); - } - } -} - - - -// copies data into a buffer and produces a hash of the data -static uint64_t vfs_memcpy(void* dest, const void* restrict src, size_t n) { - uint64_t hash = 0xcbf29ce484222325; // magic number - uint64_t prime = 0x100000001b3; // magic prime - const uint8_t* bytes = (uint8_t*) src; - uint8_t* buffer = (uint8_t*) dest; - for (size_t i = 0; i < n; i++) { - buffer[i] = bytes[i]; - - // do FNV-1 hash - hash |= bytes[i]; - hash *= prime; - } - return hash; -} - - - -// VFS hash function implemented using the FNV-1 hash -static uint64_t vfs_hash(const void* restrict buffer, size_t n) { - uint64_t hash = 0xcbf29ce484222325; // magic number - uint64_t prime = 0x100000001b3; // magic prime - const uint8_t* bytes = (uint8_t*) buffer; - for (size_t i = 0; i < n; i++) { - hash |= bytes[i]; - hash *= prime; - } - return hash; -} - - - -// initialize to empty (no deallocation) -void mj_defaultVFS(mjVFS* vfs) { - memset(vfs, 0, sizeof(mjVFS)); -} - - - -// add file to VFS, return 0: success, 1: full, 2: repeated name, -1: failed to load -int mj_addFileVFS(mjVFS* vfs, const char* directory, const char* filename) { - // check vfs size - if (vfs->nfile >= mjMAXVFS-1) { - return 1; - } - - // make full name - char fullname[1000]; - if (mju_makefullname(fullname, sizeof(fullname), directory, filename)) { - return -1; - } - - // strip path - char newname[mjMAXVFSNAME]; - vfs_strippath(newname, filename); - - // check for repeated name - for (int i=0; i < vfs->nfile; i++) { - if (strncmp(newname, vfs->filename[i], mjMAXVFSNAME) == 0) { - return 2; - } - } - - // assign name - mjSTRNCPY(vfs->filename[vfs->nfile], newname); - - // allocate and read - size_t filesize = 0; - vfs->filedata[vfs->nfile] = mju_fileToMemory(fullname, &filesize); - if (!vfs->filedata[vfs->nfile]) { - return -1; - } - - // assign size, count, and checksum - vfs->filestamp[vfs->nfile] = vfs_hash(vfs->filedata[vfs->nfile], filesize); - vfs->filesize[vfs->nfile] = filesize; - vfs->nfile++; - - return 0; -} - - - -// make empty file in VFS, return 0: success, 1: full, 2: repeated name -int mj_makeEmptyFileVFS(mjVFS* vfs, const char* filename, int filesize) { - // check vfs size - if (vfs->nfile >= mjMAXVFS-1) { - return 1; - } - - // check filesize - if (filesize <= 0) { - mjERROR("expects positive filesize"); - } - - // strip path - char newname[mjMAXVFSNAME]; - vfs_strippath(newname, filename); - - // check for repeated name - for (int i=0; i < vfs->nfile; i++) { - if (strncmp(newname, vfs->filename[i], mjMAXVFSNAME) == 0) { - return 2; - } - } - - // assign name - mjSTRNCPY(vfs->filename[vfs->nfile], newname); - - // allocate and clear - vfs->filedata[vfs->nfile] = mju_malloc(filesize); - if (!vfs->filedata[vfs->nfile]) { - mjERROR("could not allocate memory"); - } - memset(vfs->filedata[vfs->nfile], 0, filesize); - vfs->filestamp[vfs->nfile] = 0; - - // assign size and count - vfs->filesize[vfs->nfile] = filesize; - vfs->nfile++; - - return 0; -} - - - -// add file from buffer into VFS -int mj_addBufferVFS(mjVFS* vfs, const char* name, const void* buffer, int nbuffer) { - if (!vfs || !buffer || !name) { - mjERROR("null pointer"); - } - - if (vfs->nfile >= mjMAXVFS-1) { - return 1; - } - - // check buffer size - if (nbuffer <= 0) { - mjERROR("expects positive buffer size"); - } - - // strip path - char newname[mjMAXVFSNAME]; - vfs_strippath(newname, name); - - // check for repeated name - for (int i=0; i < vfs->nfile; i++) { - if (strncmp(newname, vfs->filename[i], mjMAXVFSNAME) == 0) { - return 2; - } - } - - // assign name - mjSTRNCPY(vfs->filename[vfs->nfile], newname); - - // allocate and clear - vfs->filedata[vfs->nfile] = mju_malloc(nbuffer); - if (!vfs->filedata[vfs->nfile]) { - mjERROR("could not allocate memory"); - } - vfs->filestamp[vfs->nfile] = vfs_memcpy(vfs->filedata[vfs->nfile], buffer, nbuffer); - - // assign size and count - vfs->filesize[vfs->nfile] = nbuffer; - vfs->nfile++; - - return 0; -} - - - -// return file index in VFS, or -1 if not found in VFS -int mj_findFileVFS(const mjVFS* vfs, const char* filename) { - // strip path - char newname[mjMAXVFSNAME]; - vfs_strippath(newname, filename); - // find specific file - for (int i=0; i < vfs->nfile; i++) { - if (strncmp(newname, vfs->filename[i], mjMAXVFSNAME) == 0) { - return i; - } - } - - return -1; -} - - - -// delete file from VFS, return 0: success, -1: not found in VFS -int mj_deleteFileVFS(mjVFS* vfs, const char* filename) { - // strip path - char newname[mjMAXVFSNAME]; - vfs_strippath(newname, filename); - - // find specified file - for (int i=0; i < vfs->nfile; i++) { - if (strncmp(newname, vfs->filename[i], mjMAXVFSNAME) == 0) { - // free buffer - mju_free(vfs->filedata[i]); - - // scroll remaining files forward - for (int j=i; j < vfs->nfile-1; j++) { - mjSTRNCPY(vfs->filename[j], vfs->filename[j+1]); - vfs->filesize[j] = vfs->filesize[j+1]; - vfs->filedata[j] = vfs->filedata[j+1]; - } - - // set last to 0, for style - vfs->filename[vfs->nfile-1][0] = 0; - vfs->filesize[vfs->nfile-1] = 0; - vfs->filedata[vfs->nfile-1] = NULL; - - // decrease counter - vfs->nfile--; - return 0; - } - } - - return -1; -} - - - -// delete all files from VFS -void mj_deleteVFS(mjVFS* vfs) { - for (int i=0; i < vfs->nfile; i++) { - mju_free(vfs->filedata[i]); - } - - memset(vfs, 0, sizeof(mjVFS)); -} - - - -// open callback for the VFS resource provider -static int vfs_open_callback(mjResource* resource) { - if (!resource || !resource->name || !resource->data) { - return 0; - } - - const mjVFS* vfs = (const mjVFS*) resource->data; - int i = mj_findFileVFS(vfs, resource->name); - resource->timestamp[0] = '\0'; - if (i >= 0 && vfs->filestamp[i]) { - mju_encodeBase64(resource->timestamp, (uint8_t*) &vfs->filestamp[i], - sizeof(uint64_t)); - } - return i >= 0; -} - - - -// read callback for the VFS resource provider -static int vfs_read_callback(mjResource* resource, const void** buffer) { - if (!resource || !resource->name || !resource->data) { - *buffer = NULL; - return -1; - } - - const mjVFS* vfs = (const mjVFS*) resource->data; - int i = mj_findFileVFS(vfs, resource->name); - if (i < 0) { - *buffer = NULL; - return -1; - } - - *buffer = vfs->filedata[i]; - return vfs->filesize[i]; -} - - - -// close callback for the VFS resource provider -static void vfs_close_callback(mjResource* resource) { -} - - - -// getdir callback for the VFS resource provider -static void vfs_getdir_callback(mjResource* resource, const char** dir, int* ndir) { - if (resource) { - *dir = resource->name; - *ndir = mju_dirnamelen(resource->name); - } else { - *dir = NULL; - *ndir = 0; - } -} - - -// modified callback for the VFS resource provider -// return > 0 if modified and 0 if unmodified -static int vfs_modified_callback(const mjResource* resource, const char* timestamp) { - uint64_t filestamp; - if (mju_isValidBase64(timestamp) > sizeof(uint64_t)) { - return 2; // error (assume modified) - } - - mju_decodeBase64((uint8_t*) &filestamp, timestamp); - if (!filestamp) return 3; // no hash (assume modified) - - if (resource) { - const mjVFS* vfs = (const mjVFS*) resource->data; - int i = mj_findFileVFS(vfs, resource->name); - if (i < 0) return 4; // missing file (assume modified) - if (!vfs->filestamp[i]) return 5; // missing filestamp (assume modified) - - if (vfs->filestamp[i] == filestamp) { - return 0; // unmodified - } - } - return 1; // modified -} - -// open VFS resource -mjResource* mju_openVfsResource(const char* name, const mjVFS* vfs) { - if (vfs == NULL) { - return NULL; - } - - // VFS provider - static struct mjpResourceProvider provider = { - .prefix = NULL, - .data = NULL, - .open = &vfs_open_callback, - .read = &vfs_read_callback, - .close = &vfs_close_callback, - .getdir = &vfs_getdir_callback, - .modified = &vfs_modified_callback, - }; - - // create resource - mjResource* resource = (mjResource*) mju_malloc(sizeof(mjResource)); - if (resource == NULL) { - mjERROR("could not allocate memory"); - return NULL; - } - - // clear out resource - memset(resource, 0, sizeof(mjResource)); - - // copy name - resource->name = mju_malloc(sizeof(char) * (strlen(name) + 1)); - if (resource->name == NULL) { - mju_closeResource(resource); - mjERROR("could not allocate memory"); - return NULL; - } - memcpy(resource->name, name, sizeof(char) * (strlen(name) + 1)); - resource->data = (void*) vfs; - - // open resource - resource->provider = &provider; - if (provider.open(resource)) { - return resource; - } - - // not found in VFS - mju_closeResource(resource); - return NULL; -} diff --git a/src/user/user_vfs.cc b/src/user/user_vfs.cc new file mode 100644 index 00000000..f2b05eb0 --- /dev/null +++ b/src/user/user_vfs.cc @@ -0,0 +1,322 @@ +// Copyright 2021 DeepMind Technologies Limited +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "user/user_vfs.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "engine/engine_resource.h" +#include "engine/engine_util_errmem.h" +#include "engine/engine_util_misc.h" +#include "user/user_util.h" + +namespace { + +// internal struct for VFS files +struct VFSFile { + std::string filename; + std::unique_ptr> filedata; + std::size_t filesize; + uint64_t filestamp; +}; + +// internal container class for VFS +class VFS { + public: + // returns true if the file exists in the VFS + bool HasFile(const std::string& filename) const; + + // returns inserted mjuuVFSFile if the file was added successfully. This class + // assumes ownership of the buffer and will free it when the VFS is deleted. + VFSFile* AddFile(const std::string& filename, void* buffer, + std::size_t nbuffer, uint64_t filestamp); + + // returns the internal file struct for the given filename + const VFSFile* GetFile(const std::string& filename) const; + + // deletes file from VFS, return 0: success, -1: not found + int DeleteFile(const std::string& filename); + + private: + std::unordered_map files_; +}; + +// returns the internal VFS class pointer from the VFS C struct +inline VFS* GetVFSImpl(const mjVFS* vfs) { + return vfs->impl_ ? static_cast(vfs->impl_) : nullptr; +} + +// strip path prefix from filename and make lowercase +std::string StripPath(const char* name) { + std::string newname = mjuu_strippath(name); + + // make lowercase + std::transform(newname.begin(), newname.end(), newname.begin(), + [](unsigned char c) { return std::tolower(c); }); + return newname; +} + +// copies data into a buffer and produces a hash of the data +uint64_t vfs_memcpy(void* dest, const void* src, size_t n) { + uint64_t hash = 0xcbf29ce484222325; // magic number + uint64_t prime = 0x100000001b3; // magic prime + const uint8_t* bytes = (uint8_t*) src; + uint8_t* buffer = (uint8_t*) dest; + for (size_t i = 0; i < n; i++) { + buffer[i] = bytes[i]; + + // do FNV-1 hash + hash |= bytes[i]; + hash *= prime; + } + return hash; +} + +// VFS hash function implemented using the FNV-1 hash +uint64_t vfs_hash(const void* buffer, size_t n) { + uint64_t hash = 0xcbf29ce484222325; // magic number + uint64_t prime = 0x100000001b3; // magic prime + const uint8_t* bytes = (uint8_t*) buffer; + for (size_t i = 0; i < n; i++) { + hash |= bytes[i]; + hash *= prime; + } + return hash; +} + +bool VFS::HasFile(const std::string& filename) const { + return files_.find(filename) != files_.end(); +} + +VFSFile* VFS::AddFile(const std::string& filename, void* buffer, + std::size_t nbuffer, uint64_t filestamp) { + auto [it, inserted] = files_.insert({filename, VFSFile()}); + if (!inserted) { + return nullptr; // repeated name + } + it->second.filename = filename; + it->second.filedata = std::unique_ptr( + buffer, [](void* b) { mju_free(b); }); // corresponding to mju_malloc + it->second.filesize = nbuffer; + it->second.filestamp = filestamp; + return &(it->second); +} + +const VFSFile* VFS::GetFile(const std::string& filename) const { + auto it = files_.find(filename); + if (it == files_.end()) { + return nullptr; + } + return &it->second; +} + +int VFS::DeleteFile(const std::string& filename) { + auto it = files_.find(filename); + if (it == files_.end()) { + return -1; + } + files_.erase(it); + return 0; +} + +// open callback for the VFS resource provider +int Open(mjResource* resource) { + if (!resource || !resource->name || !resource->data) { + return 0; + } + + const mjVFS* vfs = (const mjVFS*) resource->data; + const VFS* cvfs = GetVFSImpl(vfs); + const VFSFile* file = cvfs->GetFile(StripPath(resource->name)); + if (file == nullptr) { + return 0; + } + + resource->timestamp[0] = '\0'; + if (file->filestamp) { + mju_encodeBase64(resource->timestamp, (uint8_t*) &file->filestamp, + sizeof(uint64_t)); + } + return 1; +} + +// read callback for the VFS resource provider +int Read(mjResource* resource, const void** buffer) { + if (!resource || !resource->name || !resource->data) { + *buffer = nullptr; + return -1; + } + + const VFS* vfs = GetVFSImpl(static_cast(resource->data)); + const VFSFile* file = vfs->GetFile(StripPath(resource->name)); + if (file == nullptr) { + *buffer = nullptr; + return -1; + } + + *buffer = file->filedata.get(); + return file->filesize; +} + +// close callback for the VFS resource provider +void Close(mjResource* resource) { +} + +// getdir callback for the VFS resource provider +void GetDir(mjResource* resource, const char** dir, int* ndir) { + *dir = (resource) ? resource->name : nullptr; + *ndir = (resource) ? mju_dirnamelen(resource->name) : 0; +} + +// modified callback for the VFS resource provider +// return > 0 if modified and 0 if unmodified +int Modified(const mjResource* resource, const char* timestamp) { + uint64_t filestamp; + if (mju_isValidBase64(timestamp) > sizeof(uint64_t)) { + return 2; // error (assume modified) + } + + mju_decodeBase64((uint8_t*) &filestamp, timestamp); + if (!filestamp) return 3; // no hash (assume modified) + + if (resource) { + const VFS* cvfs = GetVFSImpl(static_cast(resource->data)); + const VFSFile* file = cvfs->GetFile(StripPath(resource->name)); + if (file == nullptr) return 4; // missing file (assume modified) + if (!file->filestamp) return 5; // missing filestamp (assume modified) + + if (file->filestamp == filestamp) { + return 0; // unmodified + } + } + return 1; // modified +} + +} // namespace + +// initialize to empty (no deallocation) +void mj_defaultVFS(mjVFS* vfs) { + vfs->impl_ = new VFS(); +} + +// add file to VFS, return 0: success, 2: repeated name, -1: failed to load +int mj_addFileVFS(mjVFS* vfs, const char* directory, const char* filename) { + VFS* cvfs = GetVFSImpl(vfs); + + // make full name + std::string fullname = mjuu_combinePaths(directory, filename); + + // strip path + std::string newname = StripPath(filename); + + // check beforehand for repeated name, to avoid reading file into memory + if (cvfs->HasFile(newname)) { + return 2; + } + + // allocate and read + size_t nbuffer = 0; + void* buffer = mju_fileToMemory(fullname.c_str(), &nbuffer); + if (buffer == nullptr) { + return -1; + } + + if (!cvfs->AddFile(newname, buffer, nbuffer, vfs_hash(buffer, nbuffer))) { + mju_free(buffer); + return 2; // AddFile failed, SHOULD NOT OCCUR + } + return 0; +} + +// add file from buffer into VFS +int mj_addBufferVFS(mjVFS* vfs, const char* name, const void* buffer, + int nbuffer) { + VFS* cvfs = GetVFSImpl(vfs); + + // allocate and clear + void* inbuffer = mju_malloc(nbuffer); + if (buffer == nullptr) { + mjERROR("could not allocate memory"); + } + VFSFile* file; + if (!(file = cvfs->AddFile(StripPath(name), inbuffer, nbuffer, 0))) { + mju_free(inbuffer); + return 2; // AddFile failed, repeated name + } + file->filestamp = vfs_memcpy(inbuffer, buffer, nbuffer); + return 0; +} + +// delete file from VFS, return 0: success, -1: not found in VFS +int mj_deleteFileVFS(mjVFS* vfs, const char* filename) { + VFS* cvfs = GetVFSImpl(vfs); + return cvfs->DeleteFile(StripPath(filename)); +} + +// delete all files from VFS +void mj_deleteVFS(mjVFS* vfs) { + if (vfs) { + delete GetVFSImpl(vfs); + } +} + +// open VFS resource +mjResource* mju_openVfsResource(const char* name, const mjVFS* vfs) { + if (vfs == nullptr) { + return nullptr; + } + + // VFS provider + static struct mjpResourceProvider provider = { nullptr, &Open, &Read, &Close, + &GetDir, &Modified, nullptr }; + + // create resource + mjResource* resource = (mjResource*) mju_malloc(sizeof(mjResource)); + if (resource == nullptr) { + mjERROR("could not allocate memory"); + return nullptr; + } + + // clear out resource + memset(resource, 0, sizeof(mjResource)); + + // copy name + std::size_t n = std::strlen(name); + resource->name = (char*) mju_malloc(sizeof(char) * (n + 1)); + if (resource->name == nullptr) { + mju_closeResource(resource); + mjERROR("could not allocate memory"); + return nullptr; + } + std::memcpy(resource->name, name, sizeof(char) * (n + 1)); + resource->data = (void*) vfs; + + // open resource + resource->provider = &provider; + if (provider.open(resource)) { + return resource; + } + + // not found in VFS + mju_closeResource(resource); + return nullptr; +} diff --git a/src/user/user_vfs.h b/src/user/user_vfs.h index 8896ac6d..202136fd 100644 --- a/src/user/user_vfs.h +++ b/src/user/user_vfs.h @@ -30,13 +30,13 @@ extern "C" { // Initialize an empty VFS, mj_deleteVFS must be called to deallocate the VFS MJAPI void mj_defaultVFS(mjVFS* vfs); -// add file to VFS, return 0: success, 1: full, 2: repeated name, -1: not found on disk +// add file to VFS, return 0: success, 2: repeated name, -1: not found on disk MJAPI int mj_addFileVFS(mjVFS* vfs, const char* directory, const char* filename); // deprecated: use mj_addBufferVFS MJAPI int mj_makeEmptyFileVFS(mjVFS* vfs, const char* filename, int filesize); -// add file from buffer into VFS, return 0: success, 1: full, 2: repeated name, -1: failed to load +// add file from buffer into VFS, return 0: success, 2: repeated name, -1: failed to load MJAPI int mj_addBufferVFS(mjVFS* vfs, const char* filename, const void* buffer, int nbuffer); // return file index in VFS, or -1 if not found in VFS diff --git a/test/engine/engine_util_misc_test.cc b/test/engine/engine_util_misc_test.cc index 63632349..aca400a5 100644 --- a/test/engine/engine_util_misc_test.cc +++ b/test/engine/engine_util_misc_test.cc @@ -148,92 +148,6 @@ TEST_F(MujocoTest, MuscleGainLength) { EXPECT_EQ(mju_muscleGainLength(2.0, lmin, lmax), 0); } -TEST_F(MujocoTest, mju_makefullname) { - char buffer[1000]; - constexpr char path[] = "engine/testdata/"; - constexpr char file[] = "file"; - int n = mju_makefullname(buffer, sizeof(buffer), path, file); - ASSERT_THAT(buffer, StrEq("engine/testdata/file")); - EXPECT_THAT(n, 0); -} - -TEST_F(MujocoTest, mju_makefullname2) { - char buffer[1000]; - constexpr char path[] = "engine\\testdata\\"; - constexpr char file[] = "file"; - int n = mju_makefullname(buffer, sizeof(buffer), path, file); - ASSERT_THAT(buffer, StrEq("engine\\testdata\\file")); - EXPECT_THAT(n, 0); -} - - -TEST_F(MujocoTest, mju_makefullname_missingSlash) { - char buffer[1000]; - constexpr char path[] = "engine/testdata"; - constexpr char file[] = "file"; - int n = mju_makefullname(buffer, sizeof(buffer), path, file); - ASSERT_THAT(buffer, StrEq("engine/testdata/file")); - EXPECT_THAT(n, 0); -} - -TEST_F(MujocoTest, mju_makefullname_withoutDir) { - char buffer[1000]; - constexpr char *path = NULL; - constexpr char file[] = "file"; - int n = mju_makefullname(buffer, sizeof(buffer), path, file); - ASSERT_THAT(buffer, StrEq("file")); - EXPECT_THAT(n, 0); -} - -TEST_F(MujocoTest, mju_makefullname_withoutDir2) { - char buffer[1000]; - constexpr char path[] = ""; - constexpr char file[] = "file"; - int n = mju_makefullname(buffer, sizeof(buffer), path, file); - ASSERT_THAT(buffer, StrEq("file")); - EXPECT_THAT(n, 0); -} - -TEST_F(MujocoTest, mju_makefullname_error) { - char buffer[1000]; - constexpr char path[] = "engine/testdata"; - constexpr char *file = NULL; - int n = mju_makefullname(buffer, sizeof(buffer), path, file); - EXPECT_THAT(n, Ne(0)); -} - -TEST_F(MujocoTest, mju_makefullname_error2) { - char buffer[1000]; - constexpr char path[] = "engine/testdata"; - constexpr char file[] = ""; - int n = mju_makefullname(buffer, sizeof(buffer), path, file); - EXPECT_THAT(n, Ne(0)); -} - -TEST_F(MujocoTest, mju_makefullname_error3) { - char buffer[20]; - constexpr char path[] = "engine/testdata/"; - constexpr char file[] = "file"; - int n = mju_makefullname(buffer, sizeof(buffer), path, file); - EXPECT_THAT(n, Ne(0)); -} - -TEST_F(MujocoTest, mju_makefullname_error4) { - char buffer[20]; - constexpr char path[] = "engine/testdata"; - constexpr char file[] = "file"; - int n = mju_makefullname(buffer, sizeof(buffer), path, file); - EXPECT_THAT(n, Ne(0)); -} - -TEST_F(MujocoTest, mju_makefullname_error5) { - char buffer[4]; - constexpr char path[] = ""; - constexpr char file[] = "file"; - int n = mju_makefullname(buffer, sizeof(buffer), path, file); - EXPECT_THAT(n, Ne(0)); -} - // --------------------------------- Base64 ------------------------------------ using Base64Test = MujocoTest; diff --git a/test/user/user_vfs_test.cc b/test/user/user_vfs_test.cc index cc3ce120..269accea 100644 --- a/test/user/user_vfs_test.cc +++ b/test/user/user_vfs_test.cc @@ -14,7 +14,6 @@ #include #include -#include #include #include @@ -28,9 +27,16 @@ namespace mujoco { namespace { using ::testing::NotNull; -using EngineVfsTest = MujocoTest; +using UserVfsTest = MujocoTest; -TEST_F(EngineVfsTest, AddFile) { +static bool HasFile(const mjVFS* vfs, const std::string& filename) { + mjResource* resource = mju_openVfsResource(filename.c_str(), vfs); + bool result = resource != nullptr; + mju_closeResource(resource); + return result; +} + +TEST_F(UserVfsTest, AddFile) { constexpr char path[] = "engine/testdata/actuation/"; const std::string dir = GetTestDataFilePath(path); std::string file1 = "activation.xml"; @@ -49,54 +55,159 @@ TEST_F(EngineVfsTest, AddFile) { ASSERT_THAT(fp3, NotNull()) << "Input file3 missing."; std::fclose(fp3); - auto vfs = std::make_unique(); - mj_defaultVFS(vfs.get()); + mjVFS vfs; + mj_defaultVFS(&vfs); + EXPECT_FALSE(HasFile(&vfs, file1)); + EXPECT_THAT(mj_addFileVFS(&vfs, dir.c_str(), file1.c_str()), 0); + EXPECT_TRUE(HasFile(&vfs, file1)); - EXPECT_THAT(vfs->nfile, 0); - EXPECT_THAT(mj_addFileVFS(vfs.get(), dir.c_str(), file1.c_str()), 0); - EXPECT_THAT(vfs->nfile, 1); - EXPECT_THAT(vfs->filename[0], file1); + EXPECT_THAT(mj_addFileVFS(&vfs, dir.c_str(), file2.c_str()), 0); + EXPECT_TRUE(HasFile(&vfs, file1)); + EXPECT_TRUE(HasFile(&vfs, file2)); - EXPECT_THAT(mj_addFileVFS(vfs.get(), dir.c_str(), file2.c_str()), 0); - EXPECT_THAT(vfs->nfile, 2); - EXPECT_THAT(vfs->filename[0], file1); - EXPECT_THAT(vfs->filename[1], file2); + EXPECT_THAT(mj_addFileVFS(&vfs, dir.c_str(), file3.c_str()), 0); + EXPECT_TRUE(HasFile(&vfs, file1.c_str())); + EXPECT_TRUE(HasFile(&vfs, file2.c_str())); + EXPECT_TRUE(HasFile(&vfs, file3.c_str())); - EXPECT_THAT(mj_addFileVFS(vfs.get(), dir.c_str(), file3.c_str()), 0); - EXPECT_THAT(vfs->nfile, 3); - EXPECT_THAT(vfs->filename[0], file1); - EXPECT_THAT(vfs->filename[1], file2); - EXPECT_THAT(vfs->filename[2], file3); + mj_deleteFileVFS(&vfs, file1.c_str()); + EXPECT_FALSE(HasFile(&vfs, file1.c_str())); + EXPECT_TRUE(HasFile(&vfs, file2.c_str())); + EXPECT_TRUE(HasFile(&vfs, file3.c_str())); - mj_deleteFileVFS(vfs.get(), file1.c_str()); - EXPECT_THAT(vfs->nfile, 2); - EXPECT_THAT(vfs->filename[0], file2); - EXPECT_THAT(vfs->filename[1], file3); - mj_deleteFileVFS(vfs.get(), file3.c_str()); - EXPECT_THAT(vfs->nfile, 1); - EXPECT_THAT(vfs->filename[0], file2); + mj_deleteFileVFS(&vfs, file3.c_str()); + EXPECT_FALSE(HasFile(&vfs, file1.c_str())); + EXPECT_TRUE(HasFile(&vfs, file2.c_str())); + EXPECT_FALSE(HasFile(&vfs, file3.c_str())); - mj_deleteFileVFS(vfs.get(), file2.c_str()); - EXPECT_THAT(vfs->nfile, 0); + mj_deleteFileVFS(&vfs, file2.c_str()); + EXPECT_FALSE(HasFile(&vfs, file1.c_str())); + EXPECT_FALSE(HasFile(&vfs, file2.c_str())); + EXPECT_FALSE(HasFile(&vfs, file3.c_str())); - mj_deleteVFS(vfs.get()); + mj_deleteVFS(&vfs); } -TEST_F(EngineVfsTest, AddBuffer) { - auto vfs = std::make_unique(); - mj_defaultVFS(vfs.get()); +TEST_F(UserVfsTest, AddFileStripPath) { + mjVFS vfs; + mj_defaultVFS(&vfs); + + constexpr char path[] = "engine/testdata/actuation/"; + const std::string dir = GetTestDataFilePath(path); + std::string file1 = "activation.xml"; + mj_addFileVFS(&vfs, dir.c_str(), file1.c_str()); + + EXPECT_TRUE(HasFile(&vfs, file1)); + EXPECT_TRUE(HasFile(&vfs, dir + file1)); + EXPECT_TRUE(HasFile(&vfs, "some/dir/" + file1)); + EXPECT_TRUE(HasFile(&vfs, "some/dir\\" + file1)); + + mj_deleteVFS(&vfs); +} + +TEST_F(UserVfsTest, AddFileRepeat) { + mjVFS vfs; + mj_defaultVFS(&vfs); + + constexpr char path[] = "engine/testdata/actuation/"; + const std::string dir = GetTestDataFilePath(path); + std::string file1 = "activation.xml"; + mj_addFileVFS(&vfs, dir.c_str(), file1.c_str()); + + EXPECT_TRUE(HasFile(&vfs, file1)); + EXPECT_THAT(mj_addFileVFS(&vfs, "dir/", file1.c_str()), 2); + + mj_deleteVFS(&vfs); +} + +TEST_F(UserVfsTest, DeleteFile) { + mjVFS vfs; + mj_defaultVFS(&vfs); + + constexpr char path[] = "engine/testdata/actuation/"; + const std::string dir = GetTestDataFilePath(path); + std::string file1 = "activation.xml"; + mj_addFileVFS(&vfs, dir.c_str(), file1.c_str()); + + EXPECT_TRUE(HasFile(&vfs, file1)); + EXPECT_THAT(mj_deleteFileVFS(&vfs, file1.c_str()), 0); + EXPECT_FALSE(HasFile(&vfs, file1)); + + mj_deleteVFS(&vfs); +} + +TEST_F(UserVfsTest, DeleteFileStripPath) { + mjVFS vfs; + mj_defaultVFS(&vfs); + + constexpr char path[] = "engine/testdata/actuation/"; + const std::string dir = GetTestDataFilePath(path); + std::string file = "activation.xml"; + std::string fileUpper = "Activation.xml"; + mj_addFileVFS(&vfs, dir.c_str(), file.c_str()); + + EXPECT_TRUE(HasFile(&vfs, file)); + EXPECT_THAT(mj_deleteFileVFS(&vfs, ("dir\\" + fileUpper).c_str()), 0); + EXPECT_FALSE(HasFile(&vfs, file)); + + mj_deleteVFS(&vfs); +} + +TEST_F(UserVfsTest, DeleteFileRepeat) { + mjVFS vfs; + mj_defaultVFS(&vfs); + + constexpr char path[] = "engine/testdata/actuation/"; + const std::string dir = GetTestDataFilePath(path); + std::string file = "activation.xml"; + mj_addFileVFS(&vfs, dir.c_str(), file.c_str()); + + EXPECT_TRUE(HasFile(&vfs, file)); + EXPECT_THAT(mj_deleteFileVFS(&vfs, file.c_str()), 0); + EXPECT_FALSE(HasFile(&vfs, file)); + EXPECT_THAT(mj_deleteFileVFS(&vfs, file.c_str()), -1); + + mj_deleteVFS(&vfs); +} + + +TEST_F(UserVfsTest, AddBuffer) { + mjVFS vfs; + mj_defaultVFS(&vfs); std::string buffer = ""; - mj_addBufferVFS(vfs.get(), "model", static_cast(buffer.c_str()), + mj_addBufferVFS(&vfs, "model", static_cast(buffer.c_str()), buffer.size()); std::array error; - mjModel* model = mj_loadXML("model", vfs.get(), error.data(), error.size()); + mjModel* model = mj_loadXML("model", &vfs, error.data(), error.size()); EXPECT_THAT(model, NotNull()); mj_deleteModel(model); - mj_deleteVFS(vfs.get()); + mj_deleteVFS(&vfs); } -TEST_F(EngineVfsTest, Timestamps) { +TEST_F(UserVfsTest, AddBufferRepeat) { + mjVFS vfs; + mj_defaultVFS(&vfs); + std::string buffer = ""; + const void* ptr = static_cast(buffer.c_str()); + mj_addBufferVFS(&vfs, "model", ptr, buffer.size()); + int result = mj_addBufferVFS(&vfs, "model", ptr, buffer.size()); + EXPECT_EQ(result, 2); + mj_deleteVFS(&vfs); +} + +TEST_F(UserVfsTest, BufferStripPath) { + mjVFS vfs; + mj_defaultVFS(&vfs); + std::string buffer = ""; + const void* ptr = static_cast(buffer.c_str()); + mj_addBufferVFS(&vfs, "dir/model", ptr, buffer.size()); + EXPECT_TRUE(HasFile(&vfs, "MODEL")); + EXPECT_TRUE(HasFile(&vfs, "dir\\model")); + mj_deleteVFS(&vfs); +} + +TEST_F(UserVfsTest, Timestamps) { static constexpr char cube[] = R"( v -0.500000 -0.500000 0.500000 v 0.500000 -0.500000 0.500000 @@ -107,11 +218,11 @@ TEST_F(EngineVfsTest, Timestamps) { v -0.500000 -0.500000 -0.500000 v 0.500000 -0.500000 -0.500000)"; - auto vfs = std::make_unique(); - mj_defaultVFS(vfs.get()); - mj_addBufferVFS(vfs.get(), "cube.obj", cube, sizeof(cube)); + mjVFS vfs; + mj_defaultVFS(&vfs); + mj_addBufferVFS(&vfs, "cube.obj", cube, sizeof(cube)); - mjResource* resource = mju_openVfsResource("cube.obj", vfs.get()); + mjResource* resource = mju_openVfsResource("cube.obj", &vfs); // same timestamps EXPECT_EQ(mju_isModifiedResource(resource, resource->timestamp), 0); @@ -120,7 +231,7 @@ TEST_F(EngineVfsTest, Timestamps) { EXPECT_EQ(mju_isModifiedResource(resource, "QQ=="), 1); mju_closeResource(resource); - mj_deleteVFS(vfs.get()); + mj_deleteVFS(&vfs); } } // namespace diff --git a/unity/Runtime/Bindings/MjBindings.cs b/unity/Runtime/Bindings/MjBindings.cs index 861f75e2..215834ba 100644 --- a/unity/Runtime/Bindings/MjBindings.cs +++ b/unity/Runtime/Bindings/MjBindings.cs @@ -41,8 +41,6 @@ public const double mjMINIMP = 0.0001; public const double mjMAXIMP = 0.9999; public const int mjMAXCONPAIR = 50; public const int mjMAXTREEDEPTH = 50; -public const int mjMAXVFS = 2000; -public const int mjMAXVFSNAME = 1000; public const int mjNEQDATA = 11; public const int mjNDYN = 10; public const int mjNGAIN = 10; @@ -4978,11 +4976,7 @@ public unsafe struct mjLROpt_ { [StructLayout(LayoutKind.Sequential)] public unsafe struct _mjVFS { - public int nfile; - [MarshalAs(UnmanagedType.ByValArray, SizeConst = 2000 * 1000)] public char[] filename; - [MarshalAs(UnmanagedType.ByValArray, SizeConst = 2000)] public UIntPtr[] filesize; - [MarshalAs(UnmanagedType.ByValArray, SizeConst = 2000)] public IntPtr[] filedata; - [MarshalAs(UnmanagedType.ByValArray, SizeConst = 2000)] public UInt64[] filestamp; + public void* impl_; } [StructLayout(LayoutKind.Sequential)] diff --git a/unity/Runtime/Tools/MjVfs.cs b/unity/Runtime/Tools/MjVfs.cs index 14174e6a..802d53f9 100644 --- a/unity/Runtime/Tools/MjVfs.cs +++ b/unity/Runtime/Tools/MjVfs.cs @@ -40,11 +40,6 @@ public sealed class MjVfs : IDisposable { } } - // Number of files added to the filesystem. - public int FilesCount { - get { return Data.nfile; } - } - // Adds a new file to the virtual filesystem. public unsafe void AddFile(string filename, string contents) { var contents_bytes = Encoding.UTF8.GetBytes(contents); diff --git a/unity/Tests/Editor/Tools/MjVFSEditorTests.cs b/unity/Tests/Editor/Tools/MjVFSEditorTests.cs index 12ab6562..238a494b 100644 --- a/unity/Tests/Editor/Tools/MjVFSEditorTests.cs +++ b/unity/Tests/Editor/Tools/MjVFSEditorTests.cs @@ -40,7 +40,7 @@ namespace Mujoco { var filename = "filename"; var contents = "contents"; _vfs.AddFile(filename, contents); - Assert.That(_vfs.FilesCount, Is.EqualTo(1)); + Assert.That(() => { _vfs.AddFile(filename, contents); }, Throws.Exception); // duplicate file } } }