From 68abfc1c22c0beb6d8eba11d57acbb29b4837577 Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Mon, 4 Nov 2024 17:23:26 +0100 Subject: [PATCH] Address a few TODOs (#273) --- CHANGELOG.md | 1 + src/bounding_volume/bounding_sphere_convex.rs | 4 +- .../bounding_sphere_convex_polygon.rs | 4 +- .../bounding_sphere_segment.rs | 4 +- .../bounding_sphere_triangle.rs | 4 +- src/bounding_volume/bounding_sphere_utils.rs | 11 +-- src/lib.rs | 4 +- .../mass_properties_convex_polygon.rs | 2 - .../closest_points_line_line.rs | 2 +- src/query/epa/epa3.rs | 18 ++-- src/query/gjk/voronoi_simplex2.rs | 22 ++--- src/query/gjk/voronoi_simplex3.rs | 39 +++----- ...intersection_test_composite_shape_shape.rs | 99 +------------------ src/query/intersection_test/mod.rs | 4 +- src/transformation/vhacd/vhacd.rs | 32 +----- src/transformation/voxelization/voxel_set.rs | 25 +++++ src/utils/sdp_matrix.rs | 7 +- 17 files changed, 77 insertions(+), 205 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df312927..cbb3d64b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - `TriMesh::intersection_with_aabb` - `SharedShape::trimesh` - `SharedShape::trimesh_with_flags` +- `point_cloud_bounding_sphere_with_center` now returns a `BoundingSphere`. ## v0.17.1 diff --git a/src/bounding_volume/bounding_sphere_convex.rs b/src/bounding_volume/bounding_sphere_convex.rs index 9885d0f0..793bd3ec 100644 --- a/src/bounding_volume/bounding_sphere_convex.rs +++ b/src/bounding_volume/bounding_sphere_convex.rs @@ -14,8 +14,6 @@ impl ConvexPolyhedron { /// Computes the local-space bounding sphere of this convex polyhedron. #[inline] pub fn local_bounding_sphere(&self) -> BoundingSphere { - let (center, radius) = bounding_volume::details::point_cloud_bounding_sphere(self.points()); - - BoundingSphere::new(center, radius) + bounding_volume::details::point_cloud_bounding_sphere(self.points()) } } diff --git a/src/bounding_volume/bounding_sphere_convex_polygon.rs b/src/bounding_volume/bounding_sphere_convex_polygon.rs index fafd2518..1f1479a8 100644 --- a/src/bounding_volume/bounding_sphere_convex_polygon.rs +++ b/src/bounding_volume/bounding_sphere_convex_polygon.rs @@ -14,8 +14,6 @@ impl ConvexPolygon { /// Computes the local-space bounding sphere of this convex polygon. #[inline] pub fn local_bounding_sphere(&self) -> BoundingSphere { - let (center, radius) = bounding_volume::details::point_cloud_bounding_sphere(self.points()); - - BoundingSphere::new(center, radius) + bounding_volume::details::point_cloud_bounding_sphere(self.points()) } } diff --git a/src/bounding_volume/bounding_sphere_segment.rs b/src/bounding_volume/bounding_sphere_segment.rs index 4ebc42e5..4069280a 100644 --- a/src/bounding_volume/bounding_sphere_segment.rs +++ b/src/bounding_volume/bounding_sphere_segment.rs @@ -15,8 +15,6 @@ impl Segment { #[inline] pub fn local_bounding_sphere(&self) -> BoundingSphere { let pts = [self.a, self.b]; - let (center, radius) = bounding_volume::details::point_cloud_bounding_sphere(&pts[..]); - - BoundingSphere::new(center, radius) + bounding_volume::details::point_cloud_bounding_sphere(&pts[..]) } } diff --git a/src/bounding_volume/bounding_sphere_triangle.rs b/src/bounding_volume/bounding_sphere_triangle.rs index e99fe9c7..936a3435 100644 --- a/src/bounding_volume/bounding_sphere_triangle.rs +++ b/src/bounding_volume/bounding_sphere_triangle.rs @@ -15,8 +15,6 @@ impl Triangle { #[inline] pub fn local_bounding_sphere(&self) -> BoundingSphere { let pts = [self.a, self.b, self.c]; - let (center, radius) = bounding_volume::details::point_cloud_bounding_sphere(&pts[..]); - - BoundingSphere::new(center, radius) + bounding_volume::details::point_cloud_bounding_sphere(&pts[..]) } } diff --git a/src/bounding_volume/bounding_sphere_utils.rs b/src/bounding_volume/bounding_sphere_utils.rs index 238f15c3..0a257bea 100644 --- a/src/bounding_volume/bounding_sphere_utils.rs +++ b/src/bounding_volume/bounding_sphere_utils.rs @@ -2,13 +2,14 @@ use crate::math::{Point, Real}; use crate::utils; use na::{self, ComplexField}; +use super::BoundingSphere; + /// Computes the bounding sphere of a set of point, given its center. -// TODO: return a bounding sphere? #[inline] pub fn point_cloud_bounding_sphere_with_center( pts: &[Point], center: Point, -) -> (Point, Real) { +) -> BoundingSphere { let mut sqradius = 0.0; for pt in pts.iter() { @@ -18,13 +19,11 @@ pub fn point_cloud_bounding_sphere_with_center( sqradius = distance_squared } } - - (center, ComplexField::sqrt(sqradius)) + BoundingSphere::new(center, ComplexField::sqrt(sqradius)) } /// Computes a bounding sphere of the specified set of point. -// TODO: return a bounding sphere? #[inline] -pub fn point_cloud_bounding_sphere(pts: &[Point]) -> (Point, Real) { +pub fn point_cloud_bounding_sphere(pts: &[Point]) -> BoundingSphere { point_cloud_bounding_sphere_with_center(pts, utils::center(pts)) } diff --git a/src/lib.rs b/src/lib.rs index 38351293..ab47d350 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,7 +11,7 @@ the rust programming language. #![deny(unused_parens)] #![deny(non_upper_case_globals)] #![deny(unused_results)] -#![warn(missing_docs)] // TODO: deny this +#![warn(missing_docs)] #![warn(unused_imports)] #![allow(missing_copy_implementations)] #![allow(clippy::too_many_arguments)] // Maybe revisit this one later. @@ -20,7 +20,7 @@ the rust programming language. #![allow(clippy::type_complexity)] // Complains about closures that are fairly simple. #![doc(html_root_url = "http://docs.rs/parry/0.1.1")] #![cfg_attr(not(feature = "std"), no_std)] -#![cfg_attr(not(feature = "rkyv"), deny(unused_qualifications))] // TODO: deny that everytime +#![deny(unused_qualifications)] #[cfg(all( feature = "simd-is-enabled", diff --git a/src/mass_properties/mass_properties_convex_polygon.rs b/src/mass_properties/mass_properties_convex_polygon.rs index 28708caa..379366ef 100644 --- a/src/mass_properties/mass_properties_convex_polygon.rs +++ b/src/mass_properties/mass_properties_convex_polygon.rs @@ -1,5 +1,3 @@ -#![allow(dead_code)] // TODO: remove this - use crate::mass_properties::MassProperties; use crate::math::{Point, Real}; use crate::shape::Triangle; diff --git a/src/query/closest_points/closest_points_line_line.rs b/src/query/closest_points/closest_points_line_line.rs index 6e895752..9b6ef50a 100644 --- a/src/query/closest_points/closest_points_line_line.rs +++ b/src/query/closest_points/closest_points_line_line.rs @@ -71,7 +71,7 @@ pub fn closest_points_line_line_parameters_eps( } } -// TODO: can we re-used this for the segment/segment case? +// TODO: can we re-use this for the segment/segment case? /// Closest points between two segments. #[inline] pub fn closest_points_line_line( diff --git a/src/query/epa/epa3.rs b/src/query/epa/epa3.rs index 05afdd97..9f8bdc90 100644 --- a/src/query/epa/epa3.rs +++ b/src/query/epa/epa3.rs @@ -373,20 +373,16 @@ impl EPA { for edge in &self.silhouette { if !self.faces[edge.face_id].deleted { let new_face_id = self.faces.len(); - let new_face; - // TODO: NLL - { - let face_adj = &mut self.faces[edge.face_id]; - let pt_id1 = face_adj.pts[(edge.opp_pt_id + 2) % 3]; - let pt_id2 = face_adj.pts[(edge.opp_pt_id + 1) % 3]; + let face_adj = &mut self.faces[edge.face_id]; + let pt_id1 = face_adj.pts[(edge.opp_pt_id + 2) % 3]; + let pt_id2 = face_adj.pts[(edge.opp_pt_id + 1) % 3]; - let pts = [pt_id1, pt_id2, support_point_id]; - let adj = [edge.face_id, new_face_id + 1, new_face_id - 1]; - new_face = Face::new(&self.vertices, pts, adj); + let pts = [pt_id1, pt_id2, support_point_id]; + let adj = [edge.face_id, new_face_id + 1, new_face_id - 1]; + let new_face = Face::new(&self.vertices, pts, adj); - face_adj.adj[(edge.opp_pt_id + 1) % 3] = new_face_id; - } + face_adj.adj[(edge.opp_pt_id + 1) % 3] = new_face_id; self.faces.push(new_face.0); diff --git a/src/query/gjk/voronoi_simplex2.rs b/src/query/gjk/voronoi_simplex2.rs index 451eef92..e69d230a 100644 --- a/src/query/gjk/voronoi_simplex2.rs +++ b/src/query/gjk/voronoi_simplex2.rs @@ -98,11 +98,8 @@ impl VoronoiSimplex { self.proj[0] = 1.0; self.vertices[0].point } else if self.dim == 1 { - // TODO: NLL - let (proj, location) = { - let seg = Segment::new(self.vertices[0].point, self.vertices[1].point); - seg.project_local_point_and_get_location(&Point::::origin(), true) - }; + let (proj, location) = Segment::new(self.vertices[0].point, self.vertices[1].point) + .project_local_point_and_get_location(&Point::::origin(), true); match location { SegmentPointLocation::OnVertex(0) => { @@ -123,15 +120,12 @@ impl VoronoiSimplex { proj.point } else { assert!(self.dim == 2); - // TODO: NLL - let (proj, location) = { - let tri = Triangle::new( - self.vertices[0].point, - self.vertices[1].point, - self.vertices[2].point, - ); - tri.project_local_point_and_get_location(&Point::::origin(), true) - }; + let (proj, location) = Triangle::new( + self.vertices[0].point, + self.vertices[1].point, + self.vertices[2].point, + ) + .project_local_point_and_get_location(&Point::::origin(), true); match location { TrianglePointLocation::OnVertex(i) => { diff --git a/src/query/gjk/voronoi_simplex3.rs b/src/query/gjk/voronoi_simplex3.rs index 72c141b7..8742e58d 100644 --- a/src/query/gjk/voronoi_simplex3.rs +++ b/src/query/gjk/voronoi_simplex3.rs @@ -125,11 +125,8 @@ impl VoronoiSimplex { self.proj[0] = 1.0; self.vertices[0].point } else if self.dim == 1 { - // TODO: NLL - let (proj, location) = { - let seg = Segment::new(self.vertices[0].point, self.vertices[1].point); - seg.project_local_point_and_get_location(&Point::::origin(), true) - }; + let (proj, location) = Segment::new(self.vertices[0].point, self.vertices[1].point) + .project_local_point_and_get_location(&Point::::origin(), true); match location { SegmentPointLocation::OnVertex(0) => { @@ -150,15 +147,12 @@ impl VoronoiSimplex { proj.point } else if self.dim == 2 { - // TODO: NLL - let (proj, location) = { - let tri = Triangle::new( - self.vertices[0].point, - self.vertices[1].point, - self.vertices[2].point, - ); - tri.project_local_point_and_get_location(&Point::::origin(), true) - }; + let (proj, location) = Triangle::new( + self.vertices[0].point, + self.vertices[1].point, + self.vertices[2].point, + ) + .project_local_point_and_get_location(&Point::::origin(), true); match location { TrianglePointLocation::OnVertex(i) => { @@ -192,16 +186,13 @@ impl VoronoiSimplex { proj.point } else { assert!(self.dim == 3); - // TODO: NLL - let (proj, location) = { - let tetr = Tetrahedron::new( - self.vertices[0].point, - self.vertices[1].point, - self.vertices[2].point, - self.vertices[3].point, - ); - tetr.project_local_point_and_get_location(&Point::::origin(), true) - }; + let (proj, location) = Tetrahedron::new( + self.vertices[0].point, + self.vertices[1].point, + self.vertices[2].point, + self.vertices[3].point, + ) + .project_local_point_and_get_location(&Point::::origin(), true); match location { TetrahedronPointLocation::OnVertex(i) => { diff --git a/src/query/intersection_test/intersection_test_composite_shape_shape.rs b/src/query/intersection_test/intersection_test_composite_shape_shape.rs index 3bf4df59..c3a61375 100644 --- a/src/query/intersection_test/intersection_test_composite_shape_shape.rs +++ b/src/query/intersection_test/intersection_test_composite_shape_shape.rs @@ -1,14 +1,10 @@ -#![allow(deprecated)] // Silence warning until we actually remove IntersectionCompositeShapeShapeBestFirstVisitor - use crate::bounding_volume::SimdAabb; -use crate::math::{Isometry, Real, SimdReal, Vector, SIMD_WIDTH}; -use crate::partitioning::{ - SimdBestFirstVisitStatus, SimdBestFirstVisitor, SimdVisitStatus, SimdVisitor, -}; +use crate::math::{Isometry, Real, SIMD_WIDTH}; +use crate::partitioning::{SimdVisitStatus, SimdVisitor}; use crate::query::QueryDispatcher; use crate::shape::{Shape, TypedSimdCompositeShape}; use crate::utils::IsometryOpt; -use simba::simd::{SimdBool as _, SimdPartialOrd, SimdValue}; +use simba::simd::SimdBool as _; /// Intersection test between a composite shape (`Mesh`, `Compound`) and any other shape. pub fn intersection_test_composite_shape_shape( @@ -118,92 +114,3 @@ where SimdVisitStatus::MaybeContinue(mask) } } - -/// A visitor for checking if a composite-shape and a shape intersect. -#[deprecated(note = "Use IntersectionCompositeShapeShapeVisitor instead.")] -pub struct IntersectionCompositeShapeShapeBestFirstVisitor<'a, D: ?Sized, G1: ?Sized + 'a> { - msum_shift: Vector, - msum_margin: Vector, - - dispatcher: &'a D, - pos12: &'a Isometry, - g1: &'a G1, - g2: &'a dyn Shape, -} - -impl<'a, D, G1> IntersectionCompositeShapeShapeBestFirstVisitor<'a, D, G1> -where - D: ?Sized + QueryDispatcher, - G1: ?Sized + TypedSimdCompositeShape, -{ - /// Initialize a visitor for checking if a composite-shape and a shape intersect. - pub fn new( - dispatcher: &'a D, - pos12: &'a Isometry, - g1: &'a G1, - g2: &'a dyn Shape, - ) -> IntersectionCompositeShapeShapeBestFirstVisitor<'a, D, G1> { - let ls_aabb2 = g2.compute_aabb(pos12); - - IntersectionCompositeShapeShapeBestFirstVisitor { - dispatcher, - msum_shift: Vector::splat(-ls_aabb2.center().coords), - msum_margin: Vector::splat(ls_aabb2.half_extents()), - pos12, - g1, - g2, - } - } -} - -impl<'a, D, G1> SimdBestFirstVisitor - for IntersectionCompositeShapeShapeBestFirstVisitor<'a, D, G1> -where - D: ?Sized + QueryDispatcher, - G1: ?Sized + TypedSimdCompositeShape, -{ - type Result = (G1::PartId, bool); - - fn visit( - &mut self, - best: Real, - bv: &SimdAabb, - data: Option<[Option<&G1::PartId>; SIMD_WIDTH]>, - ) -> SimdBestFirstVisitStatus { - // Compute the minkowski sum of the two Aabbs. - let msum = SimdAabb { - mins: bv.mins + self.msum_shift + (-self.msum_margin), - maxs: bv.maxs + self.msum_shift + self.msum_margin, - }; - let dist = msum.distance_to_origin(); - let mask = dist.simd_lt(SimdReal::splat(best)); - - if let Some(data) = data { - let bitmask = mask.bitmask(); - let mut found_intersection = false; - - for (ii, data) in data.into_iter().enumerate() { - if (bitmask & (1 << ii)) != 0 && data.is_some() { - let part_id = *data.unwrap(); - self.g1.map_untyped_part_at(part_id, |part_pos1, g1, _| { - found_intersection = self.dispatcher.intersection_test( - &part_pos1.inv_mul(self.pos12), - g1, - self.g2, - ) == Ok(true); - }); - - if found_intersection { - return SimdBestFirstVisitStatus::ExitEarly(Some((part_id, true))); - } - } - } - } - - SimdBestFirstVisitStatus::MaybeContinue { - weights: dist, - mask, - results: [None; SIMD_WIDTH], - } - } -} diff --git a/src/query/intersection_test/mod.rs b/src/query/intersection_test/mod.rs index fceb5383..a3efc8de 100644 --- a/src/query/intersection_test/mod.rs +++ b/src/query/intersection_test/mod.rs @@ -6,11 +6,9 @@ pub use self::intersection_test_ball_point_query::{ intersection_test_ball_point_query, intersection_test_point_query_ball, }; #[cfg(feature = "std")] -// TODO: remove this once we get rid of IntersectionCompositeShapeShapeBestFirstVisitor -#[allow(deprecated)] pub use self::intersection_test_composite_shape_shape::{ intersection_test_composite_shape_shape, intersection_test_shape_composite_shape, - IntersectionCompositeShapeShapeBestFirstVisitor, IntersectionCompositeShapeShapeVisitor, + IntersectionCompositeShapeShapeVisitor, }; pub use self::intersection_test_cuboid_cuboid::intersection_test_cuboid_cuboid; pub use self::intersection_test_cuboid_segment::{ diff --git a/src/transformation/vhacd/vhacd.rs b/src/transformation/vhacd/vhacd.rs index c7885d3e..79271b82 100644 --- a/src/transformation/vhacd/vhacd.rs +++ b/src/transformation/vhacd/vhacd.rs @@ -164,32 +164,6 @@ impl VHACD { } } - // TODO: this should be a method of VoxelSet. - fn compute_axes_aligned_clipping_planes( - vset: &VoxelSet, - downsampling: u32, - planes: &mut Vec, - ) { - let min_v = vset.min_bb_voxels(); - let max_v = vset.max_bb_voxels(); - - for dim in 0..DIM { - let i0 = min_v[dim]; - let i1 = max_v[dim]; - - for i in (i0..=i1).step_by(downsampling as usize) { - let plane = CutPlane { - abc: Vector::ith(dim, 1.0), - axis: dim as u8, - d: -(vset.origin[dim] + (i as Real + 0.5) * vset.scale), - index: i, - }; - - planes.push(plane); - } - } - } - fn refine_axes_aligned_clipping_planes( vset: &VoxelSet, best_plane: &CutPlane, @@ -328,11 +302,7 @@ impl VHACD { Self::compute_preferred_cutting_direction(&eigenvalues); let mut planes = Vec::new(); - Self::compute_axes_aligned_clipping_planes( - &voxels, - params.plane_downsampling, - &mut planes, - ); + voxels.compute_axes_aligned_clipping_planes(params.plane_downsampling, &mut planes); let (mut best_plane, mut min_concavity) = self.compute_best_clipping_plane( &voxels, diff --git a/src/transformation/voxelization/voxel_set.rs b/src/transformation/voxelization/voxel_set.rs index c1619beb..5b37ff00 100644 --- a/src/transformation/voxelization/voxel_set.rs +++ b/src/transformation/voxelization/voxel_set.rs @@ -617,6 +617,31 @@ impl VoxelSet { cov_mat.symmetric_eigenvalues() } + + pub(crate) fn compute_axes_aligned_clipping_planes( + &self, + downsampling: u32, + planes: &mut Vec, + ) { + let min_v = self.min_bb_voxels(); + let max_v = self.max_bb_voxels(); + + for dim in 0..DIM { + let i0 = min_v[dim]; + let i1 = max_v[dim]; + + for i in (i0..=i1).step_by(downsampling as usize) { + let plane = CutPlane { + abc: Vector::ith(dim, 1.0), + axis: dim as u8, + d: -(self.origin[dim] + (i as Real + 0.5) * self.scale), + index: i, + }; + + planes.push(plane); + } + } + } } #[cfg(feature = "dim2")] diff --git a/src/utils/sdp_matrix.rs b/src/utils/sdp_matrix.rs index 50b4130b..0385a117 100644 --- a/src/utils/sdp_matrix.rs +++ b/src/utils/sdp_matrix.rs @@ -3,7 +3,8 @@ use na::{Matrix2, Matrix3, Matrix3x2, SimdRealField, Vector2, Vector3}; use std::ops::{Add, Mul}; #[cfg(feature = "rkyv")] -use rkyv::{bytecheck, CheckBytes}; +#[cfg(feature = "rkyv")] +use rkyv::{bytecheck, Archive, CheckBytes}; /// A 2x2 symmetric-definite-positive matrix. #[derive(Copy, Clone, Debug, PartialEq)] @@ -11,7 +12,7 @@ use rkyv::{bytecheck, CheckBytes}; #[cfg_attr( feature = "rkyv", derive(rkyv::Archive, rkyv::Deserialize, rkyv::Serialize, CheckBytes), - archive(as = "Self", bound(archive = "N: rkyv::Archive")) + archive(as = "Self", bound(archive = "N: Archive")) )] pub struct SdpMatrix2 { /// The component at the first row and first column of this matrix. @@ -122,7 +123,7 @@ impl Mul for SdpMatrix2 { #[cfg_attr( feature = "rkyv", derive(rkyv::Archive, rkyv::Deserialize, rkyv::Serialize, CheckBytes), - archive(as = "Self", bound(archive = "N: rkyv::Archive")) + archive(as = "Self", bound(archive = "N: Archive")) )] pub struct SdpMatrix3 { /// The component at the first row and first column of this matrix.