Extend Beam::shift_region_to_valid() to also take into account collisions.

Extend Beam::shift_region_to_valid() to also take into account collisions.

[hanwenn, 2011-02-28 04:06:03]

this fixes the most egregious errors of the brute force quanting.  

There are two issues in the regtest: it gets confused by x-staff beams, and it
tries to avoid the start of staff clef when the beam crosses a line breaks.

I'll fix those problems, but don't expect large changes.

Please review.

[hanwenn, 2011-02-28 04:09:12]

On Mon, Feb 28, 2011 at 1:06 AM,  <hanwenn@gmail.com> wrote:
> this fixes the most egregious errors of the brute force quanting.
>
> There are two issues in the regtest: it gets confused by x-staff beams,
> and it tries to avoid the start of staff clef when the beam crosses a
> line breaks.
>
> I'll fix those problems, but don't expect large changes.
>
> Please review.

Here are some samples.

This does not handle squeezing beams that are between two other
voices; in a way, such squeezing is wrong, as we have a minimum length
for stems, and the squeezing typically violates that length
constraint.  (I suspect that if you tune the minimum lengths down, it
already works.)

-- 
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen

[hanwenn, 2011-02-28 04:27:17]

On 2011/02/28 04:06:03, hanwenn wrote:
> There are two issues in the regtest: it gets confused by x-staff beams, and it
> tries to avoid the start of staff clef when the beam crosses a line breaks.

fixed.

[mike_apollinemike.com, 2011-02-28 11:21:20]

On Feb 27, 2011, at 11:27 PM, hanwenn@gmail.com wrote:

> On 2011/02/28 04:06:03, hanwenn wrote:
>> There are two issues in the regtest: it gets confused by x-staff
> beams, and it
>> tries to avoid the start of staff clef when the beam crosses a line
> breaks.
> 
> fixed.
> 
> http://codereview.appspot.com/4239047/

Several of the tests that I've been running work better with this new version -
large collisions are now handled no problem.

The only things that don't work properly are the organ example as well as the
two attached examples.  Is there any way to prevent the beam collision from
pulling these up?

If you suspect that the collisions below have to do with the minimum stem
length, could you provide a regtest that shows this property being overrided &
the correct result being achieved?

I see a code dup for the covered grobs code in beam.cc and beam-quanting.cc.  I
believe that this sort of thing can be brought into beam.cc without ever being
part of the quanting.  Ideally, the covered grobs should extracted only in
beam.cc, and the quanting should be passed numerical information about the
results of these calculations rather than having to redo them.  This will allow
you as well to have collision-related properties for the beams that, when
tweaked, will likely handle many of the results below.

Cheers,
MS

[hanwenn, 2011-02-28 13:08:13]

On Mon, Feb 28, 2011 at 8:21 AM, mike@apollinemike.com
<mike@apollinemike.com> wrote:
>> fixed.
>>
>> http://codereview.appspot.com/4239047/
>
> Several of the tests that I've been running work better with this new version
- large collisions are now handled no problem.
>
> The only things that don't work properly are the organ example as well as the
two attached examples.  Is there any way to prevent the beam collision from
pulling these up?

See attached.  This was done with

      \override Stem #'details #'beamed-extreme-minimum-free-lengths
      = #'(0.3 0.2)

> I see a code dup for the covered grobs code in beam.cc and beam-quanting.cc.
 I believe that this sort of thing can be brought into beam.cc without ever
being part of the quanting.  Ideally, the covered grobs should extracted only in
beam.cc, and the quanting should be passed numerical information about the
results of these calculations rather than having to redo them.  This will allow
you as well to have collision-related properties for the beams that, when
tweaked, will likely handle many of the results below.

This would save a tiny amount of computation, with lots of extra
complexity.  Remember that all property computations are cached
anyway, so there are few calculations really being redone.

There could be an argument to collapse common code into a shared
function, but not sure if that will be practical.  Maybe you could try
a patch to see if it makes sense.

--
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen

[MikeSol, 2011-02-28 14:48:24]

Great work!  Two comments below concerning beam properties.

http://codereview.appspot.com/4239047/diff/3002/lily/beam.cc
File lily/beam.cc (right):

http://codereview.appspot.com/4239047/diff/3002/lily/beam.cc#newcode1156
lily/beam.cc:1156: Real min_y_size = 2.0;
here, we should have something like
if (to_boolean (me->get_property ("avoid-collisions"))
so that users can opt out of the avoidance if they so choose.
then, we would have an `else' that set the collision penalty in the quanting to
0 so that no collision avoidance took place.

I think that in a lot of real-music scenarios such as organ scores, people's may
in fact want beams to collide, and thus, they should be able to opt-out of this
avoidance.

http://codereview.appspot.com/4239047/diff/3002/lily/beam.cc#newcode1256
lily/beam.cc:1256: beam_left_y = point_in_interval (best, 2.0);
Here, there should be a padding property that allows users to control breathing
room for collisions.  Otherwise, you could wind up getting a beam that is pushed
just below a notehead in the quanting (see example in next email).

[mike_apollinemike.com, 2011-02-28 14:49:49]

Attached is the type of note-on-beam thing I was talking about in my previous
comments, which can be fixed w/ a padding property.

Cheers,
MS

[hanwenn, 2011-02-28 16:49:02]

On Mon, Feb 28, 2011 at 11:48 AM,  <mtsolo@gmail.com> wrote:
> Great work!  Two comments below concerning beam properties.

http://codereview.appspot.com/4239047/diff/3002/lily/beam.cc#newcode1156
> lily/beam.cc:1156: Real min_y_size = 2.0;
> here, we should have something like
> if (to_boolean (me->get_property ("avoid-collisions"))
> so that users can opt out of the avoidance if they so choose.
> then, we would have an `else' that set the collision penalty in the
> quanting to 0 so that no collision avoidance took place.

The idea is that this is only necessary for large objects
>
> I think that in a lot of real-music scenarios such as organ scores,
> people's may in fact want beams to collide, and thus, they should be
> able to opt-out of this avoidance.
>
> http://codereview.appspot.com/4239047/diff/3002/lily/beam.cc#newcode1256
> lily/beam.cc:1256: beam_left_y = point_in_interval (best, 2.0);
> Here, there should be a padding property that allows users to control
> breathing room for collisions.  Otherwise, you could wind up getting a
> beam that is pushed just below a notehead in the quanting (see example
> in next email).
>
> http://codereview.appspot.com/4239047/
>



-- 
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen

[hanwenn, 2011-02-28 16:54:58]

On Mon, Feb 28, 2011 at 11:48 AM,  <mtsolo@gmail.com> wrote:
> Great work!  Two comments below concerning beam properties.
>
>
> http://codereview.appspot.com/4239047/diff/3002/lily/beam.cc
> File lily/beam.cc (right):
>
> http://codereview.appspot.com/4239047/diff/3002/lily/beam.cc#newcode1156
> lily/beam.cc:1156: Real min_y_size = 2.0;
> here, we should have something like
> if (to_boolean (me->get_property ("avoid-collisions"))
> so that users can opt out of the avoidance if they so choose.
> then, we would have an `else' that set the collision penalty in the
> quanting to 0 so that no collision avoidance took place.

?

Why not modify the beam-collision engraver to not add the objects as
collisions in the first place?

> I think that in a lot of real-music scenarios such as organ scores,
> people's may in fact want beams to collide, and thus, they should be
> able to opt-out of this avoidance.

Let's wait for a bit for these situations to surface.  As a first line
of defense, people can simply remove the collision engraver .

> http://codereview.appspot.com/4239047/diff/3002/lily/beam.cc#newcode1256
> lily/beam.cc:1256: beam_left_y = point_in_interval (best, 2.0);
> Here, there should be a padding property that allows users to control
> breathing room for collisions.  Otherwise, you could wind up getting a
> beam that is pushed just below a notehead in the quanting (see example
> in next email).

No, the padding is part of the scoring.  This is code is just to
provide a credible starting point for the quanting code to look.  As
noted in the comments, it assumes single beams as an approximation, so
if you start using this with 128th beams, it may fail.  Are you really
using 128ths in this way? We could add a crude offset to account for
beam multiplicity.

-- 
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen