diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 4bf5dd72c..c20ed6256 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -183,7 +183,7 @@ impl CycleBuilder for PartialCycle { { edges.map(|other| { let mut this = self.add_half_edge(); - this.write().update_from_other_edge(&other, &Some(*surface)); + this.write().update_from_other_edge(&other, surface); this }) } diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index c4f5b7402..2807164c2 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -58,7 +58,7 @@ pub trait HalfEdgeBuilder { fn update_from_other_edge( &mut self, other: &Partial, - surface: &Option, + surface: &SurfaceGeometry, ); } @@ -222,7 +222,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { fn update_from_other_edge( &mut self, other: &Partial, - surface: &Option, + surface: &SurfaceGeometry, ) { let global_curve = other.read().curve.read().global_form.clone(); self.curve.write().global_form = global_curve.clone(); @@ -230,103 +230,78 @@ impl HalfEdgeBuilder for PartialHalfEdge { self.curve.write().path = other.read().curve.read().path.as_ref().and_then(|path| { - match surface { - Some(surface) => { - // We have information about the other edge's surface - // available. We need to use that to interpret what the - // other edge's curve path means for our curve path. - match surface.u { - GlobalPath::Circle(circle) => { - // The other surface is curved. We're entering - // some dodgy territory here, as only some edge - // cases can be represented using our current - // curve/surface representation. - match path { - MaybeSurfacePath::Defined( - SurfacePath::Line(_), - ) - | MaybeSurfacePath::UndefinedLine => { - // We're dealing with a line on a - // rounded surface. - // - // Based on the current uses of this - // method, we can make some assumptions: - // - // 1. The line is parallel to the u-axis - // of the other surface. - // 2. The surface that *our* edge is in - // is a plane that is parallel to the - // the plane of the circle that - // defines the curvature of the other - // surface. - // - // These assumptions are necessary - // preconditions for the following code - // to work. But unfortunately, I see no - // way to check those preconditions - // here, as neither the other line nor - // our surface is necessarily defined - // yet. - // - // Handling this case anyway feels like - // a grave sin, but I don't know what - // else to do. If you tracked some - // extremely subtle and annoying bug - // back to this code, I apologize. - // - // I hope that I'll come up with a - // better curve/surface representation - // before this becomes a problem. - Some( - MaybeSurfacePath::UndefinedCircle { - radius: circle.radius(), - }, - ) - } - _ => { - // The other edge is a line segment in a - // curved surface. No idea how to deal - // with this. - todo!( - "Can't connect edge to circle on \ - curved surface" - ) - } - } + // We have information about the other edge's surface available. + // We need to use that to interpret what the other edge's curve + // path means for our curve path. + match surface.u { + GlobalPath::Circle(circle) => { + // The other surface is curved. We're entering some + // dodgy territory here, as only some edge cases can be + // represented using our current curve/surface + // representation. + match path { + MaybeSurfacePath::Defined(SurfacePath::Line(_)) + | MaybeSurfacePath::UndefinedLine => { + // We're dealing with a line on a rounded + // surface. + // + // Based on the current uses of this method, we + // can make some assumptions: + // + // 1. The line is parallel to the u-axis of the + // other surface. + // 2. The surface that *our* edge is in is a + // plane that is parallel to the the plane of + // the circle that defines the curvature of + // the other surface. + // + // These assumptions are necessary preconditions + // for the following code to work. But + // unfortunately, I see no way to check those + // preconditions here, as neither the other line + // nor our surface is necessarily defined yet. + // + // Handling this case anyway feels like a grave + // sin, but I don't know what else to do. If you + // tracked some extremely subtle and annoying + // bug back to this code, I apologize. + // + // I hope that I'll come up with a better curve/ + // surface representation before this becomes a + // problem. + Some(MaybeSurfacePath::UndefinedCircle { + radius: circle.radius(), + }) } - GlobalPath::Line(_) => { - // The other edge is defined on a plane. - match path { - MaybeSurfacePath::Defined( - SurfacePath::Line(_), - ) - | MaybeSurfacePath::UndefinedLine => { - // The other edge is a line segment on - // a plane. That means our edge must be - // a line segment too. - Some(MaybeSurfacePath::UndefinedLine) - } - _ => { - // The other edge is a circle or arc on - // a plane. I'm actually not sure what - // that means for our edge. We might be - // able to represent it somehow, but - // let's leave that as an exercise for - // later. - todo!( - "Can't connect edge to circle on \ - plane" - ) - } - } + _ => { + // The other edge is a line segment in a curved + // surface. No idea how to deal with this. + todo!( + "Can't connect edge to circle on curved \ + surface" + ) } } } - None => { - // We know nothing about the surface the other edge is - // on. This means we can't infer anything about our - // curve from the other curve. - None + GlobalPath::Line(_) => { + // The other edge is defined on a plane. + match path { + MaybeSurfacePath::Defined(SurfacePath::Line(_)) + | MaybeSurfacePath::UndefinedLine => { + // The other edge is a line segment on a plane. + // That means our edge must be a line segment + // too. + Some(MaybeSurfacePath::UndefinedLine) + } + _ => { + // The other edge is a circle or arc on a plane. + // I'm actually not sure what that means for our + // edge. We might be able to represent it + // somehow, but let's leave that as an exercise + // for later. + todo!("Can't connect edge to circle on plane") + } + } } } });