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

Implement the ConvexHullHelper class #51

Merged
merged 15 commits into from
Jun 21, 2020

Conversation

GiulioRomualdi
Copy link
Member

This PR introduces the first version of the ConvexHullHelper class inside the Planner Component.
In order to use the helper, and in general to compile the component, the Qhull library is now required.

@GiulioRomualdi
Copy link
Member Author

GiulioRomualdi commented Jun 18, 2020

On macOS is failing because of qhull/qhull#65. On windows, I'm investigating (I think it's something related to microsoft/vcpkg#5574) I don't have any idea

@traversaro
Copy link
Collaborator

On windows, I'm investigating (I think it's something related to microsoft/vcpkg#5574) I don't have any idea

The CI seems to fail on:

2020-06-18T13:15:52.4856273Z qhullcpp.lib(QhullPointSet.obj) : error LNK2001: unresolved external symbol qh_setendpointer [D:\a\bipedal-locomotion-framework\bipedal-locomotion-framework\build\src\Planners\Contact.vcxproj]
2020-06-18T13:15:52.4856800Z qhullcpp.lib(QhullFacetSet.obj) : error LNK2001: unresolved external symbol qh_setendpointer [D:\a\bipedal-locomotion-framework\bipedal-locomotion-framework\build\src\Planners\Contact.vcxproj]
2020-06-18T13:15:52.4857127Z qhullcpp.lib(QhullFacetList.obj) : error LNK2001: unresolved external symbol qh_setendpointer [D:\a\bipedal-locomotion-framework\bipedal-locomotion-framework\build\src\Planners\Contact.vcxproj]
2020-06-18T13:15:52.4857455Z qhullcpp.lib(QhullFacet.obj) : error LNK2001: unresolved external symbol qh_setendpointer [D:\a\bipedal-locomotion-framework\bipedal-locomotion-framework\build\src\Planners\Contact.vcxproj]
2020-06-18T13:15:52.4857972Z qhullcpp.lib(QhullVertex.obj) : error LNK2001: unresolved external symbol qh_setendpointer [D:\a\bipedal-locomotion-framework\bipedal-locomotion-framework\build\src\Planners\Contact.vcxproj]
2020-06-18T13:15:52.4859050Z qhullcpp.lib(QhullVertexSet.obj) : error LNK2001: unresolved external symbol qh_setendpointer [D:\a\bipedal-locomotion-framework\bipedal-locomotion-framework\build\src\Planners\Contact.vcxproj]
2020-06-18T13:15:52.4859379Z qhullcpp.lib(QhullQh.obj) : error LNK2019: unresolved external symbol qh_memcheck referenced in function "public: __cdecl orgQhull::QhullQh::~QhullQh(void)" (??1QhullQh@orgQhull@@QEAA@XZ) [D:\a\bipedal-locomotion-framework\bipedal-locomotion-framework\build\src\Planners\Contact.vcxproj]
2020-06-18T13:15:52.5165659Z D:\a\bipedal-locomotion-framework\bipedal-locomotion-framework\build\bin\Release\BipedalLocomotionFrameworkContact.dll : fatal error LNK1120: 2 unresolved externals [D:\a\bipedal-locomotion-framework\bipedal-locomotion-framework\build\src\Planners\Contact.vcxproj]

It seems that the symbols qh_setendpointer and qh_memcheck are not found. This could mean that either

  • That they are present in some qhull library, but that library is not correctly linked
  • The symbols are present in some qhull library, but with some mangling problem (for example they have been compiled as C++ mangled symbols, but the consuming software expect them to have C mangling)
  • (only if qhull is build as shared) the symbols are not correctly exported

To debug this kind of issues, I would start to check if those symbols are actually defined somewhere by using https://github.com/lucasg/Dependencies , but you can also concentrate for now on the walking planner and solve this fun windows problems later. : )

@GiulioRomualdi
Copy link
Member Author

@traversaro, I'm trying to replicate the error on windows (right now I'm installing the superbuild)

@GiulioRomualdi
Copy link
Member Author

GiulioRomualdi commented Jun 18, 2020

qhullcpp is build only as a static library (There is no way to compile it as a shared one). For this reason, I cannot analyze the library with Dependencies
If I build the bipedal-locomotion-framework as static library I don't have any kind of linking problems however the Tests does not compile with the usual error

1>------ Build started: Project: ConvexHullHelperUnitTests, Configuration: Release x64 ------
1>qhullcpp.lib(QhullPointSet.obj) : error LNK2001: unresolved external symbol qh_setendpointer
1>qhullcpp.lib(QhullFacetSet.obj) : error LNK2001: unresolved external symbol qh_setendpointer
1>qhullcpp.lib(QhullFacetList.obj) : error LNK2001: unresolved external symbol qh_setendpointer
1>qhullcpp.lib(QhullFacet.obj) : error LNK2001: unresolved external symbol qh_setendpointer
1>qhullcpp.lib(QhullVertex.obj) : error LNK2001: unresolved external symbol qh_setendpointer
1>qhullcpp.lib(QhullVertexSet.obj) : error LNK2001: unresolved external symbol qh_setendpointer
1>qhullcpp.lib(QhullQh.obj) : error LNK2019: unresolved external symbol qh_memcheck referenced in function "public: __cdecl orgQhull::QhullQh::~QhullQh(void)" (??1QhullQh@orgQhull@@QEAA@XZ)
1>C:\Users\gromualdi\robot-code\bipedal-locomotion-framework\build\bin\Release\ConvexHullHelperUnitTests.exe : fatal error LNK1120: 2 unresolved externals
1>Done building project "ConvexHullHelperUnitTests.vcxproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 3 up-to-date, 0 skipped ==========

