Skip to content

Fix for LFO Controller desync on changing play position with the mouse. #8081

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix for LFO Controller desync on changing play position with the mouse. #8081

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
24fee79
Fix for LFO Controller desync on changing play position with the mouse.
Jan125 Oct 8, 2025
8e57977
Formatting and compiling fixes
Jan125 Oct 10, 2025
b66dbda
Replace setToTime, edit setPlayPos, remove unused code
Jan125 Oct 11, 2025
1362b8a
Revert nonexplicit Song::PlayMode::None for TimeLine Playhead moving.
Jan125 Oct 11, 2025
58230e7
Merge branch 'LMMS:master' into lfocontrollermouseplayheadmovefix
Jan125 Oct 25, 2025
a7f5d19
Merge branch 'LMMS:master' into lfocontrollermouseplayheadmovefix
Jan125 Nov 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 3 additions & 24 deletions include/Song.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,28 +147,6 @@ class LMMS_EXPORT Song : public TrackContainer
return m_elapsedMilliSeconds[static_cast<std::size_t>(playMode)];
}

inline void setToTime(TimePos const & pos)
{
setToTime(pos, m_playMode);
}

inline void setToTime(TimePos const & pos, PlayMode playMode)
{
m_elapsedMilliSeconds[static_cast<std::size_t>(playMode)] = pos.getTimeInMilliseconds(getTempo());
getPlayPos(playMode).setTicks(pos.getTicks());
}

inline void setToTimeByTicks(tick_t ticks)
{
setToTimeByTicks(ticks, m_playMode);
}

inline void setToTimeByTicks(tick_t ticks, PlayMode playMode)
{
m_elapsedMilliSeconds[static_cast<std::size_t>(playMode)] = TimePos::ticksToMilliseconds(ticks, getTempo());
getPlayPos(playMode).setTicks(ticks);
}

inline int getBars() const
{
return currentBar();
Expand Down Expand Up @@ -375,6 +353,9 @@ class LMMS_EXPORT Song : public TrackContainer

Metronome& metronome() { return m_metronome; }

void setPlayPos(tick_t ticks, PlayMode playMode);


public slots:
void playSong();
void record();
Expand Down Expand Up @@ -411,7 +392,6 @@ private slots:
void updateFramesPerTick();



private:
Song();
Song( const Song & );
Expand All @@ -434,7 +414,6 @@ private slots:
getPlayPos(m_playMode).currentFrame();
}

void setPlayPos( tick_t ticks, PlayMode playMode );

void saveControllerStates( QDomDocument & doc, QDomElement & element );
void restoreControllerStates( const QDomElement & element );
Expand Down
9 changes: 4 additions & 5 deletions src/core/Song.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ void Song::processNextBuffer()
{
if (getPlayPos() < begin || getPlayPos() >= end)
{
setToTime(begin);
setPlayPos(begin.getTicks(), Engine::getSong()->playMode());
m_vstSyncController.setPlaybackJumped(true);
emit updateSampleTracks();
return true;
Expand Down Expand Up @@ -605,9 +605,8 @@ void Song::updateLength()

void Song::setPlayPos( tick_t ticks, PlayMode playMode )
{
tick_t ticksFromPlayMode = getPlayPos(playMode).getTicks();
m_elapsedTicks += ticksFromPlayMode - ticks;
m_elapsedMilliSeconds[static_cast<std::size_t>(playMode)] += TimePos::ticksToMilliseconds( ticks - ticksFromPlayMode, getTempo() );
m_elapsedTicks = ticks;
m_elapsedMilliSeconds[static_cast<std::size_t>(playMode)] = TimePos::ticksToMilliseconds( ticks , getTempo() );
Copy link
Member

@regulus79 regulus79 Oct 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this change would mean tempo automation would incorrectly alter the millisecond counter? I believe @messmerd had concerns about it correctly handling tempo changes, which was one issue with #7454.

Copy link
Contributor Author

@Jan125 Jan125 Oct 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not seeing a change in behaviour here.
It has been handled arguably incorrectly before:
-Run song from the beginning: Get total time correctly.
-Run song from after tempo change: Time is calculated on that point as if the current tempo was always the tempo.

Copy link
Contributor Author

@Jan125 Jan125 Oct 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I am now seeing is a regression in that the timer on top no longer updates when you move the playhead with the mouse when it's not playing, so I will be fixing that.

Copy link
Contributor Author

@Jan125 Jan125 Oct 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out I was looking in the wrong place regarding the previous comment.

getPlayPos(playMode).setTicks( ticks );
getPlayPos(playMode).setCurrentFrame( 0.0f );
getPlayPos(playMode).setJumped( true );
Expand Down Expand Up @@ -681,7 +680,7 @@ void Song::stop()
if (timeline.playStartPosition() >= 0)
{
getPlayPos().setTicks(timeline.playStartPosition().getTicks());
setToTime(timeline.playStartPosition());
setPlayPos(timeline.playStartPosition().getTicks(), Engine::getSong()->playMode());

timeline.setPlayStartPosition(-1);
}
Expand Down
13 changes: 5 additions & 8 deletions src/gui/editors/TimeLineWidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,14 +333,11 @@ void TimeLineWidget::mouseMoveEvent( QMouseEvent* event )
{
case Action::MovePositionMarker:
m_pos.setTicks(timeAtCursor.getTicks());
Engine::getSong()->setToTime(timeAtCursor, m_mode);
if (!( Engine::getSong()->isPlaying()))
{
//Song::PlayMode::None is used when nothing is being played.
Engine::getSong()->setToTime(timeAtCursor, Song::PlayMode::None);
}
m_pos.setCurrentFrame( 0 );
m_pos.setJumped( true );

Engine::getSong()->setPlayPos(timeAtCursor.getTicks(), m_mode);

m_pos.setCurrentFrame(0);
m_pos.setJumped(true);
updatePosition();
break;

Expand Down
Loading