Reputation: 485
I'm trying to use QtConcurrent::run to execute a function within my MainWindow class, so that the UI remains responsive during the calculations. Here is how I implemented it:
void MainWindow::on_calculate_pb_clicked()
{
QFuture<void> future = QtConcurrent::run(this,&MainWindow::calculation);
}
void MainWindow::calculation()
{
progressBar->show();
loadMap();
integral.clear();
positions.clear();
offset.clear();
lines = 0;
for(int i=0;i<paths.size();i++)
{
if(i ==0)
{
lines = countNumberOfLines(paths.at(i));
}
double file = i+1;
ui->statusBar->showMessage(QString("Processing file %1 of %2").arg(file).arg(paths.size()));
calculateIntegral(paths.at(i));
offset.push_back(ui->tableWidget->item(i,1)->text().toDouble());
}
makePositionVector();
plotData(ui->customPlot);
ui->export_pb->setEnabled(true);
progressBar->hide();
ui->statusBar->showMessage("Done",3000);
}
void MainWindow::calculateIntegral(QString path)
{
QVector<double> mappeddata,tempdata;
mappeddata.resize(map.size());
tempdata.resize(numberofdetectors);
double currentLine = 0;
QFile File(path);
if(File.exists()==true){
File.open(QIODevice::ReadOnly);
QTextStream in(&File);
double val;
while(!in.atEnd())
{
for(int j = 0;j<numberofdetectors;j++)
{
in >> val;
tempdata[j]+=val;
currentLine++;
double progress = currentLine/lines*100;
progressBar->setValue(progress);
}
}
for(int i =0;i<map.size();i++)
{
mappeddata[i] = tempdata.at(map.at(i));
}
for(int k = 0;k<numberofdetectors; k++)
{
integral.push_back(mappeddata.at(k));
}
}
File.close();
}
It works fine and the UI is responsive and the progress bar is updated correctly, however in the output I receive the error "QObject::setParent: Cannot set parent, new parent is in a different thread" many times, from something that is executing in a loop.
Any ideas what is causing this, or suggestions for a better implementation of QtConcurrent::run?
Thanks
Upvotes: 2
Views: 1292
Reputation: 98425
You can't be touching any Qt-provided QWidget
objects from the worker thread, since most of their methods are not thread-safe.
Instead, a way to tackle this is to do the computations in the worker, and then submit functors that update the state to the main thread. See this answer for details.
Your code would then become:
void MainWindow::calculation()
{
postToThread([this]{ progressBar->show(); });
loadMap();
integral.clear();
positions.clear();
offset.clear();
lines = 0;
for(int i=0;i<paths.size();i++)
{
if (i == 0)
lines = countNumberOfLines(paths.at(i));
auto file = i+1;
postToThread([this]{
ui->statusBar->showMessage(
tr("Processing file %1 of %2").arg(file).arg(paths.size()));
});
calculateIntegral(paths.at(i));
postToThread([this]{
offset.push_back(ui->tableWidget->item(i,1)->text().toDouble());
});
}
makePositionVector();
postToThread([this]{
plotData(ui->customPlot);
ui->export_pb->setEnabled(true);
progressBar->hide();
ui->statusBar->showMessage("Done",3000);
});
}
Modify the calculateIntegral
in a similar fashion, but make sure that you don't emit progress updates too often.
Also make sure that the members that you update from the worker are not accessed elsewhere by the UI code. This can be hard since you mix UI and computations. Instead, abstract the worker out to a QObject
that has no UI, and interface it to other code via signals that indicate progress/status. You will still use QtConcurrent::run
within that object, but it becomes simpler to assure that no other threads access that object's private state.
To push finished results out of the worker functor, you can emit a signal that carries the results. The Data
type should be cheap to copy, e.g. you can implement it using QSharedData
/QSharedDataPointer
. Or you can hold it via a QSharedPointer
.
class Computation : public QObject {
Q_OBJECT
void work() {
Data data;
... // updates data
emit finished(data);
}
public:
Q_SLOT void compute() {
QtConcurrent::run(&Worker::work, this);
}
Q_SIGNAL void finished(Data data);
};
You can also store the results in the object, and pay attention that they are not accessed while the computations are active:
class Computation : public QObject {
Q_OBJECT
bool m_active { false };
Data m_data;
void work() {
... // updates m_data
m_active = false;
}
public:
Q_SLOT void compute() {
m_active = true;
QtConcurrent::run(&Worker::work, this);
}
const Data & data() const {
Q_ASSERT(! m_active);
return m_data;
}
};
Of course, if you store the reference to data()
in the main thread and then call compute()
, you'll have undefined behavior, so don't do that.
If any of the data types are an implicitly shared container, like QVector
or QString
, you should return them by value, and any access will be thread-safe:
QVector<MyData> data() const {
Q_ASSERT(! m_active);
return m_data;
}
Note that QFile
is a proper C++ class. It releases any resources held when it gets destructed. Closing the file manually is unnecessary: the compiler is supposed to help you here, that's the whole point of C++'s object model as compared to e.g. Java's.
Upvotes: 2