Adds dimension stencil command to correct with-dimension

Adds dimension stencil command to correct with-dimension

[MikeSol, 2013-08-21 13:24:41]

Fixes pad-around

[MikeSol, 2013-08-21 18:24:47]

Changes stencil name, adds comment

[Keith, 2013-08-22 06:54:36]

There is also \pad-markup and \pad-to-box

Also, the delayed stencils in \with-link and \page-ref could use this method to
reserve the size of the place-holder text

Also, harp-pedals.scm could use this method instead of the invisible box (which
is not quite invisible in PNG output and maybe others).

[lemzwerg, 2013-08-22 07:31:14]

This looks very promising!  Thanks for the work on this.

[MikeSol, 2013-08-22 08:04:42]

Responds to some of Keith's suggestions

[MikeSol, 2013-08-22 08:05:14]

I had time to implement everything but the delayed markup stuff.  I'll try to
get to that soon...

[MikeSol, 2013-08-22 08:13:08]

Uses skyline-dimensions-stencil for delayed markups

[MikeSol, 2013-08-22 08:13:54]

It was easier to implement the delayed stuff than I expected, so I got it done.

[MikeSol, 2013-08-24 06:35:48]

More generic approach to stencil dimensioning

[lemzwerg, 2013-08-24 07:18:52]

LGTM, except of using `surrogate' in the name.  Given that the stencil
dimensions are actively overridden on request, I would like rather like having
`override'.  For me, the word `surrogate' suggests something passive.

[mike7, 2013-08-24 08:05:13]

On 24 août 2013, at 10:18, lemzwerg@googlemail.com wrote:

> LGTM, except of using `surrogate' in the name.  Given that the stencil
> dimensions are actively overridden on request, I would like rather like
> having `override'.  For me, the word `surrogate' suggests something
> passive.
> 
> https://codereview.appspot.com/12957047/

The definition reads:

a substitute, esp. a person deputizing for another in a specific role or office:
she was regarded as thesurrogate for the governor during his final illness.
• (in the Christian Church) a bishop's deputy who grants marriage licenses.
• a judge in charge of probate, inheritance, and guardianship.
ORIGIN early 17th cent.: from Latin surrogatus, past participle of surrogare
‘elect as a substitute,’ from super- ‘over’ + rogare ‘ask.’

Surrogate suggests both overriding and substitution.  Here, we substitute one
stencil's skyline for another stencil's.

However, I am definitely not wed to the name surrogate.  If people like override
better I can do that.

Cheers,
MS

[dak, 2013-08-24 08:25:14]

On 2013/08/24 08:05:13, mike7 wrote:
> On 24 août 2013, at 10:18, mailto:lemzwerg@googlemail.com wrote:
> 
> > LGTM, except of using `surrogate' in the name.  Given that the stencil
> > dimensions are actively overridden on request, I would like rather like
> > having `override'.  For me, the word `surrogate' suggests something
> > passive.
> > 
> > https://codereview.appspot.com/12957047/
> 
> The definition reads:

It doesn't matter what the definition reads.  English has a live vocabulary of
over 150000 words and there is no need to use all of them in identifiers.  We
want our identifier names to be boring and following our own terminology
elsewhere as well as that of other programs.

If you pick a non-standard word with slightly different connotations, people
will think that the slightly different connotations are important, rather than
that this is just playing randomization games with a thesaurus.

[pkx166h, 2013-08-24 09:47:28]

On 2013/08/24 08:25:14, dak wrote:
> On 2013/08/24 08:05:13, mike7 wrote:
> > On 24 août 2013, at 10:18, mailto:lemzwerg@googlemail.com wrote:
> > 
> > > LGTM, except of using `surrogate' in the name.  Given that the stencil
> > > dimensions are actively overridden on request, I would like rather like
> > > having `override'.  For me, the word `surrogate' suggests something
> > > passive.
> > > 
> > > https://codereview.appspot.com/12957047/
> > 
> > The definition reads:
> 
> It doesn't matter what the definition reads.  English has a live vocabulary of
> over 150000 words and there is no need to use all of them in identifiers.  We
> want our identifier names to be boring and following our own terminology
> elsewhere as well as that of other programs.
> 
> If you pick a non-standard word with slightly different connotations, people
> will think that the slightly different connotations are important, rather than
> that this is just playing randomization games with a thesaurus.

Having to send highly technical responses to non-native English speakers daily
as part of my job, I do get Werner's point. Surrogate isn;t immediately obvious
unless you have a wide vocabulary. I don't know what this fix does, but could we
use a word like 'alternate' or 'replace' or 'proxy' (assuming it is a
function/command that has to take a value, and 'replace' it temporarily before
it puts it back - see? I told you I didn't know what this does), or perhaps
'stand-in' or 'backup'.

We had a classically educated programmer once that used to produce beautifully
composed 'event' messages and errors, however it just caused a lot of confusion
to our non-English customers (even some American ones ;) ) that we went 'boring'
as David so succinctly put it.

[mike7, 2013-08-24 10:05:17]

Sent from my iPhone

On 24 août 2013, at 12:47, pkx166h@gmail.com wrote:

> On 2013/08/24 08:25:14, dak wrote:
>> On 2013/08/24 08:05:13, mike7 wrote:
>> > On 24 août 2013, at 10:18, mailto:lemzwerg@googlemail.com wrote:
>> >
>> > > LGTM, except of using `surrogate' in the name.  Given that the
> stencil
>> > > dimensions are actively overridden on request, I would like rather
> like
>> > > having `override'.  For me, the word `surrogate' suggests
> something
>> > > passive.
>> > >
>> > > https://codereview.appspot.com/12957047/
>> >
>> > The definition reads:
> 
>> It doesn't matter what the definition reads.  English has a live
> vocabulary of
>> over 150000 words and there is no need to use all of them in
> identifiers.  We
>> want our identifier names to be boring and following our own
> terminology
>> elsewhere as well as that of other programs.
> 
>> If you pick a non-standard word with slightly different connotations,
> people
>> will think that the slightly different connotations are important,
> rather than
>> that this is just playing randomization games with a thesaurus.
> 
> Having to send highly technical responses to non-native English speakers
> daily as part of my job, I do get Werner's point. Surrogate isn;t
> immediately obvious unless you have a wide vocabulary. I don't know what
> this fix does, but could we use a word like 'alternate' or 'replace' or
> 'proxy' (assuming it is a function/command that has to take a value, and
> 'replace' it temporarily before it puts it back - see? I told you I
> didn't know what this does), or perhaps 'stand-in' or 'backup'.
> 
> We had a classically educated programmer once that used to produce
> beautifully composed 'event' messages and errors, however it just caused
> a lot of confusion to our non-English customers (even some American ones
> ;) ) that we went 'boring' as David so succinctly put it.
> 
> https://codereview.appspot.com/12957047/

The stencil command takes the skyline of stencil X and uses it as a replacement
for skyline Y. We could use replacement-skyline-stencil as the command.

Cheers,
MS

[dak, 2013-08-24 10:31:38]

On 2013/08/24 10:05:17, mike7 wrote:
> 
> The stencil command takes the skyline of stencil X and uses it as a
replacement
> for skyline Y. We could use replacement-skyline-stencil as the command.

I'd just use skyline-stencil here.  If specified, it is the stencil used for
skylines.

[dak, 2013-08-24 10:38:35]

On 2013/08/24 10:31:38, dak wrote:
> On 2013/08/24 10:05:17, mike7 wrote:
> > 
> > The stencil command takes the skyline of stencil X and uses it as a
> replacement
> > for skyline Y. We could use replacement-skyline-stencil as the command.
> 
> I'd just use skyline-stencil here.  If specified, it is the stencil used for
> skylines.

At any rate: isn't this interface at the same time unnecessarily general and
restricted?  It will only ever do for replacing all four skylines at once or
none at all.  An override of four dimensions which can be +inf.0 or -inf.0 or #f
(namely, just use the original skyline for this dimension) for different effects
seems to fit the known use cases better.

So what are the problems you are trying to solve here?

[MikeSol, 2013-08-24 13:11:58]

Changes name to skyline-stencil

[mike7, 2013-08-24 13:19:30]

On 24 août 2013, at 13:38, dak@gnu.org wrote:

> On 2013/08/24 10:31:38, dak wrote:
>> On 2013/08/24 10:05:17, mike7 wrote:
>> >
>> > The stencil command takes the skyline of stencil X and uses it as a
>> replacement
>> > for skyline Y. We could use replacement-skyline-stencil as the
> command.
> 
>> I'd just use skyline-stencil here.  If specified, it is the stencil
> used for
>> skylines.
> 
> At any rate: isn't this interface at the same time unnecessarily general
> and restricted?  It will only ever do for replacing all four skylines at
> once or none at all.  An override of four dimensions which can be +inf.0
> or -inf.0 or #f (namely, just use the original skyline for this
> dimension) for different effects seems to fit the known use cases
> better.
> 
> So what are the problems you are trying to solve here?

The question I'm asking is "How can I allow the user to replace the skyline of a
stencil with another shape?"  The previous patches asked the more specific
question "How can I allow the user to replace the skyline of a stencil with a
box?"

Cheers,
MS

[dak, 2013-08-24 13:36:49]

On 2013/08/24 13:19:30, mike7 wrote:

> The question I'm asking is "How can I allow the user to replace the skyline of
a
> stencil with another shape?"

I got that.  But what is the actual use case for that?  Before skylines became a
thing, you could override the X-extent or the Y-extent of grobs, independently. 
There were actual use cases for that with regard to controlling the arrangement
of marks and other things.

