Fix missing scroll back when stop in Song Editor

[midi-pascal]

fixes #2145 via meta-issue 2291
Make the Song Editor scroll back to the beginning of song or to the time line position depending on what is selected with the 3 states stop button.

[Umcaruje]

@midi-pascal I tested this one out. It now scrolls back to the beginning to the song editor, but regardless if autoscroll is on or off. I think that the song editor shouldn't scroll back when autoscroll is off.

[midi-pascal]

@Umcaruje Oups! I will check that.

[midi-pascal]

@Umcaruje I tested this and I think my patch works correctly since the stetting that allow to scroll back - or not - is designed to act when stopping the play back.
What I see is:

• If the auto scroll is off
  • If the scroll back setting is "After stopping keep position" there is no scroll back (I think this is what you meant here)
  • If the setting is one of the other two the song editor scrolls back to where it should (beginning of the song or to the time line respectively). I think it is what it is supposed to do IMHO.
• If the auto scroll is on, the setting works as expected when stopping the playback

So I think the way it works with my patch is OK unless I misunderstood something in the way the scroll back should work.
Let me know what you think about this.

[tresf]

@Umcaruje do you agree? If so we can merge. If not, we should fix. Scrollback behavior has been less than ideal for a very long time, this would be a nice fix to see for 1.2. 👍

[midi-pascal]

@Umcaruje @tresf Please do 👍

[musikBear]

@midi-pascal 👍

[midi-pascal]

@Umcaruje Any advice on my explanation? This PR would be nice for 1.2 IMHO 😉

[Umcaruje]

Sorry, I overlooked the mail.

My opinion is that when autoscroll is off, the song editor should never scroll, in any case. What autoscroll really means is 'follow the playhead'. And as a user, when I turn it off, I expect that the song editor never scrolls back to where the playhead is, regardless of its behavior.

If the setting is one of the other two the song editor scrolls back to where it should (beginning of the song or to the time line respectively).

The thing is, those buttons alter how the playhead behaves, and not the song editor. It should be independent.

[midi-pascal]

@Umcaruje I think I found the proper fix to avoid unwanted Song Editor scrolling. As usual, your feedback is very welcome 😄

[Umcaruje]

@midi-pascal this now works as expected 👍

Though, here's a thing that's inconsistent (this is nitpicking though): With autoscroll on, when I press stop, it does not scroll back smoothly, as it does when e.g. I move the timeline, and then press play with the playhead at the beginning. You have to enable smooth scrolling from the performance menu in the settings for this.

[midi-pascal]

@Umcaruje So... I think this new fix should do the trick for Smooth Scroll. I tried all the the test cases I could think of and it seems OK to me. (no scroll, "hard" scroll, smooth scroll, and all the combinations with the "were to scroll or not").
I must say I never used the smooth scroll before and this is a cool feature 👍 ).
You are of a very precious help to me. Thanks!
Waiting for your advice, now.

[Umcaruje]

This underscored function parameter should be renamed to just t, as that is no longer a coding convention.

[Umcaruje]

Ok, tested this out, works perfectly 👍 Great work.

You just need to adress that one styling remark, and this'll be good to merge.

[midi-pascal]

@Umcaruje Being at it, I removed the leading underscores throughout the SongEditor fonctions parameters so it is done once for all. 😉

[Umcaruje]

Why remove the space here?

[Umcaruje]

@Umcaruje Being at it, I removed the leading underscores throughout the SongEditor fonctions parameters so it is done once for all. 😉

👍

[midi-pascal]

@Umcaruje Sorry, my Qt Creator settings did that while refactoring. Spaces are back now. 😃

[midi-pascal]

@Umcaruje So?

[Umcaruje]

Ok, did a last test, all looks good. Merging. 👍

[Umcaruje]

Thanks for your work on this @midi-pascal 👍