From be99e721cc12e59dafc3b1587eeb39729299c0b1 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 20 Apr 2023 11:22:32 +0200 Subject: [PATCH 1/6] Document assumptions of `BuildShell::tetrahedron` The implementation doesn't match the documentation yet. I'm working on rectifying that. --- crates/fj-kernel/src/operations/build/shell.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/crates/fj-kernel/src/operations/build/shell.rs b/crates/fj-kernel/src/operations/build/shell.rs index a5b54957a..80ba1f630 100644 --- a/crates/fj-kernel/src/operations/build/shell.rs +++ b/crates/fj-kernel/src/operations/build/shell.rs @@ -12,6 +12,23 @@ use super::{BuildFace, Triangle}; /// Build a [`Shell`] pub trait BuildShell { /// Build a tetrahedron from the provided points + /// + /// Accepts 4 points, naturally. For the purposes of the following + /// discussion, let's call those `a`, `b`, `c`, and `d`, and assume that the + /// order they are listed in here matches the order they are provided in + /// within the array. + /// + /// Assumes that `a`, `b`, and `c` form a triangle in counter-clockwise + /// order, when arranging the viewpoint such that it is on the opposite side + /// of the triangle from `d`. If this assumption is met, the orientation of + /// all faces of the tetrahedron will be valid, meaning their + /// counter-clockwise sides are outside. + /// + /// # Implementation Note + /// + /// In principle, this method doesn't need to make assumptions about the + /// order of the points provided. It could, given some extra effort, just + /// build a correct tetrahedron, regardless of that order. fn tetrahedron( points: [impl Into>; 4], objects: &mut Service, From a42362a0bdeda407b98e0d3d13660746c11d3be0 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 20 Apr 2023 11:23:22 +0200 Subject: [PATCH 2/6] Update calls to `BuildShell::tetrahedron` They now match the assumption laid out in its documentation. --- crates/fj-kernel/src/validate/shell.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index 11d2815b9..5238d2731 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -207,7 +207,7 @@ mod tests { let mut services = Services::new(); let valid = Shell::tetrahedron( - [[0., 0., 0.], [1., 0., 0.], [0., 1., 0.], [0., 0., 1.]], + [[0., 0., 0.], [0., 1., 0.], [1., 0., 0.], [0., 0., 1.]], &mut services.objects, ); let invalid = valid.shell.update_face(&valid.face_abc, |face| { @@ -240,7 +240,7 @@ mod tests { let mut services = Services::new(); let valid = Shell::tetrahedron( - [[0., 0., 0.], [1., 0., 0.], [0., 1., 0.], [0., 0., 1.]], + [[0., 0., 0.], [0., 1., 0.], [1., 0., 0.], [0., 0., 1.]], &mut services.objects, ); let invalid = valid.shell.remove_face(&valid.face_abc); From 2c28acdb508ba6ab00792440b0c7f7a82c201023 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 20 Apr 2023 11:30:26 +0200 Subject: [PATCH 3/6] Update `BuildShell::tetrahedron` Now the implementation matches the documented assumptions. This leaves some variable names not-quite-right. I'm going to deal with that in a minute. --- crates/fj-kernel/src/operations/build/shell.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/operations/build/shell.rs b/crates/fj-kernel/src/operations/build/shell.rs index 80ba1f630..6fdee81bc 100644 --- a/crates/fj-kernel/src/operations/build/shell.rs +++ b/crates/fj-kernel/src/operations/build/shell.rs @@ -41,14 +41,14 @@ pub trait BuildShell { } = Face::triangle([a, b, c], [None, None, None], objects); let Triangle { face: face_abd, - edges: [_, bd, da], - } = Face::triangle([a, b, d], [Some(ab), None, None], objects); + edges: [_, da, bd], + } = Face::triangle([b, a, d], [Some(ab), None, None], objects); let Triangle { face: face_cad, edges: [_, _, dc], - } = Face::triangle([c, a, d], [Some(ca), Some(da), None], objects); + } = Face::triangle([d, a, c], [Some(da), Some(ca), None], objects); let Triangle { face: face_bcd, .. } = - Face::triangle([b, c, d], [Some(bc), Some(dc), Some(bd)], objects); + Face::triangle([c, b, d], [Some(bc), Some(bd), Some(dc)], objects); let faces = [face_abc, face_abd, face_cad, face_bcd] .map(|face| face.insert(objects)); From 13ddcb14dfe7d36a2d9b8df8dba2c428009e59e5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 20 Apr 2023 11:31:48 +0200 Subject: [PATCH 4/6] Update variable name to match edge direction --- crates/fj-kernel/src/operations/build/shell.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/operations/build/shell.rs b/crates/fj-kernel/src/operations/build/shell.rs index 6fdee81bc..0a3857bfa 100644 --- a/crates/fj-kernel/src/operations/build/shell.rs +++ b/crates/fj-kernel/src/operations/build/shell.rs @@ -41,12 +41,12 @@ pub trait BuildShell { } = Face::triangle([a, b, c], [None, None, None], objects); let Triangle { face: face_abd, - edges: [_, da, bd], + edges: [_, ad, bd], } = Face::triangle([b, a, d], [Some(ab), None, None], objects); let Triangle { face: face_cad, edges: [_, _, dc], - } = Face::triangle([d, a, c], [Some(da), Some(ca), None], objects); + } = Face::triangle([d, a, c], [Some(ad), Some(ca), None], objects); let Triangle { face: face_bcd, .. } = Face::triangle([c, b, d], [Some(bc), Some(bd), Some(dc)], objects); From 732bd5939bd365b5ba8d08551ddd2aa99bb937a7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 20 Apr 2023 11:31:48 +0200 Subject: [PATCH 5/6] Update variable name to match edge direction --- crates/fj-kernel/src/operations/build/shell.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/operations/build/shell.rs b/crates/fj-kernel/src/operations/build/shell.rs index 0a3857bfa..8f082ef1d 100644 --- a/crates/fj-kernel/src/operations/build/shell.rs +++ b/crates/fj-kernel/src/operations/build/shell.rs @@ -41,14 +41,14 @@ pub trait BuildShell { } = Face::triangle([a, b, c], [None, None, None], objects); let Triangle { face: face_abd, - edges: [_, ad, bd], + edges: [_, ad, db], } = Face::triangle([b, a, d], [Some(ab), None, None], objects); let Triangle { face: face_cad, edges: [_, _, dc], } = Face::triangle([d, a, c], [Some(ad), Some(ca), None], objects); let Triangle { face: face_bcd, .. } = - Face::triangle([c, b, d], [Some(bc), Some(bd), Some(dc)], objects); + Face::triangle([c, b, d], [Some(bc), Some(db), Some(dc)], objects); let faces = [face_abc, face_abd, face_cad, face_bcd] .map(|face| face.insert(objects)); From b9ded3bbdf7cfd63c86dde2bc16c0232a4f5cd89 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 20 Apr 2023 11:31:48 +0200 Subject: [PATCH 6/6] Update variable name to match edge direction --- crates/fj-kernel/src/operations/build/shell.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/operations/build/shell.rs b/crates/fj-kernel/src/operations/build/shell.rs index 8f082ef1d..8277dca2e 100644 --- a/crates/fj-kernel/src/operations/build/shell.rs +++ b/crates/fj-kernel/src/operations/build/shell.rs @@ -45,10 +45,10 @@ pub trait BuildShell { } = Face::triangle([b, a, d], [Some(ab), None, None], objects); let Triangle { face: face_cad, - edges: [_, _, dc], + edges: [_, _, cd], } = Face::triangle([d, a, c], [Some(ad), Some(ca), None], objects); let Triangle { face: face_bcd, .. } = - Face::triangle([c, b, d], [Some(bc), Some(db), Some(dc)], objects); + Face::triangle([c, b, d], [Some(bc), Some(db), Some(cd)], objects); let faces = [face_abc, face_abd, face_cad, face_bcd] .map(|face| face.insert(objects));