Change zoom in SongEditor to a Slider Zoom

[superpaik]

• Added new styles and icons for horizontal slider (derived from vertical sliders)
• Adapt TimeLineWidget to work with new continous values of zoom, instead of fixed values.
• Change zoom from fixed values to a "continous" zoom with a Slider.
  Now Zooming can be done with:
• Slider: smooth zoom in and out, from the first visible beat.
• Ctrl + MouseWheel: zoom in and out, from the beat where the cursor is at. Increments/decrements of zoom value are done acording to "pixels per bar" and visible bars are realigned acordingly.
• Crtl + Shift + MouseWheel: "slow" zoom in and out, from the beat where the cursor is at. Useful for detailed zooming.
• Crtl + 0: reset zoom to init value

[messmerd]

This is mostly a preliminary code style review. I have not thoroughly looked over the logic yet or tested the code.

[superpaik]

@messmerd thanks for the review! Sorry for the mistakes of coding conventions. I went through the code several times, but you know...

[messmerd]

A few issues I noticed while testing:

• Right-clicking on the zoom slider provides a context menu with some unnecessary options such as automation and connecting to a controller. And when I click "Edit song-global automation", LMMS crashes.
• If I click and hold to the left or right of the slider bar, the zoom will adjust in that direction, but it's incredibly slow. I think the slider bar should immediately jump to that position instead.
• Maybe the zoom should be logarithmic, because it's hard to get the zoom just where I want it when the bar is all the way to the left (zoomed out), but the level of control seems to be too fine when the bar is all the way to the right (zoomed in).

[Veratil]

• And when I click "Edit song-global automation", LMMS crashes.

We should find where this context menu is adding it and removing it. This isn't something that should be automated.

[messmerd]

A few more issues (mostly minor):

• Ctrl + MouseWheel seems to work, but if the mouse hovers over the Zoom percentage label, it loses focus and you can't zoom
• Ctrl + MouseWheel lets you zoom in/out even when hovering over the instruments on the left
• If you use Ctrl + MouseWheel to zoom, it creates the Zoom percentage label next to the mouse. If you then continue to hold down Ctrl and interact with clips by right-clicking, left-clicking, or double-clicking, the label will still remain. If you click on another window so that the Song-Editor loses focus, the label will remain until you use Ctrl+MouseWheel to zoom again or click on the label.
• Crtl + Alt + MouseWheel doesn't work

[superpaik]

Thanks for the review!

* Right-clicking on the zoom slider provides a context menu with some unnecessary options such as automation and connecting to a controller. And when I click "Edit song-global automation", LMMS crashes.

* If I click and hold to the left or right of the slider bar, the zoom will adjust in that direction, but it's incredibly slow. I think the slider bar should immediately jump to that position instead.

* Maybe the zoom should be logarithmic, because it's hard to get the zoom just where I want it when the bar is all the way to the left (zoomed out), but the level of control seems to be too fine when the bar is all the way to the right (zoomed in).

The menu comes from the "AutomatableSlider" I know how to add new entries in the menu, but not how to delete the existing ones. So, for now, there is no menu on this slider (so no crash)
The slow movement and lack of logarithmic movement (althouth the Slider has the variable to set it logarithmic, but it doesn't work) also comes from the "Automatable Slider".
I think that all these issues should be resolved in another PR. What do you think? I put them it in my backlog ;-)

* Ctrl + MouseWheel seems to work, but if the mouse hovers over the Zoom percentage label, it loses focus and you can't zoom

* Ctrl + MouseWheel lets you zoom in/out even when hovering over the instruments on the left

* If you use Ctrl + MouseWheel to zoom, it creates the Zoom percentage label next to the mouse. If you then continue to hold down Ctrl and interact with clips by right-clicking, left-clicking, or double-clicking, the label will still remain. If you click on another window so that the Song-Editor loses focus, the label will remain until you use Ctrl+MouseWheel to zoom again or click on the label.

* Crtl + Alt + MouseWheel doesn't work

I've not been able to reproduce the first one. Maybe it's related to the platform? I'm on Windows10 and Visual Studio.
The second is resolved (it worked like this on current zoom, though)
The third is resolved by setting the setVisibilityTimeOut of the textFloat to 1 second (sorry, my bad)
The forth also my bad, it's Ctrl+Shift+Wheel (not Alt). I've updated the description. So sorry ;-(

In the gif, the first part I try to go over the textfloat and it still works. The second part, check out the scale showed in the textfloat that goes through predefined values with Ctrl+Shift+Wheel

[messmerd]

Thanks, it works better now.

I've not been able to reproduce the first one. Maybe it's related to the platform?

I'm on Linux, so maybe it's related to that. It's not a very serious issue for me though

I think that all these issues should be resolved in another PR. What do you think?

Yeah, that sounds best to me

It looks like if I use Ctrl+Drag on the slider, I can still cause the crash to occur. Maybe this could be fixed by adding a disableMouseDragEvents() method on AutomatableSlider or something.

When zooming in, it can move the horizontal scrollbar at the bottom of the song editor to the right and make some clips go out of sight. This is a little annoying imo, especially when the the scrollbar is all the way to the left and the clip on the first measure of the song goes out of sight. I'm not sure if there's any good solution to this though.

Is there a reason the smallest zoom value is 13% instead of something like 10%, 15%, etc?

[superpaik]

I think that all these issues should be resolved in another PR. What do you think?

Yeah, that sounds best to me

Perfect! I'll start another PR

It looks like if I use Ctrl+Drag on the slider, I can still cause the crash to occur. Maybe this could be fixed by adding a disableMouseDragEvents() method on AutomatableSlider or something.

Sorry. I've been able to reproduce. Is that happening on the other sliders, like master volume or master pitch? Unfortunatelly there is no disableMouseDragEvent on AutomatableSlider.

When zooming in, it can move the horizontal scrollbar at the bottom of the song editor to the right and make some clips go out of sight. This is a little annoying imo, especially when the the scrollbar is all the way to the left and the clip on the first measure of the song goes out of sight. I'm not sure if there's any good solution to this though.

Yes, that can be somehow confusing to the user. I can't see a neat solution but moving the cursor position (the zoom is based on current cursor position) which I think is a worse sideeffect. If you think in a production scenario with lots of clips, I believe it's not that confusing at all, also because you won't be doing zooming-zoomout from min to max constantly.

Is there a reason the smallest zoom value is 13% instead of something like 10%, 15%, etc?

That came from previous minimun zooming, which was 12,5%. If set to 10%, pixelsPerBar become 0. I set it to 15%, that works ok (1 pixelPerBar) and it's clearer. Thanks.

[messmerd]

Sorry. I've been able to reproduce. Is that happening on the other sliders, like master volume or master pitch?

Not the crash. You can't reproduce the Ctrl+Click-and-drag of the zoom slider on your end? Dragging it to an automation track causes the crash, but we shouldn't be able to Ctrl+Drag the zoom slider in the first place since it isn't automatable.

Unfortunatelly there is no disableMouseDragEvent on AutomatableSlider.

I meant that maybe we should add a method like that to AutomatableSlider. But since we're trying to remove all capabilities for automation from AutomatableSlider, it would be a better idea to just use a plain QSlider if possible, or if not, create a new wrapper class called Slider similar to AutomatableSlider but without automation.

I can't see a neat solution but moving the cursor position (the zoom is based on current cursor position) which I think is a worse sideeffect

To be clear, I'm not totally against the current behavior. I just think it may be worth fiddling with it to see if there is any alternative behavior we like better. What would happen if it zoomed as if the mouse was always located all the way to the left? Or if it zoomed as if the mouse was located wherever the playhead is?

[superpaik]

@allejok96 Have you seen the two comments I made on your PR?

[allejok96]

That was needed for clips that do not start or end at beat times (Alt+mouse, to move start or end of the clip) and an "extreme" zoom in (clip disappears). It happens with your code as well. I think we should reintroduce that control again.

I see, go ahead. But write that in a comment so it's more clear what the problem is.

[allejok96]

I'll take a break from this PR now in case you wonder, but you've done a good job @superpaik !

[superpaik]

I see, go ahead. But write that in a comment so it's more clear what the problem is.

Ok. I pushed new commit to do that.

I'll take a break from this PR now in case you wonder, but you've done a good job @superpaik !

Thank you so much for your collaboration. You made the PR much more clear and simpler code-wise

[superpaik]

This PR is ready for final review.

[messmerd]

A few minor suggestions

[messmerd]

I did a little bit of testing just now since you've made a number of changes in the month since I last tested it.
It's looking pretty good and I'm almost ready to approve it.

A few notes:

• Is Ctrl+Shift+Scroll supposed to zoom slower now instead of faster? If so, maybe you could update the PR description.
• The Ctrl+Drag-to-automation-track crash still exists, but it also exists in master, so one of us could probably fix it in a later PR. We'd just need to create an issue for it if there isn't one already.
• When I use Ctrl+Alt+Scroll, it always zooms out regardless of scroll direction. I'm not sure why it does that. The PR description doesn't say anything about this key combination, and it's not a big deal, but I figured I'd mention it.

[superpaik]

• Is Ctrl+Shift+Scroll supposed to zoom slower now instead of faster? If so, maybe you could update the PR description.

Yes. I changed it according to @allejok96 comments to match behavior with other actions. PR description updated.

• The Ctrl+Drag-to-automation-track crash still exists, but it also exists in master, so one of us could probably fix it in a later PR. We'd just need to create an issue for it if there isn't one already.

I'll create an issue.

• When I use Ctrl+Alt+Scroll, it always zooms out regardless of scroll direction. I'm not sure why it does that. The PR description doesn't say anything about this key combination, and it's not a big deal, but I figured I'd mention it

Apparently it is a bug (or hidden behavior) of QT. When Alt is pressed, it mimics horizontal wheel movement. https://bugreports.qt.io/browse/QTBUG-91556
I introduced a work-around with this suggestion https://bugreports.qt.io/browse/QTCREATORBUG-25612

[messmerd]

The Alt button workaround works for me.

[messmerd]

I think this PR is ready to be completed now. Nice work!

[superpaik]

Is there anything missing on my part in this PR? I don't think I have anything left to do.

[RiedleroD]

bit late, ik, but would it be possible to set the zoom level directly via popup on double-click on the slider, like almost all other controls in LMMS? I don't know if I'm going to remember Ctrl+0 for reset tbh.

[superpaik]

bit late, ik, but would it be possible to set the zoom level directly via popup on double-click on the slider, like almost all other controls in LMMS? I don't know if I'm going to remember Ctrl+0 for reset tbh.

I'm planning on adding some zoom actions (for zoom-to-selection, zoom-to-loop, zoom-to-fit, etc.) I'll have a look at your suggestion, it makes sense.