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

Incorrect beaming when mixing notes and chords and stems are down #161

Closed
arshiacont opened this issue Aug 22, 2022 · 11 comments
Closed

Incorrect beaming when mixing notes and chords and stems are down #161

arshiacont opened this issue Aug 22, 2022 · 11 comments

Comments

@arshiacont
Copy link

Issue when Stems are down only (up is fine).

Example:

{[ \clef<"g2"> \stemsUp \beamBegin:1 {g1/16, e2/16  } b0/16 e1/16 {b1/16, g2/16  } \beamEnd:1 
   \stemsDown \beamBegin:1 {b1/16, g2/16  } e1/16 g1/16 {e2/16, b2/16  } \beamEnd:1 
  ]}

image

@arshiacont
Copy link
Author

@dfober I believe I have a fix for this (and starting to understand the logic in GRBeams). Will PR soon.

In short: It seems like the Polygon points in GRBeam are set only using the first and last events in a Beam. In the above example, the middle events should affect the position of the first point of the polygon (which by itself will lead to new stem lengths etc).

@dfober
Copy link
Member

dfober commented Aug 24, 2022

I have some doubts about this hypothesis : if you replace the last chord {e2/16, b2/16 } with a single note (e2 or b2), the beaming is correct. At the minimum it can be said that the chords interfere with the beaming in a different way.

@arshiacont
Copy link
Author

arshiacont commented Aug 24, 2022

Interesting.. in my fix, once the p0.y is set, I browse associated notes, look at their Stem Start Position and decide whether the initial decision was good or not (based on Direction). This works... but we might just be looking over a bug related to getStemStartPos() (which is the base decision for p0.y).

Or maybe in case of chords the refEvent (which seems to be a SingleNote) is not chosen correctly?!? 🤷🏻‍♂️

@arshiacont
Copy link
Author

am looking at #154 right now which is somehow related to p0 initialisation.. will check this hypothesis

@dfober
Copy link
Member

dfober commented Aug 25, 2022

as far as I remember and in case of chords, a shared stem is used (GRGlobalStem) instead of a simple stem (GRStem). This might be something to check.

@arshiacont
Copy link
Author

You are absolutely right! my initial solution solves this special case but creates regressions elsewhere. Investigating further... .

@arshiacont
Copy link
Author

@dfober I believe I have found the cause:

Chords seem to send isEmpty() as true which messes up with GRBeam::setStemEndPos() that fixes positions.

Is that a bug or is there a reason for this?

This explains why removing the chord at the end makes it work.

@arshiacont
Copy link
Author

I confirm, after testing that the correct solution is to find a suitable way of avoiding empty == true for chords in GRBeam::setStemEndPos().

I can see elsewhere that this is also mentioned in the code (*). How would you suggest avoiding Empty for chords in the context?

(*)

// We try to fix the following problem here: with chord, the start and end

@arshiacont
Copy link
Author

Or maybe simply the current definition of isEmpty used in GRBeam::setStemEndPos() is wrong:

bool empty = (sn->isEmpty() && sn->getDuration());

instead of bool empty = (sn->isEmpty() && !sn->getDuration()); ?

Once again: I confirm that this potentially closes the issue.

@dfober
Copy link
Member

dfober commented Aug 27, 2022

Chord encoding in guido is very special. A chord does not really exist as an object, it is represented in a similar way to gmn syntax, framed by empty, the first one carries the duration of the chord and the last one closes the chord with a zero duration. For example:

{ c, e, g } 

will be represented in AR by

\chordTag \shareLocation \displayDuration \shareStem 
empty*1/4 \chordComma c1*0/1 \chordComma e1*0/1 \chordComma g1*0/1 empty*0/1 

This is why it's necessary to distinguish between 'real' empty and those which frame chords.

Regarding the proposed change with the empty computation, I'll have a look as soon as I'll be back home. Did you checked that it works in any case with the validation tests ?

dfober added a commit that referenced this issue Aug 31, 2022
@dfober
Copy link
Member

dfober commented Aug 31, 2022

fixed.

Or maybe simply the current definition of isEmpty used in GRBeam::setStemEndPos() is wrong:

bool empty = (sn->isEmpty() && sn->getDuration());

instead of bool empty = (sn->isEmpty() && !sn->getDuration()); ?

Was a bit more complex than just changing the test (this was breaking the solved issue #146)

@dfober dfober closed this as completed Aug 31, 2022
arshiacont added a commit to Antescofo/guidolib that referenced this issue Jul 19, 2023
…o-develop

* commit 'd16a038a072e1801841d38acbef03cb0a42df007':
  issue 161
  validation process: add documentation
  fine tuning (multi voices note collisions)
  fix incorrect beaming (issue grame-cncm#161)
  propagate ARNote auto flag to GREmpty
  add auto flag to empty
  manage auto flag for empty notes
  remove useless include
  remove obsolete trajectory proll from validation
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