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

hebasto · 2021-01-07T21:59:33Z

The "macintosh" style is broken on macOS Big Sur:

hebasto · 2021-01-07T22:02:54Z

As the upcoming 0.21 release is the first one after macOS Big Sur release, do you mind considering to backport this change into the 0.21 branch in order to ship the usable binaries to Big Sur users?

prusnak · 2021-01-07T23:23:11Z

Why is there the version check for Qt? I see the same issue with Qt 5.12 on Big Sur.

Also checking for "macOS 11" is spurious.

Do we need the version checks at all? Why don't just apply this always on macOS?

hebasto · 2021-01-07T23:49:18Z

Why is there the version check for Qt?

This change, actually, is intended to fix visual quality for builds with depends, including shipped releases.

I see the same issue with Qt 5.12 on Big Sur.

With the latest Homebrew's Qt 5.15.2 the native macOS style looks much better.

Also checking for "macOS 11" is spurious.

Not sure if I understand you correctly. Do you mind elaborating your comment?

Do we need the version checks at all? Why don't just apply this always on macOS?

Many (most?) macOS users prefer the native macOS style, and the "fusion" does not look native on macOS.

jarolrod

Concept ACK

I applied this patch to the rc5 source code to see how it would react on macOS Big Sur 11.1 with bitcoin-qt built with Qt 5.9.8.

The name check does not work:
if (QSysInfo::prettyProductName().startsWith("macOS 11"))
Removing this check successfully allows the Qt Fusion Style to appear. I'll investigate what's wrong with the naming.

Branch: 9f50395
Qt Fusion Style fails to appear

Removing The Conditional
Fusion Style appears

The "macintosh" style is broken on macOS Big Sur at least for Qt 5.9.8.

hebasto · 2021-01-08T14:25:36Z

Updated 9f50395 -> 4e1154d (pr177.01 -> pr177.02, diff):

addressed @jarolrod's comment:

The name check does not work:
if (QSysInfo::prettyProductName().startsWith("macOS 11"))

jarolrod

ACK 4e1154d

Qt Fusion successfully shows. Tested with patched rc5 source code on macOS Big Sur with Qt 5.9.8

jonasschnelli · 2021-01-11T13:45:44Z

This PR macOS 11 dark mode (self compiled against Qt 5.14):

Master Dark Mode macOS 11:

I can't see any major visual differences.
Master already looks good on macOS BigSure. Is that due to Qt 5.14?

hebasto · 2021-01-11T14:07:30Z

@jonasschnelli

Is that due to Qt 5.14?

Yes:

gui/src/qt/bitcoin.cpp

Lines 470 to 475 in 4e1154d

    
           #if (QT_VERSION <= QT_VERSION_CHECK(5, 9, 8)) && defined(Q_OS_MACOS) 
        
               const auto os_name = QSysInfo::prettyProductName(); 
        
               if (os_name.startsWith("macOS 11") || os_name.startsWith("macOS 10.16")) { 
        
                   QApplication::setStyle("fusion"); 
        
               } 
        
           #endif

With Qt 5.14 the "macintosh" style looks not ideal, but usable.
With Qt 5.9.8, that is shipped to users as gitian builds, it is unusable at all.

luke-jr · 2021-01-13T16:52:38Z

src/qt/bitcoin.cpp

@@ -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)


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

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

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).

luke-jr · 2021-01-13T16:54:16Z

src/qt/bitcoin.cpp

