-
Notifications
You must be signed in to change notification settings - Fork 35
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
Implement second-order convex regions #1125
Conversation
d39222a
to
74bc044
Compare
@elliottbiondo Thanks for reviewing the convex region work! This PR is the next on the list. |
{ | ||
insert_surface(Sense::outside, PlaneZ{-hh_}); | ||
insert_surface(Sense::inside, PlaneZ{hh_}); | ||
insert_surface(CCylZ{radius_}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why aren't you inserting an interior and exterior bounding box for the cylinder region?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, the "known closed" quadric/planar surfaces (axis aligned cylinders, spheres, and planes) automatically generate the correct bounding boxes using the surface clipper. I'll add a note to the convex region builder pointing out the relationship.
Follow-on to #1119. This implements:
There is a bugfix in the deduplication code (adding
+ 0.0
to clear any potential negative zero flag) to prevent simplified surfaces from having negative zeroes. This will make output more robust and hopefully do the same if-ffast-math
is enabled.