From 8233a8fbdc3416f3eac9c6c0cf8d4a4c0bfc8b7d Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Thu, 30 Nov 2017 17:38:40 +0100 Subject: [PATCH] [bugfix] Bookmarks sorting with a proxy model Fixes #17005 spatial bookmarks can't be sorted I believe that this has been broken since https://github.com/qgis/QGIS/pull/2661 --- src/app/qgsbookmarks.cpp | 75 +++++++++++++++++++++++++++------------- src/app/qgsbookmarks.h | 31 ++++++++++++++--- 2 files changed, 77 insertions(+), 29 deletions(-) diff --git a/src/app/qgsbookmarks.cpp b/src/app/qgsbookmarks.cpp index e67fea33da9..d337d9c6f6f 100644 --- a/src/app/qgsbookmarks.cpp +++ b/src/app/qgsbookmarks.cpp @@ -69,7 +69,7 @@ QgsBookmarks::QgsBookmarks( QWidget *parent ) // open the database QSqlDatabase db = QSqlDatabase::addDatabase( QStringLiteral( "QSQLITE" ), QStringLiteral( "bookmarks" ) ); db.setDatabaseName( QgsApplication::qgisUserDatabaseFilePath() ); - if ( !db.open() ) + if ( ! db.open() ) { QMessageBox::warning( this, tr( "Error" ), tr( "Unable to open bookmarks database.\nDatabase: %1\nDriver: %2\nDatabase: %3" ) @@ -81,7 +81,8 @@ QgsBookmarks::QgsBookmarks( QWidget *parent ) return; } - mQgisModel = new QSqlTableModel( this, db ); + // Do not set parent or the destructor will try to write on the log viewer (and crash QGIS) + mQgisModel = new QSqlTableModel( nullptr, db ); mQgisModel->setTable( QStringLiteral( "tbl_bookmarks" ) ); mQgisModel->setSort( 0, Qt::AscendingOrder ); mQgisModel->select(); @@ -97,10 +98,15 @@ QgsBookmarks::QgsBookmarks( QWidget *parent ) mQgisModel->setHeaderData( 6, Qt::Horizontal, tr( "yMax" ) ); mQgisModel->setHeaderData( 7, Qt::Horizontal, tr( "SRID" ) ); - mProjectModel = new QgsProjectBookmarksTableModel(); - mModel.reset( new QgsMergedBookmarksTableModel( *mQgisModel, *mProjectModel, lstBookmarks ) ); + mProjectModel = new QgsProjectBookmarksTableModel( this ); + mModel = new QgsMergedBookmarksTableModel( *mQgisModel, *mProjectModel, lstBookmarks, this ); - lstBookmarks->setModel( mModel.get() ); + mProxyModel = new QgsBookmarksProxyModel( this ); + mProxyModel->setSourceModel( mModel ); + + lstBookmarks->setModel( mProxyModel ); + + connect( mModel, &QgsMergedBookmarksTableModel::layoutChanged, mProxyModel, &QgsBookmarksProxyModel::_resetModel ); QgsSettings settings; lstBookmarks->header()->restoreState( settings.value( QStringLiteral( "Windows/Bookmarks/headerstate" ) ).toByteArray() ); @@ -113,7 +119,6 @@ QgsBookmarks::QgsBookmarks( QWidget *parent ) QgsBookmarks::~QgsBookmarks() { delete mQgisModel; - delete mProjectModel; QSqlDatabase::removeDatabase( QStringLiteral( "bookmarks" ) ); saveWindowLocation(); } @@ -166,9 +171,11 @@ void QgsBookmarks::addClicked() { mQgisModel->setSort( 0, Qt::AscendingOrder ); mQgisModel->select(); - lstBookmarks->scrollTo( mModel->index( mQgisModel->rowCount() - 1, 1 ) ); - lstBookmarks->setCurrentIndex( mModel->index( mQgisModel->rowCount() - 1, 1 ) ); - lstBookmarks->edit( mModel->index( mQgisModel->rowCount() - 1, 1 ) ); + QModelIndex newIdx = mProxyModel->mapFromSource( mModel->index( mQgisModel->rowCount() - 1, 1 ) ); + // Edit new bookmark title + lstBookmarks->scrollTo( newIdx ); + lstBookmarks->setCurrentIndex( newIdx ); + lstBookmarks->edit( newIdx ); } else { @@ -390,17 +397,20 @@ void QgsBookmarks::exportToXml() settings.setValue( QStringLiteral( "Windows/Bookmarks/LastUsedDirectory" ), QFileInfo( fileName ).path() ); } -QgsProjectBookmarksTableModel::QgsProjectBookmarksTableModel() +QgsProjectBookmarksTableModel::QgsProjectBookmarksTableModel( QObject *parent ) + : QAbstractTableModel( parent ) { connect( QgisApp::instance(), &QgisApp::projectRead, this, &QgsProjectBookmarksTableModel::projectRead ); + connect( + QgisApp::instance(), &QgisApp::newProject, + this, &QgsProjectBookmarksTableModel::projectRead ); } int QgsProjectBookmarksTableModel::rowCount( const QModelIndex &parent ) const { Q_UNUSED( parent ); - return QgsProject::instance()->readNumEntry( QStringLiteral( "Bookmarks" ), QStringLiteral( "/count" ) ); } @@ -412,7 +422,6 @@ int QgsProjectBookmarksTableModel::columnCount( const QModelIndex &parent ) cons QVariant QgsProjectBookmarksTableModel::data( const QModelIndex &index, int role ) const { - if ( role != Qt::DisplayRole && role != Qt::EditRole ) { return QVariant(); @@ -474,7 +483,6 @@ bool QgsProjectBookmarksTableModel::setData( const QModelIndex &index, const QVa bool QgsProjectBookmarksTableModel::insertRows( int row, int count, const QModelIndex &parent ) { - Q_UNUSED( row ); Q_UNUSED( parent ); beginInsertRows( parent, row, row + count ); @@ -504,15 +512,18 @@ bool QgsProjectBookmarksTableModel::removeRows( int row, int count, const QModel return true; } +void QgsProjectBookmarksTableModel::projectRead() +{ + emit layoutChanged(); +} -QgsMergedBookmarksTableModel::QgsMergedBookmarksTableModel( QAbstractTableModel &qgisTableModel, QAbstractTableModel &projectTableModel, QTreeView *treeView ) - : mQgisTableModel( qgisTableModel ) + +QgsMergedBookmarksTableModel::QgsMergedBookmarksTableModel( QAbstractTableModel &qgisTableModel, QAbstractTableModel &projectTableModel, QTreeView *treeView, QObject *parent ) + : QAbstractTableModel( parent ) + , mQgisTableModel( qgisTableModel ) , mProjectTableModel( projectTableModel ) , mTreeView( treeView ) { - connect( - QgsProject::instance(), &QgsProject::fileNameChanged, - this, &QgsMergedBookmarksTableModel::projectFileNameChanged ); connect( &mQgisTableModel, &QAbstractTableModel::layoutChanged, @@ -530,6 +541,13 @@ QgsMergedBookmarksTableModel::QgsMergedBookmarksTableModel( QAbstractTableModel &projectTableModel, &QAbstractTableModel::layoutChanged, this, &QgsMergedBookmarksTableModel::allLayoutChanged ); + connect( + &projectTableModel, &QAbstractTableModel::rowsInserted, + this, &QgsMergedBookmarksTableModel::allLayoutChanged ); + + connect( + &projectTableModel, &QAbstractTableModel::rowsRemoved, + this, &QgsMergedBookmarksTableModel::allLayoutChanged ); } @@ -614,8 +632,6 @@ bool QgsMergedBookmarksTableModel::setData( const QModelIndex &index, const QVar Qt::ItemFlags QgsMergedBookmarksTableModel::flags( const QModelIndex &index ) const { - Q_UNUSED( index ); - Qt::ItemFlags flags = Qt::ItemIsSelectable | Qt::ItemIsEnabled; if ( index.column() == mQgisTableModel.columnCount() ) { @@ -685,6 +701,7 @@ bool QgsMergedBookmarksTableModel::projectAvailable() const void QgsMergedBookmarksTableModel::moveBookmark( QAbstractTableModel &modelFrom, QAbstractTableModel &modelTo, int row ) { + emit beginResetModel(); QSqlTableModel *qgisModel = dynamic_cast( &modelTo ); if ( !qgisModel ) { @@ -722,9 +739,19 @@ void QgsMergedBookmarksTableModel::moveBookmark( QAbstractTableModel &modelFrom, qgisModel->select(); modelFrom.removeRows( row, 1 ); } -} - -void QgsMergedBookmarksTableModel::projectFileNameChanged() -{ + emit endResetModel(); emit layoutChanged(); } + + +QgsBookmarksProxyModel::QgsBookmarksProxyModel( QObject *parent ): + QSortFilterProxyModel( parent ) +{ + +} + +QVariant QgsBookmarksProxyModel::headerData( int section, Qt::Orientation orientation, int role ) const +{ + return sourceModel()->headerData( section, orientation, role ); +} + diff --git a/src/app/qgsbookmarks.h b/src/app/qgsbookmarks.h index ec13470ffe4..6942412000b 100644 --- a/src/app/qgsbookmarks.h +++ b/src/app/qgsbookmarks.h @@ -18,6 +18,7 @@ #define QGSBOOKMARKS_H #include +#include #include #include "ui_qgsbookmarksbase.h" @@ -33,7 +34,7 @@ class QgsProjectBookmarksTableModel: public QAbstractTableModel public: - QgsProjectBookmarksTableModel(); + QgsProjectBookmarksTableModel( QObject *parent = 0 ); int rowCount( const QModelIndex &parent = QModelIndex() ) const override; @@ -48,7 +49,27 @@ class QgsProjectBookmarksTableModel: public QAbstractTableModel bool removeRows( int row, int count, const QModelIndex &parent = QModelIndex() ) override; private slots: - void projectRead() { emit layoutChanged(); }; + void projectRead(); +}; + + +class QgsBookmarksProxyModel: public QSortFilterProxyModel +{ + Q_OBJECT + + public: + + QgsBookmarksProxyModel( QObject *parent = 0 ); + + //! This override is required because the merge model only defines headers for the SQL model + QVariant headerData( int section, Qt::Orientation orientation, int role = Qt::DisplayRole ) const override; + + public slots: + + void _resetModel() + { + reset(); + } }; /* @@ -60,7 +81,7 @@ class QgsMergedBookmarksTableModel: public QAbstractTableModel public: - QgsMergedBookmarksTableModel( QAbstractTableModel &qgisTableModel, QAbstractTableModel &projectTableModel, QTreeView *treeView ); + QgsMergedBookmarksTableModel( QAbstractTableModel &qgisTableModel, QAbstractTableModel &projectTableModel, QTreeView *treeView, QObject *parent = 0 ); int rowCount( const QModelIndex &parent = QModelIndex() ) const override; @@ -83,7 +104,6 @@ class QgsMergedBookmarksTableModel: public QAbstractTableModel void moveBookmark( QAbstractTableModel &modelFrom, QAbstractTableModel &modelTo, int row ); private slots: - void projectFileNameChanged(); void allLayoutChanged() { emit layoutChanged(); @@ -113,7 +133,8 @@ class APP_EXPORT QgsBookmarks : public QgsDockWidget, private Ui::QgsBookmarksBa private: QSqlTableModel *mQgisModel = nullptr; QgsProjectBookmarksTableModel *mProjectModel = nullptr; - std::unique_ptr mModel; + QgsMergedBookmarksTableModel *mModel = nullptr; + QgsBookmarksProxyModel *mProxyModel = nullptr; void saveWindowLocation(); void restorePosition();