view not updating automatically

mzimmers

Hi all -

I'm coding a QML (grid) view that gets populated by a C++ model. In my app, I've used the approach shown in this video for several models, and most seem to be working OK, but I'm having trouble with this one.

The problem is that the view doesn't automatically display when the model changes.

I'll let the code do the talking; I've removed all but the essentials. Can someone see a step I'm missing here?

The class hierarchy:

typedef QList<QUuid> uuidList;

class Zone {
    Q_GADGET
    uuidList m_equipmentList;

public:
    Q_PROPERTY(uuidList equipmentList MEMBER m_equipmentList)
    uuidList equipmentList() const { return m_equipmentList; }

class ZoneList : public QObject
{
    Q_OBJECT
private:
    QList<Zone> m_list;
public: 
    Zone getItem(int index) { return m_list.at(index); }

class ZoneModel: public QAbstractListModel
{
    Q_OBJECT
private:
    ZoneList *m_list;
public:
    Q_INVOKABLE Zone getZone(int index) { return m_list->getItem(index); }

And the QML:

ColumnLayout {
    id: mainGrid
    property int zoneSelected: 0

    SpacesBar { // something I made to change zoneSelected.
        onButtonClicked: (i) => {
            zoneSelected = i;
                         }
    }

    GridView {
        id: equipmentView

        model: (zoneSelected === 0)
               ? equipmentModel.uuidList()
               : zoneModel.getZone(zoneSelected).equipmentList
        delegate: EquipmentCard {
        ...

Thanks for looking...

mzimmers

I managed a workaround for this -- In my equipment model, I added an emit to the setData() function:

void EquipmentModel::setList(EquipmentList *list)
{
    beginResetModel();
    if (m_list != nullptr) {
        m_list->disconnect(this);
    }

    m_list = list;

    if (m_list != nullptr) {
        connect(m_list, &EquipmentList::preItemAppended, this, [=]() {
            const int index = m_list->equipment().size();
            beginInsertRows(QModelIndex(), index, index);
        });
        connect(m_list, &EquipmentList::postItemAppended, this, [=]() {
            emit uuidListChanged(uuidList());
            endInsertRows();
        });

        connect(m_list, &EquipmentList::preItemRemoved, this, [=](int index) {
            beginRemoveRows(QModelIndex(), index, index);
        });
        connect(m_list, &EquipmentList::postItemRemoved, this, [=]() {
            endRemoveRows();
        });
    }
    endResetModel();
}

Though I'm not sure why this is necessary, because as @SGaist implied above, the endInsertRows() should have emitted this signal, right?

SGaist

Hi,

Might be a silly question but when you update your model, do you call dataChanged ?

Also, how do you update it ?

mzimmers

@SGaist not a silly question at all. My ZoneModel has a setData() method which calls dataChanged():

bool ZoneModel::setData(const QModelIndex &index, const QVariant &value, int role)
{
    bool rc = true;
    do {
        if (m_list == nullptr) {
            continue;
        }
        Zone item = m_list->zones().at(index.row());
        switch (role) {
        case EquipmentListRole:
            item.setEquipmentList(value.value<uuidList_t>());
            break;
        default:
            rc = false;
        }
        if (rc) {
            if (m_list->setItemAt(index.row(), item)) {
                emit dataChanged(index, index, QVector<int>() << role);
            } else {
                rc = false;
            }
        }
    } while (false);
    return rc;
}

(This is patterned after the example in the video.)

The updates to the model occur upon message processing, and result in calls to a setItemAt(index, item) in the List class. Again, this is from the video.

JonB

@mzimmers
Looks OK to me, but I would temporarily put in a qDebug() above emit dataChanged() to ensure m_list->setItemAt() is indeed returning true.

SGaist

That code is a bit surprising. Typically that do loop is completely useless. It's only purpose is to break early if m_list is null which could be done by simply:

if (m_list == nullptr) {
    return false;
}

Are you calling setData from your external processing function ?

mzimmers

@JonB based on @SGaist 's questions, I looked a little more closely at my code. I notice that while my setData() method contains the dataChanged() signal, my routine that updates the model doesn't use the setData() method:

zoneIndex = m_list->findItem(zoneUuid);
// if this zone isn't in the list, add it.
if (zoneIndex == NOT_IN_LIST) {
    appendItem(z);
    // if this zone is in the list, replace it.
} else {
    if (m_list->setItemAt(zoneIndex, z)) {
    }                        
}

This routine doesn't have the context of a model index or a role...not sure where to go from there.

mzimmers

@SGaist said in view not updating automatically:

Are you calling setData from your external processing function ?

Bingo.

But how do I know what my QModelIndex and role should be?

JonB

@mzimmers
Well for the role you're updating the data value, right? So default for setData(), or Qt::EditRole. That is assuming QML roles don't come in here coz I don't know any QML.

For the QModelIndex it's whatever corresponds to setItemAt(zoneIndex) (or appendItem()) in the model!

SGaist

@mzimmers since it's a list, zoneIndex is the row and if you need to add a new row you should use the begin/endInsertRows functions so the update will be triggered automatically.

JoeCFD

@SGaist break is a better option. continue is misleading since there is only one loop.

if (m_list == nullptr) {
    break;
}

I guess that default return value is true in his code and return false would be wrong.

SGaist

@JoeCFD my suggestion is to dump that useless loop. As for the return value, you are correct if following the original logic. I would argue that it depends whether the nullptr is an expected value. For me, trying to set data when the container is not present would rather show that there's a logic issue somewhere.

mzimmers

@SGaist (still looking at your other suggestion.)

The do {} while (false) loop is a coding convenience. I don't like having multiple return statements in my code, and there are times when I have so many things to test, if I did it with nested if statements, the code would disappear off to the right of the page.

This approach might seem odd to some, but for me, it's the preferred way to handle such cases.

mzimmers

@SGaist said in view not updating automatically:

if you need to add a new row you should use the begin/endInsertRows functions

~~Interesting...I don't believe the example in the video used those functions.~~ The list object appendItem function does do this, though:

bool ZoneList::appendItem(Zone item)
{
    emit preItemAppended();
    m_list.append(item);
    emit postItemAppended();
}

and in my model code:

void ZoneModel::setList(ZoneList *list)
{
    beginResetModel();
    if (m_list != nullptr) {
        m_list->disconnect(this);
    }
    m_list = list;

    if (m_list != nullptr) {
        connect(m_list, &ZoneList::preItemAppended, this, [=]() {
            const int index = m_list->zones().size();
            beginInsertRows(QModelIndex(), index, index);
        });
        connect(m_list, &ZoneList::postItemAppended, this, [=]() {
            endInsertRows();
        });
    }
    endResetModel();
}

So I think it's accomplishing the same thing.

JonB

@mzimmers
I don't think it has anything to do with your present issue, but the code ends up returning true for "successfully changed data" when the list doesn't exist. One would think that returning false here would make more sense. Or initialize your rc to false, set it true when you do something.

JoeCFD

@mzimmers I use it too. It is very convenient in some scenarios. It is kind of different that default value is true. Return true does not seem to mean setting is successful.

mzimmers

@JonB that's a good point; I'll probably make that change. Right now, though, that function's never getting called, so I'll leave it be.

Also, what might be getting lost in this discussion is that most of the ZoneModel updates are working. What is not working is the updating of the equipmentList member of the Zone.

If this helps, the heirarchy is like this:

ZoneModel {
    ZoneList {
        QList<Zone>
        
Zone {
    QList<QUuid> equipmentUuids
    
EquipmentModel {
    EquipmentList {
        EquipmentItem {
            QUuid uiud
            Qstring name
            other equipment-specific information

So, I need to:

access the ZoneModel
traverse its list of zones
for each zone, get its list of Uuids
for each equipment Uuid, traverse the EquipmentModel to get the additional information about the equipment item.

I realize this is rather complex, and I'm grateful for any suggestions.

mzimmers

I think I'm actually fairly close on this, and in fact, the C++ portion of the effort may be working fine. In plain prose, here's what I want to do:

for each zone in the ZoneModel:

get the equipment list for that zone
for each equipment list item:
2. extract the UUID of that item
3. find the item in the EquipmentModel using the UUID above
4. extract the name of that item

Here's a snippet of the zone-related code:

class Zone {
    Q_GADGET
    equipmentUuidList m_equipmentList;
public:
    Q_PROPERTY(equipmentUuidList equipmentList MEMBER m_equipmentList)
}

class ZoneModel: public QAbstractListModel
{
    Q_OBJECT
private:
    ZoneList *m_list;
public:
    Q_INVOKABLE Zone getZone(int index); // gets Zone from list
}

and the equipment-related code:

class EquipmentItem
{
    Q_GADGET
    Q_PROPERTY(QUuid uuid MEMBER m_uuid READ uuid WRITE setUuid)
    Q_PROPERTY(QString name MEMBER m_name)

    QUuid m_uuid;
    QString m_name;
public:
    Q_INVOKABLE QUuid uuid() const { return m_uuid; };
    Q_INVOKABLE QString name() const { return m_name; }
}

class EquipmentModel : public QAbstractListModel
{
    Q_OBJECT
    Q_PROPERTY(EquipmentList *list READ list WRITE setList NOTIFY listChanged)
private:
    EquipmentList *m_list; // a list of EquipmentItem objects
public:
    EquipmentList *list() { return m_list; }
    Q_INVOKABLE EquipmentItem getItem(QUuid uuid);
}

And my QML:

ListView {
id: scenesView
model: zoneModel
delegate: Column {
    property int modelIndex: index
    property var equipList: zoneModel.getZone(index).equipmentList
    Text {
        text: "zone name is " + name
    }
    Text {
        text: "zone UUID is " + uuid
    }

    ListView {
        id: equipment
        model: equipList
        delegate: Text {
            text: "zone equipment name is " + equipmentModel.getItem(WHAT GOES HERE?).name()
        }
    }
}

So...how do I use the uuid extracted in the outer view, as a parameter to getItem() in the inner view?

Thanks for reading...

mzimmers

Well, as I suspected, my QML was bad. I got that much figured out:

ListView {
    model: zoneModel
    delegate: Column {
        id: infoColumn
        property int zoneIndex: index
        property var equipList: (index === 0)
                                ? equipmentModel.uuidList()
                                : (index > 0)
                                  ? zoneModel.getZone(index).equipmentList
                                  : null
        ListView {
            height: 80
            width: 200
            model: (zoneIndex === 0)
                   ? equipmentModel.uuidList()
                   : (zoneIndex > 0)
                     ? equipList
                     : null
            delegate: Text {
                property var equipUuid: infoColumn.equipList[index]
                text: "zone equipment name is " + equipmentModel.getEquipment(equipUuid).name()
            }
        }
    }
}

Unfortunately, the auto-update still isn't working. I'll post more on this later.

mzimmers

I managed a workaround for this -- In my equipment model, I added an emit to the setData() function:

void EquipmentModel::setList(EquipmentList *list)
{
    beginResetModel();
    if (m_list != nullptr) {
        m_list->disconnect(this);
    }

    m_list = list;

    if (m_list != nullptr) {
        connect(m_list, &EquipmentList::preItemAppended, this, [=]() {
            const int index = m_list->equipment().size();
            beginInsertRows(QModelIndex(), index, index);
        });
        connect(m_list, &EquipmentList::postItemAppended, this, [=]() {
            emit uuidListChanged(uuidList());
            endInsertRows();
        });

        connect(m_list, &EquipmentList::preItemRemoved, this, [=](int index) {
            beginRemoveRows(QModelIndex(), index, index);
        });
        connect(m_list, &EquipmentList::postItemRemoved, this, [=]() {
            endRemoveRows();
        });
    }
    endResetModel();
}

Though I'm not sure why this is necessary, because as @SGaist implied above, the endInsertRows() should have emitted this signal, right?

JonB

@mzimmers said in view not updating automatically:

because as @SGaist implied above, the endInsertRows() should have emitted this signal, right?

I am literally walking into this discussion with no context. But what signal should endInsertRows() have emitted according to you? void QAbstractItemModel::rowsInserted(const QModelIndex &parent, int first, int last) will be emitted:

Note: This is a private signal. It can be used in signal connections but cannot be emitted by the user.