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

[MU4 Issue] MusicXML import hides instruments incorrectly #16135

Closed
majenkotech opened this issue Jan 30, 2023 · 16 comments · Fixed by #18556
Closed

[MU4 Issue] MusicXML import hides instruments incorrectly #16135

majenkotech opened this issue Jan 30, 2023 · 16 comments · Fixed by #18556
Assignees
Labels
MusicXML regression_ms3 Regression from MS3 (3.6.2)
Projects

Comments

@majenkotech
Copy link

majenkotech commented Jan 30, 2023

Describe the bug
Attempting to import MusicXML that has been exported from something else, such as Finale Notepad, results in missing instruments in the score (the instruments are there as evidenced by the mixer and the instrument list, they just don't appear in the score).

To Reproduce
Steps to reproduce the behavior:

  1. Export something from another program as MusicXML
  2. Import it into MuseScore - instruments are missing
  3. Examine instrument list panel you see the instruments exist but are hidden

A sample MusicXML file that fails to import:

Razzle Dazzle (full).xml.gz

There should be full SATB+Piano. Instead you get just the piano in the score, but the full SATB+Piano in the mixer.

@MarcSabatella
Copy link
Contributor

MarcSabatella commented Jan 30, 2023

FWIW, the instruments are present, they are just hidden. You can display them using the Instruments panel in the left sidebar. Not clear why - what makes this particular MusicXML file different from others where this works normally. But to be clear - MusicXML does work normally. So in the future, better to give your issue titles a more specific title, like "some instruments hidden by default on import of specific MusicXML file".

You also mentioned a particular MusicXML that exports with errors. Be sure to open a separate issue for that and attach that particular MusicXML file as well.

@majenkotech
Copy link
Author

It's not this particular XML file. It's every XML file I try and import that has been exported from Finale Notepad (the only source of XML files I have that isn't MuseScore, by the way).

And as for the exporting from MuseScore - it's every file I try to export (the same ones that I export fine from MS3) that fail to import.

@MarcSabatella
Copy link
Contributor

If every file imports from Finale Notepad successfully other than some instruments being hidden by default, great, then no need for additional issues reports. Probably you are simply hitting upon the same bug over and over.

As for export, again, nonally it works correctly, so if you have a specific file that produces a specific error, you'll need to open a separate issue so it can be investigated. The thousands of test files all work perfectly, so presumably your scores all have something in common that is causing them to run into the same bug over and over.

@majenkotech majenkotech changed the title [MU4 Issue] MusicXML import and export is just plain broken [MU4 Issue] MusicXML import hides instruments incorrectly Jan 30, 2023
@majenkotech
Copy link
Author

Sometimes on import to MS4 the instruments, while they may be there in the instrument list, are empty. The one I attached doesn't show that problem, but when I fine one that does I'll be sure to provide it.

@Jojo-Schmitz
Copy link
Contributor

Possibly related to my #12680 ?

@lvinken
Copy link
Contributor

lvinken commented Feb 14, 2023

In case of the "Razzle Dazzle" file, the issue is caused by the 'staff-details print-object="no"' element. In function MusicXMLParserPass2::staffDetails() it causes the whole staff to be invisible, while in MusicXML this element does not necessary apply to the whole staff. The issue is MuseScore 4 specific, it was introduced with commit c6dc78c.

Simple example files attached.
issue_16135.zip

Note: in MuseScore 3, individual measures can be set to invisible, which matches with the common use cases of MusicXML's 'staff-details print-object="no"' element, although this was never supported by MuseScore 3's MusicXML importer. In MuseScore 4 this function seems to be missing.

And to answer Jojo's question: no, in this specific case it is not related.

@MarcSabatella
Copy link
Contributor

@lvinken when you refer to the invisible measure, are you referring to the checkbox in Measure Properties allowing you to hide a measure for a single staff of a multi-staff score? That's still supported, and still works.

@lvinken
Copy link
Contributor

lvinken commented Feb 14, 2023

Thanks, that was indeed what I was referring to. I had expected the checkbox to be in the measure properties tab (F8) and did not expect it in the measure properties popup (right-click).

@cbjeukendrup cbjeukendrup added this to To do in MusicXML via automation Feb 25, 2023
@asattely asattely moved this from To do to In progress in MusicXML Mar 27, 2023
@asattely asattely self-assigned this Mar 27, 2023
@asattely
Copy link
Contributor

I would like to request some input as to a definitive idea of how to handle this. From what I understood from reading the issue and the proceeding conversation, it seems that if this <staff-details print-object="no"> tag is in a measure, we want to hide the measure. Is this consistent with how we expect MuseScore to deal with that tag within a measure? For reference, the draft PR above demonstrates this behavior on import.

If that's what we want, then we should definitely make sure we are all on the same page with what we expect the behavior to be, considering there's a unit test that clearly expects the staff to be hidden if the first measure of that staff has the print-object="no" attribute (https://github.com/musescore/MuseScore/blob/master/src/importexport/musicxml/tests/data/testHiddenStaves.xml).

MusicXML gurus, unite!

@lvinken
Copy link
Contributor

lvinken commented Mar 28, 2023

The MusicXML specification is not explicit on the exact semantics (see https://www.w3.org/2021/06/musicxml40/musicxml-reference/elements/staff-details/). Two of the sample files (BrookeWestSample.musicxml and SchbAvMaSample.musicxml) show print-object affects the enclosing measure and all subsequent measures until a print-object with the inverse value is encountered.

@Jojo-Schmitz
Copy link
Contributor

Seems related to #17398, a problem with exporting hidden empty staves, but also discussing an issue on their import, which in turn seems to be a regression vs. Mu3

@lvinken
Copy link
Contributor

lvinken commented Jun 9, 2023

Created a few more test files with hidden measures including screenshots of import into Finale NotePad for reference:
invisible_measures.zip
I would expect MuseScore's import to match NotePad's.

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Jun 27, 2023

Came up again in https://musescore.org/en/node/351665

Seems Mu3 on those imports does check "Hide empty staves" and does uncheck "Don't hide empty staves in first system", but Mu4 does not, instead hides the entire staff throughout ('Closed eye' icon in the instruments tab).

Definitly a Mu3 regression.

@MarcSabatella
Copy link
Contributor

MarcSabatella commented Jun 27, 2023

FWIW, if the desired behavior is to add the staff but set it to be hidden, we should not set the instrument to hidden, but instead set the staff to hidden. And this should apply to all imports involving hidden staves/instruments, including import from MU3. I had long ago submitted a PR to fix that for MU3/MU2/MU1 import that was never merged - #13293. That code could be adapted and placed somewhere further upstream so all imports get the fix.

@Jojo-Schmitz
Copy link
Contributor

'Culprit' or trigger is

<staff-details print-object="no"/>

@cbjeukendrup cbjeukendrup added the regression_ms3 Regression from MS3 (3.6.2) label Jun 27, 2023
@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Jun 27, 2023

Seems 3.x ignores staff-details on pass 1 and uses staffDetails(partId) in pass 2, but there doesn't handle print-object
Mu4 handles this though:

    QString visible = _e.attributes().value("print-object").toString();
    if (visible == "no") {
        _score->staff(staffIdx)->setVisible(false);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MusicXML regression_ms3 Regression from MS3 (3.6.2)
Projects
Status: Done
MusicXML
  
Done
7 participants