strawberrymusicplayer / strawberry

:strawberry: Strawberry Music Player
https://www.strawberrymusicplayer.org/
GNU General Public License v3.0
2.76k stars 194 forks source link

(macOS) High CPU Usage when scrolling in Playlist View on a high-DPI Retina Screen #542

Open Tibokan opened 4 years ago

Tibokan commented 4 years ago

Describe the bug Probably a macOS, QT-specific issue (as it also exists on Clementine 1.3.1), but scrolling in Playlist View (even when not playing any tracks in the background) is very choppy in performance, with large CPU spikes and a tanking framerate (down to 8-10 frames on my high-spec Mac Pro, worse if glowing track animation is turned on, and there is a large list to scroll through). Scrolling the playlist view with a track playing will also tank the framerate of the block analyser.

To Reproduce Add a list of songs to Playlist Views and scroll using mouse wheel.

Expected behavior Scroll smoothly, as there shouldn't be anything CPU-heavy happening when scrolling a relatively static/non-dynamic playlist view.

System Information:

Tibokan commented 4 years ago

I wonder if it has anything to do with this QT bug report: https://bugreports.qt.io/plugins/servlet/mobile#issue/QTBUG-73117

jonaski commented 4 years ago

The playlist is actually a QTreeView (same as the collection), but they are both based on a QAbstractItemView, so yes it might be related. I can't reproduce this problem in my VM but the screen size there is very small, so that might be the reason. The performance have actually been very good recently, not sure if it happened after I upgraded to Qt 5.15.1, but it's never been this good, there is no choppy scrolling of the playlist at all. Until recently everything was choppy and laggy for me on macOS.

Tibokan commented 4 years ago

Making the window really small improves the scrolling performance a fair bit, though there is still a noticeable hit to performance (I use an app called iStat Menus that has a frames monitor, which allows me to see the hit to number of rendered frames when scrolling - drops to 6-12 at full screen, 1680x1050, when window is very small, 18-20 (close to the magic 25 for smoothness)

Tibokan commented 4 years ago

The collection view also has impacted scrolling performance, but I think because its such a finite list, both column and size wise, the noticeability of the performance impact is minimal there.

Is there an alternate way of doing the collection and playlist view on MacOS that might alleviate this issue?

llucps commented 4 years ago

Hi.

Are you using a HDPI screen? (aka retina) I've noticed a big difference in performance when using Strawberry with the retina display from my Macbook Pro and when connected to external displays (22" 1920x1080px) and 25(2560 x 1440px) it works way better. this on Mac OS Catalina, but I don't think the macos version matters.

I would guess the performance is because the QT implementation on macOS..

Is there any improvement on QT 6 @jonaski ?

Thanks.

jonaski commented 4 years ago

Try turn off the background image and the "source" column to see if that makes any difference. But there is so much stuff going on so it's hard to tell without running some sort of profiling to see what uses most resources. But it does not do anything specific to macOS in the playlist view, so the most likely reason is a Qt-macOS problem. On macOS there is a program called "instruments", I think you need to install xcode for it to be available. I have never used it before. I think it should reveal what is going on. There is a tutorial here: https://developer.apple.com/videos/play/wwdc2019/411/

Tibokan commented 4 years ago

@llucps it's a standard Samsung 24 inch IPS monitor that I have my Mac Pro plugged into, not an Apple Retina display. I noticed that playing around with the resolutions will make this problem better or worse - lower resolutions will improve frame performance (similar to the way making the Strawberry window smaller improves performance), and higher resolutions (I can go up to 1920x1080) reduces frame performance. I can imagine it being even worse on a Retina display, as the standard resolutions on that are 2560×1600 and above.

Background image doesn't seem to be related to this issue, as there is the same performance impact on scroll with it on or off. I have also tested with source column on/off with no difference (I typically have that column off by default anyway).

It feels like something is happening in the background when scrolling these views, probably the multiple, unnecessary repaints mentioned in that bug report. Since it's unresolved in QT5, not sure if there is much @jonaski can do to fix this performance issue unless there's an alternate way of doing the views, though hopefully it will get fixed in QT6.

I will have a go with instruments to uncover more information once I get some free time later this week.

jonaski commented 4 years ago

If you can figure out instruments I think that would really help it get fixed even if the bug is in Qt. I have only tested with Qt 6 on Linux so far, but my initial impression is that there is a performance increase. I actually use strawberry compiled with Qt 6 on daily basis already on Linux, everything seem to be working fine. Final release of Qt 6 is just a few months away, November I think, and homebrew is pretty quick to update packages, so then I will probably be able to switch to Qt 6 pretty fast for macOS and windows builds.

Tibokan commented 4 years ago

Struggling to get hold of Xcode at the moment, with Apple's stringent latest OS requirements (Catalina) on Xcode and the fact I haven't added it to my Mac account in the past, meaning that I am having to delve into the Apple archives to try and find a compatible Xcode version to suit High Sierra (which looks like it tops out at Xcode 10.1). So far, everything I have downloaded has failed to extract and install.

I will gain access to a Catalina-based Apple machine within the next week, so should be able to get Xcode attached to my Mac account and get hold of the latest version to do some testing using Instruments.

On a side note, having used Strawberry a fair bit over the past week on my less powerful 2014 MacBook Pro (which has a far less powerful CPU in it compared to my 2013 Mac Pro), the scrolling performance is noticeably choppier, so it seems CPU specs plays a part in how noticeable this scrolling issue is.

Hopefully QT6 will solve the issue altogether, though I will still try and get some Instruments info and reports back to you in the meantime.

Tibokan commented 4 years ago

Would it be possible to add a beta QT6 build of Strawberry for MacOS to the Latest Builds section?

jonaski commented 4 years ago

Yes but it is a bit work to set up compiling Qt 6 dev branch for mac.

Tibokan commented 4 years ago

I have now gotten Xcode working, and have had a play with Instruments - this is on a Catalina-based iMac.

I ran a couple of traces on Strawberry using the Animation Hitches tool:

Screen Shot 2020-09-25 at 11 45 58 am

Whilst scrolling, it does appear there are multiple CPU calls every split second, which is probably what is tanking the frame rate and scrolling performance:

Screen Shot 2020-09-25 at 11 44 05 am

Here's a detailed trace on Strawberry and the primary CPU offenders:

Screen Shot 2020-09-25 at 12 02 28 pm Screen Shot 2020-09-25 at 11 53 26 am Screen Shot 2020-09-25 at 12 04 10 pm

I'm not sure if the above screenshots tell you much (outside of this definitely being CPU-related and focused around QPainter), so if there is another Instruments tool and output that would be more informative for you in diagnosing this issue, please let me know.

Tibokan commented 4 years ago

This link seems to talk about the same issue identified in the screenshots above regarding the QPainter DrawImage on MacOS impacting performance and it details a potential workaround to consider: https://forum.qt.io/topic/84262/mac-os-repainting-parented-widget-performance-seem-to-depend-on-ancestors-chain

jonaski commented 4 years ago

Pretty sure this is in Qt and not really much we can do to improve the performance in Strawberry, it shows QMainWindow::event() which we do not re-implement.

jonaski commented 4 years ago

Closing this as it doesn't seem to be resolvable.

Tibokan commented 3 years ago

I think this should be reopened now that QT6 has been released, as it's a big problem for Strawberry on any Mac's with a high-DPI Retina Screen (pretty much everyone outside of those using MacOS on Hackintoshs). High-end specs don't help either, as my second-to-top 2020 iMac with the i7 gets 100% CPU usage and Strawberry is heavily laggy, and I'm forced to keep Strawberry playing in the background minimised, or make the Strawberry window very small, to avoid having other tasks impacted.

Tibokan commented 3 years ago

Thanks for reopening, are there any working QT6 Strawberry builds available for Catalina that I can start testing with/reporting results on? The one I was able to find in the forums/download repository crashes on launch currently

jonaski commented 3 years ago

I switched to Qt 6 now. 0.8.5.131 and newer are Qt 6 and working: https://builds.strawberrymusicplayer.org/macos

Tibokan commented 3 years ago

Thanks Jonas, I have downloaded and done some initial testing using 0.8.5.136. The good news is that Strawberry graphics performance has improved slightly compared to QT5. The bad news is that the scrolling smoothness/graphics performance on the playlist view is still noticeably laggy/CPU heavy (which then causes stuff like the Block Analyser to freeze in place/stop moving briefly when scrolling). The previous trick of making the Strawberry window really small makes the scrolling performance buttery smooth, so it seems the graphics performance/CPU usage bug that is hindering the Playlist view/scroll has flowed over from QT5 to QT6 :(

jonaski commented 3 years ago

It's not really surprising, the main changes in Qt 6 are in Core, not Gui/Widgets, and if there was a bug fixed, it would probably have been fixed in 5.15.2 too.

Tibokan commented 3 years ago

Did some research on the QTreeView performance issue - I found a potential solution to improving performance (https://stackoverflow.com/questions/19691577/qtableview-slow-performance-with-1000s-of-visible-cells) - the suggestion is to set the setUniformRowHeights flag to True (it is by default false). I’m not sure if this has already been implemented in Strawberry, it might be worth a shot if not

jonaski commented 3 years ago

I can try setting that, however when looking into the playlist view code I found the following code that is unique to macOS: setVerticalScrollMode(QAbstractItemView::ScrollPerPixel); Maybe that's what's causing it, I see it was done in 2010 here: https://github.com/clementine-player/Clementine/commit/71bfbd9aaac1d968c551fd0ed023404315e682b9#diff-ddf6206a2958cd2c20232febf2177177929323c3a225746391a126e3a4830b4e It's possible that this isn't required anymore, so worth to try removing that first.

Tibokan commented 3 years ago

I think that was added to avoid per-line scrolling - are the non-MacOS Strawberry builds scrolling on a per line basis and feel smooth/perform well?

If so, it’s probably the solution for this issue on MacOS, though it’s still strange why scrollbypixel mode would be causing such a performance impact unless there was a change in scroll implementation between QT versions 4 and 5. Maybe the high pixel count of today’s retina screens/resolution is placing too much of a load on this QT function compared to the lower resolutions/screens that would have been commonly used back in 2010?

jonaski commented 3 years ago

I think so. I made some builds with the modifications available now on https://builds.strawberrymusicplayer.org/macos/ as strawberry-playlistviewmod I removed setVerticalScrollMode and added setUniformRowHeights. Can you test and see if it helps?

jonaski commented 3 years ago

I found that the cover manager crashes on macOS the second time it is opened. It is related to the native search field. This should fix it https://github.com/strawberrymusicplayer/strawberry/commit/f5bb15f72ede306f82c08951544fad57525cfe58 It would be nice if you can test too.

Tibokan commented 3 years ago

I think so. I made some builds with the modifications available now on https://builds.strawberrymusicplayer.org/macos/ as strawberry-playlistviewmod I removed setVerticalScrollMode and added setUniformRowHeights. Can you test and see if it helps?

I've done some testing over the last few hours with the playlistviewmod dev build, and unfortunately it does not seem to have made any impact/change at all. The scrolling still appears to be by pixel (rather than by row as we had anticipated) and the performance has not been improved despite the presence of setUniformRowHeights. The performance, as before, improves when the playlist window is made very small. I'm guessing there is code somewhere that is still forcing the scrolling by pixel in Playlist View, regardless of that line of code from PlaylistView.cpp being removed?

I found that the cover manager crashes on macOS the second time it is opened. It is related to the native search field. This should fix it f5bb15f It would be nice if you can test too.

I'll do some testing once it flows through to the latest builds and report back

Tibokan commented 3 years ago

As a side issue, I did pick up on the fact that album cover thumbnails no longer seem to show up on the artist/collection view under the latest builds - reverting back to the QT5 build 0.8.5, this is not an issue. Rebuilding the db doesn't seem to make a difference.

I also noticed that Strawberry re-requires access to be granted to folders in MacOS on first run after install, which means when you upgrade your version of Strawberry to a newer build, the music collection is wiped and you have to rescan once you've regranted folder access (which requires the removal of your library folders in Preferences, and readding them, which prompts the request to grant Strawberry access to those folders). Not sure if that's something that can be fixed, but I don't recall it occurring with earlier Strawberry builds, only more recently with the QT6 build of 0.8.5.131 onwards. My music folders are on a NAS, which could be a contributing factor here.

Edit - Reverting back to the release 0.8.5 doesn't cause the folder access/music collection disappearing issue to occur - I just get prompted to provide access when trying to play a song/open the artist/collection view without losing the collection, so that issue is QT6 specific it seems

Tibokan commented 3 years ago

I can create separate issue tickets for these if required with more detail, let me know - these issues have been around since 0.8.5.131 at least

Tibokan commented 3 years ago

A thought regarding the scrolling issue (having noticed that Instruments is reporting that the animation hitches are focused around the QWidget and QPainter implementation) - Strawberry is using QWidget, which uses the CPU for rendering, versus the alternative QOpenGLWidget https://doc.qt.io/qt-5/qopenglwidget.html (and QOpenGLWindow), which instead uses the GPU for rendering.

Perhaps switching the widget implementation to use OpenGL could be an option to improve the rendering/scrolling performance by shifting the load onto the GPU (which would likely be significantly more efficient at this graphics intensive task scrolling by pixel on a high resolution Retina display compared to the CPU)?

Tibokan commented 3 years ago

I found that the cover manager crashes on macOS the second time it is opened. It is related to the native search field. This should fix it f5bb15f It would be nice if you can test too.

I'll do some testing once it flows through to the latest builds and report back

Cover manager crash is now fixed, I tested 0.9.1.5 and was able to replicate the crash, could not replicate the crash with 0.9.1.7

jonaski commented 3 years ago

Not sure what I can do about the file permission thing, I think that's macOS restricting strawberry's access. But I've found that all the plugins in 0.9.1 are broken, paths in the libraries aren't pointing correctly.

jonaski commented 3 years ago

Can you try the latest from https://builds.strawberrymusicplayer.org/macos/ and see if it works now?

Tibokan commented 3 years ago

Tried the latest .15 builds, both the Playlist View Mod version and the standard version.

Both: The cover thumbnail issue is fixed, and I wasn't able to pick up on any noticeable bugs. Cover manager doesn't crash. The file permission thing seems to be more robust now too (and is now a non-issue in my opinion), where the prompts for file access pop up instantly when opening Strawberry for the first time, and Music library access is prompted for instantly when interacting with the Artist view. The music collection doesn't disappear either when performing these actions.

The Playlist View version - GUI performance is slower than the standard version for some reason, the scroll seems to lag a bit more with the same sized window (to compare performance between the two builds), and it is not scrolling by line like it should be. I don't think the Playlist View mod is viable as a performance booster for the MacOS builds, unfortunately. Can we try an OpenGL widget implementation as a dev build (maybe just the one widget/window as an initial test to see if its a viable performance solution)?

jonaski commented 3 years ago

I had no idea you could use QOpenGLWidget for that purpose, I don't know how to do it.

Tibokan commented 3 years ago

No worries, I've added some comments under an existing QT bug report (https://bugreports.qt.io/browse/QTBUG-73117), hopefully this QWidget/QPainter performance issue on Mac may get resolved in a future QT bug fix without requiring a switch to OpenGL widgets.

llucps commented 3 years ago

Hi,

This last bug report was updated a few days ago, providing what could well be a solution.. maybe? I don't know whether this is something it could be implemented in Strawberry? If not oh well it was good to try :)

https://bugreports.qt.io/browse/QTBUG-73117

Thanks!

jonaski commented 3 years ago

It's possible to experiment with a custom playlist view delegate to see if it can be optimized.

llucps commented 3 years ago

Thanks @jonaski. Happy to help testing.

Tibokan commented 3 years ago

https://codereview.qt-project.org/c/qt/qtbase/+/353233 Looks like the QT team has confirmed that there is a bug with the scroll optimisation on both Mac and Windows, hopefully a fix will be incoming in the near future that will solve this problem once and for all

foss- commented 3 years ago

For reference: QTBUG-73117 and https://codereview.qt-project.org/c/qt/qtbase/+/373409 (merged 2022-01-13) is seeing some action.

Tibokan commented 2 years ago

Looks like this has now been fixed (as foss indicates, it was committed to the QT build in January), so this bug should now be solved once the builds are updated to compile using the latest QT 6.2.3 (or the upcoming QT 6.3) instead of the current QT 6.2.2 which doesn't contain this bug fix.

jonaski commented 2 years ago

I know. It took homebrew over a month to upgrade to qt 6.2.3 so it just missed the latest strawberry release. But I have been working on building all dependencies for macOS myself so I don't have to rely on homebrew anymore. There are other problems with using homebrew, such as the gstreamer installation being incomplete, and they refuse to build the missing plugins I need. I have just finished building everything here: https://github.com/strawberrymusicplayer/strawberry-macos-dependencies I'm just working on switching the strawberry github actions use the new dependencies instead of homebrew. I will let you know once that's finished and you can test.

jonaski commented 2 years ago

Ready now on https://builds.strawberrymusicplayer.org/macos/ 1.0.2.22 and newer are the new ones.

llucps commented 2 years ago

Thank you very much @jonaski , this is great news!

One thing though, since I have an Mac mini M1, I've been compiling Strawberry using MacPorts myself. I gueds I'll have to wait until the currently qt 6.2.1 gets update, so the fix fixed gets applied. correct?

I'll try the new build with my old Macbook Pro 2013 which is retina display, and see if makes a difference.

Thanks!

llucps commented 2 years ago

For some reason the new build 1.0.2.22 doesn't detect gstreamer and usually hangs the application, getting the famous spinning beach ball.

Running from the terminal I get:

17:42:32.522 ERROR *GstEnginePipeline:230 GStreamer could not create the element "playbin" with name "pipeline-1-pipeline"

Screen Shot 2022-03-10 at 17 43 43
jonaski commented 2 years ago

Yes, looks like it still needs some work, but it's pretty close.

jonaski commented 2 years ago

gst-plugin-scanner was missing, fixed now. 1.0.2.24 and later should hopefully work.

llucps commented 2 years ago

Not there yet :) I'm getting this:

08:26:36.068 ERROR GstEngine:909 "Stream discovery for https://xxxxx.xxxxxxxxx.com/rest/stream.view?id=3d8ea5dca80540c4f868153c4e397480&c=Strawberry&v=1.11.0&f=json&u=xxxx&s=xxxxxxxxxxxxxx failed: Some plugins are missing for full discovery" 08:26:36.193 ERROR GstEnginePipeline:1144 ErrorMessageReceived ID: 1 Domain: 1827 Code: 12 Error: "No URI handler implemented for \"https\"." 08:26:36.193 ERROR GstEnginePipeline:1145 ErrorMessageReceived ID: 1 Domain: 1827 Code: 12 Debug: "../gst-plugins-base-1.20.0/gst/playback/gsturidecodebin.c(1466): gen_source_element (): /GstPlayBin:pipeline-1-pipeline/GstURIDecodeBin:uridecodebin0" 08:26:36.193 ERROR GstEngine:545 GStreamer error: 1827 12 "No URI handler implemented for \"https\"."

Screen Shot 2022-03-11 at 08 26 15

`

jonaski commented 2 years ago

Looks like gstreamer in 1.20 they changed libsoup to be loaded at runtime instead of dynamically linked, I can't get libsoup to work with that on macos (works fine on windows). I have downgraded gstreamer to 1.18.6, so now it works again. 1.0.2.27 or later should work.

jonaski commented 2 years ago

I sent an e-mail to the gstreamer mailinglist: https://lists.freedesktop.org/archives/gstreamer-devel/2022-March/079714.html regarding the issues with libsoup (http) with gstreamer 1.20.

If someone can confirm that the scrolling issues are fixed we can close this issue.