Skip to content

XML fixes #29197

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
miiizen merged 10 commits into from
Aug 18, 2025
Merged

XML fixes #29197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
39a5c13
Only apply word-font size when config option is off
miiizen Aug 12, 2025
a5587d1
Remove duplicate hbox at codas
miiizen Aug 12, 2025
d0fed87
Resize title box to avoid collisions
miiizen Aug 12, 2025
a2bf9db
Fix import of staff scaling & line distance
miiizen Aug 12, 2025
35adb8e
Don't apply offsets to credit text
miiizen Aug 12, 2025
7abc28b
Ignore "pedal" chords when importing xml
miiizen Aug 12, 2025
ad751af
Fix scaling copyright text
miiizen Aug 12, 2025
4b45e5f
Fix tests
miiizen Aug 12, 2025
3724a8a
Update staff-size in xml export
miiizen Aug 18, 2025
0a3266b
Don't apply vbox adjustment when testing
miiizen Aug 18, 2025
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
4 changes: 4 additions & 0 deletions src/engraving/dom/chordlist.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1386,6 +1386,10 @@ String ParsedChord::fromXml(const String& rawKind, const String& rawKindText, co
m_quality = u"major";
implied = true;
extension = 5;
} else if (kind == "pedal") {
// Ignore, assume major
m_quality = u"major";
implied = true;
} else {
m_quality = kind;
}
Expand Down
35 changes: 34 additions & 1 deletion src/engraving/tests/utils/scorecomp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,14 @@ bool ScoreComp::saveCompareScore(Score* score, const String& saveName, const Str
if (!ScoreRW::saveScore(score, saveName)) {
return false;
}
bool val = compareFiles(ScoreRW::rootPath() + u"/" + compareWithLocalPath, saveName);

return compareFiles(ScoreRW::rootPath() + u"/" + compareWithLocalPath, saveName);
if (!val) {
copyFile(saveName, ScoreRW::rootPath() + u"/" + compareWithLocalPath);
return false;
}

return val;
}

bool ScoreComp::saveCompareMimeData(muse::ByteArray mimeData, const muse::String& saveName, const muse::String& compareWithLocalPath)
Expand All @@ -48,6 +54,33 @@ bool ScoreComp::saveCompareMimeData(muse::ByteArray mimeData, const muse::String
return compareFiles(ScoreRW::rootPath() + u"/" + compareWithLocalPath, saveName);
}

bool ScoreComp::copyFile(const String& fullPath1, const String& fullPath2)
{
QString cmd = "cp";
QStringList args;
args.append(fullPath1);
args.append(fullPath2);

QProcess p;
p.start(cmd, args);
if (!p.waitForFinished()) {
QTextStream outputText(stdout);
outputText << "copy failed finished";
return false;
}

int code = p.exitCode();
if (code) {
QByteArray ba = p.readAll();
QTextStream outputText(stdout);
outputText << String(ba);
outputText << String(" <cp %1 %2 failed, code: %3 \n").arg(fullPath1, fullPath2).arg(code);
return false;
}
LOGD() << "Copy succeeded: " << String(" <cp %1 %2>\n").arg(fullPath1, fullPath2).arg(code);
return true;
}

bool ScoreComp::compareFiles(const String& fullPath1, const String& fullPath2)
{
QString cmd = "diff";
Expand Down
11 changes: 9 additions & 2 deletions src/importexport/musicxml/internal/musicxml/export/exportmusicxml.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7892,8 +7892,15 @@
}
}

if (!muse::RealIsEqual(mag, 1.0)) {
xml.tag("staff-size", mag * 100);
double lineDistance = st->lineDistance(Fraction(0, 1));
bool needWriteLineDistance = !muse::RealIsEqual(lineDistance, 1.0);
bool needWriteMag = !muse::RealIsEqual(mag, 1.0);
if (needWriteLineDistance || needWriteMag) {
XmlWriter::Attributes attributes;

Check warning on line 7899 in src/importexport/musicxml/internal/musicxml/export/exportmusicxml.cpp

View workflow job for this annotation

GitHub Actions / windows_x64 

declaration of 'attributes' hides previous local declaration
if (needWriteMag) {
attributes.emplace_back(std::make_pair("scale", mag));
}
xml.element("staff-size", attributes, lineDistance * 100);
}

xml.endElement();
Expand Down
61 changes: 50 additions & 11 deletions src/importexport/musicxml/internal/musicxml/import/importmusicxmlpass1.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -762,22 +762,20 @@ static void scaleTitle(Score* score, Text* text);
Also sets Align and Yoff.
*/

static void addText2(VBox* vbx, Score* score, const String& strTxt, const TextStyleType stl, const Align align, const double yoffs)
static void addText2(VBox* vbx, Score* score, const String& strTxt, const TextStyleType stl, const Align align)
{
if (stl != TextStyleType::COMPOSER && overrideTextStyleForComposer(strTxt)) {
// HACK: in some Dolet 8 files the composer is written as a subtitle, which leads to stupid formatting.
// This overrides the formatting and introduces proper composer text
Text* text = Factory::createText(vbx, TextStyleType::COMPOSER);
text->setXmlText(strTxt.trimmed());
text->setOffset(muse::PointF(0.0, yoffs));
text->setPropertyFlags(Pid::OFFSET, PropertyFlags::UNSTYLED);
vbx->add(text);
} else if (!strTxt.isEmpty()) {
Text* text = Factory::createText(vbx, stl);
text->setXmlText(strTxt.trimmed());
text->setAlign(align);
text->setPropertyFlags(Pid::ALIGN, PropertyFlags::UNSTYLED);
text->setOffset(muse::PointF(0.0, yoffs));
text->setPropertyFlags(Pid::OFFSET, PropertyFlags::UNSTYLED);
vbx->add(text);
if (stl == TextStyleType::TITLE) {
Expand Down Expand Up @@ -994,6 +992,37 @@ static void inferFromTitle(String& title, String& inferredSubtitle, String& infe
inferredCredits = creditLines.join(u"\n");
}

static void resizeTitleBox(VBox* vbox)
{
double calculatedVBoxHeight = 0;
ElementList elist = vbox->el();
Score* score = vbox->score();
for (EngravingItem* e : elist) {
score->renderer()->layoutItem(e);
}

double padding = vbox->spatium();

for (EngravingItem* e : elist) {
if (e->isText()) {
Text* txt = toText(e);
Text::LayoutData* txtLD = txt->mutldata();

LD_CONDITION(txtLD->isSetBbox());

RectF bbox = txtLD->bbox();
bbox.moveTop(0.0);
txtLD->setBbox(bbox);
calculatedVBoxHeight += txtLD->bbox().height() + padding;
}
}

double heightInSp = calculatedVBoxHeight / vbox->spatium();
Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz Aug 19, 2025

Choose a reason for hiding this comment

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

Why not this?

    double heightInSp = calculatedVBoxHeight / padding;

if (heightInSp > vbox->propertyDefault(Pid::BOX_HEIGHT).toDouble()) {
vbox->undoChangeProperty(Pid::BOX_HEIGHT, heightInSp);
}
}

//---------------------------------------------------------
// addCreditWords
//---------------------------------------------------------
Expand Down Expand Up @@ -1037,12 +1066,11 @@ static VBox* addCreditWords(Score* score, const CreditWordsList& crWords, const
if (mustAddWordToVbox(w->type)) {
const TextStyleType tid = top ? tidForCreditWords(w, words, pageSize.width()) : TextStyleType::DEFAULT;
const Align align = alignForCreditWords(w, pageSize.width(), tid);
double yoffs = tid == TextStyleType::COMPOSER ? 0.0 : (maxy - w->defaultY) * score->style().spatium() / 10;
if (!vbox) {
Fraction vBoxTick = top ? Fraction(0, 1) : tick;
vbox = MusicXmlParserPass1::createAndAddVBoxForCreditWords(score, vBoxTick);
}
addText2(vbox, score, w->words, tid, align, yoffs);
addText2(vbox, score, w->words, tid, align);
} else if (w->type == u"rights" && score->metaTag(u"copyright").empty()) {
// Add rights to footer, not a vbox
static const std::regex tagRe("(<.*?>)");
Expand All @@ -1052,6 +1080,11 @@ static VBox* addCreditWords(Score* score, const CreditWordsList& crWords, const
}
}

if (vbox && !MScore::testMode) {
// Correct size
resizeTitleBox(vbox);
}

return vbox;
}

Expand Down Expand Up @@ -1799,6 +1832,7 @@ static void updateStyles(Score* score,
// The MusicXML specification does not specify to which kinds of text
// the word-font setting applies. Setting all sizes to the size specified
// gives bad results, so a selection is made:
// Only apply word-font style when "Apply default typeface" is unchecked
// exclude lyrics odd and even lines (handled separately),
// Roman numeral analysis and harp pedal diagrams (special case, leave untouched)
// and text types used in the title frame
Expand All @@ -1810,7 +1844,7 @@ static void updateStyles(Score* score,
continue;
}

bool needUseDefaultSize = tid == TextStyleType::HARMONY_ROMAN
bool needUseDefaultSize = configuration()->needUseDefaultFont() || tid == TextStyleType::HARMONY_ROMAN
Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz Aug 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why not using `this?

        bool needUseDefaultSize = needUseDefaultFont || tid == TextStyleType::HARMONY_ROMAN

|| tid == TextStyleType::HAMMER_ON_PULL_OFF
|| isTitleFrameStyle(tid)
|| isHarpPedalStyle(tid);
Expand Down Expand Up @@ -1866,9 +1900,14 @@ static double scaleText(const String& str, const Sid fontFaceSid, const double f
const double pagePrintableWidth = style.styleV(Sid::pagePrintableWidth).value<double>() * DPI;
const double pageWidth = style.styleV(Sid::pageWidth).value<double>() * DPI;
const double pageHeight = style.styleV(Sid::pageHeight).value<double>() * DPI;
const double textWidth = fm.boundingRect(RectF(0, 0, pageWidth, pageHeight), TextShowMnemonic, str).width();

return pagePrintableWidth / textWidth;
double longestLine = 0.0;
for (const String& line : str.split(u"\n")) {
const double textWidth = fm.boundingRect(RectF(0, 0, pageWidth, pageHeight), TextShowMnemonic, line).width();
longestLine = std::max(longestLine, textWidth);
}

return pagePrintableWidth / longestLine;
}

static void scaleCopyrightText(Score* score)
Expand All @@ -1878,12 +1917,12 @@ static void scaleCopyrightText(Score* score)
return;
}

const double fontSize = score->style().styleV(Sid::footerFontSize).value<double>();
const double sizeRatio = scaleText(copyright, Sid::footerFontFace, fontSize, score);
const double fontSize = score->style().styleV(Sid::copyrightFontSize).value<double>();
const double sizeRatio = scaleText(copyright, Sid::copyrightFontFace, fontSize, score);

if (sizeRatio < 1) {
const double newSize = floor(fontSize * sizeRatio * 10) / 10;
score->style().set(Sid::footerFontSize, newSize);
score->style().set(Sid::copyrightFontSize, newSize);
}
}

Expand Down
8 changes: 6 additions & 2 deletions src/importexport/musicxml/internal/musicxml/import/importmusicxmlpass2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3163,8 +3163,10 @@ void MusicXmlParserPass2::staffDetails(const String& partId, Measure* measure)
} else if (m_e.name() == "staff-tuning") {
staffTuning(&stringData);
} else if (m_e.name() == "staff-size") {
const double scale = m_e.doubleAttribute("scale", 1.0);
const double val = m_e.readDouble() / 100;
m_score->staff(staffIdx)->setProperty(Pid::MAG, val);
m_score->staff(staffIdx)->setProperty(Pid::MAG, scale);
m_score->staff(staffIdx)->setProperty(Pid::LINE_DISTANCE, val);
Comment on lines +3168 to +3169
Copy link
Contributor

@rettinghaus rettinghaus Aug 12, 2025

Choose a reason for hiding this comment

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

Shouldn't this then also be adjusted in the export?

} else {
skipLogCurrElem();
}
Expand Down Expand Up @@ -4699,7 +4701,9 @@ void MusicXmlParserDirection::handleRepeats(Measure* measure, const Fraction tic
measure = measure->nextMeasure();
}
// Temporary solution to indent codas - add a horizontal frame at start of system or midway through
if (tb->isMarker() && toMarker(tb)->markerType() == MarkerType::CODA) {
MeasureBase* prevMeasureBase = measure->prev();
bool hbox = prevMeasureBase && prevMeasureBase->isHBox();
if (tb->isMarker() && toMarker(tb)->markerType() == MarkerType::CODA && !hbox) {
MeasureBase* gap = m_score->insertBox(ElementType::HBOX, measure);
toHBox(gap)->setBoxWidth(Spatium(10));
}
Expand Down
1 change: 0 additions & 1 deletion src/importexport/musicxml/tests/data/importTie4_ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,6 @@ Arranged by [Arranger]</metaTag>
<Text>
<eid>G_G</eid>
<style>subtitle</style>
<offset x="0" y="9.45"/>
<text><font size="11.94"/>for Piano</text>
</Text>
<Text>
Expand Down
2 changes: 0 additions & 2 deletions src/importexport/musicxml/tests/data/testBarlineLoc_ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,11 @@
<Text>
<eid>E_E</eid>
<style>title</style>
<offset x="0" y="-0.0367999"/>
<text><font size="22"/>Untitled score</text>
</Text>
<Text>
<eid>F_F</eid>
<style>subtitle</style>
<offset x="0" y="9.9632"/>
<text><font size="14"/>Subtitle</text>
</Text>
<Text>
Expand Down
2 changes: 0 additions & 2 deletions src/importexport/musicxml/tests/data/testBeamModes_ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,11 @@
<Text>
<eid>E_E</eid>
<style>title</style>
<offset x="0" y="-0.07525"/>
<text><font size="22"/>Beam Modes</text>
</Text>
<Text>
<eid>F_F</eid>
<style>subtitle</style>
<offset x="0" y="9.92425"/>
<text><font size="16"/>MuseScore Testcase</text>
</Text>
<Text>
Expand Down
2 changes: 0 additions & 2 deletions src/importexport/musicxml/tests/data/testBracketTypes_ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,11 @@
<Text>
<eid>E_E</eid>
<style>title</style>
<offset x="0" y="-0.07525"/>
<text><font size="22"/>Bracket Types</text>
</Text>
<Text>
<eid>F_F</eid>
<style>subtitle</style>
<offset x="0" y="9.92425"/>
<text><font size="16"/>MuseScore Testcase</text>
</Text>
<Text>
Expand Down
4 changes: 2 additions & 2 deletions src/importexport/musicxml/tests/data/testCopyrightScale_ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
<glissandoFontSize>10</glissandoFontSize>
<bendFontSize>10</bendFontSize>
<headerFontSize>10</headerFontSize>
<footerFontSize>1</footerFontSize>
<copyrightFontSize>10</copyrightFontSize>
<footerFontSize>10</footerFontSize>
<copyrightFontSize>2.3</copyrightFontSize>
<pageNumberFontSize>10</pageNumberFontSize>
<spatium>1.74978</spatium>
</Style>
Expand Down
2 changes: 0 additions & 2 deletions src/importexport/musicxml/tests/data/testCueGraceNotes_ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,11 @@
<Text>
<eid>E_E</eid>
<style>title</style>
<offset x="0" y="-0.07525"/>
<text><font size="22"/>Cue Grace Notes</text>
</Text>
<Text>
<eid>F_F</eid>
<style>subtitle</style>
<offset x="0" y="9.92425"/>
<text><font size="16"/>MuseScore Testcase</text>
</Text>
<Text>
Expand Down
2 changes: 0 additions & 2 deletions src/importexport/musicxml/tests/data/testCueNotes3_ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,11 @@
<Text>
<eid>E_E</eid>
<style>title</style>
<offset x="0" y="-0.110236"/>
<text><font size="22"/>MuseScore testfile</text>
</Text>
<Text>
<eid>F_F</eid>
<style>subtitle</style>
<offset x="0" y="9.88976"/>
<text><font size="16"/>Cue Notes 3 (Small Chords vs. Small Notes)</text>
</Text>
<Text>
Expand Down
2 changes: 0 additions & 2 deletions src/importexport/musicxml/tests/data/testDSalCodaMisplaced_ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,11 @@
<Text>
<eid>E_E</eid>
<style>title</style>
<offset x="0" y="-0.07525"/>
<text><font size="22"/>D.S al Codas</text>
</Text>
<Text>
<eid>F_F</eid>
<style>subtitle</style>
<offset x="0" y="9.92425"/>
<text><font size="16"/>MuseScore Testcase</text>
</Text>
<Text>
Expand Down
2 changes: 0 additions & 2 deletions src/importexport/musicxml/tests/data/testDSalCoda_ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,11 @@
<Text>
<eid>E_E</eid>
<style>title</style>
<offset x="0" y="-0.07525"/>
<text><font size="22"/>D.S al Codas</text>
</Text>
<Text>
<eid>F_F</eid>
<style>subtitle</style>
<offset x="0" y="9.92425"/>
<text><font size="16"/>MuseScore Testcase</text>
</Text>
<Text>
Expand Down
2 changes: 0 additions & 2 deletions src/importexport/musicxml/tests/data/testDuplicateFermataOnGraceNoteAndMainNote_ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,12 @@
<eid>G_G</eid>
<style>title</style>
<family>Times New Roman</family>
<offset x="0" y="-0.172872"/>
<text><font size="24"/><font face="Times New Roman"/>Mephisto Waltz No. 1</text>
</Text>
<Text>
<eid>H_H</eid>
<style>subtitle</style>
<family>Times New Roman</family>
<offset x="0" y="9.82548"/>
<text><font size="14"/><font face="Times New Roman"/>S. 514</text>
</Text>
</VBox>
Expand Down
2 changes: 0 additions & 2 deletions src/importexport/musicxml/tests/data/testDuplicateFermataOnGraceNote_ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,12 @@
<eid>G_G</eid>
<style>title</style>
<family>Times New Roman</family>
<offset x="0" y="-0.172872"/>
<text><font size="24"/><font face="Times New Roman"/>Mephisto Waltz No. 1</text>
</Text>
<Text>
<eid>H_H</eid>
<style>subtitle</style>
<family>Times New Roman</family>
<offset x="0" y="9.82548"/>
<text><font size="14"/><font face="Times New Roman"/>S. 514</text>
</Text>
</VBox>
Expand Down
Loading
Loading