Tweaks regtest and adds avoid-collisions property

Tweaks regtest and adds avoid-collisions property


Merge branch 'master' of git://git.sv.gnu.org/lilypond into 37final


Triggers second quant pass for collisions

Bad

Revert "Bad"

This reverts commit fac518ab6e4ea9fd591c849afb56e247d53c9bf3.

Implements a more robust solution to issue 37

Intermediary

Intermediary

Intermediary

Intermediary

Intermediary

Finally

Whitespace fixes

[Graham Percival (old account), 2011-01-29 14:00:09]

LGTM.

http://codereview.appspot.com/4022045/diff/12001/input/regression/beam-collis...
File input/regression/beam-collision.ly (right):

http://codereview.appspot.com/4022045/diff/12001/input/regression/beam-collis...
input/regression/beam-collision.ly:15: \clef bass g,,,8[ \clef treble d''8]
Could the notes go on their own line?

If there's no other problems with this draft, then don't bother -- there's no
point being really fussy with .ly syntax in the regtests -- but I expect there's
still a few more drafts to come, and it would be easier to read if notes were a
little bit more separate from clefs, overrides, and the like.

http://codereview.appspot.com/4022045/diff/12001/input/regression/beam-collis...
input/regression/beam-collision.ly:26: \once \override Voice . Beam
#'avoid-collisions = ##f c8[ c c c] }
Could the notes go on the next line, leaving the override by itself on the line?

Also, could I get a
^"turn off collision avoidance"
so that it's obvious in the image that these collisions are ok?  :)

http://codereview.appspot.com/4022045/diff/12001/input/regression/beam-collis...
input/regression/beam-collision.ly:54: s8 f s g s a s b s c s d s e  }
I see a stem collision for the d and e.  I'm not complaining about this, but
consider adding a
  ^"no solution possible; stem collides with notehead"
if this is intentional/acceptable.

Basically, imagine that you're a poor bug squad member doing our infrequent full
regtest check.  You know nothing about programming, and you have 900+ regtests
to review, so you're trying to go through each regtest in 30 seconds or so. 
Anything you can do to make the .png more clear is a good thing.  :)

[MikeSol, 2011-01-29 14:41:25]

All fixed.  New patchset uploaded.

http://codereview.appspot.com/4022045/diff/12001/input/regression/beam-collis...
File input/regression/beam-collision.ly (right):

http://codereview.appspot.com/4022045/diff/12001/input/regression/beam-collis...
input/regression/beam-collision.ly:15: \clef bass g,,,8[ \clef treble d''8]
On 2011/01/29 14:00:09, Graham Percival wrote:
> Could the notes go on their own line?
> 
> If there's no other problems with this draft, then don't bother -- there's no
> point being really fussy with .ly syntax in the regtests -- but I expect
there's
> still a few more drafts to come, and it would be easier to read if notes were
a
> little bit more separate from clefs, overrides, and the like.

Done.

http://codereview.appspot.com/4022045/diff/12001/input/regression/beam-collis...
input/regression/beam-collision.ly:26: \once \override Voice . Beam
#'avoid-collisions = ##f c8[ c c c] }
On 2011/01/29 14:00:09, Graham Percival wrote:
> Could the notes go on the next line, leaving the override by itself on the
line?
> 
> Also, could I get a
> ^"turn off collision avoidance"
> so that it's obvious in the image that these collisions are ok?  :)

Done.

http://codereview.appspot.com/4022045/diff/12001/input/regression/beam-collis...
input/regression/beam-collision.ly:54: s8 f s g s a s b s c s d s e  }
On 2011/01/29 14:00:09, Graham Percival wrote:
> I see a stem collision for the d and e.  I'm not complaining about this, but
> consider adding a
>   ^"no solution possible; stem collides with notehead"
> if this is intentional/acceptable.
> 
> Basically, imagine that you're a poor bug squad member doing our infrequent
full
> regtest check.  You know nothing about programming, and you have 900+ regtests
> to review, so you're trying to go through each regtest in 30 seconds or so. 
> Anything you can do to make the .png more clear is a good thing.  :)

Done.

[Graham Percival (old account), 2011-01-29 14:54:40]

http://codereview.appspot.com/4022045/diff/14001/input/regression/beam-collis...
File input/regression/beam-collision.ly (right):

http://codereview.appspot.com/4022045/diff/14001/input/regression/beam-collis...
input/regression/beam-collision.ly:17: g,,,8[ \clef treble d''8]
newline before \clef, newline before d''

I'm not saying to do this right now -- wait until somebody points out some code
problems/improvements, then do this at the same time.  :)

http://codereview.appspot.com/4022045/diff/14001/input/regression/beam-collis...
input/regression/beam-collision.ly:22: \key bes \major ees ] |
newline before ees   and could we get a 8 as well?  I've lost track of the
durations by this point.

http://codereview.appspot.com/4022045/diff/14001/input/regression/beam-collis...
input/regression/beam-collision.ly:24: \break
indent two spaces.

http://codereview.appspot.com/4022045/diff/14001/input/regression/beam-collis...
input/regression/beam-collision.ly:40: \stemUp e'8[ e e e]
newline before e'8

http://codereview.appspot.com/4022045/diff/14001/input/regression/beam-collis...
input/regression/beam-collision.ly:41: \set fontSize = #-4 c8[ c c c]
newline before c8[

[Graham Percival (old account), 2011-02-03 17:55:58]

I can confirm that the regtests are fine.

The patch applies cleanly to master, but doesn't apply completely clean if you
apply Han-Wen's beaming priority queue.  That's not a complaint about this
patch; I'm just noting it in case anybody wants to test the two together, or in
case Han-Wen pushes the beaming priority queue soon.

[hanwenn, 2011-02-04 00:33:00]

Hey Mike,

could you make a separate patch for the engraver, fixing the issue below; I
think the engraver can go in without further discussion.

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

http://codereview.appspot.com/4022045/diff/12001/lily/beam-collision-engraver...
lily/beam-collision-engraver.cc:107: }
I think you are adding the note heads of the beam itself here as well. Can you
make sure you only do that for noteheads from other voices?

[mike_apollinemike.com, 2011-02-04 01:24:44]

On Feb 3, 2011, at 7:33 PM, hanwenn@gmail.com wrote:

> Hey Mike,
> 
> could you make a separate patch for the engraver, fixing the issue
> below; I think the engraver can go in without further discussion.
> 
> 
>
http://codereview.appspot.com/4022045/diff/12001/lily/beam-collision-engraver.cc
> File lily/beam-collision-engraver.cc (right):
> 
>
http://codereview.appspot.com/4022045/diff/12001/lily/beam-collision-engraver...
> lily/beam-collision-engraver.cc:107: }
> I think you are adding the note heads of the beam itself here as well.
> Can you make sure you only do that for noteheads from other voices?
> 
> http://codereview.appspot.com/4022045/

Done & attached, but I don't know if it's a good idea to put an engraver in the
source that doesn't do anything yet.  That said, I don't mind sharing it as a
patch if you (or anyone else) wants to build work off of it.

Cheers,
MS

[Graham Percival, 2011-02-04 01:56:54]

On 2/4/11, mike@apollinemike.com <mike@apollinemike.com> wrote:
> On Feb 3, 2011, at 7:33 PM, hanwenn@gmail.com wrote:
>
>> could you make a separate patch for the engraver, fixing the issue
>> below; I think the engraver can go in without further discussion.
>
> Done & attached, but I don't know if it's a good idea to put an engraver in
> the source that doesn't do anything yet.

Well, it passes a regtest check.  ;)
In all seriousness, it may well be worth pushing it just to keep
things organized, have everybody working from a common base, etc.

Cheers,
- Graham

[mike_apollinemike.com, 2011-02-04 02:47:04]

On Feb 3, 2011, at 8:56 PM, Graham Percival wrote:

> On 2/4/11, mike@apollinemike.com <mike@apollinemike.com> wrote:
>> On Feb 3, 2011, at 7:33 PM, hanwenn@gmail.com wrote:
>> 
>>> could you make a separate patch for the engraver, fixing the issue
>>> below; I think the engraver can go in without further discussion.
>> 
>> Done & attached, but I don't know if it's a good idea to put an engraver in
>> the source that doesn't do anything yet.
> 
> Well, it passes a regtest check.  ;)
> In all seriousness, it may well be worth pushing it just to keep
> things organized, have everybody working from a common base, etc.
> 
> Cheers,
> - Graham

That works too!  We just have to make sure to make a note in the docs if it
winds up being part of 2.14 without any real beam collision.  Otherwise, if
people try to figure out the logic of the program by reading through the source
(which is how I have a quarter of the half clue I currently have about what's
going on), they may get confused.

I found a Ligeti example that I also threw up on the site w/ the other beam
collision examples - it's a minefield for voice crossing, some of which pushes
the beam up by as much as a 5th.

Cheers,
MS