Re: Allow music of nominally zero duration to be typeset. (issue 6810087)

[dak]

Reviewers: benko.pal,

Message:
On 2012/11/16 22:13:59, benko.pal wrote:
> LGTM in the sense that it won't make things worse; I've tried to
understand the
> code but failed, see below.

I have not actually tried to understand the code.  I just added checks
for existing array elements before access until I could no longer make
LilyPond segfault or produce programming errors.

So this is, indeed, strictly a patch on the "won't make things worse"
basis, except for the one, initial removal of a zero-duration check,
necessary for sane behavior in things like incipits without notes.  All
other changes are of the "not worse than before" kind.  I can't
absolutely guarantee that no other changes will be necessary under some
conditions, but we'll probably only hear about them when this gets
exposure in an unstable release.

Description:
Allow music of nominally zero duration to be typeset.

This is important for things like incipits without notes or other
material that produces output without spending time.  In particular,
this is interesting for \score in markup which more often than not
only contains few notational elements and where attempts to just get
basic key/clef/timesignature drawings are sometimes failing in
frustrating manners.

The downside are problems with material that indeed produces no
output.  The basic change is simple and just in
lily/global-context-scheme.cc.  All the rest is attempting to remove
programming errors and warnings occuring for basically empty systems.

The current state is that

\new Staff { }

will segfault, so indeed there is still code that does not expect
actually empty material.

Somebody willing to work somewhat more on this?

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

Affected files:
  M lily/constrained-breaking.cc
  M lily/global-context-scheme.cc
  M lily/optimal-page-breaking.cc
  M lily/page-spacing.cc
  M lily/simple-spacer.cc


Index: lily/constrained-breaking.cc
diff --git a/lily/constrained-breaking.cc b/lily/constrained-breaking.cc
index  
c8b7fc1d748bc06ae2074d33d436d77ab1bdd2e5..ee8b28906c5f6d476e82b49cb3afeba92974b833  
100644
--- a/lily/constrained-breaking.cc
+++ b/lily/constrained-breaking.cc
@@ -202,8 +202,10 @@ Constrained_breaking::solve (vsize start, vsize end,  
vsize sys_count)
         }
     }
   /* if we get to here, just put everything on one line */
-  warning (_ ("cannot find line breaking that satisfies constraints"));
-  ret.push_back (space_line (0, end_brk));
+  if (sys_count > 0) {
+    warning (_ ("cannot find line breaking that satisfies constraints"));
+    ret.push_back (space_line (0, end_brk));
+  }
   return ret;
 }

@@ -291,9 +293,11 @@ Constrained_breaking::line_details (vsize start, vsize  
end, vsize sys_count)
     }

   /* if we get to here, just put everything on one line */
-  Line_details details;
-  fill_line_details (&details, 0, end_brk);
-  ret.push_back (details);
+  if (sys_count > 0) {
+    Line_details details;
+    fill_line_details (&details, 0, end_brk);
+    ret.push_back (details);
+  }
   return ret;
 }

Index: lily/global-context-scheme.cc
diff --git a/lily/global-context-scheme.cc b/lily/global-context-scheme.cc
index  
1267793cf3564b3e3198aeffe3b235a7ef3e2528..86ee2b05b963132936020f212fa5ef09a613b5c6  
100644
--- a/lily/global-context-scheme.cc
+++ b/lily/global-context-scheme.cc
@@ -90,8 +90,7 @@ LY_DEFINE  
(ly_interpret_music_expression, "ly:interpret-music-expression",
   LY_ASSERT_TYPE (unsmob_global_context, ctx, 2);

   Music *music = unsmob_music (mus);
-  if (!music
-      || !music->get_length ().to_bool ())
+  if (!music)
     {
       warning (_ ("no music found in score"));
       return SCM_BOOL_F;
Index: lily/optimal-page-breaking.cc
diff --git a/lily/optimal-page-breaking.cc b/lily/optimal-page-breaking.cc
index  
2b0910f8873600feebf136424c8ffa823d0f93d6..3bddcad4ba66815806fa1e691a2c2f229e1fc8f9  
100644
--- a/lily/optimal-page-breaking.cc
+++ b/lily/optimal-page-breaking.cc
@@ -64,12 +64,19 @@ Optimal_page_breaking::solve ()
       best = space_systems_on_best_pages (0, first_page_num);

       page_count = best.systems_per_page_.size ();
-      min_sys_count = ideal_sys_count - best.systems_per_page_.back ();
+      if (page_count == 0)
+        {
+          min_sys_count = 0;
+        }
+      else
+        {
+          min_sys_count = ideal_sys_count - best.systems_per_page_.back ();

-      if (page_count > 1 && best.systems_per_page_[page_count - 2] > 1)
-        min_sys_count -= best.systems_per_page_[page_count - 2];
+          if (page_count > 1 && best.systems_per_page_[page_count - 2] > 1)
+            min_sys_count -= best.systems_per_page_[page_count - 2];

-      min_sys_count = max (min_sys_count, (vsize)1);
+          min_sys_count = max (min_sys_count, (vsize)1);
+        }
     }
   else
     {
@@ -103,7 +110,7 @@ Optimal_page_breaking::solve ()

   if (page_count == 1)
     message (_ ("Fitting music on 1 page..."));
-  else if (scm_is_integer (forced_page_count))
+  else if (scm_is_integer (forced_page_count) || page_count == 0)
     message (_f ("Fitting music on %d pages...", (int)page_count));
   else
     message (_f ("Fitting music on %d or %d pages...", (int)page_count -  
1, (int)page_count));
Index: lily/page-spacing.cc
diff --git a/lily/page-spacing.cc b/lily/page-spacing.cc
index  
9434fb910aab7705be46630374f25a5bc4e51cd6..c2988d8502e37785a54ebf658932710aa310c6bb  
100644
--- a/lily/page-spacing.cc
+++ b/lily/page-spacing.cc
@@ -155,6 +155,9 @@ Page_spacer::solve ()
     }

   Page_spacing_result ret;
+  if (simple_state_.empty ())
+    return ret;
+
   ret.penalty_ = simple_state_.back ().penalty_
                  + lines_.back ().page_penalty_ + lines_.back  
().turn_penalty_;
   ret.system_count_status_ = simple_state_.back ().system_count_status_;
Index: lily/simple-spacer.cc
diff --git a/lily/simple-spacer.cc b/lily/simple-spacer.cc
index  
88ebae09d85c4714e318013bbf5d6a5c8541a452..61afdb9bcc281004c4b0dbffbabe2fbd757ec4d5  
100644
--- a/lily/simple-spacer.cc
+++ b/lily/simple-spacer.cc
@@ -370,9 +370,12 @@ get_column_description (vector<Grob *> const &cols,  
vsize col_index, bool line_s
   if (next_col)
     description.spring_ = Spaceable_grob::get_spring (col, next_col);

-  Grob *end_col = dynamic_cast<Item *> (cols[col_index +  
1])->find_prebroken_piece (LEFT);
-  if (end_col)
-    description.end_spring_ = Spaceable_grob::get_spring (col, end_col);
+  if (col_index + 1 < cols.size ())
+    {
+      Grob *end_col = dynamic_cast<Item *> (cols[col_index +  
1])->find_prebroken_piece (LEFT);
+      if (end_col)
+        description.end_spring_ = Spaceable_grob::get_spring (col,  
end_col);
+    }

   for (SCM s = Spaceable_grob::get_minimum_distances (col);
        scm_is_pair (s); s = scm_cdr (s))