Cleanup WASM bindings struct generation

PiperOrigin-RevId: 827949686
Change-Id: I4115e397f64963244e3c30a7eaff5be7534aadf2

wasm/codegen/helpers/helpers_test.py

@@ -294,6 +294,11 @@ class StructConstructorCodeBuilderTest(absltest.TestCase):
         wrapped_structs["mjLROpt"].wrapped_source,
         """
 MjLROpt::MjLROpt(mjLROpt *ptr) : ptr_(ptr) {}
+MjLROpt::~MjLROpt() {
+  if (owned_ && ptr_) {
+    delete ptr_;
+  }
+}
 MjLROpt::MjLROpt() : ptr_(new mjLROpt) {
   owned_ = true;
   mj_defaultLROpt(ptr_);
@@ -308,14 +313,15 @@ MjLROpt& MjLROpt::operator=(const MjLROpt &other) {
   *ptr_ = *other.get();
   return *this;
 }
-MjLROpt::~MjLROpt() {
-  if (owned_ && ptr_) {
-    delete ptr_;
-  }
-}
 std::unique_ptr<MjLROpt> MjLROpt::copy() {
   return std::make_unique<MjLROpt>(*this);
 }
+mjLROpt* MjLROpt::get() const {
+  return ptr_;
+}
+void MjLROpt::set(mjLROpt* ptr) {
+  ptr_ = ptr;
+}
 """.strip(),
     )
 
@@ -326,8 +332,11 @@ std::unique_ptr<MjLROpt> MjLROpt::copy() {
         """
 MjsElement::MjsElement(mjsElement *ptr) : ptr_(ptr) {}
 MjsElement::~MjsElement() {}
-std::unique_ptr<MjsElement> MjsElement::copy() {
+mjsElement* MjsElement::get() const {
-  return std::make_unique<MjsElement>(*this);
+  return ptr_;
+}
+void MjsElement::set(mjsElement* ptr) {
+  ptr_ = ptr;
 }
 """.strip(),
     )
@@ -351,6 +360,12 @@ std::unique_ptr<MjsElement> MjsElement::copy() {
         """
 MjsTexture::MjsTexture(mjsTexture *ptr) : ptr_(ptr), element(ptr_->element) {}
 MjsTexture::~MjsTexture() {}
+mjsTexture* MjsTexture::get() const {
+  return ptr_;
+}
+void MjsTexture::set(mjsTexture* ptr) {
+  ptr_ = ptr;
+}
 """.strip(),
     )
 
