Skip to content

Commit

Permalink
Merge pull request #177 from hannobraun/comment
Browse files Browse the repository at this point in the history
Update comment
  • Loading branch information
hannobraun authored Feb 11, 2022
2 parents 1f0253b + f6bc272 commit f5204f6
Showing 1 changed file with 17 additions and 3 deletions.
20 changes: 17 additions & 3 deletions src/kernel/shapes/sketch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,23 @@ impl Shape for fj::Sketch {
let vertices = self
.to_points()
.into_iter()
// These calls to `create_at` are valid, since the points of this
// sketch come directly from the user and define new vertices of the
// shape.
// These calls to `Vertex::create_at` don't follow the rules that
// the documentation of `create_at` lays out. This method can be
// called by an outside caller, and is additionally called by other
// methods in this trait implementation, either directly or
// indirectly.
//
// This means the same vertices are re-created multiple times, which
// is forbidden. I don't think this is causing any actual problems,
// since these `Vertex` instances are created from points that come
// directly from the user, and aren't being computed here.
//
// But still, this rule exists for a reason: to prevent subtle bugs
// from creeping in. We should follow it, here and everywhere.
//
// Please refer to this issue for more context on the problem, as
// well as a proposed solution:
// https://github.com/hannobraun/Fornjot/issues/176
.map(|[x, y]| Vertex::create_at(Point::from([x, y, 0.])))
.collect();
Vertices(vertices)
Expand Down

0 comments on commit f5204f6

Please sign in to comment.