From e66b9a36b2bd6d9d811cf663bf4a727fc0678b33 Mon Sep 17 00:00:00 2001 From: Kyle Bayes Date: Fri, 21 Jun 2024 08:11:48 -0700 Subject: [PATCH] Remove mj_makeEmptyFileVFS and mj_findFileVFS from MuJoCo API. PiperOrigin-RevId: 645393666 Change-Id: Iabe939c88f60b80dce20f84c7ff9a6396717cc84 --- doc/APIreference/functions.rst | 22 +-------- doc/changelog.rst | 36 +++++++++------ doc/includes/references.h | 2 - include/mujoco/mujoco.h | 10 +--- introspect/functions.py | 48 +------------------- src/user/user_vfs.h | 6 +-- unity/Runtime/Bindings/MjBindings.cs | 6 --- unity/Runtime/Tools/MjVfs.cs | 12 ----- unity/Tests/Editor/Tools/MjVFSEditorTests.cs | 7 --- 9 files changed, 32 insertions(+), 117 deletions(-) diff --git a/doc/APIreference/functions.rst b/doc/APIreference/functions.rst index c55c15ea..87a8d02b 100644 --- a/doc/APIreference/functions.rst +++ b/doc/APIreference/functions.rst @@ -1203,7 +1203,7 @@ mj_defaultVFS .. mujoco-include:: mj_defaultVFS -Initialize VFS to empty (no deallocation). +Initialize an empty VFS, mj_deleteVFS must be called to deallocate the VFS. .. _mj_addFileVFS: @@ -1224,15 +1224,6 @@ mj_addBufferVFS Add file to VFS from buffer, return 0: success, 1: full, 2: repeated name, -1: failed to load. -.. _mj_findFileVFS: - -mj_findFileVFS -~~~~~~~~~~~~~~ - -.. mujoco-include:: mj_findFileVFS - -Return file index in VFS, or -1 if not found in VFS. - .. _mj_deleteFileVFS: mj_deleteFileVFS @@ -1249,16 +1240,7 @@ mj_deleteVFS .. mujoco-include:: mj_deleteVFS -Delete all files from VFS. - -.. _mj_makeEmptyFileVFS: - -mj_makeEmptyFileVFS -~~~~~~~~~~~~~~~~~~~ - -.. mujoco-include:: mj_makeEmptyFileVFS - -deprecated: use mj_copyBufferVFS. +Delete all files from VFS and deallocates VFS internal memory. .. _Initialization: diff --git a/doc/changelog.rst b/doc/changelog.rst index 7aa15b76..7a4bfdd2 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -7,7 +7,17 @@ Upcoming version (not yet released) General ^^^^^^^ -1. Added a new API for :doc:`procedural model manipulation`. Fixes :github:issue:`364`. +.. admonition:: Breaking API changes + :class: attention + + 1. Removed deprecated ``mj_makeEmptyFileVFS`` and ``mj_findFileVFS`` functions. + + **Migration:** Use ``mj_addBufferVFS`` to copy a buffer into a VFS file directly. + + 2. Calls to ``mj_defaultVFS`` may allocate memory inside VFS, and the corresponding + ``mj_deleteVFS`` must be called to deallocate any internal allocated memory. + +3. Added a new API for :doc:`procedural model manipulation`. Fixes :github:issue:`364`. Still missing: - Detailed documentation. @@ -17,15 +27,15 @@ General :align: right :width: 240px -2. Added support for orthographic cameras. This is available for both fixed cameras and the free camera, using the +4. 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. -3. Added :ref:`maxhullvert`, the maximum number of vertices in a mesh's convex hull. -4. Added :ref:`mj_setKeyframe` for saving the current state into a model keyframe. -5. Added support for ``ball`` joints in the URDF parser ("spherical" in URDF). -6. Deprecated :ref:`mju_rotVecMat` and :ref:`mju_rotVecMatT` in favor of :ref:`mju_mulMatVec3` and +5. Added :ref:`maxhullvert`, the maximum number of vertices in a mesh's convex hull. +6. Added :ref:`mj_setKeyframe` for saving the current state into a model keyframe. +7. Added support for ``ball`` joints in the URDF parser ("spherical" in URDF). +8. Deprecated :ref:`mju_rotVecMat` and :ref:`mju_rotVecMatT` in favor of :ref:`mju_mulMatVec3` and :ref:`mju_mulMatTVec3`. These functions names and argument ordering are more consistent with the rest of the API. -7. Replaced ``mjUSEDOUBLE`` which was previously hard-coded in +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`. @@ -34,9 +44,9 @@ General MJX ~~~ -8. Added support for :ref:`elliptic friction cones`. -9. Fixed a bug that resulted in less-optimal linesearch solutions for some difficult constraint settings. -10. Fixed a bug in the Newton solver that sometimes resulted in less-optimal gradients. +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. .. youtube:: P83tKA1iz2Y @@ -45,14 +55,14 @@ MJX Simulate ^^^^^^^^ -11. Added improved tutorial video. -12. Improved the Brownian noise generator. +13. Added improved tutorial video. +14. Improved the Brownian noise generator. |br| |br| |br| |br| Python bindings ^^^^^^^^^^^^^^^ -13. Fixed a memory leak when using ``copy.deepcopy()`` on a ``mujoco.MjData`` instance (:github:issue:`1572`). +15. 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 2e3e3429..0f71cf53 100644 --- a/doc/includes/references.h +++ b/doc/includes/references.h @@ -3068,10 +3068,8 @@ typedef struct mjvSceneState_ mjvSceneState; void mj_defaultVFS(mjVFS* vfs); int mj_addFileVFS(mjVFS* vfs, const char* directory, const char* filename); int mj_addBufferVFS(mjVFS* vfs, const char* name, const void* buffer, int nbuffer); -int mj_findFileVFS(const mjVFS* vfs, const char* filename); int mj_deleteFileVFS(mjVFS* vfs, const char* filename); void mj_deleteVFS(mjVFS* vfs); -int mj_makeEmptyFileVFS(mjVFS* vfs, const char* filename, int filesize); mjModel* mj_loadXML(const char* filename, const mjVFS* vfs, char* error, int error_sz); mjSpec* mj_parseXML(const char* filename, const mjVFS* vfs, char* error, int error_sz); mjSpec* mj_parseXMLString(const char* xml, const mjVFS* vfs, char* error, int error_sz); diff --git a/include/mujoco/mujoco.h b/include/mujoco/mujoco.h index 1c85099f..6cb6f7e5 100644 --- a/include/mujoco/mujoco.h +++ b/include/mujoco/mujoco.h @@ -74,7 +74,7 @@ MJAPI extern const char* mjRNDSTRING[mjNRNDFLAG][3]; //---------------------------------- Virtual file system ------------------------------------------- -// Initialize VFS to empty (no deallocation). +// 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: failed to load. @@ -83,18 +83,12 @@ MJAPI int mj_addFileVFS(mjVFS* vfs, const char* directory, const char* filename) // Add file to VFS from buffer, return 0: success, 1: full, 2: repeated name, -1: failed to load. MJAPI int mj_addBufferVFS(mjVFS* vfs, const char* name, const void* buffer, int nbuffer); -// Return file index in VFS, or -1 if not found in VFS. -MJAPI int mj_findFileVFS(const mjVFS* vfs, const char* filename); - // Delete file from VFS, return 0: success, -1: not found in VFS. MJAPI int mj_deleteFileVFS(mjVFS* vfs, const char* filename); -// Delete all files from VFS. +// Delete all files from VFS and deallocates VFS internal memory. MJAPI void mj_deleteVFS(mjVFS* vfs); -// deprecated: use mj_copyBufferVFS. -MJAPI int mj_makeEmptyFileVFS(mjVFS* vfs, const char* filename, int filesize); - //---------------------------------- Parse and compile --------------------------------------------- diff --git a/introspect/functions.py b/introspect/functions.py index 9733755b..58985da5 100644 --- a/introspect/functions.py +++ b/introspect/functions.py @@ -38,7 +38,7 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([ ), ), ), - doc='Initialize VFS to empty (no deallocation).', + doc='Initialize an empty VFS, mj_deleteVFS must be called to deallocate the VFS.', # pylint: disable=line-too-long )), ('mj_addFileVFS', FunctionDecl( @@ -96,26 +96,6 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([ ), doc='Add file to VFS from buffer, return 0: success, 1: full, 2: repeated name, -1: failed to load.', # pylint: disable=line-too-long )), - ('mj_findFileVFS', - FunctionDecl( - name='mj_findFileVFS', - return_type=ValueType(name='int'), - parameters=( - FunctionParameterDecl( - name='vfs', - type=PointerType( - inner_type=ValueType(name='mjVFS', is_const=True), - ), - ), - FunctionParameterDecl( - name='filename', - type=PointerType( - inner_type=ValueType(name='char', is_const=True), - ), - ), - ), - doc='Return file index in VFS, or -1 if not found in VFS.', - )), ('mj_deleteFileVFS', FunctionDecl( name='mj_deleteFileVFS', @@ -148,31 +128,7 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([ ), ), ), - doc='Delete all files from VFS.', - )), - ('mj_makeEmptyFileVFS', - FunctionDecl( - name='mj_makeEmptyFileVFS', - return_type=ValueType(name='int'), - parameters=( - FunctionParameterDecl( - name='vfs', - type=PointerType( - inner_type=ValueType(name='mjVFS'), - ), - ), - FunctionParameterDecl( - name='filename', - type=PointerType( - inner_type=ValueType(name='char', is_const=True), - ), - ), - FunctionParameterDecl( - name='filesize', - type=ValueType(name='int'), - ), - ), - doc='deprecated: use mj_copyBufferVFS.', + doc='Delete all files from VFS and deallocates VFS internal memory.', )), ('mj_loadXML', FunctionDecl( diff --git a/src/user/user_vfs.h b/src/user/user_vfs.h index 80a7fcef..8896ac6d 100644 --- a/src/user/user_vfs.h +++ b/src/user/user_vfs.h @@ -27,17 +27,17 @@ extern "C" { #endif -// initialize to empty (no deallocation) +// 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 MJAPI int mj_addFileVFS(mjVFS* vfs, const char* directory, const char* filename); -// deprecated: use mj_copyBufferVFS +// 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 -MJAPI int mj_copyBufferVFS(mjVFS* vfs, const char* filename, const void* buffer, int nbuffer); +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 MJAPI int mj_findFileVFS(const mjVFS* vfs, const char* filename); diff --git a/unity/Runtime/Bindings/MjBindings.cs b/unity/Runtime/Bindings/MjBindings.cs index 0d551cdd..c6afdeff 100644 --- a/unity/Runtime/Bindings/MjBindings.cs +++ b/unity/Runtime/Bindings/MjBindings.cs @@ -6376,18 +6376,12 @@ public static unsafe extern int mj_addFileVFS(void* vfs, [MarshalAs(UnmanagedTyp [DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)] public static unsafe extern int mj_addBufferVFS(void* vfs, [MarshalAs(UnmanagedType.LPStr)]string name, void* buffer, int nbuffer); -[DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)] -public static unsafe extern int mj_findFileVFS(void* vfs, [MarshalAs(UnmanagedType.LPStr)]string filename); - [DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)] public static unsafe extern int mj_deleteFileVFS(void* vfs, [MarshalAs(UnmanagedType.LPStr)]string filename); [DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)] public static unsafe extern void mj_deleteVFS(void* vfs); -[DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)] -public static unsafe extern int mj_makeEmptyFileVFS(void* vfs, [MarshalAs(UnmanagedType.LPStr)]string filename, int filesize); - [DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)] public static unsafe extern mjModel_* mj_loadXML([MarshalAs(UnmanagedType.LPStr)]string filename, void* vfs, StringBuilder error, int error_sz); diff --git a/unity/Runtime/Tools/MjVfs.cs b/unity/Runtime/Tools/MjVfs.cs index a52da4b2..14174e6a 100644 --- a/unity/Runtime/Tools/MjVfs.cs +++ b/unity/Runtime/Tools/MjVfs.cs @@ -59,20 +59,8 @@ public sealed class MjVfs : IDisposable { } } - // Searches the VFS for the specified file and returns its index. - // The index then can be used to retrieve the file contents from Data.filedata array. - public unsafe int FindFile(string filename) { - return mj_findFileVFS(_unmanagedVfs.ToPointer(), filename); - } - // Loads a model from the specified file. - // The file is assumed to be located in the filesystem. If it's not found, the method will throw - // an ArgumentException. public unsafe MujocoLib.mjModel_* LoadXML(string filename) { - if (FindFile(filename) < 0) { - throw new ArgumentException($"File {filename} was not added to the VFS."); - } - var errorBuf = new StringBuilder(1024); MujocoLib.mjModel_* model = MujocoLib.mj_loadXML( filename, _unmanagedVfs.ToPointer(), errorBuf, errorBuf.Capacity); diff --git a/unity/Tests/Editor/Tools/MjVFSEditorTests.cs b/unity/Tests/Editor/Tools/MjVFSEditorTests.cs index b43ea404..12ab6562 100644 --- a/unity/Tests/Editor/Tools/MjVFSEditorTests.cs +++ b/unity/Tests/Editor/Tools/MjVFSEditorTests.cs @@ -42,12 +42,5 @@ namespace Mujoco { _vfs.AddFile(filename, contents); Assert.That(_vfs.FilesCount, Is.EqualTo(1)); } - - [Test] - public unsafe void LocatingFileUsingMujocoLib() { - var filename = "filename"; - _vfs.AddFile(filename, "contents"); - Assert.That(_vfs.FindFile(filename), Is.EqualTo(0)); - } } }