Thanks for the comments! New patch uploaded.
http://codereview.appspot.com/4237057/diff/1/lily/beam.cc File lily/beam.cc (right): http://codereview.appspot.com/4237057/diff/1/lily/beam.cc#newcode173 lily/beam.cc:173: span_data.push_back (get_beam_span (orig->broken_intos_[i], commonx)); On 2011/03/09 23:03:44, hanwenn wrote:
this looks wrong - different broken pieces cannot ever share a
commonx. Fixed. http://codereview.appspot.com/4237057/diff/1/lily/beam.cc#newcode620 lily/beam.cc:620: { On 2011/03/11 14:01:29, hanwenn wrote:
it would be nice if you could collapse all of the if (feather) code.
You're
duplicating an awful lot of logic.
I'm not exactly sure what you mean here by "collapse." I folded the code that you mention below, but I'm not sure where it'd be possible to reduce this code. http://codereview.appspot.com/4237057/diff/1/lily/beam.cc#newcode647 lily/beam.cc:647: } On 2011/03/11 14:01:29, hanwenn wrote:
I see dup code. Can you fold?
Done. http://codereview.appspot.com/4237057/diff/1/lily/beam.cc#newcode653 lily/beam.cc:653: : segments[segments.size () - 1].vertical_count_); On 2011/03/11 14:01:29, hanwenn wrote:
use segments.back()
Done. http://codereview.appspot.com/4237057/diff/1/lily/beam.cc#newcode665 lily/beam.cc:665: * (placements[RIGHT] - placements[LEFT]) On 2011/03/11 14:01:29, hanwenn wrote:
placements.delta (), placements.length() ?
Done. http://codereview.appspot.com/4237057/diff/1/lily/beam.cc#newcode682 lily/beam.cc:682: weights[LEFT] = 1; On 2011/03/11 14:01:29, hanwenn wrote:
can you set multiplier to 1 in this case?
Done. http://codereview.appspot.com/4237057/diff/1/lily/spanner.cc File lily/spanner.cc (right): http://codereview.appspot.com/4237057/diff/1/lily/spanner.cc#newcode405 lily/spanner.cc:405: Spanner::broken_spanner_index (Spanner const *sp) On 2011/03/09 23:03:44, hanwenn wrote:
why not make it a real member funtcion?
Actually - sorry, I spoke too soon. I see that there's a function get_break_index. Could these two functions be merged? Is there a reason that the two functions exist separately? If not, I'll merge them together. http://codereview.appspot.com/4237057/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel