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

MusicXML: add support for numerals #24174

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

MusicXML: add support for numerals #24174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d33d472
export and import numerals
rettinghaus Aug 23, 2024
9118ea1
update and add test
rettinghaus Aug 23, 2024
75c0c1d
simplify
rettinghaus Aug 23, 2024
f1f7227
switch to std::regex
rettinghaus Aug 24, 2024
f83d366
remove wrong imports
rettinghaus Aug 25, 2024
901a9b3
use string empty
rettinghaus Aug 25, 2024
a3ac0c5
fix strings
rettinghaus Aug 25, 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
66 changes: 53 additions & 13 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 @@ -5907,35 +5907,35 @@ static void directionJump(XmlWriter& xml, const Jump* const jp)
bool isDaCapo = false;
bool isDalSegno = false;
if (jtp == JumpType::DC) {
if (jp->xmlText() == "") {
if (jp->xmlText().empty()) {
words = u"D.C.";
} else {
words = jp->xmlText();
}
isDaCapo = true;
} else if (jtp == JumpType::DC_AL_FINE) {
if (jp->xmlText() == "") {
if (jp->xmlText().empty()) {
words = u"D.C. al Fine";
} else {
words = jp->xmlText();
}
isDaCapo = true;
} else if (jtp == JumpType::DC_AL_CODA) {
if (jp->xmlText() == "") {
if (jp->xmlText().empty()) {
words = u"D.C. al Coda";
} else {
words = jp->xmlText();
}
isDaCapo = true;
} else if (jtp == JumpType::DS_AL_CODA) {
if (jp->xmlText() == "") {
if (jp->xmlText().empty()) {
words = u"D.S. al Coda";
} else {
words = jp->xmlText();
}
isDalSegno = true;
} else if (jtp == JumpType::DS_AL_FINE) {
if (jp->xmlText() == "") {
if (jp->xmlText().empty()) {
words = u"D.S. al Fine";
} else {
words = jp->xmlText();
Expand All @@ -5957,7 +5957,7 @@ static void directionJump(XmlWriter& xml, const Jump* const jp)
if (isDaCapo) {
sound = u"dacapo=\"yes\"";
} else if (isDalSegno) {
if (jp->jumpTo() == "") {
if (jp->xmlText().empty()) {
sound = u"dalsegno=\"1\"";
} else {
sound = u"dalsegno=\"" + jp->jumpTo() + u"\"";
Expand Down Expand Up @@ -8596,7 +8596,7 @@ void ExportMusicXml::harmony(Harmony const* const h, FretDiagram const* const fd
if (!h->xmlKind().isEmpty()) {
String s = u"kind";
String kindText = h->musicXmlText();
if (h->musicXmlText() != u"") {
if (!h->musicXmlText().empty()) {
s += u" text=\"" + kindText + u"\"";
}
if (h->xmlSymbols() == u"yes") {
Expand Down Expand Up @@ -8694,16 +8694,56 @@ void ExportMusicXml::harmony(Harmony const* const h, FretDiagram const* const fd
const String textName = h->hTextName();
switch (h->harmonyType()) {
case HarmonyType::NASHVILLE: {
m_xml.tag("function", h->hFunction());
m_xml.tag("kind", { { "text", textName } }, "none");
String alter;
String functionText = h->hFunction();
if (functionText.empty()) {
// we just dump the text as deprecated function
m_xml.tag("function", textName);
m_xml.tag("kind", "none");
break;
} else if (!functionText.at(0).isDigit()) {
alter = functionText.at(0);
functionText = functionText.at(1);
}
m_xml.startElement("numeral");
m_xml.tag("numeral-root", functionText);
if (alter == u"b") {
m_xml.tag("numeral-alter", "-1");
} else if (alter == u"#") {
m_xml.tag("numeral-alter", "1");
}
m_xml.endElement();
if (!h->xmlKind().isEmpty()) {
String s = u"kind";
String kindText = h->musicXmlText();
if (!h->musicXmlText().empty()) {
s += u" text=\"" + kindText + u"\"";
}
if (h->xmlSymbols() == "yes") {
s += u" use-symbols=\"yes\"";
}
if (h->xmlParens() == "yes") {
s += u" parentheses-degrees=\"yes\"";
}
m_xml.tagRaw(s, h->xmlKind());
} else {
// default is major
m_xml.tag("kind", "major");
}
}
break;
case HarmonyType::ROMAN: {
// TODO: parse?
m_xml.tag("function", h->hTextName()); // note: HTML escape done by tag()
m_xml.tag("kind", { { "text", "" } }, "none");
static const std::regex roman("[iv]+|[IV]+");
if (std::regex_match(textName.toStdString(), roman)) {
m_xml.startElement("numeral");
m_xml.tag("numeral-root", { { "text", textName } }, "1");
m_xml.endElement();
// only check for major or minor
m_xml.tag("kind", textName.at(0).isUpper() ? "major" : "minor");
Copy link
Contributor

Choose a reason for hiding this comment

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

You could modify the code you've written below to read inversions to write them here.

break;
}
}
break;
// fallthrough
case HarmonyType::STANDARD:
default: {
m_xml.startElement("root");
Expand Down
45 changes: 39 additions & 6 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 @@ -7299,7 +7299,7 @@ void MusicXMLParserPass2::harmony(const String& partId, Measure* measure, const
const String placement = m_e.attribute("placement");
const bool printObject = m_e.asciiAttribute("print-object") != "no";

String kind, kindText, functionText, symbols, parens;
String kind, kindText, functionText, inversionText, symbols, parens;
std::list<HDegree> degreeList;

FretDiagram* fd = nullptr;
Expand Down Expand Up @@ -7342,8 +7342,33 @@ void MusicXMLParserPass2::harmony(const String& partId, Measure* measure, const
ha->setRootTpc(Tpc::TPC_INVALID);
ha->setBaseTpc(Tpc::TPC_INVALID);
functionText = m_e.readText();
// TODO: parse to decide between ROMAN and NASHVILLE
ha->setHarmonyType(HarmonyType::ROMAN);
} else if (m_e.name() == "numeral") {
ha->setRootTpc(Tpc::TPC_INVALID);
ha->setBaseTpc(Tpc::TPC_INVALID);
while (m_e.readNextStartElement()) {
if (m_e.name() == "numeral-root") {
String numeralRoot = m_e.readText();
String numeralRootText = m_e.attribute("text");
// TODO analyze text and import as roman numerals
ha->setHarmonyType(HarmonyType::NASHVILLE);
Copy link
Contributor

Choose a reason for hiding this comment

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

We appear to have just switched the problem around - previously nashville numbers were imported as roman numerals, now roman numerals are imported as nashville numbers. It would be great if we could recognise numerals here, as we've lost export/import consistency for numerals with this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is not entirely true, because the numeral element has been ignored until here, the old behavior for function hasn't changed. But I will add a basic check for Roman numerals in the text attribute.

ha->setFunction(numeralRoot);
} else if (m_e.name() == "numeral-alter") {
const int alter = m_e.readText().toInt();
switch (alter) {
case -1:
ha->setFunction(u"b" + ha->hFunction());
break;
case 1:
ha->setFunction(u"#" + ha->hFunction());
break;
default:
break;
}
} else {
skipLogCurrElem();
}
}
} else if (m_e.name() == "kind") {
// attributes: use-symbols yes-no
// text, stack-degrees, parentheses-degree, bracket-degrees,
Expand All @@ -7356,8 +7381,16 @@ void MusicXMLParserPass2::harmony(const String& partId, Measure* measure, const
ha->setRootTpc(Tpc::TPC_INVALID);
}
} else if (m_e.name() == "inversion") {
// attributes: print-style
skipLogCurrElem();
const int inversion = m_e.readText().toInt();
switch (inversion) {
case 1: inversionText = u"6";
break;
case 2: inversionText = u"64";
break;
default:
inversionText = u"";
break;
}
} else if (m_e.name() == "bass") {
String step;
int alter = 0;
Expand Down Expand Up @@ -7425,15 +7458,15 @@ void MusicXMLParserPass2::harmony(const String& partId, Measure* measure, const
}

const ChordDescription* d = nullptr;
if (ha->rootTpc() != Tpc::TPC_INVALID) {
if (ha->rootTpc() != Tpc::TPC_INVALID || !ha->hFunction().empty()) {
d = ha->fromXml(kind, kindText, symbols, parens, degreeList);
}
if (d) {
ha->setId(d->id);
ha->setTextName(d->names.front());
} else {
ha->setId(-1);
String textName = functionText + kindText;
String textName = functionText + kindText + inversionText;
ha->setTextName(textName);
}
ha->render();
Expand Down
32 changes: 20 additions & 12 deletions src/importexport/musicxml/tests/data/testHarmony6_ref.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,10 @@ containing recognized text, unrecognized plaintext and unrecognized text requiri
</measure>
<measure number="2">
<harmony print-frame="no">
<function>i</function>
<kind text="">none</kind>
<numeral>
<numeral-root text="i">1</numeral-root>
</numeral>
<kind>minor</kind>
</harmony>
<note>
<pitch>
Expand All @@ -93,8 +95,10 @@ containing recognized text, unrecognized plaintext and unrecognized text requiri
</measure>
<measure number="3">
<harmony print-frame="no">
<function>1</function>
<kind text="m">none</kind>
<numeral>
<numeral-root>1</numeral-root>
</numeral>
<kind text="m">minor</kind>
</harmony>
<note>
<pitch>
Expand Down Expand Up @@ -134,8 +138,10 @@ containing recognized text, unrecognized plaintext and unrecognized text requiri
</measure>
<measure number="5">
<harmony print-frame="no">
<function>xyz</function>
<kind text="">none</kind>
<root>
<root-step text="">C</root-step>
</root>
<kind text="xyz">none</kind>
</harmony>
<note>
<pitch>
Expand All @@ -153,8 +159,8 @@ containing recognized text, unrecognized plaintext and unrecognized text requiri
</measure>
<measure number="6">
<harmony print-frame="no">
<function></function>
<kind text="xyz">none</kind>
<function>xyz</function>
<kind>none</kind>
</harmony>
<note>
<pitch>
Expand Down Expand Up @@ -194,8 +200,10 @@ containing recognized text, unrecognized plaintext and unrecognized text requiri
</measure>
<measure number="8">
<harmony print-frame="no">
<function>&lt;&gt;&amp;&quot;</function>
<kind text="">none</kind>
<root>
<root-step text="">C</root-step>
</root>
<kind text="&lt;&gt;&amp;&quot;">none</kind>
</harmony>
<note>
<pitch>
Expand All @@ -213,8 +221,8 @@ containing recognized text, unrecognized plaintext and unrecognized text requiri
</measure>
<measure number="9">
<harmony print-frame="no">
<function></function>
<kind text="&lt;&gt;&amp;&quot;">none</kind>
<function>&lt;&gt;&amp;&quot;</function>
<kind>none</kind>
</harmony>
<note>
<pitch>
Expand Down
Loading