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 #287432: Key signature appears in multimeasure rest after being deleted from underlying measure #5134

Merged
merged 1 commit into from
Jun 17, 2019

Conversation

mattmcclinch
Copy link
Contributor

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

This is a follow-up to #4884.

When an existing mmrest is reused upon reenabling multimeasure rests, the old system trailer should be removed. It will be added back if needed. Also, the mmrest should take on the layout break properties of the last underlying measure, whether an existing mmrest was reused or not.

@@ -4011,7 +4011,7 @@ void Measure::removeSystemTrailer()
changed = true;
}
setTrailer(false);
if (changed)
if (system() && changed)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the purpose of this extra check? Is it just an optimization because we know we're going to recompute the width later anyhow? My recollection is system() will be null while we are still testing whether the measure fits, I'd have thought we needed to get the width correct here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I could have put the check in computeMinWidth() instead. The purpose is to make sure that the null system is not dereferenced in computeMinWidth().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Usually when this function is called, this already has a valid system. That is not the case if this function is called from createMMRest(). I considered copying most of the code from removeSystemTrailer() and pasting it into createMMRest(), but I opted for this approach.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you're right, my mistake in thinking this was normally being called before the measure is added to the system.

BTW, I know I've seen this behavior - leftover trailer at end of score - before. I can't say for sure if this was the exact scenario or not.

@dmitrio95 dmitrio95 merged commit 55acd81 into musescore:master Jun 17, 2019
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.

None yet

3 participants