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

Remove dependency from Rhino; enable Custom Representation Mesh; adds batch upload to send larger models #86

Merged
merged 23 commits into from
May 22, 2020

Conversation

alelom
Copy link
Member

@alelom alelom commented May 7, 2020

NOTE: Depends on

BHoM/BHoM#839
BHoM/TriangleNet_Toolkit#27

Issues addressed by this PR

Closes #80
Closes #85
Closes #87

Test files

In the toolkit: \Speckle_Toolkit\Test Scripts\#86-RemoveRhinoDep-CustomMesh

Changelog

  • Dependency on Rhino removed
  • You can now send large models (implemented batch upload as explained in Mid-large size models can't be pushed #87 )
  • If you put a RenderMesh in CustomData, that is used as a representation.

Additional comments

@alelom
Copy link
Member Author

alelom commented May 7, 2020

@al-fisher I'd need CI to add TriangleNet_Toolkit in the dependencies.
Although this PR is limited to the customMesh (so no RenderMesh reference is in place yet), I added some references to methods in TriangleNet_Toolkit to avoid code duplication.

@alelom alelom requested a review from al-fisher May 7, 2020 15:39
@FraserGreenroyd
Copy link
Contributor

@alelom I have added TriangleNet_Toolkit as a dependency of Speckle_Toolkit.

Can you also remember to set the assignee, labels, and project (should be SCRUM pull request tracker) for this and all PRs within the series of work this work is solving 😄

@alelom alelom self-assigned this May 13, 2020
@alelom alelom added the type:feature New capability or enhancement label May 13, 2020
@alelom
Copy link
Member Author

alelom commented May 13, 2020

@al-fisher
Code works now as expected:

  • BHoM Representations are constructed without dependency on Rhino:
    • First, we check if there is a custom representation (RenderMesh) in the object's Customdata. If there is, we simply convert it into a SpeckleMesh.
    • If not, computing of GeometricalRepresentation() is attempted.
    • If a geometrical representation exists, we attempt to translate that in Speckle Geometries (letting speckle do the meshing).
    • If the previous fails, compute the RenderMesh out of the Geometrical Representation.
    • If all has failed, return a SpeckleAbstract object.

image

@alelom alelom changed the title Enable Custom Representation Mesh Remove dependency from Rhino; enable Custom Representation Mesh May 13, 2020
@alelom alelom changed the title Remove dependency from Rhino; enable Custom Representation Mesh Remove dependency from Rhino; enable Custom Representation Mesh; adds batch upload to send larger models May 14, 2020
@alelom
Copy link
Member Author

alelom commented May 14, 2020

@rwemay @al-fisher this solves all the main issues we had.

  • Dependency on Rhino removed
  • You can now (finally!) send large models (I implemented batch upload as explained in Mid-large size models can't be pushed #87 )
  • If you put a RenderMesh in CustomData, that is used.

Copy link
Member

@al-fisher al-fisher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested to Copy Local settings as discussed on call.

Speckle_Adapter/Speckle_Adapter.csproj Outdated Show resolved Hide resolved
@alelom alelom requested a review from al-fisher May 21, 2020 08:12
Copy link
Member

@al-fisher al-fisher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is working well as expected.
Tested with native BHoM - 3D meshing working well.
Push and Pull.
Custom objects also still good workflow.

Approving
image

image

@al-fisher al-fisher merged commit 8af5986 into master May 22, 2020
@al-fisher al-fisher deleted the Graphics_Engine-#1694-ToRenderMesh branch May 22, 2020 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New capability or enhancement
Projects
None yet
3 participants