On 5 March 2011 12:57, Mike Solomon <mike...@ufl.edu> wrote:
> Patch attached. The stuff that comes from your comments regarding
> break-visibility is implemented in Balloon_interface::is_visible.
> The patch currently represents about 85% of the original, omitting the 15%
> that Han Wan had previously identified as hold-off-on-able for a first push
> (the actual annotations). These are relatively painless to add back in.
> I realize that this is not a "small" chunk, but if I shaved anything else
> off, the footnotes wouldn't work.
> I'm running regtests now & will report back if anything breaks.
> Cheers,
> MS
> P.S. The original patch resides at http://codereview.appspot.com/4245062
I think there are several parts to this patch which should be removed:
1) the break-visibility code in balloon.cc
You've implemented something which looks suspiciously like a callback,
but you've also added a function to output-lib.scm which you call
instead via scm_call_1 () (you shouldn't need to do this for a grob).
All this code should be part of a callback for FootnoteItem
#'break-visibility.
This code doesn't work very well in some cases:
\new Staff \with {
\consists Footnote_engraver
}
\relative c' {
\footnoteGrob #'Clef #'(0 . 2) foo bar
c1
}
-> no footnote
\new Staff \with {
\consists Footnote_engraver
}
\relative c' {
c1
\footnoteGrob #'Clef #'(0 . 2) foo bar
c1
}
-> no footnote (good!), but suicided clefs are still accounted for
(all three) in account_for_footnotes ()
\new Staff \with {
\consists Footnote_engraver
}
\relative c' {
c1 \break
\footnoteGrob #'Clef #'(0 . 2) foo bar
c1
}
-> two footnotes, account_for_foonotes () still thinks there are three
For all these cases setting FootnoteItem #'break-visibility to
`inherit-x-parent-visibility' produces better results.
2) all the code related to `spanner-placement'
This is a really bad interface for selecting the correct spanner. I'd
much prefer something which would index the broken_intos_ (though I
guess this would run into the same problem you're having with the
footnote height calculations).
@@ -1237,7 +1277,7 @@
Page_breaking::space_systems_with_fixed_number_per_page (vsize
configuration,
while (system_count_on_this_page < systems_per_page_
&& line < cached_line_details_.size ())
{
- Line_details const &cur_line = cached_line_details_[line];
+ Line_details &cur_line = cached_line_details_[line];
looks like it's left over from a previous patch
Cheers,
Neil
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
