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

fix #41761: beam angle too sharp #1847

Merged
merged 1 commit into from Mar 7, 2015

Conversation

MarcSabatella
Copy link
Contributor

After reading a number of recommendations, I opted for a fairly subtle fix, just limiting the beam slant to 1sp for two-note beams. Gould talks of limiting it further depending on how crowded the spacing is, but that's not easy for us to do.

BTW, while fixing this, I discovered a couple of places where our beams were really strange. The first measure of the vtest I created actually shows cases where, in addition to issues with the beam angle, the stems were too short or two long overall. I think there were just some bad values in the table of beam metrics. I didn't do a super through job of it, but I tried different intervals transposed to different staff lines, and I fixed the most obvious cases.

This is not an area of code I know well, so I definitely would appreciate review, but it does seem to do what it is supposed to do. I tried changing the sixteenth note portion of the code to limit the slant to 0.5sp, but the subsequent code that calculates stem lengths doesn't seem to handle this, so I arrive at the values for minS and maxS I have empirically. I'm less confident about that paet of the change than the eighth note part..

@MarcSabatella
Copy link
Contributor Author

So it turns out the beam mtests are doing their job well - testing lots of combinations of beams and verifying the metrics. I checked them all out by eye and made a few more improvements (I hope) to the table, then updated the tests. So loading the beam tests A-G (which were the ones affected) would be a good way for anyone to check for themselves the different combinations.

@lasconic
Copy link
Contributor

lasconic commented Mar 7, 2015

I cannot reproduce the tests failure if I merge the pr locally, so I wouldn't worry for now.

@MarcSabatella
Copy link
Contributor Author

It works for me locally too. I've been working on the regression from my other change and haven't looked into this anyhow.

lasconic added a commit that referenced this pull request Mar 7, 2015
@lasconic lasconic merged commit 3e750ae into musescore:master Mar 7, 2015
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

2 participants