Skip to content

Use "fusion" style on macOS Big Sur with old Qt #177

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 28, 2021
Merged

Use "fusion" style on macOS Big Sur with old Qt #177

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions src/qt/bitcoin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#include <QThread>
#include <QTimer>
#include <QTranslator>
#include <QtGlobal>

#if defined(QT_STATICPLUGIN)
#include <QtPlugin>
Expand Down Expand Up @@ -466,6 +467,13 @@ int GuiMain(int argc, char* argv[])
QCoreApplication::setAttribute(Qt::AA_EnableHighDpiScaling);
#endif

#if (QT_VERSION <= QT_VERSION_CHECK(5, 9, 8)) && defined(Q_OS_MACOS)
Copy link
Member

Choose a reason for hiding this comment

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

Based on the linked Qt bug, this version check appears wrong?

Copy link
Member

Choose a reason for hiding this comment

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

This version check is correct because #136 concerns Qt compiled on macOS with depends. Compiling with depends builds bitcoin-qt with Qt 5.9.8. See: Qt Depends Package

Copy link
Member Author

Choose a reason for hiding this comment

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

Based on the linked Qt bug, this version check appears wrong?

The link to Qt bug removed from the OP. It is possible that version check could be improved, but I do not know for sure from which Qt version widgets are quite usable (I do not even say about fixing all bugs).

The recent Homebrew's Qt versions look decent (but not ideal). And this PR goal is to fix GUI look while building with depends (including gitian binary).

const auto os_name = QSysInfo::prettyProductName();
if (os_name.startsWith("macOS 11") || os_name.startsWith("macOS 10.16")) {
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it should apply to 12+ as well, though probably the issue will be long-gone by then

Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine as is. This is meant to be a patch until the Qt version is bumped up from 5.9.8, not a permanent fix. Presumably, we will get the Qt version bumped up before there is a macOS 12.

Copy link
Member Author

@hebasto hebasto Jan 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Perhaps it should apply to 12+ as well, though probably the issue will be long-gone by then

Did not test 10.12, but there are no such visual issues on Catalina 10.15.7 (19H114) with the recent 0.21.0 release.

I believe the "macintosh" style is broken on macOS Big Sur only.

UPDATE: v0.21.0 looks on Mojave 10.14 fine.

QApplication::setStyle("fusion");
}
#endif

BitcoinApplication app;

/// 2. Parse command-line options. We do this after qt in order to show an error if there are problems parsing these
Expand Down