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

use accidental width to space key signatures #9389

Merged
merged 3 commits into from
Dec 2, 2021

Conversation

Nick-Mazuk
Copy link
Contributor

Resolves: #9379

This PR uses the accidental width to calculate the spacing of key signatures. Before, all accidentals were spaced 1 space apart, regardless of font or symbol. Now, the key signature calculates each symbol's width to space them out appropriately, and the gap between them is accidentalDistance, the same style value used for accidentals on chords.

This will provide greater consistency and clarity for accidentals.

Notes

  • This PR may not work for custom key signatures. As of today, it looks like it's not possible to create custom key signatures in v4, so I was not able to test it.
  • Unlike for chords, there does not seem to be much space remove extra space with symbol cutouts. This is because key signatures have well defined patterns (like they move in 4ths or 5ths), and there's just not the vertical clearance in any of the fonts. The only places I could see making the case for adding this is sharps in Peteluma or naturals cancelling out the sharps. If this is desired, I can add that in (though it would increase the scope of the PR quite a bit).

Screen Shot 2021-10-06 at 2 57 34 PM

Screen Shot 2021-10-06 at 2 57 55 PM

Screen Shot 2021-10-06 at 2 58 16 PM

Screen Shot 2021-10-06 at 3 03 43 PM

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Oct 7, 2021

Compare with this
image
using Leland, it does seem to use cutouts and looks narrower than your 1st example. same with Bravura, Emmentaler, Gonville, MuseJazz and Petaluma too, very clearly (and possible a tad too much) for the latter two in 3.6.2, MuseJazz:
image
Petaluma:
image
Not with flats though.
So IMHO those cutouts should be used.

For more comparison see #9390's (faling) vtests (a backport of this PR on top of the 3.x branch, as the vtests on the master branch just don't work)

We can get to a denser spacing for sharp key signatures, one similar to that of MuseScore 3(.6.2) though by reducing (the default for) Sid::accidentalDistance from 0.22 to 0.1.
That in turn would make flat key signatures narrower than before though.

On the up-side: Even reducing that value to 0 does not result in that too dense appearance of MuseJazz and Petaluma, although it could be made denser still (using cutouts).

Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Oct 7, 2021
@oktophonie
Copy link
Contributor

Using the cutouts would be helpful, particularly for naturals-cancelling-sharps, as you mention, since they look rather wonky otherwise:

key signatures

I also think it would be good to have the spacing as its own setting (independent of accidentalDistance).

Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Oct 7, 2021
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Oct 8, 2021
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Oct 8, 2021
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Oct 8, 2021
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Oct 8, 2021
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Oct 8, 2021
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Oct 9, 2021
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 23, 2021
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 24, 2021
@Nick-Mazuk
Copy link
Contributor Author

Nick-Mazuk commented Nov 30, 2021

Three updates to the PR:

  1. Rebased to make sure it works with the most recent version of master
  2. The accidental distance is now defined by its own style parameter named keysigAccidentalDistance It currently has the same value as accidentalDistance.
  3. Accidental cutouts are now honored

Screen Shot 2021-11-29 at 10 57 26 PM

Screen Shot 2021-11-29 at 10 57 09 PM

Unless there's anything that needs fixing, I'm considering this PR and ready for merge (upon approval).

fix codestyle

refactor for codestyle
@Nick-Mazuk
Copy link
Contributor Author

Looks like the vtests are failing, but the exact failure is expected with this change.

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Nov 30, 2021

Before:
image
After (esp. see the naturals being much closer):
image
Seems a vtests label is in order ;-)

@oktophonie
Copy link
Contributor

The kerning-via-cutouts is excellent in principle, but the naturals in flat-formation are too close together. Indeed everything is a bit too tight so probably the basic spacing value should be increased slightly.

@oktophonie oktophonie added the vtests This PR produces approved changes to vtest results label Nov 30, 2021
@Nick-Mazuk
Copy link
Contributor Author

Quick update, this PR is not ready for merge. For some reason, if the gap size style property for key signature is updated, the score does not visually update.

@RobFog
Copy link

RobFog commented Nov 30, 2021

You can mark the pull request as a "draft".

@Nick-Mazuk
Copy link
Contributor Author

Nick-Mazuk commented Nov 30, 2021

Ok, assuming the tests pass, I believe the PR is ready for merging. I've updated the exact spacing values after a discussion with @oktophonie.

@oktophonie oktophonie added this to In progress in [MU4.0 - ENGRAVING] via automation Dec 1, 2021
[MU4.0 - ENGRAVING] automation moved this from In progress to Reviewer approved Dec 2, 2021
@RomanPudashkin RomanPudashkin merged commit b109452 into musescore:master Dec 2, 2021
[MU4.0 - ENGRAVING] automation moved this from Reviewer approved to Done Dec 2, 2021
@Nick-Mazuk Nick-Mazuk deleted the petaluma-key-signatures branch December 3, 2021 22:34
@DmitryArefiev DmitryArefiev removed their assignment Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vtests This PR produces approved changes to vtest results
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

[MU4 Issue] Improve spacing of accidentals in key signatures
6 participants