QThreads: aggregating results from different threads on main GUI thread safe ?

[starkm42]

Hi,

I have a very particular question regarding QThreads and QImages.

i have built a sample application that loads small images, applies some image processing and signals to main GUI thread once done.
GUI thread draws the part image on main widget thus effectively aggregating work done on worker threads, does so by

1. writing part of processed image to another QImage which is a member of GUI thread class i.e. MainWidget
2. does update() which calls paintEvent() to update Image with updated image (just to see it in real time)

more details:

Main Widget initializes a bunch of threads and keeps track of them like so

    // create a pool of threads
    for(int i = 0; i< nThreads; i++) {
        WorkerThread *temp = new WorkerThread(this, i, i);
        connect(temp, &WorkerThread::partImageReady, this, &MainWindow::aggregatePartResults);
        threadPool.push_back(temp);
        temp->start();
    }

Worker thread run method

void WorkerThread::run() {
    QPainter imagePainter(&partFinalImage);
        
             // 1. Read image 
            // 2. perform some operation on each Image i.e. scale image
           
            // 3. write to final image that is local to thread i.e initialized during Ctor call of thread
           }

        // it is moved, copy elison
        emit partImageReady(partFinalImage, threadNum, row); // notify GUI thread.
}

as seen earlier partImageReady signal is connected to aggregatePartResults and it looks like this.

void MainWindow::aggregatePartResults(QImage partImage, int threadNum, int imageRow) {
    // TODO: under lock ?
    // slots are addressed one by one ?

    finalImagePainter.drawImage(QPoint(0, 10 * imageRow), partImage);

    // show real time updates !
    update();

    int newRowNum = imageRow + nThreads;

    if (newRowNum >= mapTilesY) {
        return;
    }

    threadPool[threadNum]->updateRow(newRowNum);

    // start thread again i.e. call run();
    threadPool[threadNum]->start();
}

Question:

 finalImagePainter.drawImage(QPoint(0, 10 * imageRow), partImage);

should i be doing this under a mutex lock. if not

1. is drawImage thread safe ?
2. are slots executed one by one and is guaranteed to do so.

ps:

I understand that there are better alternatives to this problem. i.e QConcurrent. but this is just a sample problem to test out my understanding of QThreads.

Thanks in advance !

[jsulm]

@starkm42 said in QThreads: aggregating results from different threads on main GUI thread safe ?:

should i be doing this under a mutex lock. if not

You should do it inside paintEvent.
Inside aggregatePartResults you should only store the image and both integer for painting later.

[starkm42]

@jsulm
That makes sense, but paintEvent will not called after every update to finalMapImage, what if the smallest of the updates from each thread needs to be reflected on final image as and when they are available.

i have tried that it takes a while for the image to load. this was exactly the problem i was trying to solve in the first place.

just in case i posting the relevant snippets here.

void MainWindow::paintEvent(QPaintEvent *) {
    /* first draws an empty image */
    QPainter widgetPainter(this);
    widgetPainter.drawImage(QPoint(0, 0), finalMapImage);
}

void MainWindow::aggregatePartResults(QImage partImage, int threadNum, int imageRow) {
    // TODO: under lock ?
    // slots are addressed one by one ?

    finalImagePainter.drawImage(QPoint(0, 10 * imageRow), partImage);

    // show real time updates !
    update();

[jsulm]

@starkm42 Simply call https://doc.qt.io/qt-6/qwidget.html#update-2 unside aggregatePartResults - this will trigger paintEvent.

[starkm42]

@jsulm sorry if i was not clear in my previous reply, i was using update(), and could see real time updates to final image. without it it takes time.

any idea on whether updates to common resources in slots is safe ?

i could use lock just to be safe, but they do come with a slight overhead. and if Qt slot calls are queued or drawImage is thread safe i could skip that and get some performance benefit.

[Christian Ehrlicher]

When all is done in the same thread (which should be the case here) you don't need locking.

[starkm42]

@Christian-Ehrlicher got it, i was also searching qt doc's and found this that supports your argument.

Execution of the code following the emit statement will occur once all slots have returned

https://doc.qt.io/qt-6/signalsandslots.html#signals

slots are executed one by one even with multi threading i guess.

this should have been trivial to understand but me coming from python and its model of threading was not helping at all.

thanks everyone for all the insights !

i will marks this as solved.

[JonB]

@starkm42 said in QThreads: aggregating results from different threads on main GUI thread safe ?:

Execution of the code following the emit statement will occur once all slots have returned

slots are executed one by one even with multi threading i guess.

This is only true with the (default) connect() type where both signaller and slotter are in same thread. If cross-thread the (default) connect() type is queued: the slots to be called are queued and will not be executed until control returns to the Qt event loop. In this case the code after emit thus executes immediately, (probably, but not necessarily) before the slots execute in the other thread.

[starkm42]

@JonB that is making perfect sense.

for anybody else looking to read up on this further, this should suffice: https://doc.qt.io/qt-6/qt.html#ConnectionType-enum

and as @JonB mentioned if receiver is the same thread i.e. slots are immediately executed and following is the case

Execution of the code following the emit statement will occur once all slots have returned

if receiver and sender are on separate threads QueuedConnection is the default. and slot calls are queued.

so to my requirement, slots are executed one by one and it is safe to do operation on same resource since only one will be working on it at any given time.

[JonB]

@starkm42 said in QThreads: aggregating results from different threads on main GUI thread safe ?:

slots are executed one by one and it is safe to do operation on same resource since only one will be working on it at any given time.

Hmm, your statement concerns me. Are we still talking about using threads? For non-thread, direct connections everything is executed serially and this is not an issue. For multithread, while it is true that any given thread can only execute one slot or piece of code at a time, behaviour across more than one thread or the main thread is "undefined", insofar as multiple pieces of code/different slots can be executing simultaneously. And therefore trying to access shared resources at the same time/in unknown order. If you have any "resources" accessible by more than one thread at a time you need QMutex or similar to serialize access.

[starkm42]

@JonB that is only because in my code example shared resource is accessed under slot of main GUI thread, since calls to slots are queued, they are executed whenever main event loop gets to it one by one and again in my particular case order is not a requirement. so i made that statement. am i right ? might be misleading if anybody doesn't go through the entire code example.

but i get your point, most of the time, shared resources are shared using pointers or as volatile members of some class. so Mutex or similar would be required.

[JonB]

@starkm42
Yes, if it is only multiple slots in a thread, including the main one, which access a resource then everything is occurring serially, no locking would be needed.