Do not copy the spec during attach.

Use a reference count for managing the memory.

PiperOrigin-RevId: 716169486
Change-Id: Id270c4858c17b9250115e9544d5ea143584e2d5f

doc/APIreference/functions.rst

@@ -1510,6 +1510,15 @@ Free memory allocation in mjSpec.
 
 Activate plugin. Returns 0 on success.
 
+.. _mjs_setDeepCopy:
+
+`mjs_setDeepCopy <#mjs_setDeepCopy>`__
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. mujoco-include:: mjs_setDeepCopy
+
+Turn deep copy on or off attach. Returns 0 on success.
+
 .. _Errorandmemory:
 
 Error and memory

doc/changelog.rst

@@ -2,8 +2,13 @@
 Changelog
 =========
 
-Version 3.2.7 (Jan 14, 2025)
-----------------------------
+Upcoming version (not yet released)
+-----------------------------------
+
+- Added ``mjs_setDeepCopy`` API function. When the deep copy flag is 0, attaching a model will not copy it to the
+  parent, so the original references to the child allow to modify the parent as well. The default behavior is to perform
+  such a shallow copy. The old behavioud of creating a deep copy of the child model while attaching can be restored by
+  setting the deep copy flag to 1.
 
 Python bindings
 ^^^^^^^^^^^^^^^

doc/includes/references.h

@@ -3181,6 +3181,7 @@ mjSpec* mj_makeSpec(void);
 mjSpec* mj_copySpec(const mjSpec* s);
 void mj_deleteSpec(mjSpec* s);
 int mjs_activatePlugin(mjSpec* s, const char* name);
+int mjs_setDeepCopy(mjSpec* s, int deepcopy);
 void mj_printFormattedModel(const mjModel* m, const char* filename, const char* float_format);
 void mj_printModel(const mjModel* m, const char* filename);
 void mj_printFormattedData(const mjModel* m, mjData* d, const char* filename,

doc/programming/modeledit.rst

@@ -106,12 +106,14 @@ procedurally, default classes are passed in explicitly to element constructors.
 
 Attachment
 ^^^^^^^^^^
-This framework introduces a powerful new feature: attaching and detaching model subtrees. Attachment allows the user
-copy a subtree from one model into another, while also copying related referenced assets and referencing elements from
-outside the kinematic tree (e.g., actuators and sensors). Similarly, detaching a subtree will remove all associated
-elements from the model. This feature is already used to power the :ref:`attach<body-attach>` and
-:ref:`replicate<replicate>` meta-elements in MJCF. It is possible to :ref:`attach a body to a frame<mjs_attachBody>` and
-to :ref:`attach a body to a site<mjs_attachToSite>`:
+
+This framework introduces a powerful new feature: attaching and detaching model subtrees. This feature is already used
+to power the :ref:`attach<body-attach>` an :ref:`replicate<replicate>` meta-elements in MJCF. Attachment allows the user
+to move or copy a subtree from one model into another, while also copying or moving related referenced assets and
+referencing elements from outside the kinematic tree (e.g., actuators and sensors). Similarly, detaching a subtree will
+remove all associated elements from the model. The default behavior is to move during attach. The user can select to
+instead copy by passing the corresponding flag to ``mjs_setDeepCopy``. This flag is temporary set to true while parsing
+XMLs. It is possible to :ref:`attach a body to a frame<mjs_attachBody>`:
 
 .. code-block:: C
 
@@ -120,9 +122,17 @@ to :ref:`attach a body to a site<mjs_attachToSite>`:
    parent->compiler.degree = 0;
    child->compiler.degree = 1;
    mjsFrame* frame = mjs_addFrame(mjs_findBody(parent, "world"), NULL);
-   mjsSite* site = mjs_addSite(mjs_findBody(parent, "world"), NULL);
    mjsBody* body = mjs_addBody(mjs_findBody(child, "world"), NULL);
    mjsBody* attached_body_1 = mjs_attachBody(frame, body, "attached-", "-1");
+
+or :ref:`attach a body to a site<mjs_attachToSite>`:
+
+.. code-block:: C
+
+   mjSpec* parent = mj_makeSpec();
+   mjSpec* child = mj_makeSpec();
+   mjsSite* site = mjs_addSite(mjs_findBody(parent, "world"), NULL);
+   mjsBody* body = mjs_addBody(mjs_findBody(child, "world"), NULL);
    mjsBody* attached_body_2 = mjs_attachToSite(site, body, "attached-", "-2");
 
 or :ref:`attach a frame to a body<mjs_attachFrame>`:

doc/python.rst

@@ -531,15 +531,19 @@ Attachment
 It is possible to combine multiple specs by using attachments. The following options are possible:
 
 -   Attach a body from the child spec to a frame in the parent spec: ``body.attach_body(body, prefix, suffix)``, returns
-    the newly createdbody in the parent spec.
+    the reference to the attached body, which should be identical to the body used as input.
 -   Attach a frame from the child spec to a body in the parent spec: ``body.attach_frame(frame, prefix, suffix)``,
-    returns the newly created frame in the parent spec.
+    returns the reference to the attached frame, which should be identical to the frame used as input.
 -   Attach a body from the child spec to a site in the parent spec: ``site.attach(body, prefix, suffix)``, returns the
-    newly created body in the parent spec.
+    reference to the attached body, which should be identical to the body used as input.
 -   Attach the worldbody from the child spec to a frame in the parent spec and transform it to a frame:
     ``body.attach(spec, prefix, suffix)``, returns the newly created frame that the child worldbody was transformed
     into.
 
+Attaching does not copy, so all the child reference are still valid in the parent and therefore modifying the child will
+modify the parent. This is not true for the attach :ref:`attach<body-attach>` an :ref:`replicate<replicate>`
+meta-elements in MJCF, which create deep copies while attaching.
+
 .. code-block:: python
 
    import mujoco

include/mujoco/mujoco.h

@@ -242,6 +242,9 @@ MJAPI void mj_deleteSpec(mjSpec* s);
 // Activate plugin. Returns 0 on success.
 MJAPI int mjs_activatePlugin(mjSpec* s, const char* name);
 
+// Turn deep copy on or off attach. Returns 0 on success.
+MJAPI int mjs_setDeepCopy(mjSpec* s, int deepcopy);
+
 
 //---------------------------------- Printing ------------------------------------------------------
 

introspect/functions.py

@@ -1045,6 +1045,24 @@
          ),
          doc='Activate plugin. Returns 0 on success.',
      )),
