Remove Move-Back Behavior On Recurve Tree (QAbstractItemModel)

[TrueTrav]

I've setup my own derivative class of QAbstractItemModel for my engine's node hierarchy. It seems to perform inserts and deletes properly. On top of that, I can get my elements to my via drag and drop. I've overridden moveRows in my QAbstractItemModel subclass, and I call moveRows right before dropMimeData() returns. It appears that dropMimeData() needs to return false upon success after moveRows() is called for the QTreeView to update correctly. If it returns true, then my items will disappear in the view. I'll print out my scene's node hierarchy to the console, and everything looks correct. It just seems that the model doesn't update. Here's the implementation of my SceneModel class (subclassed from QAbstractItemModel):

const QString SceneModelMIMEType = "application/node.bin";

#include "SceneModel.h"
#include "Scene.h"
#include "Node.h"

#include <stdio.h>
#include <stdlib.h>

#include <QKeyEvent>
#include <QMimeData>
#include <QByteArray>
#include <QDebug>


Qt::ItemFlags SceneModel::flags(const QModelIndex &index) const
{
	Qt::ItemFlags defaultFlags = QAbstractItemModel::flags(index);
	if(index.isValid())
		return defaultFlags | Qt::ItemIsDropEnabled | Qt::ItemIsDragEnabled | Qt::ItemIsEditable;
	else
		return defaultFlags | Qt::ItemIsDropEnabled;

	return QAbstractItemModel::flags(index);
}

Qt::DropActions SceneModel::supportedDropActions() const
{
	return QAbstractItemModel::supportedDropActions() | Qt::MoveAction;
}

int SceneModel::rowCount(const QModelIndex &parent) const
{
	// make sure the scene's root node is valid and the parent's column is zero
	if(!Scene::GetInstance()->GetRoot() || parent.column() > 0)
		return 0;

	// return the parent's children, if valid
	if(parent.isValid())
		return static_cast<Node*>(parent.internalPointer())->GetNumChildren();

	// otherwise, return the children
	return Scene::GetInstance()->GetRoot()->GetNumChildren();
}

int SceneModel::columnCount(const QModelIndex &parent) const
{
	return 1;
}

QModelIndex SceneModel::index(int row, int column, const QModelIndex &parent) const
{
	// make sure the root, row and column data is valid
	if(!Scene::GetInstance()->GetRoot() || !hasIndex(row, column, parent))
		return QModelIndex();

	// get the parent Node from the index
	Node *parentItem = Scene::GetInstance()->GetRoot();
	if(parent.isValid())
		parentItem = static_cast<Node*>(parent.internalPointer());

	Node *childItem = parentItem->GetChild(row);
	if(childItem)
		return createIndex(row, column, childItem);

	return QModelIndex();
}

QModelIndex SceneModel::parent(const QModelIndex &index) const
{
	// make sure the index is valid
	if(!index.isValid())
		return QModelIndex();

	// get the child item to get the parent item
	Node *childItem = static_cast<Node*>(index.internalPointer());
	if(!childItem) return QModelIndex();
	Node *parentItem = childItem->GetParent();

	// return nothing if the parent item is the root
	if(!parentItem || parentItem == Scene::GetInstance()->GetRoot())
		return QModelIndex();

	return createIndex(parentItem->GetChildIndex(), 0, parentItem);
}

QVariant SceneModel::data(const QModelIndex &index, int role) const
{
	if(!index.isValid())
		return QVariant();

	// handle displaying the data
	if(role == Qt::DisplayRole || role == Qt::EditRole)
	{
		// get the child and parent Node pointers
		Node *childItem = static_cast<Node*>(index.internalPointer());
		return QString(childItem->GetName().c_str());
	}

	return QVariant();
}

bool SceneModel::submit()
{
	// check if the selected node is valid
	Node *selectedNode = Scene::GetInstance()->GetSelectedNode();
	if(selectedNode)
	{
		// get the selected node's index, and update its display data
		QModelIndex selectedIndex = createIndex(selectedNode->GetChildIndex(), 0, selectedNode);
		setData(selectedIndex, QString(selectedNode->GetName().c_str()), Qt::DisplayRole);
	}
	return true;
}

bool SceneModel::setData(const QModelIndex &index, const QVariant &value, int role)
{
	// check if the index is valid, and either in EditRole or DisplayRole mode
	if(index.isValid() && (role == Qt::EditRole || role == Qt::DisplayRole))
	{
		//
		Node *node = static_cast<Node*>(index.internalPointer());
		if(!node) return false;

		// set the node's new name, and signal that data has changed
		node->SetName(std::string(value.toString().toUtf8()));
		emit dataChanged(index, index);
		return true;
	}
	return setData(index, value, role);
}

bool SceneModel::dropMimeData(const QMimeData *data, Qt::DropAction action, int row, int column, const QModelIndex &parent)
{
	// make sure this is aciton shouldn't be ignored
	if(!data || action == Qt::IgnoreAction)
		return false;

	// get rowCount if row is invalid (this might be an old Qt bug)
	if(row == -1)
		row = rowCount() - 1;

	// get the parent node
	Node *parentNode = static_cast<Node*>(parent.internalPointer());
	if(!parentNode) return false;

	// get the encoded data of our Node pointer
	QByteArray encodedData = data->data(SceneModelMIMEType);
	Node *node = (Node*)encodedData.toULongLong();
	if(!node) return false;

	Node *sourceParentNode = node->GetParent();
	QModelIndex sourceParent = createIndex(sourceParentNode->GetChildIndex(), 0, sourceParentNode);

	//qDebug() << "dropMimeData() - movedNode:" << node->GetName().c_str() << "parentNode:" << sourceParentNode->GetName().c_str();
	/*qDebug()	<< "sourceParent:" << sourceParentNode->GetName().c_str()
				<< "destinationParent:" << parentNode->GetName().c_str()
				<< "movingNode:" << node->GetName().c_str()
				<< "sourceRow:" << node->GetChildIndex()
				<< "destinationRow:" << row;*/

	// move the row, but don't return true as that'll trigger the drop event
	moveRow(sourceParent, node->GetChildIndex(), parent, row);
	return false; // returning true causes weird behavior
}

QStringList SceneModel::mimeTypes() const
{
	QStringList types;
	types << SceneModelMIMEType;
	return types;
}

QMimeData *SceneModel::mimeData(const QModelIndexList &indexes) const
{
	// make sure there's exactly 1 index and it's valid
	if(indexes.size() != 1 || !indexes[0].isValid())
		return nullptr;

	// get the Node pointer from the index's internal pointer, convert it to quintptr, then to a UTF string
	quintptr nodeAddress = (quintptr)indexes[0].internalPointer();
	QByteArray encodedData(QString::number(nodeAddress).toUtf8());

	// allocate mimeData, provide it with encodedData, and return it
	QMimeData *mimeData = new QMimeData();
	mimeData->setData(SceneModelMIMEType, encodedData);
	return mimeData;
}

bool SceneModel::insertRows(int row, int count, const QModelIndex &parent)
{
	Node *parentNode = static_cast<Node*>(parent.internalPointer());
	if(!parentNode)
		parentNode = Scene::GetInstance()->GetRoot();

	// make sure a parent node was found before continuing
	if(parentNode)
	{
		// begin inserting rows, and add begin adding uniquely-named fields
		beginInsertRows(parent, row, row+count-1);

		// insert nodes
		for(int i=0;i<count;++i)
			parentNode->AddChild("New Node");
		endInsertRows();
		return true;
	}
	return QAbstractItemModel::insertRows(row, count, parent);
}

bool SceneModel::removeRows(int row, int count, const QModelIndex &parent)
{
	Node *parentItem = static_cast<Node*>(parent.internalPointer());
	if(!parentItem)
		parentItem = Scene::GetInstance()->GetRoot();

	// make sure a parent node was found before continuing
	if(parentItem)
	{
		beginRemoveRows(parent, row, row+count-1);
		parentItem->RemoveChildren(row, count);
		endRemoveRows();
		return true;
	}
	return QAbstractItemModel::removeRows(row, count, parent);
}

bool SceneModel::moveRows(const QModelIndex &sourceParent, int sourceRow, int count, const QModelIndex &destinationParent, int destinationChild)
{
	Node *sourceParentNode = static_cast<Node*>(sourceParent.internalPointer());
	Node *destinationParentNode = static_cast<Node*>(destinationParent.internalPointer());
	Node *childNode = sourceParentNode->GetChild(sourceRow);

	// if source and destination parents are the same, move elements locally
	if(sourceParentNode == destinationParentNode)
	{
		// only process if a local move is possible
		if(sourceParentNode->IsMovePossible(sourceRow, count, destinationChild))
		{
			beginMoveRows(sourceParent, sourceRow, sourceRow+count-1, destinationParent, destinationChild);
			sourceParentNode->MoveChildren(sourceRow, count, destinationChild);
			endMoveRows();
		}
	} else {
		// otherwise, move the node under the parent
		beginMoveRows(sourceParent, sourceRow, sourceRow+count-1, destinationParent, destinationChild);
		childNode->SetParent(destinationParentNode);
		endMoveRows();
	}
	Scene::GetInstance()->GetRoot()->PrintChildren();
	return true;
}

I didn't provide the interface since it doesn't have any member fields. It just has a constructor and destructor, but the constructor that initializes the base class with the passed-in QObject *parent parameter. I couldn't figure out how to embed the code above into code tags so that it's much easier to read. Sorry if something didn't make sense in my description above. I'm tired today.

[edit: Added missing coding tags ``` before and after code SGaist]

[SGaist]

Hi,

One thing I can see in your code (but not directly related) is that you call setData in setData rather than the base class implementation.

[TrueTrav]

Thanks for fixing my code. I saw checked out your edit below, and I'll post my code properly going forward. Also, good point about setData. This could have been bad as it could have created an infinite loop if I didn't have any conditions that resulted in another return statement. When I have dropMimeData() returning true, it looks like my changes stick for a split-second. Then, the node that was moved will disappear from the view. The output of my Node's hierarchy to the console shows that all nodes are correct.

[SGaist]

Is it really row move ? Aren't you changing the layout of your data structure ?

[TrueTrav]

^That is a good question that I should have asked originally. I am changing the layout of my data structure. So, when I drop the node I'm dragging onto my destination node in the hierarchy, I use my node's SetParent() method to change the hierarchy relationship in my Node tree. It's essentially a "move", at least as far as my nodes are concerned, but I'm not sure if this would be considered a "move" as far as my model is concerned. If this isn't a legitimate move, then should I be adding/removing rows?

EDIT: As far as software design goes: my SceneModel subclass of QAbstractItemModel is supposed to make a display model for my Scene class' internal structure. Every tutorial I've seen that has a Node hierarchy assumes that the Nodes structure is maintained within the SceneModel itself. The model as a reference to the root node that's never drawn. In my example, my root node is in my separate Scene class. My implementation of SceneModel takes a required parameter in its constructor for a Scene object. Then, whenever I insert/remove rows, my overrides add/remove the respective nodes using their AddChild()/RemoveChild() methods. The problem is, now I can't update my Scene object independently of SceneModel as it'll cause a runtime exception due to the internalPointer references in SceneModel's QModelIndex list are invalid. So, I'm forced to have to use SceneModel to insert/remove rows. This means that whenever I create a Scene object, I need to have a SceneModel object pointing to that Scene object.

My solution is to have my Scene class contain an allocated pointer to my SceneModel. Then, whenever I call AddChild()/RemoveChild() on a node, it'll signal the scene that owns it to call its SceneModel's insertRows()/removeRows() methods that simply call beginInsertRows()/endInsertRows() and beginRemoveRows()/endRemoveRows() on the correct QModelIndex targets after modifying the node's hierarchy. SceneModel will obviously have a pointer to the Scene object that owns it. when dropMimeEventData() is called, instead of calling moveRows as I am now, I will just call SetParent() or MoveChildren() on the affected nodes. Then, SetParent() and MoveChildren() which will signal my Scene object to call insertRows()/removeRows()/moveRows() as necessary. Does this sound like a decent solution? The only caveat I can think of is that I'm not modifying my Scene's structure between the begin/end methods in the model.

[SGaist]

Sounds fishy, your Scene node shouldn't have to know about the model that contains it.

[TrueTrav]

That's what I originally thought, considering QAbstractItemModel's index() and parent() methods get spammed every frame. It looks like model indexes are constantly being recreated before the QTreeView gets repainted. No luck though.