Re: setting the number of pages for a score

[Han-Wen Nienhuys]

Joe Neeman wrote:
> Oops, I sent this to myself rather than the list...
>
> > Anyway, I hope to have a (preliminary) version of the page/line 
> > breaker combination up later today, so people can at least see the 
> > design and ideas in it.
>
>
> Ok, it took a bit longer than I thought, but here it is.
>
> Known limitations:
> - it won't let you start on the right-hand page
> - it will choke badly (and probably crash) if it can't find 
> line-breaking that satisfies constraints
> - it is slow
> - the loop-breaking in Optimal_breaking::calc_subproblem is particularly 
> ugly and one reason for the slowness
> - it prints a lot of debugging output
>
> You use it by adding #(define page-breaking ly:optimal-breaking) in the 
> book's \paper block. The (paper-block) variables "blank-page-force" and 
> "blank-last-page-force" contain the penalties for leaving blank pages in 
> the middle and end of the score (the blank page will always be the 
> right-hand page). "system-height" specifies the height that the 
> page-breaker assumes systems to be. It doesn't affect how the systems 
> are actually spaced out at the end, just how they are broken into pages.

OK, here are some preliminary comments.

> +class Prob;

I've added this to lily-proto.hh

> +    %%
> +    %% the assumed system height that we use for page-breaking
> +    system-height = #(* 20 mm)

I doubt whethre this is convenient.  Since this isn't related to the 
page size, i think it's easier to use staff-space here, eg. a normal 
staff is 4 spaces high,

 system-height = #4

> ===================================================================
> RCS file: /sources/lilypond/lilypond/scm/page.scm,v
> retrieving revision 1.7
> diff -u -r1.7 page.scm
> --- scm/page.scm        10 Feb 2006 12:27:42 -0000      1.7
> +++ scm/page.scm        14 Feb 2006 00:27:26 -0000
> @@ -191,12 +191,31 @@
>  
>      head-stencil))
>  
> +;; this is basically calc-printable-page height, but it works just from the paper-book
> +(define-public (calc-page-height p-book p-number last?)

Why? Can't you do

  (calc-printable-height (make-page paper-book)) ?

alternatively, you could express calc-printable-height with 
calc-page-height. We want to avoid dupicate code.

> #include "ly-smobs.icc"

> System_spec::System_spec (Paper_score *ps, int n_brks)

Yes, this is a good idea. My plan for the 3.0 syntax was to make 
System_spec an explicit part of the input format. The book body is just 
a list of system-specs.

In your code, System_spec is a C++ object, not accessible from Scheme. I 
think that this object should considered for smobification. This will 
make dealing with GC easier. Right now, this code is leaking memory, as 
the prob_ that is created from make_paper_system() is never 
unprotected. An alternative is to store the prob_ reference
in a Protected_scm too.

>   n_score_brk_ = 0;

a nitpick, but can you use fully spelled words? This should probably be

  score_break_count_

or something similar. Similarly for idx.


>   int p1_sys_left = me.p1_sys_ - 1;
>   int p2_sys_left = me.p1_sys_ + me.p2_sys_ - 1;

p1, p2  ?

Also, I prefer /**/ style comments.

> // TODO: allow starting on the right-hand page
> // this is quite long, but I can't see any obvious places to split it
> Real
> Optimal_breaking::best_page_force (vector<System_spec> const &sys,
>                                    int nsys,
>                                    vector<int> const &div,
>                                    int p_num,
>                                    bool last,
>                                    int &p1_sys)
> {
>   SCM ph = ly_lily_module_constant("calc-page-height");

You're hooking into the backend very directly; it would be better to do

  (calc-height (make-page paper))

this allows users to make clean overrides for page-layout. At the moment 
it might not be terribly relevant, but I hope it will be possible in 
some future.

>           for (vsize j = 0; (int)j < div[score_idx]; j++)

Is it possible to write vector indexing code without casts ? this would 
benefit readability.

>   SCM props = book_->paper_->lookup_variable (ly_symbol2scm 
> ("book-title-properties"));
>   if (add_prob (props, book_->book_title ()))
>     add_break_penalty (book_->header_);

 I have some reservations about this, in the sense that the 
interactions between paper_system, paper_score, score, book, etc. are 
rather hairy now, with different code paths for normal use and 
lilypond-book. I'm worried that this adds another path, which may lead 
to more inconsistencies and bugs

in particular, the following

>   SCM page_properties
>     = scm_call_1 (ly_lily_module_constant ("layout-extract-page-properties"),
>                   book_->paper_->self_scm ());
>
>   SCM header = SCM_EOL;
>   for (SCM s = scm_reverse (book_->scores_); s != SCM_EOL; s = scm_cdr (s))
>     {
>  [..]

Seems like duplication of code that we already have.

> /* start_idx and sys here are indexed from 0.
>  * max_brk is indexed from start_brk_idx_[start_idx]
>  * (for max_brk, start_brk_idx_[start_idx] is the beginning

(see above: can you reconsider variable names?)


How about the following proposal for integrating this work?

 - my tentative schedule for 2.8 is to further stabilize 2.7 during the 
month February, and release 2.8 somewhere in March.

 - We could add restrained line breaking, eg. in 2.7.35, and let users 
shake out bugs and other problems in that code.

 - Once 2.8 is out of the door, we integrate optimal page breaking, and 
refactor things on the input side, to put System_spec in as a part of 
the input grammar.

 - In the meantime, would it be worthwhile to setup some kind of 
infrastructure so work on the page breaker can proceed in parallel?

--
 Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen