Allows users to request that beam collision occur on the Voice level.

Allows users to request that beam collision occur on the Voice level.

Normally, automatic collision resolution applies at the Staff level.
This allows for users to override this behavoir to only apply to Voice
contexts.

[MikeSol, 2011-04-07 01:32:25]

Before pushing the stem patch (thanks to Trevor and Han Wen for the LGTMs), I'd
like to push this one.  It allows for beam collision to happen momentarily on
the Voice level via an override.  I will subsequently integrate this into the
quote.ly and quote-during.ly regtests.

Please reply with any and all comments!

Cheers,
Mike

[c_sorensen, 2011-04-07 02:30:59]

On 4/6/11 7:32 PM, "mtsolo@gmail.com" <mtsolo@gmail.com> wrote:

> Reviewers: ,
> 
> Message:
> Before pushing the stem patch (thanks to Trevor and Han Wen for the
> LGTMs), I'd like to push this one.  It allows for beam collision to
> happen momentarily on the Voice level via an override.  I will
> subsequently integrate this into the quote.ly and quote-during.ly
> regtests.
> 
> Please reply with any and all comments!

I haven't followed this carefully, so I hesitate to comment, but this
doesn't seem like the semantically right way to handle the desired behavior.

Instead of making a Staff level property, it seems that it ought to be a
Voice level property.  That is, in a Voice I ought to turn off the beam
collision engraver for objects from other Voices.  But I don't know if such
an approach is feasible.

HTH,

Carl

[mike_apollinemike.com, 2011-04-07 11:46:54]

On Apr 6, 2011, at 10:30 PM, Carl Sorensen wrote:

> On 4/6/11 7:32 PM, "mtsolo@gmail.com" <mtsolo@gmail.com> wrote:
> 
>> Reviewers: ,
>> 
>> Message:
>> Before pushing the stem patch (thanks to Trevor and Han Wen for the
>> LGTMs), I'd like to push this one.  It allows for beam collision to
>> happen momentarily on the Voice level via an override.  I will
>> subsequently integrate this into the quote.ly and quote-during.ly
>> regtests.
>> 
>> Please reply with any and all comments!
> 
> I haven't followed this carefully, so I hesitate to comment, but this
> doesn't seem like the semantically right way to handle the desired behavior.
> 
> Instead of making a Staff level property, it seems that it ought to be a
> Voice level property.  That is, in a Voice I ought to turn off the beam
> collision engraver for objects from other Voices.  But I don't know if such
> an approach is feasible.

Hey Carl,

I believe this is just an issue with the regtest.

Check out this bit of recycled music from beam-collision-opposite-stem.ly :

vO = { \override Voice . Beam #'collision-voice-only = ##t }

{
  \relative c'' { << { \vO s16 e16 [ s cis ] } \\ { b'16 [ s b ] } >> }
  \relative c'' { << { \vO s16 e16 [ s cis, ] } \\ { b''16 [ s b ] } >> }
  \relative c'' { << { \vO s16 d16 [ s cis ] } \\ { b'16 [ s b ] } >> }
  \relative c'' { << { \vO s16 c16 [ s cis ] } \\ { b'16 [ s b ] } >> }
  \relative c'' { << { \vO s16 b16 [ s cis ] } \\ { b'16 [ s b ] } >> }
  \relative c'' { << { \vO s16 a16 [ s cis ] } \\ { b'16 [ s b ] } >> }
  \relative c'' { << { \vO s16 g16 [ s cis ] } \\ { b'16 [ s b ] } >> }
  \relative c'' { << { \vO s16 c,16 [ s cis' ] } \\ { b'16 [ s b ] } >> }
  \relative c'' { << { \vO s16 c,16 [ s cis'' ] } \\ { b16 [ s b ] } >> }
  \relative c'' { << { \vO s16 f,16 [ s cis ] } \\ { b'16 [ s b ] } >> }
  \relative c'' { << { \vO s16 e,16 [ s cis ] } \\ { b'16 [ s b ] } >> }
  \relative c'' { << { \vO s16 d,16 [ s cis ] } \\ { b'16 [ s b ] } >> }
  \relative c'' { << { \vO s16 e16 [ s cis ] } \\ { b'16 [ s d ] } >> }
  \relative c'' { << { \vO s16 e16 [ s cis ] } \\ { b'16 [ s f' ] } >> }
  \relative c'' { << { \vO s16 e16 [ s cis ] } \\ { b'16 [ s a ] } >> }
  \relative c'' { << { \vO s16 e16 [ s cis ] } \\ { b'16 [ s gis ] } >> }
}

I use the override on the Voice level and the expected output results (all
collisions in the upper voice reappear, whereas the lower ones are avoided).

Is what you're getting at more an issue w/ the regtest, or is it a problem w/
the C++ code?

Cheers,
Mike

[Graham Percival (old account), 2011-04-08 03:34:22]

thanks, added here:
http://code.google.com/p/lilypond/issues/detail?id=1600

[mike_apollinemike.com, 2011-04-08 14:57:40]

On Apr 7, 2011, at 11:34 PM, percival.music.ca@gmail.com wrote:

> thanks, added here:
> http://code.google.com/p/lilypond/issues/detail?id=1600
> 
> http://codereview.appspot.com/4367042/
> 
> _______________________________________________
> lilypond-devel mailing list
> lilypond-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/lilypond-devel

I got a clean compile and regtest too.
Carl - does your suggestion still stand?

I'd like to push this and the stem thing back to back tonight if possible.

Cheers,
Mike

[Neil Puttock, 2011-04-08 19:19:17]

http://codereview.appspot.com/4367042/diff/1/input/regression/beam-collision-...
File input/regression/beam-collision-voice-only.ly (right):

http://codereview.appspot.com/4367042/diff/1/input/regression/beam-collision-...
input/regression/beam-collision-voice-only.ly:1: \version "2.13.58"
2.13.59

http://codereview.appspot.com/4367042/diff/1/input/regression/beam-collision-...
input/regression/beam-collision-voice-only.ly:9: << { c8 [s c ] } \\ { s8 c' s8
} >>
c8[ s c]

http://codereview.appspot.com/4367042/diff/1/input/regression/beam-collision-...
input/regression/beam-collision-voice-only.ly:10: c, [des' ]
c,[ des']

http://codereview.appspot.com/4367042/diff/1/input/regression/beam-collision-...
input/regression/beam-collision-voice-only.ly:12: << { c,8 [s c ] } \\ { s8 c'
s8 } >>
c,8[ s c]

http://codereview.appspot.com/4367042/diff/1/input/regression/beam-collision-...
input/regression/beam-collision-voice-only.ly:13: c, [des'! ]
c,[ des'!]

http://codereview.appspot.com/4367042/diff/1/lily/beam-collision-engraver.cc
File lily/beam-collision-engraver.cc (right):

http://codereview.appspot.com/4367042/diff/1/lily/beam-collision-engraver.cc#...
lily/beam-collision-engraver.cc:52: Beam_collision_engraver::less (Grob_info i,
Grob_info j)
This name implies a comparison between engravers.  It would make more sense
defining Grob_info::less ().

http://codereview.appspot.com/4367042/diff/1/scm/define-grobs.scm
File scm/define-grobs.scm (right):

http://codereview.appspot.com/4367042/diff/1/scm/define-grobs.scm#newcode358
scm/define-grobs.scm:358: (collision-voice-only . #f)
remove

[MikeSol, 2011-04-08 19:48:27]

New patch set uploaded.

http://codereview.appspot.com/4367042/diff/1/input/regression/beam-collision-...
File input/regression/beam-collision-voice-only.ly (right):

http://codereview.appspot.com/4367042/diff/1/input/regression/beam-collision-...
input/regression/beam-collision-voice-only.ly:1: \version "2.13.58"
On 2011/04/08 19:19:18, Neil Puttock wrote:
> 2.13.59

Done.

http://codereview.appspot.com/4367042/diff/1/input/regression/beam-collision-...
input/regression/beam-collision-voice-only.ly:9: << { c8 [s c ] } \\ { s8 c' s8
} >>
On 2011/04/08 19:19:18, Neil Puttock wrote:
> c8[ s c]

Done.

http://codereview.appspot.com/4367042/diff/1/input/regression/beam-collision-...
input/regression/beam-collision-voice-only.ly:10: c, [des' ]
On 2011/04/08 19:19:18, Neil Puttock wrote:
> c,[ des']

Done.

http://codereview.appspot.com/4367042/diff/1/input/regression/beam-collision-...
input/regression/beam-collision-voice-only.ly:12: << { c,8 [s c ] } \\ { s8 c'
s8 } >>
On 2011/04/08 19:19:18, Neil Puttock wrote:
> c,8[ s c]

Done.

http://codereview.appspot.com/4367042/diff/1/input/regression/beam-collision-...
input/regression/beam-collision-voice-only.ly:13: c, [des'! ]
On 2011/04/08 19:19:18, Neil Puttock wrote:
> c,[ des'!]

Done.

http://codereview.appspot.com/4367042/diff/1/lily/beam-collision-engraver.cc
File lily/beam-collision-engraver.cc (right):

http://codereview.appspot.com/4367042/diff/1/lily/beam-collision-engraver.cc#...
lily/beam-collision-engraver.cc:52: Beam_collision_engraver::less (Grob_info i,
Grob_info j)
On 2011/04/08 19:19:18, Neil Puttock wrote:
> This name implies a comparison between engravers.  It would make more sense
> defining Grob_info::less ().
> 

Done.

http://codereview.appspot.com/4367042/diff/1/scm/define-grobs.scm
File scm/define-grobs.scm (right):

http://codereview.appspot.com/4367042/diff/1/scm/define-grobs.scm#newcode358
scm/define-grobs.scm:358: (collision-voice-only . #f)
On 2011/04/08 19:19:18, Neil Puttock wrote:
> remove

Done.

[Neil Puttock, 2011-04-08 19:50:00]

LGTM.