Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/qt6 compatibility #1315

Merged
merged 22 commits into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
214142f
eCAL Sys already compiles and runs with Qt6
FlorianReimold Jan 15, 2024
b6792bc
Intermediate state while working on eCAL Mon GUI
FlorianReimold Jan 16, 2024
d84a68e
Merge branch 'master' of https://github.com/eclipse-ecal/ecal into fe…
FlorianReimold Jan 16, 2024
b156bc1
eCAL Mon builds and runs
FlorianReimold Jan 16, 2024
605d76d
Fixed build issues with Qt 5.15
FlorianReimold Jan 16, 2024
f30a509
Fixed compatibility with Qt 5.12
FlorianReimold Jan 16, 2024
e28e752
Fixed Release build issue
FlorianReimold Jan 16, 2024
9c59f9b
eCAL Mon and all Plugins now compile and run with Qt6
FlorianReimold Jan 17, 2024
da7cad7
Made eCAL Launcher compatible with Qt6
FlorianReimold Jan 17, 2024
f0f9965
eCAL Play and Rec now compile and run with Qt6
FlorianReimold Jan 17, 2024
7e31dd2
Samples now compile with Qt6
FlorianReimold Jan 17, 2024
f7c5dfd
Introduces version-less HAS_QT cmake option
FlorianReimold Jan 17, 2024
7652940
Modified gh actions. the windows gh action should now build with qt6
FlorianReimold Jan 17, 2024
1d0e1e5
Brought back the qt_msvc_path.cmake script
FlorianReimold Jan 18, 2024
348def2
Fixed Qt6 deprecation warnings
FlorianReimold Jan 18, 2024
c08bb31
Fixed Mon exclude regular expression
FlorianReimold Jan 19, 2024
3367208
Merge branch 'master' of https://github.com/eclipse-ecal/ecal into fe…
FlorianReimold Jan 19, 2024
a6888b2
Solved a few TODOs
FlorianReimold Jan 19, 2024
51e6047
Improved documentation
FlorianReimold Jan 19, 2024
2aa5100
Fixed clang-tidy warnings
FlorianReimold Jan 19, 2024
1c37168
Added Compatibility with Qt 5.9 - 5.11
FlorianReimold Feb 7, 2024
4e80bc4
Fixed QT_VERSION Check defines
FlorianReimold Feb 7, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build-ubuntu-20.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ jobs:
cmake $GITHUB_WORKSPACE -G "Ninja" \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
-DHAS_HDF5=ON \
-DHAS_QT5=ON \
-DHAS_QT=ON \
-DHAS_CURL=ON \
-DHAS_CAPNPROTO=ON \
-DHAS_FTXUI=ON \
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build-ubuntu-22.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ jobs:
cmake $GITHUB_WORKSPACE -G "Ninja" \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
-DHAS_HDF5=ON \
-DHAS_QT5=ON \
-DHAS_QT=ON \
-DHAS_CURL=ON \
-DHAS_CAPNPROTO=ON \
-DHAS_FTXUI=ON \
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/build-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ jobs:
uses: jurplel/install-qt-action@v3
with:
setup-python: 'false'
version: '5.15.2'
version: '6.6.1'
target: 'desktop'
arch: 'win64_msvc2015_64'
arch: 'win64_msvc2019_64'

# Downgrading nuget is required as of 2021-04-23, as nuget 5.9.1.111 fails installing protobuf
# https://github.com/actions/virtual-environments/issues/3240
Expand Down Expand Up @@ -91,7 +91,7 @@ jobs:

cmake %GITHUB_WORKSPACE% -G "Visual Studio 16 2019" -A x64 -T v142 ^
-DHAS_HDF5=ON ^
-DHAS_QT5=ON ^
-DHAS_QT=ON ^
-DHAS_CURL=OFF ^
-DHAS_CAPNPROTO=OFF ^
-DHAS_FTXUI=ON ^
Expand Down Expand Up @@ -131,7 +131,7 @@ jobs:
cd "${{ runner.workspace }}/_build/complete"
cmake %GITHUB_WORKSPACE% -G "Visual Studio 16 2019" -A x64 -T v142 ^
-DHAS_HDF5=ON ^
-DHAS_QT5=ON ^
-DHAS_QT=ON ^
-DHAS_CURL=ON ^
-DHAS_CAPNPROTO=OFF ^
-DHAS_FTXUI=ON ^
Expand Down
52 changes: 30 additions & 22 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,14 @@ endif()
# use it that way cmake .. -DBUILD_APPS=ON -DBUILD_SAMPLES=ON
# --------------------------------------------------------
option(HAS_HDF5 "Platform supports HDF5 library" ON)
option(HAS_QT5 "Platform supports Qt 5 library" ON)
option(HAS_QT "Platform supports Qt 5 / 6 library." ON)

# If the user set the legacy HAS_QT5 option, but didn't set the new HAS_QT option, set it to the value of HAS_QT5
if(DEFINED HAS_QT5 AND NOT DEFINED HAS_QT)
set(HAS_QT ${HAS_QT5})
message(WARNING "The option HAS_QT5 is deprecated and may be removed at any time. Please use HAS_QT instead.")
endif()

option(HAS_CURL "Build with CURL (i.e. upload support in the recorder app)" ON)
option(HAS_CAPNPROTO "Platform supports Cap'n Proto library" OFF)
option(HAS_FLATBUFFERS "Platform supports flatbuffers library" OFF)
Expand Down Expand Up @@ -112,12 +119,12 @@ if(WIN32)
option(ECAL_THIRDPARTY_BUILD_YAML-CPP "Build yaml-cpp with eCAL" ON)
option(ECAL_THIRDPARTY_BUILD_CURL "Build CURL with eCAL" ON)
option(ECAL_THIRDPARTY_BUILD_HDF5 "Build HDF5 with eCAL" ON)
cmake_dependent_option(ECAL_THIRDPARTY_BUILD_QWT "Build qwt::qwt with eCAL" ON "HAS_QT5" OFF)
cmake_dependent_option(ECAL_THIRDPARTY_BUILD_QWT "Build qwt::qwt with eCAL" ON "HAS_QT" OFF)
else()
option(ECAL_THIRDPARTY_BUILD_PROTOBUF "Build protobuf with eCAL" OFF)
option(ECAL_THIRDPARTY_BUILD_CURL "Build CURL with eCAL" OFF)
option(ECAL_THIRDPARTY_BUILD_HDF5 "Build HDF5 with eCAL" OFF)
cmake_dependent_option(ECAL_THIRDPARTY_BUILD_QWT "Build qwt::qwt with eCAL" OFF "HAS_QT5" OFF)
cmake_dependent_option(ECAL_THIRDPARTY_BUILD_QWT "Build qwt::qwt with eCAL" OFF "HAS_QT" OFF)
option(ECAL_THIRDPARTY_BUILD_YAML-CPP "Build yaml-cpp with eCAL" OFF)
endif()

Expand Down Expand Up @@ -217,10 +224,10 @@ find_package(CMakeFunctions REQUIRED)
# detect qt library
# --------------------------------------------------------
if(MSVC)
if (HAS_QT5)
find_package(Qt5 COMPONENTS Core QUIET)
if (NOT "${Qt5_FOUND}")
autodetect_qt5_msvc_dir()
if (HAS_QT)
find_package(QT NAMES Qt6 Qt5 COMPONENTS Core QUIET)
if (NOT "${QT_FOUND}")
autodetect_qt_msvc_dir()
endif()
endif()
endif()
Expand Down Expand Up @@ -329,20 +336,20 @@ add_subdirectory(lib/ThreadingUtils)
add_subdirectory(lib/CustomTclap)
add_subdirectory(lib/ecal_utils)

if(HAS_QT5)
if(HAS_QT)
add_subdirectory(lib/CustomQt)
endif()
add_subdirectory(lib/EcalParser)
if(HAS_QT5)
if(HAS_QT)
add_subdirectory(lib/QEcalParser)
endif(HAS_QT5)
endif()

# --------------------------------------------------------
# ecal mon plugin sdk
# --------------------------------------------------------
if(HAS_QT5)
if(HAS_QT)
add_subdirectory(app/mon/mon_plugin_lib)
endif(HAS_QT5)
endif()

# --------------------------------------------------------
# ecal rec addon sdk
Expand Down Expand Up @@ -422,7 +429,7 @@ endif()
# --------------------------------------------------------
# qt applications
# --------------------------------------------------------
if(BUILD_APPS AND HAS_QT5)
if(BUILD_APPS AND (HAS_QT))
add_subdirectory(app/sys/sys_gui)
add_subdirectory(app/mon/mon_gui)
if(WIN32)
Expand All @@ -432,13 +439,14 @@ if(BUILD_APPS AND HAS_QT5)
endif()
add_subdirectory(app/mon/mon_plugins)
add_subdirectory(app/util/launcher)
# --------------------------------------------------------
# qt applications using hdf5
# --------------------------------------------------------
if(BUILD_APPS AND HAS_QT5 AND HAS_HDF5)
add_subdirectory(app/play/play_gui)
add_subdirectory(app/rec/rec_gui)
endif()

