Added support for OCCT 7.6.2

rainman110 commented 9 months ago

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:

The colors in TiGL Viewer are somewhat (slightly) different due to the new Color Space in OCCT
The test WingSegmentGuideCurves.tiglWingGetSegmentUpperSurfaceAreaTrimmed is failing due to some tolerance error. I did not look into it further.

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

Checklist:

[ ] A test for the new functionality was added.
[ ] 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.

joergbrech commented 9 months ago

Thanks @rainman110! I will have a look at the failing test

rainman110 commented 9 months ago

Do you have any idea, why the ci is not working at all?

joergbrech commented 9 months ago

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...

https://github.com/DLR-SC/tigl/issues/972

rainman110 commented 8 months ago

I fixed a shader problem (see #973). It is now working again:

joergbrech commented 8 months ago

@rainman110, I will rebase on master to see if CI is triggered.

rainman110 commented 7 months ago

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?

joergbrech commented 7 months ago

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.

rainman110 commented 7 months ago

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.

rainman110 commented 7 months ago

Oh... I see, that I fckd up the git history 🤦

rainman110 commented 7 months ago

The history is clean again

codecov[bot] commented 7 months ago

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

:exclamation: No coverage uploaded for pull request base (master@363a4ac). Click here to learn what that means.

Additional details and impacted files

[![Impacted file tree graph](https://app.codecov.io/gh/DLR-SC/tigl/pull/971/graphs/tree.svg?width=650&height=150&src=pr&token=8b0i0TsOw6&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC)](https://app.codecov.io/gh/DLR-SC/tigl/pull/971?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC) ```diff @@ Coverage Diff @@ ## master #971 +/- ## ========================================= Coverage ? 68.95% ========================================= Files ? 299 Lines ? 26499 Branches ? 0 ========================================= Hits ? 18272 Misses ? 8227 Partials ? 0 ``` | [Files](https://app.codecov.io/gh/DLR-SC/tigl/pull/971?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC) | Coverage Δ | | |---|---|---| | [src/boolean\_operations/GEOMAlgo\_Splitter.cxx](https://app.codecov.io/gh/DLR-SC/tigl/pull/971?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC#diff-c3JjL2Jvb2xlYW5fb3BlcmF0aW9ucy9HRU9NQWxnb19TcGxpdHRlci5jeHg=) | `30.61% <ø> (ø)` | | | [src/common/tiglcommonfunctions.cpp](https://app.codecov.io/gh/DLR-SC/tigl/pull/971?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC#diff-c3JjL2NvbW1vbi90aWdsY29tbW9uZnVuY3Rpb25zLmNwcA==) | `74.58% <ø> (ø)` | | | [src/configuration/CCPACSConfiguration.cpp](https://app.codecov.io/gh/DLR-SC/tigl/pull/971?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC#diff-c3JjL2NvbmZpZ3VyYXRpb24vQ0NQQUNTQ29uZmlndXJhdGlvbi5jcHA=) | `57.14% <ø> (ø)` | | | [...c/control\_devices/CControlSurfaceBorderBuilder.cpp](https://app.codecov.io/gh/DLR-SC/tigl/pull/971?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC#diff-c3JjL2NvbnRyb2xfZGV2aWNlcy9DQ29udHJvbFN1cmZhY2VCb3JkZXJCdWlsZGVyLmNwcA==) | `76.96% <ø> (ø)` | | | [src/wing/CCPACSWingSegment.cpp](https://app.codecov.io/gh/DLR-SC/tigl/pull/971?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC#diff-c3JjL3dpbmcvQ0NQQUNTV2luZ1NlZ21lbnQuY3Bw) | `84.24% <ø> (ø)` | | | [src/geometry/CTiglTriangularizer.cpp](https://app.codecov.io/gh/DLR-SC/tigl/pull/971?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DLR-SC#diff-c3JjL2dlb21ldHJ5L0NUaWdsVHJpYW5ndWxhcml6ZXIuY3Bw) | `85.71% <60.00%> (ø)` | |

joergbrech commented 7 months ago

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.

joergbrech commented 7 months ago

Hmm, I am having trouble building TiGL with OCCT 7.6.2.

Several classes include V3d_Coordinate.hxx which basically just contains a typedef. This file was removed: https://git.dev.opencascade.org/gitweb/?p=occt.git;a=commit;h=7fd4958d451a9c8dcf1c58e100c8ac898484addd. If I am not mistaken, we should replace all occurences of V3d_Coordinate with Standard_Real.
- TiglViewerInternal.h includes MMgt_TShared.hxx which has been removed: https://git.dev.opencascade.org/gitweb/?p=occt.git;a=commit;h=f4e7c3078527877b5d1f13f2c74982be4450e14b. I believe we can just remove the include. I don't think it is used.
CCPACSConfiguration.cpp includes BRepAlgo_Fuse.hxx which has been removed: https://git.dev.opencascade.org/gitweb/?p=occt.git;a=commit;h=efac1733770f766f136694da42454bdd0db62c67. I believe we can just remove the include. I don't think it is used.

@rainman110, could you verify this?

joergbrech commented 7 months ago

@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

[x] update the code according to my previous comment
[x] wait until https://github.com/DLR-SC/tigl-conda/pull/32 is merged
[ ] Update the used occt version in CI here and see if the CI here is successfull
[ ] Merge

joergbrech commented 6 months ago

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.

joergbrech commented 6 months ago

I will not yet update the opencascade version in our CI to 7.6.2, because one CI job uses the anaconda package opencascade-static, which is still at 7.4.0, see also https://github.com/DLR-SC/tigl-conda/issues/37. @rainman110, do you remember where these packages came from? Did you build them locally?

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.

rainman110 commented 6 months ago

I used the recipes from the TiGL repo: https://github.com/DLR-SC/tigl/tree/master/ci/conda

DLR-SC / tigl