MidiKeyboardComponent bug

General JUCE discussion
1

Hi. I think this function may have a bug. Line 40 in MidiKeyboardComponent.cpp.

void clicked() override
{
auto note = owner.getLowestVisibleKey();

    if (delta < 0)
        note = (note - 1) / 12;
    else
        note = note / 12 + 1;

    owner.setLowestVisibleKey (note * 12);
}

I think it is supposed to be like this

double note = owner.getLowestVisibleKey();

    if (delta < 0)
        note = note  / 12 - 1;
    else
        note = note / 12 + 1;

    owner.setLowestVisibleKey (note * 12);

I think the auto was creating an int and then rounding with the division. This was causing the octave step button to always revert back to starting at C. The calculation in the step down was wrong too, unless I’m mistaken.

Cheers.

2

This looks intentional to me. Both buttons will snap the lowest note to a multiple of 12. If the buttons were designed to just add/subtract an octave from the current position, I think it’d be written as a simple note + 12 or note - 12, instead of doing the division/multiplication we see here.

3

I did think that it could have been done more easily (±12) but to me the function made sense as a way of setting the scale start point and the up/down buttons maintained that layout on the screen.

If it’s intentional then fair enough, though I can’t see why you would always want it to revert back to multiples of 12 if you have bothered to use the setlowestvisiblekey function.

4

I think it’s simply to model how hardware MIDI keyboards work. When a keyboard does not cover the entire MIDI range, the octave up/down button will translate the center key (C) by one octave.
Edit: actually I’m doubting myself now. If a physical keyboard has a lowest note that is not C, this is the one that will be translated by one octave.

5

Yeah I understand it in that context. But we have a function which allows us to select the lowest visible note which you obviously can’t do on hardware, so it just seems it renders it unusable. It’s no bother for me but thought I’d point it out incase it was an error.