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.

Sign up for GitHub

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

utils3d: Supports dim != 3 #4

Merged
merged 1 commit into from
Feb 16, 2024
Merged

utils3d: Supports dim != 3 #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

extern crate alloc;

pub mod utils_3d;
pub mod utils3d;

use alloc::vec::Vec;
use core::ptr;
Expand Down
64 changes: 51 additions & 13 deletions src/utils_3d.rs → src/utils3d.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,19 @@ fn cross<T: Float + Debug>((ax, ay, az): (T, T, T), (bx, by, bz): (T, T, T)) ->
(ay * bz - az * by, az * bx - ax * bz, ax * by - ay * bx)
}

fn normal<T: Float + Debug>(vertices: &[T]) -> Option<(T, T, T)> {
fn normal<T: Float + Debug>(vertices: &[T], dim: usize) -> Option<(T, T, T)> {
let len = vertices.len();
if len < 9 {
if len < dim * 3 {
// At least 3 vertices required
return None;
}
let last_point = (vertices[len - 3], vertices[len - 2], vertices[len - 1]);
let last_point = (
vertices[len - dim],
vertices[len - dim + 1],
vertices[len - dim + 2],
);

let (sum, _) = vertices.chunks_exact(3).fold(
let (sum, _) = vertices.chunks_exact(dim).fold(
((T::zero(), T::zero(), T::zero()), last_point),
|(acc, prev), data| {
let (x, y, z) = (data[0], data[1], data[2]);
Comment on lines 7 to 25
Copy link

@coderabbitai coderabbitai bot Feb 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [22-34]

The calculation within the normal function uses a fold operation to accumulate cross products of vertex differences. This logic appears sound for computing a normal vector in 3D space. However, given the introduction of variable dimensions, it's crucial to verify that this approach remains valid for dimensions other than 3. Specifically, the cross product and subsequent operations assume a 3D context, which may not generalize to other dimensions without modification.

Consider verifying the mathematical correctness of this approach for dimensions other than 3 or providing additional documentation on the expected behavior and limitations.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is supposed to be used with data like position (3 dims) + texture uv (2 dims), and it just ignores non-positional dimentions.

Copy link

Choose a reason for hiding this comment

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

@ciscorn Thank you for the clarification. It's clear now that the normal function is designed to work with 3D positional data, even when additional dimensions (like texture UV coordinates) are present, by focusing on the first three dimensions and ignoring the rest. This approach ensures the mathematical operations remain valid for 3D space.

It might be beneficial to include comments or documentation in the code to explicitly state this intended behavior, ensuring that future developers understand how the function handles data with dimensions beyond 3D positional information.

Expand All @@ -36,9 +40,10 @@ fn normal<T: Float + Debug>(vertices: &[T]) -> Option<(T, T, T)> {
pub fn project3d_to_2d<T: Float + Debug>(
vertices: &[T],
num_outer: usize,
dim: usize,
buf: &mut Vec<T>,
) -> bool {
let Some((nx, ny, nz)) = normal(&vertices[0..num_outer * 3]) else {
let Some((nx, ny, nz)) = normal(&vertices[0..num_outer * dim], dim) else {
return false;
};
buf.clear();
Expand All @@ -47,10 +52,10 @@ pub fn project3d_to_2d<T: Float + Debug>(
if dd < T::from(1e-15).unwrap() {
if nz > T::zero() {
// do nothing
buf.extend(vertices.chunks_exact(3).flat_map(|d| [d[0], d[1]]))
buf.extend(vertices.chunks_exact(dim).flat_map(|d| [d[0], d[1]]))
} else {
// flip
buf.extend(vertices.chunks_exact(3).flat_map(|d| [d[1], d[0]]))
buf.extend(vertices.chunks_exact(dim).flat_map(|d| [d[1], d[0]]))
}
} else {
// rotation
Expand All @@ -68,7 +73,7 @@ pub fn project3d_to_2d<T: Float + Debug>(
let m21 = s;
let m22 = ay * ay * (T::one() - cost) + cost;
let m23 = u;
buf.extend(vertices.chunks_exact(3).flat_map(|d| {
buf.extend(vertices.chunks_exact(dim).flat_map(|d| {
let (x, y, z) = (d[0], d[1], d[2]);
[(x * m11 + y * m12 + z * m13), (x * m21 + y * m22 + z * m23)]
}))
Expand All @@ -82,39 +87,72 @@ mod test {

#[test]
fn test_do_nothing() {
// dim=3
let mut buf = Vec::new();
let vertices = &[0.0, 0.0, 0.0, 2.0, 0.0, 0.0, 2.0, 2.0, 0.0];
assert!(project3d_to_2d(vertices, 3, &mut buf));
assert!(project3d_to_2d(vertices, 3, 3, &mut buf));
assert!(buf == [0., 0., 2., 0., 2., 2.]);

// dim=5
let mut buf = Vec::new();
let vertices = &[
0.0, 0.0, 0.0, 99.0, 99.0, 2.0, 0.0, 0.0, 99.0, 99.0, 2.0, 2.0, 0.0, 99.0, 99.0,
];
assert!(project3d_to_2d(vertices, 3, 5, &mut buf));
assert!(buf == [0., 0., 2., 0., 2., 2.]);
}

#[test]
fn test_flip() {
// dim=3
let mut buf = Vec::new();
let vertices = &[0.0, 0.0, 0.0, 2.0, 2.0, 0.0, 2.0, 0.0, 0.0];
assert!(project3d_to_2d(vertices, 3, &mut buf));
assert!(project3d_to_2d(vertices, 3, 3, &mut buf));
assert!(buf == [0., 0., 2., 2., 0., 2.]);

// dim=4
let mut buf = Vec::new();
let vertices = &[
0.0, 0.0, 0.0, 99.0, 2.0, 2.0, 0.0, 99.0, 2.0, 0.0, 0.0, 99.0,
];
assert!(project3d_to_2d(vertices, 3, 4, &mut buf));
assert!(buf == [0., 0., 2., 2., 0., 2.]);
}

#[test]
fn test_rotate() {
// dim=3
let mut buf = Vec::new();
let vertices = &[0.0, 0.0, 0.0, 0.0, 0.0, 2.0, 0.0, 2.0, 2.0];
assert!(project3d_to_2d(vertices, 3, &mut buf));
assert!(project3d_to_2d(vertices, 3, 3, &mut buf));
assert!(buf == [0., 0., 2., 0., 2., 2.]);

// dim=4
let mut buf = Vec::new();
let vertices = &[
0.0, 0.0, 0.0, 99.0, 0.0, 0.0, 2.0, 99.0, 0.0, 2.0, 2.0, 99.0,
];
assert!(project3d_to_2d(vertices, 3, 4, &mut buf));
assert!(buf == [0., 0., 2., 0., 2., 2.]);
}

#[test]
fn test_invalid_input1() {
// dim=3
let mut buf = Vec::new();
let vertices = &[0., 0., 1., 1.];
assert!(!project3d_to_2d(vertices, 1, &mut buf));
assert!(!project3d_to_2d(vertices, 1, 3, &mut buf));

// dim=4
let mut buf = Vec::new();
let vertices = &[0., 0., 1., 1., 1.];
assert!(!project3d_to_2d(vertices, 1, 4, &mut buf));
}

#[test]
fn test_invalid_input2() {
// when normal is zero vector
let vertices = &[0., 0., 0., 0., 1., 0., 0., 0., 0., 0., 0., 1., 0., 0., 0.];
assert!(normal(vertices).is_none());
assert!(normal(vertices, 3).is_none());
}
}
Loading