Remove unnecessary unpolish operation of the style, before polish the new style
Using the profiler I noticed that ~11% of the main thread calculation time was consumed for updating push-button widgets (while one deck playing and the others CUE button slowly blinking). I found out that removing that removing the style->unpolish() call before every style->polish() call has no visual effect, but impacts the CPU time significiantly.
I guess this code is either not needed on todays Qt anymore (I use the official Mixxx buildenv with Qt 5.15.8) or it's a platform specific behavior.
Oh 11%.. Reverted to draft. Let's first confirm on all platforms, official builds and local ones, to not introduce another regression five to twelve.
Still valid according to the Wiki: https://wiki.qt.io/Technical_FAQ#How_can_my_stylesheet_account_for_custom_properties.3F
The Stackoverflow link should be replaced by the Wiki link.
This work is completed and ready for test and review
All skins still looking good.
For reference: QStyleSheetStyle::polish(QWidget *w) in Qt 5.12.7
I'm still worried to merge it that late before the release. @mixxxdj/developers could someone please test on macOS that skin styles are still good? Especially those for native and custom widget properties:
-
hover: style of pushbuttons, e.g. in Deere -
pressed: orange bg + black icon of pressed Loop In button in LateNight
Yes, I aggree, that this is too risky short before a release
Yeah, let's move any risky changes to 2.4.1. Both we and users will have a much easier time pinpointing regressions between 2.4.0 and 2.4.1 than in the large batch of changes between 2.3.x and 2.4.0.
(I'd argue that we should probably release more often in general. Running a dev/beta for too long will always risk having users "out of the loop" of changes that might only regress something in certain scenarios. I consider #3761 a good example of how platform-specific bugs can linger for years if there are too few users actually battle-testing the program)
@ronso0 Can we merge this now? The longer we test it, the better.
Sure, thanks for the reminder!
"The longer we test it, the better." also applies to #11526 btw :wink:
Thank you!
Oh no, didn't pay attention: This should have gone to main. I doubt 2.4.1 gets sufficient testing before release, for example if we fix a somewhat severe bug and release soon after.
The outcome of the discussion above was, that it should go into 2.4.1 - wasn't it?
Yes, but reconsidering I came to the conclusion I just posted. Anyway, if there are issues then there will be 2.4.2 : )