Support for QStandardItem proxies?

elfring

I have tried out to append a text by a QIdentityProxyModel subclass to a member variable of a custom data structure.
Unfortunately, the display of one view from my test program indicates that the function “QVariant::canConvert” did not provide the result which I expected here.
Would you like to clarify any affected implementation details?

VRonin

They try to display something and delegate edit attempts.

I disagree, or at least it's not 100% accurate.

Items are connected with their data models.

Anything said above actually applies to any data model (QAbstractItemModel subclass), regardless of the implementation

the display of one view from my test program

can you post your code?

elfring

can you post your code?

I am curious on how a code review will evolve for my test program.
The forum software gave me the information “Uploading 100%” and “You do not have enough privileges for this action.”.

elfring

Another try for a possible code review …

main.cpp:

#include <QApplication>
#include <QtUiTools/QUiLoader>
#include <QFile>
#include <QString>
#include <QWidget>
#include <QListView>
#include <QStandardItemModel>
#include <QIdentityProxyModel>
#include <QMetaType>
#include <iostream>
#include <stdexcept>
#include <cstdio>
#include <cstdlib>

struct my_message
{
    QString text;

    my_message(char const * s = "ABC")
    : text(s)
    { }

    my_message(my_message const& s)
    : text(s.text)
    { }

    ~my_message()
    { }
};
Q_DECLARE_METATYPE(my_message);

class my_item : public QStandardItem
{
public:
    my_item(my_message const mm)
    { setData(QVariant::fromValue(mm), Qt::DisplayRole); }
};

class my_proxy : public QIdentityProxyModel
{
    QVariant data(QModelIndex const & index, int role) const override
    {
        if (role != Qt::DisplayRole)
            return QIdentityProxyModel::data(index, role);

        auto x(sourceModel()->data(index));
        if (x.canConvert<my_message>())
        {
            my_message mm(x.value<my_message>());
            mm.text.append("||");
            return QVariant::fromValue(mm);
        }
        else
        {
            return "***";
        }
    }
};

struct my_views : public QWidget
{
    Q_OBJECT
    QListView* v1;
    QListView* v2;

public:
    explicit my_views(QWidget* parent = nullptr)
    : QWidget(parent)
    {
//         QFile f(":/form.ui");
        QFile f("/home/elfring/Projekte/view-test2/form.ui");
        if (!f.open(QFile::ReadOnly))
            throw std::runtime_error(tr("UI file could not be opened.").toStdString());

        QUiLoader l;
        QWidget* w = l.load(&f, this);
        if (!w)
            throw std::runtime_error(l.errorString().toStdString());

        f.close();

        v1 = findChild<QListView*>("V1");
        if (!v1)
            throw std::runtime_error(tr("First view was not found.").toStdString());

        v2 = findChild<QListView*>("V2");
        if (!v2)
            throw std::runtime_error(tr("Second view was not found.").toStdString());

        auto i1(new QStandardItem("abc"));
        auto i2(new my_item("XYZ"));
        auto m1(new QStandardItemModel);
        auto m2(new QStandardItemModel);
        auto mp(new my_proxy);
        m1->appendRow(i1);
        m2->appendRow(i2);
        mp->setSourceModel(m2);
        v1->setModel(m1);
        v2->setModel(mp);
    }
};

#include "main.moc"

int main(int argc, char** argv)
{
    try
    {
        QApplication app(argc, argv);
        qRegisterMetaType<my_message>("my_message");
        my_views mv;
        mv.show();
        return app.exec();
    }
    catch (std::bad_alloc const& b)
    {
        (void) std::fputs("out of memory\n", stderr);
        return EXIT_FAILURE;
    }
    catch (std::exception const& x)
    {
        (void) std::fprintf(stderr, "A kind of standard%s\n%s\n", " exception was caught.", x.what());
        return EXIT_FAILURE;
    }
    catch (...)
    {
        (void) std::fprintf(stderr, "An unknown%s\n", " exception was caught.");
        throw;
    }
}

CMakeLists.txt:

cmake_minimum_required(VERSION 3.1)
project(view-test CXX)
set(CMAKE_AUTOMOC ON)
find_package(Qt5 COMPONENTS Core Widgets UiTools REQUIRED)
include_directories(${Qt5Widgets_INCLUDE_DIRS} ${Qt5Core_INCLUDE_DIRS} ${Qt5UiTools_INCLUDE_DIRS})
add_executable(test1 main.cpp)
target_link_libraries(test1 Qt5::Widgets Qt5::Core Qt5::UiTools)

form.ui:

<?xml version="1.0" encoding="UTF-8"?>
<ui version="4.0">
 <class>my_views</class>
 <widget class="QWidget" name="my_views">
  <property name="geometry">
   <rect>
    <x>0</x>
    <y>0</y>
    <width>282</width>
    <height>228</height>
   </rect>
  </property>
  <property name="windowTitle">
   <string>Display test</string>
  </property>
  <widget class="QListView" name="V1">
   <property name="geometry">
    <rect>
     <x>10</x>
     <y>10</y>
     <width>256</width>
     <height>91</height>
    </rect>
   </property>
   <property name="uniformItemSizes">
    <bool>true</bool>
   </property>
  </widget>
  <widget class="Line" name="L1">
   <property name="geometry">
    <rect>
     <x>10</x>
     <y>100</y>
     <width>241</width>
     <height>20</height>
    </rect>
   </property>
   <property name="lineWidth">
    <number>9</number>
   </property>
   <property name="orientation">
    <enum>Qt::Horizontal</enum>
   </property>
  </widget>
  <widget class="QListView" name="V2">
   <property name="geometry">
    <rect>
     <x>10</x>
     <y>120</y>
     <width>256</width>
     <height>91</height>
    </rect>
   </property>
   <property name="uniformItemSizes">
    <bool>true</bool>
   </property>
  </widget>
 </widget>
 <resources/>
 <connections/>
</ui>

VRonin

does if (x.canConvert<my_message>()) return false?
what is your intention in this code snippet?

my_message mm(x.value<my_message>());
mm.text.append("||");
return QVariant::fromValue(mm);

You are leaking all 3 of the models. The view does not own the model

elfring

does if (x.canConvert<my_message>()) return false?

I get this impression after I see three asterisks in the display from my test widget on the screen.

what is your intention in this code snippet?

I am trying also to get more familiar with the provided programming interfaces around models and views.

You are leaking all 3 of the models.

I do not really need my own clean-up for this software experiment at the moment.

The view does not own the model

Thanks for your reminder.

Do you spot any other suspicious implementation details?

elfring

does `if (x.canConvert<my_message>()) return false?

I have added a test output for my local variable “x”. The programming interface “qDebug()” provides the information “QVariant(Invalid)” then so far.

Would you like to help with finding an explanation for this software behaviour?
How can a source model work here?

Christian Ehrlicher

@elfring said in Support for QStandardItem proxies?:

auto x(sourceModel()->data(index));

How should this work at all? http://doc.qt.io/qt-5/qidentityproxymodel.html#mapToSource

VRonin

Good spot, I'm surprised it didn't just assert. I got shouted at in code review for having models that didn't assert when index.model()!=this

@elfring replace auto x(sourceModel()->data(index)); with auto x=QIdentityProxyModel::data(index, role);

Christian Ehrlicher

@VRonin said in Support for QStandardItem proxies?:

I'm surprised it didn't just assert

Maybe because Qt was not compiled in debug mode. Or there is no assert and the new index check stuff is not yet used there - feel free to add it ;)

elfring

replace auto x(sourceModel()->data(index)); with auto x=QIdentityProxyModel::data(index, role);

Thanks for this suggestion.

Unfortunately, I selected a questionable member function call combination before.
The test case is working as expected together with the statement “return mm.text;” now.

elfring

Good spot, I'm surprised it didn't just assert.

Did I try the usage of model indexes out in appropriate way for my test case?
Do indexes need to be different for proxy and source models?

VRonin

@elfring said in Support for QStandardItem proxies?:

Did I try the usage of model indexes out in appropriate way for my test case?

Usually you'd want a Q_ASSERT that uses checkIndex()

Do indexes need to be different for proxy and source models?

Yes, obviously

elfring

Yes, obviously

Should the difference between indexes for proxy and source models be better described in the Qt documentation?
How do you think about the introduction of a class like QProxyModelIndex then?

VRonin

@elfring said in Support for QStandardItem proxies?:

How do you think about the introduction of a class like QProxyModelIndex then?

QAbstractPorxyModel::mapToSource/QAbstractPorxyModel::mapFromSource already do everything that class would do so I see no gain in introducing it

Should the difference between indexes for proxy and source models be better described in the Qt documentation?

A proxy model is still a model, the docs say "Note that it's undefined behavior to pass illegal indices to item models" so I feel it is documented already

elfring

A proxy model is still a model,

This information is generally appropriate.

the docs say "Note that it's undefined behavior to pass illegal indices to item models"

A constraint is mentioned.

so I feel it is documented already

Now I imagine that a better class design can prevent the passing of inappropriate indexes a bit more.
How are the chances to specify that QAbstractProxyModel-like objects need to work with QProxyModelIndex objects instead?

VRonin

@elfring said in Support for QStandardItem proxies?:

Now I imagine that a better class design can prevent the passing of inappropriate indexes a bit more.

Agree that's why I said:

I'm surprised it didn't just assert.

How are the chances to specify that QAbstractProxyModel-like objects need to work with QProxyModelIndex objects instead?

QAbstractProxyModel is a subclass of QAbstractItemModel and the view should not care whether the model it's attached to is a proxy or not

elfring

the view should not care whether the model it's attached to is a proxy or not

Can overloaded functions (from a specific proxy class) help any more to detect questionable indexes?

VRonin

@elfring said in Support for QStandardItem proxies?:

Can overloaded functions (from a specific proxy class) help any more to detect questionable indexes?

It can help but there's no bullet proof solution.

In the meantime I submitted a change to QStandardItemModel that would have made your bug assert in debug

elfring

In the meantime I submitted a change to QStandardItemModel that would have made your bug assert in debug

I find it interesting that my clarification request triggered such a software evolution finally.
Would you like to continue the development discussion also around proxies for QStandardItem (or eventually other classes)?