Allows inner-markup spacing using skylines.

Allows inner-markup spacing using skylines.

Uses the stencil skyline drawing method in stencil-integral.cc to provide
skyline spacing in Stencil::add_at_edge. Adds corresponding ly module
and Scheme functions.

[dak, 2013-08-28 21:17:20]

Where's the point?  We don't use that kind of arrangement in our code, and it's
not efficiently doable anyway.

A conceivable application would be for determining systems spacing manually (for
example, generating sequences of TeX \vspace and \break for LilyPond-book that
will reserve normal rectangular space at page breaks, but interleaving staff
spacing when not broken), but this function is mostly useless for that purpose. 
One can only indirectly try to guess the size of the spacing, one can't reuse a
skyline pair(!) when one wants to compare against several neighbors (requiring
_two_ calculation of each skyline pair when assembling a vertical arrangement),
and the spacing is appears to be determined differently than the vertical
spacing engine does it usually.

And for emulating vertical spacing, the stack-stencil operation would be more
important anyway.  But what is really wanted is a primitive just giving out
distance, and calling conventions that avoid a double calculation of each
skyline pair when stacking things.

https://codereview.appspot.com/13341044/diff/1/lily/stencil.cc
File lily/stencil.cc (right):

https://codereview.appspot.com/13341044/diff/1/lily/stencil.cc#newcode271
lily/stencil.cc:271: add_at_edge (a, d, s, padding, false);
That's nonsensical.  C++ knows defaults arguments for functions.

https://codereview.appspot.com/13341044/diff/1/lily/stencil.cc#newcode313
lily/stencil.cc:313: offset = (*first_skyp)[d].distance ((*second_skyp)[-d]);
This is an expensive non-cached operation, and yet this calculates skyline
_pairs_ and ignores half of them.

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

