rei-mirrors/Fornjot
1
0
Fork 0
mirror of https://github.com/hannobraun/Fornjot synced 2025-02-15 11:45:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Rewrite parts of sweep code

Browse Source 
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.
This commit is contained in:
Hanno Braun 2023-01-30 14:57:38 +01:00
parent 819001eb0b
commit 6ceadc6830
2 changed files with 198 additions and 177 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

300
crates/fj-kernel/src/algorithms/sweep/edge.rs
View File

@ -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,
@ -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)
};
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()),
)
.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];
// Make sure that edges are oriented correctly.
let mut i = 0;
while i < edges.len() {
let j = (i + 1) % edges.len();
let [_, prev_last] = edges[i].surface_vertices();
let [next_first, _] = edges[j].surface_vertices();
// 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);
}
i += 1;
}
Cycle::new(edges).insert(objects)
};
let face = PartialFace {
exterior: Partial::from(cycle),
// The result of sweeping an edge is a face. Let's create that.
let mut face = PartialFace {
color: Some(color),
..Default::default()
};
let face = face.build(objects).insert(objects);
(face, top_edge)
// 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],
)
};
// 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)
};
// Now, the boundaries of each edge.
let boundaries = {
let [a, b] = edge.boundary();
let [c, d] = [0., 1.].map(|coord| Point::from([coord]));
[[a, b], [c, d], [b, a], [d, c]]
};
// 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);
}
// 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;
});
// 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);
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::{
@ -246,16 +219,16 @@ 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();
@ -263,8 +236,7 @@ mod tests {
Partial::from(
top.build(&mut services.objects)
.insert(&mut services.objects)
.reverse(&mut services.objects),
.insert(&mut services.objects),
)
.read()
.clone()
@ -273,13 +245,14 @@ 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 = bottom.vertices[0].1.clone();
*front = top.vertices[1].1.clone();
*back = Some(Point::from([1.]));
*front = Some(Point::from([0.]));
*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();
@ -287,8 +260,7 @@ mod tests {
Partial::from(
side_down
.build(&mut services.objects)
.insert(&mut services.objects)
.reverse(&mut services.objects),
.insert(&mut services.objects),
)
.read()
.clone()

75
crates/fj-kernel/src/algorithms/sweep/face.rs
View File

@ -1,11 +1,13 @@
use fj_interop::ext::ArrayExt;
use fj_math::{Scalar, Vector};
use crate::{
algorithms::{reverse::Reverse, transform::TransformObject},
builder::{CycleBuilder, FaceBuilder},
geometry::path::GlobalPath,
insert::Insert,
objects::{Face, Objects, Shell},
partial::{Partial, PartialObject, PartialShell},
partial::{Partial, PartialFace, PartialObject, PartialShell},
services::Service,
storage::Handle,
};
@ -49,29 +51,76 @@ impl Sweep for Handle<Face> {
};
faces.push(bottom_face.clone());
let top_face = {
let mut face = self.clone().translate(path, objects);
if is_negative_sweep {
face = face.reverse(objects);
};
face
let mut top_face = PartialFace {
color: Some(self.color()),
..PartialFace::default()
};
faces.push(top_face);
let top_surface =
bottom_face.surface().clone().translate(path, objects);
// Generate side faces
for cycle in bottom_face.all_cycles().cloned() {
for (i, cycle) in bottom_face.all_cycles().cloned().enumerate() {
let cycle = cycle.reverse(objects);
let mut top_cycle = if i == 0 {
top_face.exterior.clone()
} else {
top_face.add_interior()
};
let mut original_edges = Vec::new();
let mut top_edges = Vec::new();
for half_edge in cycle.half_edges().cloned() {
let (face, _) = (half_edge, self.color())
let (face, top_edge) = (half_edge.clone(), self.color())
.sweep_with_cache(path, cache, objects);
faces.push(face);
original_edges.push(half_edge);
top_edges.push(Partial::from(top_edge));
}
top_cycle.write().surface = Partial::from(top_surface.clone());
top_cycle.write().connect_to_closed_edges(top_edges);
for half_edge in &mut top_cycle.write().half_edges {
for (_, surface_vertex) in &mut half_edge.write().vertices {
let mut surface_vertex = surface_vertex.write();
let global_point =
surface_vertex.global_form.read().position;
if surface_vertex.position.is_none() {
if let Some(global_point) = global_point {
surface_vertex.position = Some(
top_surface
.geometry()
.project_global_point(global_point),
);
}
}
}
}
for (bottom, top) in original_edges
.into_iter()
.zip(top_cycle.write().half_edges.iter_mut())
{
top.write().curve.write().path =
Some(bottom.curve().path().into());
let boundary = bottom.boundary();
for ((top, _), bottom) in
top.write().vertices.each_mut_ext().zip_ext(boundary)
{
*top = Some(bottom);
}
}
}
let top_face = top_face.build(objects).insert(objects);
faces.push(top_face);
let faces = faces.into_iter().map(Partial::from).collect();
PartialShell { faces }.build(objects).insert(objects)
}