Qt: Memory leaks in QGraphicsScene::addText?

Question

I'm trying to make a Rule for a Timeline like this:

I have a QGraphicsView where I put a QGraphicsScene where I add QGraphicsItems like lines and some labels.

I add the elements to the Scene only when the zoom changes (and not when a paintEvent raises).

For adding time labels, I use:

    QString label = "00:14"; // For example
    int posX = ... // Here I calculate the position of the label
    scene->addText(label,QFont("Arial",8))->setPos(posX,-1);

When I have to re-draw the Rule, I make:

    qDeleteAll(scene->items());

at the beggining and then add labels and lines again.

I realized that I have bad performance. My scene has something like 8k elements (between lines and labels) so I used Valgrind to check for problems.

It shows that 'possibly' I have memory leaks in the lines where I add text to the scene. I have some messages that look like:

    2,165,760 bytes in 470 blocks are possibly lost in loss record 9,922 of 9,923
      in TimelineWidget::drawRule() in Timeline/timelinewidget.cpp:166
      1: realloc in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so
      2: /usr/lib/x86_64-linux-gnu/libQtGui.so.4.8.3
      3: /usr/lib/x86_64-linux-gnu/libQtGui.so.4.8.3
      4: QTextDocument::setPlainText(QString const&) in /usr/lib/x86_64-linux-gnu/libQtGui.so.4.8.3
      5: /usr/lib/x86_64-linux-gnu/libQtGui.so.4.8.3
      6: QGraphicsTextItem::QGraphicsTextItem(QString const&, QGraphicsItem*, QGraphicsScene*) in /usr/lib/x86_64-linux-gnu/libQtGui.so.4.8.3
      7: QGraphicsScene::addText(QString const&, QFont const&) in /usr/lib/x86_64-linux-gnu/libQtGui.so.4.8.3
      8: TimelineWidget::drawRule() 

This only happen with addText calls, not with addLine ones.

I'm using Qt 4.8 and my questions are:

• Are those memory leaks real?

• Am I doing something wrong?

• Is there another/a better way to implement my Rule?

Thanks in advance!

Answer 1

I'd suggest all of the following:

1. Instead of qDeleteAll(scene->items()), simply clear the scene:

   scene->clear();
   

2. Use scene->addSimpleText if you're not displaying rich text.

3. For simple, repetitive items, such as a group of lines with fixed spacing, create custom items that can be re-used. Those will be faster and have much lower overhead than creating graphics scene items for each of the lines. The memory, indexing, tree traversal and lookup overheads will drop from O(N) to O(1) in terms of the number of drawn lines

   Let's say you've created a custom item that is a horizontal array of lines: | | | | | | | | |. That item simply paints a fixed number of lines. You can resize the item to change the height of the lines and the horizontal area spanned by them. Remember to reset the pen in your paint() method, as the default pen will be re-scaled and thus likely very wide.

   A simple unlabeled ruler with major and minor ticks can be obtained by overlaying three items: two line arrays and a horizontal line.

You may also want to rearchitect how you treat the scene. So far, you treat the scene as a model for the entire timeline. Instead, you should be treating at least parts of it as a viewmodel.

There are two main elements to your timeline: the rule and the clip previews.

When you scroll the rule at given zoom level, you'll notice that there's always the same amount of lines and labels visible. The only thing that changes is a slight horizontal offset of the lines, and of course the values of the labels change too.

So instead of having all of the lines and all of the labels in your scene, only have as many as fit in the current view, adjust their horizontal position in the scene so that they are visible at the correct offset, and fix up the label text. That's what makes it a view model - it's a model tightly tied to a particular view. The underlying abstract model is that of a very long ruler, but we only need it to be an abstract model, not a real one that takes memory and other resources.

The clip previews can be dynamically added and removed from the scene as you scroll as well. This may save you a lot of memory since I presume you have lots of images, most of which are not visible.

The example screenshot that you've shown has some user design issues: it's rather pointless to show horizontally truncated overlapping clip previews. At any given zoom level, you should have a minimum preview size that you're willing to display. Showing those narrow vertical pixel strips is very expensive. Below is a possible algorithm to deal with it.

Assume that the preview items are rectangles sorted left to right.

1. Start with the leftmost item that intersects the viewport rectangle (everything in scene coordinates, of course). Make it the current_item.

2. Show the current_item (add it to the scene).

3. Keep taking next items while they overlap the current_item. Store the last_overlapping_item (it can be empty if no items overlap).

4. Save the current item in previous_item. Take the next item and make it the current_item.

5. If there's no gap between current_item and previous_item, clear the last_overlapping_item as it's completely overlapped by adjacent current and previous items.

6. Show the last_overlapping_item if any (add it to the scene).

7. If current_item is entirely outside the viewport rectangle, you're done. Otherwise go to 2.