QSortFilterProxyModel is filterning, but not sorting

[VRonin]

Difficult to tell given you use a custom proxy. can you show us what UnitFormProxyModel is?

did you set the sort role?

[dijuna]

@VRonin I've prepared minimal application showing my problem, no filtering or other app-related stuff there as it would be just too much of a code:
https://github.com/Dijuna/SortExample
It works like that: you click on the buttons to change the underlying data's values, and then you click on the header... it should sort the rows based on the buttons' state, but it does nothing. You can see sort() and lessThat() methods being called.

About the sort role, how does it work? I want my own sorting, depending on more than one role and I've thought lessThan() does it.

[VRonin]

bool out = (left.data(qint32(UserData::NO)).toBool() < right.data(qint32(UserData::NO)).toBool()); are you sure you want to use toBool there instead of comparing the variants? mydata->no is qint32 so that will return true only if left.data(qint32(UserData::NO)) is 0. is that what you want?

Also, see https://pastebin.com/H1GAgB7V for a general multi-column sort implementation

[dijuna]

@VRonin it's just a mistake made when I was preparing this example for you, it looks different in my original application and I was testing with boolean UserData::SPEAKING values anyway. So it's an non-issue, but I've fixed it. :)

Setting two buttons to "1", leaving all the others as "0", and pressing B1 header twice gives that:

sorting...
...by button1: false
...by button1: false
...by button1: false
...by button1: true
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: true
...by button1: true
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: true
...by button1: true
...by button1: true
...by button1: true
...by button1: true
sorting...
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: true
...by button1: false
...by button1: true
...by button1: true
...by button1: true
...by button1: false
...by button1: true
...by button1: true
...by button1: false
...by button1: false
...by button1: false
...by button1: false
...by button1: false

[VRonin]

Disclaimers:

• isVariantLessThan is copy-pasted from Qt sources
• The below does not mean I approve in any way your design choices in the rest of the program
• instead of using filterAcceptsColumn() (and all that comes with it) you could just call setColumnVisible() on the view and simplify the proxy even more

myproxymodel.h

#ifndef MYPROXYMODEL_H
#define MYPROXYMODEL_H

#include <QSortFilterProxyModel>

class MyProxyModel : public QSortFilterProxyModel
{
    Q_OBJECT
    Q_DISABLE_COPY(MyProxyModel)
public:
    explicit MyProxyModel(bool second_button, QObject *parent = nullptr);
    const bool second_button;
protected:
    bool filterAcceptsColumn(int source_column, const QModelIndex &sourceParent = QModelIndex()) const override;
    bool lessThan(const QModelIndex &left, const QModelIndex &right) const override;
private:
    bool isVariantLessThan(const QVariant &left, const QVariant &right, Qt::CaseSensitivity cs, bool isLocaleAware) const;
};

#endif // MYPROXYMODEL_H

myproxymodel.cpp

#include "myproxymodel.h"
#include "enums.h"
#include <QDateTime>
MyProxyModel::MyProxyModel(bool second_button, QObject *parent)
    : QSortFilterProxyModel(parent)
    ,  second_button { second_button }
{

}


bool MyProxyModel::lessThan(const QModelIndex &source_left, const QModelIndex &source_right) const
{
    qint32 srtRole;
    switch (source_left.column()) {
    case qint32(ModelColumns::NUMBER):
        srtRole = qint32(UserData::NO);
    break;
    case qint32(ModelColumns::BUTTON2):
    case qint32(ModelColumns::BUTTON1):
        srtRole=qint32(UserData::SPEAKING);
    break;
    default:
        Q_UNREACHABLE(); // Unsupportd column
    }

    const QVariant l = (source_left.model() ? source_left.model()->data(source_left, srtRole) : QVariant());
    const QVariant r = (source_right.model() ? source_right.model()->data(source_right, srtRole) : QVariant());
    return isVariantLessThan(l, r, sortCaseSensitivity(), isSortLocaleAware());
}

bool MyProxyModel::isVariantLessThan(const QVariant &left, const QVariant &right, Qt::CaseSensitivity cs, bool isLocaleAware) const
{
    if (left.userType() == QVariant::Invalid)
        return false;
    if (right.userType() == QVariant::Invalid)
        return true;
    switch (left.userType()) {
    case QVariant::Int:
        return left.toInt() < right.toInt();
    case QVariant::UInt:
        return left.toUInt() < right.toUInt();
    case QVariant::LongLong:
        return left.toLongLong() < right.toLongLong();
    case QVariant::ULongLong:
        return left.toULongLong() < right.toULongLong();
    case QMetaType::Float:
        return left.toFloat() < right.toFloat();
    case QVariant::Double:
        return left.toDouble() < right.toDouble();
    case QVariant::Char:
        return left.toChar() < right.toChar();
    case QVariant::Date:
        return left.toDate() < right.toDate();
    case QVariant::Time:
        return left.toTime() < right.toTime();
    case QVariant::DateTime:
        return left.toDateTime() < right.toDateTime();
    case QVariant::String:
    default:
        if (isLocaleAware)
            return left.toString().localeAwareCompare(right.toString()) < 0;
        else
            return left.toString().compare(right.toString(), cs) < 0;
    }
}

bool MyProxyModel::filterAcceptsColumn(int source_column, const QModelIndex &sourceParent) const
{
    Q_UNUSED(sourceParent)

    if (source_column == qint32(ModelColumns::BUTTON1)) {
        return !second_button;
    }
    else if (source_column == qint32(ModelColumns::BUTTON2)) {
        return second_button;
    }
    return true;
}

Basically the problem was that lessThan allready gets an index of the source model as input and your mapToSource implementation disregarded this

[dijuna]

@VRonin I'm trying to go with your approach. I've inherited my proxy model from your MultiProxyModel and not from its original base class. Now it looks like this (for now just testing with one column):

void UnitFormProxyModel::sort(int column, Qt::SortOrder order)
{
    clearSortPriority();
    qint32 source_column = mapToSource(this->index(0, column)).column();
    switch (source_column) {
    case qint32(ModelColumns::UNIT_NO):
        addSortPriority(column, qint32(UserData::UNIT_NO));
        break;
    default:
        Q_UNREACHABLE();
    }

    MultiSortProxyModel::sort(column, order); //renamed it for aesthetic reasons, but it's your MultyProxyModel class
}

bool UnitFormProxyModel::lessThan(const QModelIndex &left, const QModelIndex &right) const
{
    return MultiSortProxyModel::lessThan(mapToSource(left), mapToSource(right));
}

And again, all the methods are being called, but I can see no other change than the header's arrow... what am I missing? The proxy is comparing things, but not sorting them... or the view is ignoring the changes, I can't tell.

Basically the problem was that lessThan allready gets an index of the source model as input and your mapToSource implementation disregarded this

I will test that, but now I'm trying to sort the first column (with 0 as index) which is 0 for both source and proxy... and I can't see the changes anyway.

About the hideColumn() and my bad design choices - I agree, but let's solve the main issue, with ANY approach... I just want it to work. ;_;

[VRonin]

@VRonin said in QSortFilterProxyModel is filterning, but not sorting:

Basically the problem was that lessThan allready gets an index of the source model as input and your mapToSource implementation disregarded this

that means you don't have to mapToSource inside lessThan. The fact that you don't get an assertion makes me doubt the correctness of your mapToSource implementation. just delete lessThan altogether and use the base class' default

[dijuna]

@VRonin said in QSortFilterProxyModel is filterning, but not sorting:

that means you don't have to mapToSource inside lessThan. The fact that you don't get an assertion makes me doubt the correctness of your mapToSource implementation. just delete lessThan altogether and use the base class' default

I had it only to qDebug() and see the action: I knew for sure it was just sending (0, 0) indexes. Now it's deleted and nothing changed... still, my mapToSource() is called somewhere anyway and it's changing

QModelIndex(0,0,0x0,UnitFormsTableModel(0x1705e268))

to

QModelIndex(0,0,0x1d762d68,UnitFormProxyModel(0x1c626940)).

Is that okay?

[VRonin]

It is but something might go wrong if you reimplemented mapToSource()

[dijuna]

@VRonin said in QSortFilterProxyModel is filterning, but not sorting:

It is but something might go wrong if you reimplemented mapToSource()

How? If the output is as it should be... it's const method, it can't break things, right? Still, I've tested it and you were right: generic mapToSource() and mapFromSource() methods have exactly the same outputs as mine (I don't know why I thought I need to create them) and so I've deleted my methods and now everything is as it should be... thank you very much! Still, I'll check the Qt code to see what was the difference...