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

Cross-Staff Beaming #154

Closed
arshiacont opened this issue May 12, 2022 · 17 comments
Closed

Cross-Staff Beaming #154

arshiacont opened this issue May 12, 2022 · 17 comments

Comments

@arshiacont
Copy link

This is an excerpt from Debussy's Children's Corner

{[ \staff<1>  
   (* meas. 1 *)  \clef<"f4"> \key<0> \meter<"C", autoBarlines="off", autoMeasuresNum="system"> 
   \staff<2> \stemsUp  \beamBegin:1 \beamBegin:2  f-1/16 c0/16 d0/16 e0/16 \beamEnd:2 \beamBegin:2 
   \staff<1> \stemsDown f0/16 a0/16 c1/16 d1/16 \beamEnd:1 \beamEnd:2 
   \staff<2> \stemsUp \beamBegin:1 \beamBegin:2 e1/16 
   \staff<1> \stemsDown d1/16 c1/16 a0/16 \beamEnd:2 \beamBegin:2 f0/16 
   \staff<2> \stemsUp e0/16 d0/16 c0/16 \beamEnd:1 \beamEnd:2 ]
 , 
[ \staff<2> 
   (* meas. 1 *) \clef<"f4"> \key<0> \meter<"C", autoBarlines="off", autoMeasuresNum="system"> \beamsOff 
   \ten<position="below">( f-1/1)
 ]
  }

Leads to
image

Whereas it should look like this:
image

Two issues:

  • The obvious beam rendering.. maybe I'm doing sth wrong in the Beam syntax?!
  • The duration value of the note in Voice 2 f-1/1
@arshiacont
Copy link
Author

@dfober any hints where to start chasing this one in the code?

@dfober
Copy link
Member

dfober commented Jun 23, 2022

beaming is computed in GRBeam.cpp, more precisely in GRBeam::tellPosition, the code is really hard to understand (very long methods). I've already add a bit more structure a couple of years ago, but it remains a nightmare.

@arshiacont
Copy link
Author

Just for memo: The problem in this issue is related to the initp0 method called in GRBeam::tellPosition which calculates a yoffset, exactly this block:

the yoffset in this context seems to be wrong.

@arshiacont
Copy link
Author

Slightly more readable test:

{[ \staff<1> 
  \clef<"f4"> \staff<2> 
   \stemsUp \beamBegin:1 \beamBegin:2 f-1/16 c0/16 d0/16 e0/16 \beamEnd:2 \beamBegin:2 
   \staff<1> \stemsDown f0/16 a0/16 c1/16 d1/16 \beamEnd:1 \beamEnd:2 _/2 ]
 , 
[ \staff<2>  \clef<"f4"> \key<0> empty/1 ]
  }

image

@dfober
Copy link
Member

dfober commented Aug 25, 2022

it looks like the beam is drawn in the wrong context e.g. the coordinates are relative to the first staff and it's drawn with the second one.

@arshiacont
Copy link
Author

Thank you @dfober .

It seems like the problematic Beam here is being decoded as smallerBeam in GRBeam. The TellPosition attempts to fix offsets at the end of its function call but they are evaded by this line while treating fSmallerBeam:

What are smallerBeam in this context?!

If I comment that return statement, I get this which is slightly better but still have way to go:

image

@arshiacont
Copy link
Author

@dfober I managed to get the following result by doing two things:

  1. Removing the return statement mentioned above so that the offset of the beam is correctly set
  2. Forcing the right-most beam [for now manually] as a fSmallerBeam of the longest beam (the first beam is included already but not the second) in GRVoiceManager::organizeBeaming(...) .

Whenever you have some time, we can go over the solution since I'm not sure if I understand the intent of fSmallerBeam and to avoid creating regressions. Ping me over email or else.

Once this is done the long beam's slope/initial points can be fixed more easily given the nested beams.

image

@arshiacont
Copy link
Author

I am half-way done!

A particular challenge would be this excerpt where the nested beams (aka fSmallerBeam) are at the system-level:

{[ \staff<1>  
   (* meas. 1 *)  \clef<"f4"> _/2 \staff<2> \stemsUp \beamBegin:1 
   \beamBegin:2 e1/16 \staff<1> \stemsDown d1/16 c1/16 a0/16 \beamEnd:2 
   \beamBegin:2 f0/16 \staff<2> \stemsUp e0/16 d0/16 c0/16 \beamEnd:2 
\beamEnd:1 ]
 , 
[ \staff<2> \clef<"f4">  _/1 ]}

arshiacont added a commit to Antescofo/guidolib that referenced this issue Aug 31, 2022
- Fix nested beaming inclusion in fSmallerBeam iin GRVoiceManager
- Fix parent beam position offset with smaller beams when on the system level
@dfober
Copy link
Member

dfober commented Sep 4, 2022

I've checked the wrong context hypothesis. This is the result when the beam is drawn in the context of the first staff
issue154
The different slopes show also that nested beams are treated separately.
My conclusion at this point is that the whole mechanics would have to be revised to handle nested beams properly.

@arshiacont
Copy link
Author

Interesting.. Is there a branch with your hypothesis?
On my branch I had a quick look at the slope issue for Nested Beam. in the setBeam() function, the "parent" beam can ideally adopt child positions but my conclusion was that it could easily work on this example but we can come up with more complex compound beaming where it wouldn't... .

@dfober
Copy link
Member

dfober commented Sep 7, 2022

No, there is no specific branch. I checked by correcting the y coordinates manually for this specific score (by subtracting the 'y' position of the staff) i.e. GRBeam::OnDraw :

if (st->p[0].y > 380) { ay[0]-=400; ay[1]-=400; ay[2]-=400; ay[3]-=400; };
	// This does the drawing!
	hdc.Polygon(ax, ay, 4);

where 400 is the second staff position().y

@arshiacont
Copy link
Author

The changes I made in this PR actually does this: check changes in GRBeam.cpp: removing the return statement will allow the tellPosition to reach the end where we fix offsets.

On top of the above: changes in GRVoiceManager fixes how beams are nested so that the second beam renders as being nested.

Do you remember which issue was being broken in the regression test?

@dfober
Copy link
Member

dfober commented Sep 13, 2022

Based on your modification, here is the best result I can get (for now and without breaking the tests)
issue154
I find it ambiguous: the second part looks like feathered beams. However I can commit that if it suits you.

@arshiacont
Copy link
Author

hmmmm I guess this is better than what we have. Let's commit this. I will then break this into several tickets as I can see what the problems are.

dfober added a commit that referenced this issue Sep 13, 2022
@dfober
Copy link
Member

dfober commented Sep 13, 2022

that's done (pushed). I'll go on with this issue.

@arshiacont
Copy link
Author

@dfober Here is the unit test that gathers several beaming issues all-together as discussed a few days back:

{[ \staff<1>  
   (* meas. 1 *)  \clef<"f4"> \key<0> 
   \staff<2> \stemsUp  \beamBegin:1 \beamBegin:2  f-1/16 c0/16 d0/16 e0/16 \beamEnd:2 \beamBegin:2 
   \staff<1> \stemsDown f0/16 a0/16 c1/16 d1/16 \beamEnd:1 \beamEnd:2 
   \staff<2> \stemsUp \beamBegin:1 \beamBegin:2 e1/16 
   \staff<1> \stemsDown d1/16 c1/16 a0/16 \beamEnd:2 \beamBegin:2 f0/16 
   \staff<2> \stemsUp e0/16 d0/16 c0/16 \beamEnd:1 \beamEnd:2 ]
 , 
[ \staff<2> 
   (* meas. 1 *) \clef<"f4"> \key<0>
 ]
  }

@dfober
Copy link
Member

dfober commented Jan 5, 2023

fixed (could be improved)

@dfober dfober closed this as completed Jan 5, 2023
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