Commit a1cf305f authored by David Redondo's avatar David Redondo 🏎

Fix slideshow crashing in invalidate()

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.

Reviewers: #plasma, broulik

Reviewed By: #plasma, broulik

Subscribers: davidedmundson, broulik, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D24723
parent 5ad4d23b
......@@ -769,7 +769,7 @@ void Image::nextSlide()
m_currentSlide += 1;
}
//We are starting again - avoid having the same random order when we restart the slideshow
if (m_slideshowMode == Random && previousSlide == m_slideFilterModel->rowCount() - 1) {
if (m_slideshowMode == Random && m_currentSlide == 0) {
m_slideFilterModel->invalidate();
}
QUrl next = m_slideFilterModel->index(m_currentSlide, 0).data(BackgroundListModel::PathRole).toUrl();
......
......@@ -24,6 +24,8 @@
#include <QRandomGenerator>
#include <QFileInfo>
#include <algorithm>
SlideFilterModel::SlideFilterModel(QObject* parent)
: QSortFilterProxyModel{parent}
, m_SortingMode{Image::Random}
......@@ -39,11 +41,43 @@ bool SlideFilterModel::filterAcceptsRow(int source_row, const QModelIndex& sourc
return m_usedInConfig || index.data(BackgroundListModel::ToggleRole).toBool();
}
void SlideFilterModel::setSourceModel(QAbstractItemModel *sourceModel)
{
if (this->sourceModel()) {
disconnect(this->sourceModel(), nullptr, this, nullptr);
}
QSortFilterProxyModel::setSourceModel(sourceModel);
if (m_SortingMode == Image::Random && !m_usedInConfig) {
buildRandomOrder();
}
if(sourceModel) {
connect(sourceModel, &QAbstractItemModel::rowsInserted, this, [this] {
if (m_SortingMode != Image::Random || m_usedInConfig) {
return;
}
const int old_count = m_randomOrder.size();
m_randomOrder.resize(this->sourceModel()->rowCount());
std::iota(m_randomOrder.begin() + old_count, m_randomOrder.end(), old_count);
});
connect(sourceModel, &QAbstractItemModel::rowsRemoved, this, [this] {
if (m_SortingMode != Image::Random || m_usedInConfig) {
return;
}
m_randomOrder.erase(std::remove_if(m_randomOrder.begin(), m_randomOrder.end(), [this] (const int v) {
return v >= this->sourceModel()->rowCount();
}), m_randomOrder.end());
});
}
}
bool SlideFilterModel::lessThan(const QModelIndex& source_left, const QModelIndex& source_right) const
{
switch (m_SortingMode) {
case Image::Random:
return (*QRandomGenerator::system())() % 2 == 0;
if (m_usedInConfig) {
return source_left.row() < source_right.row();
}
return m_randomOrder.indexOf(source_left.row()) < m_randomOrder.indexOf(source_right.row());
case Image::Alphabetical:
return QSortFilterProxyModel::lessThan(source_left, source_right);
case Image::AlphabeticalReversed:
......@@ -66,10 +100,18 @@ bool SlideFilterModel::lessThan(const QModelIndex& source_left, const QModelInde
void SlideFilterModel::setSortingMode(Image::SlideshowMode mode)
{
m_SortingMode = mode;
if (!(m_usedInConfig && mode == Image::Random)) {
QSortFilterProxyModel::invalidate();
if (m_SortingMode == Image::Random && !m_usedInConfig) {
buildRandomOrder();
}
QSortFilterProxyModel::invalidate();
}
void SlideFilterModel::invalidate()
{
if (m_SortingMode == Image::Random && !m_usedInConfig) {
std::random_shuffle(m_randomOrder.begin(), m_randomOrder.end());
}
QSortFilterProxyModel::invalidate();
}
void SlideFilterModel::invalidateFilter()
......@@ -88,3 +130,12 @@ void SlideFilterModel::openContainingFolder(int rowIndex)
auto sourceIndex = mapToSource(index(rowIndex, 0));
static_cast<SlideModel*>(sourceModel())->openContainingFolder(sourceIndex.row());
}
void SlideFilterModel::buildRandomOrder()
{
if (sourceModel()) {
m_randomOrder.resize(sourceModel()->rowCount());
std::iota(m_randomOrder.begin(), m_randomOrder.end(), 0);
std::random_shuffle(m_randomOrder.begin(), m_randomOrder.end());
}
}
......@@ -22,7 +22,7 @@
#include <image.h>
#include <QSortFilterProxyModel>
#include <QVector>
class SlideFilterModel : public QSortFilterProxyModel {
......@@ -34,7 +34,9 @@ public:
SlideFilterModel(QObject* parent);
bool lessThan(const QModelIndex& source_left, const QModelIndex& source_right) const override;
bool filterAcceptsRow(int source_row, const QModelIndex& source_parent) const override;
void setSourceModel(QAbstractItemModel *sourceModel) override;
void setSortingMode(Image::SlideshowMode mode);
void invalidate();
void invalidateFilter();
Q_INVOKABLE int indexOf(const QString& path);
......@@ -44,6 +46,9 @@ Q_SIGNALS:
void usedInConfigChanged();
private:
void buildRandomOrder();
QVector<int> m_randomOrder;
Image::SlideshowMode m_SortingMode;
bool m_usedInConfig;
};
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment