Backwards-compatible Qt6 support #50
Conversation
* changed cmake files to use qt6 instead of qt5 * changed travis ci to use qt6 and only macos (for now)
There was a problem hiding this comment.
Now that's a pretty nice PR :-) Thank you @prototypicalpro for finishing the Qt6 support! And bonus points for improving the README, that's really welcome.
There is one point where the mouse up event actually has to be constructed differently in the two versions, but otherwise looks good.
lib/src/Scene/Qt/QtEvents.cpp
Outdated
| m_pressedMouseButtons ^= button; | ||
| Qt::MouseButton eventCausingButton = getQtMouseButtonValue(button); | ||
| Qt::MouseButtons activeButtons = getQtMouseButtonValue(m_pressedMouseButtons); | ||
| m_pressedMouseButtons ^= button; |
There was a problem hiding this comment.
Unfortunately this change breaks compatibility with Qt5. You can see this when you run the example apps under Qt5. The basic example will only report Button 1 presses, the RepeaterLoader one will not work at all. Sorry I forgot to mention this earlier...
When first looking into Qt6+5 support, I didn't find any proper documentation on why Qt5 seems to expect activeButtons to be the state after the mouseUp, while Qt6 expects activeButtons to be the state before the event. I only figured this out by experiment :-/
I think we can use the QT_VERSION macro here. A bit ugly, but I guess that's how it has to be :(
https://doc.qt.io/qt-5/qtglobal.html#QT_VERSION_CHECK
There was a problem hiding this comment.
Sounds good! I updated the code to use QT_VERSION_CHECK and added the RepeaterLoader and GTest examples to CI so it'll catch bugs like this in the future.
36878df to
36d73e2
Compare
36d73e2 to
4089b6f
Compare
|
|
||
| add_executable(SpixRepeaterLoaderExample "main.cpp" "qml.qrc") | ||
| add_executable(SpixRepeaterLoaderExampleGTest "main_gtest.cpp" "qml.qrc") |
There was a problem hiding this comment.
FYI this could be it's own example if needed, but I figured just adding another target was the easiest way w/o copying every resource file.
|
|
||
| add_executable(SpixRepeaterLoaderExample "main.cpp" "qml.qrc") | ||
| add_executable(SpixRepeaterLoaderExampleGTest "main_gtest.cpp" "qml.qrc") |
This PR aims to finish the work started by @faaxm in #29: it contains the same changes plus reworking of the CMake files to allow for Qt5 and Qt6 support (I didn't attempt to add support for optional libraries). The reworks are as follows:
SPIX_QT_MAJORcache variable which controls which major Qt version Spix builds against (right now it defaults to Qt6). Updated all CMake files to use this variable.cmake_language. This change could be reverted if needed, but the alternative is a bit hacky.@faaxm let me know if you would like me to make any changes (or feel free to push changes yourself).