qh_setendpointer is defined in qhull_r that it's linked as a private library our component

Edit 1: I noticed that qhull_r should be compiled as SHARED library (see here) however when I build qhull on my windows machine the generated files are .lib
image. On the other hand on Linux the generated files are .so. Here the GitHub Action

**Edit2: ** Compiling qhull from visual studio I noticed that two versions of qhull_r are installed

18>------ Build started: Project: INSTALL, Configuration: Release x64 ------
18>-- Install configuration: "Release"
18>-- Up-to-date: C:/Users/gromualdi/robot-code/robotology-superbuild/build/install/lib/qhullcpp.lib
18>-- Up-to-date: C:/Users/gromualdi/robot-code/robotology-superbuild/build/install/lib/qhullstatic.lib
18>-- Up-to-date: C:/Users/gromualdi/robot-code/robotology-superbuild/build/install/lib/qhullstatic_r.lib
18>-- Up-to-date: C:/Users/gromualdi/robot-code/robotology-superbuild/build/install/lib/qhull_r.lib
18>-- Up-to-date: C:/Users/gromualdi/robot-code/robotology-superbuild/build/install/bin/qhull_r.dll

@traversaro Do you think this may be the cause of the problem?

@traversaro
Copy link
Collaborator

**Edit2: ** Compiling qhull from visual studio I noticed that two versions of qhull_r are installed

On Windows, shared library are composed by two files: the import library .lib that is used during the linking process, and the actual shared library .dll that is used during the loading. See https://en.wikipedia.org/wiki/Dynamic-link_library#Import_libraries .

@GiulioRomualdi
Copy link
Member Author

This commit Homebrew/homebrew-core@6ca6c77 introduces qhull 2020.1 on brew. This fixes all the problems on macOS

@GiulioRomualdi
Copy link
Member Author

Discussing with @traversaro we realized that enabling the linking of qhull through vcpkg is a complex task.
Currently, we have two separates problems:

  1. the library cppqhull is compiled only as a static library. And when the library is compiled using vcpkg with:
    ./vcpkg install qhull:x64-windows
    
    the cppqhull is deleted. Check here.
  2. When find(Qhull REQUIRED) is called the following errors arise (This is the first error)
    CMake Error at C:/Users/gromualdi/robot-code/vcpkg/installed/x64-windows/share/qhull/QhullTargets-debug.cmake:9 (set_property):
    set_property could not find TARGET Qhull::qhullcpp_d.  Perhaps it has not yet been created.
    Call Stack (most recent call first):
    C:/Users/gromualdi/robot-code/vcpkg/installed/x64-windows/share/qhull/QhullTargets.cmake:140 (include)
    C:/Users/gromualdi/robot-code/vcpkg/installed/x64-windows/share/qhull/QhullConfig.cmake:1 (include)
    C:/Users/gromualdi/robot-code/vcpkg/scripts/buildsystems/vcpkg.cmake:331 (_find_package)
    cmake/BipedalLocomotionFrameworkFindDependencies.cmake:131 (find_package)
    CMakeLists.txt:19 (include)
    
    This issue is already known in the vcpkg community [QHull] Not found using cmake microsoft/vcpkg#9836 and a PR has been already proposed [qhull] Fix target cannot be found by cmake microsoft/vcpkg#9923. Unfortunately, this PR breaks the compatibility with PCL.

For the time being, we decided to drop the support for windows for this particular component.
Possible solutions are:

  1. Write our custom FindQhull.cmake function
  2. Avoid using vcpkg for compiling Qhull. In this case, we should solve the problem Implement the ConvexHullHelper class #51 (comment)

Since the compilation on Windows is an important feature that I do not want to drop, I consider this solution only temporary.

On the light of what I just explained. I will remove the qhull compilation from windows in .github/workflows/ci.yml

@GiulioRomualdi
Copy link
Member Author

@S-Dafarra , @MiladShafiee @prashanthr05 @traversaro when you have time you can review the PR 😄

@GiulioRomualdi
Copy link
Member Author

Thanks to 63e153e and 58b1d2a Now we can compile the library in all the platforms. Before merging 474f696 has to be reverted/removed

@GiulioRomualdi GiulioRomualdi linked an issue Jun 19, 2020 that may be closed by this pull request
Copy link
Collaborator

@traversaro traversaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable the compilation of the Planner component on windows
2 participants