Fix compilation warnings when using VTK [version >= 8.2]

[nickmcummins]

Fixes #3370 by

• Replacing deprecated QVTKWidget with QVTKOpenGLNativeWidget.
• Fixing warning that VTK CMake components no longer start with 'vtk'.
• Adding FiltersGeometry to fix unresolved symbol when linking to vtkDataSetSurfaceFilter::New() which is in libvtkFiltersGeometry-8.90.so.
• Fixing undefined reference to std::ofstream missing namespace.
• Fixing compilation issue: current versions of VTK have vtkShaderProgram but vtkShaderProgram2 seems to be only in older versions.
• Numerous other minor related changes to build successfully.

[nickmcummins]

#3370

[nickmcummins]

At least some of the builds are failing because they compile against vtk7. How do I fix that?

* The file was deleted, renamed, or moved to another location.
* An install or uninstall procedure did not complete successfully.
* The installation package was faulty and contained
   "/usr/lib/cmake/vtk-7.1/VTKTargets.cmake"
but not all the files it references.

[taketwo]

Hi Nick, thanks for taking the time to contribute.

We have builds that use VTK 7.1 (Ubuntu-19-10) and even VTK 6.2 (Ubuntu-16-04). For the time being, we would like to keep PCL compatible with these versions since Ubuntu 16.04 is our baseline platform and the highest packaged version of VTK there is 6.2. But needless to say, we would also like to support the latest VTK releases.

We'll need to work a bit on your PR to make it compatible with all VTK versions (add some CMake magic and a bit of indirection here and there). As the first step, I propose to extract everything not directly related to VTK 8.9 / QVTK widget into a separate PR. This will include changes regarding:

• std::ofstream
• missing includes
• InsertCellPoints
• something else?

That PR should be compatible with the old VTK versions and thus pass our CI. Once we merge it, we can concentrate on the "real things".

[nickmcummins]

Hi Nick, thanks for taking the time to contribute.

We have builds that use VTK 7.1 (Ubuntu-19-10) and even VTK 6.2 (Ubuntu-16-04). For the time being, we would like to keep PCL compatible with these versions since Ubuntu 16.04 is our baseline platform and the highest packaged version of VTK there is 6.2. But needless to say, we would also like to support the latest VTK releases.

We'll need to work a bit on your PR to make it compatible with all VTK versions (add some CMake magic and a bit of indirection here and there). As the first step, I propose to extract everything not directly related to VTK 8.9 / QVTK widget into a separate PR. This will include changes regarding:

* `std::ofstream`

* missing includes

* `InsertCellPoints`

* something else?

That PR should be compatible with the old VTK versions and thus pass our CI. Once we merge it, we can concentrate on the "real things".

I will provide an update once I install VTK 7.1 on my machine and am able to compile PCL against it to confirm which changes are unrelated to the VTK version upgrade.

[taketwo]

You can use our Docker image to test compilation. Get the 19:10 tag, it contains VTK 7.1 and everything else needed to configure/build PCL.

[larshg]

@nickmcummins are you still working on this - else would you mind I pick it up and continue the work?

[larshg]

Hi Nick, thanks for taking the time to contribute.

We have builds that use VTK 7.1 (Ubuntu-19-10) and even VTK 6.2 (Ubuntu-16-04). For the time being, we would like to keep PCL compatible with these versions since Ubuntu 16.04 is our baseline platform and the highest packaged version of VTK there is 6.2. But needless to say, we would also like to support the latest VTK releases.

We'll need to work a bit on your PR to make it compatible with all VTK versions (add some CMake magic and a bit of indirection here and there). As the first step, I propose to extract everything not directly related to VTK 8.9 / QVTK widget into a separate PR. This will include changes regarding:

• std::ofstream
• missing includes
• InsertCellPoints
• something else?

That PR should be compatible with the old VTK versions and thus pass our CI. Once we merge it, we can concentrate on the "real things".

Hmm, what I can gather, its very little what we can extract from the PR.

AllocateEstimate is not available in 8.2.
and it doesn't allow GetNextCell with a const parameter.

why can use AllocateEstimate in hpp but not cpp?

I started changing the hpp file and pcl_visualizer compiles when I added the AllocateEstimate, but when I do the same thing for the cpp file, it complains about it not exisiting. Why?

The entire change in vtkVertexBufferObjectMapper seems unnecessary, because its only used if openglbackend is less than 2 - its set in pcl_config - but maybe its not defined correct anymore:

#if VTK_RENDERING_BACKEND_OPENGL_VERSION < 2
#include <pcl/visualization/vtk/vtkVertexBufferObjectMapper.h>
#endif

even in CMake its not added if the version is less than 2 (same applies for the header files):

if(VTK_RENDERING_BACKEND_OPENGL_VERSION VERSION_LESS 2)
  list(APPEND srcs
    "src/vtk/vtkVertexBufferObject.cxx"
    "src/vtk/vtkVertexBufferObjectMapper.cxx"
  )
endif()

and the std:: prefix to ofstream, seems a bit overkill to make a single PR for. However, it could probably be excluded from a VTK 8.9 PR.

As for the magic stuff:

I guess in code we can use something like the following:

    // Create polys from polyMesh.polygons
    vtkSmartPointer<vtkCellArray> cell_array = vtkSmartPointer<vtkCellArray>::New ();
#if VTK_MAJOR_VERSION <= 8 && VTK_MINOR_VERSION < 9
    vtkIdType* cell = cell_array->WritePointer(vertices.size(), vertices.size() * (max_size_of_polygon + 1));
#endif
    int idx = 0;
    if (!lookup.empty ())
    {
      for (std::size_t i = 0; i < vertices.size (); ++i, ++idx)
      {
        std::size_t n_points = vertices[i].vertices.size ();
#if VTK_MAJOR_VERSION <= 8 && VTK_MINOR_VERSION < 9
        *cell++ = n_points;
#else
        cell_array->InsertNextCell (n_points);
#endif
        for (std::size_t j = 0; j < n_points; j++, ++idx)
        {
#if VTK_MAJOR_VERSION <= 8 && VTK_MINOR_VERSION < 9
          *cell++ = lookup[vertices[i].vertices[j]];
#else
          cell_array->InsertCellPoint (lookup[vertices[i].vertices[j]]);
#endif

Should we do the same for the method name changes:
ie. GetInteractor becomes Interactor.
Or can we make aliases which then evaluates to either of the above?

The same with class name change, ie. QVTKWidget to QVTKOpenGLNativeWidget.

In the CMakeList we can use the the VTK_VERSION defines.

I guess for the .ui files, we would have to make two files and include the appropriate from CMake?

Any else suggestions?

[larshg]

Closing as it was superseded by #4262.