diff --git a/crates/fj-interop/src/mesh.rs b/crates/fj-interop/src/mesh.rs index 3427fd5b1..afa39519e 100644 --- a/crates/fj-interop/src/mesh.rs +++ b/crates/fj-interop/src/mesh.rs @@ -5,6 +5,7 @@ use std::{collections::HashMap, hash::Hash}; use fj_math::Point; /// A triangle mesh +#[derive(Debug)] pub struct Mesh { vertices: Vec, indices: Vec, diff --git a/crates/fj-kernel/src/algorithms/intersect/curve_face.rs b/crates/fj-kernel/src/algorithms/intersect/curve_face.rs index 1cb6391b1..43bc1b667 100644 --- a/crates/fj-kernel/src/algorithms/intersect/curve_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/curve_face.rs @@ -184,9 +184,9 @@ mod tests { #[rustfmt::skip] let interior = [ [-1., -1.], - [ 1., -1.], - [ 1., 1.], [-1., 1.], + [ 1., 1.], + [ 1., -1.], ]; let face = Face::builder(&stores, surface) diff --git a/crates/fj-kernel/src/algorithms/triangulate/mod.rs b/crates/fj-kernel/src/algorithms/triangulate/mod.rs index 5912fc12d..8ab4d2128 100644 --- a/crates/fj-kernel/src/algorithms/triangulate/mod.rs +++ b/crates/fj-kernel/src/algorithms/triangulate/mod.rs @@ -129,9 +129,9 @@ mod tests { let d = [0., 4.]; let e = [1., 1.]; - let f = [3., 1.]; + let f = [1., 2.]; let g = [3., 3.]; - let h = [1., 2.]; + let h = [3., 1.]; let surface = Surface::xy_plane(); let face = Face::builder(&stores, surface) @@ -141,17 +141,24 @@ mod tests { let triangles = triangulate(face)?; - let a = Point::from(a).to_xyz(); - let d = Point::from(d).to_xyz(); - let e = Point::from(e).to_xyz(); - let f = Point::from(f).to_xyz(); - let g = Point::from(g).to_xyz(); - let h = Point::from(h).to_xyz(); + let a = surface.point_from_surface_coords(a); + let b = surface.point_from_surface_coords(b); + let e = surface.point_from_surface_coords(e); + let f = surface.point_from_surface_coords(f); + let g = surface.point_from_surface_coords(g); + let h = surface.point_from_surface_coords(h); - // Should contain some triangles from the polygon. Don't need to test - // them all. - assert!(triangles.contains_triangle([a, e, h])); - assert!(triangles.contains_triangle([a, d, h])); + // Let's test that some correct triangles are present. We don't need to + // test them all. + // + // Please note that there are multiple valid triangulations of any given + // polygon. So if you change the input data above, or the algorithm, the + // following assertions might break. + // + // This limits the usefulness of this test. It would be better to have a + // smarter way of verifying the triangulation. + assert!(triangles.contains_triangle([a, b, e])); + assert!(triangles.contains_triangle([b, e, h])); // Shouldn't contain any possible triangle from the hole. assert!(!triangles.contains_triangle([e, f, g])); @@ -179,12 +186,12 @@ mod tests { // a ---------- b // - let a = Point::from([0., 0.]); - let b = Point::from([0.4, 0.]); - //let b = Point::from([0.5, 0.]); // test passes with this change - let c = Point::from([0.4, 1.0]); - let d = Point::from([0.1, 0.1]); - let e = Point::from([0., 0.8]); + let a = [0., 0.]; + let b = [0.4, 0.]; + //let b = [0.5, 0.]; // test passes with this change + let c = [0.4, 1.0]; + let d = [0.1, 0.1]; + let e = [0., 0.8]; let surface = Surface::xy_plane(); let face = Face::builder(&stores, surface) @@ -193,11 +200,11 @@ mod tests { let triangles = triangulate(face)?; - let a3 = a.to_xyz(); - let b3 = b.to_xyz(); - let c3 = c.to_xyz(); - let d3 = d.to_xyz(); - let e3 = e.to_xyz(); + let a3 = surface.point_from_surface_coords(a); + let b3 = surface.point_from_surface_coords(b); + let c3 = surface.point_from_surface_coords(c); + let d3 = surface.point_from_surface_coords(d); + let e3 = surface.point_from_surface_coords(e); assert!(triangles.contains_triangle([a3, b3, d3])); assert!(triangles.contains_triangle([b3, c3, d3])); diff --git a/crates/fj-kernel/src/objects/cycle.rs b/crates/fj-kernel/src/objects/cycle.rs index 81e5a24cf..d3e09f95a 100644 --- a/crates/fj-kernel/src/objects/cycle.rs +++ b/crates/fj-kernel/src/objects/cycle.rs @@ -95,6 +95,10 @@ impl Cycle { } /// Indicate the cycle's winding, assuming a right-handed coordinate system + /// + /// Please note that this is not *the* winding of the cycle, only one of the + /// two possible windings, depending on the direction you look at the + /// surface that the cycle is defined on from. pub fn winding(&self) -> Winding { // The cycle could be made up of one or two circles. If that is the // case, the winding of the cycle is determined by the winding of the diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index c0e283933..90ab3d615 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -8,6 +8,29 @@ use crate::{builder::FaceBuilder, stores::Stores}; use super::{Cycle, Surface}; /// A face of a shape +/// +/// A `Face` is a bounded area of a [`Surface`], the [`Surface`] itself being an +/// infinite 2-dimensional object in 3D space. `Face`s are bound by one exterior +/// cycle, which defines the outer boundary, and an arbitrary number of interior +/// cycles (i.e. holes). +/// +/// `Face` has a defined orientation, a front and a back side. When faces are +/// combined into [`Shell`]s, the face orientation defines what is inside and +/// outside of the shell. This stands in contrast to [`Surface`], which has no +/// defined orientation. +/// +/// You can look at a `Face` from two directions: front and back. The winding of +/// the exterior cycle will be clockwise or counter-clockwise, depending on your +/// perspective. The front side of the face, is the side where from which the +/// exterior cycle appear counter-clockwise. +/// +/// Interior cycles must have the opposite winding of the exterior cycle, +/// meaning on the front side of the face, they must appear clockwise. This +/// means that all [`HalfEdge`]s that bound a `Face` have the interior of the +/// face on their left side (on the face's front side). +/// +/// [`HalfEdge`]: super::HalfEdge +/// [`Shell`]: super::Shell #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Face { surface: Surface, @@ -47,18 +70,26 @@ impl Face { /// # Panics /// /// Panics, if the added cycles are not defined in the face's surface. + /// + /// Panics, if the winding of the interior cycles is not opposite that of + /// the exterior cycle. pub fn with_interiors( mut self, interiors: impl IntoIterator, ) -> Self { - for cycle in interiors.into_iter() { + for interior in interiors.into_iter() { assert_eq!( self.surface(), - cycle.surface(), + interior.surface(), "Cycles that bound a face must be in face's surface" ); + assert_ne!( + self.exterior().winding(), + interior.winding(), + "Interior cycles must have opposite winding of exterior cycle" + ); - self.interiors.push(cycle); + self.interiors.push(interior); } self @@ -77,7 +108,7 @@ impl Face { &self.surface } - /// Access the cycles that bound the face on the outside + /// Access the cycle that bounds the face on the outside pub fn exterior(&self) -> &Cycle { &self.exterior } @@ -108,7 +139,7 @@ impl Face { /// /// Faces *do* have an orientation, meaning they have definite front and /// back sides. The front side is the side, where the face's exterior cycle - /// is wound clockwise. + /// is wound counter-clockwise. pub fn coord_handedness(&self) -> Handedness { match self.exterior().winding() { Winding::Ccw => Handedness::RightHanded,