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

Made Lofting algorithm throw Exception when side caps are invalid #999

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

merakulix
Copy link
Contributor

Description

Throwing an exception while building the shells, using invalid side caps, prevents TiGL to build invalid solid shapes.

How Has This Been Tested?

Added test in testFuselageStandardProfileRectangle.cpp that checks for exception if side caps are invalid. If rectangular profile wires are built that cannot be used to create valid side caps, the exception is thrown.

Screenshots, that help to understand the changes(if applicable):

This solid (created by a rectangle profile with too large corner radius) is not valid. Building side caps from its closed, but entangled profile wire is not possible. Solids like this will not be built and used mistakenly, if exception is thrown.

grafik

Checklist:

  • A test for the new functionality was added.
  • [x ] All tests run without failure.
  • The new code complies with the TiGL style guide.
  • New classes have been added to the Python interface.
  • API changes were documented properly in tigl.h.

Copy link
Contributor

@joergbrech joergbrech left a comment

Choose a reason for hiding this comment

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

Looks good in principle. One unit tests triggers the new exception, see here. We should investigate and make sure that all unit tests succeed before we merge.

@joergbrech
Copy link
Contributor

As per our chat on MM: Let's make sure that this doesn't affect the TiGL performance too much. One option could be to activate the test only in debug mode (and optionally: If we have the check in debug mode only, we could use simple assertions rather than exceptions. I believe this is common for checks that are only performed in debug mode)

Copy link
Contributor

@joergbrech joergbrech left a comment

Choose a reason for hiding this comment

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

Can we add a simple test where this exception is thrown? We can use the same preprocessor directive in the test that only checks if the lofter throws in debug mode.

Comment on lines +74 to +76
if(!BRepCheck_Analyzer(cap).IsValid()){
throw CTiglError("Error occured while building side caps. Built shape invalid.");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if(!BRepCheck_Analyzer(cap).IsValid()){
throw CTiglError("Error occured while building side caps. Built shape invalid.");
}
#ifdef DEBUG
if(!BRepCheck_Analyzer(cap).IsValid()){
throw CTiglError("Error occured while building side caps. Built shape invalid.");
}
#endif

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

Successfully merging this pull request may close these issues.

None yet

2 participants