5

I derived a model from QAbstractTableModel and now I want to notify, that the data of a whole row has been changed. If for example the data of a row with index 5 is changed (4 columns), than using the following code works as expected.

emit dataChanged(index(5,0), index(5, 0));
emit dataChanged(index(5,1), index(5, 1));
emit dataChanged(index(5,2), index(5, 2));
emit dataChanged(index(5,3), index(5, 3));

But if I try to achieve the same with only one emit, ALL columns of ALL rows in the view are updated.

emit dataChanged(index(5, 0), index(5, 3));

What I am doing wrong here?

Minimal example (C++11, QTCreator 4.7.1, Windows 10 (1803), 64 Bit)

demo.h

#pragma once
#include <QAbstractTableModel>
#include <QTime>
#include <QTimer>

class Demo : public QAbstractTableModel
{
  Q_OBJECT
  QTimer * t;
public:
  Demo()
  {
    t = new QTimer(this);
    t->setInterval(1000);
    connect(t, SIGNAL(timeout()) , this, SLOT(timerHit()));
    t->start();
  }

  QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override
  {
    int c = index.column();
    if (role == Qt::DisplayRole)
    {
      QString strTime = QTime::currentTime().toString();
      if (c == 0) return "A" + strTime;
      if (c == 1) return "B" + strTime;
      if (c == 2) return "C" + strTime;
      if (c == 3) return "D" + strTime;
    }
    return QVariant();
  }

  int rowCount(const QModelIndex &) const override { return 10; }
  int columnCount(const QModelIndex &) const override { return 4; }
private slots:
  void timerHit()
  {
    //Works
    emit dataChanged(index(5,0), index(5, 0));
    emit dataChanged(index(5,1), index(5, 1));
    emit dataChanged(index(5,2), index(5, 2));
    emit dataChanged(index(5,3), index(5, 3));

    //emit dataChanged(index(5,0), index(5, 3)); // <-- Doesn't work
  }
};

main.cpp

#include "demo.h"
#include <QApplication>
#include <QTreeView>

int main(int argc, char *argv[])
{
  QApplication a(argc, argv);
  QTreeView dataView;
  Demo dataModel{};
  dataView.setModel( &dataModel );
  dataView.show();
  return a.exec();
}
SoulfreezerXP
  • 459
  • 1
  • 5
  • 19
  • 1
    What is your expected output for `emit dataChanged(index(5, 0), index(5, 3));`? To change all columns? To change only `index(5, 0)` and `index(5, 3)`? (This wasn't particularly clear to me from your question.) According to the [documentation](https://doc.qt.io/qt-5.11/qabstractitemmodel.html#dataChanged), all cells inclusive within that range will be changed. – TrebledJ Nov 04 '18 at 12:15
  • I rephrased it in my question. I expect, that all columns in that range will be changed in the view. So that should be only column 0 - 3 from row 5 and not ALL columns of all rows of the whole model. Why are ALL columns of all rows changed in the view? – SoulfreezerXP Nov 04 '18 at 12:30

3 Answers3

5

I think the problem lies with certain assumptions you're making with regard the behaviour of QTreeView when the QAbstractItemModel::dataChanged signal is emitted.

Specifically, you assume that the view will only invoke QAbstractItemModel::data on those indexes that are specified in the signal. That's not necessarily the case.

Looking at the source for QAbstractItemView::dataChanged (Qt 5.11.2) you'll see...

void QAbstractItemView::dataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight, const QVector<int> &roles)
{
    Q_UNUSED(roles);
    // Single item changed
    Q_D(QAbstractItemView);
    if (topLeft == bottomRight && topLeft.isValid()) {
        const QEditorInfo &editorInfo = d->editorForIndex(topLeft);
        //we don't update the edit data if it is static
        if (!editorInfo.isStatic && editorInfo.widget) {
            QAbstractItemDelegate *delegate = d->delegateForIndex(topLeft);
            if (delegate) {
                delegate->setEditorData(editorInfo.widget.data(), topLeft);
            }
        }
        if (isVisible() && !d->delayedPendingLayout) {
            // otherwise the items will be update later anyway
            update(topLeft);
        }
    } else {
        d->updateEditorData(topLeft, bottomRight);
        if (isVisible() && !d->delayedPendingLayout)
            d->viewport->update();
    }

#ifndef QT_NO_ACCESSIBILITY
    if (QAccessible::isActive()) {
        QAccessibleTableModelChangeEvent accessibleEvent(this, QAccessibleTableModelChangeEvent::DataChanged);
        accessibleEvent.setFirstRow(topLeft.row());
        accessibleEvent.setFirstColumn(topLeft.column());
        accessibleEvent.setLastRow(bottomRight.row());
        accessibleEvent.setLastColumn(bottomRight.column());
        QAccessible::updateAccessibility(&accessibleEvent);
    }
#endif
    d->updateGeometry();
}

