[Qt, OSX] QProgressBar CPU-Issue workaround #5296

1. 
   jonasschnelli commented at 2:55 pm on November 17, 2014: contributor

   fixes #5295

   I know it’s a bit of a hack. But it saves a lot of CPU ticks on OSX. You might decide to take this in because it does the job and the QTBUG-15631 seams to be open and untouched since 2010.

2. jonasschnelli force-pushed on Nov 17, 2014
3. [Qt, OSX] QProgressBar CPU-Issue workaround

   fixes #5295

   6093aa1bb0
4. jonasschnelli force-pushed on Nov 17, 2014
5. 
   laanwj commented at 3:29 pm on November 17, 2014: member

   I’m not too happy about more and more platform-specific code, but if this is really required, fine with me.

   Please do move the implementation to guiutil, though.

6. 
   sipa commented at 3:34 pm on November 17, 2014: member

   You can use a typedef to give the new name to the original class on non-Mac platforms, so the application code can be free of platform-specifics.
7. 
   jonasschnelli commented at 3:34 pm on November 17, 2014: contributor

   I also don’t like it. But i like it more than useless burn cpu. It would bring back the possibility of a AppNapping and a Network-Disabling-Switch (some people might do the same like i do and block bitcoin-qt traffic with soft-firewalls to prevent cpu/network-bandwidth while on the go).
8. 
   Diapolo commented at 6:29 pm on November 17, 2014: none

   Is there a patch ready from Qt upstream that could be used during the build? I know we already include some patches during build time.
9. 
   jonasschnelli commented at 8:38 am on November 18, 2014: contributor

   @Diapolo i did search for a qt patch but could nothing found. I also followed the idea of writing a qt patch (root cause fixing) but didn’t succeed.

   I just think we owe the user a more or less bug free bitcoin-qt client. This pull does not follow a state of the art architecture but makes the enduser experience better.

   I try now to move it to guiutil and try to follow sipa’s advice with the typedefapproach

10. 
    jonasschnelli commented at 9:00 am on November 18, 2014: contributor

    @sipa i just implemented typedef “switch”. But IMO it more unclear then. I would recommend to keep it as it was with the #ifdef directly while instantiating the object. I think it is better understandable and the code is shorter. What do you think?
11. 
    sipa commented at 9:03 am on November 18, 2014: member

    I’ll leave that up to @laanwj. I was just giving a suggestion on how you could keep the platform-dependence out of the application code.
12. 
    laanwj commented at 1:40 pm on November 18, 2014: member

    Agree with @sipa; something like

     0#include <QProgressBar>
     1#ifdef Q_OS_MAC
     2#include <QEvent>
     3
     4// workaround for Qt OSX Bug:
     5// https://bugreports.qt-project.org/browse/QTBUG-15631
     6// QProgressBar uses around 10% CPU even when app is in background
     7class ProgressBar : public QProgressBar
     8{
     9 bool event(QEvent *e) {
    10 return (e->type() != QEvent::StyleAnimationUpdate) ? QProgressBar::event(e) : false;
    11 }
    12};
    13#else
    14typedef QProgressBar ProgressBar;
    15#endif
    

    Then in the code you can just do

    0progressBar = new GUIUtil::ProgressBar();
    

    …would contain the platform specific code this to just guiutil.h. We just define our own progress bar class which happens to be different on a certain OS.

13. laanwj added the label GUI on Nov 18, 2014
14. laanwj added the label Mac on Nov 18, 2014
15. [Qt, OSX] move QProgressBarMac to guiutil.h 0ceab00d16
16. jonasschnelli force-pushed on Nov 19, 2014
17. 
    jonasschnelli commented at 1:10 pm on November 19, 2014: contributor

    Okay. Now i see. I just updated the pull.
18. laanwj merged this on Nov 19, 2014
19. laanwj closed this on Nov 19, 2014

    ---

20. laanwj referenced this in commit e587ecd8a6 on Nov 19, 2014
21. 
    Pokerkoffer commented at 4:51 pm on November 21, 2014: none

    Making all in src CXX libbitcoin_util_a-clientversion.o AR libbitcoin_util.a CXX qt/qt_libbitcoinqt_a-bitcoingui.o In file included from qt/bitcoingui.cpp:10: ./qt/guiutil.h:199:42: error: no member named ‘StyleAnimationUpdate’ in ‘QEvent’ return (e->type() != QEvent::StyleAnimationUpdate) ? QProg… ~~~~~~~~^ 1 error generated. make[2]: *** [qt/qt_libbitcoinqt_a-bitcoingui.o] Error 1 make[1]: *** [all-recursive] Error 1 make: *** [all-recursive] Error 1

    Got this with current commit a574189e2acdaa03658bfe495740b2d722984467

22. 
    laanwj commented at 4:52 pm on November 21, 2014: member

    What version of Qt? Probably it needs a version check too.
23. 
    Pokerkoffer commented at 4:53 pm on November 21, 2014: none

    Admin@MacBookPro-3:~/Desktop/test/bitcoin $ brew upgrade qt Error: qt-4.8.6 already installed
24. 
    laanwj commented at 5:07 pm on November 21, 2014: member

    OK - looks like this should be a Qt5-only workaround
25. 
    jonasschnelli commented at 5:20 pm on November 21, 2014: contributor

    Hmm.. Yes. The enum is only in qt5 available. I will wrap it in a Qt5 #ifdef.
26. 
    Pokerkoffer commented at 5:22 pm on November 21, 2014: none

    Thank you guys!
27. MarcoFalke locked this on Sep 8, 2021

Contributors
jonasschnelli laanwj sipa Diapolo Pokerkoffer

Labels
GUI macOS