I don't know how to properly review this, but I checked against a
current score I'm working on, and I see it removes one of the most
annoying issues I had so far.
Consider this snippet:
\version "2.19.16"
one = \relative c'' {
c2 c
\voiceOne
r8 c b a g f e d
c1
}
two = \relative c' {
c2 c
R1*2
}
\score {
\new Staff \partcombine \one \two
}
This previously (incl. current master) resulted in the attached
partcombine-fail-merge-rests.png, which is musical nonsense.
With the patch applied it results in partcombine-merge-rests.png.
So I think the patch does what it says (and helps me very much), so LGTM.
However, there's an issue with this example. I'm sure it's not related
to the patch but an independent issue - but maybe you could try to fix
that along with your patch?
When the partcombiner deals with a section that starts with different
rests (the situation you improve in your patch) it fails to set the
Voice number correctly. I even have the impression it actively sets it
wrongly. The attached image shows that
- at the first moment of the measure the voices are set correctly: The
quaver rest is \voiceOne, the full measure rest is \voiceTwo.
But the notes starting with the second quaver are obviously \oneVoice.
It is clear that the partcombiner actively sets the upper voice to
\oneVoice because
\voiceOne r8 c b a g f e d
doesn't change the output while
r8 \voiceOne c b a g f e d
or
c8 c b a g f e d
both produce the correct result.
So:
Do you think you could tackle that in the context of your current patch?
Otherwise could please someone open a new issue for that (CCing to
bug-lilypond)
Urs
Am 24.11.2014 06:12, schrieb nine.fierce.ball...@gmail.com:
Reviewers: ,
Description:
Issue 4205: Improve part combiner's rest analysis
Rests must begin and end simultaneously to be merged into the shared
voice.
Rests, skips, and multi-measure rests are kept apart even if they
begin and end simultaneously.
This does not produce ideal output in every case, but it avoids
producing musical nonsense.
Please review this at https://codereview.appspot.com/174610043/
Affected files (+131, -4 lines):
A input/regression/part-combine-mmrest-shared.ly
A input/regression/part-combine-silence.ly
A input/regression/part-combine-silence-mixed.ly
M scm/part-combiner.scm
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
