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

Import of MusicXML files: Chord symbol didn't load correctly #16638

Closed
WindyPigeon opened this issue Mar 3, 2023 · 11 comments · Fixed by #18737
Closed

Import of MusicXML files: Chord symbol didn't load correctly #16638

WindyPigeon opened this issue Mar 3, 2023 · 11 comments · Fixed by #18737
Assignees
Labels
engraving MusicXML regression MS3 Regression from MS3 (3.6.2)
Projects

Comments

@WindyPigeon
Copy link

Issue type

Engraving bug

Bug description

When importing MusicXML files, MuseScore 4 removes the root name from the chord symbols.
You can test it yourself with test/harmony2.xml or other MusicXML files, and you will get similar results as the screenshot.

The problem probably comes from the following code block
https://github.com/musescore/MuseScore/blob/v4.0.1/src/importexport/musicxml/internal/musicxml/importmxmlpass2.cpp#L5215-L5239
MuseScore 4 has a logic error in handling of MusicXML's root-step tag.
According MusicXML documentation on the root-step element, the text attribute in root-step element is not necessary, and if text attribute doesn't exist in root-step element it will use the element contents as the root name.

Below is my suggested fix.
Replace line 5215 - line 5220 of importmxmlpass2.cpp with following the code:

if ((_e.attributes().hasAttribute("text")) && (_e.attributes().value("text").toString() != "")) {
    step = _e.attributes().value("text").toString();
} else if (_e.readElementText != "") {
    step = _e.readElementText();
} else {
    invalidRoot = true;
}

I don't have a C++ development environment on my PC, please somebody help me to test the code and commit it to GitHub.

Steps to reproduce

Import any MusicXML file that contains annotations for chord symbols.

Screenshots/Screen recordings

image

MuseScore Version

4.0.1

Regression

Yes, this used to work in Musescore 3.x and now is broken

Operating system

Windows

Additional context

No response

@Tantacrul Tantacrul added engraving regression MS4 Regression on a prior release labels Mar 3, 2023
@Jojo-Schmitz
Copy link
Contributor

Isn't this a duplicate of #13214 ?

@WindyPigeon
Copy link
Author

Isn't this a duplicate of #13214 ?

Yes. I think it will be fixed soon, because the cause of the bug has been found.

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Mar 3, 2023

Hmm, not sure your analysis is correct though, that code is in since at lest Nov 17, 2020, 133fc82, so in 3.6 already, but that doesn't have this issue as far as I can tell.

But let's summon @lvinken on this ;-)

@MarcSabatella
Copy link
Contributor

The cause is known; it's what I stated in that other issue. Not the red herring that was also posted in that other issue, although it certainly looked like a good guess.

@WindyPigeon
Copy link
Author

The cause is known; it's what I stated in that other issue. Not the red herring that was also posted in that other issue, although it certainly looked like a good guess.

Yes. You're right. Although the logic of that code block is weird for me, but it doesn't cause the broken. Maybe other changing between 3.6 and 4.0 cause this problem, I'm now comparing the source code between 3.6 and 4.0 to find the culprit.

@MarcSabatella
Copy link
Contributor

Again, it's exactly as I stated multiple times already - the style initialization code no longer loads the chord description file, so it needs to be done separately. The only question is where that should be best done. I think it makes sense to do it somewhere fairly central so each importer doesn't need to do it manually. I just don't know where the right place would be. See in particular my comment where I point directly to the relevant code - #13214 (comment)

@lvinken
Copy link
Contributor

lvinken commented Mar 4, 2023

Hmm, not sure your analysis is correct though, that code is in since at lest Nov 17, 2020, 133fc82, so in 3.6 already, but that doesn't have this issue as far as I can tell.

But let's summon @lvinken on this ;-)

Actually I agree with Marc's analysis, so I have nothing to add here.

@ExcPoint
Copy link

ExcPoint commented Mar 5, 2023

I am running into the same issue in MuseScore 4.0. MusicXML is not rendering correctly when opened in all formats, Compressed (.mxl), Uncompressed (.musicxml & .xml).

@MarcSabatella
Copy link
Contributor

Indeed, the issue is not fixed, but the workarounds explained previously continue to work (change appearance to jazz & back, or save & reload).

@cbjeukendrup cbjeukendrup added this to To do in MusicXML via automation Mar 26, 2023
@DmitryArefiev DmitryArefiev added regression MS3 Regression from MS3 (3.6.2) and removed regression MS4 Regression on a prior release labels Apr 27, 2023
@asattely
Copy link
Contributor

asattely commented May 3, 2023

This issue is fixed by #17427

@asattely asattely moved this from To do to In progress in MusicXML May 3, 2023
@Jojo-Schmitz
Copy link
Contributor

Should get resolved by #18737 as far as I can tell

MusicXML automation moved this from In progress to Done Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engraving MusicXML regression MS3 Regression from MS3 (3.6.2)
Projects
Status: Done
MusicXML
  
Done
10 participants