Discussion:
Adds dimension stencil command to correct with-dimension (issue 12957047)
(too old to reply)
k***@oco.net
2013-08-22 06:54:36 UTC
Permalink
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).

https://codereview.appspot.com/12957047/
l***@googlemail.com
2013-08-22 07:31:14 UTC
Permalink
This looks very promising! Thanks for the work on this.

https://codereview.appspot.com/12957047/
m***@gmail.com
2013-08-22 08:05:14 UTC
Permalink
I had time to implement everything but the delayed markup stuff. I'll
try to get to that soon...

https://codereview.appspot.com/12957047/
m***@gmail.com
2013-08-22 08:13:54 UTC
Permalink
It was easier to implement the delayed stuff than I expected, so I got
it done.

https://codereview.appspot.com/12957047/
l***@googlemail.com
2013-08-24 07:18:52 UTC
Permalink
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/
Mike Solomon
2013-08-24 08:04:57 UTC
Permalink
Post by l***@googlemail.com
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
d***@gnu.org
2013-08-24 08:25:14 UTC
Permalink
Post by l***@googlemail.com
LGTM, except of using `surrogate' in the name. Given that the
stencil
Post by l***@googlemail.com
dimensions are actively overridden on request, I would like rather
like
Post by l***@googlemail.com
having `override'. For me, the word `surrogate' suggests something
passive.
https://codereview.appspot.com/12957047/
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.

https://codereview.appspot.com/1
p***@gmail.com
2013-08-24 09:47:28 UTC
Permalink
Post by d***@gnu.org
Post by l***@googlemail.com
LGTM, except of using `surrogate' in the name. Given that the
stencil
Post by d***@gnu.org
Post by l***@googlemail.com
dimensions are actively overridden on request, I would like rather
like
Post by d***@gnu.org
Post by l***@googlemail.com
having `override'. For me, the word `surrogate' suggests
something
Post by d***@gnu.org
Post by l***@googlemail.com
passive.
https://codereview.appspot.com/12957047/
It doesn't matter what the definition reads. English has a live
vocabulary of
Post by d***@gnu.org
over 150000 words and there is no need to use all of them in
identifiers. We
Post by d***@gnu.org
want our identifier names to be boring and following our own
terminology
Post by d***@gnu.org
elsewhere as well as that of other programs.
If you pick a non-standard word with slightly different connotations,
people
Post by d***@gnu.org
will think that the slightly different connotations are important,
rather than
Post by d***@gnu.org
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/12
m***@mikesolomon.org
2013-08-24 10:05:10 UTC
Permalink
Sent from my iPhone
Post by k***@oco.net
Post by d***@gnu.org
Post by l***@googlemail.com
LGTM, except of using `surrogate' in the name. Given that the
stencil
Post by d***@gnu.org
Post by l***@googlemail.com
dimensions are actively overridden on request, I would like rather
like
Post by d***@gnu.org
Post by l***@googlemail.com
having `override'. For me, the word `surrogate' suggests
something
Post by d***@gnu.org
Post by l***@googlemail.com
passive.
https://codereview.appspot.com/12957047/
It doesn't matter what the definition reads. English has a live
vocabulary of
Post by d***@gnu.org
over 150000 words and there is no need to use all of them in
identifiers. We
Post by d***@gnu.org
want our identifier names to be boring and following our own
terminology
Post by d***@gnu.org
elsewhere as well as that of other programs.
If you pick a non-standard word with slightly different connotations,
people
Post by d***@gnu.org
will think that the slightly different connotations are important,
rather than
Post by d***@gnu.org
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
d***@gnu.org
2013-08-24 10:31:39 UTC
Permalink
Post by m***@mikesolomon.org
The stencil command takes the skyline of stencil X and uses it as a
replacement
Post by m***@mikesolomon.org
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.

https://codereview.appspot.com/12957047/
d***@gnu.org
2013-08-24 10:38:35 UTC
Permalink
Post by m***@mikesolomon.org
Post by m***@mikesolomon.org
The stencil command takes the skyline of stencil X and uses it as a
replacement
Post by m***@mikesolomon.org
for skyline Y. We could use replacement-skyline-stencil as the
command.
Post by m***@mikesolomon.org
I'd just use skyline-stencil here. If specified, it is the stencil
used for
Post by m***@mikesolomon.org
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?

