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 #284235: implement lyricsMinDistance #4689

Merged
merged 2 commits into from Mar 4, 2019

Conversation

MarcSabatella
Copy link
Contributor

In 2.3.2, the "Minimum note distance" style setting was the only thing that determined minimum space between lyrics. For 3.0, we start there, but then also add an addition 1sp if space between lyrics. This is way too much and causes scores to take more space than before.

This PR implements a new lyricsMinDistance style setting that is added on top of the minimu note distance. I have it default to 0 right now, so default spacing is identical to 2.3.2 (except that 2.3.2 did not force hyphens to display whereas 3.0 does by default, which I think will prevent any complaints we are still too tightly spaced).

I expect there may be test failures due to the new style setting, but perhaps not.

Adding the style setting also gave me the opportunity to fix the tab order on the Lyrics page of the style dialog - it was terrible. Others need attention as well, but another day.

@MarcSabatella MarcSabatella added the work in progress not finished work or not addressed review label Feb 16, 2019
@MarcSabatella
Copy link
Contributor Author

I'm marking this work in progress as more tweaks may be needed to get good behavior with respect to hyphens. Also, seems I still don't quite have the tab order right.

@MarcSabatella MarcSabatella removed the work in progress not finished work or not addressed review label Feb 16, 2019
@MarcSabatella
Copy link
Contributor Author

Got the hyphen thing worked out. The force hyphen option actually didn't affect spacing at all, only whether the hyphen would show up at all. So it depended on ChordRest::shape() to add space, which was happening even when not needed. I have now updated that to add just enough space for the hyphen, and only when needed. I tested it with all sorts of different combinations of minimum note distance, minimum lyrics distance, and minimum hyphen length, and all works as I think it should.

As for the tab order, the remaining issue is the same one shared by other pages in the style dialog - the OffsetSelect widgets are not well-behaved here. So that needs to be addressed more globally.

Bottom line: I think this is good, but I welcome review and testing, of course!

@MarcSabatella
Copy link
Contributor Author

Tests passed with the first commit. Now added a second to handle some compatibility for read206 & read114, these will probably need updates to compat tests.

@anatoly-os
Copy link
Contributor

Rebase is needed.

@MarcSabatella
Copy link
Contributor Author

Rebased using GitHub, but also doing it again locally to get rid of that extra "merge" commit

@MarcSabatella
Copy link
Contributor Author

Good to go. Should make a noticeable difference in vocal scores, allowing them to be more compact by default but also allowing control over the lyric spacing.

@anatoly-os anatoly-os merged commit cd47399 into musescore:master Mar 4, 2019
@Jojo-Schmitz
Copy link
Contributor

Please add to 3.0.5 too

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