Re: Enhancement: Page breaking to avoid widow lines

[Joe Neeman]

> Here is the new code -- implementing all that's been discussed.  (Ok, 
> not quite all -- the line length functionality is not implemented,
> but 
> all the style remarks have been taken into account, the
> markup-list-id 
> has been replaced with is-last-line, penalty made configurable, and 
> also, (new, was not discussed yet), the opposite case of lonely first 
> (as opposed to last) line is now properly handled. 

Again, I'd suggest uploading patches to codereview.appspot.com, which
provides nice formatting and makes it easy to have multiple reviewers.


> diff --git a/lily/constrained-breaking.cc
> b/lily/constrained-breaking.cc
> index 2fead24..6497cd5 100644
> --- a/lily/constrained-breaking.cc
> +++ b/lily/constrained-breaking.cc
> @@ -525,4 +525,8 @@ Line_details::Line_details (Prob *pb, Output_def
> *paper)
>    title_ = to_boolean (pb->get_property ("is-title"));
>    compressed_lines_count_ = 1;
>    compressed_nontitle_lines_count_ = title_ ? 0 : 1;
> +  SCM last_scm  = pb->get_property ("last-markup-line");
> +  last_markup_line_  = SCM_BOOLP(last_scm)  ? scm_to_bool
> (last_scm)  : 0;

Use to_boolean

> +  SCM first_scm = pb->get_property ("first-markup-line");
> +  first_markup_line_ = SCM_BOOLP(first_scm) ? scm_to_bool
> (first_scm) : 0;
>  }
> diff --git a/lily/include/constrained-breaking.hh
> b/lily/include/constrained-breaking.hh
> index bb8a1d0..b9590a9 100644
> --- a/lily/include/constrained-breaking.hh
> +++ b/lily/include/constrained-breaking.hh
> @@ -52,6 +52,7 @@ struct Line_details {
>       class. */
>    int compressed_lines_count_;
>    int compressed_nontitle_lines_count_;
> +  bool last_markup_line_, first_markup_line_;

put this on two lines

>  
>    Line_details ()
>    {
> @@ -71,6 +72,8 @@ struct Line_details {
>      title_ = false;
>      compressed_lines_count_ = 1;
>      compressed_nontitle_lines_count_ = 1;
> +    last_markup_line_ = 0;
> +    first_markup_line_ = 0;
>    }

false instead of 0
 
>    Line_details (Prob *pb, Output_def *paper);
> diff --git a/lily/include/page-breaking.hh
> b/lily/include/page-breaking.hh
> index 2e5283d..d497d49 100644
> --- a/lily/include/page-breaking.hh
> +++ b/lily/include/page-breaking.hh
> @@ -127,6 +127,7 @@ public:
>    bool too_few_lines (int line_count) const;
>    Real min_whitespace_at_top_of_page (Line_details const&) const;
>    Real min_whitespace_at_bottom_of_page (Line_details const&) const;
> +  int orphan_penalty () const;
>  
>  protected:
>    Paper_book *book_;
> @@ -179,6 +180,7 @@ private:
>    int max_systems_per_page_;
>    int min_systems_per_page_;
>    vsize system_count_;
> +  int orphan_penalty_;
>  
>    vector<Line_division> current_configurations_;
>    vector<Break_position> current_chunks_;
> diff --git a/lily/page-breaking.cc b/lily/page-breaking.cc
> index e0a08c0..dc71659 100644
> --- a/lily/page-breaking.cc
> +++ b/lily/page-breaking.cc
> @@ -174,6 +174,7 @@ Page_breaking::Page_breaking (Paper_book *pb,
> Break_predicate is_break)
>    systems_per_page_ = max (0, robust_scm2int (pb->paper_->c_variable
> ("systems-per-page"), 0));
>    max_systems_per_page_ = max (0, robust_scm2int
> (pb->paper_->c_variable ("max-systems-per-page"), 0));
>    min_systems_per_page_ = max (0, robust_scm2int
> (pb->paper_->c_variable ("min-systems-per-page"), 0));
> +  orphan_penalty_ = robust_scm2int (pb->paper_->c_variable
> ("orphan-penalty"), 100000);
>  
>    if (systems_per_page_ && (max_systems_per_page_ ||
> min_systems_per_page_))
>      {
> @@ -1415,3 +1416,9 @@ Page_breaking::min_whitespace_at_bottom_of_page
> (Line_details const &line) const
>    // FIXME: take into account the height of the footer
>    return max (0.0, max (padding, min_distance + line.extent_[DOWN]));
>  }
> +
> +int
> +Page_breaking::orphan_penalty () const
> +{
> +  return orphan_penalty_;
> +}
> diff --git a/lily/page-spacing.cc b/lily/page-spacing.cc
> index 8919257..8dbf183 100644
> --- a/lily/page-spacing.cc
> +++ b/lily/page-spacing.cc
> @@ -245,6 +245,18 @@ Page_spacer::calc_subproblem (vsize page, vsize
> line)
>             penalty += lines_[page_start-1].page_penalty_
>               + (page % 2 == 0) ? lines_[page_start-1].turn_penalty_ :
> 0;
>  
> +         /* Deal with widow/orphan lines */
> +         /* Last line of paragraph is first line on the new page */
> +         if ((page_start > 0) &&
> +             (page_start < lines_.size ()) &&
> +             (lines_[page_start].last_markup_line_))
> +                 penalty += breaker_->orphan_penalty ();
> +         /* First line of paragraph is last line on the previous page
> */
> +         if ((page_start > 0) &&
> +             (page_start < lines_.size ()) &&
> +             (lines_[page_start-1].first_markup_line_))
> +                 penalty += breaker_->orphan_penalty ();
> +
>           demerits += penalty;
>           if (demerits < cur.demerits_ || page_start == line)
>             {
> diff --git a/lily/paper-book.cc b/lily/paper-book.cc
> index 79896e6..cec0217 100644
> --- a/lily/paper-book.cc
> +++ b/lily/paper-book.cc
> @@ -516,15 +516,19 @@ Paper_book::get_system_specs ()
>                                   paper_->self_scm (),
>                                   page_properties,
>                                   scm_car (s));
> +         Prob *ps;
>           for (SCM list = texts ; scm_is_pair (list) ; list = scm_cdr
> (list))
>             {
>               SCM t = scm_car (list);
>               // TODO: init props
> -             Prob *ps = make_paper_system (SCM_EOL);
> +             ps = make_paper_system (SCM_EOL);
>               ps->set_property ("page-break-permission",
>                                 ly_symbol2scm ("allow"));
>               ps->set_property ("page-turn-permission",
>                                 ly_symbol2scm ("allow"));
> +             ps->set_property ("last-markup-line",  SCM_BOOL_F);
> +             ps->set_property ("first-markup-line",
> +                         list == texts? SCM_BOOL_T : SCM_BOOL_F);
>  
>               paper_system_set_stencil (ps, *unsmob_stencil (t));
>               ps->set_property ("is-title", SCM_BOOL_T);
> @@ -544,6 +548,11 @@ Paper_book::get_system_specs ()
>               // FIXME: figure out penalty. 
>               //set_system_penalty (ps, scores_[i].header_);
>             }
> +         // We may want to place two checks here:
> +         // (1) is the line too short?
> +         // (2) is the paragraph longer than one line?

(2) seems to be pretty important, no? If there is only one line then the
current code will avoid placing it first or last on any page.