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.

Sign up for GitHub

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

Jump to bottom

IfcSeventhOrderPolynomialSpiral #552

Merged
merged 7 commits into from
Aug 18, 2022
Merged

IfcSeventhOrderPolynomialSpiral #552

merged 7 commits into from
Aug 18, 2022

Conversation

Elvira2227
Copy link
Collaborator

@Elvira2227 Elvira2227 commented Jun 13, 2022 •
edited
Loading

In this pull request I implemented functions for reading the IfcThirdOrderPolynomialSpiral.
Now we can implement "VienneseBend curve" and this pull request can be addressed to issue #524.
In the CurveConverter.h I added:

  1. getPointOnCurve()
  2. getDirectionOfCurve()
  3. additional statement in convertIfcCurveSegment()

During visualisation, there is an error in the unit test data #478.

  1. Line 98 parameter ParentCurve is missing

@Elvira2227 Elvira2227 requested review from pjanck and jschlenger June 13, 2022 19:39
@Elvira2227 Elvira2227 added the IFC Content related to Industry Foundation Classes (IFC) functionalities label Jun 13, 2022
@Elvira2227 Elvira2227 self-assigned this Jun 13, 2022
@Elvira2227 Elvira2227 mentioned this pull request Jun 13, 2022
Open
6 tasks
pjanck
pjanck approved these changes Jul 19, 2022
View reviewed changes
Copy link
Contributor

@pjanck pjanck left a comment

Choose a reason for hiding this comment

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

I assume that unit tests work correctly.

@jschlenger
Copy link
Collaborator

Is just the unit test not working properly?
Does the visualization in OIP work?

jschlenger
jschlenger approved these changes Aug 1, 2022
View reviewed changes
Copy link
Collaborator

@jschlenger jschlenger left a comment

Choose a reason for hiding this comment

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

Looks fine

@jschlenger
Copy link
Collaborator

Can you create an issue to check for the correctness of IfcSeventhOrderPolynomialSpiral with an example file?
I know the code is very similar to other polynomial spirals, nevertheless to remember to check its functionality as soon as we have an example file.

@Elvira2227 Elvira2227 mentioned this pull request Aug 9, 2022
Open
@Elvira2227 Elvira2227 mentioned this pull request Aug 9, 2022
Closed
5 tasks
pjanck
pjanck approved these changes Aug 11, 2022
View reviewed changes
Copy link
Contributor

@pjanck pjanck left a comment

Choose a reason for hiding this comment

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

lftm

Is there an issue/PR with the unit tests?

@jschlenger jschlenger merged commit cf604af into tumcms:development Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pjanck pjanck pjanck approved these changes

@jschlenger jschlenger jschlenger approved these changes

Assignees

@Elvira2227 Elvira2227

Labels
IFC Content related to Industry Foundation Classes (IFC) functionalities
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Elvira2227 @jschlenger @pjanck