Hi Carl, thanks for diving into this!
I love the idea of handling beam collisions, but I think the design of the backend code is flawed. See my comments. http://codereview.appspot.com/3928041/diff/30001/lily/beam.cc File lily/beam.cc (right): http://codereview.appspot.com/3928041/diff/30001/lily/beam.cc#newcode972 lily/beam.cc:972: sw[LEFT] = covered_grob->relative_coordinate (commonx, X_AXIS) + stencil->extent(X_AXIS)[LEFT] - x0; why not use covered_grob->extent(common_x, X)[LEFT] instead? the same for all other stencil->extent manipulations. http://codereview.appspot.com/3928041/diff/30001/lily/beam.cc#newcode973 lily/beam.cc:973: sw[RIGHT] = covered_grob->relative_coordinate (commony, Y_AXIS) + stencil->extent(Y_AXIS)[DOWN] - pos[LEFT]; * what is sw ? can you rename to something more explanatory? * you are mixing X and Y in an interval. Shouldnt sw be an Offset ? * Why is this code not symmetric in UP and DOWN, using flip() ? This looks error prone wrt collisions that work for upstems, but not for down stems and vice-versa http://codereview.appspot.com/3928041/diff/30001/lily/beam.cc#newcode1015 lily/beam.cc:1015: return ly_interval2scm (pos); it looks like this only handles the 1st collision found, and exits after circumventing the first one. The risks are * in case of multiple grobs, the resolution is dependent on the order of constructing the covered grobs list. Unpredictable and arbitrary. * that you will park the beam on top of something else, like the next grob you are skipping with this return. * in the last case, you may even create a beam with enormous stems (ugly) that still has a collision. Why not calculate all the locations of all covered grobs, and work collisions into the scores for beam positions? See beam-quanting.cc; you'd have to add another scoring pass to Beam::quanting(). http://codereview.appspot.com/3928041/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
