Bug Report - MidiKeyboardComponent

Juce 3.1.1 - iOS 8

Using the MidiKeyboardComponent on iPad, if I:

1. Press a key with one finger, and while still holding it:

2. Play that same key with a second finger, the note re-plays and:

3. When the key is released, the note gets stuck.

I found myself accidentally doing this when playing quick trills, or fast multi-note lines. Tested it out on the JUCE demo, and the same thing happens (though it was harder to recreate with the tiny keys).

I added a line of code in MidiKeyboardComponent.cpp::updateNoteUnderMouse(), that prevents an already pressed key from re-playing:

void MidiKeyboardComponent::updateNoteUnderMouse (Point<int> pos, bool isDown, int fingerNum)

{

    float mousePositionVelocity = 0.0f;

    const int newNote = xyToNote (pos, mousePositionVelocity);

    const int oldNote = mouseOverNotes.getUnchecked (fingerNum);


    if (oldNote != newNote)

    {

        repaintNote (oldNote);

        repaintNote (newNote);

        mouseOverNotes.set (fingerNum, newNote);

    }


    int oldNoteDown = mouseDownNotes.getUnchecked (fingerNum);

    

    if (isDown)

    {

        if (newNote != oldNoteDown)

        {

            if (oldNoteDown >= 0)

            {

                mouseDownNotes.set (fingerNum, -1);

                

                if (! mouseDownNotes.contains (oldNoteDown))

                    state.noteOff (midiChannel, oldNoteDown);

            }


            if (newNote >= 0)

            {

                if (! useMousePositionForVelocity)

                    mousePositionVelocity = 1.0f;

                
                // FIX HERE TO PREVENT A CURRENTLY DOWN NOTE FROM RE-PLAYING
                if (! mouseDownNotes.contains (newNote))

                    state.noteOn (midiChannel, newNote, mousePositionVelocity * velocity);

                

                mouseDownNotes.set (fingerNum, newNote);

            }

        }

    }

    else if (oldNoteDown >= 0)

    {

        mouseDownNotes.set (fingerNum, -1);


        if (! mouseDownNotes.contains (oldNoteDown))

            state.noteOff (midiChannel, oldNoteDown);

    }

}

No more stuck note, plus now it behaves more like a real keyboard and doesn't re-play an already held key.

Good one, thanks for catching that! Will get it fixed..

Great! I just came across another MidiKeyboardComponent issue. This may not be a bug as much as it is a preference, but nonetheless:

 

1. Hold a note with one finger.

2. Press a second finger outside the MidiKeyboardComponent

3. Drag the second finger onto the keyboard, and it plays whatever key is underneath it.

 

In my synth, and in many synths, the interface above the keyboard contains knobs. So if I'm holding a key with one finger, and dragging down a knob with another finger, that dragging finger often ends up on the keyboard, particularly for knobs that are positioned right above the keyboard. I think most users intend for that dragging finger to only change the knob, not also play the key, because they want to hear how the sound is changing without a new note interrupting it. If you think that's a worthwhile change I did the following:

 

In the MidiKeyboardComponent's timerCallback method (see inside the last for loop):


void MidiKeyboardComponent::timerCallback()
{
    if (shouldCheckState)
    {
        shouldCheckState = false;
        for (int i = rangeStart; i <= rangeEnd; ++i)
        {
            if (keysCurrentlyDrawnDown[i] != state.isNoteOnForChannels (midiInChannelMask, i))
            {
                keysCurrentlyDrawnDown.setBit (i, state.isNoteOnForChannels (midiInChannelMask, i));
                repaintNote (i);
            }
        }
    }
    if (shouldCheckMousePos)
    {
        const Array<MouseInputSource>& mouseSources = Desktop::getInstance().getMouseSources();
        for (MouseInputSource* mi = mouseSources.begin(), * const e = mouseSources.end(); mi != e; ++mi)
        {
            // CHANGE HERE TO PREVENT NOTE TRIGGER BY DRAG FROM OFF-COMPONENT 
            int downX = mi->getLastMouseDownPosition().getX();
            int downY = mi->getLastMouseDownPosition().getY();
            
            if (getScreenBounds().contains(downX, downY))
                updateNoteUnderMouse (getLocalPoint (nullptr, mi->getScreenPosition()).roundToInt(), mi->isDragging(), mi->getIndex());
        }
    }
}
​

Thanks, I'll take a look at that!

I was poking around in this class. I can’t figure out what keysCurrentlyDrawnDown does, as it is only referenced in that timerCallback(). As per @Rail_Jon_Rogut’s suggestion to change the repaintNote() method to just call repaint() for the entire component (instead of just painting a single key rectangle), keeping track of which keys are currently drawn down so only those rectangles are redrawn seems like a waste of CPU.

Looks pretty straightforward to me. It’s there to save CPU, since drawing all the notes unnecessarily will be a much bigger job than testing a few bits occasionally.

My change is to make sure that the keys adjacent to the painted rectangle also get repainted… if you like you can adjust that to only paint the note and it’s adjacent notes… the original will create artifacts if you’re using image resources.

Cheers,

Rail

2 Likes

Why would that be the case? It should be possible for any rectangle to be invalidated and repainted without artefacts appearing, regardless of whether images are involved (?) Even if the class internally repainted more than it needs to, you could still expect other reasons why random regions of the component could be repainted.

Well I was trying to change the base class as little as possible… and I made the above change (back in 2013) for the build I sent to the graphic artist before I even knew the image resource sizes (or what they’d look like)… and it didn’t have any timing issues with redrawing… so rather than rewriting getRectangleForKey() I just left it as above. :slight_smile:

Cheers,

Rail