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

IfcSecondOrderPolynomial #551

Merged
merged 6 commits into from
Sep 23, 2022
Merged

IfcSecondOrderPolynomial #551

merged 6 commits into from
Sep 23, 2022

Conversation

Elvira2227
Copy link
Collaborator

@Elvira2227 Elvira2227 commented Jun 13, 2022 •
edited
Loading

In this pull request I implemented functions for reading the IfcSecondOrderPolynomialSpiral.
Now we can implement "Helmert 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()

Here you can see the screen shot (it presents HelmertCurve_100.0_300_1000_1_Meter) :
image

@Elvira2227 Elvira2227 requested review from pjanck and jschlenger June 13, 2022 18:54
@Elvira2227 Elvira2227 self-assigned this Jun 13, 2022
@Elvira2227 Elvira2227 added the IFC Content related to Industry Foundation Classes (IFC) functionalities label 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
jschlenger reviewed Jul 27, 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.

Have you checked that you are not only visualizing the curve that Peter added to the IFC file to visually check if the calculation is done right?
(as was the case for IfcSine in the beginning)

@jschlenger
Copy link
Collaborator

It seems like quite some changes that were made for IfcSine are reverted in this pull request, which is most probably not intended. Also some bspline sreenshots that were change recently are reverted here.
I think that should not be the case. Maybe pulling the most recent changes from the development branch could help.
Let me know in case there are any issues, it seems like you tried to merge but then reverted it.

@Elvira2227 Elvira2227 mentioned this pull request Aug 9, 2022
Closed
5 tasks
@Elvira2227 Elvira2227 force-pushed the IfcSecondOrderPolynomialSpiral branch from 9d77c2e to 258fd6a Compare September 19, 2022 14:42
jschlenger
jschlenger approved these changes Sep 23, 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.

Lftm

@jschlenger jschlenger merged commit d755d14 into tumcms:development Sep 23, 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