@@ -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)
+    const auto os_name = QSysInfo::prettyProductName();
+    if (os_name.startsWith("macOS 11") || os_name.startsWith("macOS 10.16")) {


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

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.

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.

hebasto · 2021-01-21T00:43:30Z

Btw, there is an alternative to this PR. It is reverting of bitcoin/bitcoin#16578. Then it would be possible to pass -style Fusion as a command line argument.

cc @achow101

achow101 · 2021-01-21T02:31:35Z

Btw, there is an alternative to this PR. It is reverting of bitcoin/bitcoin#16578

NACK. The details will become clear soon.

luke-jr · 2021-01-21T15:21:11Z

Btw, there is an alternative to this PR. It is reverting of bitcoin/bitcoin#16578. Then it would be possible to pass -style Fusion as a command line argument.

Users should not need to pass options.

maflcko · 2021-01-21T20:10:21Z

run appveyor (will need to be rerun before merge)

maflcko · 2021-01-25T15:01:22Z

review ACK 4e1154d can't test

laanwj · 2021-01-26T08:49:32Z

So nice to see we're finally going to support MacOS dark mode.
(Concept ACK, can't test this)

jonasschnelli

Tested ACK 4e1154d

The "macintosh" style is broken on macOS Big Sur at least for Qt 5.9.8. Github-Pull: #bitcoin-core/gui#177 Rebased-From: 4e1154d

maflcko · 2021-01-28T09:17:15Z

Backported in bitcoin/bitcoin#20901

77833a3 Revert "qt: Use "fusion" style on macOS Big Sur with old Qt" (Hennadii Stepanov) Pull request description: This PR reverts workaround introduced in #177. After bumping Qt version in depends to 5.12.10 in bitcoin/bitcoin#21376, there are no reasons to use the Fusion style on macOS. ACKs for top commit: leonardojobim: tACK 77833a3. Tested on macOS Big Sur v11.2.3 jarolrod: ACK 77833a3 Talkless: utACK 77833a3 Tree-SHA512: f704f2027dd380dfc604231e3606a036a8be891aeeddf643c474131014fa080e123b42836ac643a2064fe7a5a018fa8b9aa61a31f9da1d15880de6a36c4c0d54

The "macintosh" style is broken on macOS Big Sur at least for Qt 5.9.8. Github-Pull: #bitcoin-core/gui#177 Rebased-From: 4e1154d (cherry picked from commit 6dc58e9)

This was referenced Jan 7, 2021

v0.21.0 testing bitcoin/bitcoin#20555

Closed

macOS 11 (Big Sur) visual bugs #136

Closed

maflcko added this to the 0.21.1 milestone Jan 8, 2021

jarolrod reviewed Jan 8, 2021

View reviewed changes

qt: Use "fusion" style on macOS Big Sur with old Qt

4e1154d

The "macintosh" style is broken on macOS Big Sur at least for Qt 5.9.8.

hebasto force-pushed the 210107-style branch from 9f50395 to 4e1154d Compare January 8, 2021 14:22

jarolrod reviewed Jan 8, 2021

View reviewed changes

maflcko assigned jonasschnelli Jan 11, 2021

luke-jr suggested changes Jan 13, 2021

View reviewed changes

maflcko added the macOS label Jan 21, 2021

maflcko closed this Jan 21, 2021

maflcko reopened this Jan 21, 2021

hebasto mentioned this pull request Jan 22, 2021

Disappearing progress bar with Qt 5.15.2 #193

Closed

jonasschnelli approved these changes Jan 28, 2021

View reviewed changes

jonasschnelli merged commit 02b0165 into bitcoin-core:master Jan 28, 2021

maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Jan 28, 2021

qt: Use "fusion" style on macOS Big Sur with old Qt

6dc58e9

The "macintosh" style is broken on macOS Big Sur at least for Qt 5.9.8. Github-Pull: #bitcoin-core/gui#177 Rebased-From: 4e1154d

hebasto deleted the 210107-style branch January 28, 2021 09:46

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 28, 2021

Merge bitcoin-core/gui#177: Use "fusion" style on macOS Big Sur with …

8f9dacb

jarolrod mentioned this pull request Feb 25, 2021

qt: Support macOS Dark mode #154

Merged

hebasto mentioned this pull request Mar 12, 2021

Revert "qt: Use "fusion" style on macOS Big Sur with old Qt" #246

Merged

jarolrod mentioned this pull request Jul 6, 2021

Shortcut Control+I and Control+O don't work on macOS Big Sure #370

Closed

bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 2022

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

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

Uh oh!

Conversation

hebasto commented Jan 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Jan 7, 2021

Uh oh!

prusnak commented Jan 7, 2021

Uh oh!

hebasto commented Jan 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jarolrod left a comment

Choose a reason for hiding this comment

Uh oh!

hebasto commented Jan 8, 2021

Uh oh!

jarolrod left a comment

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Jan 11, 2021

Uh oh!

hebasto commented Jan 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luke-jr Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

jarolrod Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

hebasto Jan 15, 2021

Choose a reason for hiding this comment

Uh oh!

luke-jr Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

jarolrod Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

hebasto Jan 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hebasto commented Jan 21, 2021

Uh oh!

achow101 commented Jan 21, 2021

Uh oh!

luke-jr commented Jan 21, 2021

Uh oh!

maflcko commented Jan 21, 2021

Uh oh!

maflcko commented Jan 25, 2021

Uh oh!

laanwj commented Jan 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonasschnelli left a comment

Choose a reason for hiding this comment

Uh oh!

maflcko commented Jan 28, 2021

Uh oh!

Uh oh!

hebasto commented Jan 7, 2021 •

edited

Loading

hebasto commented Jan 7, 2021 •

edited

Loading

hebasto commented Jan 11, 2021 •

edited

Loading

hebasto Jan 15, 2021 •

edited

Loading

laanwj commented Jan 26, 2021 •

edited

Loading