From 46fbbfbdd0f5a171de59289a5d5eed26fce1cfb0 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 20:39:55 +0200 Subject: [PATCH 01/19] Prepare for follow-on change --- .../checks/coincident_half_edges_are_not_siblings.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs index 1a81d4794..71ee2b01a 100644 --- a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs +++ b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs @@ -124,17 +124,17 @@ impl ValidationCheck for CoincidentHalfEdgesAreNotSiblings { continue; } - // If all points on distinct curves are within - // `distinct_min_distance`, that's a problem. - if distances( + let mut distances = distances( half_edge_a.clone(), surface_a, half_edge_b.clone(), surface_b, geometry, - ) - .all(|d| d < config.distinct_min_distance) - { + ); + + // If all points on distinct curves are within + // `distinct_min_distance`, that's a problem. + if distances.all(|d| d < config.distinct_min_distance) { let boundaries = [half_edge_a, half_edge_b].map(|half_edge| { geometry.of_half_edge(half_edge).boundary From b0bbd5fd686cebf561283148713fda49648c76b9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 20:42:08 +0200 Subject: [PATCH 02/19] Prepare for follow-on change --- .../checks/coincident_half_edges_are_not_siblings.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs index 71ee2b01a..7b0a2da35 100644 --- a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs +++ b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs @@ -124,13 +124,17 @@ impl ValidationCheck for CoincidentHalfEdgesAreNotSiblings { continue; } - let mut distances = distances( + let Some(mut distances) = distances( half_edge_a.clone(), surface_a, half_edge_b.clone(), surface_b, geometry, - ); + ) else { + // The geometry to compute the distances is not available, + // hence these half-edges can't be coincident. + continue; + }; // If all points on distinct curves are within // `distinct_min_distance`, that's a problem. @@ -174,7 +178,7 @@ fn distances( half_edge_b: Handle, surface_b: &Handle, geometry: &Geometry, -) -> impl Iterator { +) -> Option> { fn sample( percent: f64, half_edge: &Handle, @@ -203,7 +207,7 @@ fn distances( let sample2 = sample(1.0 - percent, &half_edge_b, surface_b, geometry); distances.push(sample1.distance_to(&sample2)) } - distances.into_iter() + Some(distances.into_iter()) } #[cfg(test)] From 4381bb1c3aa13c49558748ec2c4483754ec92737 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 20:42:55 +0200 Subject: [PATCH 03/19] Prepare for follow-on change --- .../coincident_half_edges_are_not_siblings.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs index 7b0a2da35..e3c56e8f6 100644 --- a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs +++ b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs @@ -184,14 +184,16 @@ fn distances( half_edge: &Handle, surface: &Handle, geometry: &Geometry, - ) -> Point<3> { + ) -> Option> { let [start, end] = geometry.of_half_edge(half_edge).boundary.inner; let path_coords = start + (end - start) * percent; let path = geometry.of_half_edge(half_edge).path; let surface_coords = path.point_from_path_coords(path_coords); - geometry - .of_surface(surface) - .point_from_surface_coords(surface_coords) + Some( + geometry + .of_surface(surface) + .point_from_surface_coords(surface_coords), + ) } // Three samples (start, middle, end), are enough to detect weather lines @@ -203,8 +205,8 @@ fn distances( let mut distances = Vec::new(); for i in 0..sample_count { let percent = i as f64 * step; - let sample1 = sample(percent, &half_edge_a, surface_a, geometry); - let sample2 = sample(1.0 - percent, &half_edge_b, surface_b, geometry); + let sample1 = sample(percent, &half_edge_a, surface_a, geometry)?; + let sample2 = sample(1.0 - percent, &half_edge_b, surface_b, geometry)?; distances.push(sample1.distance_to(&sample2)) } Some(distances.into_iter()) From 46453aff986df77178876c5ce1684571cf614dd6 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 17 Jun 2024 22:36:15 +0200 Subject: [PATCH 04/19] Read path from curve geometry in validation check --- .../checks/coincident_half_edges_are_not_siblings.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs index e3c56e8f6..e2956053e 100644 --- a/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs +++ b/crates/fj-core/src/validation/checks/coincident_half_edges_are_not_siblings.rs @@ -187,7 +187,12 @@ fn distances( ) -> Option> { let [start, end] = geometry.of_half_edge(half_edge).boundary.inner; let path_coords = start + (end - start) * percent; - let path = geometry.of_half_edge(half_edge).path; + // let path = geometry.of_half_edge(half_edge).path; + let path = geometry + .of_curve(half_edge.curve())? + .local_on(surface)? + .path; + // assert_eq!(path, path_from_curve); let surface_coords = path.point_from_path_coords(path_coords); Some( geometry From bc8414502ceb5c0a5fcf5316be9cd4af6f691b07 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 20:48:49 +0200 Subject: [PATCH 05/19] Update argument name --- .../src/validation/checks/curve_geometry_mismatch.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs index b0c96b1ef..c6ed96dad 100644 --- a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs +++ b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs @@ -70,14 +70,14 @@ impl ValidationCheck for CurveGeometryMismatch { .clone() .into_iter() .cartesian_product(edges_and_surfaces) - .filter_map(|((edge_a, surface_a), (edge_b, surface_b))| { + .filter_map(|((half_edge_a, surface_a), (edge_b, surface_b))| { // We only care about edges referring to the same curve. - if edge_a.curve().id() != edge_b.curve().id() { + if half_edge_a.curve().id() != edge_b.curve().id() { return None; } // No need to check an edge against itself. - if edge_a.id() == edge_b.id() { + if half_edge_a.id() == edge_b.id() { return None; } @@ -88,7 +88,7 @@ impl ValidationCheck for CurveGeometryMismatch { // have right now are circles, 3 would be enough to check for // coincidence. But the first and last might be identical, so // let's add an extra one. - let [a, d] = geometry.of_half_edge(&edge_a).boundary.inner; + let [a, d] = geometry.of_half_edge(&half_edge_a).boundary.inner; let b = a + (d - a) * 1. / 3.; let c = a + (d - a) * 2. / 3.; @@ -96,7 +96,7 @@ impl ValidationCheck for CurveGeometryMismatch { for point_curve in [a, b, c, d] { let a_surface = geometry - .of_half_edge(&edge_a) + .of_half_edge(&half_edge_a) .path .point_from_path_coords(point_curve); let b_surface = geometry @@ -113,7 +113,7 @@ impl ValidationCheck for CurveGeometryMismatch { if distance > config.identical_max_distance { errors.push(Self { - half_edge_a: edge_a.clone(), + half_edge_a: half_edge_a.clone(), half_edge_b: edge_b.clone(), point_curve, point_a: a_global, From 6136f94ce9868cc249df82e74bc7605cc39618ba Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 20:49:07 +0200 Subject: [PATCH 06/19] Update argument name --- .../checks/curve_geometry_mismatch.rs | 109 +++++++++--------- 1 file changed, 56 insertions(+), 53 deletions(-) diff --git a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs index c6ed96dad..b62d3aa4b 100644 --- a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs +++ b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs @@ -70,61 +70,64 @@ impl ValidationCheck for CurveGeometryMismatch { .clone() .into_iter() .cartesian_product(edges_and_surfaces) - .filter_map(|((half_edge_a, surface_a), (edge_b, surface_b))| { - // We only care about edges referring to the same curve. - if half_edge_a.curve().id() != edge_b.curve().id() { - return None; - } - - // No need to check an edge against itself. - if half_edge_a.id() == edge_b.id() { - return None; - } - - let surface_a = geometry.of_surface(&surface_a); - let surface_b = geometry.of_surface(&surface_b); - - // Let's check 4 points. Given that the most complex curves we - // have right now are circles, 3 would be enough to check for - // coincidence. But the first and last might be identical, so - // let's add an extra one. - let [a, d] = geometry.of_half_edge(&half_edge_a).boundary.inner; - let b = a + (d - a) * 1. / 3.; - let c = a + (d - a) * 2. / 3.; - - let mut errors: Vec = Vec::new(); - - for point_curve in [a, b, c, d] { - let a_surface = geometry - .of_half_edge(&half_edge_a) - .path - .point_from_path_coords(point_curve); - let b_surface = geometry - .of_half_edge(&edge_b) - .path - .point_from_path_coords(point_curve); - - let a_global = - surface_a.point_from_surface_coords(a_surface); - let b_global = - surface_b.point_from_surface_coords(b_surface); - - let distance = (a_global - b_global).magnitude(); - - if distance > config.identical_max_distance { - errors.push(Self { - half_edge_a: half_edge_a.clone(), - half_edge_b: edge_b.clone(), - point_curve, - point_a: a_global, - point_b: b_global, - distance, - }); + .filter_map( + |((half_edge_a, surface_a), (half_edge_b, surface_b))| { + // We only care about edges referring to the same curve. + if half_edge_a.curve().id() != half_edge_b.curve().id() { + return None; } - } - Some(errors) - }) + // No need to check an edge against itself. + if half_edge_a.id() == half_edge_b.id() { + return None; + } + + let surface_a = geometry.of_surface(&surface_a); + let surface_b = geometry.of_surface(&surface_b); + + // Let's check 4 points. Given that the most complex curves + // we have right now are circles, 3 would be enough to check + // for coincidence. But the first and last might be + // identical, so let's add an extra one. + let [a, d] = + geometry.of_half_edge(&half_edge_a).boundary.inner; + let b = a + (d - a) * 1. / 3.; + let c = a + (d - a) * 2. / 3.; + + let mut errors: Vec = Vec::new(); + + for point_curve in [a, b, c, d] { + let a_surface = geometry + .of_half_edge(&half_edge_a) + .path + .point_from_path_coords(point_curve); + let b_surface = geometry + .of_half_edge(&half_edge_b) + .path + .point_from_path_coords(point_curve); + + let a_global = + surface_a.point_from_surface_coords(a_surface); + let b_global = + surface_b.point_from_surface_coords(b_surface); + + let distance = (a_global - b_global).magnitude(); + + if distance > config.identical_max_distance { + errors.push(Self { + half_edge_a: half_edge_a.clone(), + half_edge_b: half_edge_b.clone(), + point_curve, + point_a: a_global, + point_b: b_global, + distance, + }); + } + } + + Some(errors) + }, + ) .flatten() } } From 59c6f7fdd2cb98804c47bd12f4bd46ddc1610745 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 20:54:49 +0200 Subject: [PATCH 07/19] Make variable name more specific --- .../fj-core/src/validation/checks/curve_geometry_mismatch.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs index b62d3aa4b..44bd8d613 100644 --- a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs +++ b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs @@ -82,7 +82,7 @@ impl ValidationCheck for CurveGeometryMismatch { return None; } - let surface_a = geometry.of_surface(&surface_a); + let surface_geom_a = geometry.of_surface(&surface_a); let surface_b = geometry.of_surface(&surface_b); // Let's check 4 points. Given that the most complex curves @@ -107,7 +107,7 @@ impl ValidationCheck for CurveGeometryMismatch { .point_from_path_coords(point_curve); let a_global = - surface_a.point_from_surface_coords(a_surface); + surface_geom_a.point_from_surface_coords(a_surface); let b_global = surface_b.point_from_surface_coords(b_surface); From 4e5b8329f7c274763a0159c961f6e722028c29a3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 20:55:10 +0200 Subject: [PATCH 08/19] Make variable name more specific --- .../fj-core/src/validation/checks/curve_geometry_mismatch.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs index 44bd8d613..3f057a0a9 100644 --- a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs +++ b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs @@ -83,7 +83,7 @@ impl ValidationCheck for CurveGeometryMismatch { } let surface_geom_a = geometry.of_surface(&surface_a); - let surface_b = geometry.of_surface(&surface_b); + let surface_geom_b = geometry.of_surface(&surface_b); // Let's check 4 points. Given that the most complex curves // we have right now are circles, 3 would be enough to check @@ -109,7 +109,7 @@ impl ValidationCheck for CurveGeometryMismatch { let a_global = surface_geom_a.point_from_surface_coords(a_surface); let b_global = - surface_b.point_from_surface_coords(b_surface); + surface_geom_b.point_from_surface_coords(b_surface); let distance = (a_global - b_global).magnitude(); From fcb87e70ddf42a335ac98ddd045cdc4640518816 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 21:13:54 +0200 Subject: [PATCH 09/19] Prepare for follow-on change --- .../src/validation/checks/curve_geometry_mismatch.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs index 3f057a0a9..e610ef851 100644 --- a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs +++ b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs @@ -177,7 +177,7 @@ mod tests { geometry.boundary = geometry.boundary.reverse(); - [HalfEdge::new( + let half_edge = HalfEdge::new( half_edge.curve().clone(), half_edge.start_vertex().clone(), ) @@ -185,7 +185,9 @@ mod tests { .set_geometry( geometry, &mut core.layers.geometry, - )] + ); + + [half_edge] }, core, ) From b67b1b3ee70cbcb2e5f109b90c07a74918ba86e7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 21:24:23 +0200 Subject: [PATCH 10/19] Make variable name more specific --- .../src/validation/checks/curve_geometry_mismatch.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs index e610ef851..2d09602ef 100644 --- a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs +++ b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs @@ -169,13 +169,14 @@ mod tests { cycle.update_half_edge( cycle.half_edges().nth_circular(0), |half_edge, core| { - let mut geometry = *core + let mut half_edge_geom = *core .layers .geometry .of_half_edge(half_edge); - geometry.path = geometry.path.reverse(); - geometry.boundary = - geometry.boundary.reverse(); + half_edge_geom.path = + half_edge_geom.path.reverse(); + half_edge_geom.boundary = + half_edge_geom.boundary.reverse(); let half_edge = HalfEdge::new( half_edge.curve().clone(), @@ -183,7 +184,7 @@ mod tests { ) .insert(core) .set_geometry( - geometry, + half_edge_geom, &mut core.layers.geometry, ); From 197374087716091b936995f728f2280201b2d4ff Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 21:15:31 +0200 Subject: [PATCH 11/19] Define missing curve geometry in test --- .../checks/curve_geometry_mismatch.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs index 2d09602ef..6a20b6783 100644 --- a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs +++ b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs @@ -178,6 +178,17 @@ mod tests { half_edge_geom.boundary = half_edge_geom.boundary.reverse(); + let mut curve_geom = core + .layers + .geometry + .of_curve(half_edge.curve()) + .unwrap() + .local_on(face.surface()) + .unwrap() + .clone(); + curve_geom.path = + curve_geom.path.reverse(); + let half_edge = HalfEdge::new( half_edge.curve().clone(), half_edge.start_vertex().clone(), @@ -188,6 +199,12 @@ mod tests { &mut core.layers.geometry, ); + core.layers.geometry.define_curve( + half_edge.curve().clone(), + face.surface().clone(), + curve_geom, + ); + [half_edge] }, core, From 58be958365e1ede81b097b496ee9e6ec39d3304c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 21:04:12 +0200 Subject: [PATCH 12/19] Read path from curve geometry in validation check --- .../checks/curve_geometry_mismatch.rs | 47 +++++++++++++++++-- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs index 6a20b6783..3a783fe21 100644 --- a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs +++ b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs @@ -82,6 +82,47 @@ impl ValidationCheck for CurveGeometryMismatch { return None; } + let Some(curve_geom) = + geometry.of_curve(half_edge_a.curve()) + else { + // No geometry defined for the curve. Nothing to test + // here. + return None; + }; + + let local_curve_geom_a = curve_geom.local_on(&surface_a); + let local_curve_geom_b = curve_geom.local_on(&surface_b); + + let (local_curve_geom_a, local_curve_geom_b) = + match (local_curve_geom_a, local_curve_geom_b) { + (Some(a), Some(b)) => (a, b), + (None, None) => { + // No geometry defined for the curve on the + // respective surface. Nothing to test here. + return None; + } + _ => { + // This means that geometry is only defined for + // one of the curves, which is a mismatch. But + // it is a mismatch that can't be represented + // with the validation error struct, as + // currently defined. + // + // I don't want to put work into addressing this + // right now, as in the future, curve geometry + // hopefully won't be redundantly defined, and + // this whole validation check will become + // redundant. + // + // For this reason, I'm choosing the easy way + // here, and that should do just fine for now. + panic!( + "Curve geometry mismatch: Curve not \ + defined on all required surfaces." + ); + } + }; + let surface_geom_a = geometry.of_surface(&surface_a); let surface_geom_b = geometry.of_surface(&surface_b); @@ -97,12 +138,10 @@ impl ValidationCheck for CurveGeometryMismatch { let mut errors: Vec = Vec::new(); for point_curve in [a, b, c, d] { - let a_surface = geometry - .of_half_edge(&half_edge_a) + let a_surface = local_curve_geom_a .path .point_from_path_coords(point_curve); - let b_surface = geometry - .of_half_edge(&half_edge_b) + let b_surface = local_curve_geom_b .path .point_from_path_coords(point_curve); From 6c7fd44c22f646b65db989cf0aa2aaccdfa7ea71 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 21:31:50 +0200 Subject: [PATCH 13/19] Read path from curve geometry in validation check --- crates/fj-core/src/validation/checks/half_edge_connection.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/validation/checks/half_edge_connection.rs b/crates/fj-core/src/validation/checks/half_edge_connection.rs index 636ca1b56..9142e9ef3 100644 --- a/crates/fj-core/src/validation/checks/half_edge_connection.rs +++ b/crates/fj-core/src/validation/checks/half_edge_connection.rs @@ -101,7 +101,10 @@ fn check_cycle<'r>( let end_pos_of_first_half_edge = { let [_, end] = geometry.of_half_edge(first).boundary.inner; geometry - .of_half_edge(first) + .of_curve(first.curve()) + .unwrap() + .local_on(surface) + .unwrap() .path .point_from_path_coords(end) }; From 2aaf431ac0cd022ae1cb7ad4b9aabadac38a9bfe Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 26 Mar 2024 11:20:11 +0100 Subject: [PATCH 14/19] Provide surface to `Cycle`'s `BoundingVolume` impl --- .../src/algorithms/bounding_volume/cycle.rs | 10 +++-- .../src/algorithms/bounding_volume/face.rs | 41 +++++++++++-------- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/crates/fj-core/src/algorithms/bounding_volume/cycle.rs b/crates/fj-core/src/algorithms/bounding_volume/cycle.rs index 138583387..49f8f0426 100644 --- a/crates/fj-core/src/algorithms/bounding_volume/cycle.rs +++ b/crates/fj-core/src/algorithms/bounding_volume/cycle.rs @@ -1,10 +1,14 @@ use fj_math::Aabb; -use crate::{geometry::Geometry, topology::Cycle}; +use crate::{ + geometry::Geometry, + storage::Handle, + topology::{Cycle, Surface}, +}; -impl super::BoundingVolume<2> for &Cycle { +impl super::BoundingVolume<2> for (&Cycle, &Handle) { fn aabb(self, geometry: &Geometry) -> Option> { - let cycle = self; + let (cycle, _) = self; let mut aabb: Option> = None; diff --git a/crates/fj-core/src/algorithms/bounding_volume/face.rs b/crates/fj-core/src/algorithms/bounding_volume/face.rs index dd2411435..375d54bcf 100644 --- a/crates/fj-core/src/algorithms/bounding_volume/face.rs +++ b/crates/fj-core/src/algorithms/bounding_volume/face.rs @@ -1,3 +1,5 @@ +use std::ops::Deref; + use fj_math::Aabb; use crate::{ @@ -7,27 +9,30 @@ use crate::{ impl super::BoundingVolume<3> for &Face { fn aabb(self, geometry: &Geometry) -> Option> { - self.region().exterior().aabb(geometry).map(|aabb2| { - let surface = geometry.of_surface(self.surface()); + (self.region().exterior().deref(), self.surface()) + .aabb(geometry) + .map(|aabb2| { + let surface = geometry.of_surface(self.surface()); - match surface.u { - GlobalPath::Circle(circle) => { - // This is not the most precise way to calculate the AABB, - // doing it for the whole circle, but it should do. + match surface.u { + GlobalPath::Circle(circle) => { + // This is not the most precise way to calculate the + // AABB, doing it for the whole circle, but it should + // do. - let aabb_bottom = circle.aabb(); - let aabb_top = Aabb { - min: aabb_bottom.min + surface.v, - max: aabb_bottom.max + surface.v, - }; + let aabb_bottom = circle.aabb(); + let aabb_top = Aabb { + min: aabb_bottom.min + surface.v, + max: aabb_bottom.max + surface.v, + }; - aabb_bottom.merged(&aabb_top) + aabb_bottom.merged(&aabb_top) + } + GlobalPath::Line(_) => Aabb { + min: surface.point_from_surface_coords(aabb2.min), + max: surface.point_from_surface_coords(aabb2.max), + }, } - GlobalPath::Line(_) => Aabb { - min: surface.point_from_surface_coords(aabb2.min), - max: surface.point_from_surface_coords(aabb2.max), - }, - } - }) + }) } } From 4f1b31101319ba363885d7f02c47b528677a1bd9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 26 Mar 2024 11:22:31 +0100 Subject: [PATCH 15/19] Give surface to `HalfEdge`'s `BoundingVolume` impl --- crates/fj-core/src/algorithms/bounding_volume/cycle.rs | 4 ++-- crates/fj-core/src/algorithms/bounding_volume/half_edge.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/fj-core/src/algorithms/bounding_volume/cycle.rs b/crates/fj-core/src/algorithms/bounding_volume/cycle.rs index 49f8f0426..a934a38a1 100644 --- a/crates/fj-core/src/algorithms/bounding_volume/cycle.rs +++ b/crates/fj-core/src/algorithms/bounding_volume/cycle.rs @@ -8,12 +8,12 @@ use crate::{ impl super::BoundingVolume<2> for (&Cycle, &Handle) { fn aabb(self, geometry: &Geometry) -> Option> { - let (cycle, _) = self; + let (cycle, surface) = self; let mut aabb: Option> = None; for half_edge in cycle.half_edges() { - let new_aabb = half_edge + let new_aabb = (half_edge, surface) .aabb(geometry) .expect("`HalfEdge` can always compute AABB"); aabb = Some(aabb.map_or(new_aabb, |aabb| aabb.merged(&new_aabb))); diff --git a/crates/fj-core/src/algorithms/bounding_volume/half_edge.rs b/crates/fj-core/src/algorithms/bounding_volume/half_edge.rs index 690cf6acc..c670f3b73 100644 --- a/crates/fj-core/src/algorithms/bounding_volume/half_edge.rs +++ b/crates/fj-core/src/algorithms/bounding_volume/half_edge.rs @@ -3,12 +3,12 @@ use fj_math::{Aabb, Vector}; use crate::{ geometry::{Geometry, SurfacePath}, storage::Handle, - topology::HalfEdge, + topology::{HalfEdge, Surface}, }; -impl super::BoundingVolume<2> for &Handle { +impl super::BoundingVolume<2> for (&Handle, &Handle) { fn aabb(self, geometry: &Geometry) -> Option> { - let half_edge = self; + let (half_edge, _) = self; let half_edge_geom = geometry.of_half_edge(half_edge); let path = half_edge_geom.path; From 8bb5e6b13fa3c4aa4d082b9e7babd79911962aff Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 21:37:15 +0200 Subject: [PATCH 16/19] Read path from curve geometry in bounding volume code --- .../fj-core/src/algorithms/bounding_volume/half_edge.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/algorithms/bounding_volume/half_edge.rs b/crates/fj-core/src/algorithms/bounding_volume/half_edge.rs index c670f3b73..5f5322f62 100644 --- a/crates/fj-core/src/algorithms/bounding_volume/half_edge.rs +++ b/crates/fj-core/src/algorithms/bounding_volume/half_edge.rs @@ -8,10 +8,15 @@ use crate::{ impl super::BoundingVolume<2> for (&Handle, &Handle) { fn aabb(self, geometry: &Geometry) -> Option> { - let (half_edge, _) = self; + let (half_edge, surface) = self; let half_edge_geom = geometry.of_half_edge(half_edge); - let path = half_edge_geom.path; + let path = geometry + .of_curve(half_edge.curve()) + .unwrap() + .local_on(surface) + .unwrap() + .path; match path { SurfacePath::Circle(circle) => { From 8ed9db867596c7e47fe4e80e232b34eea609d741 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 21:38:57 +0200 Subject: [PATCH 17/19] Read path from curve geometry in curve approx code --- crates/fj-core/src/algorithms/approx/curve.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/algorithms/approx/curve.rs b/crates/fj-core/src/algorithms/approx/curve.rs index fcd2dc82b..7de0c597b 100644 --- a/crates/fj-core/src/algorithms/approx/curve.rs +++ b/crates/fj-core/src/algorithms/approx/curve.rs @@ -31,7 +31,12 @@ impl Approx for (&Handle, &HalfEdgeGeom, &Handle) { Some(approx) => approx, None => { let approx = approx_curve( - &half_edge.path, + &geometry + .of_curve(curve) + .unwrap() + .local_on(surface) + .unwrap() + .path, geometry.of_surface(surface), half_edge.boundary, tolerance, From 082dd9f1a9512093d8fb6c39b68c3bb577b4e4ed Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 21:39:29 +0200 Subject: [PATCH 18/19] Read path from curve geometry in edge approx code --- crates/fj-core/src/algorithms/approx/edge.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/algorithms/approx/edge.rs b/crates/fj-core/src/algorithms/approx/edge.rs index 19fc1d799..bc9ea427b 100644 --- a/crates/fj-core/src/algorithms/approx/edge.rs +++ b/crates/fj-core/src/algorithms/approx/edge.rs @@ -61,7 +61,10 @@ impl Approx for (&Handle, &Handle) { approx.points.into_iter().map(|point| { let point_surface = geometry - .of_half_edge(half_edge) + .of_curve(half_edge.curve()) + .unwrap() + .local_on(surface) + .unwrap() .path .point_from_path_coords(point.local_form); From 72efa214aa6529e38cdade7c05ad29f67d5ea052 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 19 Jun 2024 21:42:52 +0200 Subject: [PATCH 19/19] Remove `path` field from `HalfEdgeGeom` This is a big milestone, as it means that all curve geometry is now exclusively defined within `CurveGeom`. --- crates/fj-core/src/algorithms/approx/curve.rs | 8 +++--- crates/fj-core/src/geometry/half_edge.rs | 27 ------------------- .../fj-core/src/operations/build/half_edge.rs | 9 ------- crates/fj-core/src/operations/build/shell.rs | 18 +------------ .../src/operations/reverse/half_edge.rs | 1 - .../fj-core/src/operations/sweep/half_edge.rs | 18 +------------ .../checks/curve_geometry_mismatch.rs | 2 -- 7 files changed, 6 insertions(+), 77 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/curve.rs b/crates/fj-core/src/algorithms/approx/curve.rs index 7de0c597b..82bff3dc3 100644 --- a/crates/fj-core/src/algorithms/approx/curve.rs +++ b/crates/fj-core/src/algorithms/approx/curve.rs @@ -203,7 +203,7 @@ mod tests { let curve = Curve::from_path_and_surface(path, surface.clone(), &mut core); let boundary = CurveBoundary::from(boundary); - let half_edge = HalfEdgeGeom { path, boundary }; + let half_edge = HalfEdgeGeom { boundary }; let tolerance = 1.; let approx = (&curve, &half_edge, &surface) @@ -226,7 +226,7 @@ mod tests { let curve = Curve::from_path_and_surface(path, surface.clone(), &mut core); let boundary = CurveBoundary::from(boundary); - let half_edge = HalfEdgeGeom { path, boundary }; + let half_edge = HalfEdgeGeom { boundary }; let tolerance = 1.; let approx = (&curve, &half_edge, &surface) @@ -248,7 +248,7 @@ mod tests { let curve = Curve::from_path_and_surface(path, surface.clone(), &mut core); let boundary = CurveBoundary::from([[0.], [TAU]]); - let half_edge = HalfEdgeGeom { path, boundary }; + let half_edge = HalfEdgeGeom { boundary }; let tolerance = 1.; let approx = (&curve, &half_edge, &surface) @@ -279,7 +279,7 @@ mod tests { let curve = Curve::from_path_and_surface(path, surface.clone(), &mut core); let boundary = CurveBoundary::from([[0.], [TAU]]); - let half_edge = HalfEdgeGeom { path, boundary }; + let half_edge = HalfEdgeGeom { boundary }; let tolerance = 1.; let approx = (&curve, &half_edge, &surface) diff --git a/crates/fj-core/src/geometry/half_edge.rs b/crates/fj-core/src/geometry/half_edge.rs index 0ea12b321..6f8e2730b 100644 --- a/crates/fj-core/src/geometry/half_edge.rs +++ b/crates/fj-core/src/geometry/half_edge.rs @@ -5,33 +5,6 @@ use super::{CurveBoundary, SurfacePath}; /// The geometry of a half-edge #[derive(Copy, Clone, Debug)] pub struct HalfEdgeGeom { - /// # The path of the half-edge - /// - /// ## Implementation Note - /// - /// Currently, all curve-related geometry is defined locally, in terms of - /// the surface that the curve is on (or purely in 2D, if there is no - /// surface associated with this geometry). However, curves exist globally, - /// independently of surfaces. Half-edges in multiple surfaces can refer to - /// the same curve, and in fact, that is the whole reason for their - /// existence as a topological object. - /// - /// This contradiction, globally defined curves but locally defined curve - /// geometry, is the reason that this curve geometry is defined right here, - /// associated with a locally existing half-edge. (And, I might add, - /// redundantly so, as multiple half-edges within the same surface context - /// can refer to the same curve.) - /// - /// Instead, it should be possible to define curve geometry *either* locally - /// or globally. Then that respective definition can be associated with the - /// curve (and possibly, in addition, a surface). How exactly that is going - /// to work is up in the air. - /// - /// The point of all this exposition is to clarify that this field doesn't - /// really belong here. It exists here for practical reasons that are, - /// hopefully, temporary. - pub path: SurfacePath, - /// # The boundary of the half-edge on its curve pub boundary: CurveBoundary>, } diff --git a/crates/fj-core/src/operations/build/half_edge.rs b/crates/fj-core/src/operations/build/half_edge.rs index 1eb04bdfb..e1c6074ce 100644 --- a/crates/fj-core/src/operations/build/half_edge.rs +++ b/crates/fj-core/src/operations/build/half_edge.rs @@ -73,7 +73,6 @@ pub trait BuildHalfEdge { core.layers.geometry.define_half_edge( half_edge.clone(), HalfEdgeGeom { - path, boundary: boundary.into(), }, ); @@ -100,14 +99,6 @@ pub trait BuildHalfEdge { core.layers.geometry.define_half_edge( half_edge.clone(), HalfEdgeGeom { - path: core - .layers - .geometry - .of_curve(half_edge.curve()) - .expect("Curve geometry was just defined in same function") - .local_on(&surface) - .expect("Curve geometry was just defined in same function") - .path, boundary: boundary.unwrap_or_default(), }, ); diff --git a/crates/fj-core/src/operations/build/shell.rs b/crates/fj-core/src/operations/build/shell.rs index 7bdcaee46..c0d94e47a 100644 --- a/crates/fj-core/src/operations/build/shell.rs +++ b/crates/fj-core/src/operations/build/shell.rs @@ -102,23 +102,7 @@ pub trait BuildShell { .update_curve(|_, _| curve.clone(), core) .insert(core) .set_geometry( - HalfEdgeGeom { - path: core - .layers - .geometry - .of_curve(&curve) - .expect( - "Curve geometry was just \ - defined in same function", - ) - .local_on(&surface) - .expect( - "Curve geometry was just \ - defined in same function", - ) - .path, - boundary, - }, + HalfEdgeGeom { boundary }, &mut core.layers.geometry, ) }) diff --git a/crates/fj-core/src/operations/reverse/half_edge.rs b/crates/fj-core/src/operations/reverse/half_edge.rs index aa241ab12..eb7d3c4c5 100644 --- a/crates/fj-core/src/operations/reverse/half_edge.rs +++ b/crates/fj-core/src/operations/reverse/half_edge.rs @@ -17,7 +17,6 @@ impl ReverseCurveCoordinateSystems for (&Handle, &Handle) { let (half_edge, surface) = self; let mut half_edge_geom = *core.layers.geometry.of_half_edge(half_edge); - half_edge_geom.path = half_edge_geom.path.reverse(); half_edge_geom.boundary = half_edge_geom.boundary.reverse(); let curve = diff --git a/crates/fj-core/src/operations/sweep/half_edge.rs b/crates/fj-core/src/operations/sweep/half_edge.rs index f964f98bc..3e350152a 100644 --- a/crates/fj-core/src/operations/sweep/half_edge.rs +++ b/crates/fj-core/src/operations/sweep/half_edge.rs @@ -143,23 +143,7 @@ impl SweepHalfEdge for Handle { .update_curve(|_, _| curve.clone(), core) .insert(core) .set_geometry( - HalfEdgeGeom { - path: core - .layers - .geometry - .of_curve(&curve) - .expect( - "Curve geometry was just defined in same \ - function", - ) - .local_on(&surface) - .expect( - "Curve geometry was just defined in same \ - function", - ) - .path, - boundary, - }, + HalfEdgeGeom { boundary }, &mut core.layers.geometry, ); diff --git a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs index 3a783fe21..bd8fb440b 100644 --- a/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs +++ b/crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs @@ -212,8 +212,6 @@ mod tests { .layers .geometry .of_half_edge(half_edge); - half_edge_geom.path = - half_edge_geom.path.reverse(); half_edge_geom.boundary = half_edge_geom.boundary.reverse();