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

System brackets messed up in horizontal continuous view #18798

Closed
mmeyn opened this issue Jul 25, 2023 · 10 comments · Fixed by #19316
Closed

System brackets messed up in horizontal continuous view #18798

mmeyn opened this issue Jul 25, 2023 · 10 comments · Fixed by #19316
Assignees
Labels
engraving P3 Priority: Low

Comments

@mmeyn
Copy link

mmeyn commented Jul 25, 2023

Issue type

Engraving bug

Bug description

In the attached score the system brackets are messed up when I change to horizontal continuous view: some bracket tips are inverted, some brackets span the wrong systems. Vertical continuous and page view are fine.

I tried, without success, to reproduce this problem with a newly created score.

Steps to reproduce

  1. Rename the attached file (.zip → .mscz) and open it.
  2. Look at the score in horizontal continuous view.
    Brackets_continuous_view.zip

Screenshots/Screen recordings

horizontal continuous:
horizontal_continuous

vertical continuous:
vertical_continuous

MuseScore Version

MuseScore version (64-bit): 4.1.0-231921359, revision: github-musescore-musescore-2e3a93a

Regression

I don't know

Operating system

OS: Manjaro Linux, Arch.: x86_64

Additional context

No response

@mmeyn

This comment was marked as resolved.

@mmeyn

This comment was marked as resolved.

@oktophonie oktophonie added needs info More information is required before action can be taken P3 Priority: Low labels Jul 25, 2023
@oktophonie oktophonie added this to To do in 4.x SHORTLIST via automation Jul 25, 2023
@zacjansheski
Copy link
Contributor

This is pretty interesting. Brackets get this way from showing and hiding instruments. I can reproduce by hiding/showing instruments at the ends of the brackets

video1618192902.mp4

@zacjansheski
Copy link
Contributor

In your score, there are many invisible instruments, but the main source of the weirdness is caused by the invisible tuba (bottom of the second bracket

video1581044734.mp4

@oktophonie oktophonie removed the needs info More information is required before action can be taken label Jul 25, 2023
@oktophonie oktophonie assigned mike-spa and unassigned zacjansheski Jul 25, 2023
@mmeyn
Copy link
Author

mmeyn commented Jul 25, 2023

Thanks for looking into the score. I must admit I had totally forgotten about the hidden instruments. You can reproduce this bug with any score that contains such brackets:

When you hide an instrument that is at the top or bottom end of a bracket the corresponding bracket end is set to the top of the score. This means that what looks like a woodwind bracket with inverted tips in my score is in fact the brass bracket:
• Because the contrabassoon at the bottom end of the woodwind bracket is missing, this bracket spans from the top end of the woodwinds to the top end of the score and therefore doesn’t show at all.
• Because the tuba at the bottom end of the brass bracket is missing, this bracket has its “upper” end at Horn 1 and its “lower” end at the top of the score. This explains the weird inverted tips.

The same thing explains the weird square bracket that spans all the woodwinds: It’s the horns bracket that is messed up because Horn 4 is hidden.

@mmeyn
Copy link
Author

mmeyn commented Aug 5, 2023

May I try this?

What I found out so far:

  • src/engraving/rendering/dev/systemlayout.cpp line 1968 and 1973: show() is always true for staves in horizontal continuous view.
  • I suppose this is because for horizontal view ScoreLayout::collectLinearSystem is called instead of SystemLayout::collectSystem and the former doesn’t call SystemLayout::hideEmptyStaves.
  • Calling hideEmptyStaves(system, ctx, true) in ScoreLayout::collectLinearSystem (after making it public for testing purposes) “fixes” the issue.

What’ I haven’t found out yet:

  • Which parts of the hideEmptyStaves code are needed to fix this issue. I think one would only want to hide staves (empty or not) that are hidden by the user in the instruments panel but I don’t know yet how that translates to code.

@cbjeukendrup
Copy link
Contributor

There is Staff::show() and SysStaff::show(); Staff represents the staff in the whole score, and is affected by the settings in the instruments panel, while SysStaff represents a staff in a system, and is affected by the layout calculations.

@oktophonie oktophonie assigned mmeyn and unassigned mike-spa Aug 6, 2023
@mmeyn
Copy link
Author

mmeyn commented Aug 10, 2023

What I haven’t found out yet:

  • Which parts of the hideEmptyStaves code are needed to fix this issue. I think one would only want to hide staves (empty or not) that are hidden by the user in the instruments panel but I don’t know yet how that translates to code.

After reading and understanding what this does (see also this thread) I think everything is needed.

What I found out so far:

  • […]
  • Calling hideEmptyStaves(system, ctx, true) in ScoreLayout::collectLinearSystem (after making it public for testing purposes) “fixes” the issue.

This would be a very simple fix which has only one problem: that method is private. Possible fixes:

  1. make it public
  2. move it to another place where both SystemLayout and ScoreLayout can access it
  3. make friends

As I’m new to the MuseScore code and have never used friend (I grepped for it and saw that it is used in some places) I’d need help with option 2. or 3.

@cbjeukendrup
Copy link
Contributor

cbjeukendrup commented Aug 23, 2023

I think it's fine to make the method public, since there are more methods in SystemLayout that are public and used in ScoreLayout.

But I had another thought about the desired behaviour: in continuous view, I think only staves that are explicitly hidden by the user (via the Instruments panel) should be hidden. Even when "hide empty staves" is enabled and a staff is empty, I think it should still appear in (horizontal) continuous view. This way, the user can use continuous view to enter notes on those staves, without having to disable and re-enable "hide empty staves".

@mmeyn
Copy link
Author

mmeyn commented Aug 24, 2023

I think it's fine to make the method public, since there are more methods in SystemLayout that are public and used in ScoreLayout.

OK, thanks for that hint.

But I had another thought about the desired behaviour: in continuous view, I think only staves that are explicitly hidden by the user (via the Instruments panel) should be hidden. Even when "hide empty staves" is enabled and a staff is empty, I think it should still appear in (horizontal) continuous view. This way, the user can use continuous view to enter notes on those staves, without having to disable and re-enable "hide empty staves".

This was my first thought too. Then I felt like this would be an unnecessary limitation of the user’s possibilities. Other arguments for just using hideEmptyStaves(system, ctx, true):

  1. When using MuseScore’s default settings, nothing is hidden, because the horizontal continuous view is treated like the first system (that’s the true in the call). The user would have to set “hide empty staves in the first system”.
  2. Copying large parts of that method might be a source for errors in the present and also in the future when something is changed.

mmeyn added a commit to mmeyn/MuseScore that referenced this issue Sep 7, 2023
@cbjeukendrup cbjeukendrup removed this from To do in 4.x SHORTLIST Sep 14, 2023
cbjeukendrup added a commit that referenced this issue Sep 14, 2023
Fix #18798: corrects system bracket display in continuous view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engraving P3 Priority: Low
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

6 participants