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] Line width fix #7941

Closed
wants to merge 1 commit into from
Closed

[MU3] Line width fix #7941

wants to merge 1 commit into from

Conversation

Eism
Copy link
Contributor

@Eism Eism commented Apr 12, 2021

Changing the staff space in the page settings affects the width of the lines( in the preview, see screenshot).
It's pretty weird. It was added in this commit 6eb8dd2 as followup to PR #6315

@MarcSabatella, can you explain to me what problem you were solving?

image

@MarcSabatella
Copy link
Contributor

MarcSabatella commented Apr 12, 2021

Prior to this fix, line segments would not be re-scaled if you change staff space setting. So reverting the change breaks that again. I'm not understanding why you would want to do that. Also, for me right now the score preview looks perfect. Maybe there is something unusual going on with the particular test score you are trying this with? Is there a bug report somewhere about this?

To be clear, before my change - and therefore also with your PR - there is a serious bug with lines reproducible as follows:

  1. default empty score
  2. add staff text line to first measure
  3. change staff space from 1.750 to 0.950 in page settings

Result: line with is now over twice as thick as it should be - instead of 0.11sp, it's 0.27sp

I'm unable to reproduce anything like the picture you show of the page settings dialog, with or without the change.

Hmm, well, I'm talking about 3.6.2. Maybe something else broke this since 3.6.2? But I can't reproduce any problem using either 3.6.2 or a 3.x nightly from a couple of days ago.

@Eism
Copy link
Contributor Author

Eism commented Apr 13, 2021

@MarcSabatella, thanks for the explanation!

The bug is reproduced when the line is stretched over several measures so that it is split into several segments. Like on the attached screenshot

Screenshot

image

Each segment(LineSegment) has the same line and when the spatium changes, each segment increases or decreases the width of this line, i.e. for the same line we change the width several times

And if we are working not with previews but directly with the notation, then this is a serious bug.

However, you can reproduce the bug on the attached score. Just change staff space from 1.750 to 3.550 in page settings

Line_Width_bug.zip

I don't know how best to fix the bug.
Maybe we can determine that we have already updated the width?
Or not update the width, but take into account the spatium when drawing the line directly?

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Apr 13, 2021

I can see a pretty fat red dashed line in the preview part of the page settings dialog:
image
but only there, not in the score, nor in navigator. And even in the page setting preview part it is gone when opening it again:
image
It is a bug for sure, but is it one worth fixing for 3.x? Certainly not at the cost of reintroducing that scaling bug in scores.

@MarcSabatella
Copy link
Contributor

MarcSabatella commented Apr 13, 2021

Interesting, indeed with this particular score I see the problem in the preview, but not in the actual score after making the change. The fact that it only happens with multi-segment lines suggests to me the issue is that the calculation is being performed too often. This function is called for each and every segment of the line, so the scaling ends up being multiplied many times over.

The "quick fix" here is to only do this particular part of the scaling once - I guess for the first segment. What's strange to me is that the same issue does not happen for the actual score. I can see in the debugger that this same thing is occuring, and yet somehow, we don't end up with the wrong answer in the end. No idea why actually, which does bother me a bit.

But anyhow, a possible solution seems simple if not especially elegant:

  if (this == line()->frontSegment())
        line()->setLineWidth(line()->lineWidth() * scale);

Not scaling at all would be an option if we adjusted the drawing but also the read/write to account for compatibility. Probably would also need to consider how the propert gets reported in plugins, etc. Chance of unforeseen side effects seems very high.

@vpereverzev
Copy link
Member

no more actual for backend

@Eism Eism deleted the line_fix branch January 9, 2023 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants