Re: [PATCH] New margin handling - final version (updated)

[Michael Käppler]

Hi all,
I've just removed some code from page.scm that becomes obsolete through 
the patch.
I would be really happy if somebody could test this for regressions. The 
problem is that for me it takes more than 12 hours to run a "make doc" 
and for this time I can't do anything other.
(Just to give you an impression about my system's slowness)

Regards,
Michael

>From 7235a6af9fc738cea7e35d0e5063f772c7e94af1 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Michael=20K=C3=A4ppler?= <address@hidden>
Date: Fri, 11 Sep 2009 12:06:37 +0200
Subject: [PATCH] Introduce new handling for paper margin settings.

* Make default margins accessible in ly/paper-defaults-init.ly

* Introduce a new method: Output_def::normalize (). It checks,
  whether left-margin, right-margin and/or line-width are set.
  Afterwards it computes missing values or takes defaults, if
  necessary. There is no need to specify line-width manually now.

* Make right-margin work

* If only line-width is set, the current behaviour persists.
  (Systems are centered)

* Output a warning if the margin values don't fit with each other

* Modify lilypond-book to get rid of the
  (define line-width (- line-width 3\mm)) stuff and introduce
  line-width-cropping for compatibility.
---
 input/regression/paper-margins.ly |   60 +++++++++++
 lily/book.cc                      |    1 +
 lily/include/output-def.hh        |    3 +-
 lily/output-def.cc                |   92 ++++++++++++++++-
 lily/paper-book.cc                |   11 ++-
 ly/paper-defaults-init.ly         |  206 ++++++++++++++++++++-----------------
 scm/page.scm                      |    6 +-
 scm/paper.scm                     |   18 ++--
 scripts/lilypond-book.py          |    2 +-
 9 files changed, 286 insertions(+), 113 deletions(-)
 create mode 100644 input/regression/paper-margins.ly

diff --git a/input/regression/paper-margins.ly 
b/input/regression/paper-margins.ly
new file mode 100644
index 0000000..2d9b5dc
--- /dev/null
+++ b/input/regression/paper-margins.ly
@@ -0,0 +1,60 @@
+\version "2.13.4"
+
+\header {
+  texidoc = "Paper margin settings don't have to be complete. Missing values 
are added automatically, finally 
+             everything is checked for consistency."
+}
+
+#(set-default-paper-size "a4")
+
+someNotes = \relative c' { \repeat unfold 40 { c4 d e f }}
+
+\book {
+  \bookpart {
+    \paper { }
+    \markup { If no paper settings are specified, default values are used. }   
 
+    \score { \someNotes }
+  }
+  \bookpart {
+    \paper {
+      left-margin = 40 \mm
+    }
+    \markup { You may also specify only some margins. Missing values are 
added, if possible. }
+    \markup { Here only left-margin is given, right-margin will remain 
default. }
+    \score { \someNotes }
+  }
+  \bookpart {
+    \paper {
+      right-margin = 40 \mm
+    }
+    \markup { Here only right-margin is given, left-margin will remain 
default. }
+    \score { \someNotes }
+  }
+  \bookpart {
+    \paper {
+      line-width = 100 \mm
+    }
+    \markup { If only line-width is given, the systems are vertically 
centered. }
+    \score { \someNotes }
+  }
+  \bookpart {
+    \paper {
+      left-margin = 20 \mm
+      right-margin = 40 \mm
+      line-width = 100 \mm
+    }
+    \markup { If you specify line-margin, right-margin and line-width, the 
values must be consistent. }
+    \markup { In case they are not, default margins are set and a warning is 
printed. }
+    \score { \someNotes }
+  }
+  \bookpart {
+    \paper {
+      left-margin = 20 \mm
+      right-margin = 40 \mm
+      line-width = 200 \mm
+      check-consistency = ##f
+    }
+    \markup { You can avoid this check with settings check-consistency to 
false in the paper block. }
+    \score { \someNotes }
+  }
+}
diff --git a/lily/book.cc b/lily/book.cc
index 303af1b..86eeda1 100644
--- a/lily/book.cc
+++ b/lily/book.cc
@@ -277,6 +277,7 @@ Book::process (Output_def *default_paper,
     }
   else
     {
+      paper_book->paper_->normalize ();
       /* Process scores */
   /* Render in order of parsing.  */
       for (SCM s = scm_reverse (scores_); scm_is_pair (s); s = scm_cdr (s))
diff --git a/lily/include/output-def.hh b/lily/include/output-def.hh
index b683cc6..9e28d18 100644
--- a/lily/include/output-def.hh
+++ b/lily/include/output-def.hh
@@ -58,13 +58,14 @@ public:
   SCM c_variable (string id) const;
   SCM lookup_variable (SCM sym) const;
   void set_variable (SCM sym, SCM val);
+  void normalize ();
   Real get_dimension (SCM symbol) const;
 };
 SCM get_font_table (Output_def *def);
 void assign_context_def (Output_def *m, SCM transdef);
 SCM find_context_def (Output_def const *m, SCM name);
 
-Interval line_dimensions_int (Output_def*def, int);
+Interval line_dimensions_int (Output_def *def, int);
  
 
 Font_metric *select_encoded_font (Output_def *layout, SCM chain);
diff --git a/lily/output-def.cc b/lily/output-def.cc
index f9fbda9..3f60861 100644
--- a/lily/output-def.cc
+++ b/lily/output-def.cc
@@ -11,6 +11,7 @@
 #include "context-def.hh"
 #include "file-path.hh"
 #include "global-context.hh"
+#include "international.hh"
 #include "interval.hh"
 #include "main.hh"
 #include "output-def.hh"
@@ -128,7 +129,96 @@ Output_def::set_variable (SCM sym, SCM val)
   scm_module_define (scope_, sym, val);
 }
 
