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 #279296: apply tempo text before fermata time stretch #4470

Merged
merged 1 commit into from
Dec 22, 2018

Conversation

dmitrio95
Copy link
Contributor

This PR is an attempt to fix https://musescore.org/en/node/279296.
The solution is to apply tempo from tempo text markings not after applying fermata time stretch. On file loading fermata markings may not know about tempo set by tempo text just because it has not been applied yet. I don't know though what was the reason for applying tempo text on system layout stage rather than measure layout so it would be good to test and review the proposed changes.

@MarcSabatella
Copy link
Contributor

As you may recall, we also had issues with relative order of tempo vs volta that was fixed by changing the order, but both in the system layout stage. It was formerly even later, with even worse problems, so moving it earlier still sounds good to me :-)

I guess the thing to test especially will be different repeat scenarios. Not sure if all the code to figure out repeat order was somehow depending on tempo coming later.

@anatoly-os anatoly-os merged commit 24020da into musescore:master Dec 22, 2018
@jeetee
Copy link
Contributor

jeetee commented Dec 23, 2018

I don't spot additional issues at first glance.

As a sidenote; I feel that a closer look at unrolling the score is warranted and the tempomap (and all rendering) should "just" flow from there. Combine this with the idea to move the RepeatList property of a volta onto every Element and you've got a very good start for rendering the score and allow different playback (dynamics but also play only on 2nd pass) for everything.
It could pose some UX challenge to nicely present this, but in my opinion this should be the (playback) way forward.

@dmitrio95 dmitrio95 deleted the 279296-fermata-time-stretch branch April 30, 2019 09:13
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

4 participants