Skip to content

Conversation

@lain-dono
Copy link

The code has become more ideomatic.
There are no warnings.
There is only one unsafe left.
There are no external dependencies.
Required compiler version upgraded to 1.56. Edition 2021!

Perhaps it makes sense to update the API and release a new version.

Comment on lines +1 to +106
#[derive(Clone, Copy, PartialEq)]
pub struct Vec3 {
pub x: f32,
pub y: f32,
pub z: f32,
}

impl Vec3 {
#[inline]
pub fn not_zero(x: f32) -> bool {
x.abs() > f32::MIN_POSITIVE
}

pub const fn zero() -> Self {
Self::new(0.0, 0.0, 0.0)
}

pub const fn new(x: f32, y: f32, z: f32) -> Self {
Self { x, y, z }
}

#[inline]
pub fn safe_normalize(self) -> Self {
if Self::not_zero(self.x) || Self::not_zero(self.y) || Self::not_zero(self.z) {
self.normalize()
} else {
self
}
}

#[inline]
pub fn normalize(self) -> Self {
self * (1.0 / self.length())
}

#[inline]
pub fn length(self) -> f32 {
self.length_squared().sqrt()
}

#[inline]
pub fn length_squared(self) -> f32 {
(self.x * self.x) + (self.y * self.y) + (self.z * self.z)
}

#[inline]
pub fn dot(&self, other: Self) -> f32 {
(self.x * other.x) + (self.y * other.y) + (self.z * other.z)
}
}

impl From<[f32; 3]> for Vec3 {
fn from([x, y, z]: [f32; 3]) -> Self {
Self { x, y, z }
}
}

impl From<Vec3> for [f32; 3] {
fn from(v: Vec3) -> Self {
[v.x, v.y, v.z]
}
}

impl core::ops::Add for Vec3 {
type Output = Self;
fn add(self, rhs: Self) -> Self {
Self {
x: self.x + rhs.x,
y: self.y + rhs.y,
z: self.z + rhs.z,
}
}
}

impl core::ops::Sub for Vec3 {
type Output = Self;
fn sub(self, rhs: Self) -> Self {
Self {
x: self.x - rhs.x,
y: self.y - rhs.y,
z: self.z - rhs.z,
}
}
}

impl core::ops::Mul<Vec3> for f32 {
type Output = Vec3;
fn mul(self, rhs: Vec3) -> Vec3 {
Vec3 {
x: rhs.x * self,
y: rhs.y * self,
z: rhs.z * self,
}
}
}

impl core::ops::Mul<f32> for Vec3 {
type Output = Self;
fn mul(self, rhs: f32) -> Self {
Self {
x: self.x * rhs,
y: self.y * rhs,
z: self.z * rhs,
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Is any of this used anywhere? I'm concerned about introducing yet another vector library.

bi_tangent: [f32; 3],
mag_s: f32,
mag_t: f32,
[mag_s, mag_t]: [f32; 2],
Copy link
Member

Choose a reason for hiding this comment

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

An interesting feature I haven't seen before. :)

Comment on lines +72 to +75
ctl_pts.push(ControlPoint {
uv: [0.0, 0.0],
dir: [1.0, -1.0, 1.0],
});
Copy link
Member

Choose a reason for hiding this comment

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

Are the formatting changes generated by something?

groups: &[Group],
indices: &[usize],
thres_cos: f32,
group_triange_buffer: &[usize],
Copy link
Member

Choose a reason for hiding this comment

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

Is this meant to be group_triangle_buffer?

triangles: &[Triangle],
groups: &[Group],
indices: &[usize],
thres_cos: f32,
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this is trying to say threshold_cos (as in cosine)

Comment on lines 583 to 584
// ///////////////////////////////////////////////////////////////////////////////////////////////////
/////////////////////////////////////////////////////////////////////////////////////////////////////
Copy link
Member

Choose a reason for hiding this comment

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

Whilst you're at it, would you mind removing these comments?

mut psTriInfos: *mut STriInfo,
iMyTriIndex: i32,
mut pGroup: *mut SGroup,
fn assign_recur(
Copy link
Member

Choose a reason for hiding this comment

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

Please expand: assign_recursive

triangles: &mut [Triangle],
index: usize,
group: &mut Group,
group_triange_buffer: &mut [usize],
Copy link
Member

Choose a reason for hiding this comment

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

Same as earlier

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants