DamRsn / NeuralNote

Audio Plugin for Audio to MIDI transcription using deep learning.
Apache License 2.0
1.3k stars 67 forks source link

[WIP] Horizontal Zoom #83

Open trirpi opened 1 year ago

trirpi commented 1 year ago

This PR introduces horizontal scrolling using ctrl+scroll as raised in #36.

DamRsn commented 12 months ago

Hey @trirpi,

Thanks for starting this. Is this already functional or still a work in progress?

Couple comments on the coding style (sorry this should be documented in a Contributing.md file)

Also I think (not sure) that mouseWheelMove be marked override?

What would be great for this as well, is that it would work with track-pad zoom movement (I'm on Mac and I'd love to have it work in addition to the ctrl/cmd + wheel). But I don't how easy it is to do that with JUCE, I've never done it myself

trirpi commented 12 months ago

Hey @trirpi,

Thanks for starting this. Is this already functional or still a work in progress?

Couple comments on the coding style (sorry this should be documented in a Contributing.md file)

* Member variables should have an appended m at the beginning (`mZoomLevel`, `mMaxZoomLevel` ...)

* Make sure to format your code with clang-format (using the `_clang-format` configuration file provided in the repo)

* Local variable should be named in snake case (`my_local_variable`)

Also I think (not sure) that mouseWheelMove be marked override?

What would be great for this as well, is that it would work with track-pad zoom movement (I'm on Mac and I'd love to have it work in addition to the ctrl/cmd + wheel). But I don't how easy it is to do that with JUCE, I've never done it myself

It's still WIP, I've marked it as such now.

Thanks a lot for the feedback, I'll incorporate it! @override is a good idea since it is a virtual function that is overwritten.

There is a JUCE event handler for the track pad zoom movement, so that should be quite easy to add once I finish this.

trirpi commented 10 months ago

Seems to work quite well now. One enhancement would be keeping the same place in view when zooming. So, I'll still try to do that.

It might also be nice to have normal scroll, perform a horizontal scroll on the combined audio region.