Qt 5.15 - QSortFilterProxyModel with QAbstractTableModel crashing on dataChanged signal

JesusKrists

I have implemented a custom QAbstractTableModel and I have run it through the QAbstractItemModelTester and there are no more issues in my model. However, I am now trying to implement sorting through a QSortFilterProxyModel and I can't seem to get anything working at all.

void RDMSensorModels::UpdateDevice(ArtNet::ArtRdmDevice* rdmDev, const RDM::RDMProcessor::RDMDeviceModel& model, int pid) {
    if (s_RequiredPIDs.contains(pid)) {
        for (int i = 0; i < m_RDMDevices.size(); i++) {
            if (m_RDMDevices[i] == rdmDev) {
                emit dataChanged(createIndex(i, 0), createIndex(i, columnCount() - 1));
                return;
            }
        }
    }
}

This is the function, which emits the models dataChanged signal and I dont think there is a problem here, but after this signal is emitted the program crashes inside QSortFilterProxyModels internal dataChanged handler

the debugger breaks inside QSortFilterProxyModel

The weirdest thing about this is, that no matter what I pass to the dataChanged signal, the proxy_columns inside QSortFilterProxyModel is always empty.

Here you can see in the debugger, that the container is empty

If it's any help, here is my QSortFilterProxyModel implementation, its completely empty basically.

class RDMSensorSortFilterProxyModel final : public QSortFilterProxyModel {
    enum SortValue {
        MANUFACTUER_MODEL,
        UNIVERSE_DMXADDRESS,
    };

public:
    RDMSensorSortFilterProxyModel(RDMSensorModels *sourceModel, QObject *parent = nullptr) : QSortFilterProxyModel(parent) {
        setSourceModel(sourceModel);
    }

    int SortIndex();
    void SetSortIndex(int value);

protected:
    bool filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const override;
    bool lessThan(const QModelIndex &left, const QModelIndex &right) const override;

 private:
    SortValue m_SortValue = MANUFACTUER_MODEL;
};

int RDMSensorSortFilterProxyModel::SortIndex() { return m_SortValue; }

void RDMSensorSortFilterProxyModel::SetSortIndex(int value) {
    m_SortValue = static_cast<SortValue>(value);
    invalidate();
}

bool RDMSensorSortFilterProxyModel::filterAcceptsRow(int sourceRow, const QModelIndex& sourceParent) const { return true; }

bool RDMSensorSortFilterProxyModel::lessThan(const QModelIndex& left, const QModelIndex& right) const {
    auto leftDeviceManufacturer  = sourceModel()->data(left, RDMSensorModels::Roles::DeviceManufacturerRole).toString();
    auto rightDeviceManufacturer = sourceModel()->data(right, RDMSensorModels::Roles::DeviceManufacturerRole).toString();

    auto same = QString::compare(leftDeviceManufacturer, rightDeviceManufacturer, Qt::CaseInsensitive) == 0;

    return same;
}

Here are my QAbstractTableModel reimplemented functions

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

    int RDMSensorModels::rowCount(const QModelIndex& parent) const {
        if (parent.isValid())
            return 0;
        return m_RDMDevices.count();
    }

    int RDMSensorModels::columnCount(const QModelIndex& parent) const {
        if (parent.isValid())
            return 0;
        return m_ColumnCount;
    }

    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 {};
        }
    }

    QHash<int, QByteArray> RDMSensorModels::roleNames() const { return s_RoleNames; }

Any help would be greatly appreciated!

I also tried to switch out my QSortFilterProxyModel implementation with the default one, and it still crashes the same way

JonB

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

Here are my QAbstractTableModel reimplemented functions

I don't know if this is the cause, but let's start with https://doc.qt.io/qt-5/qabstracttablemodel.html#subclassing

When subclassing QAbstractTableModel, you must implement rowCount(), columnCount(), and data().

But you show no data() override. Although you say

I have implemented a custom QAbstractTableModel and I have run it through the QAbstractItemModelTester and there are no more issues in my model.

?

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.