Skip to content

Commit

Permalink
BUG: Fix unrelated matrix elements overwritten in transform editor
Browse files Browse the repository at this point in the history
In transforms module, when linear transformation matrix values were edited using spinboxes and then by directly editing the matrix, the value that was previously edited in the spinbox got overwritten by the new value.

The issue was that a single slot was used for all signals coming from all sliders and in the slot implementation the active focus was used to determine which axis should be updated.
Fixed by using a separate slot for each axis. This way we don't rely on the GUI widget focus, which may go through complex transitions during transform editing using various widgets.

(cherry picked from commit 325fdbd)
  • Loading branch information
lassoan authored and jcfr committed Apr 5, 2024
1 parent 8e764ef commit 0f42f5b
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 28 deletions.
52 changes: 25 additions & 27 deletions Libs/MRML/Widgets/qMRMLTransformSliders.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,11 @@ qMRMLTransformSliders::qMRMLTransformSliders(QWidget* slidersParent)
this->setTypeOfTransform(qMRMLTransformSliders::TRANSLATION);

this->connect(d->LRSlider, SIGNAL(valueChanged(double)),
SLOT(onSliderPositionChanged(double)));
SLOT(onLRSliderPositionChanged(double)));
this->connect(d->PASlider, SIGNAL(valueChanged(double)),
SLOT(onSliderPositionChanged(double)));
SLOT(onPASliderPositionChanged(double)));
this->connect(d->ISSlider, SIGNAL(valueChanged(double)),
SLOT(onSliderPositionChanged(double)));
SLOT(onISSliderPositionChanged(double)));

this->connect(d->MinValueSpinBox, SIGNAL(valueChanged(double)),
SLOT(onMinimumChanged(double)));
Expand Down Expand Up @@ -479,32 +479,9 @@ void qMRMLTransformSliders::resetUnactiveSliders()
}

// --------------------------------------------------------------------------
void qMRMLTransformSliders::onSliderPositionChanged(double position)
void qMRMLTransformSliders::onSliderPositionChanged(qMRMLLinearTransformSlider* slider, double position)
{
Q_D(qMRMLTransformSliders);
qMRMLLinearTransformSlider* slider =
qobject_cast<qMRMLLinearTransformSlider*>(this->sender());
Q_ASSERT(slider);
QWidget* focusWidget = this->focusWidget();

// If update initiated from spinbox, consider it active, too
// (when number of decimals are updated then it may change all the sliders
// one by one, but that should not reset the axis that is currently being changed)
if (focusWidget)
{
if (focusWidget->parent() == d->LRSlider->spinBox())
{
slider = d->LRSlider;
}
else if (focusWidget->parent() == d->PASlider->spinBox())
{
slider = d->PASlider;
}
else if (focusWidget->parent() == d->ISSlider->spinBox())
{
slider = d->ISSlider;
}
}
d->ActiveSliders.insert(slider);

if (this->typeOfTransform() == qMRMLTransformSliders::ROTATION
Expand All @@ -520,6 +497,27 @@ void qMRMLTransformSliders::onSliderPositionChanged(double position)
d->ActiveSliders.remove(slider);
}

// --------------------------------------------------------------------------
void qMRMLTransformSliders::onLRSliderPositionChanged(double position)
{
Q_D(qMRMLTransformSliders);
this->onSliderPositionChanged(d->LRSlider, position);
}

// --------------------------------------------------------------------------
void qMRMLTransformSliders::onPASliderPositionChanged(double position)
{
Q_D(qMRMLTransformSliders);
this->onSliderPositionChanged(d->PASlider, position);
}

// --------------------------------------------------------------------------
void qMRMLTransformSliders::onISSliderPositionChanged(double position)
{
Q_D(qMRMLTransformSliders);
this->onSliderPositionChanged(d->ISSlider, position);
}

//-----------------------------------------------------------------------------
QPair<double, double> qMRMLTransformSliders::extractMinMaxTranslationValue(
vtkMatrix4x4 * mat, double pad)
Expand Down
7 changes: 6 additions & 1 deletion Libs/MRML/Widgets/qMRMLTransformSliders.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
class vtkMRMLNode;
class vtkMRMLTransformNode;
class vtkMatrix4x4;
class qMRMLLinearTransformSlider;
class qMRMLTransformSlidersPrivate;

class QMRML_WIDGETS_EXPORT qMRMLTransformSliders : public qMRMLWidget
Expand Down Expand Up @@ -155,7 +156,9 @@ public slots:
void setDecimals(int newDecimals);

protected slots:
void onSliderPositionChanged(double position);
void onLRSliderPositionChanged(double position);
void onPASliderPositionChanged(double position);
void onISSliderPositionChanged(double position);

void onMinimumChanged(double min);
void onMaximumChanged(double max);
Expand All @@ -167,6 +170,8 @@ protected slots:
protected:
QScopedPointer<qMRMLTransformSlidersPrivate> d_ptr;

void onSliderPositionChanged(qMRMLLinearTransformSlider* slider, double position);

/// Extract the min/max values from the matrix and
/// change the slider min/max values accordingly.
/// Needed if the matrix changed externally (python, cli, etc.)
Expand Down

0 comments on commit 0f42f5b

Please sign in to comment.