From: Dan Eble Date: Mon, 29 Dec 2014 02:54:52 +0000 (-0500) Subject: Issue 4233: Improve partcombine multi-measure rest handling. X-Git-Tag: release/2.19.16-1~2^2~38^2~29 X-Git-Url: https://git.donarmstrong.com/?a=commitdiff_plain;h=771722dc109f9da48f08d14bcde9c154be629aec;p=lilypond.git Issue 4233: Improve partcombine multi-measure rest handling. 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. --- diff --git a/input/regression/part-combine-mmrest-after-apart-silence.ly b/input/regression/part-combine-mmrest-after-apart-silence.ly index 0bea1fab53..99f1f8f34b 100644 --- a/input/regression/part-combine-mmrest-after-apart-silence.ly +++ b/input/regression/part-combine-mmrest-after-apart-silence.ly @@ -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 } } >> } diff --git a/lily/part-combine-iterator.cc b/lily/part-combine-iterator.cc index c5abcdcda0..2428d4a70f 100644 --- a/lily/part-combine-iterator.cc +++ b/lily/part-combine-iterator.cc @@ -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; } } diff --git a/scm/part-combiner.scm b/scm/part-combiner.scm index 58f7b303a8..ae51d0539b 100644 --- a/scm/part-combiner.scm +++ b/scm/part-combiner.scm @@ -50,6 +50,11 @@ (ly:in-event-class? x 'skip-event))) (filter f? (events vs))) +(define-method (any-mmrest-events (vs )) + (define (f? x) + (ly:in-event-class? x 'multi-measure-rest-event)) + (any f? (events vs))) + (define-method (previous-voice-state (vs )) (let ((i (slot-ref vs 'vector-index)) (v (slot-ref vs 'state-vector))) @@ -79,6 +84,19 @@ (display " synced ")) (display "\n" f)) +(define-method (current-or-previous-voice-states (ss )) + "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)