MidiKeyboardComponent black key width and other things


#1

Hi, I have stumbled upon several problems with MidiKeyboardComponent. I have read all of the related topics I could find on this forum and I'm a bit surprised that none of those mentions the points bellow (so it might as well be that I am missing something).

 

1) getKeyStartPosition is supposed to return the position of the key specified with index. When using vertical keyboard facing right it doesn't really return y position of the key but rather (height - position). In other words for the "lowest visible" key (which is bottom-most) it returns 0. I suppose there is a good reason for this behaviour, but it should at least be mentioned in the doc.

 

2) There is a function getLowestVisibleKey() used in 1). The problem is that there is no getHighestVisibleKey() or getVisibleRange() so I don't see any simple way for iterating over visible keys. The only way that I can think of right now is to divide the keyboard height and key height and than guess the number of black keys in between.

 

3) I think getBlackKeyWidth would be really useful when writing custom pianoroll in order to synchronize with the keyboard. I could inspire myself with protected getKeyPosition but it might be good idea to just place it there. Or there might be a public version of getKeyPos, where w reference is a key width I believe...

 

4) For the 3) to work one would actually need something like bool isKeyBlack(int midiNoteNumber) which would also be useful for more use cases


#2

Are those completely bad requests or do I really miss something important?


#3

Sorry - just haven't the time to think about it right now.


#4

Meaning you eventually will, or that I am on my own with this? Just need to know whether there is any hope in waiting for some time, or if I should start to do it somehow right away :-)


#5

Maybe suggest some code-changes..? I can't spare the time to think about your requests and do it myself, but if you can show me what you're trying to do in terms of code, that'd make me more likely to spend a few mins on it.


#6

I won't be home till wednesday, but I'll let you know as soon as I come up with something. Thanks.


#7

OK, So here is what I have come up with:

 

Instead of:

 

void MidiKeyboardComponent::getKeyPosition (int midiNoteNumber, const float keyWidth_, int& x, int& w) const
{
    jassert (midiNoteNumber >= 0 && midiNoteNumber < 128);

    static const float blackNoteWidth = 0.7f;

    static const float notePos[] = { 0.0f, 1 - blackNoteWidth * 0.6f,
                                     1.0f, 2 - blackNoteWidth * 0.4f,
                                     2.0f,
                                     3.0f, 4 - blackNoteWidth * 0.7f,
                                     4.0f, 5 - blackNoteWidth * 0.5f,
                                     5.0f, 6 - blackNoteWidth * 0.3f,
                                     6.0f };

    static const float widths[] = { 1.0f, blackNoteWidth,
                                    1.0f, blackNoteWidth,
                                    1.0f,
                                    1.0f, blackNoteWidth,
                                    1.0f, blackNoteWidth,
                                    1.0f, blackNoteWidth,
                                    1.0f };

    const int octave = midiNoteNumber / 12;
    const int note   = midiNoteNumber % 12;

    x = roundToInt (octave * 7.0f * keyWidth_ + notePos [note] * keyWidth_);
    w = roundToInt (widths [note] * keyWidth_);
}

 

There could be:

 

const float MidiKeyboardComponent::blackNoteWidth = 0.7;

void MidiKeyboardComponent::getKeyPosition(int midiNoteNumber, const float keyWidth_, int& x, int& w) const
{
    jassert(midiNoteNumber >= 0 && midiNoteNumber < 128);
    
    static const float notePos[] = { 0.0f, 1 - blackNoteWidth * 0.6f,
        1.0f, 2 - blackNoteWidth * 0.4f,
        2.0f,
        3.0f, 4 - blackNoteWidth * 0.7f,
        4.0f, 5 - blackNoteWidth * 0.5f,
        5.0f, 6 - blackNoteWidth * 0.3f,
        6.0f };
    
    const int octave = midiNoteNumber / 12;
    const int note = midiNoteNumber % 12;

    x = roundToInt(octave * 7.0f * keyWidth_ + notePos[note] * keyWidth_);
    w = roundToInt(getRelativeKeyWidth(note) * keyWidth_);
}

float MidiKeyboardComponent::getRelativeKeyWidth(int index){    
    return (isKeyBlack(index) ? blackNoteWidth : 1.0f);
}

bool MidiKeyboardComponent::isKeyBlack(int midiNoteNumber){
    static const bool black[] =   { 0, 1,
                                    0, 1,
                                    0,
                                    0, 1,
                                    0, 1,
                                    0, 1,
                                    0 };
    return black[midiNoteNumber % 12];
}

 

 

And then in header file:

 

public:

    float getBlackKeyWidth() const noexcept                            { return keyWidth*blackNoteWidth; }

    static bool isKeyBlack(int midiNoteNumber);

protected:

    static float getRelativeKeyWidth(int index);

private:

    static const float blackNoteWidth;

 

I have tested it and it seems to work. Those are just minor changes.

 

And also getKeyPos could be public because it gives you a key width, not just the position as getKeyStartPosition which is public. It would really be useful for making piano roll. Suppose that i want to align the grid with the keyboard (meaning the grid is not regular in y axis)...

 

Looking forward to hearing from You...


#8

Thanks - seems like a good request.

..although actually, having just had a deeper look, I can't really see the point of most of this. I've added a helper method to MidiMessage to tell you whether a note is a black key or not, and refactored it slightly to use that, but beyond that I can't really see what you're trying to do.


#9

Can't you do all this anyway just by subclassing MidiKeyboardComponent? All the drawing and getKeyPosition methods are protected virtual so you can call them from a subclass and even override them if you need to do your own thing.


#10

..although actually, having just had a deeper look, I can't really see the point of most of this. I've added a helper method to MidiMessage to tell you whether a note is a black key or not, and refactored it slightly to use that, but beyond that I can't really see what you're trying to do.

Well basically I have the same problem as the guy here: http://www.juce.com/forum/topic/small-request-midikeyboardcomponentgetkeypos-public

 

Finally you told him to use getKeyStartPosition which is public and told him he does not need the width of the key. He then agreed you were right but I don't quite get why you do not need it. As I said my grid is aligned with the keyboard and is not regular. And I just need to know the width of each key. At least I need to know whether the key is black so that I could multiply it with 0.7. But even then I don't think that multiplying with 0.7 outside of the class code is a nice design. There should therefore be a method for returning the width of the black key or even better to get the width of any key specified by it's index. I may miss something important because I really don't understand how is it possible that almost noone has the same problem. Either that or everybody just uses regular grid.

 

Can't you do all this anyway just by subclassing MidiKeyboardComponent? All the drawing and getKeyPosition methods are protected virtual so you can call them from a subclass and even override them if you need to do your own thing.

I could and I did, but even then I just have the feeling that I miss something, and if I don't and am quite right, I wanted to make it more simple for the others that will try to achieve the same effect with JUCE...

 

P.S. something like getHighestVisibleKey would be nice as well but since there's this clipping algorithm it is not really necessary, it may be good for some optimization purposes though.


#11

Could you please spare another minute answering the post above? I just downloaded JUCE again to reflect your changes but now I need to make the changes I made to MidiKeyboardComponent again. There's a problem with the getKeyPos. It's private so I cannot access it in any way so I cannot even derive my own subclass in order to avoid having to alter the JUCE code over and over again whenever I update it. I could duplicate the getKeyPos code (even though I do not like this solution at all) in my own subclass's method but then I run into another problem cause it uses more private variables. So it leads to altering the base class again.


#12

AFAICT there's no sane reason why you'd need to use getKeyPos(). If you're hacking the base class then you've misunderstood something, but I really don't know what.


#13

Well then,

in that case please tell me, how am I to get the width of a key specified by it's index. The point is that white keys and black keys are not of a same width and getKeyWidth() returns the width of a white key. Looking into the source code I can see that  I can calculate the width of the black key as getKeyWidth()*0.7 but hey, obviously that is not the right way to do it... That is what I suggested in the first place.

 

Thanks for your time, it is very appreciated


#14

What on earth is the problem!??

Just call getKeyPosition() and it'll return you the x and width of the note!


#15

Time for the answer from the cited thread:

My turn to mix up. Obivously you're right. Thank you.

Anyway the problem was that I misunderstood the xOffset variable used in getKeyPos. It just somehow seemed to me that it indicated the pixel shift of the first visible key and because it is private variable I saw now way to get around it.

 

So thanks again for your time.


#16

You may want to kill me, but again when updating to the new JUCE revision I failed to adapt the code. Actually I did not misunderstood the xOffset variable. The difference between getKeyPos and getKeyPosition (the naming convenction is not very fortunate here) is that getKeyPosition returns the global position whereas getKeyPos() returns screen coordinates. If I am to covert it, I need xOffset variable which is unfortunatelly private so as getKeyPos() is and I end up with altering the base code again...


#17

"screen coordinates"? "global coordinates"? Nothing in this class will use global or screen coordinates, everything is done in the component's coordinate space.

If what you mean is that you want to find the distance of a note from the leftmost note 0, then all you need to do is get the position of note 0 and subtract it, you don't need to hack the juce code to do that.