Fix slideshow crashing in invalidate()
ClosedPublic

Authored by davidre on Oct 17 2019, 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
Branch
Plasma/5.17
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 17905
Build 17923: arc lint + arc unit
davidre created this revision.Oct 17 2019, 9:15 AM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 17 2019, 9:15 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
davidre requested review of this revision.Oct 17 2019, 9:15 AM
davidre updated this revision to Diff 68115.Oct 17 2019, 9:23 AM

Need rebuild here, rowCount could have changed

davidre updated this revision to Diff 68117.Oct 17 2019, 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.Oct 17 2019, 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.Oct 19 2019, 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.Oct 19 2019, 3:27 PM

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

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