Use `COMPONENTS` when finding VTK to avoid linking agains unnecessary modules #3140

taketwo · 2019-06-10T12:00:49Z

This supersedes #3032 and partially addresses #2969.

Unlike #3032 I decided to not split VTK components in separate dependency lists for IO and Visualization modules. First reason: there would need to be too many of such lists, because in fact Surface module also needs some VTK components, as well as some of the binaries in Tools. Second reason: even if we employ separate lists, it will only improve linking during library build. Once it comes down to PCLConfig, there are no mechanics in place to separate which VTK components are needed by which target, thus no gain for downstream projects.

For future reference, this is how I found the list of required VTK components. First I set the list to only include something obvious, e.g. vtkCommonCore. Then I built the library with ninja -k 0 > /tmp/errors to get errors about undefined references in all libs and executables. Finally, the errors were passed through the following script:

#!/usr/bin/env bash
# first argument: path to VTK libraries
# compiler output should be passed through stdin
 
dumps="/tmp/vtk_dums"
mkdir -p $dumps
for so in "$1"/*so; do
  filename=$(basename "$so")
  stem="${filename%.*}"
  nm -gC --defined-only "$so" | grep -oP "(?<=typeinfo for ).+" | sort | uniq  > "$dumps/$stem"
done

find_so () {
  for d in "$dumps"/*; do
    if grep -Fxq "$1" "$d"; then
      filename=$(basename "$d")
      echo "$filename"
      break
    fi
  done
}

grep -oP "(?<=undefined reference to \`)([^:]*)(?=::.*\')" < /dev/stdin | sort | uniq | while read -r line; do find_so "$line"; done | sort | uniq

This finds all undefined classes, searches in VTK shared objects, and outputs the list of SO files where the missing symbols were found.

This PR also includes a couple of minor cleanup commits.

I tested this on my Ubuntu workstation with both system VTK (6.3) and self-built VTK (8.2). The library with all options on successfully builds, as well as some of my downstream projects that I tried. It would be great if someone with Windows also gives this a try.

Edit: Closes #3032

SergioRAgostinho · 2019-06-11T12:03:21Z

Ping for test builds if you have time @UnaNancyOwen @SunBlack @claudiofantacci

claudiofantacci · 2019-06-11T18:30:40Z

I'll test this tomorrow. Ping me otherwise 👍

SunBlack · 2019-06-12T13:57:48Z

//EDIT: Got compile issue with MSVC 19.16 (debug & release) & VTK 8.2 (didn't saw the error message first, because of a CUDA issue).

Severity	Code	Description	Project	File	Line	Suppression State
Error	LNK2001	unresolved external symbol "__declspec(dllimport) protected: virtual bool __cdecl QVTKWidget::nativeEvent(class QByteArray const &,void *,long *)" (__imp_?nativeEvent@QVTKWidget@@MEAA_NAEBVQByteArray@@PEAXPEAJ@Z)	pcl_cc_tool_fpfh_estimation	D:\pcl_build\apps\cloud_composer\pcl_cc_tool_interface.lib(fpfh_item.obj)	1	
Error	LNK2001	unresolved external symbol "__declspec(dllimport) protected: virtual bool __cdecl QVTKWidget::paintCachedImage(void)" (__imp_?paintCachedImage@QVTKWidget@@MEAA_NXZ)	pcl_cc_tool_fpfh_estimation	D:\pcl_build\apps\cloud_composer\pcl_cc_tool_interface.lib(fpfh_item.obj)	1	
Error	LNK2001	unresolved external symbol "__declspec(dllimport) protected: virtual void __cdecl QVTKWidget::dropEvent(class QDropEvent *)" (__imp_?dropEvent@QVTKWidget@@MEAAXPEAVQDropEvent@@@Z)	pcl_cc_tool_fpfh_estimation	D:\pcl_build\apps\cloud_composer\pcl_cc_tool_interface.lib(fpfh_item.obj)	1	
...

Components I build:

The following subsystems will be built:
  common
  kdtree
  octree
  search
  sample_consensus
  filters
  2d
  geometry
  io
  features
  ml
  segmentation
  visualization
  surface
  registration
  keypoints
  tracking
  recognition
  stereo
  apps
       building: 
       |_ cloud_composer
       |_ modeler
       |_ point_cloud_editor
       not building: 
       |_ 3d_rec_framework: OpenNI was not found or was disabled by the user.
       |_ in_hand_scanner: OpenNI was not found or was disabled by the user.
  cuda_common
  cuda_io
  cuda_features
  cuda_segmentation
  cuda_sample_consensus
  cuda_apps
  outofcore
  examples
  gpu_containers
  gpu_utils
  gpu_octree
  gpu_features
  gpu_kinfu
  gpu_kinfu_large_scale
  gpu_people
  gpu_segmentation
  gpu_surface
  gpu_tracking
  people
  global_tests
  tests_2d
  tests_common
  tests_features
  tests_filters
  tests_geometry
  tests_io
  tests_kdtree
  tests_keypoints
  tests_people
  tests_octree
  tests_outofcore
  tests_recognition
  tests_registration
  tests_search
  tests_surface
  tests_segmentation
  tests_sample_consensus
  tests_visualization
  tools
The following subsystems will not be built:
  simulation: Requires external library glew.

Compilation failed for pcl_gpu_surface and pcl_cc_tool_fpfh_estimation.

claudiofantacci · 2019-06-12T14:06:42Z

Failing on my macOS 10.13.6 with standard options with the following error:

Claudio @ Claudios-MBP ~ 04:05 pm
└─ ~/GitHub/pcl/build_tmp [components {untracked}| ✔ ]
└─ $ make
[  0%] Building CXX object common/CMakeFiles/pcl_common.dir/src/point_types.cpp.o
[  0%] Building CXX object common/CMakeFiles/pcl_common.dir/src/pcl_base.cpp.o
[  0%] Building CXX object common/CMakeFiles/pcl_common.dir/src/io.cpp.o
[  0%] Building CXX object common/CMakeFiles/pcl_common.dir/src/common.cpp.o
[  1%] Building CXX object common/CMakeFiles/pcl_common.dir/src/correspondence.cpp.o
[  1%] Building CXX object common/CMakeFiles/pcl_common.dir/src/distances.cpp.o
[  1%] Building CXX object common/CMakeFiles/pcl_common.dir/src/parse.cpp.o
[  1%] Building CXX object common/CMakeFiles/pcl_common.dir/src/poses_from_matches.cpp.o
[  1%] Building CXX object common/CMakeFiles/pcl_common.dir/src/print.cpp.o
[  1%] Building CXX object common/CMakeFiles/pcl_common.dir/src/projection_matrix.cpp.o
[  2%] Building CXX object common/CMakeFiles/pcl_common.dir/src/time_trigger.cpp.o
[  2%] Building CXX object common/CMakeFiles/pcl_common.dir/src/gaussian.cpp.o
[  2%] Building CXX object common/CMakeFiles/pcl_common.dir/src/colors.cpp.o
[  2%] Building CXX object common/CMakeFiles/pcl_common.dir/src/feature_histogram.cpp.o
[  2%] Building CXX object common/CMakeFiles/pcl_common.dir/src/bearing_angle_image.cpp.o
[  3%] Building CXX object common/CMakeFiles/pcl_common.dir/src/range_image.cpp.o
[  3%] Building CXX object common/CMakeFiles/pcl_common.dir/src/range_image_planar.cpp.o
[  3%] Building C object common/CMakeFiles/pcl_common.dir/src/fft/kiss_fft.c.o
[  3%] Building C object common/CMakeFiles/pcl_common.dir/src/fft/kiss_fftr.c.o
[  3%] Linking CXX shared library ../lib/libpcl_common.dylib
Undefined symbols for architecture x86_64:
  "___kmpc_dispatch_init_4", referenced from:
      _.omp_outlined. in range_image.cpp.o
  "___kmpc_dispatch_next_4", referenced from:
      _.omp_outlined. in range_image.cpp.o
  "___kmpc_end_reduce_nowait", referenced from:
      _.omp_outlined. in range_image.cpp.o
  "___kmpc_for_static_fini", referenced from:
      _.omp_outlined. in kiss_fft.c.o
  "___kmpc_for_static_init_4", referenced from:
      _.omp_outlined. in kiss_fft.c.o
  "___kmpc_fork_call", referenced from:
      pcl::RangeImage::getOverlap(pcl::RangeImage const&, Eigen::Transform<float, 3, 2, 0> const&, int, float, int) const in range_image.cpp.o
      _kf_work in kiss_fft.c.o
  "___kmpc_global_thread_num", referenced from:
      pcl::RangeImage::getOverlap(pcl::RangeImage const&, Eigen::Transform<float, 3, 2, 0> const&, int, float, int) const in range_image.cpp.o
  "___kmpc_push_num_threads", referenced from:
      pcl::RangeImage::getOverlap(pcl::RangeImage const&, Eigen::Transform<float, 3, 2, 0> const&, int, float, int) const in range_image.cpp.o
  "___kmpc_reduce_nowait", referenced from:
      _.omp_outlined. in range_image.cpp.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [lib/libpcl_common.1.9.1.99.dylib] Error 1
make[1]: *** [common/CMakeFiles/pcl_common.dir/all] Error 2
make: *** [all] Error 2

However, it seems that the error is related to OpenMP. I have brew-installed libomp and it seems that the automatic parameter it gets are wrong. Infact, I had to do some CMake tricks in my project to use it properly under macOS. Being this out of the scope of this PR, I temporarily brew unlink libomp and compiling PCL with the default options.

Compilation went just fine on mi side with standard options 👍

Compilation fails if I enable CUDA and/or GPU with the following error:

/Users/Claudio/GitHub/pcl/gpu/octree/include/pcl/gpu/octree/device_format.hpp:81:8: error: non-aggregate type 'PtrStep<int>' cannot be initialized with an initializer list
return {(int *)(data).ptr(), (max_elems) * sizeof(int)}; 
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

SunBlack · 2019-06-12T14:51:06Z

cmake/Modules/FindQVTK.cmake

@@ -10,6 +10,7 @@
 if(";${VTK_MODULES_ENABLED};" MATCHES ";vtkGUISupportQt;" AND ";${VTK_MODULES_ENABLED};" MATCHES ";vtkRenderingQt;")
  set(VTK_USE_QVTK ON)
  set(QVTK_LIBRARY vtkRenderingQt vtkGUISupportQt)
+  list(APPEND PCL_VTK_COMPONENTS vtkRenderingQt vtkGUISupportQt)
 else()
  unset(QVTK_FOUND)


QVTK_FOUND will by only unset, but never set.

Hm, but it's not used anywhere in the project, so I suppose I'll just drop this.

Or, I'd rather drop VTK_USE_QVTK and switch to using VTK_FOUND in other places.

SunBlack · 2019-06-12T14:53:04Z

CMakeLists.txt

@@ -367,6 +388,8 @@ if(WITH_VTK AND NOT ANDROID)
      # safe to assume OpenGL backend
      set(VTK_RENDERING_BACKEND "OpenGL")
    endif()
+    list(APPEND PCL_VTK_COMPONENTS vtkRenderingContext${VTK_RENDERING_BACKEND})
+    find_package(VTK COMPONENTS ${PCL_VTK_COMPONENTS})
    message(STATUS "VTK_MAJOR_VERSION ${VTK_MAJOR_VERSION}, rendering backend: ${VTK_RENDERING_BACKEND}")


include(${VTK_USE_FILE}) will be only called in case (PCL_SHARED_LIBS OR (NOT (PCL_SHARED_LIBS) AND NOT (VTK_BUILD_SHARED_LIBS))) returns true., Shouldn't it be always? (You removed some other calls to this include, so it could be an issue now).

//Edit: Just didn't noticed else tree of this code, that VTKwill be disabled otherwise. Nevertheless this whole code block should be checked. E.g. HAVE_VTK is undefined in case WITH_VTK is true, but VTK was not found.

E.g. HAVE_VTK is undefined in case WITH_VTK is true, but VTK was not found.

I think this is okay because undefined variables evaluate to false in conditionals.

taketwo · 2019-06-13T09:02:30Z

@claudiofantacci thanks for giving this a try. Let's indeed branch out the OpenMP and CUDA errors to separate issues.

@SunBlack also thanks for checking. Unfortunately I can not reproduce your problem on Ubuntu, both pcl_gpu_surface and pcl_cc_tool_fpfh_estimation build fine. I wonder what's special about these two, why specifically they fail, though there are other apps using QVTKWidget.

In the meantime, I pushed an update that merges FindQVTK script with the main CMakeLists at the same time removing all the obsolete/unnecessary stuff.

SunBlack · 2019-06-13T12:58:01Z

@SunBlack also thanks for checking. Unfortunately I can not reproduce your problem on Ubuntu, both pcl_gpu_surface and pcl_cc_tool_fpfh_estimation build fine. I wonder what's special about these two, why specifically they fail, though there are other apps using QVTKWidget.

I had rebuild it today again from scratch and I just had CUDA errors left. When #3152 is merged (and merged into this branch), I can check it again (tomorrow or monday).

This file is already included in the top-level CMakeLists.

This involves a few smaller changes: * only look for QVTK if Qt itself is enabled * use QVTK_FOUND instead of VTK_USE_QVTK do determine QVTK availability in the rest of the scripts * drop QVTK_LIBRARY since QVTK components are now together with the rest and are linked into pcl_visualization

taketwo · 2019-06-13T14:38:23Z

Rebased on top of current master, ready for new testing.

SergioRAgostinho · 2019-06-13T15:23:59Z

This is building for me on OS X. Cannot try CUDA without the commits on #3131.

taketwo · 2019-06-13T16:21:27Z

Hm, seems like there is a problem when using with non-system VTK (8.2). Although I don't get any linker errors, trying to run binaries results in:

./bin/pcl_viewer: error while loading shared libraries: libvtksys-8.2.so.1: cannot open shared object file: No such file or directory

When I build with system VTK (6.3), there is no problem. Looks like linker directories are not properly set... will need to investigate further.

taketwo · 2019-06-13T21:43:55Z

This has nothing to do with the PR and also affects master. On newer Ubuntu versions the following use-case is not working:

Build VTK from source, install to a non-conventional location (something that is not on the standard LD search paths, e.g. /opt/vtk-8.2)
Configure and build PCL with that VTK
Run any PCL binary that has to do with visualization

This thread (esp. comment 4) provides an excellent summary of what's wrong, I couldn't explain better. Not sure what to do about this at the moment. But it should not block this PR.

SergioRAgostinho · 2019-06-14T08:17:31Z

Should we merge this in the meantime while continuing the discussion? I'm ok with that.
You mentioned this is only happening with VTK 8.2 and not with 6.3. VTK 8.2 you compiled manually and installed in /opt. Is VTK 6.3 installed under the same conditions or is it the one supplied by Ubuntu?
Confirm my interpretation, even if you properly set the linking directories to find all direct and indirect dependencies, while building some visualization enabled pcl cli binary, at runtime the indirect dependency will not be found? For compiling things are ok and ld doesn't complain, but at runtime it just drops the previously used path for indirect dependencies.

taketwo · 2019-06-14T08:46:13Z

I'd wait for @SunBlack to try again and then merge.
VTK 6.3 was supplied by Ubuntu, i.e. it was in a standard location. According to my understanding thus the problem has nothing to do with VTK version.
Yes, this is how I understand the problem. At link time /opt/vtk-8.2/lib is specified explicitly with -Wl,-rpath, so transitive dependencies are discovered. And at run time not. The solution would be to make sure that VTK libraries set their own RUNPATH correctly, but so far I was not able to figure out how to do this.

SergioRAgostinho · 2019-06-14T08:58:21Z

3. Yes, this is how I understand the problem. At link time /opt/vtk-8.2/lib is specified explicitly with -Wl,-rpath, so transitive dependencies are discovered. And at run time not. The solution would be to make sure that VTK libraries set their own RUNPATH correctly, but so far I was not able to figure out how to do this.

Which means, it will need to be fixed patched upstream. Well, something to keep in mind when users come reporting issues related to this.

taketwo · 2019-06-14T09:10:12Z

It's more complicated than that. I'm not an expert, but as far as I understand generally you don't want to set RPATH on installed libraries, so the default CMake setting is to clear RPATH when installing. So it's not a bug in VTK really, it's by design. What I'm missing is an option to override this behavior for such case as mine. Maybe there is a way and I just haven't found it yet.

SergioRAgostinho · 2019-06-14T09:46:57Z

Maybe there is a way and I just haven't found it yet.

Wouldn't setting CMAKE_SKIP_INSTALL_RPATH to OFF while building and installing your custom installation of VTK 8.2, override this behavior?

SunBlack · 2019-06-14T09:49:57Z

I'd wait for @SunBlack to try again and then merge.

Release build works and Debug just has a compile error, which is not related to this PR. So everything seems to be fine :)

example_nurbs_viewer_surface.obj : error LNK2019: unresolved external symbol ON_ErrorEx referenced in function "public: class ONX_Model_Object & __cdecl ON_ClassArray<class ONX_Model_Object>::operator[](int)" (??A?$ON_ClassArray@VONX_Model_Object@@@@QEAAAEAVONX_Model_Object@@H@Z)

taketwo · 2019-06-14T10:02:45Z

Wouldn't setting CMAKE_SKIP_INSTALL_RPATH to OFF while building and installing your custom installation of VTK 8.2, override this behavior?

Yep. It's off:

$ grep "INSTALL_RPATH:BOOL" < CMakeCache.txt
CMAKE_SKIP_INSTALL_RPATH:BOOL=NO

However during installation RPATH is net to "":

-- Installing: /opt/vtk-8.2.0/lib/libvtkViewsQt-8.2.so.1
-- Set runtime path of "/opt/vtk-8.2.0/lib/libvtkViewsQt-8.2.so.1" to ""

@SunBlack thanks!

SergioRAgostinho · 2019-06-14T10:17:44Z

Is INSTALL_RPATH an empty variable?

Edit: It probably is not set, otherwise it would have been picked up in your grep.

taketwo · 2019-06-14T10:27:10Z

No, it's not set. If I specify it explicitly:

cmake .. -DCMAKE_INSTALL_RPATH=/opt/vtk-8.2.0/lib

Then installed libs get RUNPATH set as expected:

$ readelf -d /opt/vtk-8.2.0/lib/libvtkCommonCore-8.2.so.1 | grep PATH
0x000000000000001d (RUNPATH)            Library runpath: [/opt/vtk-8.2.0/lib]

And I can run PCL executables. So we have a hacky solution to offer people, though it's not a PCL problem per se.

taketwo added module: cmake needs: testing Specify why not closed/merged yet labels Jun 10, 2019

taketwo added this to the pcl-1.10.0 milestone Jun 10, 2019

SergioRAgostinho approved these changes Jun 10, 2019

View reviewed changes

SergioRAgostinho mentioned this pull request Jun 11, 2019

1.10.0 Release #2772

Closed

11 tasks

SunBlack reviewed Jun 12, 2019

View reviewed changes

SergioRAgostinho approved these changes Jun 13, 2019

View reviewed changes

SunBlack mentioned this pull request Jun 13, 2019

Remove code for CUDA below 7.5, update version checks #3152

Merged

taketwo added 6 commits June 13, 2019 16:37

Do not include VTK_USE_FILE in modules

5bfb478

This file is already included in the top-level CMakeLists.

Remove unused variable VTK_SMOOTHING_TARGET_LINK_LIBRARIES

cfd94b9

Explicitly list required VTK components

af22913

Do not report VTK status again in cuda/apps/CMakeLists

4e87324

Remove superfluous set(VTK_FOUND TRUE) and minor formatting

a9e27d8

taketwo force-pushed the vtk-components branch from 5618800 to a9e27d8 Compare June 13, 2019 14:38

SergioRAgostinho merged commit 55b909b into PointCloudLibrary:master Jun 14, 2019

SergioRAgostinho removed the needs: testing Specify why not closed/merged yet label Jun 14, 2019

taketwo deleted the vtk-components branch June 14, 2019 10:23

taketwo changed the title ~~Use COMPONENTS when finding VTK~~ Use COMPONENTS when finding VTK to avoid linking agains unnecessary modules Jan 14, 2020

taketwo mentioned this pull request Feb 22, 2020

Linking configuration is not optimized #2969

Closed

larshg mentioned this pull request Aug 10, 2020

[compile error] Fix openNURBS compile issue under Windows #4314

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `COMPONENTS` when finding VTK to avoid linking agains unnecessary modules #3140

Use `COMPONENTS` when finding VTK to avoid linking agains unnecessary modules #3140

taketwo commented Jun 10, 2019 •

edited by SergioRAgostinho

Loading

SergioRAgostinho commented Jun 11, 2019

claudiofantacci commented Jun 11, 2019

SunBlack commented Jun 12, 2019 •

edited

Loading

claudiofantacci commented Jun 12, 2019 •

edited

Loading

SunBlack Jun 12, 2019

taketwo Jun 13, 2019

taketwo Jun 13, 2019

SunBlack Jun 12, 2019 •

edited

Loading

taketwo Jun 13, 2019

taketwo commented Jun 13, 2019

SunBlack commented Jun 13, 2019

taketwo commented Jun 13, 2019

SergioRAgostinho commented Jun 13, 2019

taketwo commented Jun 13, 2019

taketwo commented Jun 13, 2019

SergioRAgostinho commented Jun 14, 2019

taketwo commented Jun 14, 2019

SergioRAgostinho commented Jun 14, 2019 •

edited

Loading

taketwo commented Jun 14, 2019

SergioRAgostinho commented Jun 14, 2019 •

edited

Loading

SunBlack commented Jun 14, 2019

taketwo commented Jun 14, 2019

SergioRAgostinho commented Jun 14, 2019 •

edited

Loading

taketwo commented Jun 14, 2019

Use COMPONENTS when finding VTK to avoid linking agains unnecessary modules #3140

Use COMPONENTS when finding VTK to avoid linking agains unnecessary modules #3140

Conversation

taketwo commented Jun 10, 2019 • edited by SergioRAgostinho Loading

SergioRAgostinho commented Jun 11, 2019

claudiofantacci commented Jun 11, 2019

SunBlack commented Jun 12, 2019 • edited Loading

claudiofantacci commented Jun 12, 2019 • edited Loading

SunBlack Jun 12, 2019

Choose a reason for hiding this comment

taketwo Jun 13, 2019

Choose a reason for hiding this comment

taketwo Jun 13, 2019

Choose a reason for hiding this comment

SunBlack Jun 12, 2019 • edited Loading

Choose a reason for hiding this comment

taketwo Jun 13, 2019

Choose a reason for hiding this comment

taketwo commented Jun 13, 2019

SunBlack commented Jun 13, 2019

taketwo commented Jun 13, 2019

SergioRAgostinho commented Jun 13, 2019

taketwo commented Jun 13, 2019

taketwo commented Jun 13, 2019

SergioRAgostinho commented Jun 14, 2019

taketwo commented Jun 14, 2019

SergioRAgostinho commented Jun 14, 2019 • edited Loading

taketwo commented Jun 14, 2019

SergioRAgostinho commented Jun 14, 2019 • edited Loading

SunBlack commented Jun 14, 2019

taketwo commented Jun 14, 2019

SergioRAgostinho commented Jun 14, 2019 • edited Loading

taketwo commented Jun 14, 2019

Use `COMPONENTS` when finding VTK to avoid linking agains unnecessary modules #3140

Use `COMPONENTS` when finding VTK to avoid linking agains unnecessary modules #3140

taketwo commented Jun 10, 2019 •

edited by SergioRAgostinho

Loading

SunBlack commented Jun 12, 2019 •

edited

Loading

claudiofantacci commented Jun 12, 2019 •

edited

Loading

SunBlack Jun 12, 2019 •

edited

Loading

SergioRAgostinho commented Jun 14, 2019 •

edited

Loading

SergioRAgostinho commented Jun 14, 2019 •

edited

Loading

SergioRAgostinho commented Jun 14, 2019 •

edited

Loading