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 #296034: Crash when multi-measure rests are enabled and press End #5415

Merged
merged 1 commit into from
Nov 19, 2019

Conversation

Jojo-Schmitz
Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz commented Oct 24, 2019

fixes https://musescore.org/en/node/296034 and also https://musescore.org/en/node/296541

also consistenltly use lastMeasureMM() rather than last() in other places of mscore/scoreview.cpp

@MarcSabatella
Copy link
Contributor

Thanks for getting this! FWIW, the downside of just using lastMeasureMM() it won't account for a frame after the last measure. Not sure if that's actually relevant in the way it's being used or not (frame won't even appear in continuous view).

@Jojo-Schmitz
Copy link
Contributor Author

I tried with a frame on the next page and it worked

@dmitrio95
Copy link
Contributor

In my tests it doesn't indeed seem to handle frame at the end correctly. Maybe we should introduce something like MeasureBase* Score::lastVisible() or lastMM() function which would return either a frame or lastMeasureMM(), depending on what exactly stands at the end of the score?

@MarcSabatella
Copy link
Contributor

Such a function seems logical to me.

@Jojo-Schmitz
Copy link
Contributor Author

test.zip
Seems to work for me with the attached score (rename to mscz).
But a lastMM() or some such might be better indeed.

@dmitrio95
Copy link
Contributor

test.zip
Seems to work for me with the attached score (rename to mscz).

For me with this PR End key moves me to the last "usual" measure (MM rest spanning for 11 measures), but not to boxes in the last two pages.

also consistenltly use `lastMeasureMM()` rather than `last()` in other
places of that file
@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Nov 4, 2019

Checked again: the End key does get me to the last page of that test score.
The Page-up key only gets be back to page 3 though, no matter how ofter it gets pressed, but that is the case without the PR too, so nothing I broke ;-), the Home key does get me to page 1, and Page-down does get me to the end (if pressed repeatedly)

@MarcSabatella
Copy link
Contributor

Still doesn't make sense to me that this would work with a frame all by itself on the last page.

A possibly related issue, though, that doesn't involve mmrests - the viewport moves to the last page with measures on it if there is a frame all by itself on the last page and I try adjusting its size. So somehow thr viewport updating code I think could also benefit from a lastMeasureBaseMM function or however we choose to define it.

@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Nov 16, 2019

Sure, a lastMeasureBaseMM() function would make sense here. Just as long as it doesn't exist, this PR is better than a crash ;-)

@dmitrio95 dmitrio95 merged commit 6344513 into musescore:master Nov 19, 2019
@Jojo-Schmitz Jojo-Schmitz deleted the page-end-crash branch November 19, 2019 11:12
@RobFog
Copy link

RobFog commented Nov 19, 2019

3.3.3 milestone?

@dmitrio95 dmitrio95 added this to the MuseScore 3.3.3 milestone Nov 19, 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.

4 participants