fix #301496: voltas excluded from navigation

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.
This commit is contained in:
MarcSabatella 2020-02-23 16:09:35 -07:00
parent 66c38772dc
commit ad3b5c2dde
5 changed files with 490 additions and 20 deletions

View file

@ -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: {
@ -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();

View file

@ -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();
});
@ -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();
});
@ -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;
}
@ -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;
}
@ -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;
@ -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;

View file

@ -947,9 +947,28 @@ 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
// see also https://musescore.org/en/node/301496
// and https://github.com/musescore/MuseScore/pull/5755
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;
}
@ -979,9 +998,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;
}

View file

@ -0,0 +1,375 @@
<?xml version="1.0" encoding="UTF-8"?>
<museScore version="3.01">
<Score>
<LayerTag id="0" tag="default"></LayerTag>
<currentLayer>0</currentLayer>
<Division>480</Division>
<Style>
<pageWidth>8.27</pageWidth>
<pageHeight>11.69</pageHeight>
<pagePrintableWidth>7.4826</pagePrintableWidth>
<lastSystemFillLimit>0</lastSystemFillLimit>
<Spatium>1.76389</Spatium>
</Style>
<showInvisible>1</showInvisible>
<showUnprintable>1</showUnprintable>
<showFrames>1</showFrames>
<showMargins>0</showMargins>
<metaTag name="arranger"></metaTag>
<metaTag name="composer"></metaTag>
<metaTag name="copyright"></metaTag>
<metaTag name="creationDate">2018-11-12</metaTag>
<metaTag name="lyricist"></metaTag>
<metaTag name="movementNumber"></metaTag>
<metaTag name="movementTitle"></metaTag>
<metaTag name="poet"></metaTag>
<metaTag name="source"></metaTag>
<metaTag name="translator"></metaTag>
<metaTag name="workNumber"></metaTag>
<metaTag name="workTitle">Treble</metaTag>
<Part>
<Staff id="1">
<StaffType group="pitched">
<name>stdNormal</name>
</StaffType>
</Staff>
<trackName>Piano</trackName>
<Instrument>
<longName>Piano</longName>
<shortName>Pno.</shortName>
<trackName>Piano</trackName>
<minPitchP>21</minPitchP>
<maxPitchP>108</maxPitchP>
<minPitchA>21</minPitchA>
<maxPitchA>108</maxPitchA>
<instrumentId>keyboard.piano</instrumentId>
<Articulation>
<velocity>100</velocity>
<gateTime>95</gateTime>
</Articulation>
<Articulation name="staccatissimo">
<velocity>100</velocity>
<gateTime>33</gateTime>
</Articulation>
<Articulation name="staccato">
<velocity>100</velocity>
<gateTime>50</gateTime>
</Articulation>
<Articulation name="portato">
<velocity>100</velocity>
<gateTime>67</gateTime>
</Articulation>
<Articulation name="tenuto">
<velocity>100</velocity>
<gateTime>100</gateTime>
</Articulation>
<Articulation name="marcato">
<velocity>120</velocity>
<gateTime>67</gateTime>
</Articulation>
<Articulation name="sforzato">
<velocity>120</velocity>
<gateTime>100</gateTime>
</Articulation>
<Channel>
<program value="0"/>
</Channel>
</Instrument>
</Part>
<Staff id="1">
<VBox>
<height>10</height>
<Text>
<style>Title</style>
<text>Treble</text>
</Text>
</VBox>
<Measure>
<voice>
<TimeSig>
<sigN>4</sigN>
<sigD>4</sigD>
</TimeSig>
<Spanner type="Volta">
<Volta>
<endHookType>1</endHookType>
<beginText>1.</beginText>
<visible>0</visible>
<Segment>
<subtype>0</subtype>
<offset x="0" y="-3"/>
<off2 x="0" y="0"/>
<visible>0</visible>
</Segment>
<endings>1</endings>
</Volta>
<next>
<location>
<measures>2</measures>
</location>
</next>
</Spanner>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Spanner type="Volta">
<prev>
<location>
<measures>-2</measures>
</location>
</prev>
</Spanner>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
<Measure>
<voice>
<Rest>
<durationType>measure</durationType>
<duration>4/4</duration>
</Rest>
</voice>
</Measure>
</Staff>
</Score>
</museScore>

View file

@ -0,0 +1,11 @@
init init/Treble.mscx
cmd note-input
cmd note-input
cmd select-next-chord
palette Volta beginHookType 1 endHookType 1 beginText 1.
cmd escape
cmd escape
cmd get-location
cmd next-element
cmd toggle-visible
test score 301496-navigate-volta.mscx