+    ('mjs_setDeepCopy',
+     FunctionDecl(
+         name='mjs_setDeepCopy',
+         return_type=ValueType(name='int'),
+         parameters=(
+             FunctionParameterDecl(
+                 name='s',
+                 type=PointerType(
+                     inner_type=ValueType(name='mjSpec'),
+                 ),
+             ),
+             FunctionParameterDecl(
+                 name='deepcopy',
+                 type=ValueType(name='int'),
+             ),
+         ),
+         doc='Turn deep copy on or off attach. Returns 0 on success.',
+     )),
     ('mj_printFormattedModel',
      FunctionDecl(
          name='mj_printFormattedModel',

python/mujoco/specs.cc

@@ -350,6 +350,14 @@ PYBIND11_MODULE(_specs, m) {
   mjSpec.def("copy", [](const MjSpec& self) -> MjSpec {
     return MjSpec(self);
   });
+  mjSpec.def_property(
+      "copy_during_attach",
+      [](MjSpec& self) {
+        throw pybind11::value_error("copy_during_attach can only be set.");
+      },
+      [](MjSpec& self, bool deepcopy) {
+        return mjs_setDeepCopy(self.ptr, deepcopy);
+      });
   mjSpec.def_property_readonly(
       "worldbody",
       [](MjSpec& self) -> raw::MjsBody* {

python/mujoco/specs_test.py

@@ -918,27 +918,31 @@ def test_attach_units(self):
     model = parent.compile()
     np.testing.assert_almost_equal(model.body_quat[1], [1, 0, 0, 0])
 
-  def test_attach_body_to_site(self):
-    child = mujoco.MjSpec()
+  def test_attach_to_site(self):
     parent = mujoco.MjSpec()
     site = parent.worldbody.add_site(pos=[1, 2, 3], quat=[0, 0, 0, 1])
-    body = child.worldbody.add_body()
 
     # Attach body to site and compile.
-    self.assertIsNotNone(site.attach_body(body, prefix='_'))
+    child1 = mujoco.MjSpec()
+    body1 = child1.worldbody.add_body()
+    self.assertIs(body1, site.attach_body(body1, prefix='_'))
+    body1.pos = [1, 1, 1]
     model1 = parent.compile()
     self.assertIsNotNone(model1)
     self.assertEqual(model1.nbody, 2)
-    np.testing.assert_array_equal(model1.body_pos[1], [1, 2, 3])
+    np.testing.assert_array_equal(model1.body_pos[1], [0, 1, 4])
     np.testing.assert_array_equal(model1.body_quat[1], [0, 0, 0, 1])
 
     # Attach entire spec to site and compile again.
-    self.assertIsNotNone(site.attach(child, prefix='child-'))
+    child2 = mujoco.MjSpec()
+    body2 = child2.worldbody.add_body(name='body')
+    self.assertIsNotNone(site.attach(child2, prefix='child-'))
+    body2.pos = [-1, -1, -1]
     model2 = parent.compile()
     self.assertIsNotNone(model2)
     self.assertEqual(model2.nbody, 3)
-    np.testing.assert_array_equal(model2.body_pos[1], [1, 2, 3])
-    np.testing.assert_array_equal(model2.body_pos[2], [1, 2, 3])
+    np.testing.assert_array_equal(model2.body_pos[1], [0, 1, 4])
+    np.testing.assert_array_equal(model2.body_pos[2], [2, 3, 2])
     np.testing.assert_array_equal(model2.body_quat[1], [0, 0, 0, 1])
     np.testing.assert_array_equal(model2.body_quat[2], [0, 0, 0, 1])
 
@@ -949,27 +953,31 @@ def test_body_to_frame(self):
     frame = body.to_frame()
     np.testing.assert_array_equal(frame.pos, [1, 2, 3])
 
-  def test_attach_spec_to_frame(self):
-    child = mujoco.MjSpec()
+  def test_attach_to_frame(self):
     parent = mujoco.MjSpec()
     frame = parent.worldbody.add_frame(pos=[1, 2, 3], quat=[0, 0, 0, 1])
-    body = child.worldbody.add_body()
 
     # Attach body to frame and compile.
-    self.assertIsNotNone(frame.attach_body(body, prefix='_'))
+    child1 = mujoco.MjSpec()
+    body1 = child1.worldbody.add_body()
+    self.assertIs(body1, frame.attach_body(body1, prefix='_'))
+    body1.pos = [1, 1, 1]
     model1 = parent.compile()
     self.assertIsNotNone(model1)
     self.assertEqual(model1.nbody, 2)
-    np.testing.assert_array_equal(model1.body_pos[1], [1, 2, 3])
+    np.testing.assert_array_equal(model1.body_pos[1], [0, 1, 4])
     np.testing.assert_array_equal(model1.body_quat[1], [0, 0, 0, 1])
 
     # Attach entire spec to frame and compile again.
-    self.assertIsNotNone(frame.attach(child, prefix='child-'))
+    child2 = mujoco.MjSpec()
+    body2 = child2.worldbody.add_body(name='body')
+    self.assertIsNotNone(frame.attach(child2, prefix='child-'))
+    body2.pos = [-1, -1, -1]
     model2 = parent.compile()
     self.assertIsNotNone(model2)
     self.assertEqual(model2.nbody, 3)
-    np.testing.assert_array_equal(model2.body_pos[1], [1, 2, 3])
-    np.testing.assert_array_equal(model2.body_pos[2], [1, 2, 3])
+    np.testing.assert_array_equal(model2.body_pos[1], [0, 1, 4])
+    np.testing.assert_array_equal(model2.body_pos[2], [2, 3, 2])
     np.testing.assert_array_equal(model2.body_quat[1], [0, 0, 0, 1])
     np.testing.assert_array_equal(model2.body_quat[2], [0, 0, 0, 1])
 

src/user/user_api.cc

@@ -208,7 +208,7 @@ int mjs_detachBody(mjSpec* s, mjsBody* b) {
     model->SetError(e);
     return -1;
   }
-  delete body;
+  model->Detach(body);
   return 0;
 }
 
@@ -254,6 +254,15 @@ int mjs_activatePlugin(mjSpec* s, const char* name) {
 
 
 
+// set deep copy flag
+int mjs_setDeepCopy(mjSpec* s, int deepcopy) {
+  mjCModel* model = static_cast<mjCModel*>(s->element);
+  model->SetDeepCopy(deepcopy);
+  return 0;
+}
+
+
+
 // delete object, return 0 if success
 int mjs_delete(mjsElement* element) {
   mjCBase* object = static_cast<mjCBase*>(element);
@@ -705,7 +714,7 @@ const char* mjs_resolveOrientation(double quat[4], mjtByte degree, const char* s
 mjsFrame* mjs_bodyToFrame(mjsBody** body) {
   mjCBody* bodyC = static_cast<mjCBody*>((*body)->element);
   mjCFrame* frameC = bodyC->ToFrame();
-  delete bodyC;
+  bodyC->model->Detach(bodyC);
   *body = nullptr;
   return &frameC->spec;
 }

src/user/user_api.h

@@ -63,6 +63,9 @@ MJAPI void mjs_addSpec(mjSpec* s, mjSpec* child);
 // Activate plugin, return 0 on success.
 MJAPI int mjs_activatePlugin(mjSpec* s, const char* name);
 
+// Turn deep copy on or off attach. Returns 0 on success.
+MJAPI int mjs_setDeepCopy(mjSpec* s, int deepcopy);
+
 
 //---------------------------------- Attachment ----------------------------------------------------
 

src/user/user_mesh.cc

@@ -278,9 +278,6 @@ void mjCMesh::CopyPlugin() {
 mjCMesh::~mjCMesh() {
   if (center_) mju_free(center_);
   if (graph_) mju_free(graph_);
-  if (spec.plugin.active && spec.plugin.name->empty() && model) {
-    model->DeleteElement(spec.plugin.element);
-  }
 }