Skip to content

Commit

Permalink
Fix #308757.
Browse files Browse the repository at this point in the history
  • Loading branch information
mattmcclinch committed Jan 9, 2021
1 parent c686fa4 commit dbaa726
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions libmscore/measure.cpp
Expand Up @@ -563,10 +563,12 @@ void Measure::layoutMeasureNumber()

unsigned nn = 1;
bool nas = score()->styleB(Sid::measureNumberAllStaves);
Placement placement = Placement(score()->styleI(Sid::measureNumberVPlacement));

if (!nas) {
//find first non invisible staff
for (unsigned staffIdx = 0; staffIdx < _mstaves.size(); ++staffIdx) {
//find first (or last) non invisible staff
for (unsigned i = 0; i < _mstaves.size(); ++i) {
unsigned staffIdx = (placement == Placement::ABOVE ? i : _mstaves.size() - i - 1);
if (visible(staffIdx)) {
nn = staffIdx;
break;
Expand Down

5 comments on commit dbaa726

@MarcSabatella
Copy link

Choose a reason for hiding this comment

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

If we elect to go with a stop-gap solution for measure numbers only as opposed to all system elements, and if we also decide to support only "above top staff" and "below bottom staff" as options rather than allowing control over which staff groups (eg, bracketed) get the numbers, I'd still vote for a new style setting rather than co-opting the existing one. The existing one might well be used by people who want measure numbers below each staff, and in general I just don't think it's a good precedent to have the placement style setting mean something so radically for different for measure numbers than it does for other element types.

@mattmcclinch
Copy link
Owner Author

Choose a reason for hiding this comment

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

This change has no effect on the case where users want measure numbers below each staff, and I do not think that this is giving such a radically different meaning to placement style setting for measure numbers as opposed to other element types. All this does is take the placement style setting into consideration when choosing the one staff upon which to attach the measure numbers. If the user wants measure numbers below the staff, but not below all staves, then he or she most likely wants them below the bottom visible staff. It makes no sense that we are not currently doing this.

@mattmcclinch
Copy link
Owner Author

Choose a reason for hiding this comment

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

By the way, I did consider adding a new style setting or two, but I felt that it would make things unnecessarily complex. Switching from above top staff to below bottom staff would require changing two style settings instead of just one. And the proposed "Above/Below System" as a placement property would conflict with the "number all staves" setting.

@MarcSabatella
Copy link

@MarcSabatella MarcSabatella commented on dbaa726 Jan 11, 2021

Choose a reason for hiding this comment

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

Indeed, a separate setting creates conflicts that are not ideal either. And I acknowledge the proposed change in meaning of placement for measure numbers is maybe not quite so "radically" different, but I do still have concerns that it's a very specialized solution to what is in reality a more general problem. I worry that it creates a certain precedent we then have to somehow figure out to how to incorporate into whatever design we ultimately come up with to allow the user to specify additional staves and also to extend this to other system elements.

However, the practicality of the situation is this: it's basically now or never for this with regard to 3.x, and for 4.0 I think it quite likely this will be the least of our worries with regard to new designs conflicting with old. This change in behavior of placement has the advantage of doing what's desired in the majority of cases and also not requiring new options to document or strings to translate. And of course, we would all like to see the functionality one way or another. I would still want to run this by Martin and/or Simon, but if we're going to have a stop gap solution for 3.6, this is probably the best we can do.

@mattmcclinch
Copy link
Owner Author

Choose a reason for hiding this comment

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

I worry that it creates a certain precedent we then have to somehow figure out to how to incorporate into whatever design we ultimately come up with to allow the user to specify additional staves

I wouldn’t worry about that. This code is about finding exactly one staff in a system to which to attach a measure number. Any design that allows a user to choose multiple staves to which to attach a measure number will by very nature not have to follow this rule.

and also to extend this to other system elements.

If there are other places in the code where something similar is done for other system elements that have a similar style setting, then this absolutely can be done there as well.

Please sign in to comment.