Skip to content

Commit

Permalink
Fix #16227 - MusicXML export is incorrect when using local time signa…
Browse files Browse the repository at this point in the history
…ture

- export all time signatures for multi-staff parts
- when streched correct duration of:
  - normal rest
  - measure rest
  - backup
Note that import also requires fixes
  • Loading branch information
lvinken committed Jun 15, 2024
1 parent 06c4973 commit 7fda26e
Show file tree
Hide file tree
Showing 32 changed files with 4,753 additions and 51 deletions.
155 changes: 107 additions & 48 deletions src/importexport/musicxml/internal/musicxml/exportxml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ class ExportMusicXml : public muse::Injectable

void write(muse::io::IODevice* dev);
void credits(XmlWriter& xml);
void moveToTick(const Fraction& t);
void moveToTick(const Fraction& t, const Fraction& stretch = {1, 1});
void words(TextBase const* const text, staff_idx_t staff);
void tboxTextAsWords(TextBase const* const text, const staff_idx_t staff, PointF position);
void rehearsal(RehearsalMark const* const rmk, staff_idx_t staff);
Expand Down Expand Up @@ -408,14 +408,14 @@ class ExportMusicXml : public muse::Injectable
void chord(Chord* chord, staff_idx_t staff, const std::vector<Lyrics*>& ll, bool useDrumset);
void rest(Rest* chord, staff_idx_t staff, const std::vector<Lyrics*>& ll);
void clef(staff_idx_t staff, const ClefType ct, const String& extraAttributes = u"");
void timesig(TimeSig* tsig);
void timesig(TimeSig* tsig, staff_idx_t staff);
void keysig(const KeySig* ks, ClefType ct, staff_idx_t staff = 0, bool visible = true);
void barlineLeft(const Measure* const m, const track_idx_t track);
void barlineMiddle(const BarLine* bl);
void barlineRight(const Measure* const m, const track_idx_t strack, const track_idx_t etrack);
void lyrics(const std::vector<Lyrics*>& ll, const track_idx_t trk);
void work(const MeasureBase* measure);
void calcDivMoveToTick(const Fraction& t);
void calcDivMoveToTick(const Fraction& t, const Fraction& stretch = {1, 1});
void calcDivisions();
void keysigTimesig(const Measure* m, const Part* p);
void chordAttributes(Chord* chord, Notations& notations, Technical& technical, TrillHash& trillStart, TrillHash& trillStop);
Expand Down Expand Up @@ -1183,18 +1183,23 @@ static void addFraction(const Fraction& len)
// calcDivMoveToTick
//---------------------------------------------------------

void ExportMusicXml::calcDivMoveToTick(const Fraction& t)
void ExportMusicXml::calcDivMoveToTick(const Fraction& t, const Fraction& stretch)
{
Fraction stretched_tick { t + stretch * (tick() - t) };
#ifdef DEBUG_TICK
LOGD() << "t (target) " << fractionToStdString(t) << " stretch " << fractionToStdString(stretch)
<< " m_tick (current) " << fractionToStdString(m_tick) << " stretchedM_tick " << fractionToStdString(stretched_tick);
#endif
if (t < tick()) {
#ifdef DEBUG_TICK
LOGD() << "backup " << fractionToStdString(tick() - t);
#endif
addFraction(tick() - t);
addFraction(stretched_tick - t);
} else if (t > tick()) {
#ifdef DEBUG_TICK
LOGD() << "forward " << fractionToStdString(t - tick());
#endif
addFraction(t - tick());
addFraction(t - stretched_tick);
}
tick() = t;
}
Expand All @@ -1208,11 +1213,26 @@ static bool isTwoNoteTremolo(Chord* chord)
return chord->tremoloTwoChord();
}

//---------------------------------------------------------
// stretch
//---------------------------------------------------------

static Fraction stretch(Score* score, track_idx_t st, Fraction tick)
{
Staff* staff { score->staff(track2staff(st)) };
Fraction res { staff->timeStretch(tick) };
#ifdef DEBUG_TICK
LOGD() << "track " << st << " tick " << fractionToStdString(tick) << " stretch " << fractionToStdString(res);
#endif
return res;
}

//---------------------------------------------------------
// calcDivisions
//---------------------------------------------------------

