Skip to content
This repository has been archived by the owner on Aug 28, 2021. It is now read-only.

Dynamo Converter #18

Merged
merged 18 commits into from
Jun 6, 2018
Merged

Dynamo Converter #18

merged 18 commits into from
Jun 6, 2018

Conversation

alvpickmans
Copy link
Collaborator

@alvpickmans alvpickmans commented May 31, 2018

This PR is a WIP to development to #10

Objects implemented:

  • Arc
  • Ellipse
  • Polygon
  • Rectangle
  • Polycurve
  • Curve
  • NurbsCurve
  • Mesh

I also started to do a bunch of cleaning from the rhino legacy code and added a few useful helper methods to check the type of a curve and get it as that geometry type (i.e. IsCircle() / GetAsCircle()), which Dynamo doesn't have OOTB.

I would't say is reayd to merge just yet, but I hope to get a working and fully tested version for the weekend.

Cheers!

@teocomi teocomi self-assigned this Jun 1, 2018
@alvpickmans alvpickmans self-assigned this Jun 3, 2018
@didimitrie
Copy link
Member

Curiosity question: does this go dynamo<->dynamo, as well as dynamo<->rhino? Great work in here!

@alvpickmans
Copy link
Collaborator Author

alvpickmans commented Jun 3, 2018

Ok so all curves are implemented. There are a few issues though:

  • Arcs are sent and rendered on the viewer great, but I feel there is something with the RhinoConverter, as all arcs recevied from Rhino have a StartAngle == 0. They also behave differently depending if drawn clock or counterclock wise.

  • Ellipses are sent and received great between Dynamo and Rhino, but viewer doesn't render them.

  • Nurbscurve are rendered great on viewer and properly received from Rhino, but not from Dynamo to Rhino.

Dynamo

Curves on Dynamo

Speckle Viewer

Curves from Dynamo on Viewer

Rhino

Curves from Dynamo on Rhino

@didimitrie
Copy link
Member

didimitrie commented Jun 3, 2018

Let's leave the viewer aside! That's going to be a separate issue.

image

Rhino to Rhino things seem fine. I'm not sure how to deal with this. I checked - and yes, all of them have a start angle of 0. Not sure what the ghost is in here. There was a previous issue (rhino to rhino), but it got sorted by adding the domain of the arc. I am quite confused on how rhino deals with arcs now to be honest...

Previous issue: speckleworks/SpeckleRhino#170

@alvpickmans
Copy link
Collaborator Author

alvpickmans commented Jun 3, 2018

Also meshes from Rhino to Dynamo and viceversa. I started to have a look on extrusions and breps and after checking Rhynamo and MantisShrimp projects it seems that they would heavily rely on NurbsSurfaces to convert Breps to DesignScript geometry, but SpeckleBrep object structures relies on the rhino brep serialization to rawData.
image

@didimitrie
Copy link
Member

Re breps, that can change to what is needed - it is meant as a efficient dirty placeholder! If we agree on a common serialisation for breps, we can do that properly!

Meshes look sweet!

@alvpickmans
Copy link
Collaborator Author

@didimitrie problem is Dynamo doesn't have a domain property so it takes the angle literally being 0 with the positive Xaxis. I found this post and it seems that an arc should give the radians from the XAxis when getting arc.StartAngle in any case?

image

@didimitrie
Copy link
Member

didimitrie commented Jun 3, 2018

Yes, or somehow extract it from the plane's rotation... I'm really confused now. Will look into this after the simaud workshop...

🙃

hen the arc starts at the plane x-axis, then the start angle is zero and the end angle equals the total angle of the arc. If this is not the case then the length of the angle domain represents the total angle of the arc. This angle is defined in radians. If this is not the case then the length of the angle domain represents the total angle of the arc. This angle is defined in radians.

These two arcs were drawn in a different order:
image
but given the way the plane is, start angle is always 0.
image

N00b question: Isn't there a way in Dynamo to set the arc in a given plane, ie like in Rhino?

@teocomi teocomi merged commit efed5b9 into speckleworks:dev Jun 6, 2018
@teocomi
Copy link
Member

teocomi commented Jun 6, 2018

I'll merge it for now, we can discuss the current issues later

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

Successfully merging this pull request may close these issues.

4 participants