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

Geometry_Engine: Add join mesh method and fix MergeVertices #2947

Merged
merged 9 commits into from
Nov 15, 2022

Conversation

IsakNaslundBh
Copy link
Contributor

@IsakNaslundBh IsakNaslundBh commented Nov 10, 2022

Issues addressed by this PR

Closes #2945
Closes #2946

  • Adding method for joining multiple meshes into single mesh. Allows for (does not check for) if the meshes are disjointed or not.
  • Fix the non-functioning MergedVertices method and rename it MergeVertices (removing the d). Complete rewrite as the previous version was not functioning, and we now have access to better grouping functionality through the domain tree clusters.

Test files

https://burohappold.sharepoint.com/:f:/s/BHoM/Eo4Nblb9345DlWdrFSNZwx8BwNsgwKB74BxCuqah5qCYPg?e=Hmavj2

Changelog

Additional comments

Method was not at all functioning before, so complete refactor, making use of methods not existing when method was added
Rename from MergedVertices to MergeVertices to align with general naming convetion of methods
@IsakNaslundBh IsakNaslundBh added type:bug Error or unexpected behaviour type:feature New capability or enhancement labels Nov 10, 2022
@IsakNaslundBh IsakNaslundBh self-assigned this Nov 10, 2022
@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 10, 2022

@IsakNaslundBh to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance

There are 19 requests in the queue ahead of you.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check compliance
@BHoMBot check unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 10, 2022

@IsakNaslundBh to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance
  • check unit-tests

There are 30 requests in the queue ahead of you.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 10, 2022

The check code-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 10, 2022

The check branch-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 10, 2022

The check dataset-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 10, 2022

The check copyright-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

Geometry_Engine/Compute/Join.cs Outdated Show resolved Hide resolved
Geometry_Engine/Compute/Join.cs Outdated Show resolved Hide resolved
Geometry_Engine/Compute/Join.cs Show resolved Hide resolved
Geometry_Engine/Modify/MergeVertices.cs Show resolved Hide resolved
@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 14, 2022

@IsakNaslundBh to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance

@pawelbaran
Copy link
Member

@BHoMBot check core
@BHoMBot check unit-tests
@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 15, 2022

@pawelbaran to confirm, the following actions are now queued:

  • check core
  • check unit-tests
  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 15, 2022

The check code-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 15, 2022

The check documentation-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 15, 2022

The check branch-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 15, 2022

The check dataset-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 15, 2022

The check copyright-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

Copy link
Member

@pawelbaran pawelbaran left a comment

Choose a reason for hiding this comment

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

Approving based on the code review, answered comments as well as testing made using provided test file as well as in promptu performance tests, which showed there is still space for improvement in finding duplicate points (but this refers to BHoM cull duplicate methods in general).

@pawelbaran
Copy link
Member

@BHoMBot check versioning
@BHoMBot check serialisation
@BHoMBot check null-handling
@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 15, 2022

@pawelbaran to confirm, the following actions are now queued:

  • check versioning
  • check serialisation
  • check null-handling
  • check ready-to-merge

@pawelbaran pawelbaran merged commit bc170c8 into main Nov 15, 2022
@pawelbaran pawelbaran deleted the Geometry_Engine-#2945-AddJoinMeshMethod branch November 15, 2022 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Error or unexpected behaviour type:feature New capability or enhancement
Projects
None yet
2 participants