[Qt5-feedback] Adding better data access method to QAbstractItemModel?

[Andre Somers]

Op 11-7-2011 14:43, Stephen Kelly schreef:
> Hi,
>
> Currently in the modelview API there are a lot of individual calls to the
> data() method. These can get expensive when proxy models are added to the
> mix.
>
> I'm thinking of adding a method like this to QAbstractItemModel:
>
> virtual QVariantist rangeData(const QModelIndex&topLeft, const QModelIndex
> &bottomRight, const QList<int>  &roles) const;
>
> with a default implementation like
>
> {
>    QVariantList list;
>    for (int row = topLeft.row(); row<= bottomRight.row(); ++row)
>      for (int col = topLeft.column(); col<= bottomRight.column() ++col)
>        foreach (int role, roles)
>          list<<  index(row, column, topLeft.parent()).data(role);
>    return list;
> }
>
> A model implementation and a proxy implementation can both do better of
> course, which is the whole idea. This change is source incompatible, but the
> itemData method currently in QAIM should maybe be deprecated.
>
> I also want to change the dataChanged signal to
>
> dataChanged(const QModelIndex&topLeft, const QModelIndex&bottomRight,
> QSet<int>  &roles = QSet<int>());
>
> so that it actaully tells you what roles have changed. The extra roles
> argument can be safely ignored, and a signal emission with an empty roles
> set has the same meaning as dataChanged has today.
>
> That means this is source compatible for the emitter but not the receiver,
> ie,
>
> emit dataChanged(foo, bar);
>
> still works, but
>
> connect(model, SIGNAL(dataChanged(QMI,QMI)), SLOT(onDataChanged(QMI,QMI));
>
> Needs to be changed to at least:
>
> connect(model, SIGNAL(dataChanged(QMI,QMI,QSet<int>)),
> SLOT(onDataChanged(QMI,QMI));
>
> The (source compatible) alternative would be to add a roleDataChanged signal
> with the signature I proposed instead of changing dataChanged (and possibly
> deprecating dataChanged).
>
> What are your thoughts? Are small source incompatibilities like that
> acceptible?
>
> Thanks,
>
> Steve.
IMHO, it is always a good idea to think about how the model/view 
interface in Qt can be improved, but I am not sure that this is the 
solution. I have a few doubts, perhaps you can clarify:

1) How is this supposed to work in the context of proxy models? As you 
know, proxy models are able to sort (so continuous blocks all of a 
sudden are not continuous at all anymore) and filter (block sizes 
change!). And we are not talking about proxies that actually modify or 
extend the data then. I don't see how this change is going to make 
implementing those easier or more efficient.

2) How will this work with the all-important tree models?

3) I have some doubts to the ease of use of the flat list of QVariants. 
I think it is *very* easy to make mistakes in using that list. You have 
to start calculating offsets for every piece of data you are after. At 
the very least, I think you should then introduce a class like a 
QItemPackage, that provides these offsets for you in a slightly more 
friendly way than a flat list, so you can at least access the EditRole 
of item (4,8) of parent X with ease. You may base that on a QVariantList 
if you want (why not a QVector<QVariant>?), but I don't think passing 
structured data in a flat list is particulary good API.

4) The data changed signal can be emitted frequently. Isn't constructing 
a QSet for each call an expensive operation?

5) Shouldn't this package of data also contain the flags() data? You 
need that piece of data anyway for rendering, so you end up querying it 
for each model index again. Or should there just be a Qt::FlagsRole be 
introduced that (also) returns this information? Would relying on _that_ 
still be source compatible?

I guess I need to give this more thought. I am not sure if this change 
is worth it.

André