# --------------------------------------------------------
# qt applications using hdf5
# --------------------------------------------------------
if(HAS_HDF5)
add_subdirectory(app/play/play_gui)
add_subdirectory(app/rec/rec_gui)
endif()
endif()

# --------------------------------------------------------
Expand Down Expand Up @@ -481,7 +489,7 @@ if(BUILD_ECAL_TESTS)
# ------------------------------------------------------
# test apps
# ------------------------------------------------------
if (HAS_HDF5 AND HAS_QT5)
if (HAS_HDF5 AND HAS_QT)
add_subdirectory(app/rec/rec_tests/rec_rpc_tests)
endif()
endif()
Expand Down Expand Up @@ -552,7 +560,7 @@ message(STATUS "Build Options:")
message(STATUS "--------------------------------------------------------------------------------")
message(STATUS "CMAKE_EXPORT_COMPILE_COMMANDS : ${CMAKE_EXPORT_COMPILE_COMMANDS}")
message(STATUS "HAS_HDF5 : ${HAS_HDF5}")
message(STATUS "HAS_QT5 : ${HAS_QT5}")
message(STATUS "HAS_QT : ${HAS_QT}")
message(STATUS "HAS_CURL : ${HAS_CURL}")
message(STATUS "HAS_CAPNPROTO : ${HAS_CAPNPROTO}")
message(STATUS "HAS_FLATBUFFERS : ${HAS_FLATBUFFERS}")
Expand Down
126 changes: 77 additions & 49 deletions app/mon/mon_gui/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,19 @@
#
# ========================= eCAL LICENSE =================================

cmake_minimum_required(VERSION 3.14)

# Allow the install command to use generator expressions
if(POLICY CMP0087)
cmake_policy(SET CMP0087 NEW)
endif()

project(mon_gui)

find_package(Qt5 COMPONENTS
Core
Widgets
REQUIRED)
# Legacy Qt5 (pre 5.15) support as suggested by teh Qt Documentation:
# https://doc.qt.io/qt-6/cmake-qt5-and-qt6-compatibility.html#supporting-older-qt-5-versions
find_package(QT NAMES Qt6 Qt5 REQUIRED COMPONENTS Core Widgets)
find_package(Qt${QT_VERSION_MAJOR} REQUIRED COMPONENTS Core Widgets)

find_package(Protobuf REQUIRED)

Expand Down Expand Up @@ -169,10 +176,14 @@ if(ECAL_NPCAP_SUPPORT)
)
endif(ECAL_NPCAP_SUPPORT)


# compile qt resource files and ui files
qt5_add_resources(autogen_resources ${qt_resource_files})
qt5_wrap_ui (autogen_ui ${ui_files})
if (${QT_VERSION_MAJOR} GREATER_EQUAL 6)
qt_add_resources(autogen_resources ${qt_resource_files})
qt_wrap_ui (autogen_ui ${ui_files})
else()
qt5_add_resources(autogen_resources ${qt_resource_files})
qt5_wrap_ui (autogen_ui ${ui_files})
endif()

# Add all files. The resource files and ui files are not necessary, but we want them to show up in the IDE
ecal_add_app_qt(${PROJECT_NAME}
Expand All @@ -190,7 +201,8 @@ target_link_libraries (${PROJECT_NAME}
eCAL::core
eCAL::core_pb
eCAL::mon_plugin_lib
Qt5::Widgets
Qt${QT_VERSION_MAJOR}::Core
Qt${QT_VERSION_MAJOR}::Widgets
CustomQt
)

Expand All @@ -217,9 +229,64 @@ target_include_directories(${PROJECT_NAME} PRIVATE src)
target_include_directories(${PROJECT_NAME} PRIVATE $<TARGET_PROPERTY:eCAL::core,INCLUDE_DIRECTORIES>)


if(WIN32)
if ((WIN32 OR APPLE) AND (${QT_VERSION_MAJOR} GREATER_EQUAL 6))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have this (or similar) code in many places. Could you please make cmake macros / functions for it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are just 3 commands (qt_generate_deploy_app_script, add_custom_command, install). The rest are comments to explain what is going on. Do you think that it is worth to make a script for that, discover the script file, include the script and then call it?
I feel like that would cause more code and be more difficult to understand, because you would have to go to that script to see what is going on. And one would have to implement some kind of argument parsing, if you intend to keep it flexible. It would also cause additional coupling of all eCAL Qt Application to some custom script that is stored somewhere. I was excited to remove custom deploy scripts for Qt6, so I am not sure if encapsulating 3 commands justify creating one, again.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But you have the same code in at least 4 places (mon_gui, play_gui, rec_gui, sys_gui, ...)
It's like arguing that you're not creating another header file and not using functions, because it's too complicated.

It's 60 lines of code, being (as far as I can see), exactly identical in at least mon_gui, play_gui, rec_gui.
If you want to keep it with the apps, put it in the app subdirectory, or define the macro in app/CMakeLists.txt.
But duplicated code is duplicated code and should be put in a function (or cmake macro, because that is in the end text replacement, and does not introduce new scopes which you may not want).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also make it part of ecal_add_app_qt, if it's something to be done for all ecal qt apps.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm ok, so your intention is to put the legacy Qt5 install(CODE ... in that script as well? I agree that that script could go in its own file, but the scope of this PR was to just add Qt6. Also the Qt5 script wasn't written by me and I honestly was afraid to touch it, so I just added the new commands for Qt6.

It's like arguing that you're not creating another header file and not using functions, because it's too complicated.

I didn't write it would be too complicated and (for Qt6) the CMakeLists.txt does use functions. But yea, it kind of is similar to copying code from one to the next header file versus creating a separate project for it that you then have to reference to. It's always a tradeoff between coupling and code duplication.
I felt like Qt6 already encapsulated everything very well and those official qt functions are probably better understandable than a custom one. Thus I didn't want to hide those 3 calls with the downside of more coupling.

You could also make it part of ecal_add_app_qt, if it's something to be done for all ecal qt apps.

Sure, could be a solution for the apps, as those are coupled to those functions anyways. It would get rid of the flexbility, but maybe we are ok with a preconfigured variant of the scripts.
We would also maybe need a preconfigured variant for samples, monitor plugins and libraries.


# Generate a script that will deploy all necessary Qt DLLs to the binary folder
# https://doc.qt.io/qt-6/qt-deploy-runtime-dependencies.html
# Available for Qt 6.3 and up (=> Not for Qt5!)
# Executing it requires CMake 3.14 and up, due to policy https://cmake.org/cmake/help/latest/policy/CMP0087.html
qt_generate_deploy_app_script(
TARGET ${PROJECT_NAME}
OUTPUT_SCRIPT qt_deploy_script
NO_COMPILER_RUNTIME
NO_UNSUPPORTED_PLATFORM_ERROR
)

# Add a postbuild script that will also execute the created script via cmake -P
# This is necessary to make the application startable / debuggable from the build directory.
add_custom_command(TARGET ${PROJECT_NAME} POST_BUILD
COMMAND ${CMAKE_COMMAND} -DQT_DEPLOY_PREFIX=$<TARGET_FILE_DIR:${PROJECT_NAME}> -DQT_DEPLOY_BIN_DIR=. -P ${qt_deploy_script}
)

# Use the script for deploying the qt dlls in the install dir
install(SCRIPT ${qt_deploy_script})

elseif(WIN32)

# For Qt5 we use our legacy script.
# Deploy Qt DLLs in the binary folder. This is necessary for starting the application from whithin the IDE without having to copy QtCore.dll, QtWidgets.dll etc. by hand each time
qt_add_windeployqt_postbuild(--no-system-d3d-compiler --no-compiler-runtime --no-opengl-sw --pdb "$<TARGET_FILE:${PROJECT_NAME}>")

get_target_property(_qmake_executable Qt5::qmake IMPORTED_LOCATION)
get_filename_component(_qt_bin_dir "${_qmake_executable}" DIRECTORY)
find_program(WINDEPLOYQT_EXECUTABLE windeployqt HINTS "${_qt_bin_dir}")
install(CODE
"
set(_file ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/Release/ecal_mon_gui.exe)
execute_process(
COMMAND \"${CMAKE_COMMAND}\" -E
env PATH=\"${_qt_bin_dir}\" \"${WINDEPLOYQT_EXECUTABLE}\"
--dry-run
--no-compiler-runtime
--no-angle
--no-opengl-sw
--list mapping
\${_file}
OUTPUT_VARIABLE _output
OUTPUT_STRIP_TRAILING_WHITESPACE
)
separate_arguments(_files WINDOWS_COMMAND \${_output})
while(_files)
list(GET _files 0 _src)
list(GET _files 1 _dest)
execute_process(
COMMAND \"${CMAKE_COMMAND}\" -E
copy \${_src} \"\${CMAKE_INSTALL_PREFIX}/bin/\${_dest}\"
)
list(REMOVE_AT _files 0 1)
endwhile()
"
)
endif()

# Create a source tree that mirrors the filesystem
Expand All @@ -244,43 +311,4 @@ source_group( autogen FILES

set_property(TARGET ${PROJECT_NAME} PROPERTY FOLDER app/mon)

ecal_install_app(${PROJECT_NAME} START_MENU_NAME "eCAL Monitor")
get_target_property(_qmake_executable Qt5::qmake IMPORTED_LOCATION)
get_filename_component(_qt_bin_dir "${_qmake_executable}" DIRECTORY)
find_program(WINDEPLOYQT_EXECUTABLE windeployqt HINTS "${_qt_bin_dir}")

# Running this with MSVC 2015 requires CMake 3.6+
if((MSVC_VERSION VERSION_EQUAL 1900 OR MSVC_VERSION VERSION_GREATER 1900)
AND CMAKE_VERSION VERSION_LESS "3.6")
message(WARNING "Deploying with MSVC 2015+ requires CMake 3.6+")
endif()

if(WIN32)
install(CODE
"
set(_file ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/Release/ecal_mon_gui.exe)
execute_process(
COMMAND \"${CMAKE_COMMAND}\" -E
env PATH=\"${_qt_bin_dir}\" \"${WINDEPLOYQT_EXECUTABLE}\"
--dry-run
--no-compiler-runtime
--no-angle
--no-opengl-sw
--list mapping
\${_file}
OUTPUT_VARIABLE _output
OUTPUT_STRIP_TRAILING_WHITESPACE
)
separate_arguments(_files WINDOWS_COMMAND \${_output})
while(_files)
list(GET _files 0 _src)
list(GET _files 1 _dest)
execute_process(
COMMAND \"${CMAKE_COMMAND}\" -E
copy \${_src} \"\${CMAKE_INSTALL_PREFIX}/bin/\${_dest}\"
)
list(REMOVE_AT _files 0 1)
endwhile()
"
)
endif()
ecal_install_app(${PROJECT_NAME} START_MENU_NAME "eCAL Monitor")
16 changes: 15 additions & 1 deletion app/mon/mon_gui/src/ecalmon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,17 @@
#include "plugin/plugin_manager.h"

#include <QSettings>
#include <QDesktopWidget>
#include <QDesktopServices>
#include <QDateTime>
#include <QScreen>
#include <QStyleFactory>
#include <QLayout>
#include <QUuid>

#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
#include <QDesktopWidget>
#endif // QT_VERSION < QT_VERSION_CHECK(5, 15, 0)

#ifndef NDEBUG
#ifdef _MSC_VER
#pragma warning(push)
Expand Down Expand Up @@ -698,7 +701,18 @@ void Ecalmon::resetLayout()
setTheme(Theme::Dark);

// Back when we saved the initial window geometry, the window-manager might not have positioned the window on the screen, yet
#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
int screen_number = QApplication::desktop()->screenNumber(this);
#else
int screen_number = 0;
QScreen* current_screen = this->screen();
if (current_screen != nullptr)
{
screen_number = QApplication::screens().indexOf(current_screen);
if (screen_number < 0)
screen_number = 0;
}
#endif // QT_VERSION < QT_VERSION_CHECK(5, 15, 0)

restoreGeometry(initial_geometry_);
restoreState(initial_state_);
Expand Down
4 changes: 4 additions & 0 deletions app/mon/mon_gui/src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ namespace QtUtil
{
inline QString variantToString(const QVariant& variant)
{
#if (QT_VERSION >= QT_VERSION_CHECK(6, 0, 0))
switch (variant.typeId())
#else
switch ((QMetaType::Type)variant.type())
#endif
{
case QMetaType::Bool:
return variant.toBool() ? "True" : "False";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ void ProcessWidget::autoSizeColumns()
//example_process_pb.set_tsync_mode();

ProcessTreeItem* example_process_item = new ProcessTreeItem(example_process_pb);
GroupTreeItem* example_group_item = new GroupTreeItem("HNAME00____", "", "", QVariant::Invalid, "");
GroupTreeItem* example_group_item = new GroupTreeItem("HNAME00____", "", "", QVariant(), "");

process_tree_model_->insertItem(example_process_item);
process_tree_model_->insertItem(example_group_item);
Expand Down
Loading
Loading