Reimplementing QFileSystemModel Checkboxes using QMap inconsistent results

moffa13

Hello, I'm trying to display a root directory with expandable children in a QTreeView.
I'm using a QFileSystemModel. Everything was working fine, I reimplemented data, setData and flags to add checkboxes to the first column.

To achieve that I had QSet<QPersistentModelIndex> which retains the checked boxes (set.contains(index) is true).
However I wanted to have an intermediate state which is Qt::partiallyChecked. To do that, I had to change QSet to a QMap<QPersistantModelIndex, Qt::CheckState> but now, everytime I run this code, different checkboxes get checked like random even if there's nothing random, data should always return the same. Here's the code simplified to its maximum :

#include "QCheckableFileSystemModel.h"
#include <QAbstractItemModel>
#include <QStack>
#include <QtDebug>

QCheckableFileSystemModel::QCheckableFileSystemModel(QObject* parent) : QFileSystemModel (parent)
{}

QVariant QCheckableFileSystemModel::data(const QModelIndex &index, int role) const{
	// Item is disabled or it's a column we don't need
	if((index.flags() & Qt::ItemIsEnabled) != Qt::ItemIsEnabled || index.column() != 0){
		return QVariant{};
	}

	if(role == Qt::CheckStateRole && index.column() == 0){ // We need to return if item is checked or not
		int checked = _indexesCheckedStates.contains(index) ? _indexesCheckedStates[index] : Qt::Unchecked;
		return checked;
	}
	return QFileSystemModel::data(index, role); // Default behavior
}

bool QCheckableFileSystemModel::setData(const QModelIndex &index, const QVariant &value, int role){

	if(role == Qt::CheckStateRole && index.column() == 0){

		_indexesCheckedStates[index] = value.toInt();

		emit dataChanged(index, index);
		return true;
	}
	return QFileSystemModel::setData(index, value, role);
}


Qt::ItemFlags QCheckableFileSystemModel::flags(const QModelIndex &index) const{
	return QFileSystemModel::flags(index) | Qt::ItemIsUserCheckable;
}

And here's the code which calls setData :

QPersistentModelIndex rootIndex = _sortModel->mapFromSource(_model->setRootPath(_installDir.absolutePath()));

// Wait for model to load
QEventLoop loop;
connect(_model, &QCheckableFileSystemModel::directoryLoaded, &loop, &QEventLoop::quit);
loop.exec();


QStack<QPersistentModelIndex> toProcess;
toProcess.push(rootIndex);

while(!toProcess.empty()){
	QPersistentModelIndex toTreat = toProcess.pop();
	if(_model->isDir(_sortModel->mapToSource(toTreat)) && _sortModel->rowCount(toTreat) > 0){
                // this returns a set of paths which should be checked in the parent (always returns correct data)
		QSet<QString> neededFiles = detectNeededFiles(
					_model->filePath(_sortModel->mapToSource(toTreat)),
					detectedMods,
					true
		);
		for(int i = 0; i < _sortModel->rowCount(toTreat); ++i){
			QPersistentModelIndex child{_sortModel->index(i, 0, toTreat)};
			if(neededFiles.contains(_model->filePath(_sortModel->mapToSource(child)))){
				qDebug() << _model->filePath(_sortModel->mapToSource(child));
				_model->setData(_sortModel->mapToSource(child), Qt::Checked, Qt::CheckStateRole);
			}
			if(_model->isDir(_sortModel->mapToSource(child))){
				_model->discover(_sortModel->mapToSource(child));
				toProcess.push(child);
			}

		}
	}
}

Here, _sortModel is the proxyfilter and _model my custom QFileSystemModel

Note that I have a QSortFilterProxyModel to sort directories I don't want to see since it can't be done using setNamesFilter

So I don't know what could cause this, maybe a race condition ? I know that QFileSystemModel loads its directories in a different thread but I always wait for the signal directoryLoaded (that's what discover does)

void QCheckableFileSystemModel::discover(QModelIndex const& index){                           
	while(canFetchMore(index)){
		QEventLoop loop;
		connect(this, &QFileSystemModel::directoryLoaded, &loop, &QEventLoop::quit);
		fetchMore(index);
		loop.exec();
	}
}

Thank you for your answers !

JonB

@moffa13

different checkboxes get checked like random even if there's nothing random

QVariant QCheckableFileSystemModel::data(const QModelIndex &index, int role) const{
...
return checked;

I am not a C++er, so this is either very right or very wrong! Are you supposed to return an int for a QVariant? Because if not this could produce "random" state value for your checkboxes....

[EDIT: Looks like my suggestion was very wrong, sigh :( ]

mrjj

@JonB
Very good question, but i think it will return a QVariant constructed from the
int checked variable. Else compiler should be very unhappy if no automatic
conversion can happen.

moffa13

I am supposed to return a Qt::CheckState elem from enum which is int.

Yes the compiler constructs a QVariant from the int value.

So, any idea about what's going wrong here ?

moffa13

Any help please

mrjj

@moffa13
Add some qDebug() statement to see what index are sent and what it set pr index.
I think its one of those cases where debugger and tools are more useful than guessing looking at code.

moffa13

@mrjj said in Reimplementing QFileSystemModel Checkboxes using QMap inconsistent results:

@moffa13
Add some qDebug() statement to see what index are sent and what it set pr index.
I think its one of those cases where debugger and tools are more useful than guessing looking at code.

If it was that simple I wouldn't have asked.

qDebug is showing me correct paths before calling setData. The problem is that for whatever reason, some checkboxes won't get checked. I almost always get a different result whenever I run this code.

qDebug in setData tells me that the function receives Qt::Checked but it's not the case.

mrjj

@moffa13
Hi
Hmm, i would suspect the QSet<QPersistentModelIndex>
could contain incorrect indexes.
Im wondering if QFileSystemModel might invalidate QPersistentModelIndexes i have seen with QSqlTableModel.

moffa13

@mrjj It only has this behavior when I use QMap if I do this with QSet everything works fine. That's what I don't get

mrjj

@moffa13
Hmm. yes that is odd.
One difference with Set/QMap is if u ask QMap for non existing value
a default-constructed value is returned but you seem to check with contains so
should not happen?

moffa13

@mrjj

Yeah, exactly, for me it should work.

However I noticed something which may be useful. I added a QEventLoop when doing setData to look for the dataChanged signal but when I do this and I use qDebug to show the files returned by model::index, I get something like this :

Normal dir is
a.txt
b.txt
c.txt

filePath in the loop returns this :

a.txt
b.txt
b.txt

And c.txt does not get checked which is also odd.

mrjj

@moffa13
Hmm. you know how b.txt is included twice ?

moffa13

@mrjj this is absolutely not normal. It' a for loop iterating from 0 to rowCount(parent). Then it uses model->index(i, 0, parent). So this is not possible to output twice the same file.

mrjj

@moffa13
Could you try to do the same for loop from say a button and use
no localEvent loop or anything like that and see if its
reproducible in other context ?

moffa13

Ok I found what was causing this awful bug.

When I check if the map contains the index, I actually check the raw index not the QPersistantModelIndex so this is not the same object and I think qmap does not check equality using == operator .

So I have to iterate over the map and check using QPersistantModelIndex "==" operators which can compare from a QModelIndex.