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

Fix for complex window geometry causing crashes in computeTriangulation #3993

Merged
merged 4 commits into from
Jul 27, 2020

Conversation

macumber
Copy link
Contributor

Added logic to rejoin previously triangulated window geometry before calling computeTriangulation on the base surface. Fixed several bugs in joinAll and translation to ThreeJS format.

Pull request overview

  • Fixes crashes when running the ViewModel measure on certain Ladybug energy models with complex triangulated windows

Pull Request Author

  • All new and existing tests passes

Labels:

Review Checklist

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • Code Style, strip trailing whitespace, etc.
  • All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
  • Labeling is ok
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified

macumber added 4 commits June 12, 2020 18:38
…Added logic to rejoin previously triangulated window geometry before calling computeTriangulation on the base surface. Fixed several bugs in joinAll and translation to ThreeJS format.
@tijcolem tijcolem added the Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. label Jun 23, 2020
@tijcolem
Copy link
Collaborator

Re-running windows tests. Mac and Linux look good.

@tijcolem tijcolem merged commit d8815c6 into NREL:develop Jul 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants