fix #286531: change staff type works for all options #4893
Conversation
|
Ah that's good. I dealt with the issue for both layout and draw for the KeySigs so I think that these two approaches should work together. In any case I realize now that it would be good to also do it on layout like in your solution. I decided not to do so at first since I only thought about normal barlines and assumed that it might just be better to leave the space that they occupied. It definitely looks a bit weird that way with the wider barlines on certain occasions though. |
libmscore/keysig.cpp
Outdated
| @@ -101,7 +101,8 @@ void KeySig::layout() | |||
| } | |||
|
|
|||
| _sig.keySymbols().clear(); | |||
| if (staff() && !staff()->genKeySig()) // no key sigs on TAB staves | |||
| // no key sigs on TAB staves or if the staff type says no key sigs | |||
| if (staff() && (!staff()->genKeySig() || !staff()->staffType(tick())->genKeysig())) | |||
There was a problem hiding this comment.
You might consider just adding the tick parameter to Staff::genKeySig(), that's how most other tick-dependent staff properties are handled. Llook at, for example, Staff::small()
There was a problem hiding this comment.
Ah okay that makes sense, I will look into that! Thank you!
There was a problem hiding this comment.
Also, there shouldn't be a need to test both the global setting and the tick-based setting. We don't do that elsewhere. The tick-based check should do the right thing no matter what.
There was a problem hiding this comment.
Yeah I will take out the first part of the OR. They do the same thing except the first part checks from the beginning of the score and the other one checks from the current tick which is what should be happening. Thank you.
| } | ||
| keysig->setKeySigEvent(keyIdx); | ||
| keysig->layout(); | ||
| if (staff->staffType(tick())->genKeysig()) { |
There was a problem hiding this comment.
So we were previously generating them here if the global staff property was set, and now we still will be for the global staff property but not if this option is set by staff type change? That seems inconsistent.
There was a problem hiding this comment.
I'm not sure I understand what you mean by it being inconsistent. I made this change based on how time signatures handled this, and it made sense at first, but I definitely could be missing something. What would you propose instead?
There was a problem hiding this comment.
Right now, if you go to Staff Properties and turn off "Generate key signatures", it seems from this code that the code below still gets executed - correct? But with you change, that same code does not get executed if instead you turn off the same property from a staff type change. That's what seems executed. Either that code should be executed in both cases or in neither. I don't have a preference offhand, you should test it both ways and see what differences there might be.
There was a problem hiding this comment.
Oh actually I just realized there is no such thing as "Generate key signatures" in Staff Properties which is a bit interesting. The only way to change this property is through "Change staff type"
There was a problem hiding this comment.
Okay I just fixed this in the UI. StaffType is what gets changed for both "Staff Properties" and "Change staff type". It's just in Staff Properties, it changes the properties from Fraction(0, 1) rather than a tick somewhere in the middle of the score. Because of this, the line of code I added before does in fact work for both cases.
peterhieuvu
force-pushed
the
286531-staff-type
branch
from
66ea070
to
dbfcae2
Compare
peterhieuvu
force-pushed
the
286531-staff-type
branch
from
dbfcae2
to
03fb1a5
Compare
|
rebase needed |
dmitrio95
added
the
archived
|
Archiving this pull request, feel free to reopen or open a new pull request on this issue. |
Change Staff Type previously didn't work for hiding barlines and not generating key signatures. In this fix I removed the key signature from layout and ceased to draw the symbol. Additionally I also made it so that it wouldn't generate the key signature in the first place when creating system headers. For barlines I just made sure not to draw the barline.