CTK icon indicating copy to clipboard operation
CTK copied to clipboard

Fix compilation for Qt6

Open jporcher opened this issue 3 years ago • 8 comments

Few changes to make code compile with Qt6. Did not test the changes as I'm not familiar with the classes modified. However, at least it compiles if one wants to play with it.

jporcher avatar Feb 03 '22 14:02 jporcher

@lassoan Thank you for your comments, I updated the code and hopefully the pull request (I'm not a Git expert). Please let me know if it looks OK now!

jporcher avatar May 05 '22 07:05 jporcher

@lassoan Let me know if there's any action expected from me to have this pull requested be merged to your master.

jporcher avatar May 05 '22 07:05 jporcher

How did you build CTK with Qt6? I'm getting errors that Qt cannot be found when I'm trying to build with Qt6.

What did you set your CTK_QT_VERSION CMake variableto: 5 or 6? Did you specify Qt5_DIR or Qt6_DIR variable?

There are lots of hardcoded references to Qt5 in the CMake files. They would need to be changed similarly to how it was done for VTK: https://github.com/Kitware/VTK/commit/190059d2b888051eb2c0267d8a2c41cdd9d01ec0

lassoan avatar May 25 '22 03:05 lassoan

@lassoan Sorry, but I'm not using the original CMakeLists.txt, I've written my own.

In mine, I simply changed: QT5_WRAP_CPP to QT_WRAP_CPP QT5_WRAP_UI to QT_WRAP_UI QT5_ADD_RESOURCES to QT_ADD_RESOURCES

Also had to set set(CMAKE_CXX_STANDARD 17)

And, as you mentioned, many variables had to be changed, if you set CTK_QT_VERSION to 6 then Qt5_DIR must be set to Qt${CTK_QT_VERSION}_DIR, find_package(Qt5Core) to find_package(Qt${CTK_QT_VERSION}Core)...etc

jporcher avatar May 30 '22 05:05 jporcher

Please commit all your changes that were necessary to build with Qt6. We would then review those changes and either merge them or fix them up/generalize. Thank you very much!

lassoan avatar May 30 '22 11:05 lassoan

As I mentioned, I did not use your CMakeLIsts.txt to build the library, so all the "changes" I did are commited. I have no more contribution available...

I gave a quick look at your code, there are many places where you do test CTK_QT_VERSION and thay all have to be reworked.

First main change is to lookup for Qt6 instead of Qt5, so in ctkMacroSetupQt.cmake:

if(CTK_QT_VERSION VERSION_GREATER "4")
...
else()
...
endif()

has to be changed to something like:

if(CTK_QT_VERSION VERSION_GREATER "5")
    cmake_minimum_required(VERSION 3.16)
    set(CMAKE_CXX_STANDARD 17)
    set(CMAKE_CXX_STANDARD_REQUIRED ON)
    find_package(Qt6)
    set(CTK_QT6_COMPONENTS Core Xml Concurrent Sql Test Multimedia)
    if(CTK_ENABLE_Widgets OR CTK_LIB_Widgets OR CTK_LIB_CommandLineModules/Frontend/QtGui OR CTK_BUILD_ALL OR CTK_BUILD_ALL_LIBRARIES)
      list(APPEND CTK_QT6_COMPONENTS Widgets OpenGL UiTools)
    endif()
    if(CTK_LIB_CommandLineModules/Frontend/QtWebKit OR CTK_BUILD_ALL OR CTK_BUILD_ALL_LIBRARIES)
      if(TARGET Qt6::WebKitWidgets)
        list(APPEND CTK_QT6_COMPONENTS WebKitWidgets)
      else()
        list(APPEND CTK_QT6_COMPONENTS WebEngineWidgets)
      endif()
    endif()
    if(CTK_LIB_XNAT/Core OR CTK_BUILD_ALL OR CTK_BUILD_ALL_LIBRARIES)
      list(APPEND CTK_QT6_COMPONENTS Script)
    endif()
    find_package(Qt6 COMPONENTS ${CTK_QT6_COMPONENTS} REQUIRED)

    mark_as_superbuild(Qt6_DIR) # Qt 6

    # XXX Backward compatible way
    if(DEFINED CMAKE_PREFIX_PATH)
      mark_as_superbuild(CMAKE_PREFIX_PATH) # Qt 6
    endif()
elseif(CTK_QT_VERSION VERSION_GREATER "4")
...
else()
...
endif()

Then all other places has to be reworked as well, like, for instance, in Libs\Visualization\VTK\Core\Testing\Cpp\CMakeLists.txt:

if(CTK_QT_VERSION VERSION_GREATER "4")
  QT5_WRAP_CPP(KIT_HELPER_SRCS ctkVTKObjectTestHelper.h)
  include_directories(${Qt5Widgets_INCLUDE_DIRS})
else()
  QT4_WRAP_CPP(KIT_HELPER_SRCS ctkVTKObjectTestHelper.h)
endif()

should be changed to:

if(CTK_QT_VERSION VERSION_GREATER "5")
  QT6_WRAP_CPP(KIT_HELPER_SRCS ctkVTKObjectTestHelper.h)
  include_directories(${Qt6Widgets_INCLUDE_DIRS})
elseif(CTK_QT_VERSION VERSION_GREATER "4")
  QT5_WRAP_CPP(KIT_HELPER_SRCS ctkVTKObjectTestHelper.h)
  include_directories(${Qt5Widgets_INCLUDE_DIRS})
else()
  QT4_WRAP_CPP(KIT_HELPER_SRCS ctkVTKObjectTestHelper.h)
endif()

etc....

You may also decide to use versionless commands, and then, you simply need to remove the Qt version suffix and use ${CTK_QT_VERSION} when needed. This would be more elegant and easy to maintain:

if(CTK_QT_VERSION VERSION_GREATER "4")
  QT_WRAP_CPP(KIT_HELPER_SRCS ctkVTKObjectTestHelper.h)
  include_directories(${Qt${CTK_QT_VERSION}Widgets_INCLUDE_DIRS})
else()
  QT4_WRAP_CPP(KIT_HELPER_SRCS ctkVTKObjectTestHelper.h)
endif()

jporcher avatar May 31 '22 15:05 jporcher

@jporcher How did you build CTK without using the CMakeLists.txt provided at https://github.com/jporcher/CTK/blob/qt6_support/CMakeLists.txt? Can you commit the CMakeLists.txt that you used for your own project?

jamesobutler avatar May 31 '22 15:05 jamesobutler

@jamesobutler That would be difficult as this CMakeLists.txt has dependencies on other files of my project, but I can paste you some part of it:

set (LIBNAME qtcommontk)

# This project compiles the external library qwt
PROJECT(${LIBNAME})

SET( CMAKE_DEBUG_POSTFIX ${SDE_DEBUG_FLAG} )

file( GLOB C_files
	Libs/Widgets/ctkBasePopupWidget.cpp
	Libs/Widgets/ctkDoubleRangeSlider.cpp
	Libs/Widgets/ctkDoubleRangeSliderEventPlayer.cpp
	Libs/Widgets/ctkDoubleRangeSliderEventTranslator.cpp
	Libs/Widgets/ctkDoubleSlider.cpp
	Libs/Widgets/ctkDoubleSpinBox.cpp
	Libs/Widgets/ctkPopupWidget.cpp
	Libs/Widgets/ctkRangeSlider.cpp
	Libs/Widgets/ctkRangeWidget.cpp
	Libs/Widgets/ctkSliderWidget.cpp
	Libs/Core/ctkUtils.cpp
	Libs/Core/ctkValueProxy.cpp
	Libs/Widgets/ctkWidgetsUtils.cpp
	../QtTesting-master/pqWidgetEventPlayer.cxx
	../QtTesting-master/pqWidgetEventTranslator.cxx )

file( GLOB H_files
	Libs/Widgets/ctkBasePopupWidget.h
	Libs/Widgets/ctkDoubleRangeSlider.h
	Libs/Widgets/ctkDoubleRangeSliderEventPlayer.h
	Libs/Widgets/ctkDoubleRangeSliderEventTranslator.h
	Libs/Widgets/ctkDoubleSlider.h
	Libs/Widgets/ctkDoubleSpinBox.h
	Libs/Widgets/ctkPopupWidget.h
	Libs/Widgets/ctkRangeSlider.h
	Libs/Widgets/ctkRangeWidget.h
	Libs/Widgets/ctkSliderWidget.h
	Libs/Core/ctkUtils.h
	Libs/Core/ctkValueProxy.h
	Libs/Widgets/ctkWidgetsUtils.h
	Libs/Widgets/ctkBasePopupWidget_p.h
	Libs/Widgets/ctkDoubleSpinBox_p.h
	Libs/Widgets/ctkPopupWidget_p.h
	../QtTesting-master/pqWidgetEventPlayer.h
	../QtTesting-master/pqWidgetEventTranslator.h )

file( GLOB UI_files Libs/Widgets/Resources/UI/ctkRangeWidget.ui Libs/Widgets/Resources/UI/ctkSliderWidget.ui )
file( GLOB QRC_files Libs/Widgets/Resources/ctkWidgets.qrc )

# We need to include all directories where the headers are
include_directories( Libs/Widgets )
include_directories( Libs/Core )
include_directories( ../QtTesting-master )
include_directories( sde )

SET(SDE3P_QTSDEVER 6)
SET(SDE3P_QTDIR "${CMAKE_CURRENT_LIST_DIR}/../../qt/6.2.2")
MESSAGE( "Set QTDIR=${SDE3P_QTDIR}" )

find_package(Qt${SDE3P_QTSDEVER}Core)
find_package(Qt${SDE3P_QTSDEVER}Gui)
find_package(Qt${SDE3P_QTSDEVER}Widgets)

set( pre_qt_wrap_sources ${C_files} )

QT_WRAP_CPP( MOC_FILES ${H_files})
list( APPEND C_files ${MOC_FILES} )

QT_WRAP_UI( UIC_SRCS ${UI_files} )
list( APPEND C_files ${UIC_SRCS} )

QT_ADD_RESOURCES( QRC_SRCS ${QRC_files} )
list( APPEND C_files ${QRC_SRCS} )

# C++17 is required by Qt6
add_definitions( "/Zc:__cplusplus" )
set(CMAKE_CXX_STANDARD 17)

# Library will depend on all source files
ADD_LIBRARY( ${LIBNAME} SHARED ${C_files} ${H_files} ${SRCS_CXX})

include_directories( ${QT_INCLUDE_DIR} )
include_directories( ${QT_INCLUDE_DIR}/QtCore )
include_directories( ${QT_INCLUDE_DIR}/QtGui )
include_directories( ${QT_INCLUDE_DIR}/QtWidgets )

jporcher avatar May 31 '22 15:05 jporcher