note-collision: rework horizontal shifts; issue 1774,2196

Also allow duplicate 'horizontal-shift's without the "too many clashing
note-columns" warning.

8 files changed, 113 insertions, 78 deletions

diff --git a/Documentation/learning/fundamental.itely b/Documentation/learning/fundamental.itely
index c64af67ed4..bcf449f276 100644
--- a/Documentation/learning/fundamental.itely
+++ b/Documentation/learning/fundamental.itely
@@ -801,7 +801,7 @@ later sections.
   \\  % Voice two
     {
       % Ignore these for now - they are explained in Ch 4
-      \once \override NoteColumn.ignore-collision = ##t
+      \once \override NoteColumn.force-hshift = #0
       <ees, c>2
       \once \override NoteColumn.force-hshift = #0.5
       des2
@@ -851,7 +851,7 @@ not understand.
   \\  % Voice two
     { \voiceTwoStyle
       % Ignore these for now - they are explained in Ch 4
-      \once \override NoteColumn.ignore-collision = ##t
+      \once \override NoteColumn.force-hshift = #0
       <ees, c>2
       \once \override NoteColumn.force-hshift = #0.5
       des2
diff --git a/Documentation/notation/simultaneous.itely b/Documentation/notation/simultaneous.itely
index c139853fc1..70846342e7 100644
--- a/Documentation/notation/simultaneous.itely
+++ b/Documentation/notation/simultaneous.itely
@@ -290,9 +290,8 @@ interpreted in different voices.
 @cindex collisions, ignoring
 
 @knownissues
-If notes from two or more voices, with stems in the same direction, are
-placed at the same position on the staff and have no shift (or have the
-same shift specified), the message:
+If notes from two or more voices, with no shifts specified,
+have stems in the same direction, the message
 
 @example
 warning: ignoring too many clashing note columns
diff --git a/Documentation/snippets/new/suppressing-warnings-for-clashing-note-columns.ly b/Documentation/snippets/new/suppressing-warnings-for-clashing-note-columns.ly
new file mode 100644
index 0000000000..cf6fe935d5
--- /dev/null
+++ b/Documentation/snippets/new/suppressing-warnings-for-clashing-note-columns.ly
@@ -0,0 +1,27 @@
+\version "2.19.14"
+
+\header {
+  lsrtags = "simultaneous-notes, tweaks-and-overrides"
+
+  texidoc = "
+If notes from two voices with stems in the same direction are placed at
+the same position, but the voices have no voice-specific shifts
+specified, the error message @samp{warning: ignoring too many clashing
+note columns} will appear when compiling the LilyPond file.  This
+message can be suppressed by setting the @code{'ignore-collision}
+property of the @code{NoteColumn} object to @code{#t}. Please note that
+this does not just suppress warnings  but stops LilyPond trying to
+resolve collisions at all and so may have  unintended results unless
+used with care.
+"
+  doctitle = "Suppressing warnings for clashing note columns"
+}
+
+ignore = \override NoteColumn.ignore-collision = ##t
+
+\relative c' {
+  \new Staff <<
+    \new Voice{ \ignore \stemDown f2 g }
+    \new Voice{ c2 \stemDown c, }
+  >>
+}
diff --git a/input/regression/beam-multiple-cross-staff.ly b/input/regression/beam-multiple-cross-staff.ly
index 7827e3d723..0c382b2f76 100644
--- a/input/regression/beam-multiple-cross-staff.ly
+++ b/input/regression/beam-multiple-cross-staff.ly
@@ -14,17 +14,17 @@ to the last beam.
 
 \context PianoStaff \relative c' <<
   \new Staff = SA {
-    \stemDown
-    c8[ c16 \change Staff = SB \stemUp c16 ]
-    \stemNeutral
+    \voiceTwo
+    c8[ c16 \change Staff = SB \voiceOne c16 ]
+    \oneVoice
     f[ g \change Staff = SA a c] 
   }
   \new Staff = SB \relative c' {
     \clef bass
-    b8[ b16 \stemUp b,,16 ]
+    b8[ b16 \voiceOne b,,16 ]
 
     g'4\rest
-    b,8[ b16 \stemDown b''16 b ]
+    b,8[ b16 \voiceTwo b''16 b ]
   }
   
 >>
diff --git a/input/regression/collisions.ly b/input/regression/collisions.ly
index c94254378b..5b03483b6b 100644
--- a/input/regression/collisions.ly
+++ b/input/regression/collisions.ly
@@ -39,6 +39,24 @@ hairyChord = \context Staff  \relative c''
   ais
 >>
 
+minims = <<
+  { e''2 e'' e' d' } \\
+  c'' \\
+  { c' c' c' c' } \\
+  g' >>
+
+semibreves = <<
+  { e''1 e'' e' d' } \\
+  c'' \\
+  { c' c' c' c' } \\
+  g' >>
+
+sequence = <<
+  \new Voice { \voiceOne g''1 e''2 d'' e'' d'' }
+  \new Voice { \voiceThree c''2 b' a'1 b' }
+  \new Voice { \voiceFour g'1 e'2 f'2 a'1 }
+  \new Voice { \voiceTwo c'1 c'2 d' e'2 d' }
+>>
 
   {
     \transpose c c' {
@@ -47,6 +65,9 @@ hairyChord = \context Staff  \relative c''
       \threevoice
     }
     \hairyChord
+    \break
+    \minims \bar "||"
+    \semibreves \bar "||"
+    \sequence
   }
-  
 
diff --git a/input/regression/stem-tremolo-note-collision.ly b/input/regression/stem-tremolo-note-collision.ly
index 6b5b32a93c..b7b2142849 100644
--- a/input/regression/stem-tremolo-note-collision.ly
+++ b/input/regression/stem-tremolo-note-collision.ly
@@ -6,8 +6,6 @@ best as possible and issue a warning otherwise.
 "
 }
 
-#(ly:expect-warning (_ "ignoring too many clashing note columns"))
-
 {
 <<
    {  b'4 f'2. }
@@ -17,4 +15,4 @@ best as possible and issue a warning otherwise.
      \repeat tremolo 32 <b g'>32
    }
 >>
-}
\ No newline at end of file
+}
diff --git a/lily/note-collision.cc b/lily/note-collision.cc
index 79a6584575..0008bad23a 100644
--- a/lily/note-collision.cc
+++ b/lily/note-collision.cc
@@ -33,22 +33,14 @@
 #include "stem.hh"
 #include "warn.hh"
 
-void
+Real
 check_meshing_chords (Grob *me,
-                      Drul_array<vector<Real> > *offsets,
-                      Drul_array<vector<Slice> > const &extents,
-                      Drul_array<vector<Grob *> > const &clash_groups)
+                      Grob *clash_up, Grob *clash_down)
 
 {
-  if (!extents[UP].size () || !extents[DOWN].size ())
-    return;
-
-  Grob *clash_up = clash_groups[UP][0];
-  Grob *clash_down = clash_groups[DOWN][0];
-
   /* Every note column should have a stem, but avoid a crash. */
   if (!Note_column::get_stem (clash_up) || !Note_column::get_stem (clash_down))
-    return;
+    return 0.0;
 
   Drul_array<Grob *> stems (Note_column::get_stem (clash_down),
                             Note_column::get_stem (clash_up));
@@ -65,7 +57,7 @@ check_meshing_chords (Grob *me,
 
   /* Too far apart to collide. */
   if (ups[0] > dps.back () + 1)
-    return;
+    return 0.0;
 
   /* If the chords just 'touch' their extreme noteheads,
      then we can align their stems.
@@ -359,11 +351,7 @@ check_meshing_chords (Grob *me,
         }
     }
 
-  for (UP_and_DOWN (d))
-    {
-      for (vsize i = 0; i < clash_groups[d].size (); i++)
-        (*offsets)[d][i] += d * shift_amount;
-    }
+  return shift_amount;
 }
 
 MAKE_SCHEME_CALLBACK (Note_collision_interface, calc_positioning_done, 1)
@@ -471,37 +459,11 @@ SCM
 Note_collision_interface::automatic_shift (Grob *me,
                                            Drul_array<vector<Grob *> > clash_groups)
 {
-  Drul_array < vector<int> > shifts;
   SCM tups = SCM_EOL;
 
-  for (UP_and_DOWN (d))
-    {
-      vector<int> &shift (shifts[d]);
-      vector<Grob *> &clashes (clash_groups[d]);
-
-      for (vsize i = 0; i < clashes.size (); i++)
-        {
-          SCM sh
-            = clashes[i]->get_property ("horizontal-shift");
-
-          if (scm_is_number (sh))
-            shift.push_back (scm_to_int (sh));
-          else
-            shift.push_back (0);
-        }
-
-      for (vsize i = 1; i < shift.size (); i++)
-        {
-          if (shift[i - 1] == shift[i])
-            {
-              clashes[0]->warning (_ ("ignoring too many clashing note columns"));
-              return tups;
-            }
-        }
-    }
-
   Drul_array<vector<Slice> > extents;
-  Drul_array<vector<Real> > offsets;
+  Drul_array<Slice> extent_union;
+  Drul_array<vector<Grob *> > stems;
   for (UP_and_DOWN (d))
     {
       for (vsize i = 0; i < clash_groups[d].size (); i++)
@@ -510,10 +472,16 @@ Note_collision_interface::automatic_shift (Grob *me,
           s[LEFT]--;
           s[RIGHT]++;
           extents[d].push_back (s);
-          offsets[d].push_back (d * 0.5 * i);
+          extent_union[d].unite (s);
+          stems[d].push_back (Note_column::get_stem (clash_groups[d][i]));
         }
     }
 
+  Real inner_offset
+    = (clash_groups[UP].size () && clash_groups[DOWN].size ())
+      ? check_meshing_chords (me, clash_groups[UP][0], clash_groups[DOWN][0])
+      : 0.0;
+
   /*
    * do horizontal shifts of each direction
    *
@@ -522,17 +490,46 @@ Note_collision_interface::automatic_shift (Grob *me,
    *  x||
    *   x|
   */
-
+  Drul_array<vector<Real> > offsets;
   for (UP_and_DOWN (d))
     {
-      for (vsize i = 1; i < clash_groups[d].size (); i++)
+      Real offset = inner_offset;
+      vector<int> shifts;
+      for (vsize i = 0; i < clash_groups[d].size (); i++)
         {
-          Slice prev = extents[d][i - 1];
-          prev.intersect (extents[d][i]);
-          if (prev.length () > 0
-              || (extents[-d].size () && d * (extents[d][i][-d] - extents[-d][0][d]) < 0))
-            for (vsize j = i; j < clash_groups[d].size (); j++)
-              offsets[d][j] += d * 0.5;
+          Grob *col = clash_groups[d][i];
+          SCM sh = col->get_property ("horizontal-shift");
+          shifts.push_back (robust_scm2int (sh, 0));
+
+          if (i == 0)
+            offset = inner_offset;
+          else if (shifts[i] == shifts[i - 1])
+            {
+              // Match the previous notecolumn offset,
+              // but warn if the user did not set these equal shifts explictly
+              if (!scm_is_number (sh))
+                col->warning (_ ("ignoring too many clashing note columns"));
+            }
+          else if (extents[d][i][UP] > extents[d][i - 1][DOWN]
+                   && extents[d][i][DOWN] < extents[d][i - 1][UP])
+            offset += 1.0; // fully clear the inner-voice heads
+          else
+            {
+              // check if we cross the inner voice
+              if (d * extents[d][i][-d] >= d * extents[d][i - 1][d])
+                offset += Stem::is_valid_stem (stems[d][i - 1])
+                          ? 1.0 : 0.5;
+              else if (Stem::is_valid_stem (stems[d][i]))
+                offset += 0.5;
+              // check if we cross the opposite-stemmed voices
+              if (d * extents[d][i][-d] < d * extent_union[-d][d])
+                offset = max (offset, 0.5);
+              if (extents[-d].size ()
+                  && extents[d][i][UP] > extents[-d][0][DOWN]
+                  && extents[d][i][DOWN] < extents[-d][0][UP])
+                offset = max (offset, 1.0);
+            }
+          offsets[d].push_back (d * offset);
         }
     }
 
@@ -546,18 +543,9 @@ Note_collision_interface::automatic_shift (Grob *me,
 
       if (dc)
         for (vsize j = i + 1; j < clash_groups[UP].size (); j++)
-          {
-            Grob *stem = Note_column::get_stem (clash_groups[UP][j]);
-            Side_position_interface::add_support (dc, stem);
-          }
+          Side_position_interface::add_support (dc, stems[UP][j]);
     }
 
-  /*
-    Check if chords are meshing
-  */
-
-  check_meshing_chords (me, &offsets, extents, clash_groups);
-
   for (UP_and_DOWN (d))
     {
       for (vsize i = 0; i < clash_groups[d].size (); i++)
diff --git a/lily/stem.cc b/lily/stem.cc
index 0a72fcf019..a809c170df 100644
--- a/lily/stem.cc
+++ b/lily/stem.cc
@@ -835,6 +835,8 @@ Stem::is_valid_stem (Grob *me)
 {
   /* TODO: make the stem start a direction ?
      This is required to avoid stems passing in tablature chords.  */
+  if (!me)
+    return false;
   Grob *lh = get_reference_head (me);
   Grob *beam = Grob::unsmob (me->get_object ("beam"));