Re: Overhauls broken bound coordinate checking (issue 5602054)

2012-02-04 Thread k-ohara5a5a

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'.


http://codereview.appspot.com/5602054/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Overhauls broken bound coordinate checking (issue 5602054)

2012-02-04 Thread k-ohara5a5a

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#newcode114
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#newcode127
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))
?

http://codereview.appspot.com/5602054/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Overhauls broken bound coordinate checking (issue 5602054)

2012-02-02 Thread mtsolo

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

http://codereview.appspot.com/5602054/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Overhauls broken bound coordinate checking (issue 5602054)

2012-02-01 Thread mtsolo

Reviewers: Neil Puttock,


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

http://codereview.appspot.com/5602054/diff/2001/input/regression/metronome-mark-broken-bound.ly#newcode1
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-mark-broken-bound.ly#newcode4
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-mark-broken-bound.ly#newcode12
input/regression/metronome-mark-broken-bound.ly:12: \tempo \markup {
"fo" } 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-mark-broken-bound.ly#newcode22
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-mark-broken-bound.ly#newcode22
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.

Description:
Overhauls broken bound coordinate checking

Please review this at http://codereview.appspot.com/5602054/

Affected files:
  A input/regression/metronome-mark-broken-bound.ly
  M lily/axis-group-interface.cc
  M lily/beam.cc
  M lily/lyric-hyphen.cc
  M lily/ottava-bracket.cc
  M lily/tuplet-bracket.cc
  M scm/define-grob-properties.scm
  M scm/define-grobs.scm


Index: input/regression/metronome-mark-broken-bound.ly
diff --git a/input/regression/metronome-mark-broken-bound.ly  
b/input/regression/metronome-mark-broken-bound.ly

new file mode 100644
index  
..5ba5064065ab8e3340aca7861f55881ac2fdbb3e

--- /dev/null
+++ b/input/regression/metronome-mark-broken-bound.ly
@@ -0,0 +1,36 @@
+\version "2.15.28"
+
+\header {
+texidoc = "A @code{MetronomeMark}, @code{RehearsalMark} and  
@code{BarNumber}

+should not effect the starting point of spanners.
+"
+}
+
+<<
+ \new Staff {
+   e'1 \time 4/4 \break |
+   \tempo \markup { "fo" } 4 = 90
+   e'1 |
+   e'1 |
+ }
+
+ \new Staff {
+   \override Score.MetronomeMark #'break-visibility = #all-visible
+   \override TupletBracket #'breakable = ##t
+   \override Beam #'breakable = ##t
+   \override Glissando #'breakable = ##t
+
+   \ottava #1 \times 1/1 { e'8\<\startTextSpan\startTrillSpan\glissando
+ [ \override NoteColumn #'glissando-skip = ##t\repeat unfold 22 e'8
+   \revert NoteColumn #'glissando-skip  
e'8\!\stopTextSpan\stopTrillSpan ] } |

+ }
+ \addlyrics { ah __ \repeat unfold 21 { \skip 4 } _ rrgh }
+ \addlyrics { ah --  \repeat unfold 21 { \skip 4 } _ rrgh }
+>>
+
+\layout {
+ \context {
+   \Voice
+   \remove "Forbid_line_break_engraver"
+ }
+}
Index: lily/axis-group-interface.cc
diff --git a/lily/axis-group-interface.cc b/lily/axis-group-interface.cc
index  
8857de109d6b3cb6ec438fae

Overhauls broken bound coordinate checking (issue 5602054)

2012-02-01 Thread n . puttock


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

http://codereview.appspot.com/5602054/diff/2001/input/regression/metronome-mark-broken-bound.ly#newcode1
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-mark-broken-bound.ly#newcode4
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-mark-broken-bound.ly#newcode12
input/regression/metronome-mark-broken-bound.ly:12: \tempo \markup {
"fo" } 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-mark-broken-bound.ly#newcode22
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)

http://codereview.appspot.com/5602054/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel