Skip to content

Commit

Permalink
Fix crash when using QgsLayoutItemComboBox when null values
Browse files Browse the repository at this point in the history
are displayed

Turns out there is NO reliable way to create a proxy model
which adds new rows to a model, so just bite the bullet and
do it in the underlying model (yuck)
  • Loading branch information
nyalldawson committed Mar 18, 2019
1 parent 7cc903a commit bdbb622
Show file tree
Hide file tree
Showing 11 changed files with 448 additions and 100 deletions.
7 changes: 0 additions & 7 deletions python/core/auto_generated/layout/qgslayoutmodel.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -180,13 +180,6 @@ Returns ``True`` if the model includes the empty item choice.
.. versionadded:: 3.8
%End

virtual int rowCount( const QModelIndex &parent = QModelIndex() ) const;

virtual QVariant data( const QModelIndex &index, int role = Qt::DisplayRole ) const;

virtual bool setData( const QModelIndex &index, const QVariant &value, int role = Qt::EditRole );


protected:
virtual bool filterAcceptsRow( int sourceRow, const QModelIndex &sourceParent ) const;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@




class QgsLayoutItemComboBox : QComboBox
{
%Docstring
Expand Down
7 changes: 7 additions & 0 deletions src/app/layout/qgslayoutitemslistview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ void QgsLayoutItemsListViewModel::setSelected( const QModelIndex &index )
mModel->setSelected( mapToSource( index ) );
}

bool QgsLayoutItemsListViewModel::filterAcceptsRow( int sourceRow, const QModelIndex & ) const
{
if ( sourceRow == 0 )
return false; // hide empty null item row
return true;
}

