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

XML ottava fixes #24776

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
+502 −108
Open

XML ottava fixes #24776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0e28d03
Add exporter software enum
miiizen Sep 16, 2024
2cc0b97
Correct placement of ottavas below the stave
miiizen Sep 16, 2024
884525b
Correct dolet and finale ottavas
miiizen Sep 17, 2024
43036c3
Separate dolet and sibelius ottava tests
miiizen Sep 17, 2024
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
37 changes: 33 additions & 4 deletions src/importexport/musicxml/internal/musicxml/importmxmlpass1.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,23 @@ void MusicXMLParserPass1::addError(const String& error)
}
}

void MusicXMLParserPass1::setExporterSoftware(String& exporter)
{
if (exporter.contains(u"sibelius")) {
if (exporter.contains(u"dolet 6")) {
m_exporterSoftware = MusicXMLExporterSoftware::DOLET6;
} else if (exporter.contains(u"dolet 8")) {
m_exporterSoftware = MusicXMLExporterSoftware::DOLET8;
} else {
m_exporterSoftware = MusicXMLExporterSoftware::SIBELIUS;
}
} else if (exporter.contains(u"finale")) {
m_exporterSoftware = MusicXMLExporterSoftware::FINALE;
} else if (exporter.contains(u"noteflight")) {
m_exporterSoftware = MusicXMLExporterSoftware::NOTEFLIGHT;
}
}

//---------------------------------------------------------
// initPartState
//---------------------------------------------------------
Expand Down Expand Up @@ -951,7 +968,7 @@ void MusicXMLParserPass1::createMeasuresAndVboxes(Score* score,
++pageNr;

if (pageNr == 1) {
vbox = addCreditWords(score, crWords, pageSize, true, m_exporterString.contains(u"sibelius"));
vbox = addCreditWords(score, crWords, pageSize, true, sibOrDolet());
if (i == 0 && vbox) {
vbox->setExcludeFromOtherParts(false);
}
Expand All @@ -978,11 +995,22 @@ void MusicXMLParserPass1::createMeasuresAndVboxes(Score* score,

// add a footer vbox if the next measure is on a new page or end of score has been reached
if ((pageStartMeasureNrs.count(int(i + 1)) || i == (ml.size() - 1)) && pageNr == 1) {
addCreditWords(score, crWords, pageSize, false, m_exporterString.contains(u"sibelius"));
addCreditWords(score, crWords, pageSize, false, sibOrDolet());
}
}
}

bool MusicXMLParserPass1::sibOrDolet() const
{
return m_exporterSoftware == MusicXMLExporterSoftware::SIBELIUS || m_exporterSoftware == MusicXMLExporterSoftware::DOLET6
|| m_exporterSoftware == MusicXMLExporterSoftware::DOLET8;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not reuse dolet() here?

}

bool MusicXMLParserPass1::dolet() const
{
return m_exporterSoftware == MusicXMLExporterSoftware::DOLET6 || m_exporterSoftware == MusicXMLExporterSoftware::DOLET8;
}

//---------------------------------------------------------
// determineMeasureStart
//---------------------------------------------------------
Expand Down Expand Up @@ -1272,7 +1300,8 @@ void MusicXMLParserPass1::identification()
// TODO
while (m_e.readNextStartElement()) {
if (m_e.name() == "software") {
m_exporterString += m_e.readText().toLower();
String exporterString = m_e.readText().toLower();
setExporterSoftware(exporterString);
} else if (m_e.name() == "supports" && m_e.asciiAttribute("element") == "beam" && m_e.asciiAttribute("type") == "yes") {
m_hasBeamingInfo = true;
m_e.skipCurrentElement();
Expand Down Expand Up @@ -2248,7 +2277,7 @@ void MusicXMLParserPass1::scoreInstrument(const String& partId, const String& cu
*/

// Finale exports all instrument names as 'Grand Piano' - use part name
if (m_exporterString.contains(u"finale")) {
if (exporterSoftware() == MusicXMLExporterSoftware::FINALE) {
instrName = m_parts[partId].getName();
if (instrName.size() <= 1) {
instrName = curPartGroupName;
Expand Down
22 changes: 17 additions & 5 deletions src/importexport/musicxml/internal/musicxml/importmxmlpass1.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,15 @@ enum class MxmlTupletFlag : char {
STOP_CURRENT = 8
};

enum class MusicXMLExporterSoftware : char {
SIBELIUS,
DOLET6,
DOLET8,
FINALE,
NOTEFLIGHT,
OTHER
};

typedef muse::Flags<MxmlTupletFlag> MxmlTupletFlags;

struct MxmlTupletState {
Expand Down Expand Up @@ -195,21 +204,24 @@ class MusicXMLParserPass1
void insertAdjustedDuration(Fraction key, Fraction value) { m_adjustedDurations.insert({ key, value }); }
std::map<Fraction, Fraction>& adjustedDurations() { return m_adjustedDurations; }
void insertSeenDenominator(int val) { m_seenDenominators.emplace(val); }
String exporterString() const { return m_exporterString; }
MusicXMLExporterSoftware exporterSoftware() const { return m_exporterSoftware; }
bool sibOrDolet() const;
bool dolet() const;

private:
// functions
void addError(const String& error); // Add an error to be shown in the GUI
void setExporterSoftware(String& exporter);

// generic pass 1 data
muse::XmlStreamReader m_e;
String m_exporterString; // Name of the software which exported the file
MusicXMLExporterSoftware m_exporterSoftware = MusicXMLExporterSoftware::OTHER; // Software which exported the file
int m_divs = 0; // Current MusicXML divisions value
std::map<String, MusicXmlPart> m_parts; // Parts data, mapped on part id
std::map<String, MusicXmlPart> m_parts; // Parts data, mapped on part id
std::set<int> m_systemStartMeasureNrs; // Measure numbers of measures starting a page
std::set<int> m_pageStartMeasureNrs; // Measure numbers of measures starting a page
std::vector<Fraction> m_measureLength; // Length of each measure
std::vector<Fraction> m_measureStart; // Start time of each measure
std::vector<Fraction> m_measureLength; // Length of each measure
std::vector<Fraction> m_measureStart; // Start time of each measure
CreditWordsList m_credits; // All credits collected
PartMap m_partMap; // TODO merge into MusicXmlPart ??
std::map<String, MusicXMLInstruments> m_instruments; // instruments for each part, mapped on part id
Expand Down
58 changes: 34 additions & 24 deletions src/importexport/musicxml/internal/musicxml/importmxmlpass2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2196,10 +2196,11 @@ void MusicXMLParserPass2::part()
// Clean up unterminated ties
for (auto tie : m_ties) {
if (tie.second) {
cleanupUnterminatedTie(tie.second, m_score, m_pass1.exporterString().contains(u"dolet 6"));
cleanupUnterminatedTie(tie.second, m_score, m_pass1.exporterSoftware() == MusicXMLExporterSoftware::DOLET6);
m_ties[tie.first] = nullptr;
}
}

m_ties.clear();

if (m_hasDrumset) {
Expand Down Expand Up @@ -3213,7 +3214,7 @@ void MusicXMLParserDirection::direction(const String& partId,
bool isVocalStaff = m_pass1.isVocalStaff(partId);
bool isPercussionStaff = m_pass1.isPercussionStaff(partId);
bool isExpressionText = false;
bool delayOttava = m_pass1.exporterString().contains(u"sibelius");
bool delayOttava = m_pass1.exporterSoftware() == MusicXMLExporterSoftware::SIBELIUS;
m_systemDirection = m_e.attribute("system") == "only-top";
//LOGD("direction track %d", track);
std::vector<MusicXmlSpannerDesc> starts;
Expand Down Expand Up @@ -3520,19 +3521,23 @@ void MusicXMLParserDirection::direction(const String& partId,
delete desc.sp;
} else {
if (spdesc.isStarted) {
// Adjustments to ottavas by the offset value are unwanted
const Fraction spTick = spdesc.sp && spdesc.sp->isOttava() ? tick : tick + m_offset;
if (spdesc.sp && spdesc.sp->isOttava() && delayOttava) {
// Sibelius writes ottava ends 1 note too early
m_pass2.setDelayedOttava(spdesc.sp);
m_pass2.delayedOttava()->setTrack2(m_track);
m_pass2.delayedOttava()->setTick2(tick + m_offset);
m_pass2.delayedOttava()->setTick2(spTick);
// need to set tick again later
m_pass2.clearSpanner(desc);
} else {
handleSpannerStop(spdesc.sp, m_track, tick + m_offset, spanners);
handleSpannerStop(spdesc.sp, m_track, spTick, spanners);
m_pass2.clearSpanner(desc);
}
} else {
spdesc.sp = desc.sp;
spdesc.tick2 = tick + m_offset;
const Fraction spTick = spdesc.sp && spdesc.sp->isOttava() ? tick : tick + m_offset;
spdesc.tick2 = spTick;
spdesc.track2 = m_track;
spdesc.isStopped = true;
}
Expand Down Expand Up @@ -3876,7 +3881,7 @@ void MusicXMLParserDirection::otherDirection()
// TODO: Multiple sets of maps for exporters other than Dolet 6/Sibelius
// TODO: Add more symbols from Sibelius
std::map<String, String> otherDirectionStrings;
if (m_pass1.exporterString().contains(u"dolet")) {
if (m_pass1.dolet()) {
otherDirectionStrings = {
{ String(u"To Coda"), String(u"To Coda") },
{ String(u"Segno"), String(u"<sym>segno</sym>") },
Expand Down Expand Up @@ -4114,7 +4119,7 @@ void MusicXMLParserDirection::textToDynamic(String& text)
}
String simplifiedText = MScoreTextToMXML::toPlainText(text).simplified();
// Correct finale's incorrect dynamic export
if (m_pass1.exporterString().contains(u"finale")) {
if (m_pass1.exporterSoftware() == MusicXMLExporterSoftware::FINALE) {
static const std::map<String,
String> finaleDynamicSubs
= { { u"π", u"pp" }, { u"P", u"mp" }, { u"F", u"mf" }, { u"ƒ", u"ff" }, { u"Ï", u"fff" }, { u"S", u"sf" }, { u"ß", u"sfz" },
Expand Down Expand Up @@ -4515,7 +4520,7 @@ void MusicXMLParserDirection::handleTempo(String& wordsString)
};

MetronomeTextMap textMap;
if (m_pass1.exporterString().contains(u"sibelius")) {
if (m_pass1.sibOrDolet()) {
textMap = sibeliusSyms;
} else if (m_fontFamily == u"MetTimes Plain") {
textMap = metTimesSyms;
Expand Down Expand Up @@ -4822,19 +4827,22 @@ void MusicXMLParserDirection::octaveShift(const String& type, const int number,
} else {
Ottava* o = spdesc.isStopped ? toOttava(spdesc.sp) : Factory::createOttava(m_score->dummy());

// if (placement.empty()) placement = "above"; // TODO ? set default

if (type == u"down" && ottavasize == 8) {
o->setOttavaType(OttavaType::OTTAVA_8VA);
}
if (type == u"down" && ottavasize == 15) {
o->setOttavaType(OttavaType::OTTAVA_15MA);
}
if (type == u"up" && ottavasize == 8) {
o->setOttavaType(OttavaType::OTTAVA_8VB);
if (type == u"down") {
m_placement = m_placement.empty() ? u"above" : m_placement;
if (ottavasize == 8) {
o->setOttavaType(OttavaType::OTTAVA_8VA);
} else if (ottavasize == 15) {
o->setOttavaType(OttavaType::OTTAVA_15MA);
}
}
if (type == u"up" && ottavasize == 15) {
o->setOttavaType(OttavaType::OTTAVA_15MB);

if (type == u"up") {
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn#t this rather be else (we don't get here otherwise) or at least else if?

m_placement = m_placement.empty() ? u"below" : m_placement;
if (ottavasize == 8) {
o->setOttavaType(OttavaType::OTTAVA_8VB);
} else if (ottavasize == 15) {
o->setOttavaType(OttavaType::OTTAVA_15MB);
}
}

const Color color = Color::fromString(m_e.asciiAttribute("color").ascii());
Expand Down Expand Up @@ -6550,7 +6558,7 @@ Note* MusicXMLParserPass2::note(const String& partId,
String noteheadValue = m_e.readText();
if (noteheadValue == "none") {
hasHead = false;
} else if (noteheadValue == "named" && m_pass1.exporterString().contains(u"noteflight")) {
} else if (noteheadValue == "named" && m_pass1.exporterSoftware() == MusicXMLExporterSoftware::NOTEFLIGHT) {
headScheme = NoteHeadScheme::HEAD_PITCHNAME;
} else {
headGroup = convertNotehead(noteheadValue);
Expand Down Expand Up @@ -6847,7 +6855,8 @@ Note* MusicXMLParserPass2::note(const String& partId,
Notation notation = Notation(u"tied");
const String type2 = u"type";
notation.addAttribute(type2, tieType);
addTie(notation, m_score, note, cr->track(), m_ties, m_logger, &m_e, m_pass1.exporterString().contains(u"dolet 6"));
addTie(notation, m_score, note, cr->track(), m_ties, m_logger, &m_e,
m_pass1.exporterSoftware() == MusicXMLExporterSoftware::DOLET6);
}
}

Expand Down Expand Up @@ -7449,7 +7458,7 @@ void MusicXMLParserPass2::harmony(const String& partId, Measure* measure, const

const HarmonyDesc newHarmonyDesc(track, ha, fd);
bool insert = true;
if (m_pass1.exporterString().contains(u"dolet")) {
if (m_pass1.sibOrDolet()) {
const int ticks = (sTime + offset).ticks();
for (auto itr = harmonyMap.begin(); itr != harmonyMap.end(); itr++) {
if (itr->first != ticks) {
Expand Down Expand Up @@ -8689,7 +8698,8 @@ void MusicXMLParserNotations::addToScore(ChordRest* const cr, Note* const note,
} else if (note && (notation.name() == "glissando" || notation.name() == "slide")) {
addGlissandoSlide(notation, note, glissandi, spanners, m_logger, &m_e);
} else if (note && notation.name() == "tied") {
addTie(notation, m_score, note, cr->track(), ties, m_logger, &m_e, m_pass1.exporterString().contains(u"dolet 6"));
addTie(notation, m_score, note, cr->track(), ties, m_logger, &m_e,
m_pass1.exporterSoftware() == MusicXMLExporterSoftware::DOLET6);
} else if (note && notation.parent() == "technical") {
addTechnical(notation, note);
} else {
Expand Down
Binary file added src/importexport/musicxml/tests/data/testDoletOttavas.xml
View file
Open in desktop
Binary file not shown.
Loading