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/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel