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

[MU3] Leland style values update #7016

Merged
merged 2 commits into from Dec 3, 2020

Conversation

vpereverzev
Copy link
Member

@vpereverzev vpereverzev added the vtests This PR produces approved changes to vtest results label Dec 3, 2020
libmscore/mscore.h Outdated Show resolved Hide resolved
@@ -593,7 +593,7 @@ static const StyleType styleTypes[] {
{ Sid::barGraceDistance, "barGraceDistance", Spatium(1.0) },
{ Sid::minVerticalDistance, "minVerticalDistance", Spatium(0.5) },
{ Sid::ornamentStyle, "ornamentStyle", int(MScore::OrnamentStyle::DEFAULT) },
{ Sid::spatium, "Spatium", SPATIUM20 },
{ Sid::spatium, "Spatium", 24.8 },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At several places SPATIUM20 is used. With this change SPATIUM and Sid::spatium are not the same anymore.
Are we sure this doesn't cause any problems because both SPATIUM and Sid::spatium are expected to be same or one is used where the other should be?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I limited myself to the changes here at Simon's request, let's see how this affects those elements that do not use it through the style, for some reason, and if something goes wrong, we will replace the value of spatium20 in the future

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If so, we should fix those places. But while Sid::spatium might have defaulted to SPATIUM20, certainly we always support changing that value, and a great many scores do, and overall, things work. We do find and fix bugs from time to time where things don't scale properly, but I don't remember any cases where it was on account of confusing these two values - normally just because we were forgetting to account for this at all, like in some particular layout(), draw(), or mag() function. Or we were missing it in spatiumChanged().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, perhaps. Probably we'll need to replace direct usage of SPATIUM20 with styleD(Sid::Spatium) in the next PRs

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

5 participants