Skip to content

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Jun 21, 2022

One more step in migration to Qt 6.

Could be tested with hebasto/bitcoin#3 or bitcoin/bitcoin#24798.

No behavior change when compiling with Qt 5.

hebasto added 4 commits June 21, 2022 13:14
In Qt 6, registration of `QDataStream` streaming operators is done
automatically. Consequently, `qRegisterMetaTypeStreamOperators()` does
no longer exist. Calls to this method have to be removed when porting
to Qt 6.

See https://doc.qt.io/qt-6/qtcore-changes-qt6.html#the-qmetatype-class
In Qt 6, high DPI pixmaps and scaling are always enabled.

https://doc.qt.io/qt-6/highdpi.html
@hebasto hebasto added the Qt 6 Transition to Qt 6 label Jun 21, 2022
@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 21, 2022

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

Concept ACK, need to finish testing

  • 3f51d0b

    • I have confirmed with Qt Docs that the commit description is correct and we do in fact need to remove the qRegisterMetaTypeStreamOperator when building with Qt6. The usage of qRegisterMetaType is the appropriate call for Qt6.
  • ad73447

    • Confirmed with Qt docs that for Qt6 we should use QLibraryInfo::path instead of QLibraryInfo::location. The new code here is correct as we use QLibraryInfoPath and we pass in a valid ::LibraryPath enum in ::TranslationsPath
  • d8d99d0

    • Confirming with Qtdocs that we don't need to set hidpi flags with Qt6: "... Qt 5 behavior assumes that AA_EnableHighDpiScaling has been set (this flag is not needed on Qt 6)"

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

ACK d8d99d0

Tested with Qt6 and the experimental Cmake branch and confirmed the window size and components look fine on a HIDPI and low dpi display. Also tested that I can still choose and switch through translations. Additionally, I performed a sanity check building with Qt5.

@laanwj
Copy link
Member

laanwj commented Jun 23, 2022

Code review ACK d8d99d0
(and lightly tested on Qt5 only, the GUI starts, doesn't look weird, translations still work)

@laanwj laanwj merged commit bc83710 into bitcoin-core:master Jun 23, 2022
@hebasto hebasto deleted the 220622-qt6 branch June 23, 2022 10:05
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 23, 2022
@bitcoin-core bitcoin-core locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Qt 6 Transition to Qt 6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants