Skip to content
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

controllers: fix waveform_zoom ranges #12393

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Dec 4, 2023

picked from #12387

updates some old mappings to use the current range 1..10

@JoergAtGithub
Copy link
Member

So far, the code looks correct, but I don't think it is sustainable, because the zoom range is likely to change again.

I think what is missing to do it right, is a JavaScript Engine API function getValueForParameter which does the opposite of:

/**
* Normalizes a specified value using the range of the given control,
* to the range of 0..1
*
* @param group Group of the control e.g. "[Channel1]"
* @param name Name of the control e.g. "play_indicator"
* @param value Value with the controls range according Mixxx Controls manual page:
* https://manual.mixxx.org/latest/chapters/appendix/mixxx_controls.html
* @returns Value normalized to range of 0..1
*/
function getParameterForValue(group: string, name: string, value: number): number;

Than we could determine the minimum value for each control by getValueForParameter(<group>, <control>, 0.0) and the maximum by getValueForParameter(<group>, <control>, 1.0).

@ronso0
Copy link
Member Author

ronso0 commented Dec 4, 2023

Thanks, I know that would indeed be much better.
Let's merge this as quick fix and file a feature request for getValueForParameter?

@JoergAtGithub
Copy link
Member

Ok, let's merge it as stop-gap fix.

@JoergAtGithub JoergAtGithub merged commit 9eda4c3 into mixxxdj:2.4 Dec 4, 2023
11 of 12 checks passed
@ronso0 ronso0 deleted the waveform-zoom-mappings branch December 4, 2023 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants