3

I have a program where I am now saving large, 18 megapixel or less images to the disk. Along the way I convert them to a QImage, display the QImage in a subclass of a QDialog, and then prompt the user to save them.

I wanted to have some sort of progress indicator for the user, so I reasoned out that my best way to achieve that would be to create a QThread, save the image with a worker within the thread, and then emit a signal back to the GUI to declare that saving has finished. I use this method to show a progress bar to the user during the saving process.

Consider this simple worker class here:

class Worker : public QObject
{
    Q_OBJECT

public slots:
    void doWork(const QImage &image, const QString &file) {

        if(image.save(file, "PNG", 100))
            emit resultReady();
        else
        {
            // handle error
            return;
        }
    }

signals:
    void resultReady();
};

My problem is that the image::save() function returns true often long before the image has actually finished writing to the disk. With this current line of code, on my 5600 HDD, the resultReady() signal is fired roughly 6 seconds after the user presses the button, and thus my GUI updates to show that the image has finished saving.

However, the QImage seems to take anywhere from 7ish seconds to up to 30ish seconds to finish writing to disk. During this time, the user could have ended the application, which causes an incomplete image on the disk as the thread considers itself to be finished. Perhaps worse still, the user could have gone on to take multiple other images that begin to exacerbate the duration spent saving images.

Is there a way to determine exactly when a Qt application has finished writing a QImage to the hard disk?

Kettamon
  • 97
  • 8

3 Answers3

2

Your diagnosis of the issue is incorrect. Once image.save has returned, the filesystem is in a consistent state and you can forcibly exit the application at that very point: the file will have correct contents. Whether the on-disk state is consistent is unknown, but all that means is that you shouldn't reset nor forcibly power-off the machine. Your application is free to quit, though.

Your issue is something else. Who knows what: you failed to provide a test case.

It's not necessary to deal with worker threads and objects manually. Leverage QtConcurrent. It's supposed to be easy, and it is. Namely:

class MyWindow : public QWidget {
  Q_OBJECT
  CountDownLatch m_latch;
  Q_SIGNAL void imageSaved(const QString & filename);
  Q_SIGNAL void imageSaveFailed(const QString & filename);
  void saveImage(const QImage & image, const QString & filename) {
    auto lock = m_latch.lock();
    QtConcurrent::run([=]{ // captures this, lock, image and filename
      if (image.save(filename, "PNG", 100))
        emit imageSaved(filename);
      else
        emit imageSaveFailed(filename);
    });
  }
  ...
  // MyWindow's destructor will block until all image savers are done
};

For implementation(s) of CountDownLatch, see this question.

Community
  • 1
  • 1
Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313
  • You're right. I realize now that my problem is the functions that are used to enable and disable GUI contents are effectively thread-unsafe due to shared threading objects. I was testing the image saving process by rapid-firing the save button whenever it was available. I think that by doing so I had a chance of spawning multiple threads that would be saving to the hard drive at or near the same time, causing congestion. Then one of them would finish and I would close the program while a different thread was still in the saving process. Something along those lines. – Kettamon Jul 15 '16 at 12:37
  • As an aside, in your example for CountDownLatch you linked, do you mean to use braces throughout the code in the Locker's definitions? I tried to look at that example and it seems like you meant to use parenthesis for the various functions, right? I can't comment in other threads yet or I would have posted this there. – Kettamon Jul 15 '16 at 13:42
  • @Ketta The code I post there has been compiled and tested. That's how modern C++ can look. C++98/03 used to overload the same parentheses to mean too many things: expression grouping, function/method call, and constructor invocation. In C++11, constructor invocation gets its own syntax with `{}` braces. It lets you easily distinguish method calls and other expressions from object initialization. – Kuba hasn't forgotten Monica Jul 15 '16 at 15:11
  • Ok that's good to know. Thanks for the lesson. I hadn't encountered that syntax before, and apparently the compiler I'm using with VS2012 doesn't support that type of initialization. Had to make a few little tweaks to test it myself. – Kettamon Jul 15 '16 at 17:05
0

Handle such big data in the main thread is a bad idea, anyway. For such issues the Qt framework offer nice tools. Look in the Qt documentation for QConcurrent. In your case I would save the files with Concurrent Run retrieving the data for a progress bar use QFutureWatcher.

Kombinator
  • 107
  • 6
  • I think I understand what you are saying. I've implemented QFuture and QFutureWatcher instances that direct at that same function above, pasted into the QDialog subclass I use. This does seem to alleviate the issue and make the images save at regular intervals instead of the mess I had before, but I'm a little confused as to what the actual difference is here. How does using QtConcurrent ensure that the QImage::save() member returns at or very close to the time that the image is fully written to the disk? Or do you think I'm missing the cause of the issue? – Kettamon Jul 13 '16 at 17:54
  • Actually I think I retract that. I have managed to force the same issue to happen with this solution as well. The QFutureWatcher signals `finished()` after the `QImage::save(...)` returns just as before, so I still end up with situations where the application GUI is recovering from the process before the image file has actually been fully written onto the hard drive... Perhaps I am misunderstanding your solution. Though I wonder if this is simply that QImage passes the saving off to another thread on its own, or something like that, in which case I don't know what needs to be done... – Kettamon Jul 13 '16 at 18:10
  • Note that `QFutureWatcher` is kinda bulky and only necessary to support C++98. With C++11, you're supposed to use a functor, and emit whatever signal you desire from that functor. Qt's signals can be safely emitted from any thread, including foreign threads, of course as long as you use the automatic or queued connection types to act on the signal. – Kuba hasn't forgotten Monica Jul 13 '16 at 21:03
0

@Ketta: how do you reproduce your the same behavior? Maybe harddisk buffering or somthing else is the reason. For your Problem I would prefer a more elaborated methode wich offers more control. Maybe with

    QImage image;
    QFile file("file.png");
    file.open(QIODevice::WriteOnly);
    QDataStream out(&file);         
    image.save(&file, "PNG");  

This may offer you mor posibilities to control the writing of data. @Kuba:

     connect( &m_futureWatcher, SIGNAL ( finished(  ) ),  this, SLOT( showText  (  )) );

...... and

      m_futureFileReader = QtConcurrent::run(readFile, pIODevice );        
      m_futureWatcher.setFuture(m_futureFileReader);

As a example for reading textfiles doesn't seem very bulky to me.

Kombinator
  • 107
  • 6