diff --git a/containments/desktop/plugins/folder/autotests/positionertest.h b/containments/desktop/plugins/folder/autotests/positionertest.h --- a/containments/desktop/plugins/folder/autotests/positionertest.h +++ b/containments/desktop/plugins/folder/autotests/positionertest.h @@ -52,6 +52,7 @@ void tst_defaultValues(); void tst_changeEnabledStatus(); void tst_changePerStripe(); + void tst_proxyMapping(); private: void checkPositions(int perStripe); diff --git a/containments/desktop/plugins/folder/autotests/positionertest.cpp b/containments/desktop/plugins/folder/autotests/positionertest.cpp --- a/containments/desktop/plugins/folder/autotests/positionertest.cpp +++ b/containments/desktop/plugins/folder/autotests/positionertest.cpp @@ -29,6 +29,7 @@ #include "foldermodel.h" #include "positioner.h" +#include "screenmapper.h" QTEST_MAIN(PositionerTest) @@ -58,6 +59,9 @@ void PositionerTest::init() { m_folderModel = new FolderModel(this); + m_folderModel->setScreen(0); + m_folderModel->setScreenMapper(ScreenMapper::instance()); + m_folderModel->setUsedByContainment(true); m_positioner = new Positioner(this); m_positioner->setEnabled(true); m_positioner->setFolderModel(m_folderModel); @@ -209,6 +213,98 @@ QCOMPARE(s.count(), 2); } +void PositionerTest::tst_proxyMapping() +{ + auto *screenMapper = ScreenMapper::instance(); + FolderModel secondFolderModel; + secondFolderModel.setUrl(m_folderDir->path() + QDir::separator() + desktop ); + secondFolderModel.setUsedByContainment(true); + secondFolderModel.setScreenMapper(screenMapper); + secondFolderModel.setScreen(1); + Positioner secondPositioner; + secondPositioner.setEnabled(true); + secondPositioner.setFolderModel(&secondFolderModel); + secondPositioner.setPerStripe(3); + + QSignalSpy s2(&secondFolderModel, &FolderModel::listingCompleted); + QVERIFY(s2.wait(1000)); + + QHash expectedSource2ProxyScreen0; + QHash expectedProxy2SourceScreen0; + QHash expectedProxy2SourceScreen1; + QHash expectedSource2ProxyScreen1; + + for (int i = 0; i < m_folderModel->rowCount(); i++) { + expectedSource2ProxyScreen0[i] = i; + expectedProxy2SourceScreen0[i] = i; + } + + // swap items 1 and 2 in the positioner + m_positioner->move({1, 2, 2, 1}); + expectedSource2ProxyScreen0[1] = 2; + expectedSource2ProxyScreen0[2] = 1; + expectedProxy2SourceScreen0[1] = 2; + expectedProxy2SourceScreen0[2] = 1; + + auto savedSource2ProxyScreen0 = expectedSource2ProxyScreen0; + auto savedProxy2SourceScreen0 = expectedProxy2SourceScreen0; + + auto verifyMapping = [](const QHash &actual, const QHash &expected) { + + auto ensureUnique = [](const QHash mapping) { + auto values = mapping.values(); + qSort(values); + auto uniqueValues = values.toSet().toList(); + qSort(uniqueValues); + QVERIFY(uniqueValues == values); + }; + + ensureUnique(actual); + QCOMPARE(actual, expected); + }; + + verifyMapping(m_positioner->proxyToSourceMapping(), expectedProxy2SourceScreen0); + verifyMapping(m_positioner->sourceToProxyMapping(), expectedSource2ProxyScreen0); + verifyMapping(secondPositioner.proxyToSourceMapping(), expectedProxy2SourceScreen1); + verifyMapping(secondPositioner.sourceToProxyMapping(), expectedSource2ProxyScreen1); + + const auto movedItem = m_folderModel->index(1, 0).data(FolderModel::UrlRole).toString(); + + // move the item 1 from source (now in position 2) to the second screen + screenMapper->addMapping(movedItem, 1); + + expectedProxy2SourceScreen1[0] = 0; + expectedSource2ProxyScreen1[0] = 0; + expectedSource2ProxyScreen0.clear(); + expectedProxy2SourceScreen0.clear(); + for (int i = 0; i < m_folderModel->rowCount(); i++) { + // as item 1 disappeared, the mapping of all items after that are shifted + auto proxyIndex = (i <= 1) ? i : i + 1; + expectedProxy2SourceScreen0[proxyIndex] = i; + expectedSource2ProxyScreen0[i] = proxyIndex; + } + + verifyMapping(m_positioner->proxyToSourceMapping(), expectedProxy2SourceScreen0); + verifyMapping(m_positioner->sourceToProxyMapping(), expectedSource2ProxyScreen0); + verifyMapping(secondPositioner.proxyToSourceMapping(), expectedProxy2SourceScreen1); + verifyMapping(secondPositioner.sourceToProxyMapping(), expectedSource2ProxyScreen1); + + // move back the same item to the first screen + screenMapper->addMapping(movedItem, 0); + + // nothing on the second screen + expectedSource2ProxyScreen1.clear(); + expectedProxy2SourceScreen1.clear(); + // first screen should look like in the beginning + expectedSource2ProxyScreen0 = savedSource2ProxyScreen0; + expectedProxy2SourceScreen0 = savedProxy2SourceScreen0; + + verifyMapping(m_positioner->proxyToSourceMapping(), expectedProxy2SourceScreen0); + verifyMapping(m_positioner->sourceToProxyMapping(), expectedSource2ProxyScreen0); + verifyMapping(secondPositioner.proxyToSourceMapping(), expectedProxy2SourceScreen1); + verifyMapping(secondPositioner.sourceToProxyMapping(), expectedSource2ProxyScreen1); +} + void PositionerTest::checkPositions(int perStripe) { QSignalSpy s(m_positioner, &Positioner::positionsChanged); diff --git a/containments/desktop/plugins/folder/positioner.h b/containments/desktop/plugins/folder/positioner.h --- a/containments/desktop/plugins/folder/positioner.h +++ b/containments/desktop/plugins/folder/positioner.h @@ -76,6 +76,15 @@ int rowCount(const QModelIndex &parent = QModelIndex()) const override; int columnCount(const QModelIndex &parent = QModelIndex()) const override; +#ifdef BUILD_TESTING + QHash proxyToSourceMapping() const { + return m_proxyToSource; + } + QHash sourceToProxyMapping() const { + return m_sourceToProxy; + } +#endif + Q_SIGNALS: void enabledChanged() const; void folderModelChanged() const; @@ -128,6 +137,7 @@ QHash m_proxyToSource; QHash m_sourceToProxy; + bool m_beginInsertRowsCalled = false; // used to sync the amount of begin/endInsertRows calls }; #endif diff --git a/containments/desktop/plugins/folder/positioner.cpp b/containments/desktop/plugins/folder/positioner.cpp --- a/containments/desktop/plugins/folder/positioner.cpp +++ b/containments/desktop/plugins/folder/positioner.cpp @@ -425,6 +425,10 @@ const int newCount = rowCount(); if (newCount > oldCount) { + if (m_beginInsertRowsCalled) { + endInsertRows(); + m_beginInsertRowsCalled = false; + } beginInsertRows(QModelIndex(), oldCount, newCount - 1); endInsertRows(); } @@ -510,14 +514,28 @@ if (m_enabled) { if (m_proxyToSource.isEmpty()) { if (!m_pendingPositions) { - emit beginInsertRows(parent, start, end); + beginInsertRows(parent, start, end); + m_beginInsertRowsCalled = true; initMaps(end + 1); } return; } + // When new rows are inserted, they might go in the beginning or in the middle. + // In this case we must update first the existing proxy->source and source->proxy + // mapping, otherwise the proxy items will point to the wrong source item. + int count = end - start + 1; + m_sourceToProxy.clear(); + for (auto it = m_proxyToSource.begin(); it != m_proxyToSource.end(); ++it) { + int sourceIdx = *it; + if (sourceIdx >= start) { + *it += count; + } + m_sourceToProxy[*it] = it.key(); + } + int free = -1; int rest = -1; @@ -538,6 +556,7 @@ int remainder = (end - rest); beginInsertRows(parent, firstNew, firstNew + remainder); + m_beginInsertRowsCalled = true; for (int i = 0; i <= remainder; ++i) { updateMaps(firstNew + i, rest + i); @@ -547,6 +566,8 @@ } } else { emit beginInsertRows(parent, start, end); + beginInsertRows(parent, start, end); + m_beginInsertRowsCalled = true; } } @@ -617,7 +638,10 @@ if (!m_ignoreNextTransaction) { if (!m_pendingPositions) { - emit endInsertRows(); + if (m_beginInsertRowsCalled) { + endInsertRows(); + m_beginInsertRowsCalled = false; + } } else { applyPositions(); }