-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Various fixes for 3.6.1 #7293
Various fixes for 3.6.1 #7293
Conversation
vpereverzev
commented
Jan 22, 2021
- https://trello.com/c/llFAEqi1
- https://trello.com/c/OxplS29w
- updates for Edwin.mss
That last commit should mention "Fix #315671" so that https://musescore.org/en/node/315671 gets marked fixed... |
@@ -62,6 +62,8 @@ | |||
<staffFontFace>Edwin</staffFontFace> | |||
<rehearsalMarkFontFace>Edwin</rehearsalMarkFontFace> | |||
<rehearsalMarkFontSize>14</rehearsalMarkFontSize> | |||
<rehearsalMarkFrameWidth>0.16</rehearsalMarkFrameWidth> | |||
<rehearsalMarkFrameRound>0</rehearsalMarkFrameRound> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? It is the new default anyhow, wouldn't it (resp. the old settings for this) need to go to one of the pre3.6 mss files instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems to cause at least one of the mtest failures. No idea where the other stems from though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edwin.mss is the subset of the new defaults that's applied if you tick the 'use Edwin' box in the score migration dialog; so it contains all settings related to updated text styles. (Or am I misunderstanding you?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. So it is not the one takes when "Reset all setting to default".
Still it might be good to add those former settings to the pre 3.6 mss files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks to me like they are already in there (legacy-style-defaults-v1.mss, legacy-style-defaults-v2.mss and legacy-style-defaults-v3.mss), with the former values (0.2 and 20).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, then I rest my case, thanks for checking
0f859e9
to
a815cb4
Compare
@@ -241,6 +241,7 @@ | |||
<barNoteDistance>1.2</barNoteDistance> | |||
<pedalPosBelow x="0" y="0"/> | |||
<trillPosAbove x="0" y="0"/> | |||
<createMultiMeasureRests>1</createMultiMeasureRests> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed now? I don't see anything in the rest of this PR that might explain this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It isn't needed anmore it seems...
4106d77
to
6e7c622
Compare
6e7c622
to
fd2bf1e
Compare
For reference, the Trello issue for the Edwin changes is here: https://trello.com/c/hihxtdrm/111-fix-edwin-martin |
Score* s = new Score(m, MScore::baseStyle()); | ||
Score* s = new Score(m, m->style()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be what is responsible for https://musescore.org/en/node/316559.
If I understand what is going on here, we are about to read the part. It may its own style settings we read from the file, but for everything else, we should be taking the defaults from the "regular" defaults. Instead, here we are taking them from the parent score, leading to exactly the problem described.
This change was made, I assume, to make sure that whatever "keep old style" setting we applied to the score also make their way into the part. Better, then, to do this directly. That is, don't look to the parent score, but the what set of style defaults the parent score was itself drawing from. We've got a whole bunch of functions available, I assume one of them does the trick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to me the same code used in setting style for the main score should be used here. So, I stole/adapted this from MasterScore::read1() and it seems to work, replacing the line above with the following:
Score* s = new Score(m, MScore::baseStyle());
int defaultsVersion = m->style().defaultStyleVersion();
s->setStyle(*MStyle::resolveStyleDefaults(defaultsVersion));
s->style().setDefaultStyleVersion(defaultsVersion);
Could be there is some detail I have wrong, or something more elegant. Anyhow, this is, I think, the basic right idea. The excerpt should have it's style defaults set in the same way the master score's style defaults were set - so the same "keep old style" settings apply. As opposed to what is happening now, which is actually copy the master score's current style settings.
Preusmably the same sort of change would happening in read302.