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 #17796: Export correct system-distance #19512

Merged
merged 2 commits into from
Sep 30, 2023

Conversation

rettinghaus
Copy link
Contributor

Fixes #17796

@cbjeukendrup
Copy link
Contributor

Looks good to me! It would be ideal if you could also add a test file for this.

@rettinghaus
Copy link
Contributor Author

@cbjeukendrup I'm really willing to add a test, but I don't know how to get it working. Any help would be appreciated.

@cbjeukendrup
Copy link
Contributor

@rettinghaus It looks like there are two options:

  1. Use mxmlMscxExportTestRef. This requires a MuseScore file, and a reference XML file, i.e. testSystemDistance.mscx and testSystemDistance_ref.xml.
    • However, this method sets PREF_EXPORT_MUSICXML_EXPORTLAYOUT to false, while you want it to be true. Easiest solution might be to change the signature of mxmlMscxExportTestRef to
      void Musicxml_Tests::mxmlMscxExportTestRef(const char* file, bool exportLayout = false)
      and you can doubtlessly fill in the rest from there.
    • When saving a MSCX file in MuseScore 4, it will save a whole folder, but for the purpose of the test you can just take only the mscx file out of that folder.
  2. Keep using mxmlIoTest, and make sure that the input file in the repo (i.e. testSystemDistance.xml) is exactly equal to the expected output file.

Since the purpose here is to test export, and not import/roundtrip, I think option 1 would make slightly more sense, do you agree?

Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 30, 2023
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 30, 2023
@rettinghaus
Copy link
Contributor Author

@cbjeukendrup Thanks for the tip! Works fine now.

Copy link
Contributor

@cbjeukendrup cbjeukendrup left a comment

Choose a reason for hiding this comment

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

Thanks!

@cbjeukendrup cbjeukendrup merged commit 0abd780 into musescore:master Sep 30, 2023
11 checks passed
@rettinghaus rettinghaus deleted the xmldist branch September 30, 2023 14:10
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Oct 1, 2023
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Oct 2, 2023
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Oct 2, 2023
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Oct 2, 2023
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Oct 2, 2023
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Oct 2, 2023
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Oct 2, 2023
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.

MusicXML exporting: system-distance value is too large for systems that end with a multi-measure rest.
4 participants