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

Implement "height" parameter on rotate_extrude to allow for helical threads. #5012

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

aprotim
Copy link

@aprotim aprotim commented Feb 24, 2024

Fixes #5011

Details of the motivation are described in detail in the linked issue.

@jordanbrown0
Copy link
Contributor

I don't have the energy right now to look at the actual implementation, but two cautions:

  • If your Z-increment isn't enough that the sample at 360° is above the sample at 0°, you'll generate a self-intersecting polyhedron, and that's bad. (I'm not completely sure, but I don't think any other primitive, other than polyhedron(), can generate a self-intersecting polyhedron.) It might be enough to do a 2D intersect of the shape and itself offset as for 360°, and confirm that the result is empty.
  • While the linear extrude scheme works based on slices perpendicular to the Z axis, a rotate_extrude scheme works based on slices perpendicular to the circle that they are rotated through. Neither is really correct for a helix. If you want to make a helix with a circular cross-section, your circles need to be tilted so that they align with the angle of the helix. Without that tilt, the linear extrude scheme needs a base shape sort of like a banana, and the rotate extrude scheme needs something more like an ellipse. (Not sure if it is an ellipse.)

Several libraries have "sweep" functions that will do the right thing.

@pca006132
Copy link
Member

@jordanbrown0 linear extrude + twist + small number of slices can generate self-intersecting polygons, but it should be a known issue.

@aprotim
Copy link
Author

aprotim commented Feb 26, 2024

@jordanbrown0 this may not be a true helical cross-section, but in many industrial/practical designs, the important cross-section is the one that's perpendicular to the central axis.

As for self-intersection, I can definitely generate a warning if the rise would cause self-intersection. I believe that should be doable with minimal additional processing.

@aprotim
Copy link
Author

aprotim commented Feb 26, 2024

Existing tests should be fixed. Additional tests for new functionality pending.

@aprotim
Copy link
Author

aprotim commented Feb 26, 2024

FWIW, attached as a PDF is an example of the kind of design I was trying to match with the above .scad file (the actual dimensions I needed turned out to be somewhat different, but the idea is there). As you can see, the important shapes/dimensions are parallel to the axis of the helix.

Similarly, see the diagram for the profile of M* metric bolts.

For every design/standard I've seen for threads (this is not extensive, but it's non-zero), the profile is specified parallel to the axis of rotation, which is something that cannot be accomplished "out of the box" with OpenSCAD.

I'm offering up the change only because it's such a common design paradigm and because it turned out to be relatively simple to implement with minimal changes and significant gains to speed and geometric accuracy. (I'm also attaching screenshots of spiral_extrude's full render vs rotate_extrue w/ heights preview to demonstrate the geometric artifacts.

201802231348444844.pdf

spiral_extrude library
rotate_extrude w/ height parameter

@pca006132
Copy link
Member

How does this compare with sweep implementations such as https://github.com/BelfrySCAD/BOSL2/wiki/skin.scad#functionmodule-spiral_sweep?

@aprotim
Copy link
Author

aprotim commented Feb 27, 2024

I'm not sure, partially because I've generated a relatively complex 2D cross-section, and those modules require that to be converted into a vector of vertices, which is non-trivial, as far as I know?

This is actually another reason that I prefer this paradigm: it allows a user to take advantage of all of the available 2D methods to create their cross section, rather than having to handcraft their polygons.

@jordanbrown0
Copy link
Contributor

BOSL2 can do fairly complex things with lists of points. It's a different way of doing things, but similarly powerful. Note also that BOSL2 has a substantial thread library.

But I'm starting to come around to the idea that this sort of extension to rotate_extrude has value. Everybody loves threads, and if threads are defined using a vertical cross-section, then this is an easy-to-understand answer. (Not that my opinion matters much.)

@aprotim
Copy link
Author

aprotim commented Feb 27, 2024

After a cursory Google, I don't see any implementation of sweep that allows you to use the 2D subsystem to specify the input shape. I also can't think of an approach to implement that in "user space", other than the spiral_extrude or hull-based approximations.

Are you aware of a sweep module that does this, or can you point me at a way to implement one?

@aprotim
Copy link
Author

aprotim commented Feb 27, 2024

Everybody loves threads

So say we all.

@jordanbrown0
Copy link
Contributor

Are you aware of a sweep module that [is based on the 2D subsystem], or can you point me at a way to implement one?

No, and I agree that with current OpenSCAD it isn't possible. (It is possible with PR#4478.) The closest you can come is successive hulls, and that doesn't work for concave shapes.

@kintel
Copy link
Member

kintel commented Mar 6, 2024

I think this is a reasonable approach. While there are opportunities to extend it further by allowing 2D scale and rotation, I think this in isolation should be pretty well-defined.

This definitely needs tests though. Look at existing tests for rotate_extrude and linear_extrude for ideas. We should probably create new test cases for this instead of adding to existing ones, as I guess there are some corner cases to consider.

Also, I'm not sure self-intersections are bad per se, especially when using the Manifold backend, as long as we're producing a manifold topology.

@pca006132
Copy link
Member

Also, I'm not sure self-intersections are bad per se, especially when using the Manifold backend, as long as we're producing a manifold topology.

lt is bad, if it does not fall into the epsilon-valid requirement. manifold will probably add a self-intersection removal phase, but probably not in a short period of time.

@kintel
Copy link
Member

kintel commented Mar 6, 2024

@pca006132 Thanks for jumping in! Just so I get it right: Self-touching volumes are fine (as long as the topology is manifold by keeping coincident vertices separate), but self-intersecting volumes are bad?

@pca006132
Copy link
Member

yup

@kintel
Copy link
Member

kintel commented Mar 9, 2024

OK, let's keep the self-intersection test, at least until we have a better way of detecting or avoiding self-intersections (e.g. we could chop up the geometry into < 360 degree chunks and union them together).

I'd say let's merge once we have sufficient tests.

@gsohler
Copy link
Member

gsohler commented Mar 9, 2024

isn't same as #4538 ?

only difference it that the parameter name is called 'translate' and there is also a 'scale' parameter

@kintel
Copy link
Member

kintel commented Mar 9, 2024

Oh, hadn't paid attention to that one.
@aprotim could you take a look at that other PR and perhaps discuss with the other author how to best proceed from a technical perspective?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a "height" parameter for rotate_extrude to help with threading
5 participants