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

fix #281007: MusicXML export: allow exporting scores without invisible elements #8273

Merged
merged 4 commits into from
Jul 5, 2021

Conversation

dmitrio95
Copy link
Contributor

Resolves: https://musescore.org/en/node/281007

This PR deals with MusicXML export of invisible items. Currently it does two kinds of changes:

  • For elements exported under the <notations> tag it marks the corresponding <notations> tag with print-object="no" attribute (which is allowed there since MusicXML 3.0). If there are items with different visibility corresponding to the same note, multiple <notations> tags with the corresponding print-object values are written.
  • For elements exported under the <direction> tag (which doesn't have print-object attribute) it adds an option to exclude such invisible elements completely from the MusicXML output. This option is currently enabled by default although it can certainly be changed.

This allows to have invisible spanners and texts, articulations and other annotations be correctly handled in MusicXML output by either marking them with print-object="no" attribute or by excluding them from the exported score if the former is not possible.

In order to be able to add new tests on MusicXML export I have also fixed some of the related tests disabled in 3e7f851. This fix is also currently included to this PR, although it can be merged separately if necessary.

Copy link
Contributor

@shoogle shoogle left a comment

Choose a reason for hiding this comment

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

Looks good to me! I've checked the include and the exclude cases and the output is what we'd hope for in each case:

  • Include invisible: anything invisible is exported to the MusicXML with the print-object="no" attribute for itself, or for the parent <notations> element if the element in question can't have a print-object attribute.

  • Exclude invisible: invisible objects are not exported at all in the MusicXML.

The output passes MusicXML validation in both cases.

I see that invisible notes, rests and tuplets are always included regardless of the preference value, which is necessary to ensure the number of beats in the measure matches the given time signature. Again, this is exactly what our friends at Daisy have requested.

@igorkorsukov, could you confirm you are happy with the first commit to re-enable the tests that you disabled in 3e7f851?

@dmitrio95
Copy link
Contributor Author

Actually the first commit would be also useful to enable setting up tests for other fixes related to MusicXML export. If that helps I can extract it to a separate PR to make it possible to review and merge it separately from the other changes.

@@ -404,6 +412,31 @@ void TestMxmlIO::mxmlMscxExportTestRefBreaks(const char* file)
delete score;
}

void TestMxmlIO::mxmlMscxExportTestRefInvisibleElements(const char* file)
Copy link
Contributor

Choose a reason for hiding this comment

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

@dmitrio95 do you have instructions how to run this test? I can't figure it out with MSVC!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have only launched this in Linux so I cannot tell for sure what may be needed for MSVC. However MuseScore seems to make use of ctest which should probably work on different platforms. So running the following from the build directory should work:

ctest -R iex_musicxml_tests

or, to get a more verbose output,

ctest --verbose -R iex_musicxml_tests

Another option is to run the relevant test executable manually. For me (with Linux and Ninja build system) the executable for this test is generated at

build.dir/src/importexport/musicxml/tests/iex_musicxml_tests

where build.dir is the name of the directory where I launched the build. Therefore I can launch MusicXML tests just by running this executable.

Maybe some of these options can work with MSVC as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

@wizofaus, if you just want to check the output then you can run the artifact from the GitHub Actions build. I think it includes invisible elements by default (but marks them invisible with print-object="no"), but if you want to try excluding them then you can edit the preferences INI file, which is located at:

Windows: %APPDATA%\MuseScore\MuseScore4Development.ini
Linux: ${XDG_CONFIG_HOME:-~/.config}/MuseScore/MuseScore4Development.ini

You need to add this line:

export/musicXML/exportInvisibleElements=false

It's a bit different on macOS, where the preferences are stored in a binary plist format.

macOS: ~/Library/Preferences/org.musescore.MuseScore4Development.plist

You can use XCode to edit the plist file. Add a new row inside the Root object:

Key Type Value
export.musicXML.exportInvisibleElements Boolean 0

Copy link
Contributor

@wizofaus wizofaus Jun 24, 2021

Choose a reason for hiding this comment

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

I was really wanting to understand how to debug unit tests in general in MSVC. I can't see anyway of even compiling them let alone running them (I tried setting up a project manually to do so but eventually got stuck with not having the .moc files being generated). BTW it does all work fine with the 3.6.2 codebase (though I had to tweak a few things).
I had thought they were just not being used currently but obviously other devs are able to develop/execute/debug unit tests and I really would like to be able to do myself, I'm not used to submitting PRs with changes and no associated unit tests!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants