[do not merge/ to discuss] freecad needs some adoptions of netgen #4

looooo · 2017-09-15T13:45:32Z

As you may know, FreeCAD uses Netgen to generate meshes. While this was working quite good for older releases, there were introduced some changes in the recent Netgen version which had to be patched from our side to get Netgen work with FreeCAD.

While some of these patches can be applied directly, others are maybe more difficult to incorporate.

luzpaz · 2017-10-17T18:16:28Z

ping @JSchoeberl @mhochsteger

JSchoeberl · 2017-10-17T18:34:55Z

libsrc/interface/nginterface.cpp

@@ -118,122 +118,122 @@ void Ng_LoadMeshFromStream ( istream & input )

 void Ng_LoadMesh (const char * filename)
 {
-  {


why do you comment this code ?

I think it was a problem with gcc4.8. This is more a problem with conda. So for sure this shouldn't be merged. I just wanted to start a little discussion to see which stuff has potential to be accepted...

mhochsteger

I'm fine with the CMake changes concerning USE_GUI, the DLL_HEADER stuff and the changes in occgeom.cpp

Concerning the install path for python modules:
We are providing installers (msi, dmg) for Windows and MacOS, so we need relocatable packages containing also the python files.
In addition, we don't know where the user installed Python.
It should be possible for you to set NG_INSTALL_DIR_PYTHON at configure time to your preferred value.

Concerning pybind headers: They are needed by dependent packages (most notably NGSolve). I could add an option do disable installing them but would rather have them installed by default.

mhochsteger · 2017-10-18T08:26:25Z

CMakeLists.txt

@@ -207,7 +228,7 @@ if(WIN32)
 else(WIN32)
    # build shared libraries
    set(NG_LIB_TYPE SHARED)
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++14")


I think about raising the minimum CMake version to 3.1 and using
CXX_STANDARD instead.

is 14 actually really necessary? I think that was my problem.

mhochsteger · 2017-10-18T08:32:31Z

nglib/CMakeLists.txt

-install(FILES nglib.h DESTINATION ${NG_INSTALL_DIR_INCLUDE}/include COMPONENT netgen_devel)
+install(FILES nglib.h DESTINATION ${NG_INSTALL_DIR_INCLUDE}/nglib COMPONENT netgen_devel)
+install(FILES nglib.h DESTINATION ${NG_INSTALL_DIR_INCLUDE} COMPONENT netgen_devel)
+


Why do you install nglib.h four times?

I did this after some frustration with the cmake of FreeCAD. The proper solution is to change the FindNetgen used in FreeCAD.
But the place where it got placed originally is also quite strange... /include/netgen/include .

Concerning the include dir structure: That's because we maintain the relative paths of the header files in the source code. The headers in
libsrc/include contain statements like

#include "../gprim/gprim.hpp"

Therefore we need an additional "include" in the path.

Concerning the FindNetgen.cmake in FreeCAD: Are there reasons not to use NetgenConfig.cmake and netgen-targets.cmake provided by Netgen itself?

I will have a look at the default cmake, but I think we need our custom FindNetgen, because we are also using older versions of netgen which was build without cmake...

mhochsteger · 2017-10-18T08:32:39Z

libsrc/meshing/meshtype.hpp

-    Element2d (Element2d &&) = default;
-    Element2d & operator= (const Element2d &) = default;
-    Element2d & operator= (Element2d &&) = default;
+    DLL_HEADER Element2d () = default;


When a whole class is exported, inline member functions are not inlined anymore. Therefore we prefer exporting members explicitly. See here for example.

looooo · 2017-10-19T20:39:34Z

In addition, we don't know where the user installed Python.
It should be possible for you to set NG_INSTALL_DIR_PYTHON at configure time to your preferred value.

Why you need this information? Also you can always get this information if python is directly available, and if it is not directly available there will be problems anyway.

luzpaz · 2017-11-08T19:07:04Z

In addition, we don't know where the user installed Python.
It should be possible for you to set NG_INSTALL_DIR_PYTHON at configure time to your preferred value.

Why you need this information? Also you can always get this information if python is directly available, and if it is not directly available there will be problems anyway.

@mhochsteger care to weigh in?

looooo · 2017-11-08T21:02:04Z

Regarding this issue: I see now, that the method to use python to find out where to install the library is also not the best way. One problem I had with this, are local installation. Python will always point to the main site-package/dist-package in the root dir, if python is installed in a root directory...
But sometimes there is the need to install something locally... With conda this is no problem as one can install conda locally and therefor python is also locally. No idea how to proceed with this.

mhochsteger · 2017-11-13T12:35:45Z

I think I understand the problem now. We have to distinguish between USE_SUPERBUILD=ON and USE_SUPERBUILD=OFF.

Let NG_INSTALL_DIR_PYTHON=${PYTHON_EXECUTABLE} -c "from distutils.sysconfig import get_python_lib; print(get_python_lib(1,0,''))"
for instance NG_INSTALL_DIR_PYTHON=lib/python3.6/site-packages

WITH superbuild the current behavior is to install python modules to ${CMAKE_INSTALL_PREFIX}/${NG_INSTALL_DIR_PYTHON}, which is fine for local and global installations.

WITHOUT superbuild, NG_INSTALL_DIR_PYTHON is not set at all, which leads to errors at configure time. I will fix that and hope this sorts out the problem.

looooo · 2017-11-13T13:21:22Z

can we check for python default install dir if no NG_INSTALL_DIR_PYTHON is given?

mhochsteger · 2017-11-13T13:29:51Z

Yes, that's whats happening now. If nothing is set by the user, it will install to
${PYTHON_EXECUTABLE} -c "from distutils.sysconfig import get_python_lib; print(get_python_lib(1,0,''))".
If you set NG_INSTALL_DIR_PYTHON at configure time, it will be used instead.

looooo · 2017-11-13T15:35:11Z

👍

Includes parts of pull request by looooo #4

mhochsteger · 2018-01-24T08:42:34Z

libsrc/meshing/meshtype.hpp

-    Element2d & operator= (const Element2d &) = default;
-    Element2d & operator= (Element2d &&) = default;
+    DLL_HEADER Element2d () = default;
+    DLL_HEADER Element2d (const Element2d &) = default;


DLL_HEADER is not needed for default constructors (Tested with VS2015 on Windows). The others functions are exported with commit 0b411e1.

mhochsteger · 2018-01-24T08:45:01Z

libsrc/occ/occgenmesh.cpp

@@ -1291,7 +1291,7 @@ namespace netgen
      if (mparam.perfstepsstart <= MESHCONST_ANALYSE)
      {
        // delete mesh;
-        // mesh = make_shared<Mesh>();
+         mesh = make_shared<Mesh>();


We need to preserve the given pointer in order to visualize the mesh during mesh generation. I assume the following works for you:

if(mesh.get() == nullptr) mesh = make_shared<Mesh>();

yes, I have experimented with exactly this and it works for us. It seems this is the missing part which makes netgen again usable for FreeCAD.
Thanks for all the work!

looooo · 2018-02-17T10:42:41Z

I think everything is included. I will make a PR for the new changes.

JSchoeberl reviewed Oct 17, 2017

View reviewed changes

mhochsteger reviewed Oct 18, 2017

View reviewed changes

peterlama mentioned this pull request Jan 1, 2018

Homebrew/science will be deprecated at the end of the year FreeCAD/homebrew-freecad#55

Closed

mhochsteger added a commit that referenced this pull request Jan 23, 2018

install mydefs.hpp, some DLL_HEADER

0b411e1

Includes parts of pull request by looooo #4

mhochsteger reviewed Jan 24, 2018

View reviewed changes

Manually apply netgen53ForSalome.patch

defbe25

looooo force-pushed the master branch 3 times, most recently from 22756da to 252a0c1 Compare February 13, 2018 18:11

nglib: delete testout on Ng_Exit

91c9b5a

looooo force-pushed the master branch from 252a0c1 to 91c9b5a Compare February 17, 2018 09:00

looooo closed this Feb 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[do not merge/ to discuss] freecad needs some adoptions of netgen #4

[do not merge/ to discuss] freecad needs some adoptions of netgen #4

looooo commented Sep 15, 2017 •

edited

luzpaz commented Oct 17, 2017

JSchoeberl Oct 17, 2017

looooo Oct 17, 2017

mhochsteger left a comment •

edited

mhochsteger Oct 18, 2017

looooo Oct 19, 2017

mhochsteger Oct 18, 2017

looooo Oct 19, 2017

mhochsteger Nov 13, 2017

looooo Nov 13, 2017

mhochsteger Oct 18, 2017

looooo commented Oct 19, 2017

luzpaz commented Nov 8, 2017

looooo commented Nov 8, 2017

mhochsteger commented Nov 13, 2017

looooo commented Nov 13, 2017

mhochsteger commented Nov 13, 2017

looooo commented Nov 13, 2017

mhochsteger Jan 24, 2018

mhochsteger Jan 24, 2018

looooo Jan 24, 2018

looooo commented Feb 17, 2018

[do not merge/ to discuss] freecad needs some adoptions of netgen #4

[do not merge/ to discuss] freecad needs some adoptions of netgen #4

Conversation

looooo commented Sep 15, 2017 • edited

luzpaz commented Oct 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhochsteger left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

looooo commented Oct 19, 2017

luzpaz commented Nov 8, 2017

looooo commented Nov 8, 2017

mhochsteger commented Nov 13, 2017

looooo commented Nov 13, 2017

mhochsteger commented Nov 13, 2017

looooo commented Nov 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

looooo commented Feb 17, 2018

looooo commented Sep 15, 2017 •

edited

mhochsteger left a comment •

edited