QVariant QgsLayoutItemsListViewModel::data( const QModelIndex &index, int role ) const
{
if ( !index.isValid() )
Expand Down
4 changes: 4 additions & 0 deletions src/app/layout/qgslayoutitemslistview.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ class QgsLayoutItemsListViewModel : public QSortFilterProxyModel
public slots:
void setSelected( const QModelIndex &index );

protected:

bool filterAcceptsRow( int sourceRow, const QModelIndex &sourceParent ) const override;

private:

QgsLayoutModel *mModel = nullptr;
Expand Down
96 changes: 28 additions & 68 deletions src/core/layout/qgslayoutmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ QgsLayoutModel::QgsLayoutModel( QgsLayout *layout, QObject *parent )
QgsLayoutItem *QgsLayoutModel::itemFromIndex( const QModelIndex &index ) const
{
//try to return the QgsLayoutItem corresponding to a QModelIndex
if ( !index.isValid() )
if ( !index.isValid() || index.row() == 0 )
{
return nullptr;
}
Expand All @@ -56,10 +56,14 @@ QModelIndex QgsLayoutModel::index( int row, int column,
return QModelIndex();
}

if ( !parent.isValid() && row >= 0 && row < mItemsInScene.size() )
if ( !parent.isValid() && row == 0 )
{
return createIndex( row, column, nullptr );
}
else if ( !parent.isValid() && row >= 1 && row < mItemsInScene.size() + 1 )
{
//return an index for the layout item at this position
return createIndex( row, column, mItemsInScene.at( row ) );
return createIndex( row, column, mItemsInScene.at( row - 1 ) );
}

//only top level supported for now
Expand Down Expand Up @@ -94,14 +98,14 @@ int QgsLayoutModel::rowCount( const QModelIndex &parent ) const
{
if ( !parent.isValid() )
{
return mItemsInScene.size();
return mItemsInScene.size() + 1;
}

QGraphicsItem *parentItem = itemFromIndex( parent );

if ( !parentItem )
{
return mItemsInScene.size();
return mItemsInScene.size() + 1;
}
else
{
Expand Down Expand Up @@ -482,15 +486,15 @@ void QgsLayoutModel::rebuildSceneItemList()
else if ( sceneListPos != -1 )
{
//in list, but in wrong spot
beginMoveRows( QModelIndex(), sceneListPos, sceneListPos, QModelIndex(), row );
beginMoveRows( QModelIndex(), sceneListPos + 1, sceneListPos + 1, QModelIndex(), row + 1 );
mItemsInScene.removeAt( sceneListPos );
mItemsInScene.insert( row, item );
endMoveRows();
}
else
{
//needs to be inserted into list
beginInsertRows( QModelIndex(), row, row );
beginInsertRows( QModelIndex(), row + 1, row + 1 );
mItemsInScene.insert( row, item );
endInsertRows();
}
Expand Down Expand Up @@ -570,27 +574,6 @@ void QgsLayoutModel::setItemRemoved( QgsLayoutItem *item )
endRemoveRows();
}

#if 0
void QgsLayoutModel::setItemRestored( QgsComposerItem *item )
{
if ( !item )
{
//nothing to do
return;
}

int pos = mItemZList.indexOf( item );
if ( pos == -1 )
{
//item not in z list, nothing to do
return;
}

item->setIsRemoved( false );
rebuildSceneItemList();
}
#endif

void QgsLayoutModel::updateItemDisplayName( QgsLayoutItem *item )
{
if ( !item )
Expand Down Expand Up @@ -798,7 +781,7 @@ bool QgsLayoutModel::reorderItemToTop( QgsLayoutItem *item )

//move item to top
int row = itemIndex.row();
beginMoveRows( QModelIndex(), row, row, QModelIndex(), 0 );
beginMoveRows( QModelIndex(), row, row, QModelIndex(), 1 );
refreshItemsInScene();
endMoveRows();
return true;
Expand Down Expand Up @@ -896,15 +879,22 @@ Qt::ItemFlags QgsLayoutModel::flags( const QModelIndex &index ) const
return flags | Qt::ItemIsDropEnabled;
}

switch ( index.column() )
if ( index.row() == 0 )
{
case Visibility:
case LockStatus:
return flags | Qt::ItemIsEnabled | Qt::ItemIsSelectable | Qt::ItemIsUserCheckable | Qt::ItemIsEditable | Qt::ItemIsDragEnabled;
case ItemId:
return flags | Qt::ItemIsEnabled | Qt::ItemIsSelectable | Qt::ItemIsEditable | Qt::ItemIsDragEnabled;
default:
return flags | Qt::ItemIsEnabled | Qt::ItemIsSelectable;
return flags | Qt::ItemIsEnabled | Qt::ItemIsSelectable;
}
else
{
switch ( index.column() )
{
case Visibility:
case LockStatus:
return flags | Qt::ItemIsEnabled | Qt::ItemIsSelectable | Qt::ItemIsUserCheckable | Qt::ItemIsEditable | Qt::ItemIsDragEnabled;
case ItemId:
return flags | Qt::ItemIsEnabled | Qt::ItemIsSelectable | Qt::ItemIsEditable | Qt::ItemIsDragEnabled;
default:
return flags | Qt::ItemIsEnabled | Qt::ItemIsSelectable;
}
}
}

Expand All @@ -922,7 +912,7 @@ QModelIndex QgsLayoutModel::indexForItem( QgsLayoutItem *item, const int column
return QModelIndex();
}

return index( row, column );
return index( row + 1, column );
}

///@cond PRIVATE
Expand Down Expand Up @@ -950,7 +940,6 @@ QgsLayoutProxyModel::QgsLayoutProxyModel( QgsLayout *layout, QObject *parent )
if ( mLayout )
setSourceModel( mLayout->itemsModel() );

// TODO doesn't seem to work correctly - not updated when item changes
setDynamicSortFilter( true );
setSortLocaleAware( true );
sort( QgsLayoutModel::ItemId );
Expand All @@ -977,35 +966,6 @@ bool QgsLayoutProxyModel::lessThan( const QModelIndex &left, const QModelIndex &
return QString::localeAwareCompare( item1->displayName(), item2->displayName() ) < 0;
}

int QgsLayoutProxyModel::rowCount( const QModelIndex &parent ) const
{
return QSortFilterProxyModel::rowCount( parent ) + ( mAllowEmpty ? 1 : 0 );
}

QVariant QgsLayoutProxyModel::data( const QModelIndex &index, int role ) const
{
if ( mAllowEmpty && index.row() == rowCount() - 1 )
{
return QVariant();
}
else
{
return QSortFilterProxyModel::data( index, role );
}
}

bool QgsLayoutProxyModel::setData( const QModelIndex &index, const QVariant &value, int role )
{
if ( mAllowEmpty && index.row() == rowCount() - 1 )
{
return false;
}
else
{
return QSortFilterProxyModel::setData( index, value, role );
}
}

QgsLayoutItem *QgsLayoutProxyModel::itemFromSourceIndex( const QModelIndex &sourceIndex ) const
{
if ( !mLayout )
Expand Down
4 changes: 0 additions & 4 deletions src/core/layout/qgslayoutmodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -372,10 +372,6 @@ class CORE_EXPORT QgsLayoutProxyModel: public QSortFilterProxyModel
*/
bool allowEmptyItem() const;

int rowCount( const QModelIndex &parent = QModelIndex() ) 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;

protected:
bool filterAcceptsRow( int sourceRow, const QModelIndex &sourceParent ) const override;
bool lessThan( const QModelIndex &left, const QModelIndex &right ) const override;
Expand Down
6 changes: 5 additions & 1 deletion src/gui/layout/qgslayoutitemcombobox.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
#include "qgslayoutitemcombobox.h"
#include "qgslayoutmodel.h"

//
// QgsLayoutItemComboBox
//

QgsLayoutItemComboBox::QgsLayoutItemComboBox( QWidget *parent, QgsLayout *layout )
: QComboBox( parent )
{
Expand Down Expand Up @@ -60,7 +64,7 @@ void QgsLayoutItemComboBox::setItem( const QgsLayoutItem *item )
return;
}
}
setCurrentIndex( mProxyModel->allowEmptyItem() ? mProxyModel->rowCount() - 1 : -1 );
setCurrentIndex( mProxyModel->allowEmptyItem() ? 0 : -1 );
}

QgsLayoutItem *QgsLayoutItemComboBox::currentItem() const
Expand Down
1 change: 1 addition & 0 deletions src/gui/layout/qgslayoutitemcombobox.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "qgis_gui.h"
#include "qgslayoutmodel.h"


/**
* \class QgsLayoutItemComboBox
* \ingroup gui
Expand Down
Loading

0 comments on commit bdbb622

Please sign in to comment.