search

lenmus / lomse

A C++ library for rendering, editing and playing back music scores.
MIT License
120 stars 28 forks source link

Display naturals on a key change to Cmaj/Amin #320

Closed dmitrio95 closed 3 years ago

dmitrio95 commented 3 years ago

Currently Lomse doesn't display anything on a key change if the new key doesn't have any accidentals. This makes it impossible for a performer to figure out that a key change has happened (example score: key-change-naturals.musicxml.txt): Screenshot_20210919_163106

In such cases naturals are commonly displayed to inform a performer about a key change. This PR implements this way of displaying such key changes: Screenshot_20210919_163037

Some extra spacing between naturals was also required to avoid naturals in a key signature being visually glued together, so that was also added to the key engraver.

cecilios commented 3 years ago

Thank you! I will review this as soon as possible.

cecilios commented 3 years ago

Thank you. It is an important fix. The code looks OK to me and I have not detected issues in visual regression tests, apart from a few scores that now, correctly, display the key change. So I will merge this.

cecilios commented 3 years ago

Just some information to avoid duplicating work. This PR has made me see that there is a related issue that Lomse does not do: when a change of time or key signature occurs after a system break, a cautionary indication must be added at the end of previous system. I will try to fix this when finishing what I'm currently doing.

dmitrio95 commented 3 years ago

Yes, I have noticed that if a key change occurs at a system break then two key signatures are displayed: the old one at system start and the new one just after it: изображение

Is this the issue you are talking about? A cautionary indication would indeed help here but also a choice of a key signature displayed in a system header somehow needs to be changed in such cases.

cecilios commented 3 years ago

I was referring to displaying the new key signature (or time signature) at the end of the previous system, such as in first system of this image: a1

This fix will automatically fix the issue displayed in your picture. I'm a little busy now but I hope to fix all this in three/four weeks.

dmitrio95 commented 3 years ago

Ah, thanks for clarification, so both these issues will be covered by that fix.