what is the best way to convert a Qt object(QgraphicsItem) into C++ class object

Question

I am trying to convert a Qt object into a C++ class object in a loop like this

std::vector<PBWMPlugDeviceGraphicsItem*> deviceItms;

for(int i=0; i<fScene->items().size(); i++)
    deviceItms.push_back(dynamic_cast<PBWMPlugDeviceGraphicsItem*>(fScene->items().at(i)));

Where PBWMPlugDeviceGraphicsItem is a C++ class. It seems that, when total no. of items is larger than a certain threshold (e.g. fScene->items().size() >900), it takes a considerable amount of time to convert these objects and because of that I can see operation on my QGraphicsScene to be very slow. I read that dynamic_cast has serious performace issues.

Is there any other great/fast way to achieve the same result?

Thanks!

Answer 1

The problem is calling QList::at, which returns reference to list item, which causes QList to do copy-on-write detach.

Minimal solution is to call QList::value to get the pointer, instead of at.

Other solution is to fetch the list once, as explained in other answers. This way detach will only happen once even if you use at.

You could also force use of const version of at by cast or temp variable.

Answer 2

Use this:

for (auto p: fScene->items())
    deviceItms.push_back(dynamic_cast<PBWMPlugDeviceGraphicsItem*>(p));

Two important differences:

• It only calls fScene->items() once. Unless that function returns by (const) reference, retrieving and discarding a container for each iteration is useless overhead, in particular if the content is not supposed to change between iterations.
• Instead of using an index and validating that index in at(), it uses C++11 for loops with an automatically typed variable. The at() is only useful if you have doubts whether an index is valid (e.g. when it was input from the user), but if you can't program a simple loop, you really have more important issues.

I didn't take the third optimization of using vector::reserve() to pre-allocate memory. If you really only have a few objects (yes, 900 is few), then it's probably hardly noticeable. Still, do some research on that, as it's a tool that you should know about.

Answer 3

for(int i=0; i<fScene->items().size(); i++)
  deviceItms.push_back(dynamic_cast<PBWMPlugDeviceGraphicsItem*>(fScene->items().at(i)));

Based on a comment reply, it seems fScene->items() returns something like QList<QGraphicsItem *>.

As your code is written, instances of this QList are created and discarded at every loop iteration. So, as first optimization, I'd try getting that list outside the loop once, and then processing that same instance inside the loop, avoiding wasting creation/destruction of the list at each loop iteration.

If you can use C++11, I agree with @Ulrich's suggestion of using a range-based for loop: this will make the code both simpler and faster.

---

EDIT

In a comment here you wrote that you can't use C++11, so you can't enjoy range-based for loops.
Then, an alternative might be:

// Get the collection of items once, outside the loop
QList<QGraphicsItem *> items = fScene->items();

// For each graphics item in the collection...
for (int i = 0; i < items.size(); i++) {
    deviceItems.push_back(dynamic_cast<PBWMPlugDeviceGraphicsItem*>(items.at(i)));
}

Answer 4

ThankYou all! your suggestion are very useful. I implemented in following way(as i can't use C++11)

        QList<QGraphicsItem*> temp= fScene->items();
    for(int i=0; i<temp.size(); i++)
        deviceItms.push_back(dynamic_cast<PBWMPlugDeviceGraphicsItem*>(temp[i]));

and it is working absolutely fine.

Thankyou very much for the useful tips! I really learned somethign new.