]> git.donarmstrong.com Git - lilypond.git/commitdiff
Issue 4233: Improve partcombine multi-measure rest handling.
authorDan Eble <nine.fierce.ballads@gmail.com>
Mon, 29 Dec 2014 02:54:52 +0000 (21:54 -0500)
committerDan Eble <nine.fierce.ballads@gmail.com>
Mon, 5 Jan 2015 01:24:41 +0000 (20:24 -0500)
Add "silence1" and "silence2" states which constrain the partcombine
iterator to take events from a specific part during intervals of
silence.  These new states allow the following improvements.

Detect when a multi-measure rest starts in the middle of a
multi-measure rest in the other voice and engrave it as a shared rest.
Previously, the new rest would have been engraved as a voice-specific
rest.

In solo analysis, when a rest and a multi-measure rest begin
simultaneously, engrave the rest as a shared rest and omit the
multi-measure rest.  Previously, both rests would have been engraved
as voice-specific rests.

input/regression/part-combine-mmrest-after-apart-silence.ly
lily/part-combine-iterator.cc
scm/part-combiner.scm

index 0bea1fab535a854c353e60ebd451edbd44185eae..99f1f8f34bf9748e7dd1b4d8565ce87c95e57a52 100644 (file)
@@ -1,18 +1,18 @@
 \version "2.19.16"
 
 \header {
-  texidoc = "A multi-measure rest after apart-silence returns the state to unisilence."
+  texidoc = "Normal rests are preferred over multi-measure rests.  A multi-measure rest beginning in one part in the middle of a multi-measure rest in the other part appears as expected."
 }
 
 \score { <<
   \new Staff {
     \partcombine
-      \relative f' { r2 r2 | R1^"!!!" }
+      \relative f' { r2 r2 | R1 }
       \relative f' { R1*2       }
   }
   \new Staff {
     \partcombine
       \relative f' { R1*2       }
-      \relative f' { r2 r2 | R1_"!!!" }
+      \relative f' { r2 r2 | R1 }
   }
 >> }
index c5abcdcda0d21da1af9b71734c74a481b395c76a..2428d4a70fb41f26bfcbce5cd0e8dcb07c1a818f 100644 (file)
@@ -80,18 +80,26 @@ private:
   {
     APART,
     TOGETHER,
-    SOLO1,
-    SOLO2,
+    SOLO,
     UNISONO,
     UNISILENCE,
   };
   Status state_;
-  Status playing_state_;
 
-  /*
-    Should be SOLO1 or SOLO2
-  */
-  Status last_playing_;
+  // For states in which it matters, this is the relevant part,
+  // e.g. 1 for Solo I, 2 for Solo II.
+  int chosen_part_;
+
+  // States for generating partcombine text.
+  enum PlayingState
+  {
+    PLAYING_OTHER,
+    PLAYING_UNISONO,
+    PLAYING_SOLO1,
+    PLAYING_SOLO2,
+  } playing_state_;
+
+  int last_playing_;
 
   /*
     TODO: this is getting off hand...
@@ -107,7 +115,7 @@ private:
   void solo1 ();
   void solo2 ();
   void apart (bool silent);
-  void unisono (bool silent);
+  void unisono (bool silent, int newpart);
 };
 
 void
@@ -144,8 +152,9 @@ Part_combine_iterator::Part_combine_iterator ()
   horizontalShiftOne_ = scm_from_int (0);
   horizontalShiftTwo_ = scm_from_int (1);
   state_ = APART;
-  playing_state_ = APART;
-  last_playing_ = APART;
+  chosen_part_ = 1;
+  playing_state_ = PLAYING_OTHER;
+  last_playing_ = 0;
 
   busy_ = false;
   notice_busy_ = false;
@@ -234,26 +243,21 @@ Part_combine_iterator::kill_mmrest (int in)
 }
 
 void
-Part_combine_iterator::unisono (bool silent)
+Part_combine_iterator::unisono (bool silent, int newpart)
 {
   Status newstate = (silent) ? UNISILENCE : UNISONO;
 
-  if (newstate == state_)
+  if ((newstate == state_) and (newpart == chosen_part_))
     return;
   else
     {
-      /*
-        If we're coming from SOLO2 state, we might have kill mmrests
-        in the 1st voice, so in that case, we use the second voice
-        as a basis for events.
-      */
-      Outlet_type c1 = (last_playing_ == SOLO2) ? CONTEXT_NULL : CONTEXT_SHARED;
-      Outlet_type c2 = (last_playing_ == SOLO2) ? CONTEXT_SHARED : CONTEXT_NULL;
+      Outlet_type c1 = (newpart == 2) ? CONTEXT_NULL : CONTEXT_SHARED;
+      Outlet_type c2 = (newpart == 2) ? CONTEXT_SHARED : CONTEXT_NULL;
       substitute_both (c1, c2);
-      kill_mmrest ((last_playing_ == SOLO2) ? CONTEXT_ONE : CONTEXT_TWO);
+      kill_mmrest ((newpart == 2) ? CONTEXT_ONE : CONTEXT_TWO);
       kill_mmrest (CONTEXT_SHARED);
 
-      if (playing_state_ != UNISONO
+      if (playing_state_ != PLAYING_UNISONO
           && newstate == UNISONO)
         {
           if (!unisono_event_)
@@ -264,29 +268,31 @@ Part_combine_iterator::unisono (bool silent)
               unisono_event_->unprotect ();
             }
 
-          Context *out = (last_playing_ == SOLO2 ? second_iter_ : first_iter_)
+          Context *out = (newpart == 2 ? second_iter_ : first_iter_)
                          ->get_outlet ();
           out->event_source ()->broadcast (unisono_event_);
-          playing_state_ = UNISONO;
+          playing_state_ = PLAYING_UNISONO;
         }
       state_ = newstate;
+      chosen_part_ = newpart;
     }
 }
 
 void
 Part_combine_iterator::solo1 ()
 {
-  if (state_ == SOLO1)
+  if ((state_ == SOLO) && (chosen_part_ == 1))
     return;
   else
     {
-      state_ = SOLO1;
+      state_ = SOLO;
+      chosen_part_ = 1;
       substitute_both (CONTEXT_SOLO, CONTEXT_NULL);
 
       kill_mmrest (CONTEXT_TWO);
       kill_mmrest (CONTEXT_SHARED);
 
-      if (playing_state_ != SOLO1)
+      if (playing_state_ != PLAYING_SOLO1)
         {
           if (!solo_one_event_)
             {
@@ -298,22 +304,22 @@ Part_combine_iterator::solo1 ()
 
           first_iter_->get_outlet ()->event_source ()->broadcast (solo_one_event_);
         }
-      playing_state_ = SOLO1;
+      playing_state_ = PLAYING_SOLO1;
     }
 }
 
 void
 Part_combine_iterator::solo2 ()
 {
-  if (state_ == SOLO2)
+  if ((state_ == SOLO) and (chosen_part_ == 2))
     return;
   else
     {
-      state_ = SOLO2;
-
+      state_ = SOLO;
+      chosen_part_ = 2;
       substitute_both (CONTEXT_NULL, CONTEXT_SOLO);
 
-      if (playing_state_ != SOLO2)
+      if (playing_state_ != PLAYING_SOLO2)
         {
           if (!solo_two_event_)
             {
@@ -324,7 +330,7 @@ Part_combine_iterator::solo2 ()
             }
 
           second_iter_->get_outlet ()->event_source ()->broadcast (solo_two_event_);
-          playing_state_ = SOLO2;
+          playing_state_ = PLAYING_SOLO2;
         }
     }
 }
@@ -336,7 +342,7 @@ Part_combine_iterator::chords_together ()
     return;
   else
     {
-      playing_state_ = TOGETHER;
+      playing_state_ = PLAYING_OTHER;
       state_ = TOGETHER;
 
       substitute_both (CONTEXT_SHARED, CONTEXT_SHARED);
@@ -347,7 +353,7 @@ void
 Part_combine_iterator::apart (bool silent)
 {
   if (!silent)
-    playing_state_ = APART;
+    playing_state_ = PLAYING_OTHER;
 
   if (state_ == APART)
     return;
@@ -491,9 +497,20 @@ Part_combine_iterator::process (Moment m)
                || tag == ly_symbol2scm ("apart-spanner"))
         apart (tag == ly_symbol2scm ("apart-silence"));
       else if (tag == ly_symbol2scm ("unisono"))
-        unisono (false);
+        {
+          // Continue to use the most recently used part because we might have
+          // killed mmrests in the other part.
+          unisono (false, (last_playing_ == 2) ? 2 : 1);
+        }
       else if (tag == ly_symbol2scm ("unisilence"))
-        unisono (true);
+        {
+          // as for unisono
+          unisono (true, (last_playing_ == 2) ? 2 : 1);
+        }
+      else if (tag == ly_symbol2scm ("silence1"))
+        unisono (true, 1);
+      else if (tag == ly_symbol2scm ("silence2"))
+        unisono (true, 2);
       else if (tag == ly_symbol2scm ("solo1"))
         solo1 ();
       else if (tag == ly_symbol2scm ("solo2"))
@@ -509,13 +526,13 @@ Part_combine_iterator::process (Moment m)
   if (first_iter_->ok ())
     {
       if (try_process (first_iter_, m))
-        last_playing_ = SOLO1;
+        last_playing_ = 1;
     }
 
   if (second_iter_->ok ())
     {
       if (try_process (second_iter_, m))
-        last_playing_ = SOLO2;
+        last_playing_ = 2;
     }
 }
 
index 58f7b303a8620ac63279d571aa577b3a8698828b..ae51d0539be6796e5ebcc7d2088841a9f76d2ca4 100644 (file)
         (ly:in-event-class? x 'skip-event)))
   (filter f? (events vs)))
 
+(define-method (any-mmrest-events (vs <Voice-state>))
+  (define (f? x)
+    (ly:in-event-class? x 'multi-measure-rest-event))
+  (any f? (events vs)))
+
 (define-method (previous-voice-state (vs <Voice-state>))
   (let ((i (slot-ref vs 'vector-index))
         (v (slot-ref vs 'state-vector)))
       (display " synced "))
   (display "\n" f))
 
+(define-method (current-or-previous-voice-states (ss <Split-state>))
+  "Return voice states meeting the following conditions.  For a voice
+in sync, return the current voice state.  For a voice out of sync,
+return the previous voice state."
+  (let* ((vss (voice-states ss))
+         (vs1 (car vss))
+         (vs2 (cdr vss)))
+    (if (and vs1 (not (equal? (moment vs1) (moment ss))))
+        (set! vs1 (previous-voice-state vs1)))
+    (if (and vs2 (not (equal? (moment vs2) (moment ss))))
+        (set! vs2 (previous-voice-state vs2)))
+    (cons vs1 vs2)))
+
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 
@@ -441,47 +459,52 @@ Only set if not set previously.
                  (vs1 (car (voice-states now-state)))
                  (vs2 (cdr (voice-states now-state))))
 
-            (define (analyse-silence)
+            (define (analyse-synced-silence)
               (let ((rests1 (if vs1 (rest-and-skip-events vs1) '()))
                     (rests2 (if vs2 (rest-and-skip-events vs2) '())))
                 (cond
-                 
+
                  ;; multi-measure rests (probably), which the
                  ;; part-combine iterator handles well
-                 ((and (synced? now-state)
-                       (= 0 (length rests1))
+                 ((and (= 0 (length rests1))
                        (= 0 (length rests2)))
                   (set! (configuration now-state) 'unisilence))
-                 
+
                  ;; equal rests or equal skips, but not one of each
-                 ((and (synced? now-state)
-                       (= 1 (length rests1))
+                 ((and (= 1 (length rests1))
                        (= 1 (length rests2))
                        (equal? (ly:event-property (car rests1) 'class)
                                (ly:event-property (car rests2) 'class))
                        (equal? (ly:event-property (car rests1) 'duration)
                                (ly:event-property (car rests2) 'duration)))
                   (set! (configuration now-state) 'unisilence))
-                 
+
                  ;; rests of different durations or mixed with
                  ;; skips or multi-measure rests
-                 ((synced? now-state)
-                  ;; TODO When one part has a rest and the other has a
-                  ;; multi-measure rest, tell the part-combine
-                  ;; iterator to route the part with the rest to the
-                  ;; shared voice.  Until there is a way to do this,
-                  ;; we print them both; it does not look very good,
-                  ;; but failing to print the rest is misleading.
-                  ;;
-                  ;; Maybe do something similar for skips; route
-                  ;; the rest to the shared voice and the skip to
-                  ;; the voice for its part.
+                 (else
+                  ;; TODO For skips, route the rest to the shared
+                  ;; voice and the skip to the voice for its part?
                   (set! (configuration now-state) 'apart-silence))
-                 
-                 ;; TODO At a multi-measure rest, return to unisilence
-                 ;; even after having been apart.  The results are not
-                 ;; good now because of the deficiency mentioned
-                 ;; above.
+
+                 )))
+
+            (define (analyse-unsynced-silence vs1 vs2)
+              (let ((any-mmrests1 (if vs1 (any-mmrest-events vs1) #f))
+                    (any-mmrests2 (if vs2 (any-mmrest-events vs2) #f)))
+                (cond
+                 ;; If a multi-measure rest begins now while the other
+                 ;; part has an ongoing multi-measure rest (or has
+                 ;; ended), start displaying the one that begins now.
+                 ((and any-mmrests1
+                       (equal? (moment vs1) (moment now-state))
+                       (or (not vs2) any-mmrests2))
+                  (set! (configuration now-state) 'silence1))
+
+                 ;; as above with parts swapped
+                 ((and any-mmrests2
+                       (equal? (moment vs2) (moment now-state))
+                       (or (not vs1) any-mmrests1))
+                  (set! (configuration now-state) 'silence2))
                  )))
 
             (if (or vs1 vs2)
@@ -495,9 +518,22 @@ Only set if not set previously.
                               (equal? (ly:event-property (car notes1) 'pitch)
                                       (ly:event-property (car notes2) 'pitch)))
                          (set! (configuration now-state) 'unisono))
-                        ((and (= 0 (length notes1))
-                              (= 0 (length notes2)))
-                         (analyse-silence)))))
+
+                        ((synced? now-state)
+                         (if (and (= 0 (length notes1))
+                                  (= 0 (length notes2)))
+                             (analyse-synced-silence)))
+
+                        (else ;; not synchronized
+                         (let* ((vss
+                                 (current-or-previous-voice-states now-state))
+                                (vs1 (car vss))
+                                (vs2 (cdr vss)))
+                           (if (and
+                                (or (not vs1) (= 0 (length (note-events vs1))))
+                                (or (not vs2) (= 0 (length (note-events vs2)))))
+                               (analyse-unsynced-silence vs1 vs2))))
+                        )))
             (analyse-a2 (1+ result-idx)))))
 
     (define (analyse-solo12 result-idx)
@@ -562,7 +598,50 @@ the mark when there are no spanners active.
             ;; try-solo
             start-idx))
 
-      (define (analyse-moment result-idx)
+      (define (analyse-apart-silence result-idx)
+        "Analyse 'apart-silence starting at RESULT-IDX.  Return next index."
+        (let* ((now-state (vector-ref result result-idx))
+               (vs1 (current-voice-state now-state 1))
+               (vs2 (current-voice-state now-state 2))
+               (rests1 (if vs1 (rest-and-skip-events vs1) '()))
+               (rests2 (if vs2 (rest-and-skip-events vs2) '()))
+               (prev-state (if (> result-idx 0)
+                               (vector-ref result (- result-idx 1))
+                               #f))
+               (prev-config (if prev-state
+                                (configuration prev-state)
+                                'apart-silence)))
+          (cond
+           ;; rest with multi-measure rest: choose the rest
+           ((and (synced? now-state)
+                 (= 1 (length rests1))
+                 (ly:in-event-class? (car rests1) 'rest-event)
+                 (= 0 (length rests2))) ; probably mmrest
+            (put 'silence1))
+
+           ;; as above with parts swapped
+           ((and (synced? now-state)
+                 (= 1 (length rests2))
+                 (ly:in-event-class? (car rests2) 'rest-event)
+                 (= 0 (length rests1))) ; probably mmrest
+            (put 'silence2))
+
+           ((synced? now-state)
+            (put 'apart-silence))
+
+           ;; remain in the silence1/2 states until resync
+           ((equal? prev-config 'silence1)
+            (put 'silence1))
+
+           ((equal? prev-config 'silence2)
+            (put 'silence2))
+
+           (else
+            (put 'apart-silence)))
+
+          (1+ result-idx)))
+
+      (define (analyse-apart result-idx)
         "Analyse 'apart starting at RESULT-IDX.  Return next index."
         (let* ((now-state (vector-ref result result-idx))
                (vs1 (current-voice-state now-state 1))
@@ -577,8 +656,10 @@ the mark when there are no spanners active.
           (max
            ;; we should always increase.
            (cond ((and (= n1 0) (= n2 0))
-                  (put 'apart-silence)
-                  (1+ result-idx))
+                  ;; If we hit this, it means that the previous passes
+                  ;; have designated as 'apart what is really
+                  ;; 'apart-silence.
+                  (analyse-apart-silence result-idx))
                  ((and (= n2 0)
                        (equal? (moment vs1) (moment now-state))
                        (null? (previous-span-state vs1)))
@@ -593,9 +674,14 @@ the mark when there are no spanners active.
            (1+ result-idx))))
 
       (if (< result-idx (vector-length result))
-          (if (equal? (configuration (vector-ref result result-idx)) 'apart)
-              (analyse-solo12 (analyse-moment result-idx))
-              (analyse-solo12 (1+ result-idx))))) ; analyse-solo12
+          (let ((conf (configuration (vector-ref result result-idx))))
+            (cond
+             ((equal? conf 'apart)
+              (analyse-solo12 (analyse-apart result-idx)))
+             ((equal? conf 'apart-silence)
+              (analyse-solo12 (analyse-apart-silence result-idx)))
+             (else
+              (analyse-solo12 (1+ result-idx))))))) ; analyse-solo12
 
     (analyse-spanner-states voice-state-vec1)
     (analyse-spanner-states voice-state-vec2)