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

[joeneeman]

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)
I don't understand why riders are necessary. Is it because of this
cyclic dependence stuff?

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.
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.

http://codereview.appspot.com/5626052/diff/23001/lily/skyline.cc#newcode668
lily/skyline.cc:668: Skyline::left () const
This is linear in the number of buildings, but it should be constant.

http://codereview.appspot.com/5626052/diff/23001/lily/skyline.cc#newcode680
lily/skyline.cc:680: Skyline::right () const
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);
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.

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