https://codereview.appspot.com/12957047/
Mike Solomon
2013-08-24 13:19:15 UTC
Permalink
Post by m***@mikesolomon.org
Post by m***@mikesolomon.org
Post by m***@mikesolomon.org
The stencil command takes the skyline of stencil X and uses it as a
replacement
Post by m***@mikesolomon.org
for skyline Y. We could use replacement-skyline-stencil as the
command.
Post by m***@mikesolomon.org
I'd just use skyline-stencil here. If specified, it is the stencil
used for
Post by m***@mikesolomon.org
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
d***@gnu.org
2013-08-24 13:36:50 UTC
Permalink
Post by Mike Solomon
The question I'm asking is "How can I allow the user to replace the
skyline of a
Post by Mike Solomon
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.

https://codereview.appspot.com/12957047/
Mike Solomon
2013-08-24 16:26:11 UTC
Permalink
Post by Mike Solomon
Post by Mike Solomon
The question I'm asking is "How can I allow the user to replace the
skyline of a
Post by Mike Solomon
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
d***@gnu.org
2013-08-24 16:19:27 UTC
Permalink
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#newcode55
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#newcode915
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.scm
File scm/define-markup-commands.scm (right):

https://codereview.appspot.com/12957047/diff/46001/scm/define-markup-commands.scm#newcode730
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#newcode1997
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.

https://codereview.appspot.com/12957047/
m***@gmail.com
2013-08-24 16:33:40 UTC
Permalink
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#newcode55
lily/paper-system.cc:55: if (head == ly_symbol2scm ("skyline-stencil"))
Post by d***@gnu.org
It seems awkward and error-prone to go through a number of stencil
types here
Post by d***@gnu.org
just for fishing out footnotes. Of course, this is just another layer
of
Post by d***@gnu.org
ugliness on top of existing ugliness, but it would seem that we
actually want a
Post by d***@gnu.org
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#newcode915
lily/stencil-integral.cc:915: SCM skyline_stencil = scm_cadr (expr);
Post by d***@gnu.org
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.scm
File scm/define-markup-commands.scm (right):

https://codereview.appspot.com/12957047/diff/46001/scm/define-markup-commands.scm#newcode730
scm/define-markup-commands.scm:730: (make-box-skyline-stencil
Post by d***@gnu.org
Stupid question: _iff_ you store a _whole_ replacement skyline anyway,
shouldn't
Post by d***@gnu.org
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
Post by d***@gnu.org
corners, though)?
Of course this would be incompatible with previous behavior, but more
likely
Post by d***@gnu.org
matching the expectations?
I am not sure that replacement skylines are the right thing anyway,
but it seems
Post by d***@gnu.org
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)))
Post by d***@gnu.org
A filled-box stencil seems like serious overkill here when all you
want is
Post by d***@gnu.org
setting the dimensions. A stencil operator for just setting
dimensions seems
Post by d***@gnu.org
less awkward. There are two possibilities: have it produce an empty
stencil
Post by d***@gnu.org
expression but with dimensions, and then use make-skyline-stencil on
it. Or
Post by d***@gnu.org
have it contain an inner expression anyway.
In that case, make-box-skyline-stencil (what an awkward name) does not
need to
Post by d***@gnu.org
call either make-filled-box-stencil nor contain skyline-stencil
anyway.
Post by d***@gnu.org
It means that one needs one more stencil primitive _if_ one wants to
support
Post by d***@gnu.org
make-skyline-stencil. But it avoids juggling with an obscure
combination of
Post by d***@gnu.org
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.

https://codereview.appspot.com/12957047/
k***@oco.net
2013-08-25 06:15:52 UTC
Permalink
Post by m***@gmail.com
Post by d***@gnu.org
Stupid question: _iff_ you store a _whole_ replacement skyline
anyway,
Post by m***@gmail.com
Post by d***@gnu.org
shouldn't pad-around just shift the original left/right/up/down
skylines
Post by m***@gmail.com
Post by d***@gnu.org
left/right/up/down by the given amount (don't ask me what to do
about the
Post by m***@gmail.com
Post by d***@gnu.org
corners, though)?
It's a good idea, but stencils don't have left/right/up/down, so it'd
be hard to
Post by m***@gmail.com
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.

https://codereview.appspot.com/12957047/
Mike Solomon
2013-08-25 08:21:49 UTC
Permalink
Post by d***@gnu.org
Post by m***@gmail.com
Post by d***@gnu.org
Stupid question: _iff_ you store a _whole_ replacement skyline
anyway,
Post by m***@gmail.com
Post by d***@gnu.org
shouldn't pad-around just shift the original left/right/up/down
skylines
Post by m***@gmail.com
Post by d***@gnu.org
left/right/up/down by the given amount (don't ask me what to do
about the
Post by m***@gmail.com
Post by d***@gnu.org
corners, though)?
It's a good idea, but stencils don't have left/right/up/down, so it'd
be hard to
Post by m***@gmail.com
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
d***@gnu.org
2013-08-25 13:04:14 UTC
Permalink
Post by Mike Solomon
Post by k***@oco.net
A second stencil is not a very good data structure for reserving
space.
Post by Mike Solomon
Post by k***@oco.net
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
Post by Mike Solomon
to solve. To me, the general problem seems to be "how can we replace
the
Post by Mike Solomon
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.
Post by Mike Solomon
So far, this question is only ever
asked in the specific form "how can we replace the outline of one
shape with a
Post by Mike Solomon
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.
Post by Mike Solomon
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
Post by Mike Solomon
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.
Post by Mike Solomon
The necessity that comes to mind for the more general solution is the
work I did
Post by Mike Solomon
on line spanners, where we often want different skyline padding for
the word in
Post by Mike Solomon
the spanner and the line.
"Different skyline padding" is not the same as "skyline of an existing
different
Mike Solomon
2013-08-26 04:08:10 UTC
Permalink
Post by k***@oco.net
Post by Mike Solomon
Post by k***@oco.net
A second stencil is not a very good data structure for reserving
space.
Post by Mike Solomon
Post by k***@oco.net
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
Post by Mike Solomon
to solve. To me, the general problem seems to be "how can we replace
the
Post by Mike Solomon
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
k***@oco.net
2013-08-26 02:59:53 UTC
Permalink
The cost-benefit balance shows extra memory use, against potential
gain that we have trouble imagining, so no net gain.
Post by Mike Solomon
The necessity that comes to mind for the more general solution is the
work I did
Post by Mike Solomon
on line spanners, where we often want different skyline padding for
the word in
Post by Mike Solomon
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.
Post by Mike Solomon
That could be implemented by using the
skyline-stencil primitive as I've coded it in the most recent patch
set.

Demonstrate.

https://codereview.appspot.com/12957047/
d***@gnu.org
2013-08-26 04:36:11 UTC
Permalink
Post by Mike Solomon
Post by k***@oco.net
Post by Mike Solomon
Post by k***@oco.net
A second stencil is not a very good data structure for reserving
space.
Post by Mike Solomon
Post by k***@oco.net
Skylines are better at this, and we can \once\override 'padding
and
Post by Mike Solomon
Post by k***@oco.net
Post by Mike Solomon
Post by k***@oco.net
'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
Post by Mike Solomon
to solve. To me, the general problem seems to be "how can we
replace
Post by Mike Solomon
Post by k***@oco.net
the
Post by Mike Solomon
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
Post by Mike Solomon
Post by k***@oco.net
a different outline/skyline?"
It seems like what would make sense is for stencils to carry their own
skyline
Post by Mike Solomon
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.

