-
Notifications
You must be signed in to change notification settings - Fork 4
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
Rhinoceros_Engine: add convert methods for Extrusion #204
Conversation
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.
Small comments below, have just looked here on GitHub, so not tested. Will be nice to get this in!
@rolyhudson @al-fisher The method works by extrapolating the sweep rail from the centroid of the base curve (or middle point, if it is open). For some reason, the sweep is interrupted a bit before the top of the provided direction line, see below. I guess this feature can be improvable, but I'd rather raise a separate issue on this than keeping this PR open, which is already useful as it is for our applications. |
Good approach returning sweep for not perpendicular BHoM.Extrusions. Agreed. Standard extrude working well for me now in preview |
Sorry yes I was missing to save the file then commit! |
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.
LGTM now thanks @alelom.
Standard Extrude working as expected - and can recreate your behaviour (including the potential endpoint/cut off issue) with the Rhino.Sweep/BHoM.NonOrthognalExtrude
If @rolyhudson and @IsakNaslundBh are happy - good to merge from me
/azp run Rhinoceros_Toolkit.CheckInstaller |
Azure Pipelines successfully started running 1 pipeline(s). |
Thanks @al-fisher , I had to push 4acbbf0 to fix build not working for Grasshopper as highlighted by CheckInstaller. |
/azp run Rhinoceros_Toolkit.CheckInstaller |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Issues addressed by this PR
Closes #182
Test files
Create any BHoM Extrusion, then try to convert it ToRhino(), and then FromRhino().
To test the FromRhino() from a script, you may want to temporarily modify the IFromRhino method as below:
otherwise, due to the Goo casting being enforced for BHoM components, you'll never be able to pass a Rhino object to FromRhino() using Grasshopper components.
Changelog
Additional comments