Reposition voiced rests (Issue 3902)

This moves voiced rests closer together, more closely matching the recommendations of Gould, Read, etc.  There are a lot of regression test changes, but all associated with repositioned rests.

[PhilEHolmes, 2014-05-27 09:38:11]

Please review

[uliska, 2014-05-27 09:59:45]

On 2014/05/27 09:38:11, PhilEHolmes wrote:
> Please review

According to the comparison images in
https://code.google.com/p/lilypond/issues/detail?id=3902#c5 this is definitely
an improvement. (I had looked for that issue myself, but I have to admit that I
have to stop when entering the C++ area ...).

But what would you think about going one step further and place voiced rests by
default at the same position as \oneVoice rests? This would make the regularly
asked question about merging rests obsolete. The rest would appear identical to
a \oneVoice rest if there isn't any other object in the way. The voice number
assignment would then "only" determine the direction the rests would move to.

Another approach which would be an even more significant improvement (but
definitely much more involved) would be to try to determine pitches of
surrounding notes in the same voice, which can avoid ambiguities sometimes.
(Maybe it's a good idea to reread
http://blog.steinberg.net/2014/02/development-diary-part-the-fifth/#more-523)
which was my first encounter of the problem).

Please note that these are only suggestions, no objections against the patch.

[lemzwerg, 2014-05-27 10:00:53]

LGTM.

What about making this configurable?  I can imagine that for some special
situations a higher default position of rests is desirable (e.g., fugues and the
like).

[uliska, 2014-05-27 10:04:35]

On 2014/05/27 10:00:53, lemzwerg wrote:
> LGTM.
> 
> What about making this configurable?  I can imagine that for some special
> situations a higher default position of rests is desirable (e.g., fugues and
the
> like).

+1. Maybe with my suggested \oneVoice behaviour as the default?

[email_philholmes.net, 2014-05-27 10:08:46]

----- Original Message ----- 
From: <lilyliska@googlemail.com>
To: <PhilEHolmes@googlemail.com>
Cc: <lilypond-devel@gnu.org>; <reply@codereview-hr.appspotmail.com>
Sent: Tuesday, May 27, 2014 10:59 AM
Subject: Re: Reposition voiced rests (Issue 3902) (issue 101720045)


> On 2014/05/27 09:38:11, PhilEHolmes wrote:
>> Please review
>
> According to the comparison images in
> https://code.google.com/p/lilypond/issues/detail?id=3902#c5 this is
> definitely an improvement. (I had looked for that issue myself, but I
> have to admit that I have to stop when entering the C++ area ...).
>
> But what would you think about going one step further and place voiced
> rests by default at the same position as \oneVoice rests? This would
> make the regularly asked question about merging rests obsolete. The rest
> would appear identical to a \oneVoice rest if there isn't any other
> object in the way. The voice number assignment would then "only"
> determine the direction the rests would move to.

I would definitely argue against that.  I frequently want separated rests 
for separate voices, and to remove that would not match the recomended 
typesetting procedure.  If I wanted "merged" rests, I'd just spacer-rest one 
of them and use \voiceOne.

> Another approach which would be an even more significant improvement
> (but definitely much more involved) would be to try to determine pitches
> of surrounding notes in the same voice, which can avoid ambiguities
> sometimes.
> (Maybe it's a good idea to reread
> http://blog.steinberg.net/2014/02/development-diary-part-the-fifth/#more-523)
> which was my first encounter of the problem).
>
> Please note that these are only suggestions, no objections against the
> patch.
>
> https://codereview.appspot.com/101720045/

I'm starting with something very simple here, so that's too complex at my 
stage of understanding.  If you look at all the comments on the issue, 
you'll see my disquiet over the misplaced ledger lines.  That will likely be 
a new issue that I may work on later.

--
Phil Holmes 

[mail_philholmes.net, 2014-05-27 10:15:49]

----- Original Message ----- 
From: <lemzwerg@googlemail.com>
To: <PhilEHolmes@googlemail.com>
Cc: <reply@codereview-hr.appspotmail.com>; <lilypond-devel@gnu.org>
Sent: Tuesday, May 27, 2014 11:00 AM
Subject: Re: Reposition voiced rests (Issue 3902) (issue 101720045)


> LGTM.
>
> What about making this configurable?  I can imagine that for some
> special situations a higher default position of rests is desirable
> (e.g., fugues and the like).
>
> https://codereview.appspot.com/101720045/


In a sense it is already configurable, either with pitched rests or 
\override Rest.staff-position

--
Phil Holmes 

