[regulus79]

Description

This pull request adds the option to have auto-scrolling continuously scroll the view to keep up with the playback.

Changes

• New icons for Stepped/Continuous auto-scrolling
• AutoScrollState enum changed from Enabled/Disabled to Stepped/Continuous/Disabled
• The auto-scroll button in TimeLineWidget.cpp is changed from having 2 states (Enabled/Disabled) to having 3 states (Stepped/Continuous/Disabled)
• In the code for auto-scrolling in both SongEditor.cpp and PianoRoll.cpp, a check is added for whether the mode is Stepped or Continuous; if it's Stepped, then is runs the old code for auto-scrolling; if it's Continuous, then it snaps the horizontal scroll to halfway behind the time line so that the line appears in the center of the view.
• SongEditor.cpp's horizontal scrollbar's functionality was changed to make it scroll by ticks instead of bars. This required modifying TrackContentWidget.cpp to be positioned by ticks instead of bars to properly draw the grid background.
• Additionally, due to the change in scrollbar step size, the scrolling sensitivity for the Song Editor had to be changed to a reasonable rate. In this PR, the scrolling rate behaves as normal and does not depend on the zooming. I had mentioned the possibility of adding that in the discord, but I decided to save it for another PR.

[Rossmaxx]

Minor style review, also pay attention to codefactor.

Btw you forgot classic theme.

[regulus79]

Okay, I made some icons for classic, and hopefully I've fixed all the style issues.

[Rossmaxx]

Looks good.

[sakertooth]

We should do testing before merge.

[michaelgregorius]

I have found a bug that's introduced by this PR, i.e. that is not in master.

Steps to reproduce

1. Load a song.
2. Set the mode to "Continuous".
3. Play the song.
4. Quickly change to "Off" and then to "Paged".

The line of the play head line will not move with the play head anymore.

[regulus79]

I think I found the problem. The return statement in the simplification of the stepped auto scrolling code exited the function too soon, which happened to skip the code for updating the play head line position. I fixed it by modifying the if statement to not require a return.

[sakertooth]

I think I found the problem. The return statement in the simplification of the stepped auto scrolling code exited the function too soon, which happened to skip the code for updating the play head line position. I fixed it by modifying the if statement to not require a return.

Had a feeling this would break something, my bad. I got tunnel vision on that one piece of code and forgot about the surrounding function.

[sakertooth]

I'll test then approve. 👍

[sakertooth]

I think it might be helpful to have more descriptions for the tooltip when toggling between the modes (i.e., you can tell the user what this mode will do in particular rather than simply saying "Auto scrolling", because not all modes auto scroll).

[regulus79]

I added specific tooltips for each button state: "Stepped auto scrolling", "Continuous auto scrolling", and "Auto scrolling disabled"

[sakertooth]

LGTM, also something minor I noticed

[regulus79]

I realized that changing the scrolling step size also broke zooming centered around the mouse position. I have now fixed that; just a simple multiplication by TimePos::ticksPerBar().

[regulus79]

Just remembered that there was another bug when drawing the rubberband selection rect, where the left side would get teleported to the left as everything scrolled. Once again, a simple division by TimePos::ticksPerBar() fixed it.

[regulus79]

Alright, I think those are all the necessary changes, so unless anyone spots a new issue, this should pr should be ready to merge.

[Gabrielxd195]

I have a question about this, and it is that the continuous scrolling is more fluid and consistent in the Piano Roll than in the Song Editor, since in the latter the scrolling stops suddenly and is not constant. I show you this in the following videos:

Autoscrolling in Piano Roll

Autoscroolling.Piano.Roll.mp4

Autoscrolling in Song Editor

Autoscroolling.Editor.de.Cancion.mp4

Is this on purpose? Or is there some other way to improve this?

[regulus79]

Ah, whoops, I forgot to test it with smooth scroll enabled. I should have set the scroll using m_leftRightScroll->setValue() instead of animateScroll().

For now, you can temporarily fix it by disabling smooth scroll in the settings.