Skip to content

Commit

Permalink
Less "jumpy" knobs
Browse files Browse the repository at this point in the history
Implement a more stable knob behavior.

Remove the jumping behavior if the users directly click on a volume
knob. By storing the offset from the knob center and taking it into
account during the changes it now also feels like the users are
dragging the knob.

Changes to the amplification are now only applied when the mouse is
moved. This makes the double click behavior much more stable, i.e. if
users click on the knob when it is at 0 dB the dialog will also show
0 dB and not something like 0.3 dB because the first click is already
registered as a change of volume.

If the users click next to the knob the amplification will still be
changed immediately to that value.

## Technical details

To make the knobs more stable a variable called `m_knobCenterOffset` was
introduced. It stores the offset of the click from the knob center so
that this value can be taken into account for in the method
`setVolumeByLocalPixelValue`.
  • Loading branch information
michaelgregorius committed Dec 27, 2024
1 parent 1f9f96d commit a86ca84
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 1 deletion.
5 changes: 5 additions & 0 deletions include/Fader.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@ class LMMS_EXPORT Fader : public QWidget, public FloatModelView

QPixmap m_knob {embed::getIconPixmap("fader_knob")};

// Stores the offset to the knob center when the user drags the fader knob.
// This is needed to make it feel like the users drag the knob without it
// jumping immediately to the click position.
int m_knobCenterOffset {0};

bool m_levelsDisplayedInDBFS {true};
bool m_modelIsLinear {false};

Expand Down
32 changes: 31 additions & 1 deletion src/gui/widgets/Fader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,31 @@ void Fader::mousePressEvent(QMouseEvent* mouseEvent)
}

const int localY = mouseEvent->y();
setVolumeByLocalPixelValue(localY);
const auto knobLowerPosY = calculateKnobPosYFromModel();
const auto knobUpperPosY = knobLowerPosY - m_knob.height();

const auto clickedOnKnob = localY >= knobUpperPosY && localY <= knobLowerPosY;

if (clickedOnKnob)
{
// If the users clicked on the knob we want to compensate for the offset to the center line
// of the knob when dealing with mouse move events.
// This will make it feel like the users have grabbed the knob where they clicked.
const auto knobCenterPos = knobLowerPosY - (m_knob.height() / 2);
m_knobCenterOffset = localY - knobCenterPos;

// In this case we also will not call setVolumeByLocalPixelValue, i.e. we do not make any immediate
// changes. This should only happen if the users actually move the mouse while grabbing the knob.
// This makes the knobs less "jumpy".
}
else
{
// If the users did not click on the knob then we assume that the fader knob's center should move to
// the position of the click. We do not compensate for any offset.
m_knobCenterOffset = 0;

setVolumeByLocalPixelValue(localY);
}

updateTextFloat();
s_textFloat->show();
Expand Down Expand Up @@ -206,6 +230,9 @@ void Fader::mouseReleaseEvent(QMouseEvent* mouseEvent)
}
}

// Always reset the offset to 0 regardless of which mouse button is pressed
m_knobCenterOffset = 0;

s_textFloat->hide();
}

Expand Down Expand Up @@ -327,6 +354,9 @@ void Fader::setVolumeByLocalPixelValue(int y)
{
auto* m = model();

// Compensate the offset where users have actually clicked
y -= m_knobCenterOffset;

// The y parameter gives us where the mouse click went.
// Assume that the middle of the fader should go there.
int const lowerFaderKnob = y + (m_knob.height() / 2);
Expand Down

0 comments on commit a86ca84

Please sign in to comment.