Re: Gets vertical skylines from grob stencils (issue 5626052)

[mtsolo]

http://codereview.appspot.com/5626052/diff/23001/lily/axis-group-interface.cc
File lily/axis-group-interface.cc (right):

http://codereview.appspot.com/5626052/diff/23001/lily/axis-group-interface.cc#newcode659
lily/axis-group-interface.cc:659: vector<Grob *> *riders)
On 2012/02/16 08:09:10, joeneeman wrote:
> I don't understand why riders are necessary. Is it because of this
cyclic
> dependence stuff?

Not cyclical, but imagine that a grob (say tuplet bracket, for example)
has its outside staff priority set whereas one of its Y_AXIS children
(say tuplet number, for example), doesn't.  If the tuplet number's
skyline is added to the skyline_pair using add boxes before its parent
is shifted, it will be placed too low in the skyline.  Thus, it must be
added to the skyline only after its parent's outside-staff-priority has
been set to SCM_BOOL_F.  This is why riders are used.

http://codereview.appspot.com/5626052/diff/23001/lily/skyline.cc
File lily/skyline.cc (right):

http://codereview.appspot.com/5626052/diff/23001/lily/skyline.cc#newcode467
lily/skyline.cc:467: Strips all old sloped material, adds new.
On 2012/02/16 08:09:10, joeneeman wrote:
> You're assuming that all sloped material came from skyline padding.
That's true
> right now, but there's no reason that it will continue to be true.
It's probably
> better to avoid adding padding at creation time altogether, and
instead to add
> it when calling Skyline::distance.

Right you are...I agree that this is entirely dependent on the
implementation of skylines at the moment.  I know that I always respond
to you that it's something that can be done in a future patch, but I'd
put this in that category as well, as it seems like a separate problem
that doesn't need to be tacked on to this patch to be solved.  That
said, if I forget, nag me!

http://codereview.appspot.com/5626052/diff/23001/lily/skyline.cc#newcode668
lily/skyline.cc:668: Skyline::left () const
On 2012/02/16 08:09:10, joeneeman wrote:
> This is linear in the number of buildings, but it should be constant.

How would one do this?

http://codereview.appspot.com/5626052/diff/23001/lily/skyline.cc#newcode680
lily/skyline.cc:680: Skyline::right () const
On 2012/02/16 08:09:10, joeneeman wrote:
> Ditto

Ditto

http://codereview.appspot.com/5626052/diff/23001/lily/stencil-integral.cc
File lily/stencil-integral.cc (right):

http://codereview.appspot.com/5626052/diff/23001/lily/stencil-integral.cc#newcode543
lily/stencil-integral.cc:543: SCM glyph_info = scm_hashq_ref (font_list,
scm_string_to_symbol (glyph), SCM_EOL);
On 2012/02/16 08:09:10, joeneeman wrote:
> There isn't much error-checking here. What if the user substitutes an
unofficial
> font which isn't in the list?

> By the way, lilypond's fonts embed extra data in font tables (look for
LILC and
> LILY in open-type-font.cc). That may be a better way to embed this
information
> than by putting it in a scm file. For example, it would allow
unofficial fonts
> to support integrals by embedding an extra table.

I think this is a good idea...I'll have a look at it.  Question, though
- aren't font tables done as alists?  I think it's important to use a
hash table here, as there is a lot of data getting thrown around and
hash table look ups are in constant time.  I'll investigate.

http://codereview.appspot.com/5626052/diff/23001/mf/GNUmakefile
File mf/GNUmakefile (right):

http://codereview.appspot.com/5626052/diff/23001/mf/GNUmakefile#newcode231
mf/GNUmakefile:231: $(LILYPOND_BINARY) --verbose
$(top-src-dir)/ly/generate-font-integrals > $@
On 2012/02/15 10:26:17, Graham Percival wrote:
> This looks good, but not immediately relevant to the rest of the
vertical
> skylines stuff.  Could you make this a separate patch so that it can
be pushed
> sooner?

Sorry for not responding to this comment inlined...the response is that
it'd be hard as it requires new files and/or changes to old ones to go
along w/ it.

http://codereview.appspot.com/5626052/