// Loop over all voices in all staves and determine a suitable value for divisions.
// All parts are taken into account, a global divisions value results.

// Length of time in MusicXML is expressed in "units", which should allow expressing all time values
// as an integral number of units. Divisions contains the number of units in a quarter note.
Expand Down Expand Up @@ -1246,9 +1266,10 @@ void ExportMusicXml::calcDivisions()
if (e->track() == st && e->type() == ElementType::FIGURED_BASS) {
const FiguredBass* fb = toFiguredBass(e);
#ifdef DEBUG_TICK
LOGD("figuredbass tick %d duration %d", fb->tick().ticks(), fb->ticks().ticks());
LOGD() << "figuredbass tick " << fractionToStdString(fb->tick())
<< " tickLen " << fractionToStdString(fb->ticks());
#endif
addFraction(fb->ticks());
addFraction(stretch(m_score, st, m->tick()) * fb->ticks());
}
}

Expand All @@ -1263,7 +1284,7 @@ void ExportMusicXml::calcDivisions()
}

if (m_tick != seg->tick()) {
calcDivMoveToTick(seg->tick());
calcDivMoveToTick(seg->tick(), stretch(m_score, st, m->tick()));
}

if (el->isChordRest()) {
Expand All @@ -1277,13 +1298,13 @@ void ExportMusicXml::calcDivisions()
LOGD() << "chordrest tick " << fractionToStdString(el->tick())
<< " tickLen" << durElemTicksToStdString(*toChordRest(el));
#endif
addFraction(l);
addFraction(stretch(m_score, st, m->tick()) * l);
m_tick += l;
}
}
}
// move to end of measure (in case of incomplete last voice)
calcDivMoveToTick(m->endTick());
calcDivMoveToTick(m->endTick(), stretch(m_score, etrack - 1, m->tick()));
}
}

