6

I have time consuming image loading (image is big), also some operations on it are done when loading. I do not want to block application GUI.

My idea is to load image in another thread, emit signal that image is loaded and then redraw view with this image.

My approach:

void Window::loadImage()
{ 
    ImageLoader* loaderThread = new ImageLoader();
    connect(loaderThread,SIGNAL(imageLoaded()),this,SLOT(imageLoadingFinished());
    loaderThread->loadImage(m_image, m_imagesContainer, m_path);
}
void Window::imageLoadingFinished()
{
    m_imagesContainer->addImage(m_image);
    redrawView();
}

class ImageLoader : public QThread
{
    Q_OBJECT
    public:
       ImageLoader(QObject *parent = 0) : m_image(NULL), m_container(NULL)

       void loadImage(Image* img, Container* cont, std::string path)
       {
            m_image = img;
            m_container = cont;
            ...
            start();
       }
    signals:
       void imageLoaded();
    protected:
       void run()
       {
           //loading image and operations on it
           emit imageLoaded();
       }
    protected:
       Image* m_image;
       Container* m_container;
}

I was basing on quedcustomtype example from Qt writing this code. When googling and searching in stackoverflow I've also find out that subclassing QThread is not a good idea.

So the question is what is the correct way to do it? As I said I want non blocking GUI, loading and operations done in another thread and signal which says loading is finished. After signal is emited view should be redrawn. I don't know much about multithreading however think to understand or have sufficient knowledge to understand basic ideas.

krzych
  • 2,126
  • 7
  • 31
  • 50
  • If `QThread` was inherently bad, it would be taken out of the library. Instead of just believing "never use QThread" you should find out what issues people believe exists, decide if they are valid for you, and decide how to make use of them. Alternatively, you can use platform-specific (not necessarily portable) thread mechanisms like pthreads for example. Just don't drive the UI in your threads, instead use Qt events to tell the UI thread to drive it. – mah Dec 14 '12 at 12:35
  • @mah I don't think that QThread is inherently bad, but there are some [different understandings how to use it](http://blog.qt.digia.com/blog/2010/06/17/youre-doing-it-wrong/). – Andreas Fester Dec 14 '12 at 12:39
  • @Andreas that blog post seems to be indicating that many people don't know how to properly use QThread. The link at the bottom of the post has an example page, whose salient point seems to be: to properly subclass QThread, you must overload the `run()` method. I'm really not sure why there would be a perception (valid or not) of a pattern of misuse as your link suggests though, when Qt's documentation seems pretty clear. http://doc.qt.digia.com/qt/threads-starting.html for example. – mah Dec 14 '12 at 13:33
  • http://stackoverflow.com/questions/4093159/what-is-the-correct-way-to-implement-a-qthread-example-please might be of interest – g19fanatic Dec 14 '12 at 15:36

2 Answers2

7

Use QtConcurent framework.

#include <QtConcurentRun>
#include <QFutureWatcher>

//....
class Window: public QWidget /*or something*/
{
//....
private:
    QFutureWatcher<QImage> _wacther; //this object will signal when loading finished
};

//...

void Window::loadImage()
{
   connect(&_watcher, SIGNAL(finished(), SLOT(finishLoading());
    _wacther.setFuture(QtConcurent::run<QImage>(this, &Window::doLoadImage));
}

QImage Window::doLoadImage() //this function will be executed in the new thread. SHOULD BE Thread Safe
{
   return someImage;
}

void window::finishLoading()
{
    QImage result = _watcher.result();
}
Lol4t0
  • 12,444
  • 4
  • 29
  • 65
  • 1
    good idea, you beat me to it. One nitpick, the _watcher variable in global scope should not begin with _ because that is reserved in global scope. see http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier – odinthenerd Dec 14 '12 at 12:50
  • @PorkyBrain, it should probably be a member of `Window`. Let me put it there. – Lol4t0 Dec 14 '12 at 12:52
  • 1
    Watch out for spelling mistakes – koan Dec 14 '12 at 13:25
  • I think that I've simplified to much. I did not want to complicate and posted example little different I have. The problem is with calling `QtConcurrent::run(*this,&ClassName::function)`. Output says `QObject::QObject(const QObject&) is private`. I've tried `QtConcurent::run(&className::function` but also failed. I see that copying somwere is dissabled. Don't know if I provided enough information in this comment. If not please say where clarification is needed. – krzych Dec 14 '12 at 14:22
  • Consider that Qt guys does not consider QtConcurrent to be the best way to go: http://www.mail-archive.com/development@qt-project.org/msg07794.html. I like it but I've always been discouraged using it. – Luca Carlon Dec 14 '12 at 14:25
  • @krzych, `QtConucrent::run` also accept pointer to object. You can use `this` instead of `*this` – Lol4t0 Dec 14 '12 at 15:08
  • @LucaCarlon, it is just is opinion, concurrent framework works well and it is easy to use. I don't know any bugs to fix there. – Lol4t0 Dec 14 '12 at 15:12
  • @Lol4t0 As I said I like it either :-) I use QtConcurrent and it is not deprecated nor bugged at all (at least not that I know). Anyway, the question seemed to ask for the typical way to go, so I reported that it is not what Qt guys suggest. Your answer is totally fine for me. – Luca Carlon Dec 14 '12 at 15:27
3

I suppose this is the best way to go:

#include <QApplication>
#include <QLabel>
#include <QThread>

class ImageLoader : public QObject
{
   Q_OBJECT
public:
   ImageLoader() : QObject() {
      moveToThread(&t);
      t.start();
   }
   ~ImageLoader() {
      qDebug("Bye bye!");
      t.quit();
      t.wait();
   }

   void requestImage(QString absPath) {
      QMetaObject::invokeMethod(this, "loadImage", Q_ARG(QString, absPath));
   }

public slots:
   void loadImage(QString absPath) {
      // Simulate large image.
      QImage image(absPath);
      sleep(10);
      qDebug("Image loaded!");
      emit imageReady(image);
   }

signals:
   void imageReady(QImage image);

private:
   QThread t;
};

class MyLabel : public QLabel
{
   Q_OBJECT
public:
   MyLabel() : QLabel() {}

   void mousePressEvent(QMouseEvent* ev) {
      Q_UNUSED(ev);
      qDebug("I got the event!");
   }

public slots:
   void setImage(QImage image) {
      setPixmap(QPixmap::fromImage(image));
      resize(image.width(), image.height());
      qDebug("Image shown!");
   }
};

int main(int argc, char *argv[])
{
   QApplication a(argc, argv);

   MyLabel label;
   label.show();

   ImageLoader imageLoader;
   QObject::connect(&imageLoader, SIGNAL(imageReady(QImage)), &label, SLOT(setImage(QImage)));
   imageLoader.requestImage(some_abs_path);

   return a.exec();
}

#include "main.moc"

I also like QtConcurrent, but consider that its use is somehow discouraged: http://www.mail-archive.com/development@qt-project.org/msg07794.html.

Luca Carlon
  • 9,546
  • 13
  • 59
  • 91
  • You move the ImageLoader to the thread , but in the meantime, you also move the Thread in its "own" thread because its a member variable. QThread is just the thread manager, and shouldn't move. See how the official documentation uses a Controller (wrapper around a thread) and worker (which uses moveToThread()) – B. Decoster Feb 12 '16 at 22:35
  • No, moveToThread moves the CHILDREN according to the docs: http://doc.qt.io/qt-5/qobject.html#moveToThread. That QThread is not a child, it is just a member. Docs make this clear http://doc.qt.io/qt-5/qobject.html#thread-affinity: "the object's member variables will remain in the old thread when moveToThread() is called". This means the QThread will remain in the original thread and just the object is moved, as there is no child. – Luca Carlon Feb 13 '16 at 01:18
  • Can you do a quick edit so that I can evert my vote? Adding a space is sufficient it seems – B. Decoster Mar 18 '16 at 22:59