Skip to content

Commit

Permalink
Rewrite parts of sweep code
Browse files Browse the repository at this point in the history 
The rewrite of the edge sweeping code uses the builder API, making the
code more compact, and making the sweep code for `Vertex` redundant (it
can be removed in a follow-up commit.

The rewrite of the face sweeping code is less of a slam dunk. It adds a
fair bit of complexity over the previous version, but I think this is
okay, due to two factors:

1. I believe that this complexity can be reigned in later, once the core
   data structures have been simplified. This is an ongoing effort.
2. It fixes existing bugs. See below.

The initial reason for the rewrite was the ongoing `HalfEdge` cleanup:
As part of the celanup, the `Reverse` implementation for `HalfEdge`
needs to be removed, and this new code no longer uses it.

However, as it turned out, there is a significant side effect: The new
code generates different (but still correct) curves, which are less
uniform than the ones generated by the old code, which happens to
trigger approximation failures that exposed existing bugs.

Those bugs basically boil down to the previous sweep code generating
coincident edges that don't refer to the same global edge. This was
already known (#1162), but so far a proper fix was blocked on missing
validation code, to ensure all instances of this bug are fixed.

That validation code still needs to be written, but the approximation
failures already did a pretty good job of guiding me towards the various
sources of the bug.
  • Loading branch information
@hannobraun
hannobraun committed Feb 16, 2023
1 parent 7f7a675 commit ccd99b4
Show file tree
Hide file tree
Showing 2 changed files with 190 additions and 169 deletions.
284 changes: 128 additions & 156 deletions crates/fj-kernel/src/algorithms/sweep/edge.rs
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
use fj_interop::{ext::ArrayExt, mesh::Color};
use fj_math::{Line, Scalar, Vector};
use iter_fixed::IntoIteratorFixed;
use fj_math::{Point, Scalar, Vector};

use crate::{
algorithms::{reverse::Reverse, transform::TransformObject},
geometry::path::SurfacePath,
builder::{CycleBuilder, HalfEdgeBuilder},
insert::Insert,
objects::{
Curve, Cycle, Face, GlobalEdge, HalfEdge, Objects, SurfaceVertex,
},
objects::{Face, HalfEdge, Objects},
partial::{Partial, PartialFace, PartialObject},
services::Service,
storage::Handle,
Expand All @@ -28,158 +24,135 @@ impl Sweep for (Handle<HalfEdge>, Color) {
let (edge, color) = self;
let path = path.into();

let surface =
edge.curve().clone().sweep_with_cache(path, cache, objects);

// We can't use the edge we're sweeping from as the bottom edge, as that
// is not defined in the right surface. Let's create a new bottom edge,
// by swapping the surface of the original.
let bottom_edge = {
let points_curve_and_surface = edge
.boundary()
.map(|point| (point, [point.t, Scalar::ZERO]));

let curve = {
// Please note that creating a line here is correct, even if the
// global curve is a circle. Projected into the side surface, it
// is going to be a line either way.
let path =
SurfacePath::Line(Line::from_points_with_line_coords(
points_curve_and_surface,
));

Curve::new(
surface.clone(),
path,
edge.curve().global_form().clone(),
)
.insert(objects)
};

let boundary = {
let points_surface = points_curve_and_surface
.map(|(_, point_surface)| point_surface);

edge.boundary()
.zip_ext(edge.surface_vertices())
.into_iter_fixed()
.zip(points_surface)
.collect::<[_; 2]>()
.map(|((point, surface_vertex), point_surface)| {
let surface_vertex = SurfaceVertex::new(
point_surface,
surface.clone(),
surface_vertex.global_form().clone(),
)
.insert(objects);

(point, surface_vertex)
})
};

HalfEdge::new(curve, boundary, edge.global_form().clone())
.insert(objects)
// The result of sweeping an edge is a face. Let's create that.
let mut face = PartialFace {
color: Some(color),
..Default::default()
};

let side_edges = bottom_edge
.boundary()
.zip_ext(bottom_edge.surface_vertices())
.map(|(point, surface_vertex)| {
(point, surface_vertex.clone(), surface.clone())
.sweep_with_cache(path, cache, objects)
});

let top_edge = {
let surface_vertices = side_edges.clone().map(|edge| {
let [_, surface_vertex] = edge.surface_vertices();
surface_vertex.clone()
});

let points_curve_and_surface = bottom_edge
.boundary()
.map(|point| (point, [point.t, Scalar::ONE]));

let curve = {
let global = bottom_edge
.curve()
.global_form()
.clone()
.translate(path, objects);

// Please note that creating a line here is correct, even if the
// global curve is a circle. Projected into the side surface, it
// is going to be a line either way.
let path =
SurfacePath::Line(Line::from_points_with_line_coords(
points_curve_and_surface,
));

Curve::new(surface, path, global).insert(objects)
};

let global = GlobalEdge::new(
curve.global_form().clone(),
surface_vertices
.clone()
.map(|surface_vertex| surface_vertex.global_form().clone()),
// A face (and everything in it) is defined on a surface. A surface can
// be created by sweeping a curve, so let's sweep the curve of the edge
// we're sweeping.
face.exterior.write().surface = Partial::from(
edge.curve().clone().sweep_with_cache(path, cache, objects),
);

// Now we're ready to create the edges.
let mut edge_bottom = face.exterior.write().add_half_edge();
let mut edge_up = face.exterior.write().add_half_edge();
let mut edge_top = face.exterior.write().add_half_edge();
let mut edge_down = face.exterior.write().add_half_edge();

// Those edges aren't fully defined yet. We'll do that shortly, but
// first we have to figure a few things out.
//
// Let's start with the global vertices and edges.
let (global_vertices, global_edges) = {
let [a, b] = edge
.surface_vertices()
.map(|surface_vertex| surface_vertex.global_form().clone());
let (edge_right, [_, c]) =
b.clone().sweep_with_cache(path, cache, objects);
let (edge_left, [_, d]) =
a.clone().sweep_with_cache(path, cache, objects);

(
[a, b, c, d],
[edge.global_form().clone(), edge_right, edge_left],
)
.insert(objects);

let boundary = bottom_edge
.boundary()
.into_iter_fixed()
.zip(surface_vertices)
.collect::<[_; 2]>();

HalfEdge::new(curve, boundary, global).insert(objects)
};

let cycle = {
let a = bottom_edge;
let [d, b] = side_edges;
let c = top_edge.clone();

let mut edges = [a, b, c, d];
// Next, let's figure out the surface coordinates of the edge vertices.
let surface_points = {
let [a, b] = edge.boundary();

[
[a.t, Scalar::ZERO],
[b.t, Scalar::ZERO],
[b.t, Scalar::ONE],
[a.t, Scalar::ONE],
]
.map(Point::from)
};

// Make sure that edges are oriented correctly.
let mut i = 0;
while i < edges.len() {
let j = (i + 1) % edges.len();
// Now, the boundaries of each edge.
let boundaries = {
let [a, b] = edge.boundary();
let [c, d] = [0., 1.].map(|coord| Point::from([coord]));

let [_, prev_last] = edges[i].surface_vertices();
let [next_first, _] = edges[j].surface_vertices();
[[a, b], [c, d], [b, a], [d, c]]
};

// Need to compare surface forms here, as the global forms might
// be coincident when sweeping circles, despite the vertices
// being different!
if prev_last.id() != next_first.id() {
edges[j] = edges[j].clone().reverse(objects);
// Armed with all of that, we can set the edge's vertices.
[
edge_bottom.write(),
edge_up.write(),
edge_top.write(),
edge_down.write(),
]
.zip_ext(boundaries)
.zip_ext(surface_points)
.zip_ext(global_vertices)
.map(
|(((mut half_edge, boundary), surface_point), global_vertex)| {
for ((a, _), b) in
half_edge.vertices.each_mut_ext().zip_ext(boundary)
{
*a = Some(b);
}

i += 1;
}

Cycle::new(edges).insert(objects)
};
// Writing to the start vertices is enough. Neighboring half-
// edges share surface vertices, so writing the start vertex of
// each half-edge writes to all vertices.
let mut vertex = half_edge.vertices[0].1.write();
vertex.position = Some(surface_point);
vertex.global_form = Partial::from(global_vertex);
},
);

// With the vertices set, we can now update the curves.
//
// Those are all line segments. For the bottom and top curve, because
// even if the original edge was a circle, it's still going to be a line
// when projected into the new surface. For the side edges, because
// we're sweeping along a straight path.
for mut edge in [
edge_bottom.write(),
edge_up.write(),
edge_top.write(),
edge_down.write(),
] {
edge.update_as_line_segment();
}

// Finally, we can make sure that all edges refer to the correct global
// edges.
[edge_bottom.write(), edge_up.write(), edge_down.write()]
.zip_ext(global_edges)
.map(|(mut half_edge, global_edge)| {
let global_edge = Partial::from(global_edge);

half_edge.curve.write().global_form =
global_edge.read().curve.clone();
half_edge.global_form = global_edge;
});

let face = PartialFace {
exterior: Partial::from(cycle),
color: Some(color),
..Default::default()
};
// And we're done creating the face! All that's left to do is build our
// return values.
let face = face.build(objects).insert(objects);

(face, top_edge)
let edge_top = edge_top.build(objects);
(face, edge_top)
}
}

#[cfg(test)]
mod tests {
use fj_interop::{ext::ArrayExt, mesh::Color};
use fj_math::Point;
use pretty_assertions::assert_eq;

use crate::{
algorithms::{reverse::Reverse, sweep::Sweep},
algorithms::sweep::Sweep,
builder::HalfEdgeBuilder,
insert::Insert,
partial::{
Expand Down Expand Up @@ -246,25 +219,24 @@ mod tests {
top.curve.write().surface = surface.clone();

{
let [back, front] = top
.vertices
.each_mut_ext()
.map(|(_, surface_vertex)| surface_vertex);
let [(back, back_surface), (front, front_surface)] =
top.vertices.each_mut_ext();

let mut back = back.write();
back.position = Some([0., 1.].into());
back.surface = surface.clone();
*back = Some(Point::from([1.]));
*back_surface = side_up.vertices[1].1.clone();

*front = side_up.vertices[1].1.clone();
*front = Some(Point::from([0.]));
let mut front_surface = front_surface.write();
front_surface.position = Some([0., 1.].into());
front_surface.surface = surface.clone();
}

top.infer_global_form();
top.update_as_line_segment();

Partial::from(
top.build(&mut services.objects)
.insert(&mut services.objects)
.reverse(&mut services.objects),
.insert(&mut services.objects),
)
.read()
.clone()
Expand All @@ -273,22 +245,22 @@ mod tests {
let mut side_down = PartialHalfEdge::default();
side_down.curve.write().surface = surface;

let [back, front] = side_down
.vertices
.each_mut_ext()
.map(|(_, surface_vertex)| surface_vertex);
let [(back, back_surface), (front, front_surface)] =
side_down.vertices.each_mut_ext();

*back = Some(Point::from([1.]));
*front = Some(Point::from([0.]));

*back = bottom.vertices[0].1.clone();
*front = top.vertices[1].1.clone();
*back_surface = top.vertices[1].1.clone();
*front_surface = bottom.vertices[0].1.clone();

side_down.infer_global_form();
side_down.update_as_line_segment();

Partial::from(
side_down
.build(&mut services.objects)
.insert(&mut services.objects)
.reverse(&mut services.objects),
.insert(&mut services.objects),
)
.read()
.clone()
Expand Down
Loading

0 comments on commit ccd99b4

Please sign in to comment.