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

Math fixes #545

Merged
merged 9 commits into from
May 6, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 0 additions & 6 deletions crates/fj-kernel/src/algorithms/triangulation/delaunay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ pub fn triangulate(
let triangle = match orientation {
Winding::Ccw => [v0, v1, v2],
Winding::Cw => [v0, v2, v1],
Winding::None => {
panic!(
"Triangle returned from triangulation isn't actually a \
triangle"
);
}
};

triangles.push(triangle);
Expand Down
2 changes: 1 addition & 1 deletion crates/fj-math/src/point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl<const D: usize> Point<D> {
}
}

/// Gives the distance between two Points.
/// Gives the distance between two points.
pub fn distance(p1: &Point<D>, p2: &Point<D>) -> Scalar {
(p1.coords - p2.coords).magnitude()
}
Expand Down
23 changes: 14 additions & 9 deletions crates/fj-math/src/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ use std::ops;

use nalgebra::Perspective3;

use crate::Scalar;

use super::{Aabb, Point, Segment, Triangle, Vector};

/// A transform
/// An affine transform
#[repr(C)]
#[derive(Debug, Clone, Copy)]
pub struct Transform(nalgebra::Transform<f64, nalgebra::TAffine, 3>);
Expand Down Expand Up @@ -81,21 +83,24 @@ impl Transform {
))
}

/// Project transform according to camera specfication, return data as a slice.
/// Project transform according to camera specfication, return data as an array.
/// Used primarily for graphics code.
pub fn project_to_slice(
pub fn project_to_array(
&self,
aspect_ratio: f64,
fovy: f64,
znear: f64,
zfar: f64,
) -> [f64; 16] {
) -> [Scalar; 16] {
let projection = Perspective3::new(aspect_ratio, fovy, znear, zfar);
let mut res = [0f64; 16];
res.copy_from_slice(
(projection.to_projective() * self.0).matrix().as_slice(),
);
res
(projection.to_projective() * self.0)
.matrix()
.as_slice()
.iter()
.map(|f| Scalar::from(*f))
.collect::<Vec<Scalar>>()
Copy link
Owner

Choose a reason for hiding this comment

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

It's a bit unfortunate that we're going through a Vec here, due to the heap allocation that's probably expensive. I don't know how to change it off the top of my head, however. I'd have to experiment with it hands-on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also wanted to avoid a Vec here. You can't collect an iterator into an array, though. One alternative way to implement this is to iterate over the slice elements and assign into some mutable array (not as pretty, but avoids the Vec)

Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, why not leave the code as it was before, with a slight modification:

let projection = Perspective3::new(aspect_ratio, fovy, znear, zfar);
let mut res = [0f64; 16];
res.copy_from_slice(
    (projection.to_projective() * self.0).matrix().as_slice(),
);
res.map(|f| Scalar::from(*f))

Should work?

But yeah, it's not that important. I'd be happy to merge a change that avoids the Vec, but I'm also fine with leaving it as-is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be a good way to avoid the Vec while still keeping things tidy. I don't think this will make or break the performance but it'll be good to keep in mind for later.

Copy link
Owner

Choose a reason for hiding this comment

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

Agreed. No reason to bend over backwards right now. If there's really a problem, we can fix it.

.try_into()
.unwrap()
}

/// Transform the given axis-aligned bounding box
Expand Down
52 changes: 26 additions & 26 deletions crates/fj-math/src/triangle.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,9 @@
use crate::Vector;

use super::{Point, Scalar};

use parry2d_f64::utils::point_in_triangle::{corner_direction, Orientation};
use parry3d_f64::query::{Ray, RayCast as _};

/// Winding direction of a triangle.
pub enum Winding {
/// Counter-clockwise
Ccw,
/// Clockwise
Cw,
/// Neither (straight lines)
None,
}
use crate::Vector;

impl From<Orientation> for Winding {
fn from(o: Orientation) -> Self {
match o {
Orientation::Ccw => Winding::Ccw,
Orientation::Cw => Winding::Cw,
Orientation::None => Winding::None,
}
}
}
use super::{Point, Scalar};

/// A triangle
///
Expand Down Expand Up @@ -79,8 +59,8 @@ impl<const D: usize> Triangle<D> {
impl Triangle<2> {
/// Returns the direction of the line through the points of the triangle.
pub fn winding_direction(&self) -> Winding {
let [v0, v1, v2] = self.points;
corner_direction(&v0.to_na(), &v1.to_na(), &v2.to_na()).into()
let [v0, v1, v2] = self.points.map(|point| point.to_na());
corner_direction(&v0, &v1, &v2).into()
}
}

Expand All @@ -97,13 +77,15 @@ impl Triangle<3> {
dir: Vector<3>,
max_toi: f64,
solid: bool,
) -> Option<f64> {
) -> Option<Scalar> {
let ray = Ray {
origin: origin.to_na(),
dir: dir.to_na(),
};

self.to_parry().cast_local_ray(&ray, max_toi, solid)
self.to_parry()
.cast_local_ray(&ray, max_toi, solid)
.map(|f| f.into())
}
}

Expand All @@ -116,6 +98,24 @@ where
}
}

