Selection of overlapping elements #2622
Conversation
MarcSabatella
changed the title
| @@ -5958,14 +5965,19 @@ static bool elementLower(const Element* e1, const Element* e2) | |||
| } | |||
| } | |||
| } | |||
| return e1->z() < e2->z(); | |||
| // force z of stem to be seen as higher than note | |||
| else if (e1->type() == Element::Type::STEM) | |||
There was a problem hiding this comment.
I had to change Element::Type::STEM into Ms::ElementType::STEM in order to compile
There was a problem hiding this comment.
Or simply el->isStem(), new master style?
There was a problem hiding this comment.
thanks for pointing that out to me.
| // force z of stem to be seen as higher than note | ||
| else if (e1->type() == Element::Type::STEM) | ||
| z1 = n.z() + 1; | ||
| else if (e2->type() == Element::Type::STEM) |
There was a problem hiding this comment.
I had to change Element::Type::STEM into Ms::ElementType::STEM in order to compile
There was a problem hiding this comment.
Strange that Travis even compiled this, the change from Element::Type to ElementType happend quite a while ago?
There was a problem hiding this comment.
Sorry I wasn't clear. I just tested out his PR today, but rebased it ontop of latest master.
There was a problem hiding this comment.
Oh, I see, I missed that this PR is from May 2016!
In that case I'm puzzlead why there's not merge copnflict shown.
There was a problem hiding this comment.
There was no merge conflict with the code. There was just a compile error when I tried compiling on my local machine after rebasing. So if Marc were to rebase and force push update, then I suppose Travis would report failure to compile then.
|
I tried it, and I like it except I don't feel it is necessary to take over "Control" click for this. I actually think that don't even need to have any keystroke modifier. (Control->Click already has an interpretation of adding an element to the selection, and I am opposed to having control mean something different here.) Rather I think could just have regular click be enhanced such that if already have selected an element at the click point, then pressing click again will cycle to select the next element in the rotation. So don't even worry about Alt. Other than that, I support this PR. |
|
I don't understand. In my code there is no change to the behavior of Ctrl click if no overlapping elements are involved - that's the whole reason I went that way. It continues to add an element if it is not currently selected, remove it if it is. The only change is that in the case where the selected element overlaps another, ctrl+click both deselects if and selects the element underneath. It would be a much bigger change to change the behavior of regular click. Currently you can click an element multiple times with no effect after the first click. If we made this change, then either the second click would deselect, or else it would only to do so if it overlapped another. This to me would be more troublesome. |
|
yes I know that it doesn't change ctrl->click if no overlapping elements are involved, sorry I didn't communicate that properly. I really should have just left it as saying that just override regular click is better, so I shouldn't have even said anything about ctrl. The other thing I forgot to mention is that the feature would be hard to discover if it was Ctrl->click... That is because first instinct when user unsuccessfully tries to select an element is to try clicking again, so would more easily discover the feature. |
|
reading back again I realize I should have never brought up that point in parenthesis. |
|
My concerns remains, though, that changing the behavior of a fundamental action like clicking is not to be undertaken lightly. It would be a significant from current behavior if you had to worry that accidentally clicking something already selected would suddenly start having the unintended side effect of deselecting it - I think we'd get a lot of questions / complaints about that. That is why I think tying this to Ctrl+click, which already has this effect, is the way to go. However, I do agree it isn't necessarily easily discoverable. That to me isn't a deal-breaker, though 0 I'm more concerned about breaking something that used to work and in fact works in virtually every other program (multiple clicks is a safe operation in pretty much every program I can think of). It's not out of the question that we tweak it to make it so that subsequent clicks have no effect unless there is another element underneath the selected one, but I am not crazy about the inconsistency. Maybe it would feel natural, maybe it would be confusing. Would need to try it out to see. |
|
OK, well I'll yield to your judgement here. It seems the concern is how much risk could result from changing the behavior of click may be worse than the possible benefits from increased discovery. If inkscape and other programs uses Alt for this behavior (I actually don't have enough knowledge to know if that is standard practice), then musescore should probably use Alt as well, so is at least discoverable to those people. |
|
FWIW, I still think this is worth considering. I realize the merge conflicts would need to be resolved first though. |
|
See also https://musescore.org/en/node/279999. Would be good to show feedback of what is selected. I do hope to revisit this at some point... |
|
I kind of want to revisit this one too... Short summary: right now, ctrl-click on an element adds it to the selection if it is not currently selected, removes it from the selection if it is currently selected. My PR here keeps this behavior so in most cases, there is no difference whatsoever. The only difference is that if the clicked element is already selected and it overlaps other elements, then not only do we deselect the clicked element, but we also select the next element down in the stack. The idea being to make it easier to select individual elements that are overlapping each other. |
|
Now superceded by #4717 |
Seeking feedback - does the following make sense, how does it feel in practice?
This PR implements two related tweaks to how clicking works with respect to selection of overlapping elements:
For elements that overlap, Ctrl+click will now cycle through all overlapping elements. So if one element in a stack is selected, Ctrl+click will deselect it (as it currently does) but also select the next deeper element in in the stack.
For purpose of selection, stems are considered to be deeper in the stack than notes (although I left the actual z-order alone, because it seems important for layout in tab staves). Thus, clicking the overlap area between the notehead and stem will select the note rather than the stem. This makes it much easier to select a note at small magnifications.
Apparently Inkscape uses "Alt" instead of "Ctrl" to do the cycle thing. In principle I assume we could do this as well, but my system seems unable to captrue Alt+click - I guess the OS (Ubuntu running in a chroot under Chrome OS) traps it. It doesn't work for me in Inkspace, and I can't get it to work in MuseScore either. Which in itself makes me prefer Ctrl, but if Alt+click really is a standard for this sort of operation and if it works on "most" systems, we could certainly use that instead.
I should note there appear to be issues with Ctr+click in the current master - if an object is already selected and you Ctrl+click to deselect it, the visual feedback doesn't seem to update properly. It is deselected (as a subsequent oepration like Delete demonstrates), but the display is not updated. I guess that's related to the new incremental layout code. My code does the deslection a bit differently, but seems to have the same issue in the end.