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

IfcSine #553

Merged
merged 8 commits into from
Jul 20, 2022
Merged

IfcSine #553

merged 8 commits into from
Jul 20, 2022

Conversation

Elvira2227
Copy link
Collaborator

@Elvira2227 Elvira2227 commented Jul 8, 2022 •
edited
Loading

In this pull request new implementation for IfcSpline added and it partlly resolved the issue #524 .

@Elvira2227 Elvira2227 requested review from pjanck and jschlenger July 8, 2022 16:51
@Elvira2227 Elvira2227 self-assigned this Jul 8, 2022
@Elvira2227 Elvira2227 added enhancement New feature or request IFC Content related to Industry Foundation Classes (IFC) functionalities labels Jul 8, 2022
pjanck
pjanck suggested changes Jul 8, 2022
View reviewed changes
@Elvira2227
Copy link
Collaborator Author

In the last commit added overloads for functions getPointOnCurve and getDirectionOfCurve.
After fixing some mistakes, geometry renders faster than before.
Here is a screenshot:
image

@Elvira2227 Elvira2227 requested a review from pjanck July 11, 2022 20:27
pjanck
pjanck reviewed Jul 12, 2022
View reviewed changes
Copy link
Contributor

@pjanck pjanck left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Can you post a screen shot of the result of a sine example? Otherwise it seems fine.

Right, already available. Seems ok. Approving.

jschlenger
jschlenger requested changes Jul 15, 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.

Could you add the new types of IfcSpirals to the list of supported Ifc Entities?
And also add a check mark to IfcSine in case everything necessary is in place.
I think there are also some other check marks missing (e.g. for IfcClothoid)
So please take a quick look and see if some of the entities that you've implemented lately are missing. Please also update the release notes.

@Elvira2227 Elvira2227 mentioned this pull request Jul 15, 2022
Open
6 tasks
@Elvira2227 Elvira2227 requested a review from jschlenger July 15, 2022 12:27
pjanck
pjanck suggested changes Jul 15, 2022
View reviewed changes
@Elvira2227 Elvira2227 requested a review from pjanck July 15, 2022 13:12
jschlenger
jschlenger requested changes Jul 17, 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.

Only a small comment regarding the release notes.

@Elvira2227 Elvira2227 requested a review from jschlenger July 17, 2022 15:22
pjanck
pjanck approved these changes Jul 19, 2022
View reviewed changes
jschlenger
jschlenger approved these changes Jul 20, 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 to me.

@jschlenger jschlenger merged commit 4ab3f36 into tumcms:development Jul 20, 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
enhancement New feature or request 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 @pjanck @jschlenger