Skip to content

[MU3 Backend] ENG-78 fix spanner offsets #8766

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 5, 2021
Merged

[MU3 Backend] ENG-78 fix spanner offsets #8766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
86e9930
ENG-68: Fix pedal endpoints on other tracks
iveshenry18 Jul 22, 2021
aae940d
ENG-85: Improve tuplet rounding error correction
iveshenry18 Jul 27, 2021
d22df37
ENG-78: fix wedges with <offset> values
iveshenry18 Jul 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 5 additions & 14 deletions importexport/musicxml/importmxmlnoteduration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ QString mxmlNoteDuration::checkTiming(const QString& type, const bool rest, cons
errorStr = "";
}
else {
const int maxDiff = 3; // maximum difference considered a rounding error
if (qAbs(calcDura.ticks() - _dura.ticks()) <= maxDiff) {
if (qAbs(calcDura.ticks() - _dura.ticks()) <= _pass1->maxDiff()) {
errorStr += " -> assuming rounding error";
_pass1->insertAdjustedDuration(_dura, calcDura);
_dura = calcDura;
}
}
Expand Down Expand Up @@ -157,6 +157,7 @@ QString mxmlNoteDuration::checkTiming(const QString& type, const bool rest, cons
_dura = Fraction(4, 4);
}

_pass1->insertSeenDenominator(_dura.reduced().denominator());
return errorStr;
}

Expand All @@ -173,19 +174,9 @@ void mxmlNoteDuration::duration(QXmlStreamReader& e)
Q_ASSERT(e.isStartElement() && e.name() == "duration");
_logger->logDebugTrace("MusicXMLParserPass1::duration", &e);

_dura.set(0, 0); // invalid unless set correctly
_dura.set(0, 0); // invalid unless set correctly
int intDura = e.readElementText().toInt();
if (intDura > 0) {
if (_divs > 0) {
_dura.set(intDura, 4 * _divs);
_dura.reduce(); // prevent overflow in later Fraction operations
}
else
_logger->logError("illegal or uninitialized divisions", &e);
}
else
_logger->logError("illegal duration", &e);
//qDebug("duration %s valid %d", qPrintable(dura.print()), dura.isValid());
_dura = _pass1->calcTicks(intDura, &e); // Duration reading (and rounding) code consolidated to pass1
}

//---------------------------------------------------------
Expand Down
5 changes: 4 additions & 1 deletion importexport/musicxml/importmxmlnoteduration.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

#include "libmscore/durationtype.h"
#include "libmscore/fraction.h"
#include "importmxmlpass1.h"

namespace Ms {

Expand All @@ -31,7 +32,8 @@ class MxmlLogger;
class mxmlNoteDuration
{
public:
mxmlNoteDuration(int divs, MxmlLogger* logger) : _divs(divs), _logger(logger) { /* nothing so far */ }
mxmlNoteDuration(int divs, MxmlLogger* logger, MusicXMLParserPass1* pass1) :
_divs(divs), _logger(logger), _pass1(pass1) { /* nothing so far */ }
QString checkTiming(const QString& type, const bool rest, const bool grace);
Fraction dura() const { return _dura; }
int dots() const { return _dots; }
Expand All @@ -47,6 +49,7 @@ class mxmlNoteDuration
Fraction _dura;
TDuration _normalType;
Fraction _timeMod { 1, 1 }; // default to no time modification
MusicXMLParserPass1* _pass1;
MxmlLogger* _logger; ///< Error logger
};

Expand Down
65 changes: 49 additions & 16 deletions importexport/musicxml/importmxmlpass1.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3254,7 +3254,7 @@ void MusicXMLParserPass1::note(const QString& partId,
QString instrId;
MxmlStartStop tupletStartStop { MxmlStartStop::NONE };

mxmlNoteDuration mnd(_divs, _logger);
mxmlNoteDuration mnd(_divs, _logger, this);

while (_e.readNextStartElement()) {
if (mnd.readProperties(_e)) {
Expand Down Expand Up @@ -3419,6 +3419,49 @@ void MusicXMLParserPass1::notePrintSpacingNo(Fraction& dura)
Q_ASSERT(_e.isEndElement() && _e.name() == "note");
}

//---------------------------------------------------------
// calcTicks
//---------------------------------------------------------

Fraction MusicXMLParserPass1::calcTicks(const int& intTicks, const QXmlStreamReader* const xmlReader)
{
Fraction dura(0, 1); // invalid unless set correctly

if (_divs > 0) {
dura.set(intTicks, 4 * _divs);
dura.reduce(); // prevent overflow in later Fraction operations

// Correct for previously adjusted durations
// This is necessary when certain tuplets are
// followed by a <backup> element.
// There are two strategies:
// 1. Use a lookup table of previous adjustments
// 2. Check if within maxDiff of a seenDenominator
if (_adjustedDurations.contains(dura)) {
dura = _adjustedDurations.value(dura);
}
else if (dura.reduced().denominator() > 64) {
for (auto seenDenominator : _seenDenominators) {
int seenDenominatorTicks = Fraction(1, seenDenominator).ticks();
if (qAbs(dura.ticks() % seenDenominatorTicks) <= _maxDiff) {
Fraction roundedDura = Fraction(std::round(dura.ticks() / double(seenDenominatorTicks)), seenDenominator);
roundedDura.reduce();
_logger->logError(QString("calculated duration (%1) assumed to be a rounding error by proximity to (%2)")
.arg(dura.print(), roundedDura.print()));
insertAdjustedDuration(dura, roundedDura);
dura = roundedDura;
break;
}
}
}
}
else
_logger->logError("illegal or uninitialized divisions", xmlReader);
//qDebug("duration %s valid %d", qPrintable(dura.print()), dura.isValid());

return dura;
}

//---------------------------------------------------------
// duration
//---------------------------------------------------------
Expand All @@ -3427,24 +3470,14 @@ void MusicXMLParserPass1::notePrintSpacingNo(Fraction& dura)
Parse the /score-partwise/part/measure/note/duration node.
*/

void MusicXMLParserPass1::duration(Fraction& dura)
void MusicXMLParserPass1::duration(Fraction& dura, QXmlStreamReader& e)
{
Q_ASSERT(_e.isStartElement() && _e.name() == "duration");
//_logger->logDebugTrace("MusicXMLParserPass1::duration", &_e);
Q_ASSERT(e.isStartElement() && e.name() == "duration");
_logger->logDebugTrace("MusicXMLParserPass1::duration", &e);

dura.set(0, 0); // invalid unless set correctly
int intDura = _e.readElementText().toInt();
if (intDura > 0) {
if (_divs > 0) {
dura.set(intDura, 4 * _divs);
dura.reduce(); // prevent overflow in later Fraction operations
}
else
_logger->logError("illegal or uninitialized divisions", &_e);
}
else
_logger->logError("illegal duration", &_e);
//qDebug("duration %s valid %d", qPrintable(dura.print()), dura.isValid());
int intDura = e.readElementText().toInt();
dura = calcTicks(intDura);
}

//---------------------------------------------------------
Expand Down
12 changes: 11 additions & 1 deletion importexport/musicxml/importmxmlpass1.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,10 @@ class MusicXMLParserPass1 {
void notations(MxmlStartStop& tupletStartStop);
void note(const QString& partId, const Fraction cTime, Fraction& missingPrev, Fraction& dura, Fraction& missingCurr, VoiceOverlapDetector& vod, MxmlTupletStates& tupletStates);
void notePrintSpacingNo(Fraction& dura);
void duration(Fraction& dura);
Fraction calcTicks(const int& intTicks, const QXmlStreamReader* const xmlReader);
Fraction calcTicks(const int& intTicks) { return calcTicks(intTicks, &_e); }
void duration(Fraction& dura, QXmlStreamReader& e);
void duration(Fraction& dura) { duration(dura, _e); }
void forward(Fraction& dura);
void backup(Fraction& dura);
void timeModification(Fraction& timeMod);
Expand Down Expand Up @@ -177,6 +180,10 @@ class MusicXMLParserPass1 {
const std::set<int>& pageStartMeasureNrs,
const CreditWordsList& crWords,
const QSize pageSize);
const int maxDiff() { return _maxDiff; }
void insertAdjustedDuration(Fraction key, Fraction value) { _adjustedDurations.insert(key, value); }
QMap<Fraction, Fraction>& adjustedDurations() { return _adjustedDurations; }
void insertSeenDenominator(int val) { _seenDenominators.emplace(val); }
QString supportsTranspose() const { return _supportsTranspose; }
void addInferredTranspose(const QString& partId);
void setHasInferredHeaderText(bool b) { _hasInferredHeaderText = b; }
Expand All @@ -202,6 +209,9 @@ class MusicXMLParserPass1 {
bool _hasBeamingInfo; ///< Whether the score supports or contains beaming info
QString _supportsTranspose; ///< Whether the score supports transposition info
bool _hasInferredHeaderText;
const int _maxDiff = 5; ///< Duration rounding tick threshold;
QMap<Fraction, Fraction> _adjustedDurations; ///< Rounded durations
std::set<int> _seenDenominators; ///< Denominators seen. Used for rounding errors.

// part specific data (TODO: move to part-specific class)
Fraction _timeSigDura; ///< Measure duration according to last timesig read
Expand Down
71 changes: 35 additions & 36 deletions importexport/musicxml/importmxmlpass2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -959,14 +959,22 @@ static void handleTupletStop(Tuplet*& tuplet, const int normalNotes)
tuplet->setTicks(f);
// TODO determine usefulness of following check
int totalDuration = 0;
foreach (DurationElement* de, tuplet->elements()) {
int ticksPerNote = f.ticks() / tuplet->ratio().numerator();
bool ticksCorrect = true;
for (DurationElement* de : tuplet->elements()) {
if (de->type() == ElementType::CHORD || de->type() == ElementType::REST) {
totalDuration+=de->globalTicks().ticks();
int globalTicks = de->globalTicks().ticks();
if (globalTicks != ticksPerNote)
ticksCorrect = false;
totalDuration += globalTicks;
}
}
if (!(totalDuration && normalNotes)) {
if (totalDuration != f.ticks()) {
qDebug("MusicXML::import: tuplet stop but bad duration"); // TODO
}
if (!ticksCorrect) {
qDebug("MusicXML::import: tuplet stop but uneven note ticks"); // TODO
}
tuplet = 0;
}

Expand Down Expand Up @@ -2033,9 +2041,20 @@ void MusicXMLParserPass2::part()
auto sp = i.key();
Fraction tick1 = Fraction::fromTicks(i.value().first);
Fraction tick2 = Fraction::fromTicks(i.value().second);
if (sp->isPedal() && toPedal(sp)->endHookType() == HookType::HOOK_45)
if (sp->isPedal() && toPedal(sp)->endHookType() == HookType::HOOK_45) {
// Handle pedal change end tick (slightly hacky)
tick2 += _score->findCR(tick2, sp->track())->ticks();
// Find CR on the end tick of
ChordRest* terminatingCR = _score->findCR(tick2, sp->effectiveTrack2());
for (int track = _pass1.getPart(id)->startTrack(); track <= _pass1.getPart(id)->endTrack(); ++track) {
ChordRest* tempCR = _score->findCR(tick2, track);
if (!terminatingCR
|| (tempCR && tempCR->tick() > terminatingCR->tick())
|| (tempCR && tempCR->tick() == terminatingCR->tick() && tempCR->ticks() < terminatingCR->ticks()))
terminatingCR = tempCR;
}
tick2 += terminatingCR->ticks();
sp->setTrack2(terminatingCR->track());
}
//qDebug("spanner %p tp %d tick1 %s tick2 %s track1 %d track2 %d",
// sp, sp->type(), qPrintable(tick1.print()), qPrintable(tick2.print()), sp->track(), sp->track2());
if (incompleteSpanners.find(sp) == incompleteSpanners.end()) {
Expand Down Expand Up @@ -2371,7 +2390,7 @@ void MusicXMLParserPass2::measure(const QString& partId,
attributes(partId, measure, time + mTime);
else if (_e.name() == "direction") {
MusicXMLParserDirection dir(_e, _score, _pass1, *this, _logger);
dir.direction(partId, measure, time + mTime, _divs, _spanners, delayedDirections, inferredFingerings);
dir.direction(partId, measure, time + mTime, _spanners, delayedDirections, inferredFingerings);
}
else if (_e.name() == "figured-bass") {
FiguredBass* fb = figuredBass();
Expand Down Expand Up @@ -2751,25 +2770,6 @@ void MusicXMLParserPass2::measureStyle(Measure* measure)
}
}

//---------------------------------------------------------
// calcTicks
//---------------------------------------------------------

static Fraction calcTicks(const QString& text, int divs, MxmlLogger* logger, const QXmlStreamReader* const xmlreader)
{
Fraction dura(0, 0); // invalid unless set correctly

int intDura = text.toInt();
if (divs > 0) {
dura.set(intDura, 4 * divs);
dura.reduce();
}
else
logger->logError(QString("illegal or uninitialized divisions (%1)").arg(divs), xmlreader);

return dura;
}

//---------------------------------------------------------
// preventNegativeTick
//---------------------------------------------------------
Expand Down Expand Up @@ -2855,7 +2855,6 @@ void DelayedDirectionsList::combineTempoText()
void MusicXMLParserDirection::direction(const QString& partId,
Measure* measure,
const Fraction& tick,
const int divisions,
MusicXmlSpannerMap& spanners,
DelayedDirectionsList& delayedDirections,
InferredFingeringsList& inferredFingerings)
Expand Down Expand Up @@ -2884,7 +2883,7 @@ void MusicXMLParserDirection::direction(const QString& partId,
if (_e.name() == "direction-type")
directionType(starts, stops);
else if (_e.name() == "offset") {
_offset = calcTicks(_e.readElementText(), divisions, _logger, &_e);
_offset = _pass1.calcTicks(_e.readElementText().toInt(), &_e);
preventNegativeTick(tick, _offset, _logger);
}
else if (_e.name() == "sound")
Expand Down Expand Up @@ -3065,20 +3064,20 @@ void MusicXMLParserDirection::direction(const QString& partId,
}

// handle the spanner stops first
foreach (auto desc, stops) {
for (auto desc : stops) {
auto& spdesc = _pass2.getSpanner({ desc._tp, desc._nr });
if (spdesc._isStopped) {
_logger->logError("spanner already stopped", &_e);
delete desc._sp;
}
else {
if (spdesc._isStarted) {
handleSpannerStop(spdesc._sp, track, tick, spanners);
handleSpannerStop(spdesc._sp, track, tick + _offset, spanners);
_pass2.clearSpanner(desc);
}
else {
spdesc._sp = desc._sp;
spdesc._tick2 = tick;
spdesc._tick2 = tick + _offset;
spdesc._track2 = track;
spdesc._isStopped = true;
}
Expand All @@ -3087,7 +3086,7 @@ void MusicXMLParserDirection::direction(const QString& partId,

// then handle the spanner starts
// TBD handle offset ?
foreach (auto desc, starts) {
for (auto desc : starts) {
auto& spdesc = _pass2.getSpanner({ desc._tp, desc._nr });
if (spdesc._isStarted) {
_logger->logError("spanner already started", &_e);
Expand All @@ -3098,13 +3097,13 @@ void MusicXMLParserDirection::direction(const QString& partId,
_pass2.addSpanner(desc);
// handleSpannerStart and handleSpannerStop must be called in order
// due to allocation of elements in the map
handleSpannerStart(desc._sp, track, placement(), tick, spanners);
handleSpannerStart(desc._sp, track, placement(), tick + _offset, spanners);
handleSpannerStop(spdesc._sp, spdesc._track2, spdesc._tick2, spanners);
_pass2.clearSpanner(desc);
}
else {
_pass2.addSpanner(desc);
handleSpannerStart(desc._sp, track, placement(), tick, spanners);
handleSpannerStart(desc._sp, track, placement(), tick + _offset, spanners);
spdesc._isStarted = true;
}
}
Expand Down Expand Up @@ -5343,7 +5342,7 @@ Note* MusicXMLParserPass2::note(const QString& partId,
MusicXMLParserLyric lyric { _pass1.getMusicXmlPart(partId).lyricNumberHandler(), _e, _score, _logger };
MusicXMLParserNotations notations { _e, _score, _logger };

mxmlNoteDuration mnd { _divs, _logger };
mxmlNoteDuration mnd { _divs, _logger, &_pass1 };
mxmlNotePitch mnp { _logger };

while (_e.readNextStartElement()) {
Expand Down Expand Up @@ -5768,7 +5767,7 @@ void MusicXMLParserPass2::duration(Fraction& dura)
dura.set(0, 0); // invalid unless set correctly
const auto elementText = _e.readElementText();
if (elementText.toInt() > 0)
dura = calcTicks(elementText, _divs, _logger, &_e);
dura = _pass1.calcTicks(elementText.toInt(), &_e);
else
_logger->logError(QString("illegal duration %1").arg(dura.print()), &_e);
//qDebug("duration %s valid %d", qPrintable(dura.print()), dura.isValid());
Expand Down Expand Up @@ -6197,7 +6196,7 @@ void MusicXMLParserPass2::harmony(const QString& partId, Measure* measure, const
else if (_e.name() == "level")
skipLogCurrElem();
else if (_e.name() == "offset") {
offset = calcTicks(_e.readElementText(), _divs, _logger, &_e);
offset = _pass1.calcTicks(_e.readElementText().toInt(), &_e);
preventNegativeTick(sTime, offset, _logger);
}
else if (_e.name() == "staff") {
Expand Down
4 changes: 2 additions & 2 deletions importexport/musicxml/importmxmlpass2.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,15 +359,15 @@ class MusicXMLParserPass2 {
class MusicXMLParserDirection {
public:
MusicXMLParserDirection(QXmlStreamReader& e, Score* score, MusicXMLParserPass1& pass1, MusicXMLParserPass2& pass2, MxmlLogger* logger);
void direction(const QString& partId, Measure* measure, const Fraction& tick, const int divisions,
void direction(const QString& partId, Measure* measure, const Fraction& tick,
MusicXmlSpannerMap& spanners, DelayedDirectionsList& delayedDirections, InferredFingeringsList& inferredFingerings);
qreal totalY() const { return _defaultY + _relativeY; }
QString placement() const;

private:
QXmlStreamReader& _e;
Score* const _score; // the score
MusicXMLParserPass1& _pass1; // the pass1 results
MusicXMLParserPass1& _pass1; // the pass1 results
MusicXMLParserPass2& _pass2; // the pass2 results
MxmlLogger* _logger; ///< Error logger

Expand Down
Loading