QImage and Threads

Question

I am having problems with QImages and Qthreads. I am trying to load big images in a Thread and then display them as QPixmap on a QLabel. My problem is that as long as I don't use a different thread to load the QImages, everything is perfect but as soon as I use a different thread, nothing is renderder. Though I still have a valid size for my QImage.

The thing that puzzles me is that, if I just comment the 22nd line in the cpp that moves the loader to the other thread, the label displays nicely.

Does anyone have an idea?

Here is my very simplified code: Header :

class Loader : public QObject
{
    Q_OBJECT
public:
    explicit Loader(QObject *parent = 0);

signals:
    void imageLoaded(QString, const QImage &);
public slots:
    void loadImage(const QString& fichier);
};

namespace Ui {
class MainWindow;
}
class LoaderImages;

class MainWindow : public QMainWindow
{
    Q_OBJECT

public:
    explicit MainWindow(QWidget *parent = 0);
    ~MainWindow();

signals:
    void loadImage(const QString& dossier);
private slots:
    void imageAvailable(const QString& dossier, const QImage& img);

private:
    Ui::MainWindow *ui;
    //QString mDossier;
    Loader* mLoader;
    //QMap<QString, QImage*> mMapDesImages;
    int mWidth;
};

cpp:

#include "mainwindow.h"
#include "ui_mainwindow.h"
#include <QFile>
#include <QPixmap>
#include <QImage>
#include <QDir>
#include <QThread>
#include <QDebug>
#include <QLabel>

MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    ui(new Ui::MainWindow),
    mLoader(new Loader(NULL)),
    mWidth(0)
{
    ui->setupUi(this);

    QThread* thread = new QThread(this);
    mLoader->moveToThread(thread);
    thread->start();

    connect(this, SIGNAL(loadImage(QString)), mLoader, SLOT(loadImage(QString)));
    connect(mLoader, SIGNAL(imageLoaded(QString,QImage)), this, SLOT(imageAvailable(QString,QImage)));

    emit loadImage("C:/img.jpg");
}

void MainWindow::imageAvailable(const QString &dossier, const QImage& img)
{
    mWidth += (img.width() + 20);
    ui->mScrollContent->setMinimumSize(mWidth,img.height());
    QLabel* lab = new QLabel(ui->mScrollContent);
    lab->setFixedSize(img.width(), img.height());
    lab->setGeometry(mWidth - img.width() + 20, 0, img.width(), img.height());
    lab->setPixmap(QPixmap::fromImage(img));
}

MainWindow::~MainWindow()
{
    delete mLoader;
    delete ui;
}

Loader::Loader(QObject *parent) :
    QObject(parent)
{
}


void Loader::loadImage(const QString& fichier)
{
    QImage* image = new QImage(fichier);

    emit imageLoaded(fichier, *image);
}

Thx!

Answer 1

There are several mistakes:

1. You're not showing the label. When the image loader is in the GUI thread, the image is loaded and the label added to the contents pane before the main window is shown. Since the parent is shown, the children become visible.

   When the loading is done in another thread, you'll be adding image labels to a widget that's already shown. Such child widgets are not visible unless you explicitly show() them.

2. You're leaking the image in loadImage. There's no reason to put that QImage on the heap.

3. You're allowing a running QThread to be destructed. That's a common error since QThread is essentially broken by design. Sane C++ classes should be always destructible. QThread isn't. Thus you need a workaround.

4. You're not setting the minimum height of the contents widget as well.

5. You might wish to consider the use QtConcurrent::run instead of a dedicated thread. This is especially worthwhile when the operation you're undertaking is a one liner, more or less. I've shown both, the implementations are alternated between at runtime. Note that you need to add the concurrent module and CONFIG += c++11 to the project file.

6. Style bugs:

   • There's no reason to pass NULL for default-valued parameters that are already zero.

   • There's no reason to keep QObject members that have the lifetime of the parent object on the heap, if such members are constructed along with the parent object.

   • Just because Qt Creator comes with silly template files doesn't mean that you shouldn't be using a std::unique_ptr or QScopedPointer to hold the ui member. Naked pointers should almost never be members unless they're pointers to QObjects with parents.

As quite a bit of the code is missing, I can't really tell what else might be wrong. Below is a complete example.

// https://github.com/KubaO/stackoverflown/tree/master/questions/image-loader-24853687
#include <QtWidgets>
#include <QtConcurrent>

class Thread final : public QThread {
public:
    ~Thread() { quit(); wait(); }
};

class Loader : public QObject
{
    Q_OBJECT
public:
    explicit Loader(QObject *parent = nullptr) : QObject(parent) {}
    Q_SIGNAL void imageLoaded(const QString &, const QImage &);
    Q_SLOT void loadImage(const QString& fichier) {
        QImage img(fichier);
        if (! img.isNull()) emit imageLoaded(fichier, img);
    }
};

class MainWindow : public QWidget
{
    Q_OBJECT
    Loader m_loader;
    Thread m_loaderThread;
    QGridLayout m_layout{this};
    QPushButton m_open{"Open"};
    QScrollArea m_view;
    QWidget m_content;
    int m_width{};
    bool m_threadImpl = true;
    Q_SIGNAL void loadImage(const QString &);
    Q_SIGNAL void imageLoaded(const QString &, const QImage & img);
    Q_SLOT void imageAvailable(const QString &, const QImage & img) {
        int spacing = 20;
        if (m_width) m_width += spacing;
        auto lab = new QLabel(&m_content);
        lab->setFixedSize(img.width(), img.height());
        lab->setGeometry(m_width, 0, img.width(), img.height());
        lab->setPixmap(QPixmap::fromImage(img));
        lab->show();
        m_width += img.width();
        m_content.setMinimumWidth(m_width);
        m_content.setMinimumHeight(qMax(m_content.minimumHeight(), img.height()));
    }
    Q_SLOT void open() {
        auto dialog = new QFileDialog(this);
        dialog->setAttribute(Qt::WA_DeleteOnClose);
        dialog->show();
        if (m_threadImpl)
            connect(dialog, &QFileDialog::fileSelected, this, &MainWindow::loadImage);
        else
            connect(dialog, &QFileDialog::fileSelected, [this](const QString & fichier){
                QtConcurrent::run([this, fichier]{
                    QImage img(fichier);
                    if (! img.isNull()) emit this->imageLoaded(fichier, img);
                });
            });
        m_threadImpl = !m_threadImpl;
    }
public:
    explicit MainWindow(QWidget *parent = nullptr) : QWidget(parent) {
        m_layout.addWidget(&m_open);
        m_layout.addWidget(&m_view);
        m_view.setWidget(&m_content);
        m_loader.moveToThread(&m_loaderThread);
        m_loaderThread.start();
        connect(&m_open, &QPushButton::clicked, this, &MainWindow::open);
        connect(this, &MainWindow::loadImage, &m_loader, &Loader::loadImage);
        connect(this, &MainWindow::imageLoaded, this, &MainWindow::imageAvailable);
        connect(&m_loader, &Loader::imageLoaded, this, &MainWindow::imageAvailable);
    }
};

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);
    MainWindow w;
    w.show();
    return a.exec();
}

#include "main.moc"