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

Qt6 + ubuntu #4679

Merged
merged 11 commits into from
Feb 16, 2022
25 changes: 24 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ if(POLICY CMP0071)
cmake_policy(SET CMP0071 NEW)
endif()

# Propagate interface link properties
if(POLICY CMP0099)
# This avoids a warning when qt deals with different behaviours controlled by this policy
# in its cmake functions. See
# https://github.com/qt/qtbase/commit/e3e1007f9778d8fc629a06f7d3d216bb7f81351b
cmake_policy(SET CMP0099 NEW)
endif()


# Set up vcpkg
if(DEFINED ENV{VCPKG_ROOT} AND NOT DEFINED VCPKG_ROOT)
set(VCPKG_ROOT "$ENV{VCPKG_ROOT}")
Expand Down Expand Up @@ -1256,7 +1265,16 @@ elseif(UNIX)
endif()

# The mixxx executable
add_executable(mixxx WIN32 src/main.cpp)
if(QT6)
find_package(Qt6 REQUIRED COMPONENTS Core) # For Qt Core cmake functions
# qt_add_executable() is the recommended initial call for qt_finalize_target()
# below that takes care of the correct object order in the resulting binary
# According to https://doc.qt.io/qt-6/qt-finalize-target.html it is importand for
# builds with Qt < 3.21
qt_add_executable(mixxx WIN32 src/main.cpp MANUAL_FINALIZATION)
Copy link
Member

Choose a reason for hiding this comment

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

Please add a comment that explains why this is necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

In my test with cmake 3.16 it was actually not necessary.
It is just mentioned as the counterpart to the required qt-finalize-target which takes care of object order during linking. See https://doc.qt.io/qt-6/qt-finalize-target.html#qt6-finalize-target
So I think it is better to add it and follow the docs.

Copy link
Member

@Holzhaus Holzhaus Feb 16, 2022

Choose a reason for hiding this comment

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

From the docs I don't see any indication why this is only necessary on cmake < 3.21?

Copy link
Member Author

Choose a reason for hiding this comment

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

"This is especially important if using a statically built Qt or a CMake version earlier than 3.21."

There is also a pending PR that removes the related tests in the QT CI for cmake > 3.21. Unfortunately I do not have saved the link.

Copy link
Member

Choose a reason for hiding this comment

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

I read that too, but in my understanding it means "It's always important, but especially important on CMake < 3.21". So I don't think we should use a version guard here. Less code paths = less chances we run into weird bugs that CI doesn't detect or that we can't reproduce.

I tested it with this patch on CMake 3.22.2 and it works fine:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index c7c7743072..f0f31773ea 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1265,12 +1265,10 @@ elseif(UNIX)
 endif()

 # The mixxx executable
-if(QT6 AND CMAKE_VERSION VERSION_LESS "3.21")
+if(QT6)
   find_package(Qt6 REQUIRED COMPONENTS Core) # For Qt Core cmake functions
   # qt_add_executable() is the recommended initial call for qt_finalize_target()
   # below that takes care of the correct object order in the resulting binary
-  # According to https://doc.qt.io/qt-6/qt-finalize-target.html it is importand for
-  # builds with Qt < 3.21
   qt_add_executable(mixxx WIN32 src/main.cpp MANUAL_FINALIZATION)
 else()
   add_executable(mixxx WIN32 src/main.cpp)
@@ -2208,11 +2206,9 @@ if(QT6)
   )
   target_link_libraries(mixxx-lib PRIVATE mixxx-qml-mixxxcontrolsplugin)

-  if(CMAKE_VERSION VERSION_LESS "3.21")
-    # qt_finalize_target takes care that the resources :/mixxx.org/imports/Mixxx/
-    # and :/mixxx.org/imports/Mixxx/Controls are placed into beginning of the binary
-    qt_finalize_target(mixxx)
-  endif()
+  # qt_finalize_target takes care that the resources :/mixxx.org/imports/Mixxx/
+  # and :/mixxx.org/imports/Mixxx/Controls are placed into beginning of the binary
+  qt_finalize_target(mixxx)
 endif()

 target_compile_definitions(mixxx-lib PUBLIC QT_TABLET_SUPPORT QT_USE_QSTRINGBUILDER)

else()
add_executable(mixxx WIN32 src/main.cpp)
endif()
# ugly hack to get #include "preferences/dialog/ui_dlgpreferencesdlg.h" to work in
# src/qmldlgpreferencesproxy.h, which is #included from src/qmlapplication.h.
target_include_directories(mixxx PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/mixxx-lib_autogen/include")
Expand Down Expand Up @@ -2162,6 +2180,7 @@ if(QT6)
URI Mixxx
VERSION 0.1
RESOURCE_PREFIX /mixxx.org/imports
IMPORTS QtQuick
QML_FILES
res/qml/Mixxx/MathUtils.mjs
res/qml/Mixxx/PlayerDropArea.qml
Expand Down Expand Up @@ -2201,6 +2220,10 @@ if(QT6)
res/qml/Mixxx/Controls/WaveformOverview.qml
)
target_link_libraries(mixxx-lib PRIVATE mixxx-qml-mixxxcontrolsplugin)

# qt_finalize_target takes care that the resources :/mixxx.org/imports/Mixxx/
# and :/mixxx.org/imports/Mixxx/Controls are placed into beginning of the binary
qt_finalize_target(mixxx)
endif()

target_compile_definitions(mixxx-lib PUBLIC QT_TABLET_SUPPORT QT_USE_QSTRINGBUILDER)
Expand Down
1 change: 1 addition & 0 deletions res/qml/Mixxx/qmldir
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ linktarget mixxx-libplugin
optional plugin mixxx-libplugin
classname MixxxPlugin
typeinfo mixxx-lib.qmltypes
import QtQuick
prefer :/mixxx.org/imports/Mixxx/
MathUtils 0.0 res/qml/Mixxx/MathUtils.mjs
PlayerDropArea 0.0 res/qml/Mixxx/PlayerDropArea.qml
1 change: 1 addition & 0 deletions src/qml/qmlapplication.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "qml/qmlvisibleeffectsmodel.h"
#include "qml/qmlwaveformoverview.h"
#include "soundio/soundmanager.h"
Q_IMPORT_QML_PLUGIN(MixxxPlugin)
Q_IMPORT_QML_PLUGIN(Mixxx_ControlsPlugin)

namespace {
Expand Down
2 changes: 2 additions & 0 deletions src/qml/qmlwaveformoverview.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ class QmlWaveformOverview : public QQuickPaintedItem {
Q_ENUM(Renderer)

QmlWaveformOverview(QQuickItem* parent = nullptr);
~QmlWaveformOverview() override = default;

void paint(QPainter* painter);

void setPlayer(QmlPlayerProxy* player);
Expand Down