Re: Approximates cross-staff slurs in VerticalAxisGroup vertical-skylines. (issue 6498077)

[mtsolo]

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/