Skip to content

Commit

Permalink
Do not hold pointer to dialog which can become invalidated
Browse files Browse the repository at this point in the history
  • Loading branch information
robertapplin committed Jan 29, 2024
1 parent 064c230 commit 1dfba42
Show file tree
Hide file tree
Showing 18 changed files with 62 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ ConvFitAddWorkspaceDialog::ConvFitAddWorkspaceDialog(QWidget *parent) : QDialog(

connect(m_uiForm.dsWorkspace, SIGNAL(dataReady(const QString &)), this, SLOT(workspaceChanged(const QString &)));
connect(m_uiForm.ckAllSpectra, SIGNAL(stateChanged(int)), this, SLOT(selectAllSpectra(int)));
connect(m_uiForm.pbAdd, SIGNAL(clicked()), this, SIGNAL(addData()));
connect(m_uiForm.pbAdd, SIGNAL(clicked()), this, SLOT(emitAddData()));
connect(m_uiForm.pbClose, SIGNAL(clicked()), this, SLOT(close()));
}

Expand Down Expand Up @@ -128,6 +128,8 @@ void ConvFitAddWorkspaceDialog::workspaceChanged(const QString &workspaceName) {
setAllSpectraSelectionEnabled(false);
}

void ConvFitAddWorkspaceDialog::emitAddData() { emit addData(this); }

void ConvFitAddWorkspaceDialog::setWorkspace(const std::string &workspace) {
setAllSpectraSelectionEnabled(true);
if (m_uiForm.ckAllSpectra->isChecked()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@ class MANTIDQT_INELASTIC_DLL ConvFitAddWorkspaceDialog : public QDialog, public
void updateSelectedSpectra() override;

signals:
void addData();
void addData(MantidWidgets::IAddWorkspaceDialog *dialog);

private slots:
void selectAllSpectra(int state);
void workspaceChanged(const QString &workspaceName);
void emitAddData();

private:
void setWorkspace(const std::string &workspace);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ ConvFitDataView::ConvFitDataView(const QStringList &headers, QWidget *parent) :

void ConvFitDataView::showAddWorkspaceDialog() {
auto dialog = new ConvFitAddWorkspaceDialog(parentWidget());
connect(dialog, SIGNAL(addData()), this, SLOT(notifyAddData()));
connect(dialog, SIGNAL(addData(MantidWidgets::IAddWorkspaceDialog *)), this,
SLOT(notifyAddData(MantidWidgets::IAddWorkspaceDialog *)));

dialog->setAttribute(Qt::WA_DeleteOnClose);
dialog->setWSSuffices(m_wsSampleSuffixes);
Expand All @@ -44,8 +45,6 @@ void ConvFitDataView::showAddWorkspaceDialog() {
dialog->setResolutionFBSuffices(m_fbResolutionSuffixes);
dialog->updateSelectedSpectra();
dialog->show();

m_addWorkspaceDialog = dialog;
}

void ConvFitDataView::addTableEntry(size_t row, FitDataRow newRow) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ FqFitAddWorkspaceDialog::FqFitAddWorkspaceDialog(QWidget *parent) : QDialog(pare
connect(m_uiForm.dsWorkspace, SIGNAL(dataReady(const QString &)), this, SLOT(emitWorkspaceChanged(const QString &)));
connect(m_uiForm.cbParameterType, SIGNAL(currentIndexChanged(const QString &)), this,
SLOT(emitParameterTypeChanged(const QString &)));
connect(m_uiForm.pbAdd, SIGNAL(clicked()), this, SIGNAL(addData()));
connect(m_uiForm.pbAdd, SIGNAL(clicked()), this, SLOT(emitAddData()));
connect(m_uiForm.pbClose, SIGNAL(clicked()), this, SLOT(close()));
}

Expand Down Expand Up @@ -71,4 +71,6 @@ void FqFitAddWorkspaceDialog::emitParameterTypeChanged(const QString &type) {
emit parameterTypeChanged(this, type.toStdString());
}

void FqFitAddWorkspaceDialog::emitAddData() { emit addData(this); }

} // namespace MantidQt::CustomInterfaces::IDA
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,16 @@ class MANTIDQT_INELASTIC_DLL FqFitAddWorkspaceDialog : public QDialog, public Ma

void updateSelectedSpectra() override{};

public slots:
void emitWorkspaceChanged(const QString &name);
void emitParameterTypeChanged(const QString &index);

signals:
void addData();
void addData(MantidWidgets::IAddWorkspaceDialog *dialog);
void workspaceChanged(FqFitAddWorkspaceDialog *dialog, const std::string &workspace);
void parameterTypeChanged(FqFitAddWorkspaceDialog *dialog, const std::string &type);

private slots:
void emitWorkspaceChanged(const QString &name);
void emitParameterTypeChanged(const QString &index);
void emitAddData();

private:
Ui::FqFitAddWorkspaceDialog m_uiForm;
};
Expand Down
5 changes: 2 additions & 3 deletions qt/scientific_interfaces/Inelastic/Analysis/FqFitDataView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ FqFitDataView::FqFitDataView(const QStringList &headers, QWidget *parent) : Indi

void FqFitDataView::showAddWorkspaceDialog() {
auto dialog = new FqFitAddWorkspaceDialog(parentWidget());
connect(dialog, SIGNAL(addData()), this, SLOT(notifyAddData()));
connect(dialog, SIGNAL(addData(MantidWidgets::IAddWorkspaceDialog *)), this,
SLOT(notifyAddData(MantidWidgets::IAddWorkspaceDialog *)));
connect(dialog, SIGNAL(workspaceChanged(FqFitAddWorkspaceDialog *, const std::string &)), this,
SLOT(notifyWorkspaceChanged(FqFitAddWorkspaceDialog *, const std::string &)));
connect(dialog, SIGNAL(parameterTypeChanged(FqFitAddWorkspaceDialog *, const std::string &)), this,
Expand All @@ -49,8 +50,6 @@ void FqFitDataView::showAddWorkspaceDialog() {
dialog->setFBSuffices(m_fbSampleSuffixes);
dialog->updateSelectedSpectra();
dialog->show();

m_addWorkspaceDialog = dialog;
}

void FqFitDataView::notifyAddClicked() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ void IndirectFitDataPresenter::handleAddData(MantidWidgets::IAddWorkspaceDialog
m_tab->handleDataChanged();
} catch (const std::runtime_error &ex) {
displayWarning(ex.what());
} catch (const std::invalid_argument &ex) {
displayWarning(ex.what());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,19 +199,20 @@ void IndirectFitDataView::setResolutionWSSuffices(const QStringList &suffixes) {
void IndirectFitDataView::setResolutionFBSuffices(const QStringList &suffixes) { m_fbResolutionSuffixes = suffixes; }

void IndirectFitDataView::showAddWorkspaceDialog() {
auto dialog = new MantidWidgets::AddWorkspaceDialog(parentWidget());
connect(dialog, SIGNAL(addData()), this, SLOT(notifyAddData()));
auto dialog = std::make_unique<MantidWidgets::AddWorkspaceDialog>(parentWidget());
connect(dialog.get(), SIGNAL(addData(MantidWidgets::IAddWorkspaceDialog *)), this,
SLOT(notifyAddData(MantidWidgets::AddWorkspaceDialog *)));

dialog->setAttribute(Qt::WA_DeleteOnClose);
dialog->setWSSuffices(m_wsSampleSuffixes);
dialog->setFBSuffices(m_fbSampleSuffixes);
dialog->updateSelectedSpectra();
dialog->show();

m_addWorkspaceDialog = dialog;
}

void IndirectFitDataView::notifyAddData() { m_presenter->handleAddData(m_addWorkspaceDialog); }
void IndirectFitDataView::notifyAddData(MantidWidgets::IAddWorkspaceDialog *dialog) {
m_presenter->handleAddData(dialog);
}

void IndirectFitDataView::notifyRemoveClicked() { m_presenter->handleRemoveClicked(); }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class MANTIDQT_INELASTIC_DLL IndirectFitDataView : public QTabWidget, public IIn
void displayWarning(const std::string &warning) override;

protected slots:
void notifyAddData();
void notifyAddData(MantidWidgets::IAddWorkspaceDialog *dialog);

protected:
IndirectFitDataView(const QStringList &headers, QWidget *parent);
Expand All @@ -66,7 +66,6 @@ protected slots:
QStringList m_wsResolutionSuffixes;
QStringList m_fbResolutionSuffixes;

MantidWidgets::IAddWorkspaceDialog *m_addWorkspaceDialog;
IIndirectFitDataPresenter *m_presenter;

protected slots:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ void InelasticDataManipulationElwinTabView::notifyRemoveDataClicked() { m_presen
void InelasticDataManipulationElwinTabView::notifyAddWorkspaceDialog() { showAddWorkspaceDialog(); }

void InelasticDataManipulationElwinTabView::showAddWorkspaceDialog() {
if (m_addWorkspaceDialog)
return;

auto dialog = new MantidWidgets::AddWorkspaceDialog(parentWidget());
connect(dialog, SIGNAL(addData()), this, SLOT(notifyAddData()));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@ class EXPORT_OPT_MANTIDQT_COMMON AddWorkspaceDialog : public QDialog, public IAd
std::string getFileName() const;

signals:
void addData();
void addData(MantidWidgets::IAddWorkspaceDialog *dialog);

private slots:
void selectAllSpectra(int state);
void workspaceChanged(const QString &workspaceName);
void emitAddData();

private:
void setWorkspace(const std::string &workspace);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ class EXPORT_OPT_MANTIDQT_COMMON FitScriptGeneratorPresenter : public IFitScript
[[maybe_unused]] std::string const &arg2 = "") override;
void notifyPresenter(ViewEvent const &event, std::vector<std::string> const &vec) override;
void notifyPresenter(ViewEvent const &event, FittingMode fittingMode) override;
void handleAddDomainAccepted(std::vector<Mantid::API::MatrixWorkspace_const_sptr> const &workspaces,
FunctionModelSpectra const &workspaceIndices) override;

void openFitScriptGenerator() override;

Expand All @@ -50,7 +52,6 @@ class EXPORT_OPT_MANTIDQT_COMMON FitScriptGeneratorPresenter : public IFitScript
void handleADSRenameEvent(std::string const &workspaceName, std::string const &newName);
void handleRemoveDomainClicked();
void handleAddDomainClicked();
void handleAddDomainAccepted();
void handleSelectionChanged();
void handleStartXChanged();
void handleEndXChanged();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ class EXPORT_OPT_MANTIDQT_COMMON FitScriptGeneratorView : public IFitScriptGener
double endX) override;

void openAddWorkspaceDialog() override;
[[nodiscard]] std::vector<Mantid::API::MatrixWorkspace_const_sptr> getDialogWorkspaces() override;
[[nodiscard]] FunctionModelSpectra getDialogWorkspaceIndices() const override;
[[nodiscard]] std::vector<Mantid::API::MatrixWorkspace_const_sptr>
getDialogWorkspaces(MantidWidgets::IAddWorkspaceDialog *dialog) override;

void openEditLocalParameterDialog(std::string const &parameter, std::vector<std::string> const &workspaceNames,
std::vector<std::string> const &domainNames, std::vector<double> const &values,
Expand Down Expand Up @@ -112,7 +112,6 @@ class EXPORT_OPT_MANTIDQT_COMMON FitScriptGeneratorView : public IFitScriptGener
FitScriptGeneratorDataTable *tableWidget() const override { return m_dataTable.get(); }
QPushButton *removeButton() const override { return m_ui.pbRemoveDomain; }
QPushButton *addWorkspaceButton() const override { return m_ui.pbAddDomain; }
AddWorkspaceDialog *addWorkspaceDialog() const override { return m_addWorkspaceDialog; }
QPushButton *generateScriptToFileButton() const override { return m_ui.pbGenerateScriptToFile; }
QPushButton *generateScriptToClipboardButton() const override { return m_ui.pbGenerateScriptToClipboard; }

Expand All @@ -121,7 +120,7 @@ private slots:
void notifyADSClearEvent();
void notifyADSRenameEvent(std::string const &workspaceName, std::string const &newName);

void addWorkspaceDialogAccepted();
void addWorkspaceDialogAccepted(MantidWidgets::IAddWorkspaceDialog *dialog);

void onRemoveDomainClicked();
void onAddDomainClicked();
Expand Down Expand Up @@ -153,7 +152,6 @@ private slots:
void setFittingMode(FittingMode fittingMode);

IFitScriptGeneratorPresenter *m_presenter;
AddWorkspaceDialog *m_addWorkspaceDialog;
std::unique_ptr<FitScriptGeneratorDataTable> m_dataTable;
std::unique_ptr<FunctionTreeView> m_functionTreeView;
std::unique_ptr<FitScriptOptionsBrowser> m_fitOptionsBrowser;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@

#include "DllOption.h"
#include "IFitScriptGeneratorView.h"
#include "MantidAPI/MatrixWorkspace_fwd.h"
#include "MantidQtWidgets/Common/FittingGlobals.h"
#include "MantidQtWidgets/Common/FittingMode.h"
#include "MantidQtWidgets/Common/FunctionModelSpectra.h"

#include <string>
#include <vector>
Expand All @@ -27,6 +29,8 @@ class EXPORT_OPT_MANTIDQT_COMMON IFitScriptGeneratorPresenter {
[[maybe_unused]] std::string const &arg2 = "") = 0;
virtual void notifyPresenter(ViewEvent const &event, std::vector<std::string> const &vec) = 0;
virtual void notifyPresenter(ViewEvent const &event, FittingMode fittingMode) = 0;
virtual void handleAddDomainAccepted(std::vector<Mantid::API::MatrixWorkspace_const_sptr> const &workspaces,
FunctionModelSpectra const &workspaceIndices) = 0;

virtual void openFitScriptGenerator() = 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ namespace MantidWidgets {

class AddWorkspaceDialog;
class FitScriptGeneratorDataTable;
class IAddWorkspaceDialog;
class IFitScriptGeneratorPresenter;
struct GlobalParameter;
struct GlobalTie;
Expand All @@ -42,7 +43,6 @@ class EXPORT_OPT_MANTIDQT_COMMON IFitScriptGeneratorView : public API::MantidWid
ADSRenameEvent,
RemoveDomainClicked,
AddDomainClicked,
AddDomainAccepted,
StartXChanged,
EndXChanged,
SelectionChanged,
Expand Down Expand Up @@ -89,8 +89,8 @@ class EXPORT_OPT_MANTIDQT_COMMON IFitScriptGeneratorView : public API::MantidWid
double endX) = 0;

virtual void openAddWorkspaceDialog() = 0;
[[nodiscard]] virtual std::vector<Mantid::API::MatrixWorkspace_const_sptr> getDialogWorkspaces() = 0;
[[nodiscard]] virtual FunctionModelSpectra getDialogWorkspaceIndices() const = 0;
[[nodiscard]] virtual std::vector<Mantid::API::MatrixWorkspace_const_sptr>
getDialogWorkspaces(MantidWidgets::IAddWorkspaceDialog *dialog) = 0;

virtual void openEditLocalParameterDialog(std::string const &parameter,
std::vector<std::string> const &workspaceNames,
Expand Down Expand Up @@ -128,7 +128,6 @@ class EXPORT_OPT_MANTIDQT_COMMON IFitScriptGeneratorView : public API::MantidWid
virtual FitScriptGeneratorDataTable *tableWidget() const = 0;
virtual QPushButton *removeButton() const = 0;
virtual QPushButton *addWorkspaceButton() const = 0;
virtual AddWorkspaceDialog *addWorkspaceDialog() const = 0;
virtual QPushButton *generateScriptToFileButton() const = 0;
virtual QPushButton *generateScriptToClipboardButton() const = 0;
};
Expand Down
4 changes: 3 additions & 1 deletion qt/widgets/common/src/AddWorkspaceDialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ AddWorkspaceDialog::AddWorkspaceDialog(QWidget *parent) : QDialog(parent) {

connect(m_uiForm.dsWorkspace, SIGNAL(dataReady(const QString &)), this, SLOT(workspaceChanged(const QString &)));
connect(m_uiForm.ckAllSpectra, SIGNAL(stateChanged(int)), this, SLOT(selectAllSpectra(int)));
connect(m_uiForm.pbAdd, SIGNAL(clicked()), this, SIGNAL(addData()));
connect(m_uiForm.pbAdd, SIGNAL(clicked()), this, SLOT(emitAddData()));
connect(m_uiForm.pbClose, SIGNAL(clicked()), this, SLOT(close()));
}

Expand Down Expand Up @@ -113,6 +113,8 @@ void AddWorkspaceDialog::workspaceChanged(const QString &workspaceName) {
setAllSpectraSelectionEnabled(false);
}

void AddWorkspaceDialog::emitAddData() { emit addData(this); }

void AddWorkspaceDialog::setWorkspace(const std::string &workspace) {
setAllSpectraSelectionEnabled(true);
if (m_uiForm.ckAllSpectra->isChecked()) {
Expand Down
9 changes: 2 additions & 7 deletions qt/widgets/common/src/FitScriptGeneratorPresenter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,6 @@ void FitScriptGeneratorPresenter::notifyPresenter(ViewEvent const &event, [[mayb
case ViewEvent::AddDomainClicked:
handleAddDomainClicked();
return;
case ViewEvent::AddDomainAccepted:
handleAddDomainAccepted();
return;
case ViewEvent::StartXChanged:
handleStartXChanged();
return;
Expand Down Expand Up @@ -148,10 +145,8 @@ void FitScriptGeneratorPresenter::handleRemoveDomainClicked() { removeDomains(m_

void FitScriptGeneratorPresenter::handleAddDomainClicked() { m_view->openAddWorkspaceDialog(); }

void FitScriptGeneratorPresenter::handleAddDomainAccepted() {
auto const workspaces = m_view->getDialogWorkspaces();
auto const workspaceIndices = m_view->getDialogWorkspaceIndices();

void FitScriptGeneratorPresenter::handleAddDomainAccepted(std::vector<MatrixWorkspace_const_sptr> const &workspaces,
FunctionModelSpectra const &workspaceIndices) {
if (!workspaces.empty() && !workspaceIndices.empty())
addWorkspaces(workspaces, workspaceIndices);
}
Expand Down
26 changes: 13 additions & 13 deletions qt/widgets/common/src/FitScriptGeneratorView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -325,20 +325,24 @@ void FitScriptGeneratorView::addWorkspaceDomain(std::string const &workspaceName
}

void FitScriptGeneratorView::openAddWorkspaceDialog() {
m_addWorkspaceDialog = new AddWorkspaceDialog(this);
m_addWorkspaceDialog->setAttribute(Qt::WA_DeleteOnClose);
m_addWorkspaceDialog->show();
connect(m_addWorkspaceDialog, SIGNAL(addData()), this, SLOT(addWorkspaceDialogAccepted()));
auto dialog = new AddWorkspaceDialog(this);
dialog->setAttribute(Qt::WA_DeleteOnClose);
dialog->show();
connect(dialog, SIGNAL(addData(MantidWidgets::IAddWorkspaceDialog *)), this,
SLOT(addWorkspaceDialogAccepted(MantidWidgets::IAddWorkspaceDialog *)));
}

void FitScriptGeneratorView::addWorkspaceDialogAccepted() {
m_presenter->notifyPresenter(ViewEvent::AddDomainAccepted);
void FitScriptGeneratorView::addWorkspaceDialogAccepted(MantidWidgets::IAddWorkspaceDialog *dialog) {
if (auto addWorkspaceDialog = dynamic_cast<MantidWidgets::AddWorkspaceDialog const *>(dialog)) {
m_presenter->handleAddDomainAccepted(getDialogWorkspaces(dialog), addWorkspaceDialog->workspaceIndices());
}
}

std::vector<MatrixWorkspace_const_sptr> FitScriptGeneratorView::getDialogWorkspaces() {
std::vector<MatrixWorkspace_const_sptr>
FitScriptGeneratorView::getDialogWorkspaces(MantidWidgets::IAddWorkspaceDialog *dialog) {
std::vector<MatrixWorkspace_const_sptr> workspaces;
if (m_addWorkspaceDialog) {
auto const wsName = m_addWorkspaceDialog->workspaceName();
if (dialog) {
auto const wsName = dialog->workspaceName();
auto &ads = AnalysisDataService::Instance();
if (ads.doesExist(wsName)) {
workspaces.emplace_back(ads.retrieveWS<MatrixWorkspace>(wsName));
Expand All @@ -349,10 +353,6 @@ std::vector<MatrixWorkspace_const_sptr> FitScriptGeneratorView::getDialogWorkspa
return workspaces;
}

FunctionModelSpectra FitScriptGeneratorView::getDialogWorkspaceIndices() const {
return m_addWorkspaceDialog->workspaceIndices();
}

void FitScriptGeneratorView::openEditLocalParameterDialog(
std::string const &parameter, std::vector<std::string> const &workspaceNames,
std::vector<std::string> const &domainNames, std::vector<double> const &values, std::vector<bool> const &fixes,
Expand Down

0 comments on commit 1dfba42

Please sign in to comment.