@@ -358,6 +373,11 @@ MjsTexture::~MjsTexture() {}
     self.assertEqual(
         structs.build_struct_source("mjvLight", []),
         """MjvLight::MjvLight(mjvLight *ptr) : ptr_(ptr) {}
+MjvLight::~MjvLight() {
+  if (owned_ && ptr_) {
+    delete ptr_;
+  }
+}
 MjvLight::MjvLight() : ptr_(new mjvLight) {
   owned_ = true;
 }
@@ -371,14 +391,16 @@ MjvLight& MjvLight::operator=(const MjvLight &other) {
   *ptr_ = *other.get();
   return *this;
 }
-MjvLight::~MjvLight() {
-  if (owned_ && ptr_) {
-    delete ptr_;
-  }
-}
 std::unique_ptr<MjvLight> MjvLight::copy() {
   return std::make_unique<MjvLight>(*this);
-}""".strip(),
+}
+mjvLight* MjvLight::get() const {
+  return ptr_;
+}
+void MjvLight::set(mjvLight* ptr) {
+  ptr_ = ptr;
+}
+""".strip(),
     )
 
 

wasm/codegen/helpers/structs.py

@@ -459,28 +459,25 @@ def _build_struct_header_internal(
 
   builder = code_builder.CodeBuilder()
   with builder.struct(f"{w}"):
-    if not is_mjs:
-      builder.line(f"{w}();")
-      builder.line(f"{w}(const {w} &);")
-      builder.line(f"{w} &operator=(const {w} &);")
-
     builder.line(f"explicit {w}({s} *ptr);")
     builder.line(f"~{w}();")
 
-    if shallow_copy:
+    if not is_mjs:
+      builder.line(f"{w}();")
+
+    if shallow_copy and not is_mjs:
+      builder.line(f"{w}(const {w} &);")
+      builder.line(f"{w} &operator=(const {w} &);")
       builder.line(f"std::unique_ptr<{w}> copy();")
 
+    builder.line(f"{s}* get() const;")
+    builder.line(f"void set({s}* ptr);")
+
     for field in wrapped_fields:
       if field.definition and field not in fields_with_init:
         for line in field.definition.splitlines():
           builder.line(line)
 
-    with builder.function(f"{s}* get() const"):
-      builder.line("return ptr_;")
-
-    with builder.function(f"void set({s}* ptr)"):
-      builder.line("ptr_ = ptr;")
-
     builder.private()
     builder.line(f"{s}* ptr_;")
     if not is_mjs:
@@ -491,6 +488,7 @@ def _build_struct_header_internal(
       for field in fields_with_init:
         if field.definition:
           builder.line(f"{field.definition}")
+
   return builder.to_string() + ";"
 
 
@@ -600,8 +598,15 @@ def build_struct_source(
   with builder.function(f"{w}::{w}({s} *ptr) : ptr_(ptr){fields_init}"):
     pass
 
-  # constructor with default values
+  # destructor
+  with builder.function(f"{w}::~{w}()"):
     if not is_mjs:
+      with builder.block("if (owned_ && ptr_)"):
+        delete_ptr = _delete_ptr_statement(s)
+        builder.line(delete_ptr)
+
+  if not is_mjs:
+    # default constructor
     with builder.function(f"{w}::{w}() : ptr_(new {s}){fields_init}"):
       builder.line("owned_ = true;")
       default_func = _default_function_statement(s)
@@ -626,18 +631,16 @@ def build_struct_source(
           builder.line(field_with_init.ptr_copy_reset)
       builder.line("return *this;")
 
-  # destructor
+    # explicit copy function
-  with builder.function(f"{w}::~{w}()"):
-    if not is_mjs:
-      with builder.block("if (owned_ && ptr_)"):
-        delete_ptr = _delete_ptr_statement(s)
-        builder.line(delete_ptr)
-
-  # copy function
-  if shallow_copy:
     with builder.function(f"std::unique_ptr<{w}> {w}::copy()"):
       builder.line(f"return std::make_unique<{w}>(*this);")
 
+  with builder.function(f"{s}* {w}::get() const"):
+    builder.line("return ptr_;")
+
+  with builder.function(f"void {w}::set({s}* ptr)"):
+    builder.line("ptr_ = ptr;")
+
   return builder.to_string()
 
 

wasm/codegen/templates/bindings.cc

@@ -55,9 +55,9 @@ struct MjVisualGlobal {
   MjVisualGlobal &operator=(const MjVisualGlobal &);
   ~MjVisualGlobal();
   std::unique_ptr<MjVisualGlobal> copy();
+  mjVisualGlobal* get() const;
+  void set(mjVisualGlobal* ptr);
   // INSERT-GENERATED-MjVisualGlobal-DEFINITIONS
-  mjVisualGlobal* get() const { return ptr_; }
-  void set(mjVisualGlobal* ptr) { ptr_ = ptr; }
 
  private:
   mjVisualGlobal* ptr_;
@@ -71,9 +71,9 @@ struct MjVisualQuality {
   MjVisualQuality &operator=(const MjVisualQuality &);
   ~MjVisualQuality();
   std::unique_ptr<MjVisualQuality> copy();
+  mjVisualQuality* get() const;
+  void set(mjVisualQuality* ptr);
   // INSERT-GENERATED-MjVisualQuality-DEFINITIONS
-  mjVisualQuality* get() const { return ptr_; }
-  void set(mjVisualQuality* ptr) { ptr_ = ptr; }
 
  private:
   mjVisualQuality* ptr_;
@@ -87,9 +87,9 @@ struct MjVisualHeadlight {
   MjVisualHeadlight &operator=(const MjVisualHeadlight &);
   ~MjVisualHeadlight();
   std::unique_ptr<MjVisualHeadlight> copy();
+  mjVisualHeadlight* get() const;
+  void set(mjVisualHeadlight* ptr);
   // INSERT-GENERATED-MjVisualHeadlight-DEFINITIONS
-  mjVisualHeadlight* get() const { return ptr_; }
-  void set(mjVisualHeadlight* ptr) { ptr_ = ptr; }
 
  private:
   mjVisualHeadlight* ptr_;
@@ -103,9 +103,9 @@ struct MjVisualMap {
   MjVisualMap &operator=(const MjVisualMap &);
   ~MjVisualMap();
   std::unique_ptr<MjVisualMap> copy();
+  mjVisualMap* get() const;
+  void set(mjVisualMap* ptr);
   // INSERT-GENERATED-MjVisualMap-DEFINITIONS
-  mjVisualMap* get() const { return ptr_; }
-  void set(mjVisualMap* ptr) { ptr_ = ptr; }
 
  private:
   mjVisualMap* ptr_;
@@ -119,9 +119,9 @@ struct MjVisualScale {
   MjVisualScale &operator=(const MjVisualScale &);
   ~MjVisualScale();
   std::unique_ptr<MjVisualScale> copy();
+  mjVisualScale* get() const;
+  void set(mjVisualScale* ptr);
   // INSERT-GENERATED-MjVisualScale-DEFINITIONS
-  mjVisualScale* get() const { return ptr_; }
-  void set(mjVisualScale* ptr) { ptr_ = ptr; }
 
  private:
   mjVisualScale* ptr_;
@@ -135,9 +135,9 @@ struct MjVisualRgba {
   MjVisualRgba &operator=(const MjVisualRgba &);
   ~MjVisualRgba();
   std::unique_ptr<MjVisualRgba> copy();
+  mjVisualRgba* get() const;
+  void set(mjVisualRgba* ptr);
   // INSERT-GENERATED-MjVisualRgba-DEFINITIONS
-  mjVisualRgba* get() const { return ptr_; }
-  void set(mjVisualRgba* ptr) { ptr_ = ptr; }
 
  private:
   mjVisualRgba* ptr_;
@@ -151,9 +151,9 @@ struct MjVisual {
   MjVisual &operator=(const MjVisual &);
   ~MjVisual();
   std::unique_ptr<MjVisual> copy();
+  mjVisual* get() const;
+  void set(mjVisual* ptr);
   // INSERT-GENERATED-MjVisual-DEFINITIONS
-  mjVisual* get() const { return ptr_; }
-  void set(mjVisual* ptr) { ptr_ = ptr; }
 
  private:
   mjVisual* ptr_;
@@ -173,9 +173,9 @@ struct MjModel {
   explicit MjModel(const MjModel &other);
   ~MjModel();
   std::unique_ptr<MjModel> copy();
+  mjModel* get() const;
+  void set(mjModel* ptr);
   // INSERT-GENERATED-MjModel-DEFINITIONS
-  mjModel* get() const { return ptr_; }
-  void set(mjModel* ptr) { ptr_ = ptr; }
 
  private:
   mjModel* ptr_;
@@ -195,9 +195,9 @@ struct MjData {
   std::vector<MjWarningStat> InitWarningArray();
   std::vector<MjContact> contact() const;
   std::unique_ptr<MjData> copy();
+  mjData* get() const;
+  void set(mjData* ptr);
   // INSERT-GENERATED-MjData-DEFINITIONS
-  mjData* get() const { return ptr_; }
-  void set(mjData* ptr) { ptr_ = ptr; }
 
  private:
   mjData* ptr_;
@@ -219,6 +219,9 @@ struct MjvScene {
   std::vector<MjvLight> InitLightsArray();
   std::vector<MjvGLCamera> InitCameraArray();
 
+  mjvScene* get() const;
+  void set(mjvScene* ptr);
+
   std::vector<MjvGeom> geoms() const;
 
   emscripten::val geomorder() const {
@@ -294,8 +297,6 @@ struct MjvScene {
         6 * MjvScene::GetSumFlexFaces(), ptr_->flextexcoord));
   }
   // INSERT-GENERATED-MjvScene-DEFINITIONS
-  mjvScene* get() const { return ptr_; }
-  void set(mjvScene* ptr) { ptr_ = ptr; }
 
  private:
   mjvScene* ptr_;
@@ -314,9 +315,9 @@ struct MjSpec {
   MjSpec &operator=(const MjSpec &);
   ~MjSpec();
   std::unique_ptr<MjSpec> copy();
+  mjSpec* get() const;
+  void set(mjSpec* ptr);
   // INSERT-GENERATED-MjSpec-DEFINITIONS
-  mjSpec* get() const { return ptr_; }
-  void set(mjSpec* ptr) { ptr_ = ptr; }
 
  private:
   mjSpec* ptr_;
@@ -495,6 +496,8 @@ MjModel::~MjModel() {
     mj_deleteModel(ptr_);
   }
 }
+mjModel* MjModel::get() const { return ptr_; }
+void MjModel::set(mjModel *ptr) { ptr_ = ptr; }
 
 // TODO(manevi): Consider passing `const MjModel& m` here, mj_makeData uses a const model.
 // =============== MjData =============== //
@@ -520,6 +523,9 @@ MjData::~MjData() {
     mj_deleteData(ptr_);
   }
 }
+mjData* MjData::get() const { return ptr_; }
+void MjData::set(mjData *ptr) { ptr_ = ptr; }
+
 std::vector<MjSolverStat> MjData::InitSolverArray() {
   std::vector<MjSolverStat> arr;
   arr.reserve(mjNSOLVER * mjNISLAND);
@@ -597,6 +603,9 @@ MjvScene::~MjvScene() {
   }
 }
 
+mjvScene* MjvScene::get() const { return ptr_; }
+void MjvScene::set(mjvScene *ptr) { ptr_ = ptr; }
+
 // Taken from the python mujoco bindings code for MjvScene Wrapper
 int MjvScene::GetSumFlexFaces() const {
   int nflexface = 0;
@@ -720,6 +729,9 @@ MjSpec::~MjSpec() {
   }
 }
 
+mjSpec *MjSpec::get() const { return ptr_; }
+void MjSpec::set(mjSpec *ptr) { ptr_ = ptr; }
+
 // =============== MjsOrientation =============== //
 // INSERT-GENERATED-MjsOrientation-CONSTRUCTOR
 
@@ -965,7 +977,6 @@ EMSCRIPTEN_BINDINGS(mujoco_bindings) {
       ;
 
   emscripten::class_<MjsOrientation>("MjsOrientation")
-      .function("copy", &MjsOrientation::copy, take_ownership())
       // INSERT-GENERATED-MjsOrientation-BINDINGS
       ;