Fix scrolling issue with coverart columns visible #13719
Conversation
The ID is the unique ID which is always returned. When finding a track by canonical location, without an ID, verify that the location stored in the database which can contain symlinks is also matching. If not a different track referencing that same file is already cached and we need to return null to not introduce a data race. This avoids the issue that wrong track can be returned when using drag and drop.
…() by a proper handling of GlobalTrackCacheLookupResult.
…eady a more significant warning about the dupicate situation.
Co-authored-by: ronso0 <[email protected]>
… shall not be stored in library and track file.
…adata. This fixes a priority inversion when scrolling the library mixxxdj#11128
|
This seems to fix a long-standing performance issue, does one of @mixxxdj/developers have time to take a look? |
|
what is the status of the beta / release? While this is annoying, it has been around forever without causing harm so it seems better to fixing in the main branch (just in case there is an unforeseen regression) |
|
I did a short test on Windows. I had not the described issue before, therefore I can not say if this PR fixes the bug. I've no delay, with and without this PR. But what I noticed, is that the caching does not work properly, if I scroll up and down in my track list, it starts reloading coverart of tracks that should be in the cache. It can't be the cache size, which is set to 20MByte and I've only a few hundereds tracks with tiny coverart pixmaps where I estimate the overall size (assuming uncompressd pixmaps) with 1.4MByte. |
|
@daschuer can you repoint to main? |
|
main is for new features. This is a bug fix, I am affected on. I have my tracks on an external spinning HDD. So the 2.4 target is correct. Please note that this is not on the 2.4.2 milestone, because it shall not block the release in case of pending issues. @JoergAtGithub, how do you test exctly. I can confirm, that covers are only loaded and cached after stopping for the inhibit timeout. Caching requests for not visible rows are deleted. This means that you can easily scroll to the library several times and find uncashed covers. This behaviour did not change by this PR. Or is it an other regression by this PR. I have double checked that by searching for covers that are loaded twice via mixxx.log and addition debug messages. No issue found. |
I scrolled up and down multiple times, and are pretty sure that the same covers were shown with the preview-color before the image appeared. |
|
That is the desired behavior, not changed in this PR. Columns are only cache if they are shown with stopped scrolling. The caching request is discarded if they are no longer visible. This happens if you continue scroppling during the inhibit delay. Can you confirm with plain 2.4 or any other branch you have at hand? |
|
I can confirm that this behaviour is already in plain 2.4, therefore it's not a regression of this PR. |
This finally fixing a long standing and puzzling bug that momentary locks up Mixxx during scrolling the library with cover art enabled. #11128
The issue happens due to a priory inversion on the track cache mutex. During cover art reading the cache is locked to protect this incomplete track, which prevents the main thread checking for pending modifications on cached tracks. Scrolling continues when all covers are read even it is done in worker threads.
The solution implemented here is to allow to bypass the lock in case the current track populated, yet still incomplete track is not of interest. It is implemented by limiting that actual look for the track cache only for aching actions and use a wait condition in case a thread is waiting for completing an incomplete track.
Now scrolling is smooth all the time and together with removing unnecessary painting #13715 also quite fast.
This PR is on top of #13708 which should be merged first.