Bug 509711

Summary:	Random wallpaper order: Non-ideal solution for avoiding a duplicate item when slideshow restarts
Product:	[Plasma] plasmashell	Reporter:	bastimeyer123
Component:	Image & Slideshow wallpaper plugins	Assignee:	Plasma Bugs List <plasma-bugs-null>
Status:	RESOLVED INTENTIONAL
Severity:	wishlist	CC:	kde, kdedev, nate, notmart, qydwhotmail
Priority:	NOR
Version First Reported In:	6.4.90
Target Milestone:	1.0
Platform:	Other
OS:	Linux
Latest Commit:		Version Fixed In:	6.5.0
Sentry Crash Report:

Description bastimeyer123 2025-09-20 13:03:20 UTC

SUMMARY
In the image slideshow, when a random order is selected and all items of the current list were consumed and the next set of items is queued, the solution for avoiding a duplicate item selection in the transition is not ideal and leads to skipping one of the items.

The `ImageBackend::nextSlide()` function currently does this:
https://invent.kde.org/plasma/plasma-workspace/-/blob/v6.4.90/wallpapers/image/plugin/imagebackend.cpp#L411-420
- When the end of the slideshow has been reached (index is 0 again), reshuffle it (`invalidate()`)
- Set the next item to the item with index 0
- If the previous item is the same as the next item, increase the index by 1 and set next that instead

This means that the item with index 0 will not be shown in this case, as it's simply skipped due to it being the same as the previous one, as a valid wallpaper transition needs to be ensured.

A more sensible idea would be swapping the first item with item [2...n] of the reshuffled list, so that no item is skipped and every item will be shown.

From looking at the code (I'm neither familiar with C++/Qt nor with the plugin's implementation), it seems like this was the simplest solution, as there's no interface on the `SlideFilterModel` class for swapping specific items:
https://invent.kde.org/plasma/plasma-workspace/-/blob/v6.4.90/wallpapers/image/plugin/slidefiltermodel.h#L17

Alternatively, the slideshow could be reshuffled again until the first item is not the previous one, but this of course doesn't work in constant time.

STEPS TO REPRODUCE
Have a wallpaper image slideshow in random order with at least 3 items

OBSERVED RESULT
Current slideshow:
A B C
Potential next slideshows:
A B C
A C B
B A C
B C A
A B (C was skipped)
B A (C was skipped)

EXPECTED RESULT
Current slideshow:
A B C
Potential next slideshows:
A B C
A C B
B A C
B C A
A C B (via C A B)
B A C (via C A B)
B C A (via C B A)
A B C (via C B A)

Comment 1 David Redondo 2025-09-22 09:11:17 UTC

Is it a real problem? C will shown in the next round

Comment 2 bastimeyer123 2025-09-22 10:21:00 UTC

(In reply to David Redondo from comment #1)
> Is it a real problem? C will shown in the next round

No, it's a very minor issue, but I think that this still deserves a fix, if it's trivial to fix.

Skipping an item in the queue means that the queue size is reduced by one, so if a user expects their wallpaper slideshow to repeat in strict cycles, even though the order of items is randomized, then this cycle shifts over time, which can be annoying. (Yes, I do actually notice this from time to time, otherwise I wouldn't have posted this)

So, as said, if it's trivial to fix by swapping items in the `SlideFilterModel : QSortFilterProxyModel` instead of increasing the index by one, or maybe just reversing the random order, then there's no reason not to do that. If I was more familiar with the code, I'd submit a MR, but unfortunately, I am not.

Thanks for your consideration.

Comment 3 TraceyC 2025-09-22 17:11:25 UTC

Sending along for consideration

Comment 4 Nate Graham 2025-09-23 16:59:42 UTC

The code for random wallpaper slideshows is already very complex; surprisingly so, even. I think adding additional complexity for something quite minor is unfortunately not worth the risk. Thanks for the idea anyway, though!

Comment 5 bastimeyer123 2025-09-23 19:19:15 UTC

(In reply to Nate Graham from comment #4)
> The code for random wallpaper slideshows is already very complex;
> surprisingly so, even. I think adding additional complexity for something
> quite minor is unfortunately not worth the risk. Thanks for the idea anyway,
> though!

Would you still accept a merge request which adds a very simple `SlideFilterModel::reverse()` for reversing the `QList<int> m_randomOrder` instead of the `m_currentSlide += 1` increment? I don't think this is very complex at all.

$ git diff --shortstat wallpapers/
 3 files changed, 9 insertions(+), 1 deletion(-)

I tested this with added debug log messages and it's working fine. If you don't want a MR for this, then I can keep this patch in my own plasma-workspace builds, but I don't see a reason not to upstream this.

Comment 6 TraceyC 2025-09-24 15:51:37 UTC

(In reply to bastimeyer123 from comment #5)
> Would you still accept a merge request which adds a very simple
> `SlideFilterModel::reverse()` for reversing the `QList<int> m_randomOrder`
> instead of the `m_currentSlide += 1` increment? I don't think this is very
> complex at all.

The best thing to do would be to create the MR and let the development community review it. They don't read individual bug reports for patch suggestions. The worst that can happen is it isn't accepted. Since it's a small change, they might be open to it.

Comment 7 Nate Graham 2025-09-24 18:28:22 UTC

If you have a simple patch that you think doesn't present risk, I'd agree that you can go ahead and submit it, and we'll see where it goes.

Make sure to put "BUG: 509711" on its own line somewhere in the commit message, so this bug report changes from RESOLVED INTENTIONAL to RESOLVED FIXED if and when the patch gets merged.