-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pitchbend direction fix lp1770745 #1906
Conversation
Before the patch we had: I have tested this and with the parallel waveforms, the odd thing is that "faster" moves the waveform faster to the left, this means "<-" relative to the second deck waveform. |
Appveyor fails unrelated. |
that's a consequence of the opposite directions of the play position on the overview and the scrolling waveform moving underneath the static playposition. Neither the Play icon nor any seek or nudge icons are intuitive if they are related to the scrolling waveform. Since those buttons are located in sections of a deck they're closer to the overview, so I think the nudge icons are more intuitive as they are now than if we'd mirror them. Maybe some kind of right-pointing arrow overlay/icon on or near the waveform play position marker would emphasize it's not the waveform underneath but actually the (static) play position line that's moving. Would that reduce the confusion? |
Right. Since I have never noticed the issue before and we have no user complaining about this. So I think we can stick with the current situation. |
Does this introduce conflicts with #1924? |
Conflicts: src/engine/ratecontrol.cpp src/engine/ratecontrol.h
I haven't checked every conditional branch, but so far the code looks good. It is at least more reasonable than before. |
Does it mean we can merge this? |
Tested successfully. LGTM. |
Thank you :-) |
This fixes some pitchbend issues including https://bugs.launchpad.net/mixxx/+bug/1770745
See commit messages.
It also adds a test.