#1496 Notation: Selected single note not always blue

[D. Michael McIntyre]

I recorded a note from each of my drums, then I went into the notation editor to sweep select the notes one by one. (The goal was to hear the individual note without any possibility of moving it accidentally. I was making a key map.)

Some of the notes turn blue when you select them, and some of them don't. Some of the notes can be coaxed into turning blue, and some of them never do unless you grab them with some broader selection method.

I have a vague memory that one of David Faure's patches was a tweak pertaining to notation selection. If that memory is accurate, it's pretty likely this is new behavior that resulted from a well-intentioned fix that had unexpected side effects, and this is the side effect. I'd almost bet money that's going to turn out to be the case. I probably have the patch on my hard drive still.

I'm going to look into this in the next week or so, but probably not today.

[D. Michael McIntyre]

I tried this with 12.12 and had exactly the same problem. So much for tracing it back to a discrete chunk of recent work.

Everything in NotePixmapFactory seems pretty cut and dry. If m_selected then it uses blue, so m_selected must not be getting set somehow.

Gah.

Well, I piddled around for an hour and didn't make any progress untangling the skein. This code is extremely complicated.

This will end up being a piddle project for a good while.

[Philip Leishman]

This is indeed complicated.
The problem seems to be caused by notes having a notation time outside of the range of the renderElements call in NotationScene.
I have attempted a fix for this. See merge request.

[Ted Felix]

Using the provided example file, the issue is seen in bar 22 and bar 26 if you try to select one of the very low notes using click and drag with the selection tool. If you start high and to the left, it works. If you start low and to the left, it does not. The issue appears to be related to the height of the upper point. In fact, if you start below and to the right, you can see that at some point above the notes, the selection will suddenly work.

[Ted Felix]

I can't build the lman/bug-1496-selection branch. I get the following:

src/gui/editors/notation/NotationScene.cpp:1916:14: error: expected ‘;’ before string constant
 1916 |     RG_DEBUG "From and to times:" << oldFrom << oldTo << newFrom << newTo;
      |              ^~~~~~~~~~~~~~~~~~~~

Looks like a missing "<<".

[Philip Leishman]

OK fixed it !

[Ted Felix]

I also noticed that the note indicator in the status bar on the right stops at C1. It will not go lower. Are we beyond the amount of vertical space allocated for a staff?

The changes do seem a bit better, but it is subtle in my testing. I can still drag a rectangle over the lowest notes and end up with nothing selected. It's not until I drag at least a seventh above the lowest note that it becomes selected. I need to go back to 21.12 and get a better feel for the original behavior.

I have a feeling I'm testing the wrong thing. The original description doesn't call out exactly which notes do not turn blue. Is there a pattern? I'm just having problems with the lowest.

Other than my case described above, are there any other cases where the notes don't get selected?

[Philip Leishman]

With 21.12 load "try-to-select-notes-in-notation.rg"
Click exactly on the first note - it turns blue
Click on the second note - it turns blue
Click on the third, fourth and fifth notes - they do not turn blue. They are selected - crl-X will cut them.
The changes in the bug-1496-selection branch only change the range of rendered notes (in the x direction). All selected notes should now be correctly rendered as blue.

I think you mean the note in bar 22 - This is a C#0. As you point out the lowest note that can be input with the mouse is a C1. I think you must drag the rectangle up to the C1 level to select the C#0 !!
I have made no changes affecting the y coordinates of the drag area. The C#0 is actually outside the range of the staff (although it is rendered correctly) - I think this is a separate issue.

[Ted Felix]

Ah! Got it. Thanks. I had a feeling I was doing this wrong. I'll have another look.

[Ted Felix]

This fix appears to work and certainly looks safe. The getNotation*Time() calls are a bit costly, but since we're talking about user interaction, not a big deal. Couple of comments...

There are two debugging for loops that should be removed. NotationScene::setSelection() line 1856-1863. You could wrap them in an #ifndef for RG_NO_DEBUG_PRINT:

#ifndef RG_NO_DEBUG_PRINT
    if (s) {
        const EventContainer ec = s->getSegmentEvents();
        for (EventContainer::iterator i = ec.begin();
             i != ec.end(); ++i) {
            Event *e = *i;
            NOTATION_DEBUG << "Selection contains" << *e;
        }
    }
#endif

The other is at the top of NotationStaff::renderElements(). Same technique will work there as well.

In regards to the getNotation*Time() calls, they can be potentially cut in half and simplified like so:

// Original
oldFrom = oldSelection->getStartTime();
if (oldSelection->getNotationStartTime() < oldFrom) {
    oldFrom = oldSelection->getNotationStartTime();
}

// Simplifies to...
oldFrom = std::min(oldSelection->getStartTime(),
                   oldSelection->getNotationStartTime());

[Ted Felix]

...and of course use std::max() for oldTo and newTo.

[Philip Leishman]

OK - I made your suggested optimization changes.
I just deleted the debug outputs - they were only temporary for debugging this issue.

[Ted Felix]

Pushed as [44451d]. Please test latest git.