Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ js-sys = { version = "0.3.81", optional = true }
serde-wasm-bindgen = { version = "0.6.5", optional = true }
serde_json = { version = "1.0.145", optional = true }
serde = { version = "1.0.228", optional = true }
bytemuck = "1.24.0"

[target.'cfg(target_arch = "wasm32")'.dependencies]
uuid = { version = "1.18", features = ["js"] }
Expand Down
70 changes: 69 additions & 1 deletion src/mesh/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ use crate::float_types::{
use crate::mesh::{bsp::Node, plane::Plane, polygon::Polygon, vertex::Vertex};
use crate::sketch::Sketch;
use crate::traits::CSG;
use bytemuck::cast;
use geo::{CoordsIter, Geometry, Polygon as GeoPolygon};
use nalgebra::{
Isometry3, Matrix4, Point3, Quaternion, Unit, Vector3, partial_max, partial_min,
};
use std::{cmp::PartialEq, fmt::Debug, num::NonZeroU32, sync::OnceLock};
use std::{cmp::PartialEq, collections::HashMap, fmt::Debug, num::NonZeroU32, sync::OnceLock};

#[cfg(feature = "parallel")]
use rayon::{iter::IntoParallelRefIterator, prelude::*};
Expand Down Expand Up @@ -47,6 +48,18 @@ pub mod smoothing;
pub mod tpms;
pub mod vertex;

/// Stored as `(position: [f32; 3], normal: [f32; 3])`
pub type GraphicsMeshVertex = ([f32; 3], [f32; 3]);

/// Mesh ready for rendering. Uses f32s and provides vertices, indices and
/// normals.
#[derive(Debug, Clone)]
pub struct GraphicsMesh {
/// Vertices contain both position and normal
pub vertices: Vec<GraphicsMeshVertex>,
pub indices: Vec<u32>,
}

#[derive(Clone, Debug)]
pub struct Mesh<S: Clone + Send + Sync + Debug> {
/// 3D polygons for volumetric shapes
Expand Down Expand Up @@ -237,7 +250,62 @@ impl<S: Clone + Send + Sync + Debug> Mesh<S> {
dot.acos()
}

/// Converts a mesh to a graphics mesh which is more appropriate for rendering.
///
/// Allocates a hashmap and a storage vec for removing redundant vertices.
pub fn build_graphics_mesh(&self) -> GraphicsMesh {
let triangles = self.triangulate().polygons;

let triangle_count = triangles.len();

let mut indices: Vec<u32> = Vec::with_capacity(triangle_count);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The capacity for the indices vector is initialized to triangle_count, but it will eventually hold triangle_count * 3 elements. This will likely cause multiple reallocations for large meshes, impacting performance. Initializing it with the final capacity is more efficient.

Suggested change
let mut indices: Vec<u32> = Vec::with_capacity(triangle_count);
let mut indices: Vec<u32> = Vec::with_capacity(triangle_count * 3);

let mut vertices: Vec<GraphicsMeshVertex> = Vec::with_capacity(triangle_count);
const VERT_DIM_SIZE: usize = std::mem::size_of::<[f32; 3]>();
let mut vertices_hash: HashMap<([u8; VERT_DIM_SIZE], [u8; VERT_DIM_SIZE]), u32> =
HashMap::with_capacity(triangle_count);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The capacity for vertices and vertices_hash is set to triangle_count. For many meshes, the number of unique vertices differs significantly from the number of triangles. For example, in a closed manifold mesh, the number of vertices is typically about half the number of triangles. The PR description itself implies a vertex-to-triangle ratio of about 1.3. Using triangle_count as capacity may lead to unnecessary reallocations. A better heuristic, like triangle_count * 3 / 2, could provide a more reasonable starting capacity.

Suggested change
let mut vertices: Vec<GraphicsMeshVertex> = Vec::with_capacity(triangle_count);
const VERT_DIM_SIZE: usize = std::mem::size_of::<[f32; 3]>();
let mut vertices_hash: HashMap<([u8; VERT_DIM_SIZE], [u8; VERT_DIM_SIZE]), u32> =
HashMap::with_capacity(triangle_count);
let mut vertices: Vec<GraphicsMeshVertex> = Vec::with_capacity(triangle_count * 3 / 2);
const VERT_DIM_SIZE: usize = std::mem::size_of::<[f32; 3]>();
let mut vertices_hash: HashMap<([u8; VERT_DIM_SIZE], [u8; VERT_DIM_SIZE]), u32> =
HashMap::with_capacity(triangle_count * 3 / 2);


let mut i_new_vertex: u32 = 0;

for triangle in triangles {
for vertex in triangle.vertices {
let pos_x = vertex.pos.x as f32;
let pos_y = vertex.pos.y as f32;
let pos_z = vertex.pos.z as f32;

let norm_x = vertex.normal.x as f32;
let norm_y = vertex.normal.y as f32;
let norm_z = vertex.normal.z as f32;

let pos_xyz = [pos_x, pos_y, pos_z];
let norm_xyz = [norm_x, norm_y, norm_z];

let pos_xyz_bytes: [u8; std::mem::size_of::<[f32; 3]>()] = cast(pos_xyz);
let norm_xyz_bytes: [u8; std::mem::size_of::<[f32; 3]>()] = cast(norm_xyz);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This block can be simplified. nalgebra provides a cast method to convert between numeric types for points and vectors, which is cleaner than casting each component individually. Additionally, the VERT_DIM_SIZE constant defined on line 263 should be used here for consistency instead of calling std::mem::size_of again.

Suggested change
let pos_x = vertex.pos.x as f32;
let pos_y = vertex.pos.y as f32;
let pos_z = vertex.pos.z as f32;
let norm_x = vertex.normal.x as f32;
let norm_y = vertex.normal.y as f32;
let norm_z = vertex.normal.z as f32;
let pos_xyz = [pos_x, pos_y, pos_z];
let norm_xyz = [norm_x, norm_y, norm_z];
let pos_xyz_bytes: [u8; std::mem::size_of::<[f32; 3]>()] = cast(pos_xyz);
let norm_xyz_bytes: [u8; std::mem::size_of::<[f32; 3]>()] = cast(norm_xyz);
let pos_xyz: [f32; 3] = vertex.pos.cast::<f32>().coords.into();
let norm_xyz: [f32; 3] = vertex.normal.cast::<f32>().into();
let pos_xyz_bytes: [u8; VERT_DIM_SIZE] = cast(pos_xyz);
let norm_xyz_bytes: [u8; VERT_DIM_SIZE] = cast(norm_xyz);


let vertex_f32 = (pos_xyz, norm_xyz);
let vertex_f32_bytes = (pos_xyz_bytes, norm_xyz_bytes);

if let Some(i_vertex) = vertices_hash.get(&vertex_f32_bytes) {
indices.push(*i_vertex);
} else {
vertices_hash.insert(vertex_f32_bytes, i_new_vertex);
vertices.push(vertex_f32);

indices.push(i_new_vertex);

i_new_vertex += 1;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation performs a get and then potentially an insert on the HashMap, which results in two lookups for new vertices. Using the entry API is more idiomatic and efficient as it performs only one lookup. This also makes the code more concise.

                let vertex_f32_bytes = (pos_xyz_bytes, norm_xyz_bytes);

                let index = *vertices_hash.entry(vertex_f32_bytes).or_insert_with(|| {
                    let new_index = i_new_vertex;
                    vertices.push((pos_xyz, norm_xyz));
                    i_new_vertex += 1;
                    new_index
                });
                indices.push(index);

}
}

vertices.shrink_to_fit();

GraphicsMesh { vertices, indices }
}

/// Extracts vertices and indices from the Mesh's tessellated polygons.
///
/// Implementation does *not* remove redundant vertices.
pub fn get_vertices_and_indices(&self) -> (Vec<Point3<Real>>, Vec<[u32; 3]>) {
let tri_csg = self.triangulate();
let vertices = tri_csg
Expand Down