Overhauls broken bound coordinate checking

Overhauls broken bound coordinate checking

[Neil Puttock, 2012-02-01 15:04:47]

http://codereview.appspot.com/5602054/diff/2001/input/regression/metronome-ma...
File input/regression/metronome-mark-broken-bound.ly (right):

http://codereview.appspot.com/5602054/diff/2001/input/regression/metronome-ma...
input/regression/metronome-mark-broken-bound.ly:1: \version "2.15.27"
2.15.28

http://codereview.appspot.com/5602054/diff/2001/input/regression/metronome-ma...
input/regression/metronome-mark-broken-bound.ly:4: texidoc = "A broken spanner
must not interfere with @code{\\tempo}
Surely you mean break-alignable grobs shouldn't interfere with broken spanners? 
You have this back-to-front.

http://codereview.appspot.com/5602054/diff/2001/input/regression/metronome-ma...
input/regression/metronome-mark-broken-bound.ly:12: \tempo \markup { "fooooo" }
4 = 90
You might add

\override Score.MetronomeMark #'break-visibility = #all-visible

otherwise you're only checking the left-broken spanners.

http://codereview.appspot.com/5602054/diff/2001/input/regression/metronome-ma...
input/regression/metronome-mark-broken-bound.ly:22: \ottava #1 \times 1/1 {
e'8\<\startTextSpan\startTrillSpan\glissando\episemInitium
If you want to test episema, you need to add the engraver (it's not worth it imo
since you wouldn't use it outside a VaticanaVoice and it would never be broken).

The code's a bit messy here

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

http://codereview.appspot.com/5602054/diff/2001/scm/define-grobs.scm#newcode1420
scm/define-grobs.scm:1420: (bound-alignment-forbidden-interfaces .
(metronome-mark-interface))
I question whether you need this property.  Would be better just to exclude
break-alignable-interface in axis-group-interface.cc (so it includes
RehearsalMark and BarNumber - note that these will also interfere with bounds if
you leave them out here)

[MikeSol, 2012-02-01 15:14:47]

http://codereview.appspot.com/5602054/diff/2001/input/regression/metronome-ma...
File input/regression/metronome-mark-broken-bound.ly (right):

http://codereview.appspot.com/5602054/diff/2001/input/regression/metronome-ma...
input/regression/metronome-mark-broken-bound.ly:1: \version "2.15.27"
On 2012/02/01 15:04:47, Neil Puttock wrote:
> 2.15.28

Fixed.

http://codereview.appspot.com/5602054/diff/2001/input/regression/metronome-ma...
input/regression/metronome-mark-broken-bound.ly:4: texidoc = "A broken spanner
must not interfere with @code{\\tempo}
On 2012/02/01 15:04:47, Neil Puttock wrote:
> Surely you mean break-alignable grobs shouldn't interfere with broken
spanners? 
> You have this back-to-front.

I may or may not have copied and pasted this text with out reading it.

http://codereview.appspot.com/5602054/diff/2001/input/regression/metronome-ma...
input/regression/metronome-mark-broken-bound.ly:12: \tempo \markup { "fooooo" }
4 = 90
On 2012/02/01 15:04:47, Neil Puttock wrote:
> You might add
> 
> \override Score.MetronomeMark #'break-visibility = #all-visible
> 
> otherwise you're only checking the left-broken spanners.

Done.

http://codereview.appspot.com/5602054/diff/2001/input/regression/metronome-ma...
input/regression/metronome-mark-broken-bound.ly:22: \ottava #1 \times 1/1 {
e'8\<\startTextSpan\startTrillSpan\glissando\episemInitium
On 2012/02/01 15:04:47, Neil Puttock wrote:
> If you want to test episema, you need to add the engraver (it's not worth it
imo
> since you wouldn't use it outside a VaticanaVoice and it would never be
broken).
> 
> The code's a bit messy here

Fair nuf - removed.

http://codereview.appspot.com/5602054/diff/2001/input/regression/metronome-ma...
input/regression/metronome-mark-broken-bound.ly:22: \ottava #1 \times 1/1 {
e'8\<\startTextSpan\startTrillSpan\glissando\episemInitium
On 2012/02/01 15:04:47, Neil Puttock wrote:
> If you want to test episema, you need to add the engraver (it's not worth it
imo
> since you wouldn't use it outside a VaticanaVoice and it would never be
broken).
> 
> The code's a bit messy here

Fair nuf - omitted.  What'd be the proper formatting here?

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

http://codereview.appspot.com/5602054/diff/2001/scm/define-grobs.scm#newcode1420
scm/define-grobs.scm:1420: (bound-alignment-forbidden-interfaces .
(metronome-mark-interface))
On 2012/02/01 15:04:47, Neil Puttock wrote:
> I question whether you need this property.  Would be better just to exclude
> break-alignable-interface in axis-group-interface.cc (so it includes
> RehearsalMark and BarNumber - note that these will also interfere with bounds
if
> you leave them out here)

If one could never conceive of a case where a bound would need to include these
things, then I could hard code it, but I tend to favor using properties in case
there is a scenario I can't imagine where a person would want these to be
included in NonMusicalPaperColumn bounds.

[MikeSol, 2012-02-02 08:11:03]

Hey all,

The newest version of this misses up some of the hairpin regtests
(hairpin-barline-break.ly, for example).  Still not sure why - I don't have time
to check it out for a bit, but if anyone has intuition as to why, lemme know!

Cheers,
MS

[Keith, 2012-02-04 08:19:17]

LFTM

http://codereview.appspot.com/5602054/diff/6003/lily/axis-group-interface.cc
File lily/axis-group-interface.cc (right):

http://codereview.appspot.com/5602054/diff/6003/lily/axis-group-interface.cc#...
lily/axis-group-interface.cc:114: SCM forbidden_interfaces = me->get_property
("bound-alignment-forbidden-interfaces");
The logic is quite complicated, with the positive set
"bound-alignment-interfaces", which if unspecified includes everything, and then
negative set "bound-alignment-forbidden-interfaces".

Did you try leaving this file alone, but specifying 'bound-alignment-interfaces
where needed ?

http://codereview.appspot.com/5602054/diff/6003/lily/axis-group-interface.cc#...
lily/axis-group-interface.cc:127: new_elts.erase (new_elts.begin () + i); }
Did you mean to put a break; ?
That is, if any item ever supports two 'forbidden interfaces', you don't want to
check for an interface in the item you just deleted.

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

http://codereview.appspot.com/5602054/diff/6003/scm/define-grobs.scm#newcode1420
scm/define-grobs.scm:1420: (bound-alignment-forbidden-interfaces .
(break-alignable-interface))
Why not, instead, 
 (bound-alignment-interfaces . (break-aligned-interface))
?

[Keith, 2012-02-04 08:57:56]

Dumping 'bound-alignment-forbidden-interfaces and the changes to
axis-group-interface.cc, in favor of a 'bound-alignment-interfaces for
NonMusicalPaperColumn, works for me.  It restores that one regression test
'text-spanner-full-rest.ly'.