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

Flexible section amounts for bounding_cylinder #2273

Closed
wants to merge 1 commit into from
Closed

Flexible section amounts for bounding_cylinder #2273

wants to merge 1 commit into from

Conversation

djean032
Copy link

Adds ability to specify number of sections to better approximate cylinders.

… number selection in bounding_cylinder function. Default sections is still 32
@djean032 djean032 closed this Aug 27, 2024
@mikedh
Copy link
Owner

mikedh commented Aug 27, 2024

Thanks for the PR! Yeah it's a good point we might want better control of all of the parameters used for the magic @property values like mesh.ray (controlled by use_embree: bool right now), mesh.bounding_cylinder, etc. I'm not totally sure where the right place to put those would be, but I think it should probably also include values for mesh.bounding_sphere, mesh.bounding_cylinder, mesh.ray. Maybe the right place to put these would be as a dataclass although it is a bit wordy:

@dataclass
class PropertySettings:
    # keyword arguments passed to `trimesh.primitives.Cylinder`
    bounding_cylinder: dict

    use_embree: bool = True

Although I guess for cylinders you could also call the methods it's currently using:

from trimesh import bounds, primitives

kwargs = bounds.minimum_cylinder(a_mesh)
mincyl = primitives.Cylinder(mutable=False, sections=100, **kwargs)

@djean032
Copy link
Author

Thanks for considering the PR. I think that's an interesting idea. I closed this for now, because my solution was hacky, but I'll keep working on it, because it's pretty useful in my current work.

@djean032 djean032 deleted the circle_smoothing branch August 28, 2024 01:38
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.

None yet

2 participants