Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support arcs in sketches #100

Closed
hannobraun opened this issue Jan 28, 2022 · 8 comments · Fixed by #1484
Closed

Support arcs in sketches #100

hannobraun opened this issue Jan 28, 2022 · 8 comments · Fixed by #1484
Labels
topic: core Issues relating to core geometry, operations, algorithms type: feature New features and improvements to existing features
Milestone

Comments

@hannobraun
Copy link
Owner

As of this writing, sketches only support straight edges (i.e. lines segments). The CAD kernel already supports circles though, so adding support for arcs to sketches should not be that difficult.

The way sketches are defined would have to change, of course. Right now they are defined as just a list of points.

  • The starting point of each edge is implicit. That prevents redundancy in the definition and ensures the sketch always forms a valid cycle.
  • Hence, each edge is just defined by its end point, what type of edge it is, and edge-type-specific data required to fully define the edge.
  • Line segments are just defined by their end point.
  • Arcs are defined by their center point and angle (the radius can be inferred, since center and start point are known).

fj::Sketch could provide a convenient API:

let mut sketch = fj::Sketch::new();
sketch.line(p1); // line starts at `p2`, the last point defined below
sketch.arc(center, angle); // arc starts at `p1` and arcs around center; distance and direction defined by `angle`
sketch.line(p2); // line starts at the end point of the arc

It doesn't need to look exactly like that, of course. I'm just sketching (no pun intended).

@hannobraun hannobraun added type: feature New features and improvements to existing features topic: core Issues relating to core geometry, operations, algorithms topic: model labels Jan 28, 2022
@hannobraun
Copy link
Owner Author

This feature should ideally be able to fully replace fj::Circle. I just noticed that the representation I proposed above would not allow for that though.

If we can come up with something better before or while implementing this, great. If not, good enough, this would still be an improvement. In that case we should open a follow-up issue though, for fully replacing fj::Circle with fj::Sketch.

@Psykopear
Copy link

I think you should also specify whether the arc is clockwise or not

@hannobraun
Copy link
Owner Author

I think you should also specify whether the arc is clockwise or not

Thanks for the note! That would be defined by the sign of the angle argument (positive angle => counter-clockwise, negative angle => clockwise, presumably).

@Psykopear
Copy link

Psykopear commented Jan 28, 2022

yes that works. I'm just looking at the project, but it's really interesting, and I like the idea of something like openSCAD but with Rust code, keep up with it :)

@hannobraun
Copy link
Owner Author

Thank you, @Psykopear!

@hannobraun
Copy link
Owner Author

With #1057 merged (and some of the work leading up to it), this issue should have become much easier to implement:

  • Circles are bound by vertices now, just like any other edge. So a circle is just one example of an arc, and it should no longer be necessary to handle it in any special way.
  • The kernel should already support arcs, although this isn't tested anywhere yet.

@antonok-edm
Copy link
Contributor

This feature should ideally be able to fully replace fj::Circle. I just noticed that the representation I proposed above would not allow for that though.

Indeed; just to be explicit: with the center, angle representation, an arc with a full 360° angle cannot be resolved because there is no start or end point, hence no radius can be inferred. Other representations are problematic too; with endpoint, angle or endpoint, center, direction, circles must select an arbitrary endpoint (resulting in an extra "seam" in the boundary representation). IMO for this reason it makes sense to keep Circle distinct from arcs in a sketch.

I also lean towards endpoint, angle as an internal representation for arcs. Each segment in a chain can be defined as an endpoint with a "way to get to that point" - e.g. Route::Direct, Route::Arc(angle), eventually Route::Bezier(c1, c2), etc.

One small remaining issue: Angle currently normalizes values to the range [0, 2pi) radians, but it'd have to be possible to specify a negative angle when creating an arc in a sketch. @hannobraun do you have a preference between making a separate new Angle type, or adding another parameter to arcs for counter-/clockwise directions, or changing the existing Angle representation, or something else?

@hannobraun
Copy link
Owner Author

Thank you for your interest, @antonok-edm!

Other representations are problematic too; with endpoint, angle or endpoint, center, direction, circles must select an arbitrary endpoint (resulting in an extra "seam" in the boundary representation). IMO for this reason it makes sense to keep Circle distinct from arcs in a sketch.

Please note that the kernel is currently doing that (having an extra "seam" for circles). Originally it didn't, but I switched that around a few months ago, and it simplified a lot of code (because now every edge is bounded by vertices) and made the representation more flexible (arcs can be represented in the kernel, using the same mechanism used for circle representation; this capability isn't exercised anywhere though, and might not work correctly).

This isn't a final decision (nothing is!), so I'm happy to discuss changing it back. However, that would require some good arguments. The possible disadvantages of that additional seam are a bit nebulous to me right now, while those code complexity issues were very real.

I also lean towards endpoint, angle as an internal representation for arcs. Each segment in a chain can be defined as an endpoint with a "way to get to that point" - e.g. Route::Direct, Route::Arc(angle), eventually Route::Bezier(c1, c2), etc.

Sounds reasonable!

One small remaining issue: Angle currently normalizes values to the range [0, 2pi) radians, but it'd have to be possible to specify a negative angle when creating an arc in a sketch. @hannobraun do you have a preference between making a separate new Angle type, or adding another parameter to arcs for counter-/clockwise directions, or changing the existing Angle representation, or something else?

Changing the existing Angle seems like the best option. I don't think there's any specific reason for its current representation, except that it happens to be the way it was done initially.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: core Issues relating to core geometry, operations, algorithms type: feature New features and improvements to existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants