Skip to content

Commit

Permalink
Fix #1193 for real this time
Browse files Browse the repository at this point in the history
Only create the widget when needed, only connect the signal as-needed
Prevents a query for a layer before there's even a molecule

Signed-off-by: Geoff Hutchison <[email protected]>
  • Loading branch information
ghutchis committed Oct 25, 2023
1 parent 301aa7d commit ab93b94
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 15 deletions.
26 changes: 15 additions & 11 deletions avogadro/qtplugins/selectiontool/selectiontool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@
#include <avogadro/qtgui/rwlayermanager.h>
#include <avogadro/qtgui/rwmolecule.h>

#include <QAction>
#include <QtCore/QDebug>
#include <QtGui/QIcon>
#include <QtGui/QMouseEvent>
#include <QAction>

#include <queue>
#include <set>
Expand All @@ -43,8 +43,7 @@ namespace Avogadro::QtPlugins {

SelectionTool::SelectionTool(QObject* parent_)
: QtGui::ToolPlugin(parent_), m_activateAction(new QAction(this)),
m_molecule(nullptr), m_renderer(nullptr),
m_toolWidget(new SelectionToolWidget(qobject_cast<QWidget*>(parent_))),
m_molecule(nullptr), m_renderer(nullptr), m_toolWidget(nullptr),
m_drawSelectionBox(false), m_doubleClick(false), m_initSelectionBox(false),
m_layerManager("Selection Tool")
{
Expand All @@ -57,16 +56,19 @@ SelectionTool::SelectionTool(QObject* parent_)
"Right Mouse: \tClick outside the molecule to clear selection\n"
"Use Ctrl to toggle the selection and shift to add to the selection.\n"
"Double-Click: \tSelect an entire fragment."));

connect(m_toolWidget, SIGNAL(colorApplied(Vector3ub)), this,
SLOT(applyColor(Vector3ub)));
connect(m_toolWidget, SIGNAL(changeLayer(int)), this, SLOT(applyLayer(int)));
}

SelectionTool::~SelectionTool() {}

QWidget* SelectionTool::toolWidget() const
{
if (m_toolWidget == nullptr) {
m_toolWidget = new SelectionToolWidget(qobject_cast<QWidget*>(parent()));
connect(m_toolWidget, SIGNAL(colorApplied(Vector3ub)), this,
SLOT(applyColor(Vector3ub)));
connect(m_toolWidget, SIGNAL(changeLayer(int)), this,
SLOT(applyLayer(int)));
}
return m_toolWidget;
}

Expand Down Expand Up @@ -132,7 +134,7 @@ QUndoCommand* SelectionTool::mouseReleaseEvent(QMouseEvent* e)
}
}
}
if (anySelect) {
if (anySelect && m_toolWidget != nullptr) {
m_toolWidget->setDropDown(m_layerManager.getLayerID(selectedIndex),
m_layerManager.layerCount());
}
Expand Down Expand Up @@ -262,7 +264,8 @@ void SelectionTool::applyLayer(int layer)
layer = layerInfo.maxLayer();

// update the menu too
m_toolWidget->setDropDown(layer, m_layerManager.layerCount());
if (m_toolWidget != nullptr)
m_toolWidget->setDropDown(layer, m_layerManager.layerCount());
changes |= Molecule::Layers | Molecule::Added;
}

Expand Down Expand Up @@ -360,7 +363,8 @@ void SelectionTool::setMolecule(QtGui::Molecule* mol)
maxLayers = m_layerManager.layerCount();
}

m_toolWidget->setDropDown(currentLayer, maxLayers);
if (m_toolWidget != nullptr)
m_toolWidget->setDropDown(currentLayer, maxLayers);
}

} // namespace Avogadro
} // namespace Avogadro::QtPlugins
2 changes: 1 addition & 1 deletion avogadro/qtplugins/selectiontool/selectiontool.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ private slots:
QAction* m_activateAction;
QtGui::Molecule* m_molecule;
Rendering::GLRenderer* m_renderer;
SelectionToolWidget* m_toolWidget;
mutable SelectionToolWidget* m_toolWidget;
bool m_drawSelectionBox;
bool m_doubleClick;
bool m_initSelectionBox;
Expand Down
10 changes: 7 additions & 3 deletions avogadro/qtplugins/selectiontool/selectiontoolwidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ SelectionToolWidget::SelectionToolWidget(QWidget* parent)
setDropDown(0, 1);
connect(m_ui->applyColorButton, SIGNAL(clicked()), this,
SLOT(userClickedColor()));
connect(m_ui->changeLayerDropDown, SIGNAL(currentIndexChanged(int)), this,
SIGNAL(changeLayer(int)));
}

SelectionToolWidget::~SelectionToolWidget()
Expand All @@ -28,13 +26,19 @@ SelectionToolWidget::~SelectionToolWidget()

void SelectionToolWidget::setDropDown(size_t current, size_t max)
{
// disconnect the signal so we don't send it accidentally
disconnect(m_ui->changeLayerDropDown, nullptr, nullptr, nullptr);
m_ui->changeLayerDropDown->clear();
for (size_t i = 0; i < max; ++i) {
m_ui->changeLayerDropDown->addItem(QString::number(i + 1));
}
m_ui->changeLayerDropDown->addItem(tr("New Layer"));
if (current != m_ui->changeLayerDropDown->currentIndex())
m_ui->changeLayerDropDown->setCurrentIndex(current);

// reconnect the signal
connect(m_ui->changeLayerDropDown, SIGNAL(currentIndexChanged(int)), this,
SIGNAL(changeLayer(int)));
}

void SelectionToolWidget::userClickedColor()
Expand All @@ -59,4 +63,4 @@ void SelectionToolWidget::userClickedColor()
}
}

} // namespace Avogadro
} // namespace Avogadro::QtPlugins

0 comments on commit ab93b94

Please sign in to comment.