https://codereview.appspot.com/12957
Mike Solomon
2013-08-26 04:51:47 UTC
Permalink
Post by p***@gmail.com
Post by Mike Solomon
Post by k***@oco.net
Post by Mike Solomon
Post by k***@oco.net
A second stencil is not a very good data structure for reserving
space.
Post by Mike Solomon
Post by k***@oco.net
Skylines are better at this, and we can \once\override 'padding
and
Post by Mike Solomon
Post by k***@oco.net
Post by Mike Solomon
Post by k***@oco.net
'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
Post by Mike Solomon
to solve. To me, the general problem seems to be "how can we
replace
Post by Mike Solomon
Post by k***@oco.net
the
Post by Mike Solomon
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
Post by Mike Solomon
Post by k***@oco.net
a different outline/skyline?"
It seems like what would make sense is for stencils to carry their own
skyline
Post by Mike Solomon
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
d***@gnu.org
2013-08-26 05:20:49 UTC
Permalink
Post by Mike Solomon
Post by p***@gmail.com
Post by Mike Solomon
Post by k***@oco.net
Post by Mike Solomon
Post by k***@oco.net
A second stencil is not a very good data structure for
reserving
Post by Mike Solomon
Post by p***@gmail.com
Post by Mike Solomon
Post by k***@oco.net
space.
Post by Mike Solomon
Post by k***@oco.net
Skylines are better at this, and we can \once\override
'padding
Post by Mike Solomon
Post by p***@gmail.com
and
Post by Mike Solomon
Post by k***@oco.net
Post by Mike Solomon
Post by k***@oco.net
'horizon-padding to get padding that follows the outlines of
the
Post by Mike Solomon
Post by p***@gmail.com
Post by Mike Solomon
Post by k***@oco.net
Post by Mike Solomon
Post by k***@oco.net
stencil.
[...]
If
we're going to do that, then we should think of the general
problem
Post by Mike Solomon
Post by p***@gmail.com
Post by Mike Solomon
Post by k***@oco.net
we're trying
Post by Mike Solomon
to solve. To me, the general problem seems to be "how can we
replace
Post by Mike Solomon
Post by k***@oco.net
the
Post by Mike Solomon
outline of one shape with that of another?"
No, that is _absolutely_ not the general problem here. The
general
Post by Mike Solomon
Post by p***@gmail.com
Post by Mike Solomon
Post by k***@oco.net
problem is: "how can we replace the outline/skylines of one
stencil
Post by Mike Solomon
Post by p***@gmail.com
with
Post by Mike Solomon
Post by k***@oco.net
a different outline/skyline?"
It seems like what would make sense is for stencils to carry their
own
Post by Mike Solomon
Post by p***@gmail.com
skyline
Post by Mike Solomon
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
Post by Mike Solomon
Post by p***@gmail.com
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.

