Remove getter functions from SurfaceReconstruction

Author: w1th0utnam3

CHANGELOG.md

@@ -9,7 +9,7 @@ The following changes are present in the `main` branch of the repository and are
  - Lib: Add support for "dense" `DensityMap` (borrowed & owned) as input for the marching cubes triangulation, useful for the Python bindings
  - Lib: Replace usage of `DiscreteSquaredDistanceCubicKernel` by standard `CubicSplineKernel` for SPH interpolation (no noticeable performance difference)
  - Lib: Enforce that `Index` types are signed integers implementing the `num_traits::Signed` trait. Currently, the reconstruction does not work (correctly) with unsigned integers.
- - Lib: Make most fields of `SurfaceReconstruction` public
+ - Lib: Make most fields of `SurfaceReconstruction` public, remove previous getter functions
  - CLI: Add some tests for the `reconstruction_pipeline` function
  - CLI: Fix post-processing when particle AABB filtering is enabled
  - Lib: Support subdomain "ghost particle" margins to be up to the size of the subdomain itself (previously limited to half the size)

splashsurf/src/reconstruct.rs

@@ -1040,8 +1040,8 @@ pub fn reconstruction_pipeline<'a, I: Index, R: Real>(
         }
     }
 
-    let grid = reconstruction.grid();
-    let mut mesh_with_data = MeshWithData::new(Cow::Borrowed(reconstruction.mesh()));
+    let grid = &reconstruction.grid;
+    let mut mesh_with_data = MeshWithData::new(Cow::Borrowed(&reconstruction.mesh));
 
     // Perform post-processing
     {
@@ -1054,7 +1054,7 @@ pub fn reconstruction_pipeline<'a, I: Index, R: Real>(
             let verts_before = mesh_with_data.mesh.vertices.len();
             vertex_connectivity = Some(splashsurf_lib::postprocessing::marching_cubes_cleanup(
                 mesh_with_data.mesh.to_mut(),
-                reconstruction.grid(),
+                &reconstruction.grid,
                 postprocessing
                     .mesh_cleanup_snap_dist
                     .map(|d| R::from_float(d)),
@@ -1092,7 +1092,10 @@ pub fn reconstruction_pipeline<'a, I: Index, R: Real>(
             // TODO: Re-use filtered particles from reconstruction?
             filtered_quantity(
                 particle_positions,
-                reconstruction.particle_inside_aabb().map(Vec::as_slice),
+                reconstruction
+                    .particle_inside_aabb
+                    .as_ref()
+                    .map(Vec::as_slice),
             )
         } else {
             Cow::Borrowed(particle_positions)
@@ -1114,7 +1117,7 @@ pub fn reconstruction_pipeline<'a, I: Index, R: Real>(
             let particle_rest_mass = particle_rest_volume * particle_rest_density;
 
             let particle_densities = reconstruction
-                .particle_densities()
+                .particle_densities.as_ref()
                 .ok_or_else(|| anyhow::anyhow!("Particle densities were not returned by surface reconstruction but are required for SPH normal computation"))?
                 .as_slice();
             assert_eq!(
@@ -1150,7 +1153,7 @@ pub fn reconstruction_pipeline<'a, I: Index, R: Real>(
 
             // Global neighborhood search
             let nl = reconstruction
-                .particle_neighbors()
+                .particle_neighbors.as_ref()
                 .map(Cow::Borrowed)
                 .unwrap_or_else(||
                     {
@@ -1337,7 +1340,10 @@ pub fn reconstruction_pipeline<'a, I: Index, R: Real>(
             {
                 info!("Interpolating attribute \"{}\"...", attribute.name);
 
-                let particles_inside = reconstruction.particle_inside_aabb().map(Vec::as_slice);
+                let particles_inside = reconstruction
+                    .particle_inside_aabb
+                    .as_ref()
+                    .map(Vec::as_slice);
                 match &attribute.data {
                     AttributeData::ScalarReal(values) => {
                         let filtered_values = filtered_quantity(&values, particles_inside);
@@ -1532,7 +1538,7 @@ pub fn reconstruction_pipeline<'a, I: Index, R: Real>(
             } = mesh;
 
             // Avoid copy if original mesh was not modified
-            let (mesh, take_mesh) = if std::ptr::eq(mesh.as_ref(), reconstruction.mesh())
+            let (mesh, take_mesh) = if std::ptr::eq(mesh.as_ref(), &reconstruction.mesh)
                 && !postprocessing.output_raw_mesh
             {
                 // Ensure that borrow of reconstruction is dropped
@@ -1602,7 +1608,7 @@ pub(crate) fn reconstruction_pipeline_from_path<I: Index, R: Real>(
     if postprocessing.output_raw_mesh {
         profile!("write surface mesh to file");
 
-        let mesh = reconstruction.mesh();
+        let mesh = &reconstruction.mesh;
 
         let output_path = paths
             .output_file

splashsurf/tests/test_pipeline.rs

@@ -27,7 +27,7 @@ fn test_basic_pipeline() -> Result<(), Box<dyn std::error::Error>> {
         .as_ref()
         .expect("reconstruction should produce a triangle mesh")
         .mesh;
-    let raw_mesh = reconstruction.raw_reconstruction.mesh();
+    let raw_mesh = &reconstruction.raw_reconstruction.mesh;
     vtk_format::write_vtk(mesh, "../out/bunny_test_basic_pipeline.vtk", "mesh")?;
 
     // Compare raw and final mesh
@@ -203,7 +203,7 @@ fn test_basic_pipeline_postprocessing() -> Result<(), Box<dyn std::error::Error>
         .as_ref()
         .expect("reconstruction should produce a triangle mesh")
         .mesh;
-    let raw_mesh = reconstruction.raw_reconstruction.mesh();
+    let raw_mesh = &reconstruction.raw_reconstruction.mesh;
     vtk_format::write_vtk(
         mesh,
         "../out/bunny_test_basic_pipeline_postprocessing.vtk",
@@ -286,7 +286,7 @@ fn test_basic_pipeline_postprocessing_with_aabb() -> Result<(), Box<dyn std::err
         .as_ref()
         .expect("reconstruction should produce a triangle mesh")
         .mesh;
-    let raw_mesh = reconstruction.raw_reconstruction.mesh();
+    let raw_mesh = &reconstruction.raw_reconstruction.mesh;
     vtk_format::write_vtk(
         mesh,
         "../out/bunny_test_basic_pipeline_postprocessing_aabb.vtk",

splashsurf_lib/benches/benches/bench_full.rs

@@ -268,7 +268,7 @@ pub fn surface_reconstruction_double_dam_break_inplace(c: &mut Criterion) {
     group.finish();
 
     write_vtk(
-        reconstruction.mesh(),
+        reconstruction.mesh,
         "../out/reconstruct_surface_double_dam_break.vtk",
         "mesh",
     )

splashsurf_lib/benches/benches/bench_mesh.rs

@@ -36,7 +36,7 @@ fn reconstruct_particles<P: AsRef<Path>>(particle_file: P) -> SurfaceReconstruct
 pub fn mesh_vertex_normals(c: &mut Criterion) {
     //let reconstruction = reconstruct_particles("../../canyon_13353401_particles.vtk");
     let reconstruction = reconstruct_particles("../data/hilbert_46843_particles.bgeo");
-    let mesh = reconstruction.mesh();
+    let mesh = &reconstruction.mesh;
 
     let mut group = c.benchmark_group("mesh");
     group.sample_size(50);
@@ -56,7 +56,7 @@ pub fn mesh_vertex_normals(c: &mut Criterion) {
 pub fn mesh_vertex_normals_parallel(c: &mut Criterion) {
     //let reconstruction = reconstruct_particles("../../canyon_13353401_particles.vtk");
     let reconstruction = reconstruct_particles("../data/hilbert_46843_particles.bgeo");
-    let mesh = reconstruction.mesh();
+    let mesh = &reconstruction.mesh;
 
     let mut group = c.benchmark_group("mesh");
     group.sample_size(50);

splashsurf_lib/src/lib.rs

@@ -266,34 +266,6 @@ impl<I: Index, R: Real> Default for SurfaceReconstruction<I, R> {
     }
 }
 
-// TODO: Remove these functions
-impl<I: Index, R: Real> SurfaceReconstruction<I, R> {
-    /// Returns a reference to the surface mesh that is the result of the reconstruction
-    pub fn mesh(&self) -> &TriMesh3d<R> {
-        &self.mesh
-    }
-
-    /// Returns a reference to the global particle density vector if computed during the reconstruction (currently, all reconstruction approaches return this)
-    pub fn particle_densities(&self) -> Option<&Vec<R>> {
-        self.particle_densities.as_ref()
-    }
-
-    /// Returns a reference to the per input particle boolean vector indicating whether the particle was inside the specified AABB (if any)
-    pub fn particle_inside_aabb(&self) -> Option<&Vec<bool>> {
-        self.particle_inside_aabb.as_ref()
-    }
-
-    /// Returns a reference to the global list of per-particle neighborhood lists if computed during the reconstruction (`None` if not specified in the parameters)
-    pub fn particle_neighbors(&self) -> Option<&Vec<Vec<usize>>> {
-        self.particle_neighbors.as_ref()
-    }
-
-    /// Returns a reference to the virtual background grid that was used for marching cubes
-    pub fn grid(&self) -> &UniformGrid<I, R> {
-        &self.grid
-    }
-}
-
 impl<I: Index, R: Real> From<SurfaceReconstruction<I, R>> for TriMesh3d<R> {
     /// Extracts the reconstructed mesh
     fn from(result: SurfaceReconstruction<I, R>) -> Self {

splashsurf_lib/tests/integration_tests/test_full.rs

@@ -104,33 +104,33 @@ macro_rules! generate_test {
             let reconstruction =
                 reconstruct_surface::<i64, _>(particle_positions.as_slice(), &parameters).unwrap();
 
-            write_vtk(reconstruction.mesh(), &output_file, "mesh").unwrap();
+            write_vtk(&reconstruction.mesh, &output_file, "mesh").unwrap();
 
             println!(
                 "Reconstructed mesh '{}' from particle file '{}' with {} particles has {} triangles.",
                 output_file.display(),
                 $input_file,
                 particle_positions.len(),
-                reconstruction.mesh().triangles.len()
+                reconstruction.mesh.triangles.len()
             );
 
             // Ensure that the number of triangles is roughly correct
             assert!(
-                reconstruction.mesh().triangles.len() > $min_triangles,
+                reconstruction.mesh.triangles.len() > $min_triangles,
                 "Mesh has probably too few triangles (min expected: {}, is: {})",
                 $min_triangles,
-                reconstruction.mesh().triangles.len()
+                reconstruction.mesh.triangles.len()
             );
             assert!(
-                reconstruction.mesh().triangles.len() < $max_triangles,
+                reconstruction.mesh.triangles.len() < $max_triangles,
                 "Mesh has probably too many triangles (max expected: {}, is: {})",
                 $max_triangles,
-                reconstruction.mesh().triangles.len()
+                reconstruction.mesh.triangles.len()
             );
 
             if test_for_boundary(&parameters) {
                 // Ensure that the mesh does not have a boundary
-                if let Err(e) = check_mesh_consistency(reconstruction.grid(), reconstruction.mesh(), true, true, true)
+                if let Err(e) = check_mesh_consistency(&reconstruction.grid, &reconstruction.mesh, true, true, true)
                 {
                     eprintln!("{}", e);
                     panic!("Mesh contains topological/manifold errors");

splashsurf_lib/tests/integration_tests/test_simple.rs

@@ -76,28 +76,20 @@ fn test_edge_above_threshold_to_outside_of_compact_support_global() {
     println!(
         "Reconstructed mesh from {} particles has {} triangles.",
         particle_positions.len(),
-        reconstruction.mesh().triangles.len()
+        reconstruction.mesh.triangles.len()
     );
 
+    assert_eq!(reconstruction.mesh.vertices.len(), 6, "Number of vertices");
     assert_eq!(
-        reconstruction.mesh().vertices.len(),
-        6,
-        "Number of vertices"
-    );
-    assert_eq!(
-        reconstruction.mesh().triangles.len(),
+        reconstruction.mesh.triangles.len(),
         8,
         "Number of triangles"
     );
 
     // Ensure that the mesh does not have a boundary
-    if let Err(e) = check_mesh_consistency(
-        reconstruction.grid(),
-        reconstruction.mesh(),
-        true,
-        true,
-        true,
-    ) {
+    if let Err(e) =
+        check_mesh_consistency(&reconstruction.grid, &reconstruction.mesh, true, true, true)
+    {
         eprintln!("{}", e);
         panic!("Mesh contains topological/manifold errors");
     }
@@ -113,28 +105,20 @@ fn test_edge_above_threshold_to_outside_of_compact_support_subdomains() {
     println!(
         "Reconstructed mesh from {} particles has {} triangles.",
         particle_positions.len(),
-        reconstruction.mesh().triangles.len()
+        reconstruction.mesh.triangles.len()
     );
 
+    assert_eq!(reconstruction.mesh.vertices.len(), 6, "Number of vertices");
     assert_eq!(
-        reconstruction.mesh().vertices.len(),
-        6,
-        "Number of vertices"
-    );
-    assert_eq!(
-        reconstruction.mesh().triangles.len(),
+        reconstruction.mesh.triangles.len(),
         8,
         "Number of triangles"
     );
 
     // Ensure that the mesh does not have a boundary
-    if let Err(e) = check_mesh_consistency(
-        reconstruction.grid(),
-        reconstruction.mesh(),
-        true,
-        true,
-        true,
-    ) {
+    if let Err(e) =
+        check_mesh_consistency(&reconstruction.grid, &reconstruction.mesh, true, true, true)
+    {
         eprintln!("{}", e);
         panic!("Mesh contains topological/manifold errors");
     }