views:

385

answers:

2

Am I correct in thinking that the QGraphics* classes are not thread-safe? I am porting an old app to Qt and in the process attempting to make it multi-threaded too. I looked at the update code and I see no locks whatsoever.

I started off right, all my processing is done in a set of worker threads so that the GUI does not have to block. But as soon as I come to display my visual representation the whole thing falls down like a pack of cards as the update code attempts to read from the buffer the other thread is writing to.

Here is my test case:

  1. Create a bunch of ellipse objects
  2. Create a thread and pass it the scene pointer
  3. In a loop modify any setting on any object in the scene.

Test function:

bool CBasicDocument::Update( float fTimeStep )
{
const QList<QGraphicsItem*> tObjects = items();

for( QList<QGraphicsItem*>::const_iterator tIter = tObjects.constBegin();
tIter != tObjects.constEnd();
++tIter )
 {
    QGraphicsEllipseItem* pElipse = (QGraphicsEllipseItem*)(*tIter);
   if( pElipse )
   {   
      pElipse->setPen( QPen( QColor( (int)(255.0f * sinf( fTimeStep )), (int)(255.0f * cosf( fTimeStep )), (int)(255.0f * sinf( fTimeStep )) ) ) );
   }
 }
return true;
}

I have been thinking about ways I can fix this and none of them are particularly pretty.

Ideally what I want to happen is when I change a setting on an object it is buffered until the next render call, but for the time being I'll settle with it not crashing!

At the moment I have four options:

  1. Double buffer the whole scene maintaining two scene graphs in lockstep (one rendering, one updating). This is how our multithreaded game engine works. This is horrible here though because it will require double the CPU time and double the memory. Not to mention the logistics of maintaining both scene graphs.

  2. Modify QGraphics* to be thread safe as above. This is probably the most practical approach but it will be a lot of work to get it done.

  3. Push modifications to the scene into a queue and process them from the main thread.

  4. Throw multithreading to the wind for the time being and just let my app stall when the document updates. (Not pretty given the data size for some documents)

None of them are particularly appealing and all require a massive amount of work.

Does anybody have any ideas or attempted multithreading QGraphicsScene before?

Cheers

+2  A: 

I've always read it's a better idea to have all the GUI work happen in the main thread, no matter you're using GTK or Qt or else.

In your case I would go for option 2.

Modifying Qt's code to put locks here and there is the worst thing to do ihmo as you won't be able to sync your modified Qt with upstream without a significant amount of work.

Gregory Pakosz
Yeah, unfortunately setting up the components could potentially take quite a long time (although not as long as the processing required to get to that stage). Given the nature of the documents rendered I am treating the QGraphicsScene as the data and the QGraphicsView as the GUI.As you rightly noted, merging my changes with the main Qt branch would be a maintinance nightmare so its not something I'm keen to do. Furthermore, there are so many entrance points into the update code it would be non-trivial to put the locks in.
icStatic
A: 

I notice in your test function that you are already casting (using c-style casts, at that) the QGraphicsItem to a QGraphicsEllipseItem. If you only use a certain number of item types, it might be easier to subclass those types, and provide the thread-safe functions in those subclasses. Then cast down to those subclasses, and call what you want to from the other thread. They can handle buffering the changes, or mutex locking, or whatever you want to do for those changes.

An alternative would be to have one mutex, and each thread locks that when processing. This would likely slow you down a lot as you wait for the mutex to become available. You could also maintain a map of items->mutexes somewhere that both threads can get to, and each thread locks the mutex for the item it is working on while it is working with it. This would be simpler to layer in if you think the subclassing option would be too complex.

Caleb Huitt - cjhuitt
Yeah thats deliberate as it's only a very basic example. In reality I'd be doing proper casting to a more sensible subclass in a much safer way.The problem with mutexes is that I would need to be able to lock the mutex on the main thread. Unfortunately the objects are accessed inside an event deep inside the QGraphicsView object, and the accessor functions used to set the data is not virtual.I'm wondering if I could overload the event handlers in QGraphicsView and put a global lock in there.The bit it is actually dying on is the call to q_updateLaterSlot from a QMetaCallEvent.
icStatic
If you can subclass and access the objects through that subclass, you could send all of the actual data changes through events. That is, the worker thread tells object A to change foo, object A creates a "ChangeFoo" event and posts it to itself, and in customEvent handles the ChangeFoo event to actually change foo to the new setting. That way, the data changes would alway happen in the UI thread, even though the controller dictating those changes is in another thread.
Caleb Huitt - cjhuitt
I think that's probably the best idea yet. Thanks :) I'll investigate. I think I'll also try and get hold of one of the Trolltech people and see what they say.
icStatic