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.

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

Merged
merged 6 commits into from
Jan 9, 2024
Merged

Added support for OCCT 7.6.2 #971

merged 6 commits into from
Jan 9, 2024

Conversation

rainman110
Copy link
Collaborator

@rainman110 rainman110 commented Oct 17, 2023

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
Copy link
Contributor

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

@rainman110
Copy link
Collaborator Author

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

@joergbrech
Copy link
Contributor

joergbrech commented Oct 18, 2023

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

#972

@rainman110
Copy link
Collaborator Author

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

image

@joergbrech
Copy link
Contributor

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

@rainman110
Copy link
Collaborator Author

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
Copy link
Contributor

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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

@rainman110
Copy link
Collaborator Author

The history is clean again

Copy link

codecov bot commented Dec 8, 2023

Codecov Report

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

❗ 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

@@            Coverage Diff            @@
##             master     #971   +/-   ##
=========================================
  Coverage          ?   68.95%           
=========================================
  Files             ?      299           
  Lines             ?    26499           
  Branches          ?        0           
=========================================
  Hits              ?    18272           
  Misses            ?     8227           
  Partials          ?        0           
Files Coverage Δ
src/boolean_operations/GEOMAlgo_Splitter.cxx 30.61% <ø> (ø)
src/common/tiglcommonfunctions.cpp 74.58% <ø> (ø)
src/configuration/CCPACSConfiguration.cpp 57.14% <ø> (ø)
...c/control_devices/CControlSurfaceBorderBuilder.cpp 76.96% <ø> (ø)
src/wing/CCPACSWingSegment.cpp 84.24% <ø> (ø)
src/geometry/CTiglTriangularizer.cpp 85.71% <60.00%> (ø)

@joergbrech
Copy link
Contributor

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
Copy link
Contributor

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

@rainman110, could you verify this?

@joergbrech
Copy link
Contributor

joergbrech commented Dec 13, 2023

@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

rainman110 and others added 5 commits January 9, 2024 15:04
Note: The test WingSegmentGuideCurves.tiglWingGetSegmentUpperSurfaceAreaTrimmed
is failing due to some tolerance error. I did not look into it further.
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.
@joergbrech
Copy link
Contributor

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
Copy link
Contributor

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 DLR-SC/tigl-conda#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.

@joergbrech joergbrech changed the title Added support for OCCT 7.6.1 Added support for OCCT 7.6.2 Jan 9, 2024
@rainman110
Copy link
Collaborator Author

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

@joergbrech joergbrech merged commit ab2e704 into master Jan 9, 2024
17 checks passed
@joergbrech joergbrech deleted the occt_761 branch January 9, 2024 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TiglViewer: Wrong shader code with OCCT >= 7.6.0, Zebra-Stripes not working
2 participants