From 6c10571ba4b496619c49e36aff3b5028a258ca95 Mon Sep 17 00:00:00 2001 From: Jaye Evins Date: Wed, 10 Dec 2025 13:17:25 -0500 Subject: [PATCH] In MergeView, replace QTableWidget with QTableView with custom model (#266,#217) QTableWidget was a major bottleneck for large merge sources (#217). This is because a QTableWidgetItem needed to be created for every field in every record of the merge data, whether they are being displayed or not. This was not a problem for small merge sources (only a few dozen records max), however for larger data sets this would severely affect performance and make the application unresponsive. QTableView only renders the fields and records currently visible. --- glabels/CMakeLists.txt | 2 + glabels/MergeTableModel.cpp | 178 ++++++++++++++++++++++++++++++++++++ glabels/MergeTableModel.h | 80 ++++++++++++++++ glabels/MergeView.cpp | 158 ++------------------------------ glabels/MergeView.h | 5 - glabels/ui/MergeView.ui | 29 ++++-- 6 files changed, 291 insertions(+), 161 deletions(-) create mode 100644 glabels/MergeTableModel.cpp create mode 100644 glabels/MergeTableModel.h diff --git a/glabels/CMakeLists.txt b/glabels/CMakeLists.txt index 34ea793..0626a1d 100644 --- a/glabels/CMakeLists.txt +++ b/glabels/CMakeLists.txt @@ -21,6 +21,7 @@ set (glabels_sources Help.cpp LabelEditor.cpp MainWindow.cpp + MergeTableModel.cpp MergeView.cpp MiniPreviewPixmap.cpp NotebookUtil.cpp @@ -57,6 +58,7 @@ set (glabels_qobject_headers File.h LabelEditor.h MainWindow.h + MergeTableModel.h MergeView.h ObjectEditor.h PreferencesDialog.h diff --git a/glabels/MergeTableModel.cpp b/glabels/MergeTableModel.cpp new file mode 100644 index 0000000..764a7f0 --- /dev/null +++ b/glabels/MergeTableModel.cpp @@ -0,0 +1,178 @@ +/* MergeTableModel.cpp + * + * Copyright (C) 2025 Jaye Evins + * + * This file is part of gLabels-qt. + * + * gLabels-qt is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * gLabels-qt is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with gLabels-qt. If not, see . + */ + + +#include "MergeTableModel.h" + +#include + + +namespace glabels +{ + + /// + /// Constructor + /// + MergeTableModel::MergeTableModel( merge::Merge* merge, QObject* parent ) + : QAbstractTableModel( parent ), + mMerge( merge ) + { + // Copy keys, make sure primary key is first + mDisplayKeys.push_back( mMerge->primaryKey() ); + for ( auto& key : mMerge->keys() ) + { + if ( key != mMerge->primaryKey() ) + { + mDisplayKeys.push_back( key ); + } + } + + connect( mMerge, SIGNAL(selectionChanged()), + this, SLOT(onSelectionChanged()) ); + } + + + /// + /// Row count + /// + int MergeTableModel::rowCount( const QModelIndex& parent ) const + { + return mMerge->recordList().size(); + } + + + /// + /// Column count + /// + int MergeTableModel::columnCount( const QModelIndex& parent ) const + { + return mDisplayKeys.size() + 1; + } + + + /// + /// Header data + /// + QVariant MergeTableModel::headerData( int section, Qt::Orientation orientation, int role ) const + { + if ( orientation == Qt::Vertical ) + { + return QAbstractTableModel::headerData( section, orientation, role ); + } + + if ( (role != Qt::DisplayRole) || section >= mDisplayKeys.size() ) + { + return QVariant(); + } + + return mDisplayKeys[ section ]; + } + + + /// + /// Data + /// + QVariant MergeTableModel::data( const QModelIndex& index, int role ) const + { + if ( !index.isValid() ) + { + return QVariant(); + } + + if ( (index.row() >= mMerge->recordList().size()) || + (index.column() >= mDisplayKeys.size()) ) + { + return QVariant(); + } + + + if ( (role == Qt::CheckStateRole) && (index.column() == 0) ) + { + auto record = mMerge->recordList()[ index.row() ]; + return record.isSelected() ? Qt::Checked : Qt::Unchecked; + } + + if ( role == Qt::DisplayRole ) + { + auto record = mMerge->recordList()[ index.row() ]; + auto key = mDisplayKeys[ index.column() ]; + + if ( record.contains( key ) ) + { + return record[ key ]; + } + } + + return QVariant(); + } + + + /// + /// Set data + /// + bool MergeTableModel::setData( const QModelIndex& index, const QVariant& value, int role ) + { + if ( !index.isValid() || (index.column() != 0) || (role != Qt::CheckStateRole) ) + { + return false; + } + + bool isChecked = static_cast(value.toInt()) != Qt::Unchecked; + + mMerge->blockSignals( true ); + mMerge->setSelected( index.row(), isChecked ); + mMerge->blockSignals( false ); + return true; + } + + + /// + /// Flags + /// + Qt::ItemFlags MergeTableModel::flags( const QModelIndex& index ) const + { + if ( !index.isValid() ) + { + return Qt::NoItemFlags; + } + + if ( index.column() == 0 ) + { + return Qt::ItemIsEnabled | Qt::ItemIsUserCheckable; + } + + return Qt::ItemIsEnabled; + } + + + /// + /// Selection changed handler + /// + void MergeTableModel::onSelectionChanged() + { + for ( int iRow = 0; iRow < mMerge->recordList().size(); iRow++ ) + { + auto index = createIndex( iRow, 0 ); + emit dataChanged( index, index, {Qt::CheckStateRole} ); + } + } + + +} // namespace glabels diff --git a/glabels/MergeTableModel.h b/glabels/MergeTableModel.h new file mode 100644 index 0000000..cb010d3 --- /dev/null +++ b/glabels/MergeTableModel.h @@ -0,0 +1,80 @@ +/* MergeTableModel.h + * + * Copyright (C) 2025 Jaye Evins + * + * This file is part of gLabels-qt. + * + * gLabels-qt is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * gLabels-qt is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with gLabels-qt. If not, see . + */ + +#ifndef MergeTableModel_h +#define MergeTableModel_h + + +#include "merge/Merge.h" + +#include + + +namespace glabels +{ + + /// + /// MergeTable proxy model + /// + class MergeTableModel : public QAbstractTableModel + { + Q_OBJECT + + ///////////////////////////////// + // Life Cycle + ///////////////////////////////// + public: + MergeTableModel( merge::Merge* merge, QObject* parent = nullptr ); + + + ///////////////////////////////// + // Public methods + ///////////////////////////////// + public: + int rowCount( const QModelIndex& parent = QModelIndex() ) const override; + int columnCount( const QModelIndex &parent = QModelIndex() ) const override; + + QVariant headerData( int section, Qt::Orientation orientation, int role = Qt::DisplayRole ) const override; + QVariant data( const QModelIndex& index, int role = Qt::DisplayRole ) const override; + bool setData( const QModelIndex& index, const QVariant& value, int role = Qt::EditRole ) override; + Qt::ItemFlags flags( const QModelIndex& index ) const override; + + + ///////////////////////////////// + // Private slots + ///////////////////////////////// + private slots: + void onSelectionChanged(); + + + ///////////////////////////////// + // Private Members + ///////////////////////////////// + private: + merge::Merge* mMerge; + + QStringList mDisplayKeys; + + }; + +} + + +#endif // MergeTableModel_h diff --git a/glabels/MergeView.cpp b/glabels/MergeView.cpp index 1901503..e399e38 100644 --- a/glabels/MergeView.cpp +++ b/glabels/MergeView.cpp @@ -21,6 +21,8 @@ #include "MergeView.h" +#include "MergeTableModel.h" + #include "merge/Factory.h" #include "model/FileUtil.h" @@ -38,7 +40,7 @@ namespace glabels /// Constructor /// MergeView::MergeView( QWidget *parent ) - : QWidget(parent), mModel(nullptr), mUndoRedoModel(nullptr), mBlock(false), mOldFormatComboIndex(0) + : QWidget(parent), mModel(nullptr), mUndoRedoModel(nullptr), mOldFormatComboIndex(0) { setupUi( this ); @@ -98,19 +100,11 @@ namespace glabels break; } - recordsTable->clear(); - recordsTable->setColumnCount( 0 ); - loadHeaders( mModel->merge() ); - loadTable( mModel->merge() ); + recordsTableView->setModel( new MergeTableModel( mModel->merge() ) ); + recordsTableView->resizeColumnsToContents(); connect( mModel->merge(), SIGNAL(sourceChanged()), this, SLOT(onMergeSourceChanged()) ); - - connect( mModel->merge(), SIGNAL(selectionChanged()), - this, SLOT(onMergeSelectionChanged()) ); - - connect( recordsTable, SIGNAL(cellChanged(int,int)), - this, SLOT(onCellChanged(int,int)) ); } @@ -122,32 +116,8 @@ namespace glabels QString fn = model::FileUtil::makeRelativeIfInDir( mModel->dir(), mModel->merge()->source() ); locationLineEdit->setText( fn ); - recordsTable->clear(); - recordsTable->setColumnCount( 0 ); - loadHeaders( mModel->merge() ); - loadTable( mModel->merge() ); - } - - - /// - /// Merge selection changed handler - /// - void MergeView::onMergeSelectionChanged() - { - mBlock = true; // Don't recurse - - auto& records = mModel->merge()->recordList(); - - int iRow = 0; - for ( auto& record : records ) - { - QTableWidgetItem* item = recordsTable->item( iRow, 0 ); - item->setCheckState( record.isSelected() ? Qt::Checked : Qt::Unchecked ); - - iRow++; - } - - mBlock = false; + recordsTableView->setModel( new MergeTableModel( mModel->merge() ) ); + recordsTableView->resizeColumnsToContents(); } @@ -212,118 +182,6 @@ namespace glabels } - /// - /// Cell changed handler - /// - void MergeView::onCellChanged( int iRow, int iCol ) - { - if ( !mBlock ) - { - QTableWidgetItem* item = recordsTable->item( iRow, 0 ); - bool state = (item->checkState() == Qt::Unchecked) ? false : true; - - mModel->merge()->setSelected( iRow, state ); - } - } - - - /// - /// Load headers - /// - void MergeView::loadHeaders( merge::Merge* merge ) - { - mPrimaryKey = merge->primaryKey(); - mKeys = merge->keys(); - - if ( mKeys.size() > 0 ) - { - recordsTable->setColumnCount( mKeys.size() + 1 ); // Include extra column - - // First column = primary Key - auto* item = new QTableWidgetItem( mPrimaryKey ); - item->setFlags( Qt::ItemIsEnabled ); - recordsTable->setHorizontalHeaderItem( 0, item ); - - // Starting on second column, one column per key, skip primary Key - int iCol = 1; - foreach ( QString key, mKeys ) - { - if ( key != mPrimaryKey ) - { - auto* item = new QTableWidgetItem( key ); - item->setFlags( Qt::ItemIsEnabled ); - recordsTable->setHorizontalHeaderItem( iCol, item ); - - iCol++; - } - - } - - // Extra dummy column to fill any extra horizontal space - auto* fillItem = new QTableWidgetItem(); - fillItem->setFlags( Qt::NoItemFlags ); - recordsTable->setHorizontalHeaderItem( iCol, fillItem ); - recordsTable->horizontalHeader()->setStretchLastSection( true ); - } - } - - - /// - /// Load table - /// - void MergeView::loadTable( merge::Merge* merge ) - { - mBlock = true; - - auto& records = merge->recordList(); - recordsTable->setRowCount( records.size() ); - - int iRow = 0; - for ( auto record : records ) - { - // First column for primary field - auto* item = new QTableWidgetItem(); - if ( record.contains( mPrimaryKey ) ) - { - auto text = printableTextForView( record[mPrimaryKey] ); - item->setText( text ); - } - item->setFlags( Qt::ItemIsEnabled | Qt::ItemIsUserCheckable ); - item->setCheckState( record.isSelected() ? Qt::Checked : Qt::Unchecked ); - recordsTable->setItem( iRow, 0, item ); - recordsTable->resizeColumnToContents( 0 ); - - // Starting on 2nd column, 1 column per field, skip primary field - int iCol = 1; - for ( auto& key : mKeys ) - { - if ( key != mPrimaryKey ) - { - if ( record.contains( key ) ) - { - auto text = printableTextForView( record[key] ); - auto* item = new QTableWidgetItem( text ); - item->setFlags( Qt::ItemIsEnabled ); - recordsTable->setItem( iRow, iCol, item ); - recordsTable->resizeColumnToContents( iCol ); - } - - iCol++; - } - } - - // Extra dummy column to fill any extra horizontal space - auto* fillItem = new QTableWidgetItem(); - fillItem->setFlags( Qt::NoItemFlags ); - recordsTable->setItem( iRow, iCol, fillItem ); - - iRow++; - } - - mBlock = false; - } - - /// /// modify text to be printable e.g. replace newlines /// @@ -337,4 +195,6 @@ namespace glabels return text; } + + } // namespace glabels diff --git a/glabels/MergeView.h b/glabels/MergeView.h index 698747c..279ad45 100644 --- a/glabels/MergeView.h +++ b/glabels/MergeView.h @@ -64,22 +64,18 @@ namespace glabels private slots: void onMergeChanged(); void onMergeSourceChanged(); - void onMergeSelectionChanged(); void onFormatComboActivated(); void onLocationBrowseButtonClicked(); void onSelectAllButtonClicked(); void onUnselectAllButtonClicked(); void onReloadButtonClicked(); - void onCellChanged( int iRow, int iCol ); ///////////////////////////////// // Private methods ///////////////////////////////// private: - void loadHeaders( merge::Merge* merge ); - void loadTable( merge::Merge* merge ); static QString printableTextForView( QString text ); @@ -97,7 +93,6 @@ namespace glabels QString mCwd; - bool mBlock; int mOldFormatComboIndex; }; diff --git a/glabels/ui/MergeView.ui b/glabels/ui/MergeView.ui index 8f940b2..ba0eea3 100644 --- a/glabels/ui/MergeView.ui +++ b/glabels/ui/MergeView.ui @@ -95,13 +95,6 @@ Records - - - - Qt::NoFocus - - - @@ -140,6 +133,28 @@ + + + + Qt::NoFocus + + + QAbstractItemView::NoSelection + + + Qt::ElideRight + + + true + + + true + + + false + + +