[MU4] Move ScoreAccessibility::barbeat(Element) to Element::barbeat()

[mirabilos]

No caller passes a nil argument, and an Element can now describe its own position, which is useful when generating debugging output.

A function QString Element::accessibleBarbeat() is also added, based on ScoreAccessibility::currentInfoChanged() for elements.

Both methods can be used on const elements, which, again, is important for debugging output.

Finally, the accessibility strings generated by ScoreAccessibility::currentInfoChanged() were inconsistent; now, the status line text uses colons and semicolons consistently, and both it and the score->setAccessibleInfo() argument have a consistent amount of spaces added (before, sometimes there were two or even (worse) no space where one was expected).

As a downside, since ScoreAccessibility::currentInfoChanged() needs to calculate the information twice, and also for ranges and spanners, there is a tiny amount of code duplication (as also the order of execution differs) between it and the new Element::accessibleBarbeat(). This could be fixed by refactoring more (introduce accessibleBarbeat() to spanners and range selections and make it return an std::pair<QString, QString> providing both information) but I’ve decided to keep it simple for now.

Use case: I have a code path in a local patch to the synthesiser in which I wish to output information (pitch, voice, beat, measure, stave) for a const Note*. I imagine others have the same problem.

[dmitrio95]

Actually it looks like Element is not the best place for a method like this. It could probably be better to make a separate class for constructing accessible info for elements, or at least make methods like this virtual and override them in descendant classes. Both these solutions might go beyond the scope of this PR though.

[mirabilos]

Dmitri Ovodok dixit:

Actually it looks like `Element` is not the best place for a method like this. It could probably be better to make a separate class for constructing accessible info for elements

Uh, why?

, or at least make methods like this virtual and override them in descendant classes.

That sounds sensible, should that ever be needed.

Both these solutions might go beyond the scope of this PR though.

Yes, this PR addresses a real problem I have, although I’m using this currently only in some print-to-stderr code, not the UI. bye, //mirabilos -- <cnuke> den AGP stecker anfeilen, damit er in den slot aufm 440BX board passt… oder netzteile, an die man auch den monitor angeschlossen hat und die dann für ein elektrisch aufgeladenes gehäuse gesorgt haben […] für lacher gut auf jeder LAN party │ <nvb> damals, als der pizzateig noch auf dem monior "gegangen" ist

[MarcSabatella]

I do think there is room for cleaning up how this is handled, but I'm not terribly crazy about having Element construct this information itself. Why not just adapt the existing barBeat to be a publicly-visible static member function of ScoreAccessibility? Or perhaps a general utility function that you pass in a tick to.

In any case, it also needs rebasing to account for the actual changes in how the info is presented. @shoogle has also been investigating other architectural changes, not sure how that is going or if that would conflict.

[shoogle]

My architectural changes have been put on hold for the time being (see GSoC idea Tree model for libmscore) so feel free to make more straightforward improvements in the meantime.

[mirabilos]

Marc Sabatella dixit:

Why not just adapt the existing barBeat to be a publicly-visible static member function of ScoreAccessibility?

Because I need it in libmscore.

[MarcSabatella]

OK, although it might help to understand why. To me, it "feels" like libmscore shoudl be able to do things like tell you the tick of an element, maybe even return an integer for measure number and fraction or qreal for beat. But mscore is more appropriate for code that turns that into any sort of English (or other language) descriptive phrase. After all, it's UI-ish.

If there is some good reason I'm not seeing for having it in libmscore, though, then maybe have it be a static member of Score instead.

[mirabilos]

Marc Sabatella dixit:

If there is some good reason I'm not seeing for having it in libmscore,

Debugging, in my case in the MIDI renderer. bye, //mirabilos -- 15:41⎜<Lo-lan-do:#fusionforge> Somebody write a testsuite for helloworld :-)

[MarcSabatella]

I'm not understanding what debugging has to to do with it, debuggers are just as capable of accessing mscore code as libmscore code. Sounds like you have some sort of real unusual special project going on here, but we don't have the context needed to understand its unique requirements.

[mirabilos]

Marc Sabatella dixit:

I'm not understanding what debugging has to to do with it, debuggers

Debugging, not debugger. As in, output on stdout or stderr, to inform the user.

[Jojo-Schmitz]

Rebase needed. Maybe on top of 3.x?

[mirabilos]

Joachim Schmitz dixit:

Rebase needed. Maybe on top of 3.x?

On top of 3.x is attached, it’s the original version of the diff anyway. bye, //mirabilos -- /⁀\ The UTF-8 Ribbon ╲ ╱ Campaign against ╳ HTML eMail! Also, ╱ ╲ header encryption! Description: Add Element::barbeat() and Element::accessibleBarbeat() These are const, and useful for determining the positions of elements within the score, generally; used by experiments/log-collisions.diff . Also fix some screenreader label mistakes. Author: mirabilos ***@***.***> Forwarded: #5096

…

--- a/libmscore/element.cpp +++ b/libmscore/element.cpp @@ -2432,4 +2432,53 @@ void Element::autoplaceMeasureElement(bo setOffsetChanged(false); } +//--------------------------------------------------------- +// barbeat +//--------------------------------------------------------- + +std::pair<int, float> Element::barbeat() const + { + int bar = 0; + int beat = 0; + int ticks = 0; + TimeSigMap* tsm = this->score()->sigmap(); + const Element* p = this; + int ticksB = ticks_beat(tsm->timesig(0).timesig().denominator()); + while(p && p->type() != ElementType::SEGMENT && p->type() != ElementType::MEASURE) + p = p->parent(); + + if (!p) { + return std::pair<int, float>(0, 0.0F); + } + else if (p->type() == ElementType::SEGMENT) { + const Segment* seg = static_cast<const Segment*>(p); + tsm->tickValues(seg->tick().ticks(), &bar, &beat, &ticks); + ticksB = ticks_beat(tsm->timesig(seg->tick().ticks()).timesig().denominator()); + } + else if (p->type() == ElementType::MEASURE) { + const Measure* m = static_cast<const Measure*>(p); + bar = m->no(); + beat = -1; + ticks = 0; + } + return std::pair<int,float>(bar + 1, beat + 1 + ticks / static_cast<float>(ticksB)); + } + +//--------------------------------------------------------- +// accessibleBarbeat +//--------------------------------------------------------- + +QString Element::accessibleBarbeat() const + { + QString barsAndBeats = ""; + std::pair<int, float>bar_beat = barbeat(); + if (bar_beat.first) { + barsAndBeats += "; " + QObject::tr("Measure: %1").arg(QString::number(bar_beat.first)); + if (bar_beat.second) + barsAndBeats += "; " + QObject::tr("Beat: %1").arg(QString::number(bar_beat.second)); + } + if (staffIdx() + 1) + barsAndBeats += "; " + QObject::tr("Staff: %1").arg(QString::number(staffIdx() + 1)); + return barsAndBeats; + } } --- a/libmscore/element.h +++ b/libmscore/element.h @@ -197,6 +197,8 @@ class Element : public ScoreElement { virtual bool isElement() const override { return true; } qreal spatium() const; + std::pair<int, float>barbeat() const; + QString accessibleBarbeat() const; inline void setFlag(ElementFlag f, bool v) { if (v) _flags |= f; else _flags &= ~ElementFlags(f); } inline void setFlag(ElementFlag f, bool v) const { if (v) _flags |= f; else _flags &= ~ElementFlags(f); } --- a/mscore/scoreaccessibility.cpp +++ b/mscore/scoreaccessibility.cpp @@ -124,8 +124,8 @@ void ScoreAccessibility::currentInfoChan QString barsAndBeats = ""; if (el->isSpanner()){ Spanner* s = static_cast<Spanner*>(el); - std::pair<int, float> bar_beat = barbeat(s->startSegment()); - barsAndBeats += tr("Start Measure: %1; Start Beat: %2").arg(QString::number(bar_beat.first)).arg(QString::number(bar_beat.second)); + std::pair<int, float> bar_beat = s->startSegment()->barbeat(); + barsAndBeats += " " + tr("Start Measure: %1; Start Beat: %2").arg(QString::number(bar_beat.first)).arg(QString::number(bar_beat.second)); Segment* seg = s->endSegment(); if(!seg) seg = score->lastSegment()->prev1MM(SegmentType::ChordRest); @@ -135,11 +135,11 @@ void ScoreAccessibility::currentInfoChan s->type() != ElementType::TIE ) seg = seg->prev1MM(SegmentType::ChordRest); - bar_beat = barbeat(seg); + bar_beat = seg->barbeat(); barsAndBeats += "; " + tr("End Measure: %1; End Beat: %2").arg(QString::number(bar_beat.first)).arg(QString::number(bar_beat.second)); } else { - std::pair<int, float>bar_beat = barbeat(el); + std::pair<int, float>bar_beat = el->barbeat(); if (bar_beat.first) { barsAndBeats += " " + tr("Measure: %1").arg(QString::number(bar_beat.first)); if (bar_beat.second) @@ -149,12 +149,12 @@ void ScoreAccessibility::currentInfoChan QString rez = e->accessibleInfo(); if (!barsAndBeats.isEmpty()) - rez += "; " + barsAndBeats; + rez += ";" + barsAndBeats; QString staff = ""; if (e->staffIdx() + 1) { staff = tr("Staff %1").arg(QString::number(e->staffIdx() + 1)); - rez = QString("%1; %2").arg(rez).arg(staff); + rez += "; " + tr("Staff: %1").arg(QString::number(e->staffIdx() + 1)); } statusBarLabel->setText(rez); @@ -165,7 +165,7 @@ void ScoreAccessibility::currentInfoChan QString barsAndBeats = ""; std::pair<int, float> bar_beat; - bar_beat = barbeat(score->selection().startSegment()); + bar_beat = score->selection().startSegment()->barbeat(); barsAndBeats += " " + tr("Start Measure: %1; Start Beat: %2").arg(QString::number(bar_beat.first)).arg(QString::number(bar_beat.second)); Segment* endSegment = score->selection().endSegment(); @@ -174,7 +174,7 @@ void ScoreAccessibility::currentInfoChan else endSegment = endSegment->prev1MM(); - bar_beat = barbeat(endSegment); + bar_beat = endSegment->barbeat(); barsAndBeats += " " + tr("End Measure: %1; End Beat: %2").arg(QString::number(bar_beat.first)).arg(QString::number(bar_beat.second)); statusBarLabel->setText(tr("Range Selection") + barsAndBeats); score->setAccessibleInfo(tr("Range Selection") + barsAndBeats); @@ -217,36 +217,4 @@ void ScoreAccessibility::updateAccessibi QAccessibleValueChangeEvent ev(obj, w->score()->accessibleInfo()); QAccessible::updateAccessibility(&ev); } - -std::pair<int, float> ScoreAccessibility::barbeat(Element *e) - { - if (!e) { - return std::pair<int, float>(0, 0.0F); - } - - int bar = 0; - int beat = 0; - int ticks = 0; - TimeSigMap* tsm = e->score()->sigmap(); - Element* p = e; - int ticksB = ticks_beat(tsm->timesig(0).timesig().denominator()); - while(p && p->type() != ElementType::SEGMENT && p->type() != ElementType::MEASURE) - p = p->parent(); - - if (!p) { - return std::pair<int, float>(0, 0.0F); - } - else if (p->type() == ElementType::SEGMENT) { - Segment* seg = static_cast<Segment*>(p); - tsm->tickValues(seg->tick().ticks(), &bar, &beat, &ticks); - ticksB = ticks_beat(tsm->timesig(seg->tick().ticks()).timesig().denominator()); - } - else if (p->type() == ElementType::MEASURE) { - Measure* m = static_cast<Measure*>(p); - bar = m->no(); - beat = -1; - ticks = 0; - } - return pair<int,float>(bar + 1, beat + 1 + ticks / static_cast<float>(ticksB)); - } } --- a/mscore/scoreaccessibility.h +++ b/mscore/scoreaccessibility.h @@ -39,7 +39,6 @@ class ScoreAccessibility : public QObjec QMainWindow* mainWindow; QLabel* statusBarLabel; ScoreAccessibility(QMainWindow* statusBar); - std::pair<int, float>barbeat(Element* e); public: ~ScoreAccessibility();