The important point is that this code behaves differently depending on whether or not the signal specifies a single QModelIndex -- e.g. topLeft is the same as bottomRight. If they are the same then the view tries to ensure that only that model index is updated. However, if multiple model indexes are specified then it will invoke...

d->viewport->update();

which will, presumably, result in the data for all visible model indexes being queried.

Since your implementation of Demo::data always returns new data based on the current time you will see the entire visible part of the view update giving the impression that the dataChanged signal was emitted for all rows and columns.

So the fix is really to make your data model more ``stateful'' -- it needs to keep track of values rather than simply generating them on demand.

G.M.
  • 12,232
  • 2
  • 15
  • 18
  • Thanx for that answer. So in my real-world-model, when I have 500k rows inserted and mostly only one single row changes in the model, I have to call x times dataChanged instead of a single "range-call", right?! – SoulfreezerXP Nov 04 '18 at 16:20
  • Not necessarily. If you emit the `dataChanged` signal specifying multiple model indexes it might update more than you specify -- but not all 500k. – G.M. Nov 04 '18 at 16:24
  • I made a test for 100k elements (changing 2 columns in a single row). Result: Calling the range-method results in 3 times more calls to data() for the same viewport. Ur right, that not all data is queried, only a few times (31), but the "range-call" will call much more times the data(). – SoulfreezerXP Nov 04 '18 at 16:37
2

This is a genuine efficiency bug in Qt.

The Qt project is no longer accepting changes to Qt 5, so I made the change and pushed it to my fork on GitHub. You can see a fix for the issue you've encountered here.

If you want to build your own copy of 5.15.2, you may be interested in my other fixes.

ulatekh
  • 1,311
  • 1
  • 14
  • 19
1

Not sure whether this is what you're looking for but I'll put it up anyways.


Even using emit dataChanged(...), you would still see that clicks/selection on rows will cause them to self-update (doing this from a Mac, so might be different).

Instead of using the QAbstractItemModel::dataChanged signal, I will be using the QAbstractItemModel::setData() function.

This is my implementation of demo.h

#pragma once
#include <QAbstractTableModel>
#include <QTime>
#include <QTimer>

class Demo : public QAbstractTableModel
{
    Q_OBJECT

public:
    Demo()
    {
        int cCount = columnCount(index(0, 0));
        int rCount = rowCount(index(0, 0));

        //  populate model data with *static* values
        QString strTime = QTime::currentTime().toString();

        QStringList temp;
        for (int j = 0; j < cCount; j++)
            temp.append(strTime);
        for (int i = 0; i < rCount; i++)
            demoModelData.append(temp);

        //  nothing new here
        t = new QTimer(this);
        t->setInterval(1000);
        connect(t, SIGNAL(timeout()) , this, SLOT(timerHit()));
        t->start();
    }

    QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override
    {

        //  tells the *view* what to display
        //      if this was dynamic (e.g. like your original strTime implementation)
        //      then the view (QTreeView in main.cpp) will constantly update
        if (role == Qt::DisplayRole)
            return demoModelData.at(index.row()).at(index.column());    //  retrieve data from model

        return QVariant();
    }

    //  reimplemented from QAbstractTableModel
    virtual bool setData(const QModelIndex &index, const QVariant &value, int role = Qt::DisplayRole) override
    {
        if (role == Qt::DisplayRole)
        {
            demoModelData[index.row()][index.column()] = value.toString();  //  set the new data

            emit dataChanged(index, index);     //  explicitly emit dataChanged signal, notifies TreeView to update by 
                                                //  calling this->data(index, Qt::DisplayRole)
        }

        return true;
    }

    int rowCount(const QModelIndex &) const override { return 10; }
    int columnCount(const QModelIndex &) const override { return 4; }

private slots:
    void timerHit()
    {
        QString strTime = QTime::currentTime().toString();
        setData(index(5, 0), QVariant(strTime), Qt::DisplayRole);   //  only changes index at (row = 5, col = 0)
    }

private:
    QTimer *t;

    QList<QStringList> demoModelData;       //  stores the table model's data

};

Since the class is a "model", there should be some way of storing/retrieving data for display. Here, I've used a QList<QStringList>, but you can store data in other ways that suit you as well (e.g. a tree, QVector, QMap).

TrebledJ
  • 8,713
  • 7
  • 26
  • 48
  • 1
    Thanx for that, but I need an answer only for the question i asked. Have I found a bug in Qt or am I misinterpreting the behavior of dataChanged? – SoulfreezerXP Nov 04 '18 at 14:25
  • Misinterpreting, I believe. From the documentation: `This signal [dataChanged] is emitted whenever the data in an existing item changes.`, but from your example, the signal is being emitted to change the data. (It's going the other way round, not what it's meant for. Use `setData` instead.) – TrebledJ Nov 04 '18 at 14:37
  • 1
    My example is correct. My data changes (over time), and I tell the model that something has been changed. My question is clear I think...I believe, qt has not to update its whole view, when I told them, that the data of the columns of row with index 5 has been changed. – SoulfreezerXP Nov 04 '18 at 14:48