search

trelau / SMESH

Mesh module from the Salome Platform
GNU Lesser General Public License v2.1
50 stars 31 forks source link

Migrate to SMESH 9.6.0 #31

Closed trelau closed 3 years ago

trelau commented 3 years ago

This PR changes the nature of this project from containing sources files itself to using upstream repositories from Salome as much as possible. This is being done to make keeping up-to-date with the upstream sources easier and more robust. A few notable changes in this PR compared to the existing project:

trelau commented 3 years ago

@looooo @wwmayer thoughts on this PR?

looooo commented 3 years ago

@looooo @wwmayer thoughts on this PR?

I think I will go with the freecad internal smesh and will drop the conda-forge smesh support for now.

trelau commented 3 years ago

@looooo why is that?

looooo commented 3 years ago

@looooo why is that?

I do not want to revert netgen to an older version. I guess this will lead to some difficulties with the dependency-solver. So this is a temporary workaround to simplify the packaging for me.

The other option is to create a netgen 5.3 package. I guess there is no such package available for conda-forge right now.

trelau commented 3 years ago

@looooo in this project, netgen 5.3 is now a submodule, built from sources, and the nglib and headers are part of SMESH. So the SMESH project now comes shipped with netgen as another library, no external dependency.

put another, it's self-contained and easier to stay synced with upstream Salome. i think it could be adopted to built with the latest netgen. it's not too hard to apply patches to get the right interface in NETGENPlugin. But, Salome applies a bunch of patches to netgen 5.3 to make it work, and those can't be easily applied to later versions of netgen.

trelau commented 3 years ago

i guess the library could be renamed to SMESHnglib or something to avoid conflicts in case you have other nglib library in your path

wwmayer commented 3 years ago

This targets Salome 9.6.0 and OCCT 7.5.0

What exactly does this mean? Does this repo now directly depend on master of the smesh sub-module? So a regression there would automatically affect this repo, too?

Netgen 5.3 is used to match upstream. The Netgen API for v6+ seems to be changing so much that making it work with sources meant for v5.3 was becoming a pain and the benefits unclear. nglib is built in this project. Patches can be submitted for specific Netgen fixes if found to be useful.

IMO, this is not necessarily a bad idea. With recent netgen releases I couldn't see much technical improvements but mainly code refactorings and (questionable) usage of the newer C++ standards (C++17 is mandatory now).

PR #28 showed that netgen 6.2 technically even is a regression because the option MESHCONST_OPTSURFACE is not supported any more.

Uses VTK 9.0

Are older versions still supported or is VTK 9.0 or higher mandatory?

trelau commented 3 years ago

@wwmayer The submodules target a specific release of upstream sources, right now the v9.6.0 tag. They don't point straight to master, so if you go through the "build from sources" in the readme, it should show you how to clone the sources, prepare them with a python script locally, and then build like normal. As new upstream Salome sources are provided, the submodules in this repo can be updated (or not) as needed. So yes, a regression in the upstream sources can affect this project, but we can control when the upstream sources are used, and patches can be applied in this project as well. a good reason to write tests in this project for stuff that matters downstream, among other reasons.

I haven't tried building v9.6.0 with older versions of VTK. Upstream Salome uses VTK 9 now, so was trying to stay in sync with that and also the latest OCCT 7.5.0 on conda-forge is also using VTK 9, so trying to keep them all the same.

wwmayer commented 3 years ago

The submodules target a specific release of upstream sources, right now the v9.6.0 tag. They don't point straight to master, so if you go through the "build from sources" in the readme, it should show you how to clone the sources, prepare them with a python script locally, and then build like normal. As new upstream Salome sources are provided, the submodules in this repo can be updated (or not) as needed. So yes, a regression in the upstream sources can affect this project, but we can control when the upstream sources are used, and patches can be applied in this project as well. a good reason to write tests in this project for stuff that matters downstream, among other reasons

OK, this sounds good to me, then.

I haven't tried building v9.6.0 with older versions of VTK. Upstream Salome uses VTK 9 now, so was trying to stay in sync with that and also the latest OCCT 7.5.0 on conda-forge is also using VTK 9, so trying to keep them all the same.

If older versions of third party libraries are not supported then this will be problematic for FreeCAD as there we try to support all still maintained Linux distributions (mainly Debian and derivatives)

looooo commented 3 years ago

@looooo in this project, netgen 5.3 is now a submodule, built from sources, and the nglib and headers are part of SMESH. So the SMESH project now comes shipped with netgen as another library, no external dependency.

ok, thats a good point. so netgen from conda-forge and the netgen included within smesh shouldn't have any conflicing files?

put another, it's self-contained and easier to stay synced with upstream Salome. i think it could be adopted to built with the latest netgen. it's not too hard to apply patches to get the right interface in NETGENPlugin. But, Salome applies a bunch of patches to netgen 5.3 to make it work, and those can't be easily applied to later versions of netgen.

ok yes, I understand. I will try to build it for conda-forge, but not yet sure how to handle the submodules.