https://codereview.appspot.com/13341044/diff/1/scm/define-markup-commands.scm...
scm/define-markup-commands.scm:2038: (ly:stencil-combine-at-edge m1 axis dir m2
0.0 #t)))
Why use the same function in the first place when the last argument is never
used as a variable?

[MikeSol, 2013-08-29 06:56:20]

Implements David's suggestions

[MikeSol, 2013-08-29 06:57:53]

The point is to allow users to achieve the same snug skyline spacing between
stencils that is achieved between grobs.

I agree that it is inefficient as skylines are never cached, but as it is not
default beahvior anywhere in the code base, it allows people to decide on the
efficiency versus snugness-of-placement trade-off.  For people creating complex
markups, this is useful.

https://codereview.appspot.com/13341044/diff/1/lily/stencil.cc
File lily/stencil.cc (right):

https://codereview.appspot.com/13341044/diff/1/lily/stencil.cc#newcode271
lily/stencil.cc:271: add_at_edge (a, d, s, padding, false);
On 2013/08/28 21:17:20, dak wrote:
> That's nonsensical.  C++ knows defaults arguments for functions.

Done.

https://codereview.appspot.com/13341044/diff/1/lily/stencil.cc#newcode313
lily/stencil.cc:313: offset = (*first_skyp)[d].distance ((*second_skyp)[-d]);
On 2013/08/28 21:17:20, dak wrote:
> This is an expensive non-cached operation, and yet this calculates skyline
> _pairs_ and ignores half of them.

Made a Skyline-only function.

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

https://codereview.appspot.com/13341044/diff/1/scm/define-markup-commands.scm...
scm/define-markup-commands.scm:2038: (ly:stencil-combine-at-edge m1 axis dir m2
0.0 #t)))
On 2013/08/28 21:17:20, dak wrote:
> Why use the same function in the first place when the last argument is never
> used as a variable?

What would be a better way to achieve this?

[dak, 2013-08-29 08:25:40]

On 2013/08/29 06:57:53, MikeSol wrote:
> The point is to allow users to achieve the same snug skyline spacing between
> stencils that is achieved between grobs.
> 
> I agree that it is inefficient as skylines are never cached, but as it is not
> default beahvior anywhere in the code base, it allows people to decide on the
> efficiency versus snugness-of-placement trade-off.  For people creating
complex
> markups, this is useful.

Mike, the question is not whether or not it is useful to work with the
distance of skylines.  The problem is that you just pile on complex
derived functionality nilly-willy without stopping to think what the
actually sensible building blocks would be.

And then you defend a lousy design with "it's useful" and "people can
decide themselves on the tradeoff between using a lousy inefficient
design or not".  While it is a popular belief that democracy is a
cure-all, that giving people a choice between different evils somehow
makes them all good, that's not the way to do software development.

Skylines are elusive for markups yet are used in positioning, yes and
that's a problem.  But the solution to that is not to add some more
functions that magically and intransparently do something related to
skylines which might be abused for figuring out some skyline-related
properties.

The solution is to add _proper_ interfaces to skylines.  Functions
that can figure out distances and so on.  Between skylines.

And a function to query the skyline(s) of a stencil, yes.

Now querying skylines is expensive, so the question is how to cache
them.  The important thing to note is that skylines are not actually a
property of stencils but rather of stencil expressions (combined
stencils don't actually contain the original stencils but only their
respective stencil expressions, so caching in the stencils themselves
does not actually help at all).  The current form of stencil
expressions does not make it easy to tack a skyline cache on without
changing object identity, so it might be worth considering a different
representation.

Once that's sorted out satisfactorily and supported by primitives, one
can stack on uses of skylines.

But increasing the number of functions that magically and
intransparently somehow work with skylines not otherwise accessible is
just a bad idea, and hiding this kind of thing in existing functions
is even worse.

[mike7, 2013-08-29 11:56:08]

On 29 août 2013, at 10:25, dak@gnu.org wrote:

> On 2013/08/29 06:57:53, MikeSol wrote:
>> The point is to allow users to achieve the same snug skyline spacing
> between
>> stencils that is achieved between grobs.
> 
>> I agree that it is inefficient as skylines are never cached, but as it
> is not
>> default beahvior anywhere in the code base, it allows people to decide
> on the
>> efficiency versus snugness-of-placement trade-off.  For people
> creating complex
>> markups, this is useful.
> 
> Mike, the question is not whether or not it is useful to work with the
> distance of skylines.  The problem is that you just pile on complex
> derived functionality nilly-willy without stopping to think what the
> actually sensible building blocks would be.

What would they be?

> 
> 
> The solution is to add _proper_ interfaces to skylines.  Functions
> that can figure out distances and so on.  Between skylines.
> 

Doesn't that exist already with Skyline::distance ?

> And a function to query the skyline(s) of a stencil, yes.
> 

Doesn't this already exist with Stencil::skyline_pair_from_stencil and
Stencil::skyline_from_stencil (proposed in this patch) or the function in
current master Stencil::skylines_from_stencil ?

> Now querying skylines is expensive, so the question is how to cache
> them.

Agreed.

> The important thing to note is that skylines are not actually a
> property of stencils but rather of stencil expressions (combined
> stencils don't actually contain the original stencils but only their
> respective stencil expressions, so caching in the stencils themselves
> does not actually help at all).

If skylines rode around with stencils (like dimensions do), we'd have to do
operations on the skylines when we do operations on the expression and
dimension.

> The current form of stencil
> expressions does not make it easy to tack a skyline cache on without
> changing object identity, so it might be worth considering a different
> representation.

This is certainly a possibility.

> 
> Once that's sorted out satisfactorily and supported by primitives, one
> can stack on uses of skylines.
> 
> But increasing the number of functions that magically and
> intransparently somehow work with skylines not otherwise accessible is
> just a bad idea, and hiding this kind of thing in existing functions
> is even worse.

It would be a good idea to have Scheme bindings for skylines and to make them
accessible, similarly to how dimensions are extensible and changeable.

What is your proposal for a good interface with Skylines?  This patch allows one
to ask LilyPond "find the distance between two stencils' shapes along the
horizontal or vertical axis and move shift a stencil by that amount" and
LilyPond does that.  Where is there transparency needed?  In
side-position-interface.cc and axis-group-interface.cc, the skyline operations
are not transparent (meaning not visible to the user) but the end result (move
object X above or below Y based on the shapes of the objects) is achieved.  It
is also not transparent, but I wouldn't have thought it needed to be - I think
people just want it to work and we should do a good job implementing it cleanly.
 I'm of course open to proposals on how to implement it cleanl.y

Cheers,
MS