Skip to content

Commit

Permalink
fix #301496: voltas excluded from navigation
Browse files Browse the repository at this point in the history 
Resolves: https://musescore.org/en/node/301496

Alt+Left/Right commands were skipping voltas because
we were checking start element and checking against the active staff,
but the start element is actually the measure for voltas.
The main change here is to go ahead and visit the volta
if the active staff if you are navigating the top staff.
Arguably, it could make sense to check for the top *visible* staff,
since that is what the volta at least appears to be attached to.
So I have code here to that.
But I disabled it because in practice,
neither the navigation commands themsevles nor the screen reader
treat invisible staves specially.
So a blind user navigating would have no way of knowing
the top staff is not visible.
So they would likely continue to see it as relevant.

I would not the same issue occurs for system text,
which we always treat as attached to the top staff only.
I added a TODO to indicate where this code would need updating.

Eventually we could consider coming up with some way
of presenting information about hidden staves.
Perhaps in conjunction with a facility allow user
to hide staves on specific systems only,
which seems to be a fairly common request.
  • Loading branch information
@MarcSabatella
MarcSabatella committed Feb 23, 2020
1 parent 66c3877 commit a91414c
Show file tree
Hide file tree
Showing 5 changed files with 488 additions and 20 deletions.
24 changes: 22 additions & 2 deletions libmscore/navigate.cpp
View file
Expand Up @@ -575,13 +575,23 @@ Element* Score::nextElement()
else
return score()->firstElement();
}
#if 1
case ElementType::VOLTA_SEGMENT:
#else
case ElementType::VOLTA_SEGMENT: {
// TODO: see Spanner::nextSpanner()
System* sys = toSpannerSegment(e)->system();
if (sys)
staffId = sys->firstVisibleStaff();
}
// fall through
#endif
case ElementType::SLUR_SEGMENT:
case ElementType::TEXTLINE_SEGMENT:
case ElementType::HAIRPIN_SEGMENT:
case ElementType::OTTAVA_SEGMENT:
case ElementType::TRILL_SEGMENT:
case ElementType::VIBRATO_SEGMENT:
case ElementType::VOLTA_SEGMENT:
case ElementType::LET_RING_SEGMENT:
case ElementType::PALM_MUTE_SEGMENT:
case ElementType::PEDAL_SEGMENT: {
Expand Down Expand Up @@ -682,13 +692,23 @@ Element* Score::prevElement()
Segment* s = toSegment(e);
return s->prevElement(staffId);
}
#if 1
case ElementType::VOLTA_SEGMENT:
#else
case ElementType::VOLTA_SEGMENT: {
// TODO: see Spanner::nextSpanner()
System* sys = toSpannerSegment(e)->system();
if (sys)
staffId = sys->firstVisibleStaff();
}
// fall through
#endif
case ElementType::SLUR_SEGMENT:
case ElementType::TEXTLINE_SEGMENT:
case ElementType::HAIRPIN_SEGMENT:
case ElementType::OTTAVA_SEGMENT:
case ElementType::TRILL_SEGMENT:
case ElementType::VIBRATO_SEGMENT:
case ElementType::VOLTA_SEGMENT:
case ElementType::PEDAL_SEGMENT: {
SpannerSegment* s = toSpannerSegment(e);
Spanner* sp = s->spanner();
Expand Down
55 changes: 43 additions & 12 deletions libmscore/segment.cpp
View file
Expand Up @@ -1177,7 +1177,8 @@ Element* Segment::nextAnnotation(Element* e)
auto ei = std::find(_annotations.begin(), _annotations.end(), e);
if (ei == _annotations.end())
return nullptr; // element not found


// TODO: firstVisibleStaff() for system elements? see Spanner::nextSpanner()
auto resIt = std::find_if(ei + 1, _annotations.end(), [e](Element* nextElem){
return nextElem && nextElem->staffIdx() == e->staffIdx();
});
Expand All @@ -1197,7 +1198,8 @@ Element* Segment::prevAnnotation(Element* e)
auto reverseIt = std::find(_annotations.rbegin(), _annotations.rend(), e);
if (reverseIt == _annotations.rend())
return nullptr; // element not found


// TODO: firstVisibleStaff() for system elements? see Spanner::nextSpanner()
auto resIt = std::find_if(reverseIt + 1, _annotations.rend(), [e](Element* prevElem){
return prevElem && prevElem->staffIdx() == e->staffIdx();
});
Expand All @@ -1212,6 +1214,7 @@ Element* Segment::prevAnnotation(Element* e)
Element* Segment::firstAnnotation(Segment* s, int activeStaff)
{
for (auto i = s->annotations().begin(); i != s->annotations().end(); ++i) {
// TODO: firstVisibleStaff() for system elements? see Spanner::nextSpanner()
if ((*i)->staffIdx() == activeStaff)
return *i;
}
Expand All @@ -1225,6 +1228,7 @@ Element* Segment::firstAnnotation(Segment* s, int activeStaff)
Element* Segment::lastAnnotation(Segment* s, int activeStaff)
{
for (auto i = --s->annotations().end(); i != s->annotations().begin(); --i) {
// TODO: firstVisibleStaff() for system elements? see Spanner::nextSpanner()
if ((*i)->staffIdx() == activeStaff)
return *i;
}
Expand Down Expand Up @@ -1429,8 +1433,22 @@ Spanner* Segment::firstSpanner(int activeStaff)
Element* e = s->startElement();
if (!e)
continue;
if (s->startSegment() == this && s->startElement()->staffIdx() == activeStaff)
return s;
if (s->startSegment() == this) {
if (e->staffIdx() == activeStaff)
return s;
#if 1
else if (e->isMeasure() && activeStaff == 0)
return s;
#else
// TODO: see Spanner::nextSpanner()
else if (e->isMeasure()) {
SpannerSegment* ss = s->frontSegment();
int top = ss && ss->system() ? ss->system()->firstVisibleStaff() : 0;
if (activeStaff == top)
return s;
}
#endif
}
}
}
return nullptr;
Expand All @@ -1445,16 +1463,29 @@ Spanner* Segment::lastSpanner(int activeStaff)
std::multimap<int, Spanner*> mmap = score()->spanner();
auto range = mmap.equal_range(tick().ticks());
if (range.first != range.second){ // range not empty
for (auto i = --range.second; i != range.first; --i) {
for (auto i = --range.second; ; --i) {
Spanner* s = i->second;
Element* st = s->startElement();
if (!st)
Element* e = s->startElement();
if (!e)
continue;
if (s->startElement()->staffIdx() == activeStaff)
return s;
}
if ((range.first)->second->startElement()->staffIdx() == activeStaff) {
return (range.first)->second;
if (s->startSegment() == this) {
if (e->staffIdx() == activeStaff)
return s;
#if 1
else if (e->isMeasure() && activeStaff == 0)
return s;
#else
// TODO: see Spanner::nextSpanner()
else if (e->isMeasure()) {
SpannerSegment* ss = s->frontSegment();
int top = ss && ss->system() ? ss->system()->firstVisibleStaff() : 0;
if (activeStaff == top)
return s;
}
#endif
}
if (i == range.first)
break;
}
}
return nullptr;
Expand Down
43 changes: 37 additions & 6 deletions libmscore/spanner.cpp
View file
Expand Up @@ -947,9 +947,26 @@ Spanner* Spanner::nextSpanner(Element* e, int activeStaff)
Element* st = s->startElement();
if (!st)
continue;
if (s->startSegment() == toSpanner(e)->startSegment() &&
st->staffIdx() == activeStaff)
return s;
if (s->startSegment() == toSpanner(e)->startSegment()) {
if (st->staffIdx() == activeStaff)
return s;
#if 1
else if (st->isMeasure() && activeStaff == 0)
return s;
#else
// TODO: when navigating system spanners, check firstVisibleStaff()?
// currently, information about which staves are hidden
// is not exposed through navigation,
// so it may make more sense to continue to navigate systems elements
// only when actually on staff 0
else if (st->isMeasure()) {
SpannerSegment* ss = s->frontSegment();
int top = ss && ss->system() ? ss->system()->firstVisibleStaff() : 0;
if (activeStaff == top)
return s;
}
#endif
}
//else
//return nullptr;
}
Expand Down Expand Up @@ -979,9 +996,23 @@ Spanner* Spanner::prevSpanner(Element* e, int activeStaff)
while (i != range.first) {
--i;
Spanner* s = i->second;
if (s->startSegment() == toSpanner(e)->startSegment() &&
s->startElement()->staffIdx() == activeStaff)
return s;
Element* st = s->startElement();
if (s->startSegment() == toSpanner(e)->startSegment()) {
if (st->staffIdx() == activeStaff)
return s;
#if 1
else if (st->isMeasure() && activeStaff == 0)
return s;
#else
// TODO: see nextSpanner()
else if (st->isMeasure()) {
SpannerSegment* ss = s->frontSegment();
int top = ss && ss->system() ? ss->system()->firstVisibleStaff() : 0;
if (activeStaff == top)
return s;
}
#endif
}
}
break;
}
Expand Down

0 comments on commit a91414c

Please sign in to comment.