https://github.com/conda-forge/smesh-feedstock/pull/38

looooo commented 3 years ago

I see some issues with freecad compiied against this library and vtk9:

https://dev.azure.com/sppedflyer/feedstock-builds/_build/results?buildId=245&view=logs&j=d0d954b5-f111-5dc4-4d76-03b6c9d0cf7e&t=841356e0-85bb-57d8-dbbc-852e683d1642&l=2836

wwmayer commented 3 years ago

I see some issues with freecad compiied against this library and vtk9

It doesn't look like it's complicated to fix them. There is a change of the interface of some smesh classes (an int as argument has been dropped).

wwmayer commented 3 years ago

https://github.com/FreeCAD/FreeCAD/commit/135525a8066703059603be1a395986327e635568 ports FreeCAD to smesh 9.6

looooo commented 3 years ago

thanks @wwmayer . Indeed everything compiled fine for linux. Also doing some basic tests looks promising. I will continue with the scripts for osx and mac. Once thats done I think it's best to create a kind of a release of this repo (@trelau). Once that happened and everything is packaged I will start the occt7.5 migration for conda-forge.

wwmayer commented 3 years ago

Nice that you got it working. When building it on Linux I encountered two issues:

looooo commented 3 years ago

Somehow the GL.so file isn't used to link FreeCAD with. This results into linker errors on all FreeCAD modules that directly uses OpenGL functions. I had to manually fix it by adding the GL.so to the link.txt files. Did this work out of the box for you?

Not sure what you mean, but with conda (osx, and linux) I had an issue with wrong paths to opengl, I simply sed'ed them away: sed -i '380,384d' ${PREFIX}/lib/cmake/vtk-9.0/VTK-targets.cmake But in my case there were no linking errors. I am not sure but I think this is handled by conda somhow (linking to system-libraries is a difficult topic in the conda-world).

Regarding windows: I see a problem with finding lzma. Somehow this is related to vtk. But I am not sure if lzma is needed for FreeCAD.

looooo commented 3 years ago

And yes for sure, a crash is happening when closing freecad.

Program received signal SIGSEGV, Segmentation fault.
#0  /lib/x86_64-linux-gnu/libc.so.6(+0x3f040) [0x7fb1a55bc040]
#1  /home/lo/miniconda3/envs/freecad_vtk9/bin/../lib/libpython3.9.so.1.0(PyObject_GC_Del+0x1b) [0x7fb1a77d03bb]
#2  0x7fb1a8256018 in Gui::ViewProviderPythonFeatureImp::~ViewProviderPythonFeatureImp() from /home/lo/miniconda3/envs/freecad_vtk9/bin/../lib/libFreeCADGui.so+0x208
#3  0x7fb0b85a934d in Gui::ViewProviderPythonFeatureT<PartGui::ViewProviderPart>::~ViewProviderPythonFeatureT() from /home/lo/miniconda3/envs/freecad_vtk9/lib/PartGui.so+0x2d
#4  0x7fb1a7eb6bb4 in Gui::Document::~Document() from /home/lo/miniconda3/envs/freecad_vtk9/bin/../lib/libFreeCADGui.so+0x294
#5  0x7fb1a7eb730a in Gui::Document::~Document() from /home/lo/miniconda3/envs/freecad_vtk9/bin/../lib/libFreeCADGui.so+0xa

maybe also related to python3.9...

edit: yes I can't reproduce with python3.8 / vtk9 -> so it should be related to python3.9

to reproduce:

So it might be related to python features and py3.9 and for sure is not related to this topic at all ;)

looooo commented 3 years ago

the win build now stops at finding netgen:

CMake Error at cMake/FindNETGEN.cmake:112 (file):
  file STRINGS file
  "C:/Users/lo/miniconda3/conda-bld/debug_1611225804771/work/NETGEN_DIR_include-NOTFOUND/mydefs.hpp"
  cannot be read.
Call Stack (most recent call first):
  CMakeLists.txt:55 (find_package)

-- Cannot find NETGEN library.

edit: fixed with: https://github.com/looooo/freecad-feedstock/commit/b12b4cfdd3c4dcce43b5d0de0dd689807db7b398#diff-54cf74e113dd3f6d11e092fdb1d888ec82c69bdafbb15cfb6570c83ecad28f33R24

wwmayer commented 3 years ago

Not sure what you mean, but with conda (osx, and linux) I had an issue with wrong paths to opengl, I simply sed'ed them away:

vtk9 uses its own FindOpenGL.cmake module and there it fails to set the variable OPENGL_gl_LIBRARY, i.e. it's empty. Now, I found a way to fix it by changing the order of SetupOpenGL() and SetupSalomeSMESH()

In older SMESH versions there was the function SMDS_MeshElement::facesIterator() that has been removed now.

I realized that this was already broken in SMESH 7. Fixed with https://github.com/FreeCAD/FreeCAD/commit/f490852ab92389d5236eb31de846d464ac205ad5

