5

In Qt, I have a model subclassing QAbstractItemModel - it's a tree displayed in a QTreeView.

The model supports various forms of change which all work OK. The two of relevance are:

1) Some data in a small number of related rows changes

2) A visualisation change means that the majority of rows should change their formatting - in particular they have a change of background highlighting. Their DisplayRole data does not change.

The current design deals with both of these in the same way: for every row that has any change the model emits dataChanged(start_of_row_index,end_of_row_index). I emit the signal for both parent rows that change and for any of their children that have changed.

However, this performs badly in case 2 as the model gets big: a very large number of dataChanged signals are emitted.

I have changed the code so that in case 2 the model emits dataChanged only for the (single) row that is the parent of the entire tree.

This still appears to work correctly but does not accord with my understanding of the responsibilities of the model. But I suspect I may be wrong.

Perhaps I am misunderstanding the dataChanged signal? Does it actually cause the view to update all children as well as the specified range? Or can I avoid emitting dataChanged when it is not the DisplayRole that is changing?

Edited with my progress so far

As Jan points out, I ought to emit dataChanged either for most or all of the rows in case 2.

My code originally did this by emitting dataChanged for every changed row but this is too expensive - the view takes too long to process all these signals.

A possible solution could be to aggregate the dataChanged signal for any contiguous blocks of changed rows but this will still not perform well when, for example, every other row has changed - it would still emit too many signals.

Ideally I would like to just tell the view to consider all data as potentially changed (but all indexes still valid - the layout unchanged). This does not seem to be possible with a single signal.

Because of a quirk of the QTreeView class, it is possible (though incorrect according to the spec) to emit only one dataChanged(tl,br) as long as tl != br. I had this working and it passed our testing but left me nervous.

I have settled for now on a version which traverses the tree and emits a single dataChanged(tl,br) for every parent (with tl,br spanning all the children of that parent). This conforms to the model/view protocol and for our models it typically reduces the number of signals by about a factor of 10.

It does not seem ideal however. Any other suggestions anyone?

strubbly
  • 3,347
  • 3
  • 24
  • 36
  • I have read this thread which suggests that all visible cells are updated when my dataChanged is sent. Which partially explains the behaviour I am seeing. But I am still confused about what I SHOULD do. Perhaps, even in case 1, I only need to send one dataChanged? – strubbly May 15 '13 at 11:34

1 Answers1

7

You are expected to let your views know whenever any data gets changed. This "letting know" can happen through multiple ways; emitting dataChanged is the most common one when the structure of the indexes has not changed; others are the "serious" ones like modelReset or layoutChanged. By a coincidence, some of the Qt's views are able to pick up changes even without dataChanged on e.g. a mouseover, but you aren't supposed to rely on that. It's an implementation detail and a subject to change.

To answer the final bit of your question, yes, dataChanged must be emitted whenever any data returned from the QAIM::data() changes, even if it's "just" some other role than Qt::DisplayRole.

You're citing performance problems. What are the hard numbers -- are you actually getting any measurable slowdown, or are you just prematurely worried that this might be a problem later on? Are you aware of the fact that you can use both arguments to the dataChanged to signal a change over a big matrix of indexes?

EDIT:

A couple more things to try:

  • Make sure that your view does not request extra data. For example, unless you set the QTreeView's uniformRowHeights (IIRC), the view will have to execute O(n) calls for each dataChanged signal, leading to O(n^2) complexity. That's bad.

  • If you are really sure that there's no way around this, you might get away by combining the layoutAboutToBeChanged, updatePersistentIndexes and layoutChanged. As you are not actually changing the structure of your indexes, this might be rather cheap. However, the optimization opportunity in the previous point is still worthwhile taking.

Jan Kundrát
  • 3,700
  • 1
  • 18
  • 29
  • 1
    Yeah, I am getting problems: I have more than 400,000 rows and it takes minimum 5 to 10 seconds to update. Of course, far fewer than that are actually visible. Which means that if I emit any dataChanged(a,b) with a != b then actually the view correctly updates much faster. But this is not according to spec - I am exploiting the way the QTreeView actually works. – strubbly May 15 '13 at 15:34
  • In the original design I emitted one signal per (changed) row. In the current working (but non-compliant) version I emit one signal. There is an intermediate version in which I emit one signal per parent which would give me somewhere in-between (maybe 40,000 signals instead of 400,000) - haven't tried that one. – strubbly May 15 '13 at 15:38
  • OK switched to the version that emits for every block of children. This perfoms OK (not great) and seems to conform to the protocol. I have described it in the edit to my original post. – strubbly May 15 '13 at 16:36
  • Added a couple more ways of potimizing the model. – Jan Kundrát May 15 '13 at 17:06
  • 1
    Yeah I already have ``uniformRowHeights(True)`` thanks. It is still pretty sluggish. I am tempted to go back to the dodgy version - I'll get less user complaints in the short term. Until the next version of Qt causes the whole thing to go wrong :-( – strubbly May 15 '13 at 17:39
  • If I do ``LayoutChanged`` etc will that cause the view to reset (eg forget which parts of the tree are expanded and selected)? Do I need to updatePersistentIndexes, given that the index structure is completely unchanged? Maybe I should just try it... – strubbly May 15 '13 at 17:44
  • 1
    @strubbly Not sure why you accepted the answer, it seems this is an open problem still? Or am I misreading? Did you ever find the knockout punch for your problem? – eric Jul 21 '14 at 01:17
  • @neuronet I accepted the answer because it correctly describes when I should emit dataChanged which was the question even though I don't like the answer. I have left my code using the version which emits one signal (I can see in the QTreeVièw code that that works) and documented my non-compliance in my code. Not ideal but working well. – strubbly Jan 25 '15 at 08:40