beginMoveRows corrupting persistent model indexes

[Jmgr]

Hi everyone,

I am coding an application that needs to show items using a QTableView, so I subclassed QAbstractItemModel to allow handling of the data without duplication. (My data is stored into a QList using pointers)

I store this pointer into a custom role in each row element (the user can only move rows, not separate items).

When moving items using drag & drop I obtain this assert fail :

ASSERT failure in QPersistentModelIndex::~QPersistentModelIndex: "persistent model indexes corrupted", file kernel\qabstractitemmodel.cpp, line 544

ASSERT failure in QPersistentModelIndex::~QPersistentModelIndex: "persistent model indexes corrupted", file kernel\qabstractitemmodel.cpp, line 544

To copy to clipboard, switch view to plain text mode 

There are few usage examples of the beginMoveRows and endMoveRows methods because it's quite new (4.6), but I think I used it correctly, so the problem must be somewhere else...

This is the code where I use this methods :

if(!beginMoveRows(QModelIndex(), rowId, rowId, QModelIndex(), position))
		continue;
	    mScript->moveAction(rowId, position);
	    endMoveRows();

if(!beginMoveRows(QModelIndex(), rowId, rowId, QModelIndex(), position))
		continue;
	    mScript->moveAction(rowId, position);
	    endMoveRows();

To copy to clipboard, switch view to plain text mode 

mScript is my QList with pointers, and I use the move method.

Please say I you need more extracts...

[wysota]

What does moveAction() do? What values get passed as rowId and position?

[Jmgr]

Here is the code, I think it's better than a description

void Script::moveAction(int startLine, int endLine)
{
    if(startLine < 0 || startLine >= mActions.count() ||
       endLine < 0 || startLine == endLine)
	return;
 
    if(endLine >= mActions.count())
	mActions.append(mActions.takeAt(startLine));
    else
	mActions.move(startLine, endLine);
}

void Script::moveAction(int startLine, int endLine)
{
    if(startLine < 0 || startLine >= mActions.count() ||
       endLine < 0 || startLine == endLine)
	return;
    
    if(endLine >= mActions.count())
	mActions.append(mActions.takeAt(startLine));
    else
	mActions.move(startLine, endLine);
}

To copy to clipboard, switch view to plain text mode 

QList<Action *> mActions;

QList<Action *> mActions;

To copy to clipboard, switch view to plain text mode 

bool ScriptModel::dropMimeData(const QMimeData *data, Qt::DropAction action, int row, int column, const QModelIndex &parent)
{
    if(action == Qt::IgnoreAction)
        return true;
 
    if(column > 3 || parent.isValid())
        return false;
 
    int position;
 
    if(row != -1)
        position = row;
    else
        position = rowCount(QModelIndex());
 
	QByteArray encodedData = data->data("application/act.action");
	QDataStream stream(&encodedData, QIODevice::ReadOnly);
 
	QList<int> rowIdList;
	while(!stream.atEnd())
	{
	    QString text;
	    stream >> text;
	    rowIdList << text.toInt();
	}
 
	foreach(int rowId, rowIdList)
	{
	    Action *originalAction = mScript->actionAt(rowId);
	    if(!originalAction)
		continue;
 
	    if(rowId == position)
		continue;
 
	    if(!beginMoveRows(QModelIndex(), rowId, rowId, QModelIndex(), position))
		continue;
	    mScript->moveAction(rowId, position);
	    endMoveRows();
	    }
	return true;
}

bool ScriptModel::dropMimeData(const QMimeData *data, Qt::DropAction action, int row, int column, const QModelIndex &parent)
{
    if(action == Qt::IgnoreAction)
        return true;
    
    if(column > 3 || parent.isValid())
        return false;
 
    int position;
 
    if(row != -1)
        position = row;
    else
        position = rowCount(QModelIndex());

	QByteArray encodedData = data->data("application/act.action");
	QDataStream stream(&encodedData, QIODevice::ReadOnly);
	
	QList<int> rowIdList;
	while(!stream.atEnd())
	{
	    QString text;
	    stream >> text;
	    rowIdList << text.toInt();
	}

	foreach(int rowId, rowIdList)
	{
	    Action *originalAction = mScript->actionAt(rowId);
	    if(!originalAction)
		continue;
	    
	    if(rowId == position)
		continue;
	    
	    if(!beginMoveRows(QModelIndex(), rowId, rowId, QModelIndex(), position))
		continue;
	    mScript->moveAction(rowId, position);
	    endMoveRows();
	    }
	return true;
}

To copy to clipboard, switch view to plain text mode 

I know this would probably fail when moving multiple items because if the lower item is moved before the row ids will not be valid any more. But this asserts even for the first one so that is probably not the problem.

[wysota]

If row equals -1 then your code will fail. rowCount() is larger by one than maximum allowed index (row number) of an element. Moving items doesn't increase the count, so the maximum allowed row number is rowCount()-1.

[Jmgr]

True, but I have this assert on any operation, even when I move an item from row 1 to row 0 when 3 items are in the list.

[wysota]

Back to my original question - what are the parameters passed to those methods? Please use a debugger or qDebug statements to dump them. I wouldn't be surprised if row was always -1

[Jmgr]

I added a qDebug at the start of dropMimeData.
All the tests are done with 3 items in the list.

When moving from position 0 to position 1 :

row= 2 column= 1 parent= QModelIndex(-1,-1,0x0,QObject(0x0) )

row= 2 column= 1 parent= QModelIndex(-1,-1,0x0,QObject(0x0) )

To copy to clipboard, switch view to plain text mode 

When moving from position 1 to position 0 :

row= 0 column= 1 parent= QModelIndex(-1,-1,0x0,QObject(0x0) )

row= 0 column= 1 parent= QModelIndex(-1,-1,0x0,QObject(0x0) )

To copy to clipboard, switch view to plain text mode 

When moving from position 0 to position 3 : (after the last element)

row= -1 column= -1 parent= QModelIndex(-1,-1,0x0,QObject(0x0) )

row= -1 column= -1 parent= QModelIndex(-1,-1,0x0,QObject(0x0) )

To copy to clipboard, switch view to plain text mode 

I get the assert in the first two cases, and I return false when row == -1.

[wysota]

Please provide a minimal compilable example reproducing the problem. You are using 4.6.0 stable, right?

[Jmgr]

Here is a minimal sample attached. Yes I use the 4.6 stable.

I think I found the problem, I use the ModelTest from http://labs.trolltech.com/page/Proje...view/Modeltest, and there seems to be some conflict between ModelTest and beginMoveRows... When deactivating it (by commenting the new ModelTest(sm, this); row in mainwindow.cpp no more asserts appear)

Do you think I made something wrong ? Or is it a ModelTest bug ?

[wysota]

ModelTest was not meant to be used with moveRows() as the latter was introduced with 4.6. By the way... is this really a minimal example?

[Jmgr]

Ok, then my problem is fixed, thanks for your help !

By the way... is this really a minimal example?

Well... I wasn't sure where the problem was, so I included the whole Script stuff. But yes, I could have removed it since it was clearly a ModelTest issue, sorry.

[Jmgr]

Hm, I found something strange within the private QAbstractItemModelPrivate::allowMove method in Qt.

// Don't move the range within itself.
    if (destinationParent == srcParent)
        return !(destinationStart >= start && destinationStart <= end + 1);

// Don't move the range within itself.
    if (destinationParent == srcParent)
        return !(destinationStart >= start && destinationStart <= end + 1);

To copy to clipboard, switch view to plain text mode 

I don't understand the + 1 here, because this is preventing the move of an item from position 0 to position 1...

I this case destinationStart is == 1, start and end are == 0, so this method returns false, but the move operation seems to be valid.

I tried removing the + 1 and everything goes fine. But I don't want to edit Qt to be able to move an item from position 0 to position 1

*Could* this be a bug in Qt ?

[raven-worx]

i know this thread is quite old, but i may help others...

*Could* this be a bug in Qt ?

This isn't a bug, see http://bugreports.qt.nokia.com/browse/QTBUG-6940