Re: Fix 1563: System start bars interpreted collapse-height as absolute length. (issue4693043)

[reinhold . kainhofer]

Reviewers: J_lowe, Keith, Neil Puttock,

Message:
On 2011/07/18 20:24:43, Neil Puttock wrote:
> lily/system-start-delimiter.cc:117: staffspace =
> Staff_symbol_referencer::staff_space (sp);
> A mostly theoretical gripe, I guess: what about collapse-height
comparisons
> where multiple staves are involved?  If staff-space settings differ,
this only
> picks up the last stave's value.

Yes, that was a deliberate decision. After all, the collapse-height is
mainly intended to hide/show the bracket for single-staff systems.
In my eyes it would make more sense to show/hide the bracked depending
on the number or staves involved rather than the grob height.
I can't imagine a situation, where using the last staff's staff-space
would lead to unwanted output. Usually, collapse-height will be either
5.0 (the default) or something smaller as advocated in the documentation
and the LSR. In these cases, the last staff alone (plus some spacing to
the staff above for 5.0) already suffices to show the bracket, even if
the other staves have a different staff-space.


Description:
Fix 1563: System start bars interpreted collapse-height as absolute
length.

If you increased the staff-space, this meant sometimes the
collapse-height
would not be enough to hide the start-bar for a staff, while in other
cases
it was enough...

This patch interprets the collapse-height in multiples of the
staff-space.

However, I think that the notion of a collapse-height (as a length) for
hiding/showing the system start delimiter is not the best approach in
general.

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

Affected files:
  A input/regression/system-start-bar-collapse-staffspace.ly
  M lily/system-start-delimiter.cc


Index: input/regression/system-start-bar-collapse-staffspace.ly
diff --git a/input/regression/system-start-bar-collapse-staffspace.ly  
b/input/regression/system-start-bar-collapse-staffspace.ly
new file mode 100644
index  
0000000000000000000000000000000000000000..a7d6a0a976fc76b43adb09e8f68ed77f4798bd24
--- /dev/null
+++ b/input/regression/system-start-bar-collapse-staffspace.ly
@@ -0,0 +1,12 @@
+\version "2.15.5"
+
+\header {
+  texidoc = "When the staff-space is increased, the system-start delimiter
+should still be collapsed (i.e. the collapse-height should not give an  
absolute
+length, but a multiple of staff-spaces)."
+}
+
+\new Staff \with { \override StaffSymbol #'staff-space = #1.4 }
+{
+  a4 b c d
+}
Index: lily/system-start-delimiter.cc
diff --git a/lily/system-start-delimiter.cc b/lily/system-start-delimiter.cc
index  
3f671e3316135f60c593753eab550e545cb1395f..25da0494395a5c158bab33a1f5860e51c6bb5c15  
100644
--- a/lily/system-start-delimiter.cc
+++ b/lily/system-start-delimiter.cc
@@ -100,6 +100,7 @@ System_start_delimiter::print (SCM smob)
   Grob *common = common_refpoint_of_array (elts, me, Y_AXIS);

   Interval ext;
+  Real staffspace = 1.0;
   int non_empty_count = 0;
   for (vsize i = elts.size (); i--;)
     {
@@ -113,14 +114,17 @@ System_start_delimiter::print (SCM smob)
            {
              non_empty_count ++;
              ext.unite (dims);
+              staffspace = Staff_symbol_referencer::staff_space (sp);
            }
        }
     }

   SCM glyph_sym = me->get_property ("style");
   Real len = ext.length ();
+
+  // Use collapse-height in multiples of the staff-space
   if (ext.is_empty ()
-      || (robust_scm2double (me->get_property ("collapse-height"), 0.0) >=  
ext.length ()))
+      || (robust_scm2double (me->get_property ("collapse-height"), 0.0) >=  
(len / staffspace)))
     {
       me->suicide ();
       return SCM_UNSPECIFIED;