vtkAddon with VTK 8.2.0 build fails with C++17

Development
1

Hi,

VTK 8.2.0 in VTK/Common/Core/vtkMath.h needs to have the following include:

#include <algorithm> // for std::clamp

aimed to support C++17 (and higher).

With MSVC I used to successfully compile Slicer with C++17 but now I’m working on Ubuntu and the build fails with error (GCC 9.3):

In file included from /vtkAddon/vtkOrientedBSplineTransform.cxx:13:
/VTK/Common/Core/vtkMath.h: In static member function ‘static T vtkMath::ClampValue(const T&, const T&, const T&)’:
/VTK/Common/Core/vtkMath.h:1516:15: error: ‘clamp’ is not a member of ‘std’
 1516 |   return std::clamp(value, min, max);
      |               ^~~~~
make[2]: *** [CMakeFiles/vtkAddon.dir/build.make:121: CMakeFiles/vtkAddon.dir/vtkOrientedBSplineTransform.cxx.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:95: CMakeFiles/vtkAddon.dir/all] Error 2
make: *** [Makefile:149: all] Error 2

By the way VTK 9 has fixed this

2

Thanks for reporting this. In Slicer, we don’t support building VTK8.2 with C++17 (C++17 is disabled in Slicer superbuild), so they do not have to be compatible, but the change you suggest is harmless, so if you submit a pull request to vtkAddon then we’ll merge it.

1 Like
3

In best scenario I should push a PR to VTK 8.2.0 as #include <algorithm> should normally reside there.
Less accurate decision is probably to put #include <algorithm> before including vtkMath.h in vtkAddon.

I don’t know (I’m not much experienced git user) is it possible to push PR to VTK 8.2.0 (or is it worth it?)?
If not I will try to push PR to vtkAddon

4

I decided to make PR to vtkAddon

5

Reading VTK 8.2 does not compile with c++17 (#17615) · Issues · VTK / VTK · GitLab, it appears that VTK 8.2 never was explicitly supporting C++17.

Is there a reason you are not building Slicer with VTK9 where it does not have this issue with C++17? The Slicer preview builds are already using VTK9 well. If you are building a Slicer custom application or building Slicer from source you would just need to set the VTK version to use as “9” instead of the default “8”.

@lassoan Should the default Slicer build option for VTK be switched to “9” now? Most VTK9 related issues have been resolved and since it is used in the Slicer preview packages now it is likely worthy of being the default CMake option?

1 Like
6

Hi,

Thank you for information.

Actually I would prefer VTK 9, but when I’m trying to force VTK 9 in SlicerCAT with the line-code in my CMakeLists.txt:

set(Slicer_VTK_VERSION_MAJOR 9) # or set(Slicer_VTK_VERSION_MAJOR:STRING "9")

I can see that Slicer still builds VTK 8.2.0. So for now I just tried to fix VTK 8.2.0 bug and in the near future I will try to investigate why I can’t build it against VTK 9.

I took a first look how External_VTK.cmake works:

  if("${Slicer_VTK_VERSION_MAJOR}" STREQUAL "8")

    set(_git_tag "97904fdcc7e73446b3131f32eac9fc9781b23c2d") # slicer-v8.2.0-2018-10-02-74d9488523

    set(vtk_egg_info_version "8.2.0")

  elseif("${Slicer_VTK_VERSION_MAJOR}" STREQUAL "9")

    set(_git_tag "f3c1a72fbf0f7287575ae876efced9c85776d9b4") # slicer-v9.0.20201111-733234c785

    set(vtk_egg_info_version "9.0.20201111")

  else()

    message(FATAL_ERROR "error: Unsupported Slicer_VTK_VERSION_MAJOR: ${Slicer_VTK_VERSION_MAJOR}")

  endif()

and it is quite straghtforward. But as I said SlicerCAT have built VTK 8.2.0 for me.

7

You will need to make sure your SlicerCAT is using the latest Slicer repo git hash and then you can select Slicer_VTK_VERSION_MAJOR as “9” and it should build well. If you are using an earlier Slicer git hash version that could’ve been when VTK9 support wasn’t working appropriately.

You should keep the VTK git hash version that Slicer specified in External_VTK which is a specific Slicer fork of VTK.

1 Like
8

Good point. I’ve submitted a pull request to change the default VTK version to 9: