Qt 5.15 - QSortFilterProxyModel with QAbstractTableModel crashing on dataChanged signal

[JesusKrists]

@JonB I am sorry, but I just copy and pasted this from my original question, did you miss it?

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

        int deviceIndex = index.row();

        switch (role) {
            case SensorGraphReadingsRole: {
                auto& readings  = m_RDMDevices[deviceIndex]->Sensors()[index.column() - 1]->LastReadings();
                auto maxElement = f_SensorMaxReading(index.row(), index.column() - 1);
                auto minElement = f_SensorMinReading(index.row(), index.column() - 1);

                QVariantList values;
                for (int i = 0; i < readings.size(); i++) {
                    values.push_back(Utils::Math::map(readings[i], maxElement, minElement, 0, 1));
                }
                return values;
            }
            case SensorMinReadingRole: return f_SensorMinReading(deviceIndex, index.column() - 1);
            case SensorMaxReadingRole: return f_SensorMaxReading(deviceIndex, index.column() - 1);

            case DeviceUIDRole: return f_DeviceUIDString(deviceIndex);
            case DeviceUniverseRole: return f_DeviceUniverseString(deviceIndex);
            case DeviceLabelRole: return f_DeviceLabelString(deviceIndex);
            case DeviceManufacturerRole: return f_DeviceManufacturerString(deviceIndex);
            case DeviceModelRole: return f_DeviceModelString(deviceIndex);

            case SensorRangeMaxValueRole: return f_SensorRangeMaxValueString(deviceIndex, index.column() - 1);
            case SensorRangeMinValueRole: return f_SensorRangeMinValueString(deviceIndex, index.column() - 1);
            case SensorCurrentValueRole: return f_SensorCurrentValueString(deviceIndex, index.column() - 1);
            case SensorNameRole: return f_SensorNameString(deviceIndex, index.column() - 1);
            case SensorCurrentValueNormalizedRole: return f_SensorCurrentValueNormalized(deviceIndex, index.column() - 1);
            case SensorMinNormalValueNormalizedRole: return f_SensorMinNormalValueNormalized(deviceIndex, index.column() - 1);
            case SensorMaxNormalValueNormalizedRole: return f_SensorMaxNormalValueNormalized(deviceIndex, index.column() - 1);

            case SensorValidRole: {
                auto sensorCount = f_DeviceSensorCount(deviceIndex);
                return sensorCount && (index.column() <= sensorCount);
            }
            default: return {};
        }
    }

[JonB]

@JesusKrists
Damn, sorry, I failed to see scrollbar!

[JesusKrists]

@JonB No problem haha. This issue is driving me crazy as I can't seem to find anything wrong, or anything existing already documenting this.

[JonB]

@JesusKrists
I don't know. I have done my own abstract model stuff and my own sort filter stuff and I did get there OK.

Firstly since you say it goes wrong with the base QSortFilterProxyModel I'd test with just that. Or only override one function. I'd reduce your code down & down, and I'd step/examine with debugger, till I found the issue/get it right. Just as a *for example, if you have a minimal lessThan() override with a breakpoint does it ever get called? I think a common issue is that you get mixed up and use a source model index into the proxy model or vice versa. Only debugging hints, that's all i can think of.

[SGaist]

Hi and welcome to devnet,

Your lessThan function looks a bit surprising. Your QString::compare looks rather like something that should be done in the filterAcceptsRow method.

lessThan goal is to allow the model to know what value is considered smaller between the two passed as parameters. Your implementation compares the content of the strings for equality which may never happen.

[JonB]

@SGaist
Indeed, good spot!

@JesusKrists
In fact, your lessThan always returns false, unless two strings are the same, in which case they are not "less than", but you return true.

The lessThan always returning false may end up confusing the sort proxy....

And btw looking at yours you may not to override this at all, in QSortFilterProxyModel you can specify the sort role to be used and case-insensitive comparison via methods already there.

[JesusKrists]

@SGaist You are correct about that. However, my problem still persists as i tried not using my own QSortFilterProxyModel subclass, but using the original one like so:

s_RDMSensorProxyModel = new QSortFilterProxyModel();
s_RDMSensorProxyModel->setSourceModel(s_RDMSensors);

And it crashed after emitting dataChanged signal again, same place, same line. Not even parsing any data out of the proxy model. just emiting dataChanged from my model crashes.

[JesusKrists]

I just did a test where I immediately emitted dataChanged signal after adding a row like so

void RDMSensorModels::Add(const ArtNet::ArtRdmDevice* rdmDevice) {
    if (!m_RDMDevices.contains(rdmDevice)) {
        beginInsertRows(QModelIndex(), rowCount(), rowCount());
        m_RDMDevices.push_back(rdmDevice);
        endInsertRows();
        emit dataChanged(createIndex(0, 0), createIndex(0, columnCount() - 1));
    }
}

It added the first row perfectly. However it crashed on the second row add.

Crashed at the same place again

[Christian Ehrlicher]

@JesusKrists said in Qt 5.15 - QSortFilterProxyModel with QAbstractTableModel crashing on dataChanged signal:

RDMSensorModels::Add(const ArtNet::ArtRdmDevice* rdmDevice)

This looks fishy. Are you sure your pointer is valid the whole lifetime? I would guess no.

Please provide a minimal, compilable example.

[JesusKrists]

@Christian-Ehrlicher The pointer is valid the whole lifetime of the program, however, I cant seem to create a minimal compileable example, it seems when i create a test case, it doesn't crash anymore and im scratching my head trying to understand what is different in this scenario

#pragma once

#include <QAbstractTableModel>

class TestModel : public QAbstractTableModel {
    Q_OBJECT
public:
    explicit TestModel(QObject* parent = nullptr) : QAbstractTableModel(parent) {}

    Q_INVOKABLE QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override {
        if (section < 1)
            return QString("Device");
        else
            return QString("Sensor %1").arg(section);
    }

    int rowCount(const QModelIndex& parent = QModelIndex()) const override {
        if (parent.isValid())
            return 0;
        return m_Values.count();
    }

    int columnCount(const QModelIndex& parent = QModelIndex()) const override {
        if (parent.isValid())
            return 0;
        return m_ColumnCount;
    }
    QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override { return QVariant(); }

    void Add(const int* value) {
        if (!m_Values.contains(value)) {
            beginInsertRows(QModelIndex(), rowCount(), rowCount());
            m_Values.push_back(value);
            endInsertRows();
            emit dataChanged(index(0, 0), index(0, columnCount() - 1));
        }
    }

private:
    QVector<const int*> m_Values;
    int m_ColumnCount = 9;
};

#include "QApplication"
#include "QQuickWindow"

#include "Testing/TestModel.h"

int main(int argc, char **argv) {
    QGuiApplication::setAttribute(Qt::AA_UseOpenGLES);
    QGuiApplication::setAttribute(Qt::AA_EnableHighDpiScaling);
    //QQuickWindow::setTextRenderType(QQuickWindow::NativeTextRendering);

    QLoggingCategory::setFilterRules(QStringLiteral("qt.qml.binding.removal.info=true"));
    QLoggingCategory::setFilterRules(QStringLiteral("qt.scenegraph.general=true"));

    QApplication app(argc, argv);

    auto mainWindow = new QMainWindow();
    mainWindow->resize(1280, 720);
    mainWindow->show();

    auto testModel  = new TestModel();
    auto proxyModel = new QSortFilterProxyModel();
    proxyModel->setSourceModel(testModel);

    auto testTimer = new QTimer();
    QObject::connect(testTimer, &QTimer::timeout, [&]() {
        testModel->Add(new int(1));
        testModel->Add(new int(2));
        testModel->Add(new int(3));
    });

    testTimer->setInterval(1000);
    testTimer->start();

    return app.exec();
}

[JesusKrists]

Well it turns out, trying to replicate the issue on a smaller scale made my brain neurons fire enough, that i figured out the problem. My model column count can change and it does change, however, I had not written anything that notifies about column count changing beginRemoveColumns and endRemoveColumns and beginInsertColumns and endInsertColumns. I implemented those in my code like so

void RDMSensorModels::UpdateColumnCount() {
        int sensorCount = 1;
        for (auto device : m_RDMDevices) {
            int deviceSensorCount = device->Sensors().size();
            if (deviceSensorCount + 1 > sensorCount)
                sensorCount = deviceSensorCount + 1; // +1 for device column
        }

        if (m_ColumnCount != sensorCount) {
            if (m_ColumnCount < sensorCount) {
                beginInsertColumns(QModelIndex(), m_ColumnCount, sensorCount - 1);
                m_ColumnCount = sensorCount;
                endInsertColumns();
            } else {
                beginRemoveColumns(QModelIndex(), sensorCount, m_ColumnCount - 1);
                m_ColumnCount = sensorCount;
                endRemoveColumns();
            }
        }
    }

And the proxy model now works as expected. Hopefully this helps anyone else having issues with QSortFilterProxyModel.

It's interesting to note that the QAbstractItemModelTester did not catch this problem as I would have expected it to as my model changes column count depending on the largest sensor count for devices currently found.