Skip to content
Permalink
Browse files

Fix crash on nested dataChanged from set current index

Add new methods and tests, try to make it clear when
a method expects a model index and when it expects
a proxy model index.
  • Loading branch information
elpaso committed Oct 29, 2020
1 parent 2ff56fc commit 884bd9a42e26bcfed28bb5c1855fbdb97cad189b
@@ -112,7 +112,7 @@ This can be used to set filters controlling which layers are shown in the view.

QgsLayerTreeNode *index2node( const QModelIndex &index ) const;
%Docstring
Returns layer tree node for given tree index. Returns root node for invalid index.
Returns layer tree node for given proxy model tree ``index``. Returns root node for invalid index.
Returns ``None`` if index does not refer to a layer tree node (e.g. it is a legend node)

Unlike :py:func:`QgsLayerTreeModel.index2Node()`, calling this method correctly accounts
@@ -123,17 +123,26 @@ for mapping the view indexes through the view's proxy model to the source model.

QModelIndex node2index( QgsLayerTreeNode *node ) const;
%Docstring
Returns index for a given node. If the node does not belong to the layer tree, the result is undefined
Returns proxy model index for a given node. If the node does not belong to the layer tree, the result is undefined

Unlike :py:func:`QgsLayerTreeModel.node2index()`, calling this method correctly accounts
for mapping the view indexes through the view's proxy model to the source model.

.. versionadded:: 3.18
%End


QModelIndex node2sourceIndex( QgsLayerTreeNode *node ) const;
%Docstring
Returns source model index for a given node. If the node does not belong to the layer tree, the result is undefined

.. versionadded:: 3.18
%End


QgsLayerTreeModelLegendNode *index2legendNode( const QModelIndex &index ) const;
%Docstring
Returns legend node for given index. Returns ``None`` for invalid index
Returns legend node for given proxy model tree ``index``. Returns ``None`` for invalid index

Unlike :py:func:`QgsLayerTreeModel.index2legendNode()`, calling this method correctly accounts
for mapping the view indexes through the view's proxy model to the source model.
@@ -143,12 +152,20 @@ for mapping the view indexes through the view's proxy model to the source model.

QModelIndex legendNode2index( QgsLayerTreeModelLegendNode *legendNode );
%Docstring
Returns index for a given legend node. If the legend node does not belong to the layer tree, the result is undefined.
Returns proxy model index for a given legend node. If the legend node does not belong to the layer tree, the result is undefined.
If the legend node is belongs to the tree but it is filtered out, invalid model index is returned.

Unlike :py:func:`QgsLayerTreeModel.legendNode2index()`, calling this method correctly accounts
for mapping the view indexes through the view's proxy model to the source model.

.. versionadded:: 3.18
%End

QModelIndex legendNode2sourceIndex( QgsLayerTreeModelLegendNode *legendNode );
%Docstring
Returns index for a given legend node. If the legend node does not belong to the layer tree, the result is undefined.
If the legend node is belongs to the tree but it is filtered out, invalid model index is returned.

.. versionadded:: 3.18
%End

@@ -566,13 +566,7 @@ QModelIndex QgsLayerTreeModel::currentIndex() const

void QgsLayerTreeModel::setCurrentIndex( const QModelIndex &currentIndex )
{
QModelIndex oldIndex = mCurrentIndex;
mCurrentIndex = currentIndex;

if ( oldIndex.isValid() )
emit dataChanged( oldIndex, oldIndex );
if ( currentIndex.isValid() )
emit dataChanged( currentIndex, currentIndex );
}


@@ -852,9 +846,9 @@ void QgsLayerTreeModel::layerFlagsChanged()
if ( !nodeLayer )
return;

QModelIndex index = node2index( nodeLayer );
const QModelIndex index = node2index( nodeLayer );
Q_ASSERT( checkIndex( index ) );
emit dataChanged( index, index );

}

void QgsLayerTreeModel::layerNeedsUpdate()
@@ -284,16 +284,25 @@ void QgsLayerTreeView::onCurrentChanged()
return;

