Skip to content

Commit

Permalink
Fix storage of half-edge geometry
Browse files Browse the repository at this point in the history
Half-edge geometry is supposed to be stored per-object, which is what
this commit achieves. Previously, it would have been overwritten, if an
object that happened to be equal was stored.

This bug doesn't manifest so far, because the geometry that is stored is
still redundantly part of `HalfEdge` itself, so you'd never overwrite
something that wasn't equal anyway. This will change, of course, as soon
as the redundant geometry definition in `HalfEdge` is removed.

This is one of those colossally stupid bugs that shouldn't have happend
in the first place. It's good though, because it helped me realize two
things:

1. The distinction between `Handle` and `HandleWrapper` is a footgun.
2. That distinction is also no longer necessary.

With geometry being stored in a dedicated layer, the times of needing to
compare objects for equality are coming to and end. There is simply no
reason to ever expect to objects to be equal, if they aren't also
identical.

This means, it should be possible to make `Handle` behave like
`HandleWrapper`, replace `HandleWrapper` with the new and improved
`Handle`, and then remove the `Eq`/`PartialEq` implementations for all
objects. Because otherwise, that's another footgun waiting to happen.

That's what I'll be doing next.
  • Loading branch information
hannobraun committed Mar 22, 2024
1 parent 99836a3 commit 8663d6c
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions crates/fj-core/src/geometry/geometry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use super::{GlobalPath, HalfEdgeGeometry, SurfaceGeometry};

/// Geometric data that is associated with topological objects
pub struct Geometry {
half_edge: BTreeMap<Handle<HalfEdge>, HalfEdgeGeometry>,
half_edge: BTreeMap<HandleWrapper<HalfEdge>, HalfEdgeGeometry>,
surface: BTreeMap<HandleWrapper<Surface>, SurfaceGeometry>,

xy_plane: Handle<Surface>,
Expand Down Expand Up @@ -61,7 +61,7 @@ impl Geometry {
half_edge: Handle<HalfEdge>,
geometry: HalfEdgeGeometry,
) {
self.half_edge.insert(half_edge, geometry);
self.half_edge.insert(half_edge.into(), geometry);
}

pub(crate) fn define_surface_inner(
Expand All @@ -82,7 +82,7 @@ impl Geometry {
half_edge: &Handle<HalfEdge>,
) -> HalfEdgeGeometry {
self.half_edge
.get(half_edge)
.get(&half_edge.clone().into())
.copied()
.expect("Expected geometry of half-edge to be defined")
}
Expand Down

0 comments on commit 8663d6c

Please sign in to comment.