I repeat: isn't this interface at the same time unnecessarily general and
restricted? It will only ever do for replacing all four skylines at once or none
at all.  An override of four dimensions which can be +inf.0 or -inf.0 or #f
(namely, just use the original skyline for this dimension) for different effects
seems to fit the known use cases better.

You did not answer that question.  Instead you explained what the code does. 
Given the specificity of my question, I have a hard time imagining what made you
think I did not understand that when asking the question.

[dak, 2013-08-24 16:19:26]

https://codereview.appspot.com/12957047/diff/46001/lily/paper-system.cc
File lily/paper-system.cc (right):

https://codereview.appspot.com/12957047/diff/46001/lily/paper-system.cc#newco...
lily/paper-system.cc:55: if (head == ly_symbol2scm ("skyline-stencil"))
It seems awkward and error-prone to go through a number of stencil types here
just for fishing out footnotes.  Of course, this is just another layer of
ugliness on top of existing ugliness, but it would seem that we actually want a
separate _backend_ for fishing out footnotes, so that all the default
interpretations of stencils are done correctly.

https://codereview.appspot.com/12957047/diff/46001/lily/stencil-integral.cc
File lily/stencil-integral.cc (right):

https://codereview.appspot.com/12957047/diff/46001/lily/stencil-integral.cc#n...
lily/stencil-integral.cc:915: SCM skyline_stencil = scm_cadr (expr);
Why would this traverse the skyline_stencil rather than the real one?

https://codereview.appspot.com/12957047/diff/46001/scm/define-markup-commands...
File scm/define-markup-commands.scm (right):

https://codereview.appspot.com/12957047/diff/46001/scm/define-markup-commands...
scm/define-markup-commands.scm:730: (make-box-skyline-stencil
Stupid question: _iff_ you store a _whole_ replacement skyline anyway, shouldn't
pad-around just shift the original left/right/up/down skylines
left/right/up/down by the given amount (don't ask me what to do about the
corners, though)?

Of course this would be incompatible with previous behavior, but more likely
matching the expectations?

I am not sure that replacement skylines are the right thing anyway, but it seems
sort of pointless _if_ you propose they are to not use them here.

https://codereview.appspot.com/12957047/diff/46001/scm/define-markup-commands...
scm/define-markup-commands.scm:1997: (make-box-skyline-stencil m x-wide
y-wide)))
See pad-markup (which presumably does the same).  Should one of them use boxes
and one of them skylines?

https://codereview.appspot.com/12957047/diff/46001/scm/stencil.scm
File scm/stencil.scm (right):

https://codereview.appspot.com/12957047/diff/46001/scm/stencil.scm#newcode689
scm/stencil.scm:689: (let ((sur (make-filled-box-stencil x y)))
A filled-box stencil seems like serious overkill here when all you want is
setting the dimensions.  A stencil operator for just setting dimensions seems
less awkward.  There are two possibilities: have it produce an empty stencil
expression but with dimensions, and then use make-skyline-stencil on it.  Or
have it contain an inner expression anyway.

In that case, make-box-skyline-stencil (what an awkward name) does not need to
call either make-filled-box-stencil nor contain skyline-stencil anyway.

It means that one needs one more stencil primitive _if_ one wants to support
make-skyline-stencil.  But it avoids juggling with an obscure combination of
stencil operations if one doesn't.

[mike7, 2013-08-24 16:26:26]

On 24 août 2013, at 16:36, dak@gnu.org wrote:

> On 2013/08/24 13:19:30, mike7 wrote:
> 
>> The question I'm asking is "How can I allow the user to replace the
> skyline of a
>> stencil with another shape?"
> 
> I got that.  But what is the actual use case for that?  Before skylines
> became a thing, you could override the X-extent or the Y-extent of
> grobs, independently.  There were actual use cases for that with regard
> to controlling the arrangement of marks and other things.
> 
> I repeat: isn't this interface at the same time unnecessarily general
> and restricted? It will only ever do for replacing all four skylines at
> once or none at all.  An override of four dimensions which can be +inf.0
> or -inf.0 or #f (namely, just use the original skyline for this
> dimension) for different effects seems to fit the known use cases
> better.

To me, it seems like the most critical thing is making the various padding and
dimension functions work again.  This patch does that with little coding in C++
and most of it being done in Scheme.  It allows eventually for more
sophisticated padding.

Your suggestion for four different skyline replacements is possible to implement
through stencil operators.  That is, we can implement stencil union and stencil
intersect functions that keep or reject different sides of a stencil.  This way,
we don't need to specify four different stencils for the four different
skylines.

Cheers,
MS

[MikeSol, 2013-08-24 16:33:39]

https://codereview.appspot.com/12957047/diff/46001/lily/paper-system.cc
File lily/paper-system.cc (right):

https://codereview.appspot.com/12957047/diff/46001/lily/paper-system.cc#newco...
lily/paper-system.cc:55: if (head == ly_symbol2scm ("skyline-stencil"))
On 2013/08/24 16:19:27, dak wrote:
> It seems awkward and error-prone to go through a number of stencil types here
> just for fishing out footnotes.  Of course, this is just another layer of
> ugliness on top of existing ugliness, but it would seem that we actually want
a
> separate _backend_ for fishing out footnotes, so that all the default
> interpretations of stencils are done correctly.

Agreed - it's a problem in general of markups being implemented differently than
grobs, which has plagued footnotes from the beginning.

https://codereview.appspot.com/12957047/diff/46001/lily/stencil-integral.cc
File lily/stencil-integral.cc (right):

https://codereview.appspot.com/12957047/diff/46001/lily/stencil-integral.cc#n...
lily/stencil-integral.cc:915: SCM skyline_stencil = scm_cadr (expr);
On 2013/08/24 16:19:27, dak wrote:
> Why would this traverse the skyline_stencil rather than the real one?

Because we are using the skyline stencil as a substitute for the dimensions of
the real one.

https://codereview.appspot.com/12957047/diff/46001/scm/define-markup-commands...
File scm/define-markup-commands.scm (right):

https://codereview.appspot.com/12957047/diff/46001/scm/define-markup-commands...
scm/define-markup-commands.scm:730: (make-box-skyline-stencil
On 2013/08/24 16:19:27, dak wrote:
> Stupid question: _iff_ you store a _whole_ replacement skyline anyway,
shouldn't
> pad-around just shift the original left/right/up/down skylines
> left/right/up/down by the given amount (don't ask me what to do about the
> corners, though)?
> 
> Of course this would be incompatible with previous behavior, but more likely
> matching the expectations?
> 
> I am not sure that replacement skylines are the right thing anyway, but it
seems
> sort of pointless _if_ you propose they are to not use them here.

It's a good idea, but stencils don't have left/right/up/down, so it'd be hard to
figure out how to add this padding to the stencils without additional
plane-geometry functions.

https://codereview.appspot.com/12957047/diff/46001/scm/stencil.scm
File scm/stencil.scm (right):

https://codereview.appspot.com/12957047/diff/46001/scm/stencil.scm#newcode689
scm/stencil.scm:689: (let ((sur (make-filled-box-stencil x y)))
On 2013/08/24 16:19:27, dak wrote:
> A filled-box stencil seems like serious overkill here when all you want is
> setting the dimensions.  A stencil operator for just setting dimensions seems
> less awkward.  There are two possibilities: have it produce an empty stencil
> expression but with dimensions, and then use make-skyline-stencil on it.  Or
> have it contain an inner expression anyway.
> 
> In that case, make-box-skyline-stencil (what an awkward name) does not need to
> call either make-filled-box-stencil nor contain skyline-stencil anyway.
> 
> It means that one needs one more stencil primitive _if_ one wants to support
> make-skyline-stencil.  But it avoids juggling with an obscure combination of
> stencil operations if one doesn't.

The empty stencil wouldn't work, as stencil-integral.cc operates on the contents
of the stencil (lines, curves, etc.), not on the dimension.  So, it would read
an empty stencil as empty and not take it into account in the skyline,
irrespective of the dimension.

I'm not exactly sure what you mean in the rest of the comment, nor do I see why
the filled-box-stencil is overkill.  We need to draw a box around the stencil at
some time, and this gets the job done pretty efficiently (there's not much
overhead in stencil creation).  I'm open to other implementation suggestions,
though.

[Keith, 2013-08-25 06:15:51]

On 2013/08/24 16:33:39, MikeSol wrote:

> On 2013/08/24 16:19:27, dak wrote:
>
> > Stupid question: _iff_ you store a _whole_ replacement skyline anyway,
> > shouldn't pad-around just shift the original left/right/up/down skylines
> > left/right/up/down by the given amount (don't ask me what to do about the
> > corners, though)?

> It's a good idea, but stencils don't have left/right/up/down, so it'd be hard
to
> figure out how to add this padding to the stencils without additional
> plane-geometry functions.

A second stencil is not a very good data structure for reserving space.
Skylines are better at this, and we can \once\override 'padding and 
'horizon-padding to get padding that follows the outlines of the stencil.

The existing interface, \with-dimensions etc., gives a way to specify the
simplest Skyline: a single box.  I struggled to make a regression test for
these functions.  I used      \pad-to-box #'(0 . 6) #'(0 . 1) fa......}
to add a building around the dots... to the existing skyline of the "fa"
and had to use debug-skylines to get the dimensions right.

If I ever did need a more complex space-reservation shape, such as a
PostScript stencil, I would at first make the space-reservation print light
blue and \combine with the printing stencil, so I can see if I got the right
shape.  Your patch might save me a the few bytes of the \transparent version
of the PostScript outline in my output.

The meaning of the "Cells" output is a little mysterious to me, with Scheme
a garbage-collected system and all, but this patch makes one third of the
reg-tests report using between 3% and 8% more "cells", which can't be good.

[mike7, 2013-08-25 08:22:01]

On 25 août 2013, at 09:15, k-ohara5a5a@oco.net wrote:

> On 2013/08/24 16:33:39, MikeSol wrote:
> 
>> On 2013/08/24 16:19:27, dak wrote:
> 
>> > Stupid question: _iff_ you store a _whole_ replacement skyline
> anyway,
>> > shouldn't pad-around just shift the original left/right/up/down
> skylines
>> > left/right/up/down by the given amount (don't ask me what to do
> about the
>> > corners, though)?
> 
>> It's a good idea, but stencils don't have left/right/up/down, so it'd
> be hard to
>> figure out how to add this padding to the stencils without additional
>> plane-geometry functions.
> 
> A second stencil is not a very good data structure for reserving space.
> Skylines are better at this, and we can \once\override 'padding and
> 'horizon-padding to get padding that follows the outlines of the
> stencil.
> 
> The existing interface, \with-dimensions etc., gives a way to specify
> the
> simplest Skyline: a single box.  I struggled to make a regression test
> for
> these functions.  I used      \pad-to-box #'(0 . 6) #'(0 . 1) fa......}
> to add a building around the dots... to the existing skyline of the "fa"
> and had to use debug-skylines to get the dimensions right.
> 

I wrote this new patch based on my experience from slimming down the stencil
list a couple years ago.  There used to be lots of stencil primitives whose
handling code took up a bunch of space in lookup.cc, had a fair bit of code
dups, and would have made stencil-integral.cc a nightmare.  I eliminated them
(beam, bezier-sandwich, etc.) and implemented them in Scheme by combining other
stencil primitives.

I think that adding a stencil primitive should only be done if there is no
satisfactory solution using the current primitives (which is the case here).  If
we're going to do that, then we should think of the general problem we're trying
to solve.  To me, the general problem seems to be "how can we replace the
outline of one shape with that of another?"  So far, this question is only ever
asked in the specific form "how can we replace the outline of one shape with a
box?"  But I think if we hardcode the "box" in that question (as both of our
initial patch sets do), we will lead to the proliferation of multiple stencil
primitives for different shapes if the need comes up.

The necessity that comes to mind for the more general solution is the work I did
on line spanners, where we often want different skyline padding for the word in
the spanner and the line.  That could be implemented by using the
skyline-stencil primitive as I've coded it in the most recent patch set.

Cheers,
MS

[dak, 2013-08-25 13:04:13]

On 2013/08/25 08:22:01, mike7 wrote:
> On 25 août 2013, at 09:15, mailto:k-ohara5a5a@oco.net wrote:

> > A second stencil is not a very good data structure for reserving space.
> > Skylines are better at this, and we can \once\override 'padding and
> > 'horizon-padding to get padding that follows the outlines of the
> > stencil.
> [...]
> If
> we're going to do that, then we should think of the general problem we're
trying
> to solve.  To me, the general problem seems to be "how can we replace the
> outline of one shape with that of another?"

No, that is _absolutely_ not the general problem here.  The general problem is:
"how can we replace the outline/skylines of one stencil with a different
outline/skyline?"

Requiring the replacement to be the outline/skyline of an actually existing
stencil is _not_ the general problem.  Why should we need to create a stencil in
the first place if what we want is a horizontal and a vertical skyline pair?

You create a "filled box" here for \with-dimensions.  That's absurd.  Worse: if
you want to compute a skyline _from_ an existing skyline, there is no way.  You
first have to figure out what _stencil_ would have the kind of skyline you want.

> So far, this question is only ever
> asked in the specific form "how can we replace the outline of one shape with a
> box?"

No.  With regard to \with-dimensions, the question is how to replace the outline
of one shape with two specified extents.  For pad-x, the question is how to
replace the _horizontal_ (?) skylines with _one_ specified extent calculated
from box extents.  Actually, I'd argue that it would make more sense to shift
the horizontal skylines rather than place an extent.

> But I think if we hardcode the "box" in that question (as both of our
> initial patch sets do), we will lead to the proliferation of multiple stencil
> primitives for different shapes if the need comes up.

You need a primitive for replacing a subset of four skylines.  And it is not a
primitive but a derived operation, so the backends do not need to know about it.

> The necessity that comes to mind for the more general solution is the work I
did
> on line spanners, where we often want different skyline padding for the word
in
> the spanner and the line.

"Different skyline padding" is not the same as "skyline of an existing different
stencil".

[Keith, 2013-08-26 02:59:53]

The cost-benefit balance shows extra memory use, against potential 
gain that we have trouble imagining, so no net gain.

On 2013/08/25 08:22:01, mike7 wrote:

> The necessity that comes to mind for the more general solution is the work I
did
> on line spanners, where we often want different skyline padding for the word
in
> the spanner and the line. 

Note that my proposed reg-test included an example that was treated
that way, giving a bit of extra space around the .... portion, 
without messing up the skyline spacing on the 'fa'

  f'2^\markup { \pad-to-box #'(0 . 6) #'(0 . 1) fa...... }

Adding the specified box to the skyline works for that.

> That could be implemented by using the
> skyline-stencil primitive as I've coded it in the most recent patch set.

Demonstrate.

[mike7, 2013-08-26 04:08:17]

On 25 août 2013, at 16:04, dak@gnu.org wrote:

> On 2013/08/25 08:22:01, mike7 wrote:
>> On 25 août 2013, at 09:15, mailto:k-ohara5a5a@oco.net wrote:
> 
>> > A second stencil is not a very good data structure for reserving
> space.
>> > Skylines are better at this, and we can \once\override 'padding and
>> > 'horizon-padding to get padding that follows the outlines of the
>> > stencil.
>> [...]
>> If
>> we're going to do that, then we should think of the general problem
> we're trying
>> to solve.  To me, the general problem seems to be "how can we replace
> the
>> outline of one shape with that of another?"
> 
> No, that is _absolutely_ not the general problem here.  The general
> problem is: "how can we replace the outline/skylines of one stencil with
> a different outline/skyline?"
> 

It seems like what would make sense is for stencils to carry their own skyline
information, much like they carry their own dimension information.  That way, we
would replace a skyline in the same way that we replace the dimension.

This type of storage within the stencil data structure itself would be
consistent with the current use of skylines.  In fact, the X and Y dimensions
would no longer need to be specified and could come directly from the skylines.

Cheers,
MS

[dak, 2013-08-26 04:36:10]

On 2013/08/26 04:08:17, mike7 wrote:
> On 25 août 2013, at 16:04, mailto:dak@gnu.org wrote:
> 
> > On 2013/08/25 08:22:01, mike7 wrote:
> >> On 25 août 2013, at 09:15, mailto:k-ohara5a5a@oco.net wrote:
> > 
> >> > A second stencil is not a very good data structure for reserving
> > space.
> >> > Skylines are better at this, and we can \once\override 'padding and
> >> > 'horizon-padding to get padding that follows the outlines of the
> >> > stencil.
> >> [...]
> >> If
> >> we're going to do that, then we should think of the general problem
> > we're trying
> >> to solve.  To me, the general problem seems to be "how can we replace
> > the
> >> outline of one shape with that of another?"
> > 
> > No, that is _absolutely_ not the general problem here.  The general
> > problem is: "how can we replace the outline/skylines of one stencil with
> > a different outline/skyline?"
> > 
> 
> It seems like what would make sense is for stencils to carry their own skyline
> information, much like they carry their own dimension information.

So what?  Whether or not the skyline is cached or even persistent is completely
orthogonal to the issue at hand, namely what we want to be able to override a
stencil's skyline in stencil-integration with.

[mike7, 2013-08-26 04:51:50]

On 26 août 2013, at 07:36, dak@gnu.org wrote:

> On 2013/08/26 04:08:17, mike7 wrote:
>> On 25 août 2013, at 16:04, mailto:dak@gnu.org wrote:
> 
>> > On 2013/08/25 08:22:01, mike7 wrote:
>> >> On 25 août 2013, at 09:15, mailto:k-ohara5a5a@oco.net wrote:
>> >
>> >> > A second stencil is not a very good data structure for reserving
>> > space.
>> >> > Skylines are better at this, and we can \once\override 'padding
> and
>> >> > 'horizon-padding to get padding that follows the outlines of the
>> >> > stencil.
>> >> [...]
>> >> If
>> >> we're going to do that, then we should think of the general problem
>> > we're trying
>> >> to solve.  To me, the general problem seems to be "how can we
> replace
>> > the
>> >> outline of one shape with that of another?"
>> >
>> > No, that is _absolutely_ not the general problem here.  The general
>> > problem is: "how can we replace the outline/skylines of one stencil
> with
>> > a different outline/skyline?"
>> >
> 
>> It seems like what would make sense is for stencils to carry their own
> skyline
>> information, much like they carry their own dimension information.
> 
> So what?  Whether or not the skyline is cached or even persistent is
> completely orthogonal to the issue at hand, namely what we want to be
> able to override a stencil's skyline in stencil-integration with.

It changes the amount of work required to arrive at a solution.

I agree that overriding a skyline with a skyline is best way to do this.

Stencils have XY dimensions, as do grobs, and often the grob dimensions come
from the stencil but sometimes not.  Similarly, stencils should have
horizontal-vertical skylines, as do grobs, and often the grob skylines come from
the stencil but sometimes not.

That means that the Stencil class, which currently has members:

  Box dim_;
  SCM expr_;

would need a third member to hold skylines.

The issue is that if we start tackling it this way, it'd be a huge project and a
lot of juggling code around.  Keith's solution and the one I originally proposed
are more efficient expedients but seem like a band-aids on a larger issue.

Cheers,
MS

[dak, 2013-08-26 05:20:48]

On 2013/08/26 04:51:50, mike7 wrote:
> On 26 août 2013, at 07:36, mailto:dak@gnu.org wrote:
> 
> > On 2013/08/26 04:08:17, mike7 wrote:
> >> On 25 août 2013, at 16:04, mailto:dak@gnu.org wrote:
> > 
> >> > On 2013/08/25 08:22:01, mike7 wrote:
> >> >> On 25 août 2013, at 09:15, mailto:k-ohara5a5a@oco.net wrote:
> >> >
> >> >> > A second stencil is not a very good data structure for reserving
> >> > space.
> >> >> > Skylines are better at this, and we can \once\override 'padding
> > and
> >> >> > 'horizon-padding to get padding that follows the outlines of the
> >> >> > stencil.
> >> >> [...]
> >> >> If
> >> >> we're going to do that, then we should think of the general problem
> >> > we're trying
> >> >> to solve.  To me, the general problem seems to be "how can we
> > replace
> >> > the
> >> >> outline of one shape with that of another?"
> >> >
> >> > No, that is _absolutely_ not the general problem here.  The general
> >> > problem is: "how can we replace the outline/skylines of one stencil
> > with
> >> > a different outline/skyline?"
> >> >
> > 
> >> It seems like what would make sense is for stencils to carry their own
> > skyline
> >> information, much like they carry their own dimension information.
> > 
> > So what?  Whether or not the skyline is cached or even persistent is
> > completely orthogonal to the issue at hand, namely what we want to be
> > able to override a stencil's skyline in stencil-integration with.
> 
> It changes the amount of work required to arrive at a solution.

Indeed.  Instead of calling stencil-integrate on a "surrogate stencil" or
whatever for deriving a skyline, the respective stencil operation in
stencil-integrate will just plug in a a "surrogate skyline" directly specified
in the stencil operation and be done.

Which is less work.

[mike7, 2013-08-26 05:25:42]

On 26 août 2013, at 08:20, dak@gnu.org wrote:

> On 2013/08/26 04:51:50, mike7 wrote:
>> On 26 août 2013, at 07:36, mailto:dak@gnu.org wrote:
> 
>> > On 2013/08/26 04:08:17, mike7 wrote:
>> >> On 25 août 2013, at 16:04, mailto:dak@gnu.org wrote:
>> >
>> >> > On 2013/08/25 08:22:01, mike7 wrote:
>> >> >> On 25 août 2013, at 09:15, mailto:k-ohara5a5a@oco.net wrote:
>> >> >
>> >> >> > A second stencil is not a very good data structure for
> reserving
>> >> > space.
>> >> >> > Skylines are better at this, and we can \once\override
> 'padding
>> > and
>> >> >> > 'horizon-padding to get padding that follows the outlines of
> the
>> >> >> > stencil.
>> >> >> [...]
>> >> >> If
>> >> >> we're going to do that, then we should think of the general
> problem
>> >> > we're trying
>> >> >> to solve.  To me, the general problem seems to be "how can we
>> > replace
>> >> > the
>> >> >> outline of one shape with that of another?"
>> >> >
>> >> > No, that is _absolutely_ not the general problem here.  The
> general
>> >> > problem is: "how can we replace the outline/skylines of one
> stencil
>> > with
>> >> > a different outline/skyline?"
>> >> >
>> >
>> >> It seems like what would make sense is for stencils to carry their
> own
>> > skyline
>> >> information, much like they carry their own dimension information.
>> >
>> > So what?  Whether or not the skyline is cached or even persistent is
>> > completely orthogonal to the issue at hand, namely what we want to
> be
>> > able to override a stencil's skyline in stencil-integration with.
> 
>> It changes the amount of work required to arrive at a solution.
> 
> Indeed.  Instead of calling stencil-integrate on a "surrogate stencil"
> or whatever for deriving a skyline, the respective stencil operation in
> stencil-integrate will just plug in a a "surrogate skyline" directly
> specified in the stencil operation and be done.
> 
> Which is less work.

What is more work is what I talk about farther down in my previous e-mail.

It is possible to create a stencil primitive that works something like:

`(replacement-skylines ,left ,right, ,up ,down original-stencil)

but that seems kludgy.  It would be better and more consistent with currently
practices in the code base if stencils carried their own skyline information (as
they do their own dimensions), which would require a lot of juggling code
around.

Cheers,
MS

[dak, 2013-08-26 05:35:40]

On 2013/08/26 05:25:42, mike7 wrote:
> On 26 août 2013, at 08:20, mailto:dak@gnu.org wrote:
>
> > Indeed.  Instead of calling stencil-integrate on a "surrogate stencil"
> > or whatever for deriving a skyline, the respective stencil operation in
> > stencil-integrate will just plug in a a "surrogate skyline" directly
> > specified in the stencil operation and be done.
> > 
> > Which is less work.
> 
> What is more work is what I talk about farther down in my previous
> e-mail.

So why do you talk about that in the first place?

> It is possible to create a stencil primitive that works something like:
> 
> `(replacement-skylines ,left ,right, ,up ,down original-stencil)
> 
> but that seems kludgy.

Less kludgy than the code we are currently reviewing, and that's what
the current issue is.

> It would be better and more consistent with currently practices in
> the code base if stencils carried their own skyline information (as
> they do their own dimensions), which would require a lot of juggling
> code around.

In the context of reviewing this patch, this is a straw man argument.

[mike7, 2013-08-26 06:42:38]

On 26 août 2013, at 08:35, dak@gnu.org wrote:

> On 2013/08/26 05:25:42, mike7 wrote:
>> On 26 août 2013, at 08:20, mailto:dak@gnu.org wrote:
> 
>> > Indeed.  Instead of calling stencil-integrate on a "surrogate
> stencil"
>> > or whatever for deriving a skyline, the respective stencil operation
> in
>> > stencil-integrate will just plug in a a "surrogate skyline" directly
>> > specified in the stencil operation and be done.
>> >
>> > Which is less work.
> 
>> What is more work is what I talk about farther down in my previous
>> e-mail.
> 
> So why do you talk about that in the first place?

Because I'd rather spend more time implementing a solid system than less time
implementing a kludge.  What I'm proposing (making skylines a stencil data
member) is more work but seems less kludgy than creating a stencil primitive
containing skylines.

> 
>> It is possible to create a stencil primitive that works something
> like:
> 
>> `(replacement-skylines ,left ,right, ,up ,down original-stencil)
> 
>> but that seems kludgy.
> 
> Less kludgy than the code we are currently reviewing, and that's what
> the current issue is.

Agreed.

> 
>> It would be better and more consistent with currently practices in
>> the code base if stencils carried their own skyline information (as
>> they do their own dimensions), which would require a lot of juggling
>> code around.
> 
> In the context of reviewing this patch, this is a straw man argument.

In the context of figuring out the best way to do things, this is important.  It
would be better to decide what problem we're trying to solve and implement it
correctly now rather than creating a temporary solution that we do away with
later.

It seems like stencil expressions should not contain dimensional information. 
Where it does (like, for example, the stencil command 'translate-stencil defined
in define-stencil-commands.scm), this seems like legacy code that has not
evolved with LilyPond and is barely used.

So, as long as we're going to solve this problem, we should have a broader
discussion about how stencils are put together.  I argue that we should not be
putting dimension information in the stencil expression itself.  If this is the
case, then a replacement-skyline stencil primitive is not a good idea.  I am
definitely not going to work on a patch implementing skylines as a stencil data
member if people don't think it's a good idea, though, as it would take tons of
time and would represent a major reorganization of things.  However, if this is
the right way to go about solving the problem, it is better to spend the time
than create a stencil primitive (replace-skylines) that we are not happy with.

Cheers,
MS

[dak, 2013-08-26 08:41:42]

On 2013/08/26 06:42:38, mike7 wrote:
> On 26 août 2013, at 08:35, mailto:dak@gnu.org wrote:
>
> > So why do you talk about that in the first place?
> 
> Because I'd rather spend more time implementing a solid system than less time
> implementing a kludge.

In the context of this issue, we are currently talking about not doing
anything at all.  It seems like the best course forward with regard to
resolving the reported regression would be to just adapt Keith's
patch.

> What I'm proposing (making skylines a stencil data
> member) is more work but seems less kludgy than creating a stencil primitive
> containing skylines.

Again: this is _totally_ orthogonal to the issue at hand.  If you want
to talk about that, create a new issue rather than drive an existing
issue into the ground.

> >> It would be better and more consistent with currently practices in
> >> the code base if stencils carried their own skyline information (as
> >> they do their own dimensions), which would require a lot of juggling
> >> code around.
> > 
> > In the context of reviewing this patch, this is a straw man argument.
> 
> In the context of figuring out the best way to do things, this is important. 
It
> would be better to decide what problem we're trying to solve and implement it
> correctly now rather than creating a temporary solution that we do away with
> later.

Ok, let's get to the gist of it: for arranging items inside of a
markup, there are several concepts: stencil geometry, reference
points, escapement values (after placing a glyph at the current
reference point, move by the respective escapement for the current
direction of alignmen).  For lining up stencils, _only_ escapement
values and reference points are of interest.  The outlines come into
play _between_ independent markups, when we create a graphical
arrangement on the page.

So in general, we don't need outlines until the finished markup.
\with-dimensions operates on the dimensions interesting for working
within markups.  Unfortunately, we raised the expectation that those
are the same that are valid outside and advertised \with-dimensions
accordingly.  This is what this issue is about.

Generating a skyline from individual elements is a complex operation.
When done all at once, we have a complexity of O(n lg n) for n
elements.  When integrating a single element at a time, the complexity
rises to O(n^2).  So we don't want to assemble skylines one by one if
we can avoid it.  Markup functions assemble stencils to stencils.  If
we can calculate the skyline (when needed) in one go from the complete
stencil arrangement, we save a lot of effort, and that's the rule
rather than the exception.

So we don't want to carry skylines along with stencils that will not
be used individually on the page but will rather be integrated into a
markup.

> It seems like stencil expressions should not contain dimensional
> information.  Where it does (like, for example, the stencil command
> 'translate-stencil defined in define-stencil-commands.scm), this
> seems like legacy code that has not evolved with LilyPond and is
> barely used.

The dimensional information is used for stencil-add-at-edge, and most
certainly for all the various stencil stacking operations, forming
lines and columns.  "barely used" is a mischaracterization if I ever
saw one.

> So, as long as we're going to solve this problem, we should have a
> broader discussion about how stencils are put together.

In the course of this issue, we should fix the perceived regression.
It seems like Keith's code does that just fine.

> I argue that we should not be putting dimension information in the
> stencil expression itself.

Markups plug together stencils based on dimension information, and
that is what fonts do as well.  You can't row up letters convincingly
based on outlines since "visual distance" is a concept not captured
satisfactorily by computational treatment of outlines.  That is the
reason that escapement values are hardcoded by the font designer.

[mike7, 2013-08-26 09:11:39]

On 26 août 2013, at 11:41, dak@gnu.org wrote:

> On 2013/08/26 06:42:38, mike7 wrote:
>> On 26 août 2013, at 08:35, mailto:dak@gnu.org wrote:
> 
>> > So why do you talk about that in the first place?
> 
>> Because I'd rather spend more time implementing a solid system than
> less time
>> implementing a kludge.
> 
> In the context of this issue, we are currently talking about not doing
> anything at all.  It seems like the best course forward with regard to
> resolving the reported regression would be to just adapt Keith's
> patch.

I think that if we can devise a better system, the creation of a new stencil
primitive is unnecessary.  If we agree that we should implement a system where
stencils are carriers of their own skylines, then we should fix this bug using
that system.

But if there is no better system, then we should go with Keith's patch.

> 
>> What I'm proposing (making skylines a stencil data
>> member) is more work but seems less kludgy than creating a stencil
> primitive
>> containing skylines.
> 
> Again: this is _totally_ orthogonal to the issue at hand.  If you want
> to talk about that, create a new issue rather than drive an existing
> issue into the ground.

You're right that this discussion would be better on the issue tracker than on a
review of a patch - we can move it over there.

> 
>> >> It would be better and more consistent with currently practices in
>> >> the code base if stencils carried their own skyline information (as
>> >> they do their own dimensions), which would require a lot of
> juggling
>> >> code around.
>> >
>> > In the context of reviewing this patch, this is a straw man
> argument.
> 
>> In the context of figuring out the best way to do things, this is
> important.  It
>> would be better to decide what problem we're trying to solve and
> implement it
>> correctly now rather than creating a temporary solution that we do
> away with
>> later.
> 
> Ok, let's get to the gist of it: for arranging items inside of a
> markup, there are several concepts: stencil geometry, reference
> points, escapement values (after placing a glyph at the current
> reference point, move by the respective escapement for the current
> direction of alignmen).  For lining up stencils, _only_ escapement
> values and reference points are of interest.  The outlines come into
> play _between_ independent markups, when we create a graphical
> arrangement on the page.
> 
> So in general, we don't need outlines until the finished markup.
> \with-dimensions operates on the dimensions interesting for working
> within markups.  Unfortunately, we raised the expectation that those
> are the same that are valid outside and advertised \with-dimensions
> accordingly.  This is what this issue is about.
> 
> Generating a skyline from individual elements is a complex operation.
> When done all at once, we have a complexity of O(n lg n) for n
> elements.  When integrating a single element at a time, the complexity
> rises to O(n^2).  So we don't want to assemble skylines one by one if
> we can avoid it.  Markup functions assemble stencils to stencils.  If
> we can calculate the skyline (when needed) in one go from the complete
> stencil arrangement, we save a lot of effort, and that's the rule
> rather than the exception.
> 
> So we don't want to carry skylines along with stencils that will not
> be used individually on the page but will rather be integrated into a
> markup.
> 

I agree.  If stencils have both dimensions and skylines, we can use merging
based on dimensions as the default and skylines only when asked.  This would
avoid the time cost you talk about above while at the same time allowing one to
use skyline placement in the interior of a stencil when needed, as currently
skylines are only available for use between grobs.

>> It seems like stencil expressions should not contain dimensional
>> information.  Where it does (like, for example, the stencil command
>> 'translate-stencil defined in define-stencil-commands.scm), this
>> seems like legacy code that has not evolved with LilyPond and is
>> barely used.
> 
> The dimensional information is used for stencil-add-at-edge, and most
> certainly for all the various stencil stacking operations, forming
> lines and columns.  "barely used" is a mischaracterization if I ever
> saw one.

I believe you are talking about dimensional information in the stencil dimension
cache (dim_ in C++).  Above, I'm talking about the "stencil expression" (expr_
in C++).  Currently, stencil dimension information contained in the expression
is _only_ used to tell the backends where to place a stencil or how to make the
stencil skyline.  It is not used to line things up: the dim_ cache is used for
this.

> 
>> So, as long as we're going to solve this problem, we should have a
>> broader discussion about how stencils are put together.
> 
> In the course of this issue, we should fix the perceived regression.
> It seems like Keith's code does that just fine.

It does.  But we should only add a new stencil primitive if we're not convinced
that there is a better way to go about things.

> 
>> I argue that we should not be putting dimension information in the
>> stencil expression itself.
> 
> Markups plug together stencils based on dimension information,

But not based on dimension information in the stencil expression itself.  This
is all in the dim_ cache.  My whole argument is that putting dimension
information to be used for placing stencils in the stencil expression (expr_)
goes against the current stencil model.

> and
> that is what fonts do as well.

This is a good example.  Font escapement values are contained in tables outside
of the actual data holding the visual representation of the fonts.  Keith's
proposed patch and my first patch are the equivalent of rolling the escapement
data in with the data structure holding the letter.

To summarize: stencil expressions (expr_) currently do not contain information
for how to place stencils with respect to others.  They contain information
passed to the backends on how to lay out stencils.  If we adopt either Keith's
patch or my first one (which I created before I thought this fully over), we
will create one stencil primitive that uses the stencil's expression to
influence layout decisions.  This does not seem consistent with current practice
and I believe we should find a solution that avoids it.

Cheers,
MS

[wl_gnu.org, 2013-08-26 09:39:13]

> To summarize: stencil expressions (expr_) currently do not contain
> information for how to place stencils with respect to others.  They
> contain information passed to the backends on how to lay out
> stencils.  If we adopt either Keith's patch or my first one (which I
> created before I thought this fully over), we will create one
> stencil primitive that uses the stencil's expression to influence
> layout decisions.  This does not seem consistent with current
> practice and I believe we should find a solution that avoids it.

I think that Keith's (or your) patch is small enough to allow an
deviation from the `ideal' to fix a real bug.  Maybe it helps to add a
remark to the source code that this stencil primitive is a temporary
hack until a better, more generic solution gets implemented.


    Werner

[mike7, 2013-08-26 09:42:46]

On 26 août 2013, at 12:39, Werner LEMBERG <wl@gnu.org> wrote:

> 
>> To summarize: stencil expressions (expr_) currently do not contain
>> information for how to place stencils with respect to others.  They
>> contain information passed to the backends on how to lay out
>> stencils.  If we adopt either Keith's patch or my first one (which I
>> created before I thought this fully over), we will create one
>> stencil primitive that uses the stencil's expression to influence
>> layout decisions.  This does not seem consistent with current
>> practice and I believe we should find a solution that avoids it.
> 
> I think that Keith's (or your) patch is small enough to allow an
> deviation from the `ideal' to fix a real bug.  Maybe it helps to add a
> remark to the source code that this stencil primitive is a temporary
> hack until a better, more generic solution gets implemented.
> 

In French there's a saying "Temporary solutions have a way of becoming
permanent", which is why I'd like to avoid it if possible.  What we don't want
is for people to start using the stencil primitive if we know it's temporary. 
But if we can somehow prevent that (a sort of internal-only primitive) plus add
a big TODO somewhere, that'd be OK and would save time.

Cheers,
MS

[wl_gnu.org, 2013-08-26 09:49:02]

> What we don't want is for people to start using the stencil
> primitive if we know it's temporary.  But if we can somehow prevent
> that (a sort of internal-only primitive) plus add a big TODO
> somewhere, that'd be OK and would save time.

Even if it is public: Simply say in the stencil's documentation that
users must not use it!  Additionally, we already have `Internal
Properties' sections in the `lilypond-internals' documentation which
the user should ignore.


    Werner

[dak, 2013-08-26 09:50:24]