https://codereview.appspot.com/129
Mike Solomon
2013-08-26 05:25:36 UTC
Permalink
Post by k***@oco.net
Post by Mike Solomon
Post by p***@gmail.com
Post by Mike Solomon
Post by k***@oco.net
Post by Mike Solomon
Post by k***@oco.net
A second stencil is not a very good data structure for
reserving
Post by Mike Solomon
Post by p***@gmail.com
Post by Mike Solomon
Post by k***@oco.net
space.
Post by Mike Solomon
Post by k***@oco.net
Skylines are better at this, and we can \once\override
'padding
Post by Mike Solomon
Post by p***@gmail.com
and
Post by Mike Solomon
Post by k***@oco.net
Post by Mike Solomon
Post by k***@oco.net
'horizon-padding to get padding that follows the outlines of
the
Post by Mike Solomon
Post by p***@gmail.com
Post by Mike Solomon
Post by k***@oco.net
Post by Mike Solomon
Post by k***@oco.net
stencil.
[...]
If
we're going to do that, then we should think of the general
problem
Post by Mike Solomon
Post by p***@gmail.com
Post by Mike Solomon
Post by k***@oco.net
we're trying
Post by Mike Solomon
to solve. To me, the general problem seems to be "how can we
replace
Post by Mike Solomon
Post by k***@oco.net
the
Post by Mike Solomon
outline of one shape with that of another?"
No, that is _absolutely_ not the general problem here. The
general
Post by Mike Solomon
Post by p***@gmail.com
Post by Mike Solomon
Post by k***@oco.net
problem is: "how can we replace the outline/skylines of one
stencil
Post by Mike Solomon
Post by p***@gmail.com
with
Post by Mike Solomon
Post by k***@oco.net
a different outline/skyline?"
It seems like what would make sense is for stencils to carry their
own
Post by Mike Solomon
Post by p***@gmail.com
skyline
Post by Mike Solomon
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
Post by Mike Solomon
Post by p***@gmail.com
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
d***@gnu.org
2013-08-26 05:35:40 UTC
Permalink
Post by Mike Solomon
Post by d***@gnu.org
Indeed. Instead of calling stencil-integrate on a "surrogate
stencil"
Post by Mike Solomon
Post by d***@gnu.org
or whatever for deriving a skyline, the respective stencil operation
in
Post by Mike Solomon
Post by d***@gnu.org
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?
Post by Mike Solomon
It is possible to create a stencil primitive that works something
`(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.
Post by Mike Solomon
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.


https://coderevi
Mike Solomon
2013-08-26 06:42:27 UTC
Permalink
Post by d***@gnu.org
Post by Mike Solomon
Post by d***@gnu.org
Indeed. Instead of calling stencil-integrate on a "surrogate
stencil"
Post by Mike Solomon
Post by d***@gnu.org
or whatever for deriving a skyline, the respective stencil operation
in
Post by Mike Solomon
Post by d***@gnu.org
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.
Post by d***@gnu.org
Post by Mike Solomon
It is possible to create a stencil primitive that works something
`(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.
Post by d***@gnu.org
Post by Mike Solomon
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
d***@gnu.org
2013-08-26 08:41:42 UTC
Permalink
Post by Mike Solomon
Post by d***@gnu.org
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
Post by Mike Solomon
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.
Post by Mike Solomon
What I'm proposing (making skylines a stencil data
member) is more work but seems less kludgy than creating a stencil
primitive
Post by Mike Solomon
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.
Post by Mike Solomon
Post by d***@gnu.org
Post by Mike Solomon
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
Post by Mike Solomon
Post by d***@gnu.org
Post by Mike Solomon
code around.
In the context of reviewing this patch, this is a straw man
argument.
Post by Mike Solomon
In the context of figuring out the best way to do things, this is
important. It
Post by Mike Solomon
would be better to decide what problem we're trying to solve and
implement it
Post by Mike Solomon
correctly now rather than creating a temporary solution that we do
away with
Post by Mike Solomon
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.
Post by Mike Solomon
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.
Post by Mike Solomon
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.
Post by Mike Solomon
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.


ht
Mike Solomon
2013-08-26 09:11:28 UTC
Permalink
Post by Mike Solomon
Post by Mike Solomon
Post by d***@gnu.org
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
Post by Mike Solomon
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.
Post by Mike Solomon
Post by Mike Solomon
What I'm proposing (making skylines a stencil data
member) is more work but seems less kludgy than creating a stencil
primitive
Post by Mike Solomon
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.
Post by Mike Solomon
Post by Mike Solomon
Post by d***@gnu.org
Post by Mike Solomon
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
Post by Mike Solomon
Post by d***@gnu.org
Post by Mike Solomon
code around.
In the context of reviewing this patch, this is a straw man
argument.
Post by Mike Solomon
In the context of figuring out the best way to do things, this is
important. It
Post by Mike Solomon
would be better to decide what problem we're trying to solve and
implement it
Post by Mike Solomon
correctly now rather than creating a temporary solution that we do
away with
Post by Mike Solomon
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.
Post by Mike Solomon
Post by Mike Solomon
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.
Post by Mike Solomon
Post by Mike Solomon
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.
Post by Mike Solomon
Post by Mike Solomon
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.
Post by Mike Solomon
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
Werner LEMBERG
2013-08-26 09:39:04 UTC
Permalink
Post by Mike Solomon
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
Mike Solomon
2013-08-26 09:42:38 UTC
Permalink
Post by Werner LEMBERG
Post by Mike Solomon
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
Werner LEMBERG
2013-08-26 09:48:55 UTC
Permalink
Post by Mike Solomon
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
Thomas Morley
2013-08-26 10:12:20 UTC
Permalink
Post by Werner LEMBERG
Post by Mike Solomon
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
Werner LEMBERG
2013-08-26 10:46:37 UTC
Permalink
Post by Thomas Morley
Post by Werner LEMBERG
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
Keith OHara
2013-08-27 04:18:18 UTC
Permalink
Post by Mike Solomon
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) ?
Mike Solomon
2013-08-27 05:37:17 UTC
Permalink
Post by Keith OHara
Post by Mike Solomon
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
Keith OHara
2013-08-27 06:01:46 UTC
Permalink
Post by Mike Solomon
Post by Keith OHara
(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 ?
Post by Mike Solomon
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.
Post by Mike Solomon
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}}

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"
Mike Solomon
2013-08-27 06:57:58 UTC
Permalink
Post by Keith OHara
Post by Mike Solomon
Post by Keith OHara
(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.
Post by Keith OHara
Post by Mike Solomon
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.
Post by Keith OHara
Post by Mike Solomon
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.
Post by Keith OHara
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
Keith OHara
2013-08-28 03:28:38 UTC
Permalink
Post by Mike Solomon
Post by Keith OHara
Post by Mike Solomon
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.
Post by Mike Solomon
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.
Mike Solomon
2013-08-28 06:30:32 UTC
Permalink
Post by Keith OHara
Post by Mike Solomon
Post by Keith OHara
Post by Mike Solomon
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.
Post by Mike Solomon
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
Keith OHara
2013-08-28 07:47:40 UTC
Permalink
Post by Mike Solomon
Post by Keith OHara
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.
Mike Solomon
2013-08-28 08:51:49 UTC
Permalink
Post by Keith OHara
Post by Mike Solomon
Post by Keith OHara
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

d***@gnu.org
2013-08-26 09:50:25 UTC
Permalink
Post by Mike Solomon
Post by Mike Solomon
Post by Mike Solomon
Post by d***@gnu.org
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
Post by Mike Solomon
implementing a kludge.
In the context of this issue, we are currently talking about not
doing
Post by Mike Solomon
Post by Mike Solomon
anything at all. It seems like the best course forward with regard
to
Post by Mike Solomon
Post by Mike Solomon
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
Post by Mike Solomon
primitive is unnecessary. If we agree that we should implement a
system where
Post by Mike Solomon
stencils are carriers of their own skylines, then we should fix this
bug using
Post by Mike Solomon
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.
Post by Mike Solomon
I agree. If stencils have both dimensions and skylines, we can use
merging
Post by Mike Solomon
based on dimensions as the default and skylines only when asked.
And that's exactly what we do now.
Post by Mike Solomon
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.
Post by Mike Solomon
Post by Mike Solomon
Post by Mike Solomon
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
Post by Mike Solomon
Post by Mike Solomon
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
Post by Mike Solomon
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.
Post by Mike Solomon
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.


https://co
d***@gnu.org
2013-08-26 10:00:30 UTC
Permalink
Post by Mike Solomon
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.
Post by Mike Solomon
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.


https://codereview.appspot.com/12957047/
Mike Solomon
2013-08-26 10:05:13 UTC
Permalink
Post by d***@gnu.org
That's bullshit since nobody assembles stencil expressions manually.
I assemble stencil expressions manually.
Post by d***@gnu.org
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
d***@gnu.org
2013-08-26 10:13:31 UTC
Permalink
Post by Mike Solomon
Post by d***@gnu.org
That's bullshit since nobody assembles stencil expressions manually.
I assemble stencil expressions manually.
If you meddle with internals, that's your own business.
Post by Mike Solomon
Post by d***@gnu.org
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
Post by Mike Solomon
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.

https://codereview.appspot.c
Mike Solomon
2013-08-26 10:17:53 UTC
Permalink
Post by d***@gnu.org
Post by Mike Solomon
Post by d***@gnu.org
That's bullshit since nobody assembles stencil expressions manually.
I assemble stencil expressions manually.
If you meddle with internals, that's your own business.
Post by Mike Solomon
Post by d***@gnu.org
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
Post by Mike Solomon
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
d***@gnu.org
2013-08-26 10:31:52 UTC
Permalink
Post by Mike Solomon
I think Harm is right that, irrespective of documentation, users use
what's
Post by Mike Solomon
available.
If people meddle with internals instead of using provided API functions,
they deserve whatever they get.
Post by Mike Solomon
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.

https://codereview.appspot.com/12957047/
Mike Solomon
2013-08-26 16:32:05 UTC
Permalink
Post by Mike Solomon
Post by Mike Solomon
I think Harm is right that, irrespective of documentation, users use
what's
Post by Mike Solomon
available.
If people meddle with internals instead of using provided API functions,
they deserve whatever they get.
Post by Mike Solomon
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
d***@gnu.org
2013-08-26 21:48:05 UTC
Permalink
Post by Mike Solomon
Post by d***@gnu.org
_Anything_ is "publicly useable". If that's supposed to be a
criterion,
Post by Mike Solomon
Post by d***@gnu.org
we may not change LilyPond ever again. We try to support
programming
Post by Mike Solomon
Post by d***@gnu.org
interfaces as long as it is feasible. But the way those are
internally
Post by Mike Solomon
Post by d***@gnu.org
implemented is _not_ _ever_ guaranteed to remain the same. If you
mess
Post by Mike Solomon
Post by d***@gnu.org
with internals, all bets are _off_.
For me, the criterion is the use of "define-public" versus "define".
If a
Post by Mike Solomon
function is "define-public" (like the ones in lily-library.scm), then
we are
Post by Mike Solomon
telling the user that it is useable. Otherwise, it should not be
public.
Post by Mike Solomon
The same should be true of Scheme functions written in C++ - there
should be a
Post by Mike Solomon
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
efficie
d***@gnu.org
2013-08-27 08:09:10 UTC
Permalink
Post by Keith OHara
Post by Keith OHara
How are skylines asked-for, and is it possible to know if they were
asked for
Post by Keith OHara
while interpreting the markup ?
Post by Keith OHara
{c4-\markup \whiteout \pad-to-box #(-0.5 . 9) #(-0.5 . 1.5) \concat
{"Clarinet
Post by Keith OHara
in B" \flat}}
class Stencil {
Scheme expr_;
Box dim_;
Skyline_pair *vskys_; // set to 0 in the constructor
Skyline_pair *hskys_; // set to 0 in the constructor
Skyline_pair* get_vskys () { if (!vskys_) create_vskys (); return
vskys_; }
Post by Keith OHara
}
So, the idea is that we only ever create the skylines if they are
asked for or
Post by Keith OHara
if we use something like pad-to-box. Then, if we are going to combine
two
Post by Keith OHara
stencils and neither of them have skylines set already, we do not
calculate
Post by Keith OHara
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).

https://codereview.appspot.com/12957047/
Mike Solomon
2013-08-27 10:27:15 UTC
Permalink
Post by Mike Solomon
Post by Keith OHara
Post by Keith OHara
How are skylines asked-for, and is it possible to know if they were
asked for
Post by Keith OHara
while interpreting the markup ?
Post by Keith OHara
{c4-\markup \whiteout \pad-to-box #(-0.5 . 9) #(-0.5 . 1.5) \concat
{"Clarinet
Post by Keith OHara
in B" \flat}}
class Stencil {
Scheme expr_;
Box dim_;
Skyline_pair *vskys_; // set to 0 in the constructor
Skyline_pair *hskys_; // set to 0 in the constructor
Skyline_pair* get_vskys () { if (!vskys_) create_vskys (); return
vskys_; }
Post by Keith OHara
}
So, the idea is that we only ever create the skylines if they are
asked for or
Post by Keith OHara
if we use something like pad-to-box. Then, if we are going to combine
two
Post by Keith OHara
stencils and neither of them have skylines set already, we do not
calculate
Post by Keith OHara
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
Loading...