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

Manual refactoring #23

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Manual refactoring #23

wants to merge 1 commit into from

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

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.

@@ -83,8 +83,7 @@ impl Geometry for Context {
&mut self,
tangent: [f32; 3],
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