Expand Down Expand Up @@ -2143,24 +2164,28 @@ static int calculateDurationInDivisions(const Fraction& tick, const int division
// moveToTick
//---------------------------------------------------------

void ExportMusicXml::moveToTick(const Fraction& t)
void ExportMusicXml::moveToTick(const Fraction& t, const Fraction& stretch)
{
//LOGD("ExportMusicXml::moveToTick(t=%s) _tick=%s", muPrintable(t.print()), muPrintable(_tick.print()));
Fraction stretchedM_tick { t + stretch * (tick() - t) };
#ifdef DEBUG_TICK
LOGD() << "t (target) " << fractionToStdString(t) << " stretch " << fractionToStdString(stretch)
<< " tick() (current) " << fractionToStdString(m_tick) << " stretchedM_tick " << fractionToStdString(stretchedM_tick);
#endif
if (t < m_tick) {
#ifdef DEBUG_TICK
LOGD(" -> backup");
#endif
m_attr.doAttr(m_xml, false);
m_xml.startElement("backup");
m_xml.tag("duration", calculateTimeDeltaInDivisions(m_tick, t, m_div));
m_xml.tag("duration", calculateTimeDeltaInDivisions(stretchedM_tick, t, m_div));
m_xml.endElement();
} else if (t > m_tick) {
#ifdef DEBUG_TICK
LOGD(" -> forward");
#endif
m_attr.doAttr(m_xml, false);
m_xml.startElement("forward");
m_xml.tag("duration", calculateTimeDeltaInDivisions(t, m_tick, m_div));
m_xml.tag("duration", calculateTimeDeltaInDivisions(t, stretchedM_tick, m_div));
m_xml.endElement();
}
m_tick = t;
Expand All @@ -2170,7 +2195,7 @@ void ExportMusicXml::moveToTick(const Fraction& t)
// timesig
//---------------------------------------------------------

void ExportMusicXml::timesig(TimeSig* tsig)
void ExportMusicXml::timesig(TimeSig* tsig, staff_idx_t staff)
{
TimeSigType st = tsig->timeSigType();
Fraction ts = tsig->sig();
Expand All @@ -2185,6 +2210,9 @@ void ExportMusicXml::timesig(TimeSig* tsig)
} else if (st == TimeSigType::ALLA_BREVE) {
attrs = { { "symbol", "cut" } };
}
if (staff) {
attrs.push_back({ "number", staff });
}
if (!tsig->visible()) {
attrs.push_back({ "print-object", "no" });
}
Expand Down Expand Up @@ -4000,18 +4028,6 @@ static void writeNotationSymbols(XmlWriter& xml, Notations& notations, const Ele
}
}

//---------------------------------------------------------
// stretchCorrActFraction
//---------------------------------------------------------

static Fraction stretchCorrActFraction(const Note* const note)
{
// time signature stretch factor
const Fraction str = note->chord()->staff()->timeStretch(note->chord()->tick());
// chord's actual ticks corrected for stretch
return note->chord()->actualTicks() * str;
}

//---------------------------------------------------------
// tremoloCorrection
//---------------------------------------------------------
Expand Down Expand Up @@ -4074,7 +4090,7 @@ static void writeType(XmlWriter& xml, const Note* const note)
int dots = 0;
const Fraction ratio = timeModification(note->chord()->tuplet());

const Fraction strActFraction = stretchCorrActFraction(note);
const Fraction strActFraction = note->chord()->globalTicks();
const Fraction tt = strActFraction * ratio * tremoloCorrection(note);
const String s = tick2xml(tt, &dots);
if (s.isEmpty()) {
Expand Down Expand Up @@ -4267,7 +4283,7 @@ void ExportMusicXml::chord(Chord* chord, staff_idx_t staff, const std::vector<Ly

// duration
if (!grace) {
m_xml.tag("duration", calculateDurationInDivisions(stretchCorrActFraction(note), m_div));
m_xml.tag("duration", calculateDurationInDivisions(chord->globalTicks(), m_div));
}

if (!isCueNote(note)) {
Expand Down Expand Up @@ -4490,12 +4506,10 @@ void ExportMusicXml::rest(Rest* rest, staff_idx_t staff, const std::vector<Lyric
m_xml.endElement();
}

Fraction tickLen = rest->actualTicks();
if (d.type() == DurationType::V_MEASURE) {
// to avoid forward since rest->ticklen=0 in this case.
tickLen = rest->measure()->ticks();
}
m_tick += tickLen;
Fraction tickLen;
// regular rest
tickLen = rest->globalTicks(); // MusicXML requires unstretched duration
m_tick += rest->actualTicks(); // prevent <backward> or <forward> by moving to next note's tick
#ifdef DEBUG_TICK
LOGD() << "tickLen " << fractionToStdString(tickLen)
<< "newtick " << fractionToStdString(tick());
Expand Down Expand Up @@ -6793,7 +6807,7 @@ void ExportMusicXml::keysigTimesig(const Measure* m, const Part* p)
{
track_idx_t strack = p->startTrack();
track_idx_t etrack = p->endTrack();
//LOGD("keysigTimesig m %p strack %d etrack %d", m, strack, etrack);
//LOGD("keysigTimesig m %p strack %zu etrack %zu", m, strack, etrack);

// search all staves for non-generated key signatures
std::map<staff_idx_t, KeySig*> keysigs; // map staff to key signature
Expand Down Expand Up @@ -6862,18 +6876,59 @@ void ExportMusicXml::keysigTimesig(const Measure* m, const Part* p)
}
}

TimeSig* tsig = 0;
// search all staves for non-generated time signatures
std::map<staff_idx_t, TimeSig*> timesigs; // map staff to time signature
for (Segment* seg = m->first(); seg; seg = seg->next()) {
if (seg->tick() > m->tick()) {
break;
}
EngravingItem* el = seg->element(strack);
if (el && el->type() == ElementType::TIMESIG) {
tsig = (TimeSig*)el;
for (track_idx_t t = strack; t < etrack; t += VOICES) {
EngravingItem* el = seg->element(t);
if (!el) {
continue;
}
if (el->type() == ElementType::TIMESIG) {
LOGN(" found timesig %p tick %d track %zu", el, el->tick().ticks(), el->track());
staff_idx_t st = (t - strack) / VOICES;
if (!el->generated()) {
timesigs[st] = toTimeSig(el);
}
}
}
}
if (tsig) {
timesig(tsig);

// write the time signatues
if (!timesigs.empty()) {
// determine if all staves have a timesig and all timesigs are identical
// in that case a single <time> is written, without number=... attribute
size_t nstaves = p->nstaves();
bool singleTime = true;
// check if all staves have a keysig
for (staff_idx_t i = 0; i < nstaves; i++) {
if (!muse::contains(timesigs, i)) {
singleTime = false;
}
}
// check if all timesigs are identical
if (singleTime) {
for (staff_idx_t i = 1; i < nstaves; i++) {
if (!(timesigs.at(i)->sig() == timesigs.at(0)->sig())) {
singleTime = false;
}
}
}

// write the timesigs
LOGN(" singleTime %d", singleTime);
if (singleTime) {
// timesig applies to all staves
timesig(timesigs.at(0), 0);
} else {
// staff-specific timesig
for (staff_idx_t st : muse::keys(timesigs)) {
timesig(timesigs.at(st), st + 1);
}
}
}
}

Expand Down Expand Up @@ -7933,7 +7988,7 @@ void ExportMusicXml::writeMeasureTracks(const Measure* const m,
// generate backup or forward to the start time of the element
if (m_tick != seg->tick()) {
m_attr.doAttr(m_xml, false);
moveToTick(seg->tick());
moveToTick(seg->tick(), stretch(m_score, track, m->tick()));
}

// handle annotations and spanners (directions attached to this note or rest)
Expand Down Expand Up @@ -8024,7 +8079,7 @@ void ExportMusicXml::writeMeasureStaves(const Measure* m,
IF_ASSERT_FAILED(m == origM) {
return;
}
moveToTick(m->tick());
moveToTick(m->tick()); // move tick to start of measure

staff_idx_t partRelStaffNo = (nstaves > 1 ? staffIdx - startStaff + 1 : 0); // xml staff number, counting from 1 for this instrument
// special number 0 -> don’t show staff number in xml output
Expand Down Expand Up @@ -8056,6 +8111,10 @@ void ExportMusicXml::writeMeasureStaves(const Measure* m,
// restore m and _tick before advancing to next staff in part
m = origM;
m_tick += tickDelta;
if (!isLastStaffOfPart) {
// move tick to start of measure, generates backup to next staff, needs this staff's stretch
moveToTick(m->tick(), stretch(m_score, staffIdx * VOICES, m->tick()));
}
}
}

Expand Down Expand Up @@ -8149,10 +8208,10 @@ void ExportMusicXml::writeMeasure(const Measure* const m,
annotationsWithoutNote(this, strack, static_cast<int>(staves), m);

// move to end of measure (in case of incomplete last voice)
#ifdef DEBUG_TICK
#ifdef DEBUG_TICK
LOGD("end of measure");
#endif
moveToTick(m->endTick());
#endif
moveToTick(m->endTick(), stretch(m_score, etrack - 1, m->tick()));
if (partIndex == 0) {
repeatAtMeasureStop(m, strack, etrack, strack);
}
Expand Down
8 changes: 5 additions & 3 deletions src/importexport/musicxml/tests/data/iotest.mscxtomxml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/bash

# simple standalone iotest for MusicXML import
# run in mtest/musicxml/io directory as "./iotest.mscxtomxml"
# run in src/importexport/musicxml/tests/data directory as "./iotest.mscxtomxml"

# Linux
#MSCORE=../../../build.debug/mscore/mscore
Expand All @@ -10,7 +10,9 @@
# OS X Xcode build
#MSCORE=../../../build.xcode/mscore/Debug/mscore.app/Contents/MacOS/mscore
# OS X Xcode build (since early 2020)
MSCORE=../../../build.xcode/main/Debug/mscore.app/Contents/MacOS/mscore
#MSCORE=../../../build.xcode/main/Debug/mscore.app/Contents/MacOS/mscore
# OS X Qt Creator build (since early 2024)
MSCORE=../../../../../builds/Mac-Qtlibexec-qt5-Make-RelWithDebInfo/install/mscore.app/Contents/MacOS/mscore

echo "----------------------------------------------"
echo "Regression Tests for MuseScore MusicXML import"
Expand All @@ -19,7 +21,7 @@ echo
$MSCORE -v
echo

musicxml_files=`cat tst_mxml_io.cpp | grep "{ mxmlMscxExportTestRef" | awk -F\" '{print $2}' | sort`
musicxml_files=`cat ../musicxml_tests.cpp | grep mxmlMscxExportTestRef | awk -F\" '{print $2}' | sort`
testcount=0
failures=0

Expand Down
Loading

0 comments on commit 7fda26e

Please sign in to comment.