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

Bug: bottom beam height is not correctly calculated #5

Open
EpsilonD3lta opened this issue Dec 8, 2024 · 3 comments
Open

Bug: bottom beam height is not correctly calculated #5

EpsilonD3lta opened this issue Dec 8, 2024 · 3 comments

Comments

@EpsilonD3lta
Copy link

EpsilonD3lta commented Dec 8, 2024

Consider system

M:4/4
L:1/4
V:1
V:2 clef=bass
K:C
[V:1] z1/2z1/2z1/2[EAC]1/2[EF]1/2[EF]1/2z |
[V:2] z1/2z1/2z1/2[E,A,C]1/2[G,A,]1/2[G,A,]1/2z|

Result:
image

If I change

stemHeight = Mathf.Abs(stemHeight - stemPos.y);

to

else if (beam.stemHeight != Beam.unspecifiedStemHeight)
{
    // beam contains notes at different heights, used the calculated stem height
    stemHeight = Mathf.Abs(stemHeight);
}

then it changes to
image
Not sure if the Mathf.Abs is needed there anymore. Also, simple notes do not seem to have this problem. I tested various systems after this change and it seems to work correctly now (although bottom stems are tiny bit longer).
I am not going to create PR because I have too many modifications.

@matthewcpp
Copy link
Owner

matthewcpp commented Dec 9, 2024

Hey, thanks for submitting a fix for this issue! I did a little digging and it looks like the height issue you mentioned can be fixed by changing the line in question to:

else if (beam.stemHeight != Beam.unspecifiedStemHeight)
{
    // beam contains notes at different heights, used the calculated stem height
    stemHeight = Mathf.Abs(stemHeight - Beam.beamHeight);
}

Note: this will require you to make Beam.beamHeight public.

image

All the other test renders look good. That being said, I dont think the rendering in this case is 100% correct. The middle stem height should be extended to create a linear beam. I will look into that and see if I can come up with a fix, and get this merged into main

@EpsilonD3lta
Copy link
Author

EpsilonD3lta commented Dec 9, 2024

Thanks for the reply. Bit of a side question, why don't beams use LineRenderer but custom procedural mesh? Was it problematic or any other particular reason? I want to change the colors of the beams (gradient), so instead of implementing custom vertex coloring I might as well rewrite it to LineRenderer, so I am asking if you stumbled on some problems.

Edit: nevermind, I realized that LineRenderer won't work so easily, because it has slanted ends.

@EpsilonD3lta
Copy link
Author

EpsilonD3lta commented Dec 13, 2024

I found out that the line should probably look like this
stemHeight = Mathf.Abs(beam.stemHeight - stemPos.y);
Similarly for the chords pointing upwards. This fixes almost all cases, however some still remain.
Consider system:

L:1/4
M:C
V:1
V:2 clef=bass
K:C
[V:1] z4 | z4 | z4 |
[V:2] [B,DF]/2D,/2B,/2 z2 z/2 | [A,^CE]/2D,/2[C,D,]3/4 z2 z/4] | D,/2[A,^CE]/4 z/4 z/2 z2 z/2] |

Result
image
The middle beam is slightly bent, and the right secondary beam is completely wrong.

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

No branches or pull requests

2 participants