[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Issue 4419: Engraving ends too early (issue 252970043 by address@hid
From: |
dak |
Subject: |
Re: Issue 4419: Engraving ends too early (issue 252970043 by address@hidden) |
Date: |
Fri, 10 Jul 2015 09:44:04 +0000 |
Reviewers: Dan Eble,
Message:
On 2015/07/10 02:56:43, Dan Eble wrote:
I haven't absorbed enough of the background knowledge on iteration to
say
credibly that this looks good, although the C++ looks fine.
Because this is a second try at fixing a problem, perhaps a regression
test
should be written or expanded to cover the case that the first try did
not.
Well, I'd like such a test to also cover issue 2608 which is not yet on
countdown. Making it part of _this_ issue would be a bad idea since
issue 2608 is a crash, and a regtest for a crash should not be committed
before the fix.
Description:
Issue 4419: Engraving ends too early
This is a followup on the solution for issue 2010 that was too eager
killing off unrelated iterators when an iterator in the vicinity of a
Lyric_combine_music_iterator died.
The salient point is to have Simultaneous_music_iterator::process_music
check for pending_moment () going from finite to infinite when
iterating, signifying the loss of an iterator defining an end point.
It happens to also fix issue 4339.
Also contains commit:
simplify Simultaneous_music_iterator::ok
Please review this at https://codereview.appspot.com/252970043/
Affected files (+12, -18 lines):
M lily/simultaneous-music-iterator.cc
Index: lily/simultaneous-music-iterator.cc
diff --git a/lily/simultaneous-music-iterator.cc
b/lily/simultaneous-music-iterator.cc
index
74f2d6dc9abbb2b14f182bbeeab5db6368f26a60..e892776b61a39e6f4814eb350500ab706a45c4ee
100644
--- a/lily/simultaneous-music-iterator.cc
+++ b/lily/simultaneous-music-iterator.cc
@@ -82,36 +82,33 @@ Simultaneous_music_iterator::construct_children ()
}
}
-// If there are non-run-always iterators and all of them die, take the
-// rest of them along.
+// If we have some iterators with definite next moment and no of them
+// remain after processing, we take the iterators with indefinite next
+// moment along. That makes sure that no Lyric_combine_music_iterator
+// will outstay its welcome (issue 2010).
+
void
Simultaneous_music_iterator::process (Moment until)
{
- bool had_good = false;
- bool had_bad = false;
SCM *proc = &children_list_;
+ bool finite = !pending_moment ().main_part_.is_infinity ();
while (scm_is_pair (*proc))
{
Music_iterator *i = unsmob<Music_iterator> (scm_car (*proc));
- bool run_always = i->run_always ();
- if (run_always || i->pending_moment () == until)
+ if (i->run_always () || i->pending_moment () == until)
i->process (until);
if (!i->ok ())
{
- if (!run_always)
- had_bad = true;
i->quit ();
*proc = scm_cdr (*proc);
}
else
{
- if (!run_always)
- had_good = true;
proc = SCM_CDRLOC (*proc);
}
}
- // If there were non-run-always iterators and all of them died, take
- // the rest of the run-always iterators along with them. They have
+ // If there were definite-ended iterators and all of them died, take
+ // the rest of the iterators along with them. They have
// likely lost their reference iterators. Basing this on the actual
// music contexts is not reliable since something like
// \new Voice = blah {
@@ -121,7 +118,7 @@ Simultaneous_music_iterator::process (Moment until)
// }
// cannot wait for the death of context blah before ending the
// simultaneous iterator.
- if (had_bad && !had_good)
+ if (finite && pending_moment ().main_part_.is_infinity ())
{
for (SCM p = children_list_; scm_is_pair (p); p = scm_cdr (p))
unsmob<Music_iterator> (scm_car (p))->quit ();
@@ -147,16 +144,13 @@ Simultaneous_music_iterator::pending_moment () const
bool
Simultaneous_music_iterator::ok () const
{
- bool run_always_ok = false;
for (SCM s = children_list_; scm_is_pair (s); s = scm_cdr (s))
{
Music_iterator *it = unsmob<Music_iterator> (scm_car (s));
- if (!it->run_always ())
+ if (it->ok ())
return true;
- else
- run_always_ok = run_always_ok || it->ok ();
}
- return run_always_ok;
+ return false;
}
bool