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.

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

Porting a bunch of old musicXML fixes #19972

Merged
merged 39 commits into from Nov 16, 2023

Conversation

mike-spa
Copy link
Contributor

@mike-spa mike-spa commented Nov 8, 2023

Each commit here refers to porting an old commit (linked in each description).

@@ -61,6 +62,7 @@ class mxmlNoteDuration
Fraction _dura;
TDuration _normalType;
Fraction _timeMod { 1, 1 }; // default to no time modification
MusicXMLParserPass1* _pass1;
Copy link
Contributor

Choose a reason for hiding this comment

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

better swap this with the next to avoid a warning reg. wrong order of initialisation

@@ -4755,7 +5236,8 @@ Note* MusicXMLParserPass2::note(const QString& partId,
int msTrack = 0;
int msVoice = 0;

if (!_pass1.determineStaffMoveVoice(partId, staff, voice, msMove, msTrack, msVoice)) {
int voiceInt = _pass1.voiceToInt(voice);
if (!_pass1.determineStaffMoveVoice(partId, staff, voiceInt, msMove, msTrack, msVoice)) {
_logger->logDebugInfo(QString("could not map staff %1 voice '%2'").arg(staff + 1).arg(voice), &_e);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe using voiceInt here too?

@@ -707,6 +752,9 @@ TEST_F(Musicxml_Tests, multiMeasureRest4) {
TEST_F(Musicxml_Tests, multipleNotations) {
mxmlIoTestRef("testMultipleNotations");
}
TEST_F(Musicxml_Tests, negativeOffset) {
mxmlImportTestRef("testNegativeOffset");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong aphabethical order...

TEST_F(Musicxml_Tests, overlappingSpanners) {
mxmlIoTest("testOverlappingSpanners");
}
TEST_F(Musicxml_Tests, partNames) {
mxmlImportTestRef("testPartNames");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

here too

Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 9, 2023
Backport of musescore#19972, those parts that hadn't been ported already (much) ealier.
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 9, 2023
Backport of musescore#19972, those parts that hadn't been ported already (much) ealier.
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 9, 2023
Backport of musescore#19972, those parts that hadn't been ported already (much) ealier.
auto firstMeasure = _score->measures()->first();
VBox* vbox = firstMeasure->isVBox() ? toVBox(firstMeasure) : MusicXMLParserPass1::createAndAddVBoxForCreditWords(_score);
vbox->add(t);
} else if (_wordsText != "" || _rehearsalText != "" || _metroText != "") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems to collide with #8678, which , as far as I can tell, got merged into 3.6.2_backend later

@@ -175,6 +177,12 @@ class MusicXMLParserPass1
int octaveShift(const QString& id, const staff_idx_t staff, const Fraction f) const;
const CreditWordsList& credits() const { return _credits; }
bool hasBeamingInfo() const { return _hasBeamingInfo; }
bool isVocalStaff(const QString& id) const { return _parts[id].isVocalStaff(); }
static VBox* createAndAddVBoxForCreditWords(Score* const score, const int miny = 0, const int maxy = 75);
const int maxDiff() { return _maxDiff; }
Copy link
Contributor

Choose a reason for hiding this comment

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

not rather int maxDiff() const { return _maxDiff; }? Otherwise there's a warning "'const' type qualifier on return type has no effect"

@mike-spa mike-spa force-pushed the musicXMLimprovements branch 3 times, most recently from a779867 to 2bf2a0c Compare November 14, 2023 14:21
@@ -3316,7 +3316,7 @@ void MusicXMLParserDirection::handleNmiCmi(Measure* measure, const int track, co
ha->setTrack(track);
MusicXMLDelayedDirectionElement* delayedDirection = new MusicXMLDelayedDirectionElement(totalY(), ha, track, "above", measure, tick);
delayedDirections.push_back(delayedDirection);
_wordsText.replace("NmiCmi", "");
_wordsText.replace("NmiCmi", "N.C.");
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for such a staff text, as the "N.C." already gets set as a chord symbol

You'd see the issue if and when including the units tests from aca2536

@musescore musescore deleted a comment from Jojo-Schmitz Nov 15, 2023
@RomanPudashkin RomanPudashkin merged commit e2fe70b into musescore:master Nov 16, 2023
11 checks passed
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 16, 2023
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 16, 2023
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 16, 2023
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Mar 18, 2024
Backport of one forgotten commit from musescore#19972
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Mar 18, 2024
Backport of one forgotten commit from musescore#19972
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Apr 9, 2024
Backport of one forgotten commit from musescore#19972
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants