Skip to content
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

Jump to bottom

Fix #16227 - MusicXML export is incorrect when using local timsig #23255

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix #16227 - MusicXML export is incorrect when using local timsig #23255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
155 changes: 107 additions & 48 deletions src/importexport/musicxml/internal/musicxml/exportxml.cpp
View file
Open in desktop
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could call break; here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you could. It wouldn't matter much, as that would only affect parts with more than two staves.

}
}
}

// 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
View file
Open in desktop
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
Loading