Issues 4245 and 4275: User-defined rest styles

Allow user-defined rest styles.
https://code.google.com/p/lilypond/issues/detail?id=4275

Let custom stencils affect spacing.
https://code.google.com/p/lilypond/issues/detail?id=4245

This is complicated by having to choose between ledgered and unledgered glyphs based on the position of the rest on the staff.

The default stencil for rests is an now unpure-pure property.  This is not enough on its own.  To avoid a cyclic dependency in the evaluation of the unpure stencil during positioning by Rest_collision, some rather complicated code (I feel "unpure" for having written it) caches/uses the pure stencil in the internal property stencil-pure.

Rests in TabStaff are now hidden instead of having their stencils set to #f to keep regression tests passing after changes to rest stencil handling.

[Dan Eble, 2015-01-29 14:22:18]

Keith mentioned recently, "The similar issue with Accidentals (issue 3646) was
not so bad."  This might be the second time he mentioned it but I forgot about
it.  I should take a look at that issue.

[Dan Eble, 2015-01-30 04:39:21]

Eureka. Allow overriding stencil alone. (fixes 4245)

[Keith, 2015-01-31 06:02:50]

Looks pretty good.

The change to tabstaff looks suspicious the way you describe it.  Keeping
regtests unchanged is a bad reason to put invisible rests in tablature, but
keeping users' existing scores unchanged might be a good reason.   I suppose the
former behavior of giving Rests an extent regardless of their stencil had
(probably unwanted) effect on tablature, and you are preserving the (proably
unwanted) behavior.

It does seem complicated, for the simple tasks of waiting until layout is
finalized enough to know if a rest was pushed ouside the staff to draw ledgers
on rests.  But this does seem better than the old way.  Storing a tentative
stencil in 'stencil-pure is better than the old way of looking it up repeatedly
from fontforge.

I still suspect that you could put the tentative stencil in the usual 'stencil
property, and give responsibility for finalizing the rest glyph to the Rest
'positioning-done callback.  Almost always, the final stencil is the same as the
tentative, so having code to re-write the stencil for the exceptional case seems
conceptually simpler.

https://codereview.appspot.com/200860043/diff/20001/input/regression/rest-dot...
File input/regression/rest-dot-position.ly (right):

https://codereview.appspot.com/200860043/diff/20001/input/regression/rest-dot...
input/regression/rest-dot-position.ly:11: 
Why not in rest.scm ?
I guess you are going to personally tell everyone who has used the mirrored-z
rest how to add these lines to thier input files ...

https://codereview.appspot.com/200860043/diff/20001/lily/rest.cc
File lily/rest.cc (right):

https://codereview.appspot.com/200860043/diff/20001/lily/rest.cc#newcode166
lily/rest.cc:166: + offset);
I guess that get_position() could possibly trigger a full layout, but I can't
tell for sure without booting Linux to experiment.

I trust that you are certain there is a reason we need below to carefully avoid
calling is_ledgered().

https://codereview.appspot.com/200860043/diff/20001/lily/rest.cc#newcode203
lily/rest.cc:203: 
The comments depend on the meaning of the word 'pure' as used by a subset of
LilyPond programmers.  You could just say:
// Get a stencil, but if 'pure'=true do so without testing if the rest needs a
ledger

https://codereview.appspot.com/200860043/diff/20001/ly/engraver-init.ly
File ly/engraver-init.ly (right):

https://codereview.appspot.com/200860043/diff/20001/ly/engraver-init.ly#newco...
ly/engraver-init.ly:897: \hide Rest
%% Should probably change to the logical \omit Rest
%%  but before 2015 omitted rests were allocated space, 
%%  so this \omit Rest  preserves former (maybe undesired) behavior

[Dan Eble, 2015-01-31 22:49:29]

Thanks for taking a close look.  I will experiment with the positioning-done
callback.  I will also add a regtest based on your custom stencil example in
issue 4245.

https://codereview.appspot.com/200860043/diff/20001/input/regression/rest-dot...
File input/regression/rest-dot-position.ly (right):

https://codereview.appspot.com/200860043/diff/20001/input/regression/rest-dot...
input/regression/rest-dot-position.ly:11: 
On 2015/01/31 06:02:50, Keith wrote:
> Why not in rest.scm ?
> I guess you are going to personally tell everyone who has used the mirrored-z
> rest how to add these lines to thier input files ...

Simply because I don't like "z" as a style name.  I just sent an email to the
users' list asking for suggestions.  If nothing better is proposed in the next
several days, I'll add a built-in style called "z".

https://codereview.appspot.com/200860043/diff/20001/lily/rest.cc
File lily/rest.cc (right):

https://codereview.appspot.com/200860043/diff/20001/lily/rest.cc#newcode166
lily/rest.cc:166: + offset);
On 2015/01/31 06:02:50, Keith wrote:
> I guess that get_position() could possibly trigger a full layout, but I can't
> tell for sure without booting Linux to experiment.

I don't know what "trigger a full layout" means.

> I trust that you are certain there is a reason we need below to carefully
avoid
> calling is_ledgered().

Well, I believe there is a reason that it was carefully avoided in the past, and
I know that experimentally changing it to be less careful caused differences in
tests, but I can not say that I fully comprehend why it should be avoided.

https://codereview.appspot.com/200860043/diff/20001/lily/rest.cc#newcode203
lily/rest.cc:203: 
On 2015/01/31 06:02:50, Keith wrote:
> The comments depend on the meaning of the word 'pure' as used by a subset of
> LilyPond programmers.  You could just say:
> // Get a stencil, but if 'pure'=true do so without testing if the rest needs a
> ledger

If pure=true, do not cache the result in the stencil-pure property.  (This is
because the CG says looking up a pure property should never result in a call to
set_property.)

https://codereview.appspot.com/200860043/diff/20001/ly/engraver-init.ly
File ly/engraver-init.ly (right):

https://codereview.appspot.com/200860043/diff/20001/ly/engraver-init.ly#newco...
ly/engraver-init.ly:897: \hide Rest
On 2015/01/31 06:02:50, Keith wrote:
> %% Should probably change to the logical \omit Rest
> %%  but before 2015 omitted rests were allocated space, 
> %%  so this \omit Rest  preserves former (maybe undesired) behavior

I'll do that, and I should probably open a ticket if this really was undesired.

[Keith, 2015-02-01 04:27:54]

On Sat, 31 Jan 2015 14:49:29 -0800, <nine.fierce.ballads@gmail.com> wrote:

> https://codereview.appspot.com/200860043/diff/20001/lily/rest.cc#newcode166
> lily/rest.cc:166: + offset);
> On 2015/01/31 06:02:50, Keith wrote:
>> I guess that get_position() could possibly trigger a full layout, but
> I can't
>> tell for sure without booting Linux to experiment.
>
> I don't know what "trigger a full layout" means.
>

I meant that get_position() might call functions that call other functions that
choose line-breaks, set beam slopes, etc.  I was trying to imaging how there
might be a cyclical dependency.
   \new Staff \transpose c c' <<
     {r2 g'2}
     \\ {r2 d2 }
     \\ {e8 f g a b2} >>
   #(ly:set-option 'debug-skylines)
   \layout {\context { \Score
     \override PaperColumn.stencil = #ly:separation-item::print } }
The ledgers on the upper half-rest potentially depends on beam slope, which
depends on horizontal spacing.

> https://codereview.appspot.com/200860043/diff/20001/lily/rest.cc#newcode203
> lily/rest.cc:203:
> On 2015/01/31 06:02:50, Keith wrote:
>> The comments depend on the meaning of the word 'pure' as used by a
> subset of
>> LilyPond programmers.  You could just say:
>> // Get a stencil, but if 'pure'=true do so without testing if the rest
> needs a
>> ledger
>
> If pure=true, do not cache the result in the stencil-pure property.

Oh.  Then I was wrong about this patch making it less likely to repeatedly look
up the rest glyph, and I am no longer see the point of a separate 'stencil-pure
property.

This patch only solves issue 4245 for users who know how to put their custom
rest stencil in a pure-unpure-container.

The pre-patch approach seemed reasonable, in that it evaluated the X/Y-extent by
calling he stencil-building function, while leaving a link to that function in
the 'stencil property for re-evaluation when time comes to print a
possibly-moved rest.  The problem with the pre-patch code is that it assumes the
Rest.stencil points to the built-in default.

After an earlier patch <https://codereview.appspot.com/7300082#msg6> the usual
way to handle stencils that change shape depending on other layout is to put the
smarts in the X/Y-extent, rather than the 'stencil.  It would be more similar to
other Grobs if you can make the Rest.stencil actually 'pure' in the sense of
returning a stencil without asking for the beam slopes that (probably) cause the
cycle.  Probably Rest.staff-position rather than get_position() can determine
whether to use the ledgered version, without triggering beam slope calculations.
 Then the 'X/Y-extent callbacks can use get_pure_property("stencil") to
determine the extents, and later the print routine will re-evaluate the stencil
function.  For that to work properly, all code that moves rests up and down
would need to update the Rest.staff-position, but that seems like a good idea
anyway.


> (This is because the CG says looking up a pure property should never
> result in a call to set_property.)

Usually, when a property holds a function, LilyPond's evaluation routines
evaluate the function once and then overwrite the property with the result. 
Progress is thus made in determining the layout.

The idea of 'pure' evaluation of properties that hold functions was to delay
progress in determining the layout.  Joe Neeman's approach to choosing line- and
page-breaks evaluates tentative layouts, requiring that we evaluate the
functions in properties like 'Y-extent, but leave the function in place for
future evaluation.  The 'pure' methods leave any function in place, and also
promise not to cause any irreversible progress in determining the layout.

The rule that a 'pure' evaluation may not set_property() might help to keep
promise not to irreversibly advance layout, but the rule does lead to a lot of
re-computation.  There is no problem setting properties that are known to be
final before line-breaks are determined.

[Dan Eble, 2015-02-02 23:29:04]

On 2015/02/01 04:27:54, Keith wrote:
> > This patch only solves issue 4245 for users who know how to put their custom
> rest stencil in a pure-unpure-container.

What I intended was that the stencil may optionally be in a pure-unpure
container, but if it is not, it is assumed to be pure and evaluated.