On 2013/08/26 09:11:39, mike7 wrote:
> On 26 août 2013, at 11:41, mailto:dak@gnu.org wrote:
> 
> > On 2013/08/26 06:42:38, mike7 wrote:
> >> On 26 août 2013, at 08:35, mailto:dak@gnu.org wrote:
> > 
> >> > So why do you talk about that in the first place?
> > 
> >> Because I'd rather spend more time implementing a solid system than
> > less time
> >> implementing a kludge.
> > 
> > In the context of this issue, we are currently talking about not doing
> > anything at all.  It seems like the best course forward with regard to
> > resolving the reported regression would be to just adapt Keith's
> > patch.
> 
> I think that if we can devise a better system, the creation of a new stencil
> primitive is unnecessary.  If we agree that we should implement a system where
> stencils are carriers of their own skylines, then we should fix this bug using
> that system.
> 
> But if there is no better system, then we should go with Keith's
> patch.

I am not getting through to you at all.  You are conflating orthogonal
issues with the result of blocking everything.

The semantics of \with-dimensions and its advertised behavior create
the situation that we need to reserve space for a stencil in the
generated skyline.

Matching the corresponding expectations has a certain awkwardness, but
we decided to go along with it, making the non-match of expectations a
Critical Regression.

All the big plans and mechanisms you propose here do _not_, I repeat
_not_ change even the slightest bit of \with-dimensions having an
impact on skylines.

Stencil dimensions can _not_ be replaced by skylines when assembling
markups for _both_ conceptual as well as performance reasons.  So the
problem of \with-dimensions being an ugly duckling is not at all
addressed by your complex proposals far outside the range of
addressing a regression.

> I agree.  If stencils have both dimensions and skylines, we can use merging
> based on dimensions as the default and skylines only when asked.

And that's exactly what we do now.

> This would avoid the time cost you talk about above while at the
> same time allowing one to use skyline placement in the interior of a
> stencil when needed, as currently skylines are only available for
> use between grobs.

Skylines are available when you call the stencil-integrate stuff on
stencil expressions.

