Re: Issue 4275: Allow user-defined rest styles. (issue 200860043 by address@hidden)

[Keith OHara]

On Sat, 31 Jan 2015 14:49:29 -0800, <address@hidden> 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.