Re: Draws a line above footnotes (issue4167063)

[mtsolo]

Reviewers: Bertrand Bordage, mikesol_ufl.edu, joeneeman,

Message:
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#newcode65
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?

Description:
Draws a line above footnotes


Footnote sketch

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

Affected files:
  M lily/constrained-breaking.cc
  M lily/include/constrained-breaking.hh
  M lily/include/page-breaking.hh
  M lily/include/page-layout-problem.hh
  M lily/include/page-spacing.hh
  M lily/include/system.hh
  M lily/item.cc
  M lily/page-breaking.cc
  M lily/page-layout-problem.cc
  M lily/page-spacing.cc
  M lily/system.cc
  M ly/paper-defaults-init.ly
  M scm/define-grob-properties.scm