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

Merged
merged 1 commit into from
Oct 2, 2023
Merged

Conversation

Jojo-Schmitz
Copy link
Owner

Backport of musescore#19512

Resolves: musescore#17796

@@ -5987,9 +5987,10 @@ void ExportMusicXml::print(const Measure* const m, const int partNr, const int f

if (mpc.systemStart && !mpc.pageStart) {
// see System::layout2() for the factor 2 * score()->spatium()
const Measure* prevSystem = mpc.prevMeasure->isMMRest() ? mpc.prevMeasure->mmRestFirst() : mpc.prevMeasure;
Copy link
Owner Author

@Jojo-Schmitz Jojo-Schmitz Sep 25, 2023

Choose a reason for hiding this comment

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

@rettinghaus: As Mu3 doesn't have coveringMMRestOrThis(), I'm using your earlier approach here, is that OK?
Interestingly this PR here passes the mtests, but your's doe not?!

Choose a reason for hiding this comment

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

Does it work? My tests didn't, that's why I changed the approach. I'd be grateful if you could help me with that test.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Well, the tests pass here

@Jojo-Schmitz Jojo-Schmitz force-pushed the 3.x-xmldist branch 3 times, most recently from ec7032d to d3ec60c Compare October 1, 2023 19:28
@Jojo-Schmitz
Copy link
Owner Author

@rettinghaus mind to check why this doesn't pass the mtest, esp. at https://github.com/Jojo-Schmitz/MuseScore/actions/runs/6372850758/job/17296046686?pr=186#step:5:1702 (the rest seems minor things to manually correct)

@rettinghaus
Copy link

That's why I asked you if this approach works for you. It should work, but doesn't, that why I changed it in 4.2.0.

@Jojo-Schmitz
Copy link
Owner Author

Hmm, yeah, unfortunaltely Mu3 seems to be lacking the infrastructure you're using in your Mu4 PR, that coveringMMRestOrThis().

@Jojo-Schmitz
Copy link
Owner Author

Jojo-Schmitz commented Oct 2, 2023

Nonsens, it doesn't, no idea why I couldn't find it initially. Thanks for getting me on track again ;-)

@Jojo-Schmitz Jojo-Schmitz force-pushed the 3.x-xmldist branch 3 times, most recently from dc33aa1 to d83853a Compare October 2, 2023 12:41
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.

2 participants