Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address a few TODOs #273

Merged
merged 9 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
4 changes: 1 addition & 3 deletions src/bounding_volume/bounding_sphere_convex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
4 changes: 1 addition & 3 deletions src/bounding_volume/bounding_sphere_convex_polygon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
4 changes: 1 addition & 3 deletions src/bounding_volume/bounding_sphere_segment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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[..])
}
}
4 changes: 1 addition & 3 deletions src/bounding_volume/bounding_sphere_triangle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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[..])
}
}
11 changes: 5 additions & 6 deletions src/bounding_volume/bounding_sphere_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Real>],
center: Point<Real>,
) -> (Point<Real>, Real) {
) -> BoundingSphere {
let mut sqradius = 0.0;

for pt in pts.iter() {
Expand All @@ -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<Real>]) -> (Point<Real>, Real) {
pub fn point_cloud_bounding_sphere(pts: &[Point<Real>]) -> BoundingSphere {
Vrixyz marked this conversation as resolved.
Show resolved Hide resolved
point_cloud_bounding_sphere_with_center(pts, utils::center(pts))
}
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
#![deny(missing_docs)]
Copy link
Member

Choose a reason for hiding this comment

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

Let’s keep this a warn and remove the TODO. I find it more convenient to just warn locally, and have it fail only on CI by denying warnings.

#![warn(unused_imports)]
#![allow(missing_copy_implementations)]
#![allow(clippy::too_many_arguments)] // Maybe revisit this one later.
Expand All @@ -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)] // TODO: deny that everytime
Vrixyz marked this conversation as resolved.
Show resolved Hide resolved

#[cfg(all(
feature = "simd-is-enabled",
Expand Down
2 changes: 0 additions & 2 deletions src/mass_properties/mass_properties_convex_polygon.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![allow(dead_code)] // TODO: remove this
Vrixyz marked this conversation as resolved.
Show resolved Hide resolved

use crate::mass_properties::MassProperties;
use crate::math::{Point, Real};
use crate::shape::Triangle;
Expand Down
2 changes: 1 addition & 1 deletion src/query/closest_points/closest_points_line_line.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub fn closest_points_line_line_parameters_eps<const D: usize>(
}
}

// 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(
Expand Down
2 changes: 2 additions & 0 deletions src/query/epa/epa3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,8 @@ impl EPA {
let new_face_id = self.faces.len();
let new_face;

// TODO: Thierry: We can probably remove that scope now, but I prefer to discuss it first.
// I assume NLL is for Non Lexical Lifetimes.
// TODO: NLL
{
let face_adj = &mut self.faces[edge.face_id];
Vrixyz marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
2 changes: 1 addition & 1 deletion src/query/ray/ray.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pub struct RayIntersection {
/// Otherwise, the normal points outward.
///
/// If the `time_of_impact` is exactly zero, the normal might not be reliable.
// TODO: use a Unit<Vector> instead.
// TODO: use a Unit<Vector> instead. // TODO: Thierry: should we use Unit for [`Ray::dir`] too ?
pub normal: Vector<Real>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not entirely trivial to do ; and will have API impacts


/// Feature at the intersection point.
Expand Down
32 changes: 1 addition & 31 deletions src/transformation/vhacd/vhacd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<CutPlane>,
) {
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,
Expand Down Expand Up @@ -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,
Expand Down
25 changes: 25 additions & 0 deletions src/transformation/voxelization/voxel_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,31 @@ impl VoxelSet {

cov_mat.symmetric_eigenvalues()
}

pub(crate) fn compute_axes_aligned_clipping_planes(
&self,
downsampling: u32,
planes: &mut Vec<CutPlane>,
) {
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")]
Expand Down
7 changes: 4 additions & 3 deletions src/utils/sdp_matrix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@ 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)]
#[cfg_attr(feature = "serde-serialize", derive(Serialize, Deserialize))]
#[cfg_attr(
feature = "rkyv",
derive(rkyv::Archive, rkyv::Deserialize, rkyv::Serialize, CheckBytes),
archive(as = "Self", bound(archive = "N: rkyv::Archive<Archived = N>"))
archive(as = "Self", bound(archive = "N: Archive<Archived = N>"))
)]
pub struct SdpMatrix2<N> {
/// The component at the first row and first column of this matrix.
Expand Down Expand Up @@ -122,7 +123,7 @@ impl Mul<Real> for SdpMatrix2<Real> {
#[cfg_attr(
feature = "rkyv",
derive(rkyv::Archive, rkyv::Deserialize, rkyv::Serialize, CheckBytes),
archive(as = "Self", bound(archive = "N: rkyv::Archive<Archived = N>"))
archive(as = "Self", bound(archive = "N: Archive<Archived = N>"))
)]
pub struct SdpMatrix3<N> {
/// The component at the first row and first column of this matrix.
Expand Down