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

Custom keysig - count symbols width #11581

Merged
merged 3 commits into from
May 30, 2022

Conversation

sammik
Copy link
Contributor

@sammik sammik commented May 9, 2022

Custom Key Signature - default accidental positions by symbol width, not fixed xstep

Accidentals have various widths (natural is thin, triple flat is fat). If default xstep were fixed, unwanted gaps, or overlays are produced.
Test file - accidentals on default positions (xAlt = 0)
before (xStep = 1.4 sp):
Width-before

after (xstep depends od symbol width):
Width-after

other minor tweaks:

  • add more transposable symbols (Sharp Sharp, Natural Sharp, Natural Flat)
  • replaced QString by std::string on some places
  • replaced score()->spatium() / mag() by spatium() (which already has mag calculation included)
  • I signed CLA
  • I made sure the code in the PR follows the coding rules
  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code
  • I made sure the title of the PR reflects the core meaning of the issue you are solving
  • I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
  • I made sure the commit message title starts with "fix #424242:" if there is a related issue
  • I created the test (mtest, vtest, script test) to verify the changes I made

@sammik sammik force-pushed the custom-keysig-symbols-width branch from e4be0a7 to ba51bc3 Compare May 9, 2022 20:33
@RomanPudashkin
Copy link
Contributor

Could you please add some unit-tests (only for changes in the engraving module) that will test your fix?

@sammik
Copy link
Contributor Author

sammik commented May 11, 2022

Could you please add some unit-tests (only for changes in the engraving module) that will test your fix?

@RomanPudashkin sorry, I need some guidance here. Or @cbjeukendrup ?
Any hint, or doc, or wiki please?
Thank You!

@RomanPudashkin
Copy link
Contributor

Sorry, I meant vtests. You already have an experience of adding new vtests :)
#10519

Or you can modify some existing vtests and add your case to them

@sammik sammik force-pushed the custom-keysig-symbols-width branch from ba51bc3 to fc32858 Compare May 12, 2022 06:40
@sammik
Copy link
Contributor Author

sammik commented May 12, 2022

Updated test file to show accidentals in default xposition (xAlt=0).

vtest fail is IMHO good, see
before
custom-keysig-1-1 ref

after
custom-keysig-1-1

@oktophonie I think, now You can look at it.

Thanks

@sammik sammik force-pushed the custom-keysig-symbols-width branch from fc32858 to 0c922ec Compare May 17, 2022 16:48
@oktophonie oktophonie added the vtests This PR produces approved changes to vtest results label May 26, 2022
@oktophonie
Copy link
Contributor

How does one even create a custom key signature? I can do one via the palettes but then it has the spacing that I specify (according to where I drag the symbols to), not a 'default' spacing.

@sammik

This comment was marked as outdated.

@sammik

This comment was marked as outdated.

custom-keysig-1.mscx
default positions by symbol width, not fixed xstep

other minor tweaks
- add more transposable symbols (sharp sharp, natural sharp, natural flat)
- replaced QString by std::string on some places
- replaced "score()->spatium() / mag()" by "spatium()" (which already has mag calculation included)
@sammik sammik force-pushed the custom-keysig-symbols-width branch from 0c922ec to 74015ff Compare May 30, 2022 04:06
@sammik
Copy link
Contributor Author

sammik commented May 30, 2022

I updated editor, so now accidentals takes default positions (unless Control ispressed).
@oktophonie are You happy with it now?
Thank You!

key-editor-snap.mp4

@RomanPudashkin RomanPudashkin merged commit 0fe0c7e into musescore:master May 30, 2022
@sammik sammik deleted the custom-keysig-symbols-width branch May 30, 2022 11:58
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
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants