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 for partial lathes (and by extension partial cylinders) #126

Draft
wants to merge 23 commits into
base: develop
Choose a base branch
from

Conversation

Kolpa
Copy link

@Kolpa Kolpa commented Aug 6, 2024

I've run into some situations now where i would benefit from the ability to only partially complete a lathes rotation giving us the ability to easily create partial cylinders and other more complex structures as seen in the image below.

Mesh.cylinder(endAngle: .halfPi)

The current state of the pr is a very rough draft of the functionality without any documentation changes as i believe there are a few implementation details that we should discuss if a merge is desired.

How should the functionality be integrated?

I quite like the current solution of adding two more arguments to lathe and its parent functions allowing for start and end angles to be defined but defaulting to the complete circle if they are not specified.
I could also understand if instead the creation of an additional partialLathe method is desired, especially if complexity of the partial lathe implementation starts to grow.

Should the meshes be made watertight?

The current implementation will not close off the start- and end-face of the lathe if a whole rotation is not achieved as seen below.

  • Is this desirable?
  • Should we close these gaps in the mesh by default?
  • Should the user be able to choose which behaviour is desired?

I hope this is a good starting point for discussion. If you have any further requirements / ideas please reach out and i will try to implement them.

If the functionality is not desired feel free to close the PR instead.

Best Regards
Kolya

@nicklockwood
Copy link
Owner

nicklockwood commented Aug 7, 2024

@Kolpa I think this makes sense. It's possible to create this effect by subtracting a wedge from a cylinder, but it's not very efficient, and likely to produce messy mesh topology.

Regarding your questions:

  1. How should the functionality be integrated?

I agree that adding optional start/end angles seems like a good way to go. startAngle would default to zero and endAngle would default to startAngle. Need to consider if the arc should go clockwise or anticlockwise by default though (I don't recall what the precedent is - if any - in the other Euclid APIs). Also need to think about what happens for angles < 0 or greater than 2pi (should they be normalised, or should the relative distance be used to decide arc direction in some way?)

  1. Should the meshes be made watertight?

I definitely think that the result should be watertight by default, because watertight meshes are required for CSG operations, and all existing primitives are watertight. It's not obvious what to do about texture coordinates for the newly generated surfaces though. I'm fine with having an option to not include these faces, although it's not obvious how best to include that option since it's only meaningful when start and end angle are specified - should it be a separate fillExposedFaces (don't love the name) parameter, or could it somehow be added to the existing faces enum?

Rather than an endless proliferation of parameters, it might also be worth introducing a LatheOptions struct that can then be passed as an optional parameter to all lathe-based primitives. This could include startAngle, endAngle, fillExposedFaces (or whatever) along with existing options like poleDetail and slices.

@nicklockwood nicklockwood force-pushed the develop branch 2 times, most recently from 91dd394 to 9643e62 Compare August 7, 2024 11:07
@nicklockwood nicklockwood changed the base branch from develop to main August 11, 2024 06:39
@nicklockwood nicklockwood changed the base branch from main to develop August 11, 2024 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants