Re: Issue 4205: Improve part combiner's rest analysis (issue 174610043 by address@hidden)

[Urs Liska]

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 address@hidden:
> 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
> address@hidden
> https://lists.gnu.org/mailman/listinfo/lilypond-devel

partcombine-fail-merge-rests.png
Description: PNG image

partcombine-merge-rests.png
Description: PNG image