RobotLocomotion/spartan

Director doesn't build with drake/master

manuelli opened this issue · 15 comments

manuelli commented

Attempting to build spartan with drake/master. The drake sha I am using is d4b3bde6de2b89f21ad2fe5df998c069dbd9407c.

Drake builds fine but then director has the build error. Looks like the vtk version that drake is providing is no longer compatible with director. Has the vtk version changed recently?

@patmarion @fbudin69500

/home/manuelli/spartan/build/director/src/pcl/surface/src/vtk_smoothing/vtk_utils.cpp: In static member function 'static int pcl::VTKUtils::vtk2mesh(const vtkSmartPointer<vtkPolyData>&, pcl::PolygonMesh&)':
/home/manuelli/spartan/build/director/src/pcl/surface/src/vtk_smoothing/vtk_utils.cpp:124:20: error: 'class vtkUnsignedCharArray' has no member named 'GetTupleValue'
       poly_colors->GetTupleValue (i, &point_color[0]);
                    ^
/home/manuelli/spartan/build/director/src/pcl/surface/src/vtk_smoothing/vtk_utils.cpp: In static member function 'static int pcl::VTKUtils::mesh2vtk(const pcl::PolygonMesh&, vtkSmartPointer<vtkPolyData>&)':
/home/manuelli/spartan/build/director/src/pcl/surface/src/vtk_smoothing/vtk_utils.cpp:240:15: error: 'class vtkUnsignedCharArray' has no member named 'InsertNextTupleValue'
       colors->InsertNextTupleValue (color);
               ^
/home/manuelli/spartan/build/director/src/pcl/surface/src/vtk_smoothing/vtk_utils.cpp:257:16: error: 'class vtkFloatArray' has no member named 'InsertNextTupleValue'
       normals->InsertNextTupleValue (normal);
sammy-tri commented

Yes, VTK in drake update recently (I can't find the PR quickly). I've hit the same problem. VTK was then rolled back to an older version in #8325, but it wasn't the same older version and these deprecated functions are still missing (and director thus still fails to compile). cc @jamiesnape

jamiesnape commented

Yes, GetTupleValue needs to be changed to GetTypedTuple and InsertNextTupleValue needs to be changed to InsertNextTypedTuple.

patmarion commented

Director is currently building with pcl 1.8.0:

https://github.com/RobotLocomotion/director/blob/master/distro/superbuild/cmake/externals.cmake#L472

You could try changing that to 1.8.1. If it builds then I think we're good.

sammy-tri commented

Tried changing director to build with the current drake head. PCL built, but then director blew up looking for LCM, presumably because drake doesn't provide a .pc file for it:

-- Checking for module 'lcm'
--   No package 'lcm' found
CMake Error at /usr/share/cmake-3.5/Modules/FindPackageHandleStandardArgs.cmake:148 (message):
  Could NOT find LCM (missing: LCM_LIBRARY)
Call Stack (most recent call first):
  /usr/share/cmake-3.5/Modules/FindPackageHandleStandardArgs.cmake:388 (_FPHSA_FAILURE_MESSAGE)
  cmake/modules/FindLCM.cmake:29 (find_package_handle_standard_args)
  cmake/modules/FindLCM.cmake:38 (pkg_config_find_module)
  src/app/CMakeLists.txt:183 (find_package)

Will investigate a bit more.

sammy-tri commented

Deleting cmake/modules/FindLCM.cmake got me further along the build, but PointCloudLibraryPlugin has the same problem with SetTupleValue.

jamiesnape commented

I can PR a fix up to PointCloudLibraryPlugin this afternoon.

  • 👍1
jamiesnape commented

Try updating the director build to use patmarion/PointCloudLibraryPlugin@960f5ff.

sammy-tri commented

Thanks Jamie! Almost worked. Opened patmarion/PointCloudLibraryPlugin#5 which gets PointCloudLibraryPlugin compiling with the vtk installed by RobotLocomotion/drake@c55d4fd

sammy-tri commented

I have a director tree compiling in spartan now: https://github.com/sammy-tri/director/tree/drake_fixups_20180319

Highlights are roughly what's been mentioned above... bump PCL version, bump PointCloudPluginLibrary hash, delete FindLCM.cmake. It hasn't been exhaustively tested beyond loading my fork of the iiwa app.

Compilation options: -DWITH_IIWA_DRIVER_TRI=OFF -DWITH_IIWA_DRIVER_RLG=OFF -DWITH_PERCEPTION=ON -DWITH_SNOPT=ON -DWITH_ROS=OFF

I can open a PR to director if we think this is a reasonable solution. I don't know if I'm going to break any other workflows by changing how LCM is found.

jamiesnape commented

PR the two SHA bumps and meanwhile I will look into the LCM issues.

sammy-tri commented

director PR now merged, only the LCM issue should remain

jamiesnape commented

Patch related to LCM merged, so I think we should be good now.