Issue 4386: Part_combine_iterator: move mmrest handling outside the part-routing state machine (issue 236130043 by address@hidden)

[nine . fierce . ballads]

Reviewers: ,


https://codereview.appspot.com/236130043/diff/1/lily/part-combine-iterator.cc
File lily/part-combine-iterator.cc (left):

https://codereview.appspot.com/236130043/diff/1/lily/part-combine-iterator.cc#oldcode236
lily/part-combine-iterator.cc:236: kill_mmrest (CONTEXT_SHARED);
Here is the one difference worth pointing out.  The new code does not
kill mmrests in the shared voice in this case.  I have been unable to
think of a scenario in which this makes a difference.



Please review this at https://codereview.appspot.com/236130043/

Affected files (+20, -8 lines):
  M lily/part-combine-iterator.cc


Index: lily/part-combine-iterator.cc
diff --git a/lily/part-combine-iterator.cc b/lily/part-combine-iterator.cc
index  
f84e70cc9ac00bcf903ccf42cb9797fb64e6d8ac..bd44f884904d99ddc750a0bac6bc98e214fb52e4  
100644
--- a/lily/part-combine-iterator.cc
+++ b/lily/part-combine-iterator.cc
@@ -100,7 +100,7 @@ private:

   void substitute_both (Outlet_type to1,
                         Outlet_type to2);
-
+  void kill_mmrest_in_inactive_outlets ();
   /* parameter is really Outlet_type */
   void kill_mmrest (int in);
   void chords_together ();
@@ -196,11 +196,21 @@ Part_combine_iterator::substitute_both (Outlet_type  
to1,
         if (j != tos[i])
           mis[i]->substitute_outlet (handles_[j].get_context (),  
handles_[tos[i]].get_context ());
     }
+}

+void Part_combine_iterator::kill_mmrest_in_inactive_outlets ()
+{
   for (int j = 0; j < NUM_OUTLETS; j++)
     {
-      if (j != to1 && j != to2)
-        kill_mmrest (j);
+      Context *c = handles_[j].get_context ();
+
+      if (first_iter_->get_outlet () == c)
+        continue;
+
+      if (second_iter_->get_outlet () == c)
+        continue;
+
+      kill_mmrest (j);
     }
 }

@@ -232,8 +242,6 @@ Part_combine_iterator::unisono (bool silent, int  
newpart)
       Outlet_type c1 = (newpart == 2) ? CONTEXT_NULL : CONTEXT_SHARED;
       Outlet_type c2 = (newpart == 2) ? CONTEXT_SHARED : CONTEXT_NULL;
       substitute_both (c1, c2);
-      kill_mmrest ((newpart == 2) ? CONTEXT_ONE : CONTEXT_TWO);
-      kill_mmrest (CONTEXT_SHARED);

       if (playing_state_ != PLAYING_UNISONO
           && newstate == UNISONO)
@@ -256,9 +264,6 @@ Part_combine_iterator::solo1 ()
       chosen_part_ = 1;
       substitute_both (CONTEXT_SOLO, CONTEXT_NULL);

-      kill_mmrest (CONTEXT_TWO);
-      kill_mmrest (CONTEXT_SHARED);
-
       playing_state_ = PLAYING_SOLO1;
     }
 }
@@ -384,6 +389,9 @@ Part_combine_iterator::process (Moment m)

       SCM tag = scm_cdar (split_list_);

+      Context *outletsBefore[] = { first_iter_->get_outlet (),
+                                   second_iter_->get_outlet () };
+
       if (scm_is_eq (tag, ly_symbol2scm ("chords")))
         chords_together ();
       else if (scm_is_eq (tag, ly_symbol2scm ("apart"))
@@ -415,6 +423,10 @@ Part_combine_iterator::process (Moment m)
                      + (scm_is_symbol (tag) ? ly_symbol2string (tag) :  
string ("not a symbol"));
           programming_error (s);
         }
+
+      if ((first_iter_->get_outlet () != outletsBefore[0])
+          || (second_iter_->get_outlet () != outletsBefore[1]))
+        kill_mmrest_in_inactive_outlets ();
     }

   if (first_iter_->ok ())