Draws a line above footnotes

Draws a line above footnotes


Footnote sketch

[Bertrand Bordage, 2011-02-21 13:04:45]

Wow, nice job Mike !
I am also currently trying to solve this issue.
With Scheme only I managed to do a good solution for endnotes.

I have some suggestions for your work :
* There should be more space between separator line and footnotes.
* Maybe a smaller separator, like LaTeX does. line-width / 3 for example.
* Maybe spread the footnotes on several columns, like this :
1      4      7
2      5      8
3      6
* A more user-friendly set of commands ?
* This doesn't support text footnotes...

But this is a great work, indeed !

[mikesol_ufl.edu, 2011-02-21 15:24:14]

Hey Bertrand,

Thanks for taking the time to look this over!

On Feb 21, 2011, at 8:04 AM, bordage.bertrand@gmail.com wrote:

> Wow, nice job Mike !
> I am also currently trying to solve this issue.
> With Scheme only I managed to do a good solution for endnotes.
> 

Scheme is a good idea for endnotes because endnotes would not need to be
specially accounted for in the C++ spacing algorithms - you can effectively
throw a slew of markups @ the end of a document & the page breaking algorithms
will handle them correctly.

> I have some suggestions for your work :
> * There should be more space between separator line and footnotes.

I'm gonna make this tweakable in the next version of the patch

> * Maybe a smaller separator, like LaTeX does. line-width / 3 for
> example.

I'm also gonna make this tweakable.

> * Maybe spread the footnotes on several columns, like this :
> 1      4      7
> 2      5      8
> 3      6

This would be really hard to institute as default behavior - LaTeX can
automatically wrap text, but the way I have it the algorithm now, footnotes are
just markups.  People can choose to wrap them or not.  Perhaps I'll have a
layout configuration that can be passed to the footnote maker so that it can
place things according to a user-defined ordering.

> * A more user-friendly set of commands ?

Once I get more comments in, I'll figure out what these should look like & do
it.

> * This doesn't support text footnotes...
> 

I'm not exactly sure what you mean...attached is the example file I've been
generating using the .ly file I sent out to the list yesterday. This example has
text.

> But this is a great work, indeed !
> 

Thank you very much for your helpful comments!

Cheers,
Mike

[joeneeman, 2011-02-22 01:05:49]

http://codereview.appspot.com/4167063/diff/3018/lily/note-head.cc
File lily/note-head.cc (right):

http://codereview.appspot.com/4167063/diff/3018/lily/note-head.cc#newcode189
lily/note-head.cc:189: "footnote "
Any particular reason this belongs to note-head-interface (rather than, say,
item-interface)?

http://codereview.appspot.com/4167063/diff/3018/lily/page-breaking.cc
File lily/page-breaking.cc (right):

http://codereview.appspot.com/4167063/diff/3018/lily/page-breaking.cc#newcode516
lily/page-breaking.cc:516: Page_breaking::draw_page (SCM systems, SCM footnotes,
SCM configuration, int page_num, bool last)
Surely draw_page can extract the footnotes from the systems rather than taking
them as an argument? Then you don't have to keep passing them around, and you
get support for the other page breaking algorithms for free.

http://codereview.appspot.com/4167063/diff/3018/lily/page-layout-problem.cc
File lily/page-layout-problem.cc (right):

http://codereview.appspot.com/4167063/diff/3018/lily/page-layout-problem.cc#n...
lily/page-layout-problem.cc:65: Stencil mol;
Why not make the separator a (stencil-valued) property of the paper-book?

http://codereview.appspot.com/4167063/diff/3018/lily/page-spacing.cc
File lily/page-spacing.cc (right):

http://codereview.appspot.com/4167063/diff/3018/lily/page-spacing.cc#newcode81
lily/page-spacing.cc:81: for (vsize i = 0; i < line.footnotes_.size (); i++)
You'll need to do this in append_system also.

http://codereview.appspot.com/4167063/diff/3018/lily/system.cc
File lily/system.cc (right):

http://codereview.appspot.com/4167063/diff/3018/lily/system.cc#newcode623
lily/system.cc:623: footnote_search (Grob *g, vsize start, vsize end,
vector<Stencil*>* s)
Have you checked the performance of this? This part is linear and so it makes
break_into_pieces quadratic. Also, you can make it simpler by iterating through
"all-elements" instead of recursing through "elements".

[MikeSol, 2011-02-22 15:23:00]

Thanks Joe!

http://codereview.appspot.com/4167063/diff/3018/lily/note-head.cc
File lily/note-head.cc (right):

http://codereview.appspot.com/4167063/diff/3018/lily/note-head.cc#newcode189
lily/note-head.cc:189: "footnote "
On 2011/02/22 01:05:49, joeneeman wrote:
> Any particular reason this belongs to note-head-interface (rather than, say,
> item-interface)?

No, changed to item-interface.

http://codereview.appspot.com/4167063/diff/3018/lily/page-breaking.cc
File lily/page-breaking.cc (right):

http://codereview.appspot.com/4167063/diff/3018/lily/page-breaking.cc#newcode516
lily/page-breaking.cc:516: Page_breaking::draw_page (SCM systems, SCM footnotes,
SCM configuration, int page_num, bool last)
On 2011/02/22 01:05:49, joeneeman wrote:
> Surely draw_page can extract the footnotes from the systems rather than taking
> them as an argument? Then you don't have to keep passing them around, and you
> get support for the other page breaking algorithms for free.

Fixed.

http://codereview.appspot.com/4167063/diff/3018/lily/page-layout-problem.cc
File lily/page-layout-problem.cc (right):

http://codereview.appspot.com/4167063/diff/3018/lily/page-layout-problem.cc#n...
lily/page-layout-problem.cc:65: Stencil mol;
On 2011/02/22 01:05:49, joeneeman wrote:
> Why not make the separator a (stencil-valued) property of the paper-book?

I could, but currently, this patch employs its current stencil kludge for the
following (perhaps-unfounded) reason.  Please let me know if I am correct in
worrying about this.

The page spacer, when it is calculating rod_height_, takes into account the
height of footnotes in the current patch.  However, it also needs to take into
account the height of the footnote separator.  If the height and padding of this
separator is encoded in the paper-book, there will be no way for the page spacer
to have access to this information (I think...).  Is there a good way to get the
spacer this info w/o more kludgery?

http://codereview.appspot.com/4167063/diff/3018/lily/page-spacing.cc
File lily/page-spacing.cc (right):

http://codereview.appspot.com/4167063/diff/3018/lily/page-spacing.cc#newcode81
lily/page-spacing.cc:81: for (vsize i = 0; i < line.footnotes_.size (); i++)
On 2011/02/22 01:05:49, joeneeman wrote:
> You'll need to do this in append_system also.

Done.

http://codereview.appspot.com/4167063/diff/3018/lily/system.cc
File lily/system.cc (right):

http://codereview.appspot.com/4167063/diff/3018/lily/system.cc#newcode623
lily/system.cc:623: footnote_search (Grob *g, vsize start, vsize end,
vector<Stencil*>* s)
On 2011/02/22 01:05:49, joeneeman wrote:
> Have you checked the performance of this? This part is linear and so it makes
> break_into_pieces quadratic. Also, you can make it simpler by iterating
through
> "all-elements" instead of recursing through "elements".

I've made the change to iterate through all-elements, but I still need to
measure the performance of the algorithm.  How would I do this?

[Bertrand Bordage, 2011-02-22 17:24:37]

I meant : we can't use this to do a footnote when in a markup or markuplines.
Something like :
\markuplines { bla bla bla \footnote { Oh yes ? } bla bla bla }

I know lilypond-book is the best solution for this issue. But it would be easier
if LilyPond had a native support of these features.

[mikesol_ufl.edu, 2011-02-22 23:53:38]

I cooked up something that more or less does the job.  It's up on Rietveld.

Cheers,
MS

[Bertrand Bordage, 2011-02-23 00:01:10]

:p
Great job !
There's no word to thank you enough !

Thanks thanks thanks,
Bertrand

[joeneeman, 2011-02-23 00:17:51]

On 2011/02/22 15:23:00, MikeSol wrote:
> On 2011/02/22 01:05:49, joeneeman wrote:
> > Why not make the separator a (stencil-valued) property of the paper-book?
> 
> I could, but currently, this patch employs its current stencil kludge for the
> following (perhaps-unfounded) reason.  Please let me know if I am correct in
> worrying about this.
> 
> The page spacer, when it is calculating rod_height_, takes into account the
> height of footnotes in the current patch.  However, it also needs to take into
> account the height of the footnote separator.  If the height and padding of
this
> separator is encoded in the paper-book, there will be no way for the page
spacer
> to have access to this information (I think...).  Is there a good way to get
the
> spacer this info w/o more kludgery?

If you give Page_breaking a public accessor for its paper-book, then you can
access it in Page_spacing. For performance (because Page_spacing::calc_force
gets called a lot), it's probably best to access the stencil and cache its
height in Page_spacing's constructor.

> On 2011/02/22 01:05:49, joeneeman wrote:
> > Have you checked the performance of this? This part is linear and so it
makes
> > break_into_pieces quadratic. Also, you can make it simpler by iterating
> through
> > "all-elements" instead of recursing through "elements".
> 
> I've made the change to iterate through all-elements, but I still need to
> measure the performance of the algorithm.  How would I do this?

The change to all-elements was just for simplicity; I still think it will be
quadratic. You can do a profiling build with
./configure --enable-conf=prof --enable-profiling --disable-optimising
make conf=prof
(and the binary will be out-prof/bin/lilypond). You can also just time lilypond
on a largish score (ie. multiple minutes of processing time). You can find such
scores on the mailing list if you don't have one; Hu Haipeng often sends them.
Or you can try Valentin's opera, but that's more like multiple hours of
processing time...

[mikesol_ufl.edu, 2011-02-23 04:14:27]

On Feb 22, 2011, at 7:17 PM, joeneeman@gmail.com wrote:

> On 2011/02/22 15:23:00, MikeSol wrote:
>> On 2011/02/22 01:05:49, joeneeman wrote:
>> > Why not make the separator a (stencil-valued) property of the
> paper-book?
> 
>> I could, but currently, this patch employs its current stencil kludge
> for the
>> following (perhaps-unfounded) reason.  Please let me know if I am
> correct in
>> worrying about this.
> 
>> The page spacer, when it is calculating rod_height_, takes into
> account the
>> height of footnotes in the current patch.  However, it also needs to
> take into
>> account the height of the footnote separator.  If the height and
> padding of this
>> separator is encoded in the paper-book, there will be no way for the
> page spacer
>> to have access to this information (I think...).  Is there a good way
> to get the
>> spacer this info w/o more kludgery?
> 
> If you give Page_breaking a public accessor for its paper-book, then you
> can access it in Page_spacing. For performance (because
> Page_spacing::calc_force gets called a lot), it's probably best to
> access the stencil and cache its height in Page_spacing's constructor.
> 
>> On 2011/02/22 01:05:49, joeneeman wrote:
>> > Have you checked the performance of this? This part is linear and so
> it makes
>> > break_into_pieces quadratic. Also, you can make it simpler by
> iterating
>> through
>> > "all-elements" instead of recursing through "elements".
> 
>> I've made the change to iterate through all-elements, but I still need
> to
>> measure the performance of the algorithm.  How would I do this?
> 
> The change to all-elements was just for simplicity; I still think it
> will be quadratic. You can do a profiling build with
> ./configure --enable-conf=prof --enable-profiling --disable-optimising
> make conf=prof
> (and the binary will be out-prof/bin/lilypond). You can also just time
> lilypond on a largish score (ie. multiple minutes of processing time).
> You can find such scores on the mailing list if you don't have one; Hu
> Haipeng often sends them. Or you can try Valentin's opera, but that's
> more like multiple hours of processing time...
> 

You're right - it takes a long time :(

Perhaps it'd be wise to make a footnote switch that allows users to opt-out of
this slowness.

The problem is that I'm having trouble drafting a footnote engraver because said
engraver would (in theory) need to access grobs' systems, which are not
available when translation happens.

Alternatively, is there a grob that I can acknowledge during the translation
process that will always exist and will be easily accessible from the systems? 
If so, I can just use the pointer group interface to create a grob array of
footnoted grobs.

Cheers,
MS

[joeneeman, 2011-02-23 07:19:51]

On Wed, Feb 23, 2011 at 3:14 PM, Mike Solomon <mikesol@ufl.edu> wrote:

> On Feb 22, 2011, at 7:17 PM, joeneeman@gmail.com wrote:
>
> > On 2011/02/22 15:23:00, MikeSol wrote:
>
> >> On 2011/02/22 01:05:49, joeneeman wrote:
> >> > Have you checked the performance of this? This part is linear and so
> > it makes
> >> > break_into_pieces quadratic. Also, you can make it simpler by
> > iterating
> >> through
> >> > "all-elements" instead of recursing through "elements".
> >
> >> I've made the change to iterate through all-elements, but I still need
> > to
> >> measure the performance of the algorithm.  How would I do this?
> >
> > The change to all-elements was just for simplicity; I still think it
> > will be quadratic. You can do a profiling build with
> > ./configure --enable-conf=prof --enable-profiling --disable-optimising
> > make conf=prof
> > (and the binary will be out-prof/bin/lilypond). You can also just time
> > lilypond on a largish score (ie. multiple minutes of processing time).
> > You can find such scores on the mailing list if you don't have one; Hu
> > Haipeng often sends them. Or you can try Valentin's opera, but that's
> > more like multiple hours of processing time...
> >
>
> You're right - it takes a long time :(
>
> Perhaps it'd be wise to make a footnote switch that allows users to opt-out
> of this slowness.
>

Better just to make it faster :). What if you add a vector<Grob*> to System
with all the footnoted grobs? The first time get_footnotes_in_range is
called, you do the search, populate the vector and sort it according to
column rank. From then on, you respond to get_footnotes_in_range by doing a
binary search for the upper and lower bounds.

Alternatively, is there a grob that I can acknowledge during the translation
> process that will always exist and will be easily accessible from the
> systems?  If so, I can just use the pointer group interface to create a grob
> array of footnoted grobs.
>

If you'd rather use an engraver to collect the footnotes, you could just
catch all the footnoted items in Score_engraver. You'll still need to sort
the array and use binary searches, though.

Cheers,
Joe

[Bertrand Bordage, 2011-02-23 12:30:05]

New patch here : http://codereview.appspot.com/4213042