#3692 Fingering collision with accidentals

[Anonymous]

http://codereview.appspot.com/341470043

Originally created by: *anonymous

Originally created by: ColinPKC...@gmail.com

Fingerings do not take accidentals into account, and fingerings align over the first note inout of a chord, rather than centering over the whole chord.
\version "2.17.96"

{
  <e'' f'' cis'''>1 ^1^2^5
}

From Keith Ohara:
"Unfortunately this is a different bug -- or two -- so it will
not be fixed at the same time as collisions between Fingerings.

(1) LilyPond has never looked at Accidentals when placing Fingering;
until recently that caused only small overlaps with sharps because
the Fingering did not previously fit so close to the chord.

(2) Fingering on a chord as a whole should center over the main column
of note-heads; instead it centers over the first-input note.

So, you can do the same as we did when using version 2.12: rearrange
the order of pitches in the chord so a main-column note comes first.

{ <f'' e'' cis'''>1 ^1^2^5 }

***********
From Janek :

This is similar to issues
https://code.google.com/p/lilypond/issues/detail?id=2245
https://code.google.com/p/lilypond/issues/detail?id=2451
https://code.google.com/p/lilypond/issues/detail?id=2452

and i believe to solve it in a general way we would have to teach
lilypond about different kinds of extents - see:

http://lists.gnu.org/archive/html/lilypond-devel/2012-06/msg00230.html
https://code.google.com/p/lilypond/issues/detail?id=3239 (sorry, that
issue got pretty messy).

Currently the work i started on this is waiting until 2.18.0 is out...
What about leaving it for now and attacking this together after 2.18?
************
Possibly a Known Issue in NR 1.7.1 would be in order.

[Torsten Hämmerle]

Chord fingering

Many issues related to fingering positioning and collisions are caused by incorrect input.
A fingering number should always (!) be attached to a specific note, never to a chord in general.

Therefore, the coding used in this issue is not supposed to work anyway.
Instead of

{
  <e'' f'' cis'''>1 ^1^2^5
}

this should be input as

{
  <e''^1 f''^2 cis'''^5>1
}

in the first place.

Cause of collision

Currently, the New_fingering_engraver does not take care of accidentals for up/down fingerings at all. As all the chord's accidentals may get into the way (at least for chords containing intervals of a second, i.e. if there are noteheads on both sides of the stem).

Proposed Solution

Make all the accidentals known to the fingering side-positioning interface.
The attached image illustrates the result before and after the correction.
Noteheads, accidentals and fingerings are coloured to show the elements belonging together.
As you can see, the 1 fingering collides with the accidental of another note, so all the accidentals should be known in order to be avoided.

I'm either waiting for issue 5393 or extend this issue by adding left-alinged fingering collisions with dots.

All the best,
Torsten

[Torsten Hämmerle]

Sorry, change of mind:

Up/down fingerings should actually be aligned on top of each other in one single column. The solution presented above avoids collisions, but still is incorrect.
So, waiting for issue 5393...

[David Kastrup]

"Torsten Hämmerle" be-3@users.sourceforge.net writes:

Chord fingering

Many issues related to fingering positioning and collisions are caused by incorrect input.
A fingering number should always (!) be attached to a specific note, never to a chord in general.

Therefore, the coding used in this issue is not supposed to work anyway.
Instead of
~~~
{
<e'' f''="" cis'''="">1 ^1^2^5
}

Uh, says who? The Fingering_engraver goes to quite a bit of pain to
preserve stacking order for this case. If you want to get a single
number stack outside of the staff, that is the way to get it.

~~~
this should be input as
~~~
{
<e''^1 f''^2="" cis'''^5="">1
}
~~~
in the first place.

Sorry, but you are just making up things here. LilyPond has two
different engravers for fingerings exactly to be able to meet those two
different fingering placement requirements.

It may well be that one has bugs where the other doesn't, but that does
not magically make it non-existent.

--
David Kastrup

[Torsten Hämmerle]

Hi David and Harm,

You're right, I'm probably making this up.
But I haven't found a single example in Learning Manual, Notation Manual, where chord fingerings outside the chord. Besides, fingeringOrientations will only work for fingerings within <>.

OK, both possibilities can and will be used, I'll have to agree.

All the best,
Torsten

[David Kastrup]

"Torsten Hämmerle" be-3@users.sourceforge.net writes:

Hi David and Harm,

You're right, I'm probably making this up.
But I haven't found a single example in Learning Manual, Notation
Manual, where chord fingerings outside the chord.

Documentation/notation/simultaneous.itely: <a' c="" e="">1\p^"text" q2\<( q8)[-! q8.]! q16-1-2-3 q8\prall

Which is a single example. Admittedly, the only one I found with a
simple regex.

Besides, fingeringOrientations will only work for fingerings within
<>.

Well, yes. Per-chord fingerings make a single stack which would be
tricky to combine with fingeringOrientations. The two fingering
engravers are not feature equivalent and very likely not bug equivalent
either.

--
David Kastrup

[Thomas Morley]

I haven't found a single example in Learning Manual, Notation Manual, where chord fingerings outside the chord.

This sounds like a documentation-issue: if we provide a functionality we should document it ofcourse.

Besides, fingeringOrientations will only work for fingerings within <>.

Indeed.
It's a feature having those two engravers, see:

{
  <c'-4 e'-3>1-1-2
  %% in-chord-fingering is not affected
  \once \override Fingering.direction = #DOWN
  <c'-4 e'-3>-1-2
  %% general chord fingering is not affected
  \set fingeringOrientations = #'(left)
  <c'-4 e'-3>1-1-2
}

Otoh, sometimes it fails like a pain in the neck having two engravers for fingerings.
Furthermore one of them is for other stuff as well...
a real mess.

Though, as said before, I'd love to get the possibility to have StrokeFingers attached to a whole chord working as well.
P.e., look at
http://lsr.di.unimi.it/LSR/Item?id=409
There the defined strokes have to be placed inside of the chord. It would be much cleaner, if they could be attached to the chord.
Ofcourse, this is a different issue.

[Thomas Morley]

A fingering number should always (!) be attached to a specific note, never to a chord in general.

Why do you think so?
Recently I've typeset a flamenco piece, where I rather wished my right hand fingerings (indicating strokes of one finger for the whole chord) would have been accepted by a chord in general.
Adding it to a specific note in the chord is ofcourse possible, but against the musical sense.
Instead I used TextScript for those indications, but TextScript doesn't support 'add-stem-support, afaict. So the result is sub-optimal.
Admittedly this is about StrokeFingers, but there may be the case where you want to demonstrate alternative fingerings. Which is very conveniant using both fingerings to every note and to the chord in general.
Maybe other use-cases are possible as well...

Additionally I made some experiments resetting Fingerings x-parent to the NoteColumn via ScriptColumn.
It slows down compilation (at least when integrating StringNumber and StrokeFinger). So it might not be the best approach. Nevertheless I'll post it below, so everyone can watch the results without recompiling the source.

#(define RH rightHandFinger)
chrdI = <e'' f'' cis'''>1.-1-2-5 \1\2\3 -\RH #1 \RH #2 -\RH #3 -.--
chrdII = <e''-1\1-\RH #1 f''-2\2-\RH #2 cis'''-5\3-\RH #3 > -.--
%% comment next line to test StringNumbers and StrokeFingers as well
chrdII = <e''-1 f''-2 cis'''-5 > -.--

mus = {
  <>^"chord fingering"
  \chrdI
  <>^"in-chord fingerings"
  \set fingeringOrientations = #'(up)
  \set stringNumberOrientations = #'(up)
  \set strokeFingerOrientations = #'(up)
  \chrdII
  \set fingeringOrientations = #'(down)
  \set stringNumberOrientations = #'(down)
  \set strokeFingerOrientations = #'(down)
  \chrdII
  \set fingeringOrientations = #'(left)
  \set stringNumberOrientations = #'(left)
  \set strokeFingerOrientations = #'(left)
  \chrdII
  \set fingeringOrientations = #'(right)
  \set stringNumberOrientations = #'(right)
  \set strokeFingerOrientations = #'(right)
  \chrdII
}

#(define set-certain-scripts-x-parents-to-nc
  (lambda (grob)
    (let* ((scripts (ly:grob-object grob 'scripts))
           (fingerings
             (if (null? scripts)
                 '()
                 (filter
                   (lambda (f)
                     (or
                       (grob::has-interface f 'finger-interface)
                       (grob::has-interface f 'stroke-finger-interface)
                       (grob::has-interface f 'string-number-interface))
                       )
                   (ly:grob-array->list scripts))))
           (x-par (ly:grob-parent grob X))
           (nc 
             (filter
               (lambda (nc)
                 (grob::has-interface nc 'note-column-interface))
               (ly:grob-array->list (ly:grob-object x-par 'elements)))))

    (for-each
      (lambda (f) (ly:grob-set-parent! f X (car nc)))
      fingerings))))

resetCertainScriptsParents =   
\override Staff.ScriptColumn.after-line-breaking = 
  #set-certain-scripts-x-parents-to-nc

{
  \time 3/2
  \mus
  \break
  \resetCertainScriptsParents
  \mus
}    

Up/down fingerings should actually be aligned on top of each other in one single column. The solution presented above avoids collisions, but still is incorrect.

Agreed.

[Torsten Hämmerle]

issue 3692: Fingering collision with accidentals

Changes to be committed:

modified: ../Documentation/changes.tely
- Announcing the new X-align-on-main-noteheads feature
for the New_fingering_engraver

new file: ../input/regression/fingering-adjacent-note-chord-new.ly
new file: ../input/regression/fingering-adjacent-note-chord.ly
- Adjacent-note chord regression tests for New_fingering_engraver
and Fingering_engraver.

modified: ../lily/fingering-engraver.cc
- Instead of aligning the fingering on the first-to-come notehead of the
chord, now use note-column as parent and take advantage of the
side-position-interface's X-align-on-main-noteheads functionality.
- The (sometimes) incorrect alignment caused by simply using the the
first chord note entered as parent was, after all, the actual reason
for the accidental collision that never had happened with a proper
fingering alignment.

modified: ../lily/new-fingering-engraver.cc
- Making New_fingering_engraver consider the X-align-on-main-noteheads
property and use note-column as parent for up/down orientations. That
way, up/down fingerings will be aligned in a straight column, properly
centered over the main noteheads.
- With "classic" per-note alignment, use notehead as parent (as it had
been always done), but now avoid all accidentals in the chord for
up/down placement.
- If there's only one note, no special alignment/accidental treatment
needed.
- accidentals_.clear () had been missing after position_all (), so that
more and more accidentals gathered up from chord to chord.
- By default, X-align-on-main-noteheads is not set to keep everything
compatible to the previous practice.
- When setting X-align-on-main-noteheads and using just up/down orientation,
New_fingering_engraver behaves like Fingering_engraver.

http://codereview.appspot.com/341470043

[Torsten Hämmerle]

Fingering_engraver

The fingering/accidental collision bug remedy is not to avoid the accidental, but to properly align the fingering (column) on the "correct" side of the stem, i.e. on the "main noteheads".
This can be accomplished by using the note-column as parent and taking advantage of the X-align-on-main-noteheads property of the side-position-interface. Remark: it wasn't a viable option to just "pick" a notehead on the correct side of the stem, because the stem direction is not yet known when processing the fingering.
The X-align-on-main-notehead property will be set directly from the coding and completely ignore the actual Fingering property value because there's no discussion about the correct alignment and I needed this property for the New_fingering_engraver, see below.

Workaround for the time being: this issue's bone of contention can be manually resolved by just swapping the first two notes in the chord, so that the fingering column will be aligned on the correct side of the stem: Voilà !

{
  <f'' e'' cis'''>1 ^1^2^5
}

New_fingering_engraver

The New_fingering_engraver now eventually takes chord accidentals into account to avoid collisions.
If it were up to me, however, I'd much prefer a straight-column (with correct alignment) for up/down orientation even for the New_fingering_engraver (and Harm seems to agree, if I got him correctly).

Therefore, the New_fingering_engraver's behaviour now can be switched by setting X-align-on-main-hoteheads property to ##t.
I did not set this as a default in scm/define-grobs.scm for Fingering/StringNumber/StrokeFinger to keep up the standard behaviour of New_fingering_engraver.

As an attachment, I have created a PNG file comparing the regtest differences (including this issue's new regtests) between the old/new behaviour and to demonstrate the new X-align-on-main-noteheads option of the New_fingering_engraver.

All the best,
Torsten

[Peter Toye]

Is there a typo? Is the property X-align-on-main-hoteheads or ....noteheads? The latter seems more probable! but it has also migrated onto the PNG file.

[Torsten Hämmerle]

Yes, thanks, Peter, it is a typo - but only in the markup text, so it's rather cosmetic and does not affect the functionality.
I noticed it just seconds after uploading the patch and have already corrected it in my local branch.

Thanks for your attention and alertness,
Torsten

[Anonymous]

Passes make, make check and a full make doc.

[Anonymous]

Patch on countdown for Sept 13th

[Anonymous]

Patch counted down - please push

[Torsten Hämmerle]

Hi James,

patch against current master attached. Please push it for me.

Thanks,
Torsten

[Anonymous]

author  Torsten Hämmerle <torsten.haemmerle@web.de> 
    Thu, 30 Aug 2018 10:37:08 +0100 (11:37 +0200)
committer   James Lowe <pkx166h@runbox.com> 
    Sun, 16 Sep 2018 15:14:21 +0100 (15:14 +0100)
commit  c8ebee1b3eee8a5f53191dfc7c58cca4f110f677

Thank you Torsten.