Skip to content

Commit

Permalink
Improved memory managment of algorithm dialogs
Browse files Browse the repository at this point in the history
Ensure that dialogs are always correctly deleted when appropriate.

Also, if an algorithm is running in a background task and hits
an error, we automatically re-show the algorithm dialog and the
associated log for debugging.

Fixes #16858
  • Loading branch information
nyalldawson committed Jan 16, 2018
1 parent bc9d739 commit 87c1986
Show file tree
Hide file tree
Showing 9 changed files with 96 additions and 33 deletions.
10 changes: 10 additions & 0 deletions python/gui/processing/qgsprocessingalgorithmdialogbase.sip
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class QgsProcessingAlgorithmDialogBase : QDialog
%Docstring
Constructor for QgsProcessingAlgorithmDialogBase.
%End
~QgsProcessingAlgorithmDialogBase();

void setAlgorithm( QgsProcessingAlgorithm *algorithm );
%Docstring
Expand Down Expand Up @@ -143,6 +144,9 @@ from this dialog.

protected:

virtual void closeEvent( QCloseEvent *e );


QPushButton *runButton();
%Docstring
Returns the dialog's run button.
Expand Down Expand Up @@ -204,6 +208,12 @@ Returns the dialog's message bar.
void hideShortHelp();
%Docstring
Hides the short help panel.
%End

void setCurrentTask( QgsProcessingAlgRunnerTask *task /Transfer/ );
%Docstring
Sets the current ``task`` running in the dialog. The task will automatically be started
by the dialog. Ownership of ``task`` is transferred to the dialog.
%End

protected slots:
Expand Down
1 change: 0 additions & 1 deletion python/plugins/processing/ProcessingPlugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ def runAlg(file):

alg.setProvider(QgsApplication.processingRegistry().providerById('model'))
dlg = AlgorithmDialog(alg)
dlg.setAttribute(Qt.WA_DeleteOnClose)
dlg.show()
return True

Expand Down
16 changes: 3 additions & 13 deletions python/plugins/processing/gui/AlgorithmDialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,13 @@ class AlgorithmDialog(QgsProcessingAlgorithmDialogBase):
def __init__(self, alg):
super().__init__()
self.feedback_dialog = None
self.task = None

self.setAlgorithm(alg)
self.setMainWidget(self.getParametersPanel(alg, self))

self.runAsBatchButton = QPushButton(QCoreApplication.translate("AlgorithmDialog", "Run as Batch Process…"))
self.runAsBatchButton.clicked.connect(self.runAsBatch)
self.buttonBox().addButton(self.runAsBatchButton, QDialogButtonBox.ResetRole) # reset role to ensure left alignment
QgsApplication.taskManager().taskTriggered.connect(self.taskTriggered)

def getParametersPanel(self, alg, parent):
return ParametersPanel(parent, alg)
Expand Down Expand Up @@ -235,7 +233,6 @@ def accept(self):
self.cancelButton().setEnabled(self.algorithm().flags() & QgsProcessingAlgorithm.FlagCanCancel)

def on_complete(ok, results):
self.task = None
if ok:
feedback.pushInfo(self.tr('Execution completed in {0:0.2f} seconds'.format(time.time() - start_time)))
feedback.pushInfo(self.tr('Results:'))
Expand All @@ -258,9 +255,9 @@ def on_complete(ok, results):
# Make sure the Log tab is visible before executing the algorithm
self.showLog()

self.task = QgsProcessingAlgRunnerTask(self.algorithm(), parameters, context, feedback)
self.task.executed.connect(on_complete)
QgsApplication.taskManager().addTask(self.task)
task = QgsProcessingAlgRunnerTask(self.algorithm(), parameters, context, feedback)
task.executed.connect(on_complete)
self.setCurrentTask(task)
else:
self.feedback_dialog = self.createProgressDialog()
self.feedback_dialog.show()
Expand All @@ -280,13 +277,6 @@ def on_complete(ok, results):
self.messageBar().pushMessage("", self.tr("Wrong or missing parameter value: {0}").format(e.parameter.description()),
level=QgsMessageBar.WARNING, duration=5)

def taskTriggered(self, task):
if task == self.task:
self.show()
self.raise_()
self.setWindowState(self.windowState() & ~Qt.WindowMinimized | Qt.WindowActive)
self.activateWindow()

def finish(self, successful, result, context, feedback):
keepOpen = not successful or ProcessingConfig.getSetting(ProcessingConfig.KEEP_DIALOG_OPEN)

Expand Down
6 changes: 0 additions & 6 deletions python/plugins/processing/gui/ProcessingToolbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,6 @@ def executeAlgorithmAsBatchProcess(self):
dlg = BatchAlgorithmDialog(alg)
dlg.show()
dlg.exec_()
# have to manually delete the dialog - otherwise it's owned by the
# iface mainWindow and never deleted
dlg.deleteLater()

def executeAlgorithm(self):
item = self.algorithmTree.currentItem()
Expand Down Expand Up @@ -298,9 +295,6 @@ def executeAlgorithm(self):
ProcessingConfig.SHOW_RECENT_ALGORITHMS)
if showRecent:
self.addRecentAlgorithms(True)
# have to manually delete the dialog - otherwise it's owned by the
# iface mainWindow and never deleted
# dlg.deleteLater()
else:
feedback = MessageBarProgress()
context = dataobjects.createContext(feedback)
Expand Down
5 changes: 0 additions & 5 deletions python/plugins/processing/gui/ScriptEditorDialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,11 +284,6 @@ def runAlgorithm(self):
prevMapTool = canvas.mapTool()

dlg.show()
dlg.exec_()

# have to manually delete the dialog - otherwise it's owned by the
# iface mainWindow and never deleted
dlg.deleteLater()

if canvas.mapTool() != prevMapTool:
try:
Expand Down
3 changes: 0 additions & 3 deletions python/plugins/processing/gui/menus.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,6 @@ def _executeAlgorithm(alg):
prevMapTool = canvas.mapTool()
dlg.show()
dlg.exec_()
# have to manually delete the dialog - otherwise it's owned by the
# iface mainWindow and never deleted
del dlg
if canvas.mapTool() != prevMapTool:
try:
canvas.mapTool().reset()
Expand Down
3 changes: 0 additions & 3 deletions python/plugins/processing/modeler/ModelerDialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,6 @@ def runModel(self):

dlg = AlgorithmDialog(self.model)
dlg.exec_()
# have to manually delete the dialog - otherwise it's owned by the
# iface mainWindow and never deleted
dlg.deleteLater()

def save(self):
self.saveModel(False)
Expand Down
69 changes: 67 additions & 2 deletions src/gui/processing/qgsprocessingalgorithmdialogbase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#include "qgsgui.h"
#include "processing/qgsprocessingalgorithm.h"
#include "processing/qgsprocessingprovider.h"
#include "qgstaskmanager.h"
#include "processing/qgsprocessingalgrunnertask.h"
#include <QToolButton>
#include <QDesktopServices>
#include <QScrollBar>
Expand Down Expand Up @@ -92,7 +94,7 @@ QgsProcessingAlgorithmDialogBase::QgsProcessingAlgorithmDialogBase( QWidget *par
mSplitterState = splitter->saveState();
splitterChanged( 0, 0 );

connect( mButtonBox, &QDialogButtonBox::rejected, this, &QgsProcessingAlgorithmDialogBase::reject );
connect( mButtonBox, &QDialogButtonBox::rejected, this, &QgsProcessingAlgorithmDialogBase::closeClicked );
connect( mButtonBox, &QDialogButtonBox::accepted, this, &QgsProcessingAlgorithmDialogBase::accept );

// Rename OK button to Run
Expand All @@ -109,6 +111,8 @@ QgsProcessingAlgorithmDialogBase::QgsProcessingAlgorithmDialogBase( QWidget *par
mMessageBar = new QgsMessageBar();
mMessageBar->setSizePolicy( QSizePolicy::Minimum, QSizePolicy::Fixed );
verticalLayout->insertWidget( 0, mMessageBar );

connect( QgsApplication::taskManager(), &QgsTaskManager::taskTriggered, this, &QgsProcessingAlgorithmDialogBase::taskTriggered );
}

void QgsProcessingAlgorithmDialogBase::setAlgorithm( QgsProcessingAlgorithm *algorithm )
Expand Down Expand Up @@ -271,6 +275,47 @@ void QgsProcessingAlgorithmDialogBase::linkClicked( const QUrl &url )
QDesktopServices::openUrl( url.toString() );
}

void QgsProcessingAlgorithmDialogBase::algExecuted( bool successful, const QVariantMap & )
{
mAlgorithmTask = nullptr;

if ( !successful )
{
// show dialog to display errors
show();
raise();
setWindowState( ( windowState() & ~Qt::WindowMinimized ) | Qt::WindowActive );
activateWindow();
showLog();
}
else
{
// delete dialog if closed
if ( !isVisible() )
{
deleteLater();
}
}
}

void QgsProcessingAlgorithmDialogBase::taskTriggered( QgsTask *task )
{
if ( task == mAlgorithmTask )
{
show();
raise();
setWindowState( ( windowState() & ~Qt::WindowMinimized ) | Qt::WindowActive );
activateWindow();
showLog();
}
}

void QgsProcessingAlgorithmDialogBase::closeClicked()
{
reject();
close();
}

void QgsProcessingAlgorithmDialogBase::reportError( const QString &error )
{
setInfo( error, true );
Expand Down Expand Up @@ -323,6 +368,19 @@ QDialog *QgsProcessingAlgorithmDialogBase::createProgressDialog()
return dialog;
}

void QgsProcessingAlgorithmDialogBase::closeEvent( QCloseEvent *e )
{
QDialog::closeEvent( e );

if ( !mAlgorithmTask )
{
// when running a background task, the dialog is kept around and deleted only when the task
// completes. But if not running a task, we auto cleanup (later - gotta give callers a chance
// to retrieve results and execution status).
deleteLater();
}
}

void QgsProcessingAlgorithmDialogBase::setPercentage( double percent )
{
// delay setting maximum progress value until we know algorithm reports progress
Expand Down Expand Up @@ -398,6 +456,13 @@ void QgsProcessingAlgorithmDialogBase::hideShortHelp()
textShortHelp->setVisible( false );
}

void QgsProcessingAlgorithmDialogBase::setCurrentTask( QgsProcessingAlgRunnerTask *task )
{
mAlgorithmTask = task;
connect( mAlgorithmTask, &QgsProcessingAlgRunnerTask::executed, this, &QgsProcessingAlgorithmDialogBase::algExecuted );
QgsApplication::taskManager()->addTask( mAlgorithmTask );
}

void QgsProcessingAlgorithmDialogBase::setInfo( const QString &message, bool isError, bool escapeHtml )
{
if ( isError )
Expand All @@ -417,8 +482,8 @@ void QgsProcessingAlgorithmDialogBase::setInfo( const QString &message, bool isE
QgsProcessingAlgorithmProgressDialog::QgsProcessingAlgorithmProgressDialog( QWidget *parent )
: QDialog( parent )
{
setWindowFlags( Qt::Dialog ); // hide close button
setupUi( this );
QgsGui::enableAutoGeometryRestore( this );
}

QProgressBar *QgsProcessingAlgorithmProgressDialog::progressBar()
Expand Down
16 changes: 16 additions & 0 deletions src/gui/processing/qgsprocessingalgorithmdialogbase.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ class QgsProcessingAlgorithm;
class QToolButton;
class QgsProcessingAlgorithmDialogBase;
class QgsMessageBar;
class QgsProcessingAlgRunnerTask;
class QgsTask;

#ifndef SIP_RUN

Expand Down Expand Up @@ -87,6 +89,7 @@ class GUI_EXPORT QgsProcessingAlgorithmDialogBase : public QDialog, private Ui::
* Constructor for QgsProcessingAlgorithmDialogBase.
*/
QgsProcessingAlgorithmDialogBase( QWidget *parent = nullptr, Qt::WindowFlags flags = nullptr );
~QgsProcessingAlgorithmDialogBase();

/**
* Sets the \a algorithm to run in the dialog.
Expand Down Expand Up @@ -189,6 +192,8 @@ class GUI_EXPORT QgsProcessingAlgorithmDialogBase : public QDialog, private Ui::

protected:

void closeEvent( QCloseEvent *e ) override;

/**
* Returns the dialog's run button.
*/
Expand Down Expand Up @@ -248,6 +253,12 @@ class GUI_EXPORT QgsProcessingAlgorithmDialogBase : public QDialog, private Ui::
*/
void hideShortHelp();

/**
* Sets the current \a task running in the dialog. The task will automatically be started
* by the dialog. Ownership of \a task is transferred to the dialog.
*/
void setCurrentTask( QgsProcessingAlgRunnerTask *task SIP_TRANSFER );

protected slots:

/**
Expand All @@ -262,6 +273,9 @@ class GUI_EXPORT QgsProcessingAlgorithmDialogBase : public QDialog, private Ui::

void splitterChanged( int pos, int index );
void linkClicked( const QUrl &url );
void algExecuted( bool successful, const QVariantMap &results );
void taskTriggered( QgsTask *task );
void closeClicked();

private:

Expand All @@ -275,6 +289,8 @@ class GUI_EXPORT QgsProcessingAlgorithmDialogBase : public QDialog, private Ui::
QVariantMap mResults;
QWidget *mMainWidget = nullptr;
QgsProcessingAlgorithm *mAlgorithm = nullptr;
QgsProcessingAlgRunnerTask *mAlgorithmTask = nullptr;

bool mHelpCollapsed = false;

QString formatHelp( QgsProcessingAlgorithm *algorithm );
Expand Down

0 comments on commit 87c1986

Please sign in to comment.