Project directory structure

[Thiago Macieira]

On quarta-feira, 3 de abril de 2013 12.13.40, Dirk Hohndel wrote:
> > For Subsurface, the notification that the download from the divecomputer
> > finished would be a signal. Since that code is in C, we'd simply do with a
> > regular callback that in turn emits the signal. The UI will react soon.
> > Similarly for a progress bar, if any: the callback from the download
> > routine with the update emits a signal.
> 
> So how would this be handled from C code? Right now (because of
> limitations with the way Gtk does things) the main thread does something
> like this:
> 
> 	while (!import_thread_done) {
> 		if (!import_thread_cancelled) {
> 			int result;
> 			g_timeout_add(100, timeout_func, dialog);
> 			update_progressbar(&data->progress, progress_bar_fraction);
> 			update_progressbar_text(&data->progress, progress_bar_text);
> 			result = gtk_dialog_run(dialog);
> 			switch (result) {
> 			case GTK_RESPONSE_CANCEL:
> 				import_thread_cancelled = TRUE;
> 				progress_bar_text = _("Cancelled...");
> 				break;
> 			default:
> 				/* nothing */
> 				break;
> 			}
> 		} else {
> 			update_progressbar(&data->progress, progress_bar_fraction);
> 			update_progressbar_text(&data->progress, progress_bar_text);
> 			usleep(100000);
> 		}
> 	}

Hmm... let me see... showing a modal dialog with a progress bar and a Cancel 
button that tells the import thread to stop. Right?

sleep, usleep and other arbitrary timeouts are evil. Don't tell Arjan you're 
waking up 10 times a second to update a progress bar. So the first question is: 
is there a callback from the download thread when there's and update to the 
progress?

If there is no such callback, the UI thread will need to do the polling. The 
code could use QProgressDialog, which provides the dialog and the cancel 
button, and look very similar to the current Gtk code. Or it could change a 
little by using a QTimer and letting the event loop run unimpeded.

If there is such a callback, we can connect the adapted Qt signals to a 
QProgressDialog and to the main window.

Something like:

// in the main window's constructor:
    // range is [0..100]
    progress_dialog = new QProgressDialog(_("Downloading..."), 
        _("Stop download"), 0, 100, this);
    connect(progress_dialog, SIGNAL(canceled()), SLOT(cancel_download());

#if we have to poll
    timer.setInterval(100);
    connect(&timer, SIGNAL(timeout()), SLOT(update_progress_bar()));
#endif


// slots
void MainWindow::cancel_download()
{
    download_job->cancel();
    progress_dialog->hide();
    // dispose of download_job somehow later
}

void MainWindow::on_download_button_clicked()
{
    download_job = new DownloadJob;
    connect(download_job, SIGNAL(progress(int)), 
        progress_dialog, SLOT(setValue(int)));
    connect(download_job, SIGNAL(updated_text(QString)), 
        progress_dialog, SLOT(setLabelText(QString)));
    connect(download_job, SIGNAL(finished()), SLOT(process_results()));

    progress_dialog->setValue(0);
    download->start();
}

The above might look a little more complex, but it has two added benefits 
compared to the Gtk code you pasted:
 1) no polling, the main thread can sleep for as long as it wants
 2) no nested event loops

> And we use the global variables import_thread_cancelled,
> import_thread_done progress_bar_text and progress_bar_fraction to
> communicate from the download thread to the UI thread.
> The stupid timeout mechanism is working around Gtk's inability to do the
> updates from a separate thread (I understand the same is true for Qt).

Indeed. But the code above neatly works around the issue by using signals and 
slots. Qt will take care of crossing the thread barrier for you. All slots 
associated with MainWindow or QProgressDialog will be executed in the main 
(UI) thread automatically.

> We use pthreads right now:
> 
> 	pthread_create(&pthread, NULL, pthread_wrapper, data);
> 
> and then
> 
> 	if (pthread_join(pthread, &retval) < 0)
> 		retval = _("Odd pthread error return");

Even on Windows?

If the threading code is inside libdivecomputer, I'd leave it where it is. If 
it's in Subsurface, I'd recommend wrapping it with that DownloadJob class 
above. It would create a QThread and provide a few C callbacks for the actual 
download (written in C). The callbacks would emit the signals I listed.

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center
      PGP/GPG: 0x6EF45358; fingerprint:
      E067 918B B660 DBD1 105C  966C 33F5 F005 6EF4 5358
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.hohndel.org/pipermail/subsurface/attachments/20130403/9ec66961/attachment-0001.sig>