> >> It seems like stencil expressions should not contain dimensional
> >> information.  Where it does (like, for example, the stencil command
> >> 'translate-stencil defined in define-stencil-commands.scm), this
> >> seems like legacy code that has not evolved with LilyPond and is
> >> barely used.
> > 
> > The dimensional information is used for stencil-add-at-edge, and most
> > certainly for all the various stencil stacking operations, forming
> > lines and columns.  "barely used" is a mischaracterization if I ever
> > saw one.
> 
> I believe you are talking about dimensional information in the stencil
dimension
> cache (dim_ in C++).

Not really a cache.  It's part of the stencil just like the stencil
expression is and assembled in parallel with it.

> It does.  But we should only add a new stencil primitive if we're
> not convinced that there is a better way to go about things.

There is none.  We want to bypass stencil integration, and there is no
primitive for doing that right now.

What you call a "better way" is adding a new stencil primitive that
does something much more complex but that can be tricked into behaving
like the simpler stencil primitive implemented by Keith at the cost of
additional complexity and using unrelated existing primitives like
filled-box that map to a rounded-box for the backends which gets
calculated but never makes it anywhere.

It's like refusing to implement an abs(x) primitive since you can
write sqrt(x*x) instead with only a moderate loss of performance, some
new error cases and a slight loss of precision.

Or like saying that TeX does not need an expandable way to repeat a
given sequence a run-time specified number of times since it has the
\romannumeral primitive allowing for things like

\def\recur#1{\csname rn#1\recur}
\def\rn#1{}
\def\rnm#1{\endcsname{#1}#1}
\def\replicate#1{\csname rn\expandafter\recur
  \romannumeral\number\number#1 000\endcsname\endcsname}
\message{\replicate{100}{I shall not ask for primitives in class. \space}}

Nobody wants to maintain that sort of thing.  So independent of the
theoretical possibility of abusing some kind of overgeneric construct
to-be-designed into the behavior we want to see, there is merit for
having a primitive that does just what we want it to do.

[dak, 2013-08-26 10:00:30]

On 2013/08/26 09:42:46, mike7 wrote:

> In French there's a saying "Temporary solutions have a way of
> becoming permanent", which is why I'd like to avoid it if possible.

Allow me to raise an eyebrow.

> What we don't want is for people to start using the stencil
> primitive if we know it's temporary.

That's bullshit since nobody assembles stencil expressions manually.
This is done using opaque functions like ly:stencil-translate.  If we
provide a respective function for creating this kind of expression,
swapping out the generated expression for something else will always
be possible.

[mike7, 2013-08-26 10:05:25]

On 26 août 2013, at 13:00, dak@gnu.org wrote:

> That's bullshit since nobody assembles stencil expressions manually.

I assemble stencil expressions manually.

> This is done using opaque functions like ly:stencil-translate.

ly:make-stencil is all over the place in the code and is a public function which
accepts a user-made expression. Are you saying this should not be a public
function? If it is the case that users cannot access stencil primitives, then it
is easier to swap things out.

Cheers,
MS

[thomasmorley651, 2013-08-26 10:12:20]

2013/8/26 Werner LEMBERG <wl@gnu.org>:
>
>> What we don't want is for people to start using the stencil
>> primitive if we know it's temporary.  But if we can somehow prevent
>> that (a sort of internal-only primitive) plus add a big TODO
>> somewhere, that'd be OK and would save time.
>
> Even if it is public: Simply say in the stencil's documentation that
> users must not use it!  Additionally, we already have `Internal
> Properties' sections in the `lilypond-internals' documentation which
> the user should ignore.
>
>
>     Werner

Well, I do use 'Internal Properties' in several definitions/function
and I post them on the user-list, if needed.
So be sure: it will be spreaded.

-Harm

[dak, 2013-08-26 10:13:31]

On 2013/08/26 10:05:25, mike7 wrote:
> On 26 août 2013, at 13:00, mailto:dak@gnu.org wrote:
> 
> > That's bullshit since nobody assembles stencil expressions manually.
> 
> I assemble stencil expressions manually.

If you meddle with internals, that's your own business.
 
> > This is done using opaque functions like ly:stencil-translate.
 
> ly:make-stencil is all over the place in the code and is a public function
which
> accepts a user-made expression.

The only documented way to generate "user-made" expressions is to create a
stencil with the normal ly:stencil-... or stencil-... API functions and extract
its expression with ly:stencil-expr.  There is _no_ guarantee that anything else
will work, and there is no guarantee that the format of stencil expressions will
be what it is now.

You of all people should know that since you juggled around quite a bit
reorganizing stencil primitives used in the backend.

[mike7, 2013-08-26 10:18:04]

On 26 août 2013, at 13:13, dak@gnu.org wrote:

> On 2013/08/26 10:05:25, mike7 wrote:
>> On 26 août 2013, at 13:00, mailto:dak@gnu.org wrote:
> 
>> > That's bullshit since nobody assembles stencil expressions manually.
> 
>> I assemble stencil expressions manually.
> 
> If you meddle with internals, that's your own business.
> 
>> > This is done using opaque functions like ly:stencil-translate.
> 
>> ly:make-stencil is all over the place in the code and is a public
> function which
>> accepts a user-made expression.
> 
> The only documented way to generate "user-made" expressions is to create
> a stencil with the normal ly:stencil-... or stencil-... API functions
> and extract its expression with ly:stencil-expr.  There is _no_
> guarantee that anything else will work, and there is no guarantee that
> the format of stencil expressions will be what it is now.
> 
> You of all people should know that since you juggled around quite a bit
> reorganizing stencil primitives used in the backend.

I was aware that the juggling would potential screw up scores (including my own)
but proposed it because it was a move in the right direction, meaning that I
knew we would never be adding these stencils back in again.

I think Harm is right that, irrespective of documentation, users use what's
available.  If we're going to add something, we want to be sure that it has a
certain degree of permanency.  I don't think we can treat it as an
implementation detail if it is publicly useable.

Cheers,
MS

[dak, 2013-08-26 10:31:50]

On 2013/08/26 10:18:04, mike7 wrote:

> I think Harm is right that, irrespective of documentation, users use what's
> available.

If people meddle with internals instead of using provided API functions, they
deserve whatever they get.

> If we're going to add something, we want to be sure that it has a
> certain degree of permanency.  I don't think we can treat it as an
> implementation detail if it is publicly useable.

_Anything_ is "publicly useable".  If that's supposed to be a criterion, we may
not change LilyPond ever again.  We try to support programming interfaces as
long as it is feasible.  But the way those are internally implemented is _not_
_ever_ guaranteed to remain the same.  If you mess with internals, all bets are
_off_.

Anything else would preclude us from improving how LilyPond works internally.

[wl_gnu.org, 2013-08-26 10:46:47]

>> Even if it is public: Simply say in the stencil's documentation
>> that users must not use it!  Additionally, we already have
>> `Internal Properties' sections in the `lilypond-internals'
>> documentation which the user should ignore.
> 
> Well, I do use 'Internal Properties' in several definitions/function
> and I post them on the user-list, if needed.

Interesting.  This effectively means that affected properties should
be moved out of the `Internal Properties' documentation sections...

Please create a bug tracker!  Additionally, it should probably be
discussed why such properties are originally treated as being
internal, and whether the way you use them is the `right' one.


    Werner

[mike7, 2013-08-26 16:32:24]

On 26 août 2013, at 13:31, dak@gnu.org wrote:

> On 2013/08/26 10:18:04, mike7 wrote:
> 
>> I think Harm is right that, irrespective of documentation, users use
> what's
>> available.
> 
> If people meddle with internals instead of using provided API functions,
> they deserve whatever they get.
> 
>> If we're going to add something, we want to be sure that it has a
>> certain degree of permanency.  I don't think we can treat it as an
>> implementation detail if it is publicly useable.
> 
> _Anything_ is "publicly useable".  If that's supposed to be a criterion,
> we may not change LilyPond ever again.  We try to support programming
> interfaces as long as it is feasible.  But the way those are internally
> implemented is _not_ _ever_ guaranteed to remain the same.  If you mess
> with internals, all bets are _off_.
> 

For me, the criterion is the use of "define-public" versus "define".  If a
function is "define-public" (like the ones in lily-library.scm), then we are
telling the user that it is useable.  Otherwise, it should not be public.

The same should be true of Scheme functions written in C++ - there should be a
way to make them publicly not-useable but available from .scm files.

Then, we could make ly:make-stencil private so that users could never use it.

Cheers,
MS

[dak, 2013-08-26 21:48:04]

On 2013/08/26 16:32:24, mike7 wrote:
> On 26 août 2013, at 13:31, mailto:dak@gnu.org wrote:

> > _Anything_ is "publicly useable".  If that's supposed to be a criterion,
> > we may not change LilyPond ever again.  We try to support programming
> > interfaces as long as it is feasible.  But the way those are internally
> > implemented is _not_ _ever_ guaranteed to remain the same.  If you mess
> > with internals, all bets are _off_.
> > 
> 
> For me, the criterion is the use of "define-public" versus "define".  If a
> function is "define-public" (like the ones in lily-library.scm), then we are
> telling the user that it is useable.  Otherwise, it should not be public.
> 
> The same should be true of Scheme functions written in C++ - there should be a
> way to make them publicly not-useable but available from .scm files.
> 
> Then, we could make ly:make-stencil private so that users could never use it.

Mike, that's inconsistent hogwash and a total strawman argument.  Nobody ever
claimed that one should not be able to use ly:make-stencil, and pretending that
I stated such nonsense is a cheap trick.  The question is rather what one should
be allowed to feed to ly:make-stencil.  It turns out that the first argument to
ly:make-stencil is a Scheme expression rather than some C++ data structure.

But that's an implementation detail.  That you are able to print such an
expression does not mean that its contents are in any way guaranteed or reliable
to put together manually.  A _lot_ of things in LilyPond happen to be assembled
as Scheme expressions.  And that's perfectly legitimate.  The purpose of using
C++ is not introducing barriers but providing efficiency.

[mike7, 2013-08-27 05:37:28]

On 27 août 2013, at 07:18, "Keith OHara" <k-ohara5a5a@oco.net> wrote:

> On Mon, 26 Aug 2013 02:11:28 -0700, Mike Solomon <mike@mikesolomon.org>
wrote:
> 
>> 
>> I think that if we can devise a better system, the creation of a new stencil
primitive is unnecessary.  If we agree that we should implement a system where
stencils are carriers of their own skylines, then we should fix this bug using
that system.
>> 
> 
> In any system of building skylines from markup, we want to regain the
spacing-control that we had with
>  \with-dimensions #(0 . 1) #(0 . 1) "text"
> and maybe we might have in the future
>  \with-shape {/* shape for purposes of spacing around music */} "text"
> 
> Maybe someday both of these will be implemented using a skyline-generator and
skyline-remover
>  \combine
>    \transparent \make-shape {/* shape for spacing */}
>    \no-skyline "text"
> 
> In any case we need a way to omit or ignore the usual skyline from some
markup.
> 
> 
> (1) At the moment, the stencil expressions are assembled first, when markup is
interpreted, and then stencil_traverser() reads the stencil expression to figure
the skylines.  So long as that approach remains in place we need some sign in
the stencil expression to be read by stencil_traverser() telling it to omit the
usual skyline around part of the stencil.  (I could re-use
'delay-stencil-evaluation, but think a new primitive is wiser.)
> 
> 
> (2) If someday the skylines are assembled in parallel with stencil-expressions
when the markup is interpreted, then \with-dimensions or \no-skyline could
simply throw away the skyline it built for "text" (in the examples above) and
there is no need for a 'with-dimensions' primitive.  (There would still be
primitives that move markup like 'translate-stencil' for things like \concat
{B\flat}.)
> 
> 
> Are systems organized as in (2) better systems than the one we have organized
as in (1) ?
> 

Good summary of the current state of things.  I'd argue that (2) is better.  The
original function definition for \with-dimension does not need a special
primitive because dimension information is encoded in the stencil in parallel
with stencil-expressions when the markup is interpreted.  The same could be true
of skylines, with the caveat that skylines should only be generated and used
when asked for to save computation time, and otherwise dimensions should be
used.

Cheers,
MS

[mike7, 2013-08-27 06:58:05]

On 27 août 2013, at 09:01, "Keith OHara" <k-ohara5a5a@oco.net> wrote:

> On Mon, 26 Aug 2013 22:37:17 -0700, Mike Solomon <mike@mikesolomon.org>
wrote:
> 
>> On 27 août 2013, at 07:18, "Keith OHara" <k-ohara5a5a@oco.net> wrote:
>> 
>>> (1) At the moment, the stencil expressions are assembled first, when markup
is interpreted, and then stencil_traverser() reads the stencil expression to
figure the skylines.  So long as that approach remains in place we need some
sign in the stencil expression to be read by stencil_traverser() telling it to
omit the usual skyline around part of the stencil.  (I could re-use
'delay-stencil-evaluation, but think a new primitive is wiser.)
>>> 
>>> 
>>> (2) If someday the skylines are assembled in parallel with
stencil-expressions when the markup is interpreted, then \with-dimensions or
\no-skyline could simply throw away the skyline it built for "text" (in the
examples above) and there is no need for a 'with-dimensions' primitive.  (There
would still be primitives that move markup like 'translate-stencil' for things
like \concat {B\flat}.)
>>> 
>>> Are systems organized as in (2) better systems than the one we have
organized as in (1) ?
>> 
>> Good summary of the current state of things.  I'd argue that (2) is better.
> 
> What, then, is your argument ?

What I put below - namely, that it is consistent with the previous practice of
stencils separating layout information from graphical content.

> 
>> The original function definition for \with-dimension does not need a special
primitive because dimension information is encoded in the stencil in parallel
with stencil-expressions when the markup is interpreted.
> 
> The only problem that I have heard of a special primitive is that it would be
redundant, *if* we change a system of type (2) for building skylines.  In the
current system, the primitive seems fine.

It's true that a new primitive works.  But, it seems that, on a more systemic
level, this is not a tenable solution as it would lead to the creation of many
different stencil primitives for different types of padding.

> 
>> The same could be true of skylines, with the caveat that skylines should only
be generated and used when asked for to save computation time, and otherwise
dimensions should be used.
> 
> How are skylines asked-for, and is it possible to know if they were asked for
while interpreting the markup ?
> {c4-\markup \whiteout \pad-to-box #(-0.5 . 9) #(-0.5 . 1.5) \concat {"Clarinet
in B" \flat}}
> 

class Stencil {
  private :
    Scheme expr_;
    Box dim_;
    Skyline_pair *vskys_; // set to 0 in the constructor
    Skyline_pair *hskys_; // set to 0 in the constructor

  public :
    Skyline_pair* get_vskys () { if (!vskys_) create_vskys (); return vskys_; }
}

So, the idea is that we only ever create the skylines if they are asked for or
if we use something like pad-to-box.  Then, if we are going to combine two
stencils and neither of them have skylines set already, we do not calculate
their skylines yet.  If one of them has skylines set, then somebody must have
set it with something like with-dimension and we calculate the skyline of the
other to create a composite skyline.

pad-to-box would not require the finding of the skyline of Clarinet in Bb, since
we already have its X and Y dimensions.  The skyline resulting from the
pad-to-box call would be calculated off of that.

> The current system (1) seems more efficient at generating only the needed
skylines.  We only need the skyline if the markup ends up in a TextScript with
'vertical-skylines set, as opposed to titling markup, and then
stencil_traverser() works through the stencil expression and sees that it need
not traverse through argument of \pad-to-box and need not trace the letters in
"Clarinet in B"
> 

In the pseudocode above, imagine that a call to create_vskys () would only
happen in the stencil traverser and in certain padding or concatenating
scenarios.  This would be less efficient than (1) but not by much.

Cheers,
MS

[dak, 2013-08-27 08:09:09]

On 2013/08/27 06:58:05, mike7 wrote:
> On 27 août 2013, at 09:01, "Keith OHara" <mailto:k-ohara5a5a@oco.net> wrote:

> > How are skylines asked-for, and is it possible to know if they were asked
for
> while interpreting the markup ?
> > {c4-\markup \whiteout \pad-to-box #(-0.5 . 9) #(-0.5 . 1.5) \concat
{"Clarinet
> in B" \flat}}
> > 
> 
> class Stencil {
>   private :
>     Scheme expr_;
>     Box dim_;
>     Skyline_pair *vskys_; // set to 0 in the constructor
>     Skyline_pair *hskys_; // set to 0 in the constructor
> 
>   public :
>     Skyline_pair* get_vskys () { if (!vskys_) create_vskys (); return vskys_;
}
> }
> 
> So, the idea is that we only ever create the skylines if they are asked for or
> if we use something like pad-to-box.  Then, if we are going to combine two
> stencils and neither of them have skylines set already, we do not calculate
> their skylines yet.

Either you don't listen or you don't think about your code.  Delaying skyline
calculation until it is needed does _nothing_ whatsoever with regard to
computational complexity if the skyline is needed eventually, which it will
almost always be.

What's much worse is that the performance drops drastically if the evaluation is
not done in bulk but structured along the lines of the underlying expression.

Since the calculation of a total skyline rather than predetermined sub-skylines
is free to make more efficient choices for its subdivision of the data, one
wants to avoid calculating _any_ intermediate skylines.

Assigning individual skylines to the respective parts of a composite stencil
expression forces a particular evaluation order.  This leads to the typical
O(n^2) behavior for accumulating a skyline bit by bit rather than by a
size-driven divide-and-conquer approach taking O(n lg n).

[mike7, 2013-08-27 10:27:22]

On 27 août 2013, at 11:09, dak@gnu.org wrote:

> On 2013/08/27 06:58:05, mike7 wrote:
>> On 27 août 2013, at 09:01, "Keith OHara" <mailto:k-ohara5a5a@oco.net>
> wrote:
> 
>> > How are skylines asked-for, and is it possible to know if they were
> asked for
>> while interpreting the markup ?
>> > {c4-\markup \whiteout \pad-to-box #(-0.5 . 9) #(-0.5 . 1.5) \concat
> {"Clarinet
>> in B" \flat}}
>> >
> 
>> class Stencil {
>>   private :
>>     Scheme expr_;
>>     Box dim_;
>>     Skyline_pair *vskys_; // set to 0 in the constructor
>>     Skyline_pair *hskys_; // set to 0 in the constructor
> 
>>   public :
>>     Skyline_pair* get_vskys () { if (!vskys_) create_vskys (); return
> vskys_; }
>> }
> 
>> So, the idea is that we only ever create the skylines if they are
> asked for or
>> if we use something like pad-to-box.  Then, if we are going to combine
> two
>> stencils and neither of them have skylines set already, we do not
> calculate
>> their skylines yet.
> 
> Either you don't listen or you don't think about your code.  Delaying
> skyline calculation until it is needed does _nothing_ whatsoever with
> regard to computational complexity if the skyline is needed eventually,
> which it will almost always be.

I don't see how this system precludes O(n log n) in the majority of cases.  Take
:

\header {
  title = "foo"
  composer "bar"
}

\markup { mit herzlichen foo }

\new Staff \relative c'' { a1^"und bar" }

The skylines for the title, composer, and top-level markup will never be
calculated.  For everything in the staff, there are no stencil skyline
overrides, which means a stencil's get_skyline method will not be called before
it is fully formed, which means that the existing divide-and-conquer method will
be used to traverse the stencils' expressions and create the skyline.  This
keeps the complexity at O(n log n) instead of O(n^2).

The only cases where O(n^2) behavior would be exhibited is if the stencil's
skyline creation is triggered before a call to translate_axis, translate,
rotate, rotate_degrees_absolute, rotate, scale, add_at_edge, etc. in which case
we'd need to perform these operations on the skylines as well.  But the only
time that we would trigger skyline creation is when we want the skyline to
differ from the natural one of the stencil.

Cheers,
MS

[mike7, 2013-08-28 06:30:48]

On 28 août 2013, at 05:28, "Keith OHara" <k-ohara5a5a@oco.net> wrote:

> On Mon, 26 Aug 2013 23:57:58 -0700, Mike Solomon <mike@mikesolomon.org>
wrote:
> 
>> On 27 août 2013, at 09:01, "Keith OHara" <k-ohara5a5a@oco.net> wrote:
>> 
>>> On Mon, 26 Aug 2013 22:37:17 -0700, Mike Solomon <mike@mikesolomon.org>
wrote:
>>> 
>>>> I'd argue that (2) is better.
>>> 
>>> What, then, is your argument ?
>> 
>> What I put below - namely, that it is consistent with the previous practice
of stencils separating layout information from graphical content.
>> 
> 
> That kind of separation is not always good.
> 
> We like having the 'staff-padding' between the staff and tempo mark separate
from the text "Adagio", because we usually want to control the padding globally
for all tempo marks.
> 
> We would like to keep the space request closely associated with the graphic
in
> { d'2-\markup {\italic { c r e s c. } \pad-around #0.5 {- - -} } d' b b }
> so that as the font and inter-word space change, the space request follows.
> 
> 
>> It's true that a new primitive works.  But, it seems that, on a more systemic
level, this is not a tenable solution as it would lead to the creation of many
different stencil primitives for different types of padding.
>> 
> 
> The new stencil primitive in the patch set I have posted now does not assume
any kind of padding.  You said the stencil expression should not contain data
influencing layout, so I went back to the patch set that simply turns off
skylines for markup using \with-dimensions and similar.
> 
> I withdrew the patches that preserve the space around the "- - -" leader,
because they put data based on the #0.5 into the stencil expression.
> 
> Of course I think it would be better to allow box dimensions in the stencil
expression.  Boxes are simple enough to enter as coordinates in markup
expressions like \pad-to-box, they are a useful building block for arbitrary
skylines, and the current code builds skylines from the stencil expression.
> 

If we are willing to say that boxes should be an exception because of how
primitive they are, then it makes more sense to make an exception for them, as
they can be used in concord to create more complex structures.  In that case, we
may want to accept a list of boxes (or a list of quadrilaterals) instead of just
a box, as at that point we can approximate any shape.

Cheers,
MS

[MikeSol, 2013-08-28 08:48:42]

Overrides stencil skylines with other stencils

[MikeSol, 2013-08-28 08:49:23]

Fixes harp stencils

[mike7, 2013-08-28 08:52:02]

On 28 août 2013, at 09:47, "Keith OHara" <k-ohara5a5a@oco.net> wrote:

> On Tue, 27 Aug 2013 23:30:32 -0700, Mike Solomon <mike@mikesolomon.org>
wrote:
> 
>> On 28 août 2013, at 05:28, "Keith OHara" <k-ohara5a5a@oco.net> wrote:
>> 
>>> Of course I think it would be better to allow box dimensions in the stencil
expression.  Boxes are simple enough to enter as coordinates in markup
expressions like \pad-to-box, they are a useful building block for arbitrary
skylines, and the current code builds skylines from the stencil expression.
>>> 
>> 
>> If we are willing to say that boxes should be an exception because of how
primitive they are, then it makes more sense to make an exception for them, as
they can be used in concord to create more complex structures.  In that case, we
may want to accept a list of boxes (or a list of quadrilaterals) instead of just
a box, as at that point we can approximate any shape.
>> 
> 
> Another reason for making an exception for boxes is the preexistence of 
\pad-to-box  as a markup command, while we now base our padding on skylines
derived from the stencil expressions.
> 
> If we still think this way in a day, I'll repost the patch that adds the
stencil-primitive 'with-dimensions,  supports {cresc. \pad-around 0.5 "- - -"},
and removes the faint box around harp-pedals.
> 
> A stencil-primitive 'with-dimension could be extended in a natural way to a
list of boxes, if we want that in the future.
> 

Just for kicks, I've proposed a new patch along with a new issue 3255.  It's
worth mulling over - whatever solution we implement should fix this issue as
well.

Cheers,
MS

[MikeSol, 2013-08-28 09:04:58]

fixes braino

[Keith, 2013-09-04 18:36:16]

https://codereview.appspot.com/12957047/diff/88001/lily/stencil-integral.cc
File lily/stencil-integral.cc (right):

https://codereview.appspot.com/12957047/diff/88001/lily/stencil-integral.cc#n...
lily/stencil-integral.cc:917: }
My best guess for the cause of the extra memory, showing in the `make check`
profile output, is this loop written as tail recursion:

stencil_traverser (PangoMatrix trans, SCM expr) {
  [...]
  else if (scm_car (expr) == ly_symbol2scm ("skyline-stencil")) {
      SCM skyline_stencil = scm_cadr (expr);
      return stencil_traverser (trans, skyline_stencil);
  }

Maybe having 'skyline_stencil' as an automatic variable and also a function
parameter, which would be optimized to a different automatic variable 'expr' in
the same function, confuses the reference counting in Guile's
garbage-collection.

The current patches for our tracked issues 3255 and 3522 avoid creating SCM data
types in the C code, and did not show extra memory in the profile output, so
seem be safe enough.

[dak, 2013-09-04 19:00:22]

On 2013/09/04 18:36:16, Keith wrote:
> https://codereview.appspot.com/12957047/diff/88001/lily/stencil-integral.cc
> File lily/stencil-integral.cc (right):
> 
>
https://codereview.appspot.com/12957047/diff/88001/lily/stencil-integral.cc#n...
> lily/stencil-integral.cc:917: }
> My best guess for the cause of the extra memory, showing in the `make check`
> profile output, is this loop written as tail recursion:
> 
> stencil_traverser (PangoMatrix trans, SCM expr) {
>   [...]
>   else if (scm_car (expr) == ly_symbol2scm ("skyline-stencil")) {
>       SCM skyline_stencil = scm_cadr (expr);
>       return stencil_traverser (trans, skyline_stencil);
>   }
> 
> Maybe having 'skyline_stencil' as an automatic variable and also a function
> parameter, which would be optimized to a different automatic variable 'expr'
in
> the same function, confuses the reference counting in Guile's
> garbage-collection.

No, GUILE just scans the stack.  Variable names are not relevant.  Since we are
doing our statistics without optimisations, the "tail recursion" will actually
be a full recursion.  However, by the time the statistics are done, the stack
frame is no longer there.  So this should not make a difference.