[david.nalesnik, 2014-05-27 14:12:34]

https://codereview.appspot.com/101720045/diff/1/lily/rest.cc
File lily/rest.cc (right):

https://codereview.appspot.com/101720045/diff/1/lily/rest.cc#newcode138
lily/rest.cc:138: 
To be consistent with your change above, shouldn't this be:

  return neutral + 2 * dir;

[email_philholmes.net, 2014-05-27 14:27:07]

----- Original Message ----- 
From: <david.nalesnik@gmail.com>
To: <PhilEHolmes@googlemail.com>; <lemzwerg@googlemail.com>; 
<lilyliska@googlemail.com>; <email@philholmes.net>; <mail@philholmes.net>
Cc: <lilypond-devel@gnu.org>; <reply@codereview-hr.appspotmail.com>
Sent: Tuesday, May 27, 2014 3:12 PM
Subject: Re: Reposition voiced rests (Issue 3902) (issue 101720045)


>
> https://codereview.appspot.com/101720045/diff/1/lily/rest.cc
> File lily/rest.cc (right):
>
> https://codereview.appspot.com/101720045/diff/1/lily/rest.cc#newcode138
> lily/rest.cc:138:
> To be consistent with your change above, shouldn't this be:
>
>   return neutral + 2 * dir;
>
> https://codereview.appspot.com/101720045/


Probably.  I can't think of a use case where this code would actually be 
exercised, so would welcome a suggestion.

--
Phil Holmes 

[david.nalesnik, 2014-05-27 16:24:29]

On 2014/05/27 14:27:07, email_philholmes.net wrote:
> ----- Original Message ----- 
> From: <mailto:david.nalesnik@gmail.com>
> To: <mailto:PhilEHolmes@googlemail.com>; <mailto:lemzwerg@googlemail.com>; 
> <mailto:lilyliska@googlemail.com>; <mailto:email@philholmes.net>;
<mailto:mail@philholmes.net>
> Cc: <mailto:lilypond-devel@gnu.org>;
<mailto:reply@codereview-hr.appspotmail.com>
> Sent: Tuesday, May 27, 2014 3:12 PM
> Subject: Re: Reposition voiced rests (Issue 3902) (issue 101720045)
> 
> 
> >
> > https://codereview.appspot.com/101720045/diff/1/lily/rest.cc
> > File lily/rest.cc (right):
> >
> > https://codereview.appspot.com/101720045/diff/1/lily/rest.cc#newcode138
> > lily/rest.cc:138:
> > To be consistent with your change above, shouldn't this be:
> >
> >   return neutral + 2 * dir;
> >
> > https://codereview.appspot.com/101720045/
> 
> 
> Probably.  I can't think of a use case where this code would actually be 
> exercised, so would welcome a suggestion.
> 
> --
> Phil Holmes 
> 

It appears that this is a check that the automatic calculations of the positions
of voiced half and whole rests are sensible.  The check can fail when you mess
with the number of staff lines, in which case the fallback is assigned.

Hmm.  It doesn't appear that I can attach anything here, so I'll have to give my
examples (before and after with the revised line) on the page for Issue 3902.

[PhilEHolmes, 2014-05-28 14:35:04]

Updated with David N's suggestion

[Keith, 2014-05-31 19:42:19]

Looks like a new property is the only way to get an overall improvement. 
LilyPond has good infrastructure for using properties, so there is not much code
to add.

https://codereview.appspot.com/101720045/diff/2/lily/rest.cc
File lily/rest.cc (right):

https://codereview.appspot.com/101720045/diff/2/lily/rest.cc#newcode75
lily/rest.cc:75: pos = 2 * dir;
pos = dir * robust_scm2int(me->get_property("voiced-rest-position"), 2);

https://codereview.appspot.com/101720045/diff/2/lily/rest.cc#newcode139
lily/rest.cc:139: return neutral + 2 * dir;
This one should remain 2, because the voiced rest position has already been
applied.  
/* one staff-space, 2 staff positions, from the neutral position */

The logic above seems that it would prevent half- and longer rests from crossing
the centerline, even if voiced-rest-position is 0, which I think you want.

https://codereview.appspot.com/101720045/diff/2/lily/rest.cc#newcode319
lily/rest.cc:319: "minimum-distance "
As you say, Rest.minimum-distance is ignored, so we can remove it here.

https://codereview.appspot.com/101720045/diff/2/lily/rest.cc#newcode320
lily/rest.cc:320: "style "
"voiced-rest-position "
would go here, along with an entry in scm/define-grob-properties.scm