-
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
Rhino_Toolkit #87 #86 #85 Conversion between new Solid Geometry representations #89
Rhino_Toolkit #87 #86 #85 Conversion between new Solid Geometry representations #89
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.
It looks great!
I added some comments on the code.
One thing that is weird is that Grasshopper still thinks it is an Open Brep
, even if it is closed.
tagging @IsakNaslundBh for a quick check about planar surfaces. It looks fine to me anyway!
Rhinoceros_Engine/Convert/ToBHoM.cs
Outdated
/***************************************************/ | ||
|
||
|
||
private static BHG.ISolid ToBHoM(this RHG.Brep brep, bool isSolid) |
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.
is the isSolid
parameter doing something somewhere else?
It looks like it is not used in this method.
Ah, right I get it, it is separating out two different Brep
methods.
But what happens if I pass ToBHoM(brep, false)
?
Since the method BHG.ISolid ToBHoM(this RHG.Brep brep, bool isSolid)
is private, we could change its name (e.g. ToBHoMSolid
) and remove the isSolid
parameter?
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.
Yes - neater. Will do
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.
Thanks 👍
Rhinoceros_Engine/Convert/ToBHoM.cs
Outdated
{ | ||
if (brep == null) return null; | ||
|
||
if (brep.Surfaces.Count == 0) return null; | ||
|
||
// PlanarSurface case | ||
|
||
if (brep.IsSolid) return brep.ToBHoM(true); |
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.
As below, what if we call ToBHoM(false)
?
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.
Thanks Al!
Happy to merge if you are!
Raised new issue #91 with respect to returning closed Breps. |
NOTE: Depends on
This PR follows the development in the Geometry oM in the now merged PR here BHoM/BHoM#456
This is to be tested in conjunction with BHoM/Grasshopper_UI#346
Issues addressed by this PR
Closes #87
Closes #86
Closes #85
Initial implementation of Rhino conversion
.ToRhino
and.ToBHoM
for primative solids and the new BHoM Solid Boundary Representation.For non-primative solids, functionality is still limited to planar surface representations.
Test files
Test file for new Solid Object Converts
Test file for PolySurface to Brep fix