looooo commented 3 years ago

vtk9 uses its own FindOpenGL.cmake module and there it fails to set the variable OPENGL_gl_LIBRARY, i.e. it's empty. Now, I found a way to fix it by changing the order of SetupOpenGL() and SetupSalomeSMESH()

great. I will test if this makes the sed-commands also obsolete.

There are still some tests failing: Bildschirmfoto von 2021-01-21 15-03-51

wwmayer commented 3 years ago

I only got the first three errors but not the last two. The three errors are related to an API change in SMESH 9. It's fixed with: https://github.com/FreeCAD/FreeCAD/commit/7066bfed678be13117489b1af2e7b6ae2768c200

looooo commented 3 years ago

@wwmayer thanks. I tested with an updated build and there are only two errors remaining. I will continue with reporting in the freecad forum as this is not related to smesh anymore.

What is still remianing are some tests with osx as we saw some errors with the tests.

opoplawski commented 3 years ago

Why was setting the SOVERSION for the libraries dropped in this patch?

looooo commented 3 years ago

the tests for freecad are somehow failing on osx (illegal instruction). Not sure if this is related to smesh /netgen.

trelau commented 3 years ago

@looooo it probably is. Remember we had to exclude the unit test on osx for feedstock. Hmm...probably worth opening up an issue in the SMESH repo. Will need to add osx support on GitHub actions to this repo I think. Probably will need someone with osx system to efficiently debug.

looooo commented 3 years ago

@trelau it's related to meshing with netgen. I can remember having similar issues with netgen some time ago. Maybe some compile option are the reason for the crash, but I am not sure

https://github.com/conda-forge/netgen-feedstock/blob/623566d586b3d1a28e989844d2488887b11a4ec5/recipe/build.sh#L17

looooo commented 3 years ago

@trelau @wwmayer I see also a crash in the tests with linux:

****************************************************************************************************
********** run FEM TestObjectOpen tests ************************************************************
**************************************************************************************************** 
load old document objects 
#12  0x7fd9747d8a1b in App::Application::openDocument(char const*, bool) from /home/conda/feedstock_root/build_artifacts/freecad_1614279970351/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/bin/../lib/libFreeCADApp.so+0xbb
#13  0x7fd9747e2b99 in App::Application::sOpenDocument(_object*, _object*, _object*) from /home/conda/feedstock_root/build_artifacts/freecad_1614279970351/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/bin/../lib/libFreeCADApp.so+0xe9
#14  /home/conda/feedstock_root/build_artifacts/freecad_1614279970351/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/bin/../lib/libpython3.8.so.1.0(PyCFunction_Call+0x66) [0x7fd97424c696]
#15  /home/conda/feedstock_root/build_artifacts/freecad_1614279970351/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/bin/../lib/libpython3.8.so.1.0(_PyObject_MakeTpCall+0x230) [0x7fd9741f4550]
#16  /home/conda/feedstock_root/build_artifacts/freecad_1614279970351/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/bin/../lib/libpython3.8.so.1.0(_PyEval_EvalFrameDefault+0x4ddd) [0x7fd97425e28d]
#17  /home/conda/feedstock_root/build_artifacts/freecad_1614279970351/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/bin/../lib/libpython3.8.so.1.0(+0x17aa74) [0x7fd974242a74]
#18  /home/conda/feedstock_root/build_artifacts/freecad_1614279970351/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/bin/../lib/libpython3.8.so.1.0(_PyEval_EvalFrameDefault+0x392) [0x7fd974259842]
#19  /home/conda/feedstock_root/build_artifacts/freecad_1614279970351/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/bin/../lib/libpython3.8.so.1.0(_PyFunction_Vectorcall+0xfb) [0x7fd9742265bb]
#20  /home/conda/feedstock_root/build_artifacts/freecad_1614279970351/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/bin/../lib/libpython3.8.so.1.0(_PyEval_EvalFrameDefault+0x6e3) [0x7fd974259b93]
#21  /home/conda/feedstock_root/build_artifacts/freecad_1614279970351/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/bin/../lib/libpython3.8.so.1.0(_PyEval_EvalCodeWithName+0x2ea) [0x7fd9741f5d9a]
#22  /home/conda/feedstock_root/build_artifacts/freecad_1614279970351/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/bin/../lib/libpython3.8.so.1.0(_PyFunction_Vectorcall+0x18e) [0x7fd97422664e]
#23  /home/conda/feedstock_root/build_artifacts/freecad_1614279970351/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/bin/../lib/libpython3.8.so.1.0(+0x17ad13) [0x7fd974242d13]

https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=282512&view=logs&jobId=d0d954b5-f111-5dc4-4d76-03b6c9d0cf7e&j=d0d954b5-f111-5dc4-4d76-03b6c9d0cf7e&t=841356e0-85bb-57d8-dbbc-852e683d1642

this build is using the freecad 0.19 tag. So it can be related to freecad too, as I didn't see this issue some days ago with an older snapshot of freecad.