Thanks for the review!
http://codereview.appspot.com/6498077/diff/21/lily/axis-group-interface.cc File lily/axis-group-interface.cc (right): http://codereview.appspot.com/6498077/diff/21/lily/axis-group-interface.cc#newcode419 lily/axis-group-interface.cc:419: */ On 2012/09/02 15:59:00, dak wrote:
See above.
Removed - was old code I needed to read to make sure I waas getting stuff right. Forgot it waas there. http://codereview.appspot.com/6498077/diff/21/lily/page-layout-problem.cc File lily/page-layout-problem.cc (right): http://codereview.appspot.com/6498077/diff/21/lily/page-layout-problem.cc#newcode1004 lily/page-layout-problem.cc:1004: if (is_spaceable (staves[i])) On 2012/09/02 15:59:00, dak wrote:
This indentation is simply wrong and does not match the program
structure. Cruft. Fixed. http://codereview.appspot.com/6498077/diff/21/lily/phrasing-slur-engraver.cc File lily/phrasing-slur-engraver.cc (right): http://codereview.appspot.com/6498077/diff/21/lily/phrasing-slur-engraver.cc#newcode119 lily/phrasing-slur-engraver.cc:119: if (slur_stubs_.find (slurs_[j]) == slur_stubs_.end ()) On 2012/09/02 15:59:00, dak wrote:
It is quite nonsensical to have slur_stubs be a map indexed via
slurs_[j] rather
than just an array indexed through j.
That is inefficient use of time, memory, and complexity.
It is sensical because: --) It avoids having two vectors (slur_stubs_ and end_slur_stubs_ would be needed), thus making the code easier to read and maintain. --) It has a "find" method built into it. Unless people are crunching scores with thousands of slurs on Strong Bad's original Tandy 400, I'm not too worried about the tradeoff. http://codereview.appspot.com/6498077/diff/21/lily/phrasing-slur-engraver.cc#newcode332 lily/phrasing-slur-engraver.cc:332: map<Grob *, Grob *> vag_to_slur; On 2012/09/02 15:59:00, dak wrote:
Again: why a map here?
find method, see above. I know that find exists for vectors in algorithm, but I think this is easier to read and more consistent. http://codereview.appspot.com/6498077/diff/21/lily/script-column.cc File lily/script-column.cc (right): http://codereview.appspot.com/6498077/diff/21/lily/script-column.cc#newcode60 lily/script-column.cc:60: if (unsmob_grob (i1->get_object ("slur")) && unsmob_grob (i2->get_object ("slur"))) On 2012/09/02 15:59:00, dak wrote:
If both scripts have a field "slur" that is a grob, return #t is the
avoid-slur
property of the second script is "outside" or "around" while that of
the first
is neither.
Why don't you write this in a comment? Why this complex code? And
what is the
ultimate purpose?
I didn't put it in a comment because it can be understood by reading it, as you explain with your eloquent prose :-) This exists to avoid the situation where a grob has a lower slur priority than another but is typeset outside a slur whereas the other is put inside, leading to a contradiction. Priority is given to the slur directive. http://codereview.appspot.com/6498077/diff/21/lily/slur.cc File lily/slur.cc (right): http://codereview.appspot.com/6498077/diff/21/lily/slur.cc#newcode623 lily/slur.cc:623: "thickness " On 2012/09/02 15:59:00, dak wrote:
No entry for "surrogate"?
scm/define-grob-interfaces.scm http://codereview.appspot.com/6498077/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