/// Winding direction of a triangle.
pub enum Winding {
/// Counter-clockwise
Ccw,
/// Clockwise
Cw,
}

impl From<Orientation> for Winding {
fn from(o: Orientation) -> Self {
match o {
Orientation::Ccw => Winding::Ccw,
Orientation::Cw => Winding::Cw,
Orientation::None => unreachable!("not a triangle"),
}
}
}

#[cfg(test)]
mod tests {
use crate::Point;
Expand Down
10 changes: 5 additions & 5 deletions crates/fj-viewer/src/camera.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,11 @@ impl Camera {
far_plane: Self::DEFAULT_FAR_PLANE,

rotation: Transform::identity(),
translation: Transform::translation(Vector::from_components_f64([
initial_offset.x.into_f64(),
initial_offset.y.into_f64(),
-initial_distance.into_f64(),
])),
translation: Transform::translation([
initial_offset.x,
initial_offset.y,
-initial_distance,
]),
}
}

Expand Down
11 changes: 5 additions & 6 deletions crates/fj-viewer/src/graphics/transform.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
use bytemuck::{Pod, Zeroable};

use crate::camera::Camera;
use fj_math::Transform as fjTransform;

#[derive(Clone, Copy, Pod, Zeroable)]
#[repr(transparent)]
pub struct Transform(pub [f32; 16]);

impl Transform {
pub fn identity() -> Self {
Self::from(&fjTransform::identity())
Self::from(&fj_math::Transform::identity())
}

/// Compute transform used for vertices
Expand All @@ -18,14 +17,14 @@ impl Transform {
pub fn for_vertices(camera: &Camera, aspect_ratio: f64) -> Self {
let field_of_view_in_y = camera.field_of_view_in_x() / aspect_ratio;

let transform = camera.camera_to_model().project_to_slice(
let transform = camera.camera_to_model().project_to_array(
aspect_ratio,
field_of_view_in_y,
camera.near_plane(),
camera.far_plane(),
);

Self(transform.map(|val| val as f32))
Self(transform.map(|scalar| scalar.into_f32()))
}

/// Compute transform used for normals
Expand All @@ -39,8 +38,8 @@ impl Transform {
}
}

impl From<&fjTransform> for Transform {
fn from(other: &fjTransform) -> Self {
impl From<&fj_math::Transform> for Transform {
fn from(other: &fj_math::Transform) -> Self {
let mut native = [0.0; 16];
native.copy_from_slice(other.data());

Expand Down
6 changes: 1 addition & 5 deletions crates/fj-viewer/src/graphics/vertices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,7 @@ impl Vertices {
color: [f32; 4],
) {
let line = line.into_iter().map(|point| Vertex {
position: [
point.x.into_f32(),
point.y.into_f32(),
point.z.into_f32(),
],
position: point.coords.components.map(|scalar| scalar.into_f32()),
normal,
color,
});
Expand Down
13 changes: 7 additions & 6 deletions crates/fj-viewer/src/input/movement.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use fj_math::{Point, Transform, Vector};
use fj_math::{Point, Scalar, Transform, Vector};
use winit::dpi::PhysicalPosition;

use crate::{
Expand Down Expand Up @@ -49,11 +49,12 @@ impl Movement {
let diff = (cursor - previous) * d2 / d1;
let offset = camera.camera_to_model().transform_vector(&diff);

camera.translation =
camera.translation
* Transform::translation(Vector::from_components_f64(
[offset.x.into(), offset.y.into(), 0.0],
));
camera.translation = camera.translation
* Transform::translation(Vector::from([
offset.x,
offset.y,
Scalar::ZERO,
]));
}
}

Expand Down