-  
+void
+Output_def::normalize ()
+{
+  Real paper_width;
+  SCM scm_paper_width = c_variable ("paper-width");
+ 
+  Real left_margin, left_margin_default;
+  SCM scm_left_margin_default = c_variable ("left-margin-default");
+  SCM scm_left_margin = c_variable ("left-margin");
+
+  Real right_margin, right_margin_default;
+  SCM scm_right_margin_default = c_variable ("right-margin-default");
+  SCM scm_right_margin = c_variable ("right-margin");
+
+  if (scm_paper_width == SCM_UNDEFINED 
+      || scm_left_margin_default == SCM_UNDEFINED 
+      || scm_right_margin_default == SCM_UNDEFINED) 
+    {
+      programming_error ("called normalize() on paper with missing settings");
+      return;
+    } 
+  else
+    {
+      paper_width = scm_to_double (scm_paper_width);
+      left_margin_default = scm_to_double (scm_left_margin_default);
+      right_margin_default = scm_to_double (scm_right_margin_default); 
+    }
+
+  Real line_width;
+  Real line_width_default = paper_width - left_margin_default - 
right_margin_default;
+  SCM scm_line_width = c_variable ("line-width");
+
+  if (scm_line_width == SCM_UNDEFINED)
+    {
+      left_margin = ((scm_left_margin == SCM_UNDEFINED) ? left_margin_default 
: scm_to_double(scm_left_margin));
+      right_margin = ((scm_right_margin == SCM_UNDEFINED) ? 
right_margin_default : scm_to_double(scm_right_margin)); 
+      line_width = paper_width - left_margin - right_margin;
+    }
+  else
+    {
+      line_width = scm_to_double (scm_line_width);
+      if (scm_left_margin == SCM_UNDEFINED)
+        {
+          if (scm_right_margin == SCM_UNDEFINED) // Vertically center systems 
if only line-width is given
+            {
+              left_margin = (paper_width - line_width) / 2;
+              right_margin = left_margin;
+            }
+          else
+            {
+              right_margin = scm_to_double (scm_right_margin);
+              left_margin = paper_width - line_width - right_margin; 
+            }
+        }
+      else
+        {
+          left_margin = scm_to_double (scm_left_margin);
+          right_margin = ((scm_right_margin == SCM_UNDEFINED) 
+                           ? (paper_width - line_width - left_margin) 
+                           : scm_to_double (scm_right_margin)); 
+        }
+    }
+ 
+  if (to_boolean (c_variable ("check-consistency")))
+    {
+      // Consistency checks. If values don't match, set defaults.
+      if (abs(paper_width - line_width - left_margin - right_margin) > 1e-6) 
+        {
+          line_width = line_width_default;
+          left_margin = left_margin_default;
+          right_margin = right_margin_default;
+          warning (_ ("margins do not fit with line-width, setting default 
values"));          
+        } 
+      else if ((left_margin < 0) || (right_margin < 0))
+        {
+          line_width = line_width_default;
+          left_margin = left_margin_default;
+          right_margin = right_margin_default;
+          warning (_ ("systems run off the page due to improper paper 
settings, setting default values"));
+        }
+    }
+
+  // Lilypond-book needs the possibility to crop the line-width by a given 
amount
+  line_width = line_width - robust_scm2double (c_variable 
("line-width-cropping"), 0);  
+
+  set_variable (ly_symbol2scm ("left-margin"), scm_from_double (left_margin));
+  set_variable (ly_symbol2scm ("right-margin"), scm_from_double 
(right_margin));
+  set_variable (ly_symbol2scm ("line-width"), scm_from_double (line_width));
+}
+ 
 /* FIXME.  This is broken until we have a generic way of
    putting lists inside the \layout block.  */
 Interval
diff --git a/lily/paper-book.cc b/lily/paper-book.cc
index 3db1321..12a7f09 100644
--- a/lily/paper-book.cc
+++ b/lily/paper-book.cc
@@ -159,12 +159,21 @@ Paper_book::output (SCM output_channel)
 {
   int first_page_number = robust_scm2int (paper_->c_variable 
("first-page-number"), 1);
   int first_performance_number = 0;
+
+  /* FIXME: We need a line-width for ps output (framework-ps.scm:92). If we 
don't have any, we take the paper-width
+     unless we know better which line-width to choose (e.g. if there are 
\bookparts with different line-widths)
+     and why we need it at all. */
+
+  if (paper_->c_variable ("line-width") == SCM_UNDEFINED)
+    paper_->set_variable (ly_symbol2scm ("line-width"), paper_->c_variable 
("paper-width"));
+ 
   if (!output_aux (output_channel,
                   true,
                   &first_page_number,
                   &first_performance_number))
     return;
-      
+    
+ 
   SCM scopes = SCM_EOL;
   if (ly_is_module (header_))
     scopes = scm_cons (header_, scopes);
diff --git a/ly/paper-defaults-init.ly b/ly/paper-defaults-init.ly
index 1ee92ed..d742115 100644
--- a/ly/paper-defaults-init.ly
+++ b/ly/paper-defaults-init.ly
@@ -1,96 +1,116 @@
 \version "2.12.0"
 
 \paper {
-  %%% WARNING
-  %%%
-  %%% If you add any new dimensions, don't forget to update
-  %%% the dimension-variables variable.  See paper.scm.
-
-  unit = #(ly:unit)
-  mm = 1.0
-  in = 25.4
-  pt = #(/ in 72.27)
-  cm = #(* 10 mm)
-
-  print-page-number = ##t
-
-  %%
-  %% 20pt staff, 5 pt = 1.75 mm
-  %%
-
-  output-scale = #1.7573
-
-  #(define-public book-title (marked-up-title 'bookTitleMarkup))
-  #(define-public score-title (marked-up-title 'scoreTitleMarkup))
-
-  %%
-  %% ugh. hard coded?
-  %%
-
-  #(layout-set-absolute-staff-size (* 20.0 pt))
-
-
-  #(define-public score-title-properties
-     '((is-title . #t)
-       (is-book-title . #f)))
-  #(define-public book-title-properties
-     '((is-title . #t)
-       (is-book-title . #t)))
-
-  %% Note: these are not scaled; they are in staff-spaces.
-  between-system-spacing = #'((space . 12) (minimum-distance . 8) (padding . 
1))
-  between-scores-system-spacing = #'((space . 14) (minimum-distance . 8) 
(padding . 1))
-  after-title-spacing = #'((space . 2) (padding . 0.5))
-  before-title-spacing = #'((space . 5) (padding . 0.5))
-  between-title-spacing = #'((space . 1) (padding . 0.5))
-  top-system-spacing = #'((space . 1) (padding . 1) (min-distance . 0))
-  top-title-spacing = #'((space . 1) (padding . 1) (min-distance . 0))
-  bottom-system-spacing = #'((space . 1) (padding . 1) (min-distance . 0) 
(stretchability . 5))
-
-  ragged-bottom = ##f
-
-  %%
-  %% looks best for shorter scores.
-  %%
-  ragged-last-bottom = ##t
-
-  %%
-  %% settings for the page breaker
-  %%
-  blank-last-page-force = 0
-  blank-after-score-page-force = 2
-  blank-page-force = 5
-
-  %%
-  %% To limit space between systems on a page with a lot of space left
-  %%
-  page-limit-inter-system-space = ##f
-  page-limit-inter-system-space-factor = 1.4
-
-  #(define font-defaults
-     '((font-encoding . fetaMusic)))
-
-  %%
-  %% the font encoding `latin1' is a dummy value for Pango fonts
-  %%
-  #(define text-font-defaults
-     `((font-encoding . latin1)
-       (baseline-skip . 3)
-       (word-space . 0.6)))
-
-  #(define page-breaking ly:optimal-breaking)
-
-  #(define make-header (marked-up-headfoot 'oddHeaderMarkup 'evenHeaderMarkup))
-  #(define make-footer (marked-up-headfoot 'oddFooterMarkup 'evenFooterMarkup))
-  #(set-paper-dimension-variables (current-module))
-
-  \include "titling-init.ly"
-
-  top-margin = 5 \mm
-  bottom-margin = 6 \mm
-  head-separation = 4 \mm
-  foot-separation = 4 \mm
-
-  first-page-number = #1
-  print-first-page-number =##f
-}
+
+    %%% WARNING
+    %%%
+    %%% If you add any new dimensions, don't forget to update
+    %%% the dimension-variables variable.  See paper.scm.
+    
+    unit = #(ly:unit)
+    mm = 1.0
+    in = 25.4
+    pt = #(/ in 72.27)
+    cm = #(* 10 mm)
+
+    print-page-number = ##t
+
+    %%
+    %% 20pt staff, 5 pt = 1.75 mm
+    %%
+
+    output-scale = #1.7573
+    
+    #(define-public book-title (marked-up-title 'bookTitleMarkup))
+    #(define-public score-title (marked-up-title 'scoreTitleMarkup))
+    
+    %%
+    %% ugh. hard coded?
+    %%
+
+    #(layout-set-absolute-staff-size (* 20.0 pt))
+
+
+    #(define-public score-title-properties
+      '((is-title . #t)
+       (is-book-title . #f)
+       ))
+    #(define-public book-title-properties
+      '((is-title . #t)
+       (is-book-title . #t)
+       ))
+    
+    %% Note: these are not scaled; they are in staff-spaces.
+    between-system-spacing = #'((space . 12) (minimum-distance . 8) (padding . 
1))
+    between-scores-system-spacing = #'((space . 14) (minimum-distance . 8) 
(padding . 1))
+    after-title-spacing = #'((space . 2) (padding . 0.5))
+    before-title-spacing = #'((space . 5) (padding . 0.5))
+    between-title-spacing = #'((space . 1) (padding . 0.5))
+    top-system-spacing = #'((space . 1) (padding . 1) (min-distance . 0))
+    top-title-spacing = #'((space . 1) (padding . 1) (min-distance . 0))
+    bottom-system-spacing = #'((space . 1) (padding . 1) (min-distance . 0) 
(stretchability . 5))
+
+    ragged-bottom = ##f
+
+    %%
+    %% looks best for shorter scores.
+    %%
+    ragged-last-bottom= ##t
+
+    %%
+    %% settings for the page breaker
+    %%
+    blank-last-page-force = 0
+    blank-after-score-page-force = 2
+    blank-page-force = 5
+
+    %%
+    %% To limit space between systems on a page with a lot of space left
+    %%
+    page-limit-inter-system-space = ##f
+    page-limit-inter-system-space-factor = 1.4
+
+    #(define font-defaults
+      '((font-encoding . fetaMusic)))
+
+    %%
+    %% the font encoding `latin1' is a dummy value for Pango fonts
+    %%
+    #(define text-font-defaults
+      `((font-encoding . latin1)
+       (baseline-skip . 3)
+       (word-space . 0.6)))
+
+    #(define page-breaking ly:optimal-breaking)
+
+    #(define write-page-layout (ly:get-option 'dump-tweaks))
+    #(define system-maximum-stretch-procedure
+       (lambda (line)
+        (if (stretchable-line? line)
+            (let ((height (line-height line)))
+              (/ (* height height) 80.0))
+            0.0)))
+
+%    #(define page-music-height default-page-music-height )
+%    #(define page-make-stencil default-page-make-stencil )
+
+    #(define make-header (marked-up-headfoot 'oddHeaderMarkup 
'evenHeaderMarkup))
+    #(define make-footer (marked-up-headfoot 'oddFooterMarkup 
'evenFooterMarkup))
+    #(set-paper-dimension-variables (current-module))
+
+    \include "titling-init.ly"
+
+    check-consistency = ##t
+
+    top-margin = 5 \mm
+    bottom-margin = 6 \mm
+
+    left-margin-default = 10 \mm
+    right-margin-default = 10 \mm   
+
+    head-separation = 4 \mm
+    foot-separation = 4 \mm
+
+    first-page-number = #1
+    print-first-page-number =##f
+  }
diff --git a/scm/page.scm b/scm/page.scm
index ca4e0d8..959402c 100644
--- a/scm/page.scm
+++ b/scm/page.scm
@@ -184,11 +184,7 @@
   (let*
       ((paper-height (ly:output-def-lookup layout 'paper-height))
        (paper-width (ly:output-def-lookup layout 'paper-width))
-       (lmargin (ly:output-def-lookup layout 'left-margin #f))
-       (left-margin (if lmargin
-                      lmargin
-                      (/ (- paper-width
-                            (ly:output-def-lookup layout 'line-width)) 2)))
+       (left-margin (ly:output-def-lookup layout 'left-margin))
        (bottom-edge (- paper-height
                       (ly:output-def-lookup layout 'bottom-margin)) )
        (top-margin (ly:output-def-lookup layout 'top-margin))
diff --git a/scm/paper.scm b/scm/paper.scm
index 48f4a46..ab26e44 100644
--- a/scm/paper.scm
+++ b/scm/paper.scm
@@ -16,13 +16,16 @@
                    indent
                    ledger-line-thickness
                    left-margin
+                    left-margin-default
                    line-thickness
                    line-width
+                    line-width-cropping
                    mm
                    paper-height
                    paper-width
                    pt
                    right-margin
+                    right-margin-default
                    short-indent
                    staff-height
                    staff-space
@@ -207,23 +210,16 @@ size. SZ is in points"
     ("f4" . (cons (* 210 mm) (* 330 mm)))
    ))
 
-;; todo: take dimension arguments.
+; todo: take dimension arguments.
 
 (define (set-paper-dimensions m w h)
   "M is a module (i.e. layout->scope_ )"
-  (let* ((mm (eval 'mm m)))
+  (begin
+    ;; page layout - what to do with (printer specific!) margin settings?
     (module-define! m 'paper-width w)
     (module-define! m 'paper-height h)
-    (module-define! m 'line-width (- w
-                                    (ly:modules-lookup (list m) 'left-margin 
(* 10 mm))
-                                    (ly:modules-lookup (list m) 'right-margin 
(* 10 mm))))
-
     (module-define! m 'indent (/ w 14))
-    (module-define! m 'short-indent 0)
-
-    ;; page layout - what to do with (printer specific!) margin settings?
-
-    ))
+    (module-define! m 'short-indent 0)))
 
 (define (internal-set-paper-size module name landscape?)
   (define (swap x)
diff --git a/scripts/lilypond-book.py b/scripts/lilypond-book.py
index dc035b4..0308ada 100644
--- a/scripts/lilypond-book.py
+++ b/scripts/lilypond-book.py
@@ -830,7 +830,7 @@ PREAMBLE_LY = '''%%%% Generated by %(program_name)s
 \paper {
   %(paper_string)s
   force-assignment = #""
-  line-width = #(- line-width (* mm  %(padding_mm)f))
+  line-width-cropping = %(padding_mm)f \mm
 }
 
 \layout {
-- 
1.6.0.2