-
Notifications
You must be signed in to change notification settings - Fork 61
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
Added support for OCCT 7.6.2 #971
Conversation
Thanks @rainman110! I will have a look at the failing test |
Do you have any idea, why the ci is not working at all? |
Hmm, seems that we need to update the workflow file again...https://github.com/DLR-SC/tigl/actions/runs/6548880919. It is a bit of a pain that github actions are so unstable... |
I fixed a shader problem (see #973). It is now working again: |
@rainman110, I will rebase on master to see if CI is triggered. |
So, it does not compile anymore with OCCT 7.4.0 🤦 . @joergbrech Do you already have the (proper) changes in Geoml, so that we can backport it to TiGL? |
Unfortunately no, because in paraDiGMS we fixed the OCCT version to 7.6.2 so we never had the need to build it with OCCT 7.4.0. |
The code now compiles with both OCCT 7.4.0 and 7.6.1. Something is wrong though with the macos test job, but this should not be related to the PR. |
Oh... I see, that I fckd up the git history 🤦 |
The history is clean again |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #971 +/- ##
=========================================
Coverage ? 68.95%
=========================================
Files ? 299
Lines ? 26499
Branches ? 0
=========================================
Hits ? 18272
Misses ? 8227
Partials ? 0
|
Awesome! Thanks so much @rainman110. I will try to build this branch with OCCT 7.6 and if it works we are good to merge. |
Hmm, I am having trouble building TiGL with OCCT 7.6.2.
@rainman110, could you verify this? |
@rainman110, if you like, I can take over from here (but please give me a heads-up if this is ok for you, so that I don't mess around in your branch). Thank you very much! I propose to
|
Note: The test WingSegmentGuideCurves.tiglWingGetSegmentUpperSurfaceAreaTrimmed is failing due to some tolerance error. I did not look into it further.
Closes #973
The stripes were computed based on the normal vector of the vertex, not on the fragment. This lead to visual distortions. Now, we use the normal vector from the fragment shader as offered by the occt api.
Ok so I tested this locally on Ubuntu 20.04 with the new opencascade anaconda packages for version 7.6.2 in our dlr-sc channel. There still was an issue with a faulty integration test which succeeded with opencasce 7.4, but not with 7.6.2 which can easily be fixed. I will allow myself to rebase this on master and fix the test. Then I am fine to merge. Sorry for keeping this open so long. |
I will not yet update the opencascade version in our CI to 7.6.2, because one CI job uses the anaconda package Note to self: Bumping the opencascade version actually used in our release boils down to modifying the environment.yml here and the conda recipe at https://github.com/DLR-SC/tigl-conda after the next release. |
I used the recipes from the TiGL repo: https://github.com/DLR-SC/tigl/tree/master/ci/conda |
OpenCASCADE 7.6.2 support
Description
I adapted the code to allow TiGL to be used with OCCT 7.6.1 as well.
Fixes #973
How Has This Been Tested?
I compiled it with OCCT 7.6.1, executed the unit tests, and started TiGL Viewer.
Here are some notes:
Screenshots, that help to understand the changes(if applicable):
Checklist: