From d5023301f8fc02af255ba2336987ed81768db16b Mon Sep 17 00:00:00 2001 From: Jared Van Bortel Date: Tue, 8 Oct 2024 11:06:35 -0400 Subject: [PATCH 1/5] chat: do not connect layoutChanged to countChanged layoutChanged is only used when the *order* changes. When rows are inserted or removed, one of the other three signals is fired. Signed-off-by: Jared Van Bortel --- gpt4all-chat/src/localdocsmodel.cpp | 2 -- gpt4all-chat/src/modellist.cpp | 2 -- 2 files changed, 4 deletions(-) diff --git a/gpt4all-chat/src/localdocsmodel.cpp b/gpt4all-chat/src/localdocsmodel.cpp index fba4c4ff32dc..a10773625e0c 100644 --- a/gpt4all-chat/src/localdocsmodel.cpp +++ b/gpt4all-chat/src/localdocsmodel.cpp @@ -20,7 +20,6 @@ LocalDocsCollectionsModel::LocalDocsCollectionsModel(QObject *parent) connect(this, &LocalDocsCollectionsModel::rowsInserted, this, &LocalDocsCollectionsModel::countChanged); connect(this, &LocalDocsCollectionsModel::rowsRemoved, this, &LocalDocsCollectionsModel::countChanged); connect(this, &LocalDocsCollectionsModel::modelReset, this, &LocalDocsCollectionsModel::countChanged); - connect(this, &LocalDocsCollectionsModel::layoutChanged, this, &LocalDocsCollectionsModel::countChanged); } bool LocalDocsCollectionsModel::filterAcceptsRow(int sourceRow, @@ -67,7 +66,6 @@ LocalDocsModel::LocalDocsModel(QObject *parent) connect(this, &LocalDocsModel::rowsInserted, this, &LocalDocsModel::countChanged); connect(this, &LocalDocsModel::rowsRemoved, this, &LocalDocsModel::countChanged); connect(this, &LocalDocsModel::modelReset, this, &LocalDocsModel::countChanged); - connect(this, &LocalDocsModel::layoutChanged, this, &LocalDocsModel::countChanged); } int LocalDocsModel::rowCount(const QModelIndex &parent) const diff --git a/gpt4all-chat/src/modellist.cpp b/gpt4all-chat/src/modellist.cpp index d53c8fbfa316..88f5d5e636df 100644 --- a/gpt4all-chat/src/modellist.cpp +++ b/gpt4all-chat/src/modellist.cpp @@ -398,7 +398,6 @@ InstalledModels::InstalledModels(QObject *parent, bool selectable) connect(this, &InstalledModels::rowsInserted, this, &InstalledModels::countChanged); connect(this, &InstalledModels::rowsRemoved, this, &InstalledModels::countChanged); connect(this, &InstalledModels::modelReset, this, &InstalledModels::countChanged); - connect(this, &InstalledModels::layoutChanged, this, &InstalledModels::countChanged); } bool InstalledModels::filterAcceptsRow(int sourceRow, @@ -423,7 +422,6 @@ DownloadableModels::DownloadableModels(QObject *parent) connect(this, &DownloadableModels::rowsInserted, this, &DownloadableModels::countChanged); connect(this, &DownloadableModels::rowsRemoved, this, &DownloadableModels::countChanged); connect(this, &DownloadableModels::modelReset, this, &DownloadableModels::countChanged); - connect(this, &DownloadableModels::layoutChanged, this, &DownloadableModels::countChanged); } bool DownloadableModels::filterAcceptsRow(int sourceRow, From 286f164b31c2512759eb696bf1f4b2a231bb7c5f Mon Sep 17 00:00:00 2001 From: Jared Van Bortel Date: Tue, 8 Oct 2024 11:08:57 -0400 Subject: [PATCH 2/5] modellist: always emit layoutChanged when sorting, never otherwise Signed-off-by: Jared Van Bortel --- gpt4all-chat/src/modellist.cpp | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/gpt4all-chat/src/modellist.cpp b/gpt4all-chat/src/modellist.cpp index 88f5d5e636df..a8c586b85718 100644 --- a/gpt4all-chat/src/modellist.cpp +++ b/gpt4all-chat/src/modellist.cpp @@ -819,7 +819,11 @@ QVariant ModelList::data(const QModelIndex &index, int role) const void ModelList::updateData(const QString &id, const QVector> &data) { + // We only sort when one of the fields used by the sorting algorithm actually changes that + // is implicated or used by the sorting algorithm + bool shouldSort = false; int index; + { QMutexLocker locker(&m_mutex); if (!m_modelMap.contains(id)) { @@ -834,10 +838,6 @@ void ModelList::updateData(const QString &id, const QVector return; } - // We only sort when one of the fields used by the sorting algorithm actually changes that - // is implicated or used by the sorting algorithm - bool shouldSort = false; - for (const auto &d : data) { const int role = d.first; const QVariant value = d.second; @@ -998,15 +998,11 @@ void ModelList::updateData(const QString &id, const QVector info->isEmbeddingModel = LLModel::Implementation::isEmbeddingModel(modelPath.toStdString()); info->checkedEmbeddingModel = true; } - - if (shouldSort) { - auto s = m_discoverSort; - auto d = m_discoverSortDirection; - std::stable_sort(m_models.begin(), m_models.end(), [s, d](const ModelInfo* lhs, const ModelInfo* rhs) { - return ModelList::lessThan(lhs, rhs, s, d); - }); - } } + + if (shouldSort) + resortModel(); + emit dataChanged(createIndex(index, 0), createIndex(index, 0)); // FIXME(jared): for some reason these don't update correctly when the source model changes, so we explicitly invalidate them @@ -1120,7 +1116,6 @@ void ModelList::removeClone(const ModelInfo &model) return; removeInternal(model); - emit layoutChanged(); } void ModelList::removeInstalled(const ModelInfo &model) @@ -1129,7 +1124,6 @@ void ModelList::removeInstalled(const ModelInfo &model) Q_ASSERT(!model.isClone()); Q_ASSERT(model.isDiscovered() || model.isCompatibleApi || model.description() == "" /*indicates sideloaded*/); removeInternal(model); - emit layoutChanged(); } void ModelList::removeInternal(const ModelInfo &model) @@ -1926,7 +1920,6 @@ void ModelList::clearDiscoveredModels() } for (ModelInfo &info : infos) removeInternal(info); - emit layoutChanged(); } float ModelList::discoverProgress() const @@ -2174,7 +2167,6 @@ void ModelList::handleDiscoveryItemFinished() emit discoverProgressChanged(); if (discoverProgress() >= 1.0) { - emit layoutChanged(); m_discoverInProgress = false; emit discoverInProgressChanged();; } From e224867400921d30c55fc8f05dbede6e28141f17 Mon Sep 17 00:00:00 2001 From: Jared Van Bortel Date: Tue, 8 Oct 2024 11:14:15 -0400 Subject: [PATCH 3/5] modellist: emit dataChanged on the correct index dataChanged will not work correctly if we emit it on an old index after sorting. Signed-off-by: Jared Van Bortel --- gpt4all-chat/src/modellist.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gpt4all-chat/src/modellist.cpp b/gpt4all-chat/src/modellist.cpp index a8c586b85718..fcdccb958903 100644 --- a/gpt4all-chat/src/modellist.cpp +++ b/gpt4all-chat/src/modellist.cpp @@ -1000,11 +1000,11 @@ void ModelList::updateData(const QString &id, const QVector } } + emit dataChanged(createIndex(index, 0), createIndex(index, 0)); + if (shouldSort) resortModel(); - emit dataChanged(createIndex(index, 0), createIndex(index, 0)); - // FIXME(jared): for some reason these don't update correctly when the source model changes, so we explicitly invalidate them m_selectableModels->invalidate(); m_installedModels->invalidate(); From e528d46fd6f8a025a08236d6d6769fc995c07658 Mon Sep 17 00:00:00 2001 From: Jared Van Bortel Date: Tue, 8 Oct 2024 11:16:24 -0400 Subject: [PATCH 4/5] modellist: remove invalidate calls now that we have fixed the signals These were causing the model view scroll position to jump back to the start whenever a change happened, which was not intended. Hopefully, they are no longer needed after the previous changes. Signed-off-by: Jared Van Bortel --- gpt4all-chat/src/modellist.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/gpt4all-chat/src/modellist.cpp b/gpt4all-chat/src/modellist.cpp index fcdccb958903..bb987c435c21 100644 --- a/gpt4all-chat/src/modellist.cpp +++ b/gpt4all-chat/src/modellist.cpp @@ -1005,11 +1005,6 @@ void ModelList::updateData(const QString &id, const QVector if (shouldSort) resortModel(); - // FIXME(jared): for some reason these don't update correctly when the source model changes, so we explicitly invalidate them - m_selectableModels->invalidate(); - m_installedModels->invalidate(); - m_downloadableModels->invalidate(); - emit selectableModelListChanged(); } From 0efdcc09ad806f361d67ec03c143929e1e446d36 Mon Sep 17 00:00:00 2001 From: Jared Van Bortel Date: Tue, 8 Oct 2024 11:24:21 -0400 Subject: [PATCH 5/5] changelog: add this PR Signed-off-by: Jared Van Bortel --- gpt4all-chat/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/gpt4all-chat/CHANGELOG.md b/gpt4all-chat/CHANGELOG.md index f8be3f5adb61..284e0b944bcf 100644 --- a/gpt4all-chat/CHANGELOG.md +++ b/gpt4all-chat/CHANGELOG.md @@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - Fix "regenerate" always forgetting the most recent message ([#3011](https://github.com/nomic-ai/gpt4all/pull/3011)) - Fix loaded chats forgetting context when there is a system prompt ([#3015](https://github.com/nomic-ai/gpt4all/pull/3015)) - Make it possible to downgrade and keep some chats, and avoid crash for some model types ([#3030](https://github.com/nomic-ai/gpt4all/pull/3030)) +- Fix scroll positition being reset in model view, and attempt a better fix for the clone issue ([#3042](https://github.com/nomic-ai/gpt4all/pull/3042)) ## [3.3.1] - 2024-09-27 ([v3.3.y](https://github.com/nomic-ai/gpt4all/tree/v3.3.y))