Issue 3208: compressed MMRs choose correct glyphs (issue 335410043 by address@hidden)

[lilypond]

Reviewers: ,


https://codereview.appspot.com/335410043/diff/1/lily/multi-measure-rest.cc
File lily/multi-measure-rest.cc (left):

https://codereview.appspot.com/335410043/diff/1/lily/multi-measure-rest.cc#oldcode291
lily/multi-measure-rest.cc:291: // calculated from there.
This worked for maxima/longa/brevis/semibrevis rests. But half and
shorter rests cannot just hang from the staff line.

Have a look at the output of the following code:

\version "2.19.80"
\new TabStaff {
  \tabFullNotation
  \compressFullBarRests
  R\breve
  \time 4/2
  R\breve r\breve
}

It’s not consistent: in 4/4 the breve rests hangs, in 4/2 both r and R
stand. This needs a fix in rest.cc (or in the font?) so I thought it
wouldn’t be too bad to “break” it by making it more consistent while
making shorter rests work at the same time. Of course, that has to be
fixed before this patch can be applied.

I tried to separate the selection of rest glyphs and their positioning
but I didn’t succeed :/

Description:
Issue 3208: compressed MMRs choose correct glyphs

usable-duration-logs now affects only the choice of glyph for the
representation of a single measure. For multiple measures, glyphs for
longer rests up to maxima are used.

This is a second attempt to make it work and has little to do with my
first attempt at https://codereview.appspot.com/333340043 Therefore I
created a new Rietveld issue.

Also the vertical positioning of half and shorter rest glyphs in MMRs
now is handled correctly. BUT: this breaks make check because of a
different positioning of breve rests in TabStaffs. For details see
discussion below.

Please review this at https://codereview.appspot.com/335410043/

Affected files (+30, -58 lines):
  M lily/include/multi-measure-rest.hh
  M lily/multi-measure-rest.cc


Index: lily/include/multi-measure-rest.hh
diff --git a/lily/include/multi-measure-rest.hh  
b/lily/include/multi-measure-rest.hh
index  
f4d379ade4a639c3a6ac39c6bebee33507434874..aebb02d1aceb004fccb614a7b31641e0f8a87b88  
100644
--- a/lily/include/multi-measure-rest.hh
+++ b/lily/include/multi-measure-rest.hh
@@ -36,7 +36,7 @@ public:
   static void calculate_spacing_rods (Grob *me, Real length);
   static Stencil big_rest (Grob *, Real);
   static Stencil symbol_stencil (Grob *, Real);
-  static Stencil church_rest (Grob *, Font_metric *, int, Real);
+  static Stencil church_rest (Grob *, Font_metric *, int, int, Real);
   static Interval bar_width (Spanner *me);
 };

Index: lily/multi-measure-rest.cc
diff --git a/lily/multi-measure-rest.cc b/lily/multi-measure-rest.cc
index  
38dd598ee95d329be323342eeddb4da4817b946b..b1737441dde381368ae6dc6348d5a32e90853d73  
100644
--- a/lily/multi-measure-rest.cc
+++ b/lily/multi-measure-rest.cc
@@ -131,9 +131,16 @@ Multi_measure_rest::height (SCM smob)
 }

 int
-calc_closest_duration_log (Grob *me, double duration, bool force_round_up)
+calc_measure_duration_log (Grob *me)
 {
-  bool round_up = force_round_up
+  SCM sml = dynamic_cast<Spanner *> (me)->get_bound (LEFT)
+            ->get_property ("measure-length");
+  Rational ml = (unsmob<Moment> (sml)) ? unsmob<Moment> (sml)->main_part_
+                : Rational (1);
+  double duration = ml.Rational::to_double ();
+  bool round_up = to_boolean (scm_list_p (scm_member (scm_cons  
(scm_from_int64 (ml.numerator ()),
+                                                                 
scm_from_int64 (ml.denominator ())),
+                                                      me->get_property  
("round-up-exceptions"))))
                   || to_boolean (me->get_property  
("round-up-to-longer-rest"));
   int closest_usable_duration_log;

@@ -184,20 +191,6 @@ calc_closest_duration_log (Grob *me, double duration,  
bool force_round_up)
   return closest_usable_duration_log;
 }

-int
-calc_measure_duration_log (Grob *me)
-{
-  SCM sml = dynamic_cast<Spanner *> (me)->get_bound (LEFT)
-            ->get_property ("measure-length");
-  Rational ml = (unsmob<Moment> (sml)) ? unsmob<Moment> (sml)->main_part_
-                : Rational (1);
-  double measure_duration = ml.Rational::to_double ();
-  bool force_round_up = to_boolean (scm_list_p (scm_member (scm_cons  
(scm_from_int64 (ml.numerator ()),
-                                                            scm_from_int64  
(ml.denominator ())),
-                                                             
me->get_property ("round-up-exceptions"))));
-  return calc_closest_duration_log (me, measure_duration, force_round_up);
-}
-
 Stencil
 Multi_measure_rest::symbol_stencil (Grob *me, Real space)
 {
@@ -216,26 +209,12 @@ Multi_measure_rest::symbol_stencil (Grob *me, Real  
space)
       s.translate_axis (padding * space, X_AXIS);
       return s;
     }
-
-  Font_metric *musfont = Font_interface::get_default_font (me);
-  int mdl = calc_measure_duration_log (me);
-
-  if (measure_count == 1)
+  else
     {
-      if (scm_is_null (me->get_property ("staff-position")))
-        {
-          int dir = get_grob_direction (me);
-          Real pos = Rest::staff_position_internal (me, mdl, dir);
-          me->set_property ("staff-position", scm_from_double (pos));
-        }
-
-      Stencil s = musfont->find_by_name (Rest::glyph_name (me, mdl, "",  
true, 0.0));
-
-      s.translate_axis ((space - s.extent (X_AXIS).length ()) / 2, X_AXIS);
-      return s;
+      Font_metric *musfont = Font_interface::get_default_font (me);
+      int mdl = calc_measure_duration_log (me);
+      return church_rest (me, musfont, measure_count, mdl, space);
     }
-  else
-    return church_rest (me, musfont, measure_count, space);
 }

 /*
@@ -271,47 +250,40 @@ Multi_measure_rest::big_rest (Grob *me, Real width)
 */
 Stencil
 Multi_measure_rest::church_rest (Grob *me, Font_metric *musfont, int  
measure_count,
-                                 Real space)
+                                 int mdl, Real space)
 {
+  double displayed_duration = measure_count * pow (2.0, -mdl);
   SCM mols = SCM_EOL;
   int symbol_count = 0;
   Real symbols_width = 0.0;
-  double total_duration = measure_count * pow (2.0,  
-calc_measure_duration_log (me));
-
-  SCM staff_position = me->get_property ("staff-position");

-  if (!scm_is_number (staff_position))
+  if (scm_is_null (me->get_property ("staff-position")))
     {
-      // Staff position is somewhat icky regarding its definition for
-      // compatibility reasons.  It is intended to be the baseline of
-      // a breve rest.  However, when the staff space is more than
-      // single space (like with tablature), it looks better if all
-      // rests are actually hanging.  So staff position, in reality,
-      // is the semi-breve position - 2.  Everything else is
-      // calculated from there.
       int dir = get_grob_direction (me);
-      Real pos = Rest::staff_position_internal (me, 0, dir);
-      me->set_property ("staff-position", scm_from_double (pos - 2));
+      Real pos = Rest::staff_position_internal (me, mdl, dir);
+      me->set_property ("staff-position", scm_from_double ((mdl == 0) ?  
(pos - 2) : pos));
     }

-  while (total_duration > 0)
+  int dl = -3;
+  while (displayed_duration > 0)
     {
-      int dl = calc_closest_duration_log (me, total_duration, false);
       double duration = pow (2.0, -dl);

-      total_duration -= duration;
+      if (displayed_duration < duration)
+        {
+          dl++;
+          continue;
+        }

-      Stencil r = musfont->find_by_name (Rest::glyph_name (me, dl, "",  
true, 2));
+      displayed_duration -= duration;

-      Real staff_space = Staff_symbol_referencer::staff_space (me);
+      Stencil r = musfont->find_by_name (Rest::glyph_name (me, dl, "",  
true, (dl == 0) ? 2 : 0));
       if (dl == 0)
         {
+          Real staff_space = Staff_symbol_referencer::staff_space (me);
           r.translate_axis (staff_space, Y_AXIS);
         }
-      else
-        {
-          r.translate_axis (staff_space-r.extent (Y_AXIS).at (UP), Y_AXIS);
-        }
+
       symbols_width += r.extent (X_AXIS).length ();
       mols = scm_cons (r.smobbed_copy (), mols);
       symbol_count++;