Skip to content
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.

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

Fix Bug with Continuous Autoscrolling when Smooth Scrolling is Enabled #7433

Merged
merged 1 commit into from
Aug 11, 2024

Conversation

regulus79
Copy link
Contributor

Fixing a bug Gabrielxd195 pointed out, where enabling smooth scroll in the song editor causes the view to move at an uneven rate.

This is fixed by setting the scroll value directly through m_leftRightScroll->setValue() instead of using animateScroll().

Also one thing I wanted to point out is that this means there will be no smooth scrolling for continuous autoscrolling whatsoever. So if the view needs to move a long distance to get to the current play position (like if you are editing the end of the song but then play it from the start), then it will do so instantly, without any smooth transition. That's probably fine for most people, but it means that the smooth scroll setting doesn't have any effect for continuous autoscrolling.

@Gabrielxd195
Copy link

@regulus79 Brother, I tried this improvement and I loved it, thank God there were no problems. I had suggested this function in #5010 and thanks to you and the other devs that we now have it available, but please (if it is not too much to ask) before merging make this enabled by default, because it is more modern and less abrupt when making small changes while the song is playing. For my part I am going to close #5010. Thank you

@Rossmaxx Rossmaxx linked an issue Aug 8, 2024 that may be closed by this pull request
Copy link
Contributor

@sakertooth sakertooth left a comment

Choose a reason for hiding this comment

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

Seems like @Gabrielxd195 already tested this and was the original submitter for this issue, so if they are fine with it, so am I (not much to say about the code change to be honest). 👍

Copy link
Member

@DomClark DomClark left a comment

Choose a reason for hiding this comment

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

I haven't tested this, nor did I review the original PR, but the change looks reasonable to me.

@sakertooth sakertooth merged commit 0e96c26 into LMMS:master Aug 11, 2024
11 checks passed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Playhead: Continuous playhead tracking.
4 participants