// update the current index in model (the item will be underlined)
QModelIndex nodeLayerIndex;
QModelIndex proxyModelNodeLayerIndex;
if ( layerCurrent )
{
QgsLayerTreeLayer *nodeLayer = layerTreeModel()->rootGroup()->findLayer( layerCurrentID );
if ( nodeLayer )
nodeLayerIndex = layerTreeModel()->node2index( nodeLayer );
proxyModelNodeLayerIndex = node2index( nodeLayer );
}

if ( ! proxyModelNodeLayerIndex.isValid() )
{
mCurrentLayerID = QString();
layerTreeModel()->setCurrentIndex( QModelIndex() );
}
else
{
mCurrentLayerID = layerCurrentID;
layerTreeModel()->setCurrentIndex( mProxyModel->mapToSource( proxyModelNodeLayerIndex ) );
}
layerTreeModel()->setCurrentIndex( nodeLayerIndex );

mCurrentLayerID = layerCurrentID;
emit currentLayerChanged( layerCurrent );
}

@@ -645,7 +654,12 @@ QgsLayerTreeNode *QgsLayerTreeView::index2node( const QModelIndex &index ) const

QModelIndex QgsLayerTreeView::node2index( QgsLayerTreeNode *node ) const
{
return mProxyModel->mapFromSource( layerTreeModel()->node2index( node ) );
return mProxyModel->mapFromSource( node2sourceIndex( node ) );
}

QModelIndex QgsLayerTreeView::node2sourceIndex( QgsLayerTreeNode *node ) const
{
return layerTreeModel()->node2index( node );
}

QgsLayerTreeModelLegendNode *QgsLayerTreeView::index2legendNode( const QModelIndex &index ) const
@@ -655,7 +669,12 @@ QgsLayerTreeModelLegendNode *QgsLayerTreeView::index2legendNode( const QModelInd

QModelIndex QgsLayerTreeView::legendNode2index( QgsLayerTreeModelLegendNode *legendNode )
{
return mProxyModel->mapFromSource( layerTreeModel()->legendNode2index( legendNode ) );
return mProxyModel->mapFromSource( legendNode2sourceIndex( legendNode ) );
}

QModelIndex QgsLayerTreeView::legendNode2sourceIndex( QgsLayerTreeModelLegendNode *legendNode )
{
return layerTreeModel()->legendNode2index( legendNode );
}

QgsLayerTreeProxyModel::QgsLayerTreeProxyModel( QgsLayerTreeModel *treeModel, QObject *parent )
@@ -135,7 +135,7 @@ class GUI_EXPORT QgsLayerTreeView : public QTreeView
QgsLayerTreeProxyModel *proxyModel() const;

/**
* Returns layer tree node for given tree index. Returns root node for invalid index.
* Returns layer tree node for given proxy model tree \a index. Returns root node for invalid index.
* Returns NULLPTR if index does not refer to a layer tree node (e.g. it is a legend node)
*
* Unlike QgsLayerTreeModel::index2Node(), calling this method correctly accounts
@@ -146,7 +146,7 @@ class GUI_EXPORT QgsLayerTreeView : public QTreeView
QgsLayerTreeNode *index2node( const QModelIndex &index ) const;

/**
* Returns index for a given node. If the node does not belong to the layer tree, the result is undefined
* Returns proxy model index for a given node. If the node does not belong to the layer tree, the result is undefined
*
* Unlike QgsLayerTreeModel::node2index(), calling this method correctly accounts
* for mapping the view indexes through the view's proxy model to the source model.
@@ -155,8 +155,17 @@ class GUI_EXPORT QgsLayerTreeView : public QTreeView
*/
QModelIndex node2index( QgsLayerTreeNode *node ) const;


/**
* Returns source model index for a given node. If the node does not belong to the layer tree, the result is undefined
*
* \since QGIS 3.18
*/
QModelIndex node2sourceIndex( QgsLayerTreeNode *node ) const;


/**
* Returns legend node for given index. Returns NULLPTR for invalid index
* Returns legend node for given proxy model tree \a index. Returns NULLPTR for invalid index
*
* Unlike QgsLayerTreeModel::index2legendNode(), calling this method correctly accounts
* for mapping the view indexes through the view's proxy model to the source model.
@@ -166,7 +175,7 @@ class GUI_EXPORT QgsLayerTreeView : public QTreeView
QgsLayerTreeModelLegendNode *index2legendNode( const QModelIndex &index ) const;

/**
* Returns index for a given legend node. If the legend node does not belong to the layer tree, the result is undefined.
* Returns proxy model index for a given legend node. If the legend node does not belong to the layer tree, the result is undefined.
* If the legend node is belongs to the tree but it is filtered out, invalid model index is returned.
*
* Unlike QgsLayerTreeModel::legendNode2index(), calling this method correctly accounts
@@ -176,6 +185,14 @@ class GUI_EXPORT QgsLayerTreeView : public QTreeView
*/
QModelIndex legendNode2index( QgsLayerTreeModelLegendNode *legendNode );

/**
* Returns index for a given legend node. If the legend node does not belong to the layer tree, the result is undefined.
* If the legend node is belongs to the tree but it is filtered out, invalid model index is returned.
*
* \since QGIS 3.18
*/
QModelIndex legendNode2sourceIndex( QgsLayerTreeModelLegendNode *legendNode );

//! Gets access to the default actions that may be used with the tree view
QgsLayerTreeViewDefaultActions *defaultActions();

@@ -125,12 +125,15 @@ def testDefaultActions(self):

# show in overview action
view.setCurrentLayer(self.layer)
self.assertEqual(view.currentNode().customProperty('overview', 0), False)
self.assertEqual(
view.currentNode().customProperty('overview', 0), False)
show_in_overview = actions.actionShowInOverview()
show_in_overview.trigger()
self.assertEqual(view.currentNode().customProperty('overview', 0), True)
self.assertEqual(
view.currentNode().customProperty('overview', 0), True)
show_in_overview.trigger()
self.assertEqual(view.currentNode().customProperty('overview', 0), False)
self.assertEqual(
view.currentNode().customProperty('overview', 0), False)

def testMoveOutOfGroupActionLayer(self):
"""Test move out of group action on layer"""
@@ -173,11 +176,13 @@ def testMoveToTopActionLayer(self):
if USE_MODEL_TESTER:
proxy_tester = QAbstractItemModelTester(view.model())
actions = QgsLayerTreeViewDefaultActions(view)
self.assertEqual(self.project.layerTreeRoot().layerOrder(), [self.layer, self.layer2, self.layer3])
self.assertEqual(self.project.layerTreeRoot().layerOrder(), [
self.layer, self.layer2, self.layer3])
view.setCurrentLayer(self.layer3)
movetotop = actions.actionMoveToTop()
movetotop.trigger()
self.assertEqual(self.project.layerTreeRoot().layerOrder(), [self.layer3, self.layer, self.layer2])
self.assertEqual(self.project.layerTreeRoot().layerOrder(), [
self.layer3, self.layer, self.layer2])

def testMoveToTopActionGroup(self):
"""Test move to top action on group"""
@@ -292,11 +297,13 @@ def testMoveToBottomActionLayer(self):
if USE_MODEL_TESTER:
proxy_tester = QAbstractItemModelTester(view.model())
actions = QgsLayerTreeViewDefaultActions(view)
self.assertEqual(self.project.layerTreeRoot().layerOrder(), [self.layer, self.layer2, self.layer3])
self.assertEqual(self.project.layerTreeRoot().layerOrder(), [
self.layer, self.layer2, self.layer3])
view.setCurrentLayer(self.layer)
movetobottom = actions.actionMoveToBottom()
movetobottom.trigger()
self.assertEqual(self.project.layerTreeRoot().layerOrder(), [self.layer2, self.layer3, self.layer])
self.assertEqual(self.project.layerTreeRoot().layerOrder(), [
self.layer2, self.layer3, self.layer])

def testMoveToBottomActionGroup(self):
"""Test move to bottom action on group"""
@@ -409,18 +416,25 @@ def testSetLayerVisible(self):
view.setModel(self.model)
if USE_MODEL_TESTER:
proxy_tester = QAbstractItemModelTester(view.model())
self.project.layerTreeRoot().findLayer(self.layer).setItemVisibilityChecked(True)
self.project.layerTreeRoot().findLayer(self.layer2).setItemVisibilityChecked(True)
self.assertTrue(self.project.layerTreeRoot().findLayer(self.layer).itemVisibilityChecked())
self.assertTrue(self.project.layerTreeRoot().findLayer(self.layer2).itemVisibilityChecked())
self.project.layerTreeRoot().findLayer(
self.layer).setItemVisibilityChecked(True)
self.project.layerTreeRoot().findLayer(
self.layer2).setItemVisibilityChecked(True)
self.assertTrue(self.project.layerTreeRoot().findLayer(
self.layer).itemVisibilityChecked())
self.assertTrue(self.project.layerTreeRoot().findLayer(
self.layer2).itemVisibilityChecked())

view.setLayerVisible(None, True)
view.setLayerVisible(self.layer, True)
self.assertTrue(self.project.layerTreeRoot().findLayer(self.layer).itemVisibilityChecked())
self.assertTrue(self.project.layerTreeRoot().findLayer(
self.layer).itemVisibilityChecked())
view.setLayerVisible(self.layer2, False)
self.assertFalse(self.project.layerTreeRoot().findLayer(self.layer2).itemVisibilityChecked())
self.assertFalse(self.project.layerTreeRoot().findLayer(
self.layer2).itemVisibilityChecked())
view.setLayerVisible(self.layer2, True)
self.assertTrue(self.project.layerTreeRoot().findLayer(self.layer2).itemVisibilityChecked())
self.assertTrue(self.project.layerTreeRoot().findLayer(
self.layer2).itemVisibilityChecked())

def testProxyModel(self):
"""Test proxy model filtering and hidden layers"""
@@ -447,15 +461,15 @@ def testProxyModel(self):

self.assertEqual(proxy_items, ['layer1', 'layer2', 'layer3'])

self.layer.setFlags(self.layer.Hidden)
self.layer3.setFlags(self.layer.Hidden)
self.assertEqual(tree_model.rowCount(), 3)
self.assertEqual(proxy_model.rowCount(), 2)

proxy_items = []
for r in range(proxy_model.rowCount()):
proxy_items.append(proxy_model.data(proxy_model.index(r, 0)))

self.assertEqual(proxy_items, ['layer2', 'layer3'])
self.assertEqual(proxy_items, ['layer1', 'layer2'])

view.setShowHidden(True)

@@ -475,7 +489,7 @@ def testProxyModel(self):
for r in range(proxy_model.rowCount()):
proxy_items.append(proxy_model.data(proxy_model.index(r, 0)))

self.assertEqual(proxy_items, ['layer2', 'layer3'])
self.assertEqual(proxy_items, ['layer1', 'layer2'])

# Test filters
proxy_model.setFilterText('layer2')
@@ -488,6 +502,42 @@ def testProxyModel(self):

self.assertEqual(proxy_items, ['layer2'])

def testProxyModelCurrentIndex(self):
"""Test a crash spotted out while developing the proxy model"""

view = QgsLayerTreeView()
view.setModel(self.model)
if USE_MODEL_TESTER:
proxy_tester = QAbstractItemModelTester(view.model())

view.setCurrentLayer(self.layer3)
self.layer3.setFlags(self.layer.Hidden)

def testNode2IndexMethods(self):
"""Test node2index and node2sourceIndex"""

view = QgsLayerTreeView()
view.setModel(self.model)
if USE_MODEL_TESTER:
proxy_tester = QAbstractItemModelTester(view.model())
tree_model = view.layerTreeModel()
proxy_model = view.proxyModel()

proxy_index = proxy_model.index(1, 0)
self.assertTrue(proxy_model.checkIndex(proxy_index))
node2 = view.index2node(proxy_index)
self.assertEqual(node2.name(), 'layer2')

proxy_layer2_index = view.node2index(node2)
self.assertTrue(proxy_model.checkIndex(proxy_layer2_index))
self.assertEqual(proxy_layer2_index, view.node2index(node2))

source_index = tree_model.index(1, 0)
self.assertTrue(tree_model.checkIndex(source_index))
tree_layer2_index = view.node2sourceIndex(node2)
self.assertTrue(tree_model.checkIndex(tree_layer2_index))
self.assertEqual(tree_layer2_index, view.node2sourceIndex(node2))


if __name__ == '__main__':
unittest.main()

0 comments on commit 884bd9a

Please sign in to comment.
You can’t perform that action at this time.