Discussion:
[Interest] QProgressDialog not showing & processEvents()
Etienne Sandré-Chardonnal
2013-11-15 13:05:48 UTC
Permalink
Dear all,

I have a long file saving function (gigabytes - several minutes) and I am
trying to use QProcessDialog. I am using it the Modal way as explained in
the documentation. However, the dialog appears after about 50 seconds,
despite having set the minimumDuration to 0, and called setValue() several
times before.

Googling for the issue says I need to call QApplication::processEvents
regularly in order for the GUI to refresh. However, the doc says :

"If the progress dialog is modal
(seeQProgressDialog::QProgressDialog<qthelp://com.trolltech.qt.481/qdoc/qprogressdialog.html#QProgressDialog>()),
setValue() callsQApplication::processEvents<qthelp://com.trolltech.qt.481/qdoc/qcoreapplication.html#processEvents>(),
so take care that this does not cause undesirable re-entrancy in your code"

So I'm puzzled... Do I need to call it, or is it already called by
setValue? In the example there are no explicit calls to processEvents().

Qt 4.8.1

Thanks!

Etienne
André Somers
2013-11-18 07:09:41 UTC
Permalink
Post by Etienne Sandré-Chardonnal
Dear all,
I have a long file saving function (gigabytes - several minutes) and I
am trying to use QProcessDialog. I am using it the Modal way as
explained in the documentation. However, the dialog appears after
about 50 seconds, despite having set the minimumDuration to 0, and
called setValue() several times before.
Googling for the issue says I need to call QApplication::processEvents
"If the progress dialog is modal (seeQProgressDialog::QProgressDialog
<qthelp://com.trolltech.qt.481/qdoc/qprogressdialog.html#QProgressDialog>()),
setValue() callsQApplication::processEvents
<qthelp://com.trolltech.qt.481/qdoc/qcoreapplication.html#processEvents>(),
so take care that this does not cause undesirable re-entrancy in your code"
So I'm puzzled... Do I need to call it, or is it already called by
setValue? In the example there are no explicit calls to processEvents().
Qt 4.8.1
It is *not* called by setValue. That is a Good Thing(TM). In fact, I
think the whole call is dangerous to begin with.

You have two viable approaches here:
* break up the saving method into chunks. That is, do a small part of
the work of the saving, queue the next piece of work to be done, and
return to the eventloop. The eventloop will then trigger the next queued
piece of the work being done. This way, there is no need for
processEvents() at all.
* move your saving code to a worker object, and execute it in a separate
thread. Also, no need to use processEvents, as the heavy lifting is done
outside of the GUI thread.

Note that this does not only go for saving or loading, but it goes for
any task that may block your GUI thread for a substantial amount of time.

André
Mandeep Sandhu
2013-11-18 07:59:14 UTC
Permalink
* break up the saving method into chunks. That is, do a small part of the
work of the saving, queue the next piece of work to be done, and return to
the eventloop. The eventloop will then trigger the next queued piece of the
work being done. This way, there is no need for processEvents() at all.
* move your saving code to a worker object, and execute it in a separate
thread. Also, no need to use processEvents, as the heavy lifting is done
outside of the GUI thread.
I'd too suggest the separate thread method as it's more generic and
will across different scenario's.

The only thing that you need to worry about is that your thread's
actions are in sync with what the GUI is showing. Eg: in case you
provide a cancel button, you'll have to inform the worker thread to
abort and wait for it to exit cleanly.

HTH,
-mandeep
Bo Thorsen
2013-11-18 08:27:35 UTC
Permalink
Post by Mandeep Sandhu
* break up the saving method into chunks. That is, do a small part of the
work of the saving, queue the next piece of work to be done, and return to
the eventloop. The eventloop will then trigger the next queued piece of the
work being done. This way, there is no need for processEvents() at all.
* move your saving code to a worker object, and execute it in a separate
thread. Also, no need to use processEvents, as the heavy lifting is done
outside of the GUI thread.
I'd too suggest the separate thread method as it's more generic and
will across different scenario's.
The only thing that you need to worry about is that your thread's
actions are in sync with what the GUI is showing. Eg: in case you
provide a cancel button, you'll have to inform the worker thread to
abort and wait for it to exit cleanly.
I strongly object to using a thread for something this simple. Threading
is to be avoided whenever possible, because it's so hard to debug. Use
the chunks method instead. Something like this:

class FileLoader : public QObject {
Q_OBJECT

public:
explicit FileLoader(const QString& fileName) {
m_file(fileName);
m_timer.setSingleShot(false);
m_timer.setInterval(0);
m_timer.start();
connect(&m_timer, SIGNAL(triggered()), SLOT(readChunk()));

m_file.open(QIODevice::ReadOnly));
}

signals:
void done(const QByteArray& bytes);
void failed();
void progress(qreal percent);

private slots:
void readChunk() {
if (!m_file.isOpen()) {
m_timer.stop();
emit failed();
} else {
m_bytes.append(m_file.read(1024));
if (m_bytes.size() == m_file.size()) {
m_timer.stop();
emit done(m_bytes);
} else {
emit progress(100.0 * m_bytes.size() / m_file.size());
}
}
}

private:
QFile m_file;
QTimer m_timer;
QByteArray m_bytes;
};

This was written from memory, don't take it as real code. You can also
consider QBuffer and other things. It's not the purpose here to show
loading of the file.

The point with this is to use the timer to allow the rest of the system
to keep doing everything it should, while the file is loading. Now the
progressbar has the time it needs to show the updates.

This technique is important for Qt people. Chunks with timers is a
standard way of avoiding threads. When threads are necessary, so be it.
But don't introduce them when this could be done instead.

Bo.
--
Bo Thorsen, European Engineering Manager, ICS
Integrated Computer Solutions. Delivering World-Class Applications
http://ics.com/services
André Somers
2013-11-18 09:36:39 UTC
Permalink
Post by Bo Thorsen
Post by Mandeep Sandhu
* break up the saving method into chunks. That is, do a small part of the
work of the saving, queue the next piece of work to be done, and return to
the eventloop. The eventloop will then trigger the next queued piece of the
work being done. This way, there is no need for processEvents() at all.
* move your saving code to a worker object, and execute it in a separate
thread. Also, no need to use processEvents, as the heavy lifting is done
outside of the GUI thread.
I'd too suggest the separate thread method as it's more generic and
will across different scenario's.
The only thing that you need to worry about is that your thread's
actions are in sync with what the GUI is showing. Eg: in case you
provide a cancel button, you'll have to inform the worker thread to
abort and wait for it to exit cleanly.
I strongly object to using a thread for something this simple. Threading
is to be avoided whenever possible, because it's so hard to debug. Use
the chunks method instead.
To me, it really depends how easy splitting the work in chunks is going
to be. Without knowing what is being saved, there is no way to know.
Threading is indeed difficult to debug, but if used localized in the
right way is also not *that* hard to manage. The big benefit is that the
actual saving stays localized in a single simple function. Using the
chunks method will spread it around. You'll end up with local variables,
flags, etc. That might be easier, or it might be harder. It depends on
the situation. That's why I mentioned both approaches in my reply.

We actually _do_ use threading for asynchronous I/O in our application,
and that works rather nicely.

André
Mandeep Sandhu
2013-11-18 09:38:42 UTC
Permalink
Post by Bo Thorsen
I strongly object to using a thread for something this simple. Threading
is to be avoided whenever possible, because it's so hard to debug. Use
There's no black magic involved in threads. Though I agree it might be
an overkill in some cases, not so sure about this case.
Post by Bo Thorsen
class FileLoader : public QObject {
Q_OBJECT
explicit FileLoader(const QString& fileName) {
m_file(fileName);
m_timer.setSingleShot(false);
m_timer.setInterval(0);
m_timer.start();
connect(&m_timer, SIGNAL(triggered()), SLOT(readChunk()));
m_file.open(QIODevice::ReadOnly));
}
void done(const QByteArray& bytes);
void failed();
void progress(qreal percent);
void readChunk() {
if (!m_file.isOpen()) {
m_timer.stop();
emit failed();
} else {
m_bytes.append(m_file.read(1024));
The read/write chunk size needs to be selected with care. The exact
answer will depend on the underlying filesystem in use. I think it
should not be smaller than the file system block size.

-mandeep
David Faure
2013-11-20 11:30:34 UTC
Permalink
Post by André Somers
It is *not* called by setValue. That is a Good Thing(TM).
I strongly encourage reading code or documentation before making such
answers...

\warning If the progress dialog is modal
(see QProgressDialog::QProgressDialog()),
setValue() calls QApplication::processEvents(), so take care that
this does not cause undesirable re-entrancy in your code. For example,
don't use a QProgressDialog inside a paintEvent()!


void QProgressDialog::setValue(int progress)
{
Q_D(QProgressDialog);
if (progress == d->bar->value()
|| (d->bar->value() == -1 && progress == d->bar->maximum()))
return;

d->bar->setValue(progress);

if (d->shown_once) {
if (isModal())
QApplication::processEvents();
} else {
[...]
}

This is not a bug however, it's the expected use case for QProgressDialog.
The usually-very-dangerous processEvents() is only midly dangerous because we
are "protected by the progress dialog", i.e. the user cannot close the
mainwindow, for instance. But of course, timers and sockets could still
trigger code that deletes objects etc.

In any case - to come back to the initial question: did you call setValue(0)
upfront?

I just pushed a review request for this to happen automatically, but it's not
merged yet. https://codereview.qt-project.org/71619
--
David Faure | ***@kdab.com | Managing Director KDAB France
KDAB (France) S.A.S., a KDAB Group company
Tel. France +33 (0)4 90 84 08 53, Sweden (HQ) +46-563-540090
KDAB - Qt Experts - Platform-independent software solutions
André Somers
2013-11-20 13:14:13 UTC
Permalink
Post by David Faure
Post by André Somers
It is *not* called by setValue. That is a Good Thing(TM).
I strongly encourage reading code or documentation before making such
answers...
Oops...

/me goes to hide quietly in a corner and be ashamed of himself

André

Konrad Rosenbaum
2013-11-18 07:15:48 UTC
Permalink
Hi,

it is always safe to call processEvents yourself. If the QProgressDialog has
display issues it obviously does not get enough CPU to update itself, calling
processEvents more often may(!!) help.

When I want to see a QProgressDialog early I normally force the issue by
calling show() immediately. That's more reliable than leaving it to the
internal heuristics. Especially if there may be a long time before the first
call to processEvents.

If you are just copying a lot of data it may be a good idea to do this in a
separate thread and just throw the updates (Qt::QueuedConnection!) to the GUI
thread from time to time. This way the GUI remains fully intact. (Hint: if you
delete objects during saving: use deleteLater; if you allocate new stuff do it
in a slot with BlockingQueuedConnection or use moveToThread).



Konrad
Post by Etienne Sandré-Chardonnal
Dear all,
I have a long file saving function (gigabytes - several minutes) and I am
trying to use QProcessDialog. I am using it the Modal way as explained in
the documentation. However, the dialog appears after about 50 seconds,
despite having set the minimumDuration to 0, and called setValue() several
times before.
Googling for the issue says I need to call QApplication::processEvents
"If the progress dialog is modal
(seeQProgressDialog::QProgressDialog<qthelp://com.trolltech.qt.481/qdoc/qpr
ogressdialog.html#QProgressDialog>()), setValue()
callsQApplication::processEvents<qthelp://com.trolltech.qt.481/qdoc/qcorea
pplication.html#processEvents>(), so take care that this does not cause
undesirable re-entrancy in your code"
So I'm puzzled... Do I need to call it, or is it already called by
setValue? In the example there are no explicit calls to processEvents().
Qt 4.8.1
Thanks!
Etienne
Thiago Macieira
2013-11-18 07:48:57 UTC
Permalink
Post by Konrad Rosenbaum
it is always safe to call processEvents yourself.
No, it's not. There are a lot of situations when doing that will cause you
problems elsewhere.
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Loading...