Fix slideshow crashing in invalidate()
ClosedPublic

Authored by davidre on Thu, Oct 17, 9:15 AM.

Details

Summary

QSortFilterProxyModel uses std::stable_sort internally which requires that the
comparison function generates a strict weak ordering. Returning true or false
randomly didn't fullfil this requirement causing a crash in some calls to invalidate.
To keep the random order consistent a vector of row indices is used which records
the current random order.

BUG: 413018
FIXED-IN: 5.17.1

Test Plan

To reproduce the bug use a slideshow in random order with few pictures and a small
time intervall.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidre created this revision.Thu, Oct 17, 9:15 AM
Restricted Application added a project: Plasma. · View Herald TranscriptThu, Oct 17, 9:15 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
davidre requested review of this revision.Thu, Oct 17, 9:15 AM
davidre updated this revision to Diff 68115.Thu, Oct 17, 9:23 AM

Need rebuild here, rowCount could have changed

davidre updated this revision to Diff 68117.Thu, Oct 17, 9:41 AM

Simplify the logic. In configmode we don't need the random order. We just
use the order of the underlying model.

broulik added inline comments.
wallpapers/image/slidefiltermodel.cpp
51

Perhaps only invalidate if order is actually set to random?

I am actually not sure we need to invalidate at all when inserting items, unless you want to shuffle every time that happens? Perhaps all we need to do is shrink/grow the random vector when new items are added?

136

Can sourceModel() be null here?

davidre added inline comments.Thu, Oct 17, 5:14 PM
wallpapers/image/slidefiltermodel.cpp
51

Good point. No need to invalidate in the other cases.

Maybe. It depends where images are added in the model? Maybe we would end up showing pictures again because the indexes have shifted? Or the new images are always shown at the end because they get appended? Either way I think that*'s not to bad if the order is set to Random. The result could also be the result of a random permutation.

Other than Kai's comments, ship it.

davidre updated this revision to Diff 68302.Sat, Oct 19, 3:07 PM
davidre marked 3 inline comments as done.

Like this?
I also changed the condition in image.cpp otherwise the order wouldn't have been random on the first run with these changes.

davidre updated this revision to Diff 68304.Sat, Oct 19, 3:27 PM

Build the new order only after setting the source model.
In invalidate just shuffling is enough.

broulik accepted this revision.Mon, Oct 21, 8:46 AM
This revision is now accepted and ready to land.Mon, Oct 21, 8:46 AM
This revision was automatically updated to reflect the committed changes.