New loop marker shortcuts, attempt 2

[Spekular]

Changes:

• Move all loop marker adjustments from RMB to Shift + click actions
• Offer users a few options for loop marker adjustment via the settings menu
  • "Move closest" emulates FL Studio (current behavior on master)
  • "Handles" emulates Ableton: clicking the handle near the loop's edge (or the screen's edge if the full loop isn't visible) moves that point. Clicking the region between handles slides the entire loop.
  • "Dual-button" is custom, LMB sets start and RMB sets end.
• Display a custom cursor when shift is held in "Move closest" or "Handles" mode, indicating which handle will be moved on click
• Refactor position calculations in TimeLineWidget.
  • Instead of changing m_xOffset based on the playhead icon's width, and then undoing this change in tons of places (sometimes with magic numbers), let it always represent the width of the unused timeline region. The few calculations that need to adjust for pixmap width do so on their own instead.
  • Remove m_posMarkerX, because it's equivalent to markerX(m_pos)
  • Remove m_moveXOff, which is unnecessary now that m_xOffset isn't being modified
• More comments

TODO:

• Better handle drawing
  • Align with actual loop (but how? Too high when offset 0, too low when offset 1, 0.5 behaves like 0...)
• Basic RMB menu
• Proper settings layout

[allejok96]

Yay! This was fast.

I suggest putting the Mode option in a radio list in a context menu of the timeline.

My guess is that "Closest mode people" will complain most about having to press shift and not so much about having to alternate between left and right mouse button. So since RMB is already killed by this design I think we can safely drop Closest mode in favor of Speedy mode.

To calm down the anti-shifters (and to educate people about speedy mode) we put in the context menu:

• Set start (shift+left)
• Set end (shift+right)

[Spekular]

Welp, I can't get a handle on this submodule situation. Will probably open yet another PR later this week instead of trying to fix this branch.

Edit: force pushed instead

[LmmsBot]

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Windows

• Windows 32-bit: lmms-1.3.0-alpha.1.190+ga5f055d74-mingw-win32.exe (build link)
• Windows 64-bit: lmms-1.3.0-alpha.1.190+ga5f055d74-mingw-win64.exe (build link)
• Windows 32-bit: lmms-1.3.0-alpha-msvc2017-win32.exe (build link)
• Windows 64-bit: lmms-1.3.0-alpha-msvc2017-win64.exe (build link)

Linux

• Linux (AppImage): lmms-1.3.0-alpha.1.190+ga5f055d74-linux-x86_64.AppImage (build link)

macOS

• macOS : lmms-1.3.0-alpha.1.190+ga5f055d74-mac10.14.dmg (build link)

🤖

{"platform_name_to_artifacts": {"Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://output.circle-artifacts.com/output/job/62588f89-15da-483e-a511-a2dd45f2bfb7/artifacts/0/lmms-1.3.0-alpha.1.190+ga5f055d74-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/17021?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://output.circle-artifacts.com/output/job/2390394d-b20e-4254-9635-fe82f988d17f/artifacts/0/lmms-1.3.0-alpha.1.190+ga5f055d74-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/17020?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/1kyvca754o0u1t70/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/43716334"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/knqbnycpbujbkymb/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/43716334"}], "Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://output.circle-artifacts.com/output/job/9bebac8b-5795-4574-890c-15ebb500ace1/artifacts/0/lmms-1.3.0-alpha.1.190+ga5f055d74-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/17023?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://output.circle-artifacts.com/output/job/f6ecc9a0-05e0-4d74-a5a9-b546e87e454a/artifacts/0/lmms-1.3.0-alpha.1.190+ga5f055d74-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/17024?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "a5f055d7420cacf3cc6893415e6eb3f230e492f7"}

[Spekular]

I suggest putting the Mode option in a radio list in a context menu of the timeline.

I feel like this is more of a "set once and get used to it" setting.

safely drop Closest mode

I'd rather keep it, to facilitate users moving to/from LMMS with one fewer workflow shock.

Use native cursors

All the non-custom cursors look native to me on ubuntu & windows, and I don't think there are any native cursors to indicate "grab left" and "grab right".

[allejok96]

set once and get used to it

Not for the sake of it being quick to change, but for the sake of being discovered. You can almost expect users to fiddle around with the timeline when trying to set the loop marker, eventually opening the context menu. In there they would see the key combos for how to set the marker and also that there is a setting to select the mode they are most comfortable with. If you place it it settings dialog most users wouldn't even look for it and if they did come across it it's not immediately clear what the setting does. You'd also have to close the window to test the setting.

All the non-custom cursors look native to me

Hm, for me they are 3x the size of normal (on a regular DPI monitor). Yeah sadly Qt doesn't include a |< and >| cursor.

[allejok96]

Please forgive my great exaggeration. It's 50% bigger. This is all fine, just needs to be aligned right.

[DomClark]

@Spekular I'd love to have this PR finished up and merged - are you still interested in working on it, or could someone else take it over?

[Spekular]

@Spekular I'd love to have this PR finished up and merged - are you still interested in working on it, or could someone else take it over?

Since I've been a bit busy and several conflicting PRs were merged while this stayed open, I unfortunately don't think I'll have time to get it back in a mergeable state myself. I'd be very thankful if someone wants to get it fixed up! (Then, maybe a follow-up PR could bring back the RMB menu from #6021? 😉 )

[DomClark]

I've merged master into this branch (one of the more finicky merges I've done), and it appears to be working still after a quick test. I hope to be able to add the finishing touches soon. (I'm tempted to add the context menu in this PR, in order to implement allejok96's suggestion. It doesn't have to be mutually exclusive with a config dialog entry, and it would be a useful thing to have in place for future enhancements.)

[DomClark]

Aside from resolving conflicts and some tidying up and formatting fixes, I have made the following changes:

• Fixed handle drawing so they align with the loop at all scaling factors
• Changed the left and right selection cursor hotspots to be vertically centred within the cursor image
• Changed the handle hitboxes to match the drawn handle width, rather than a fixed 5px width
• Fixed the shift modifier getting stuck down
• Added a label to the config dialog entry
• Made the config dialog options translatable and changed code not to rely on their English values
• Set the default edit mode to be dual-button, instead of having the loop be uneditable for unset/unknown modes
• Added a context menu with options to set loop points and change the loop edit mode

I believe that addresses all the TODOs in the PR description, allejok96's feedback, and a few other bugs. I chose "dual-button" as the default mode because it will be more familiar for users coming from 1.2, "closest" mode was generally unpopular, and both "closest" and "handle" mode can make it difficult to move loop points that are off-screen, but I am open to suggestions.

[Veratil]

LGTM

[Veratil]

Tested MSVC build, all seems to be working 👍

[sakertooth]

LGTM, tested and everything seems fine. Awesome feature. Just got a few suggestions code wise, but they are not blocking by any means.

[Spekular]

Thanks for getting this fixed up and merged! 🎉