Re: -dpreview crops staff bracket

[David Kastrup]

"Phil Holmes" <address@hidden> writes:

> "Phil Holmes" <address@hidden> wrote in message
> news:address@hidden
>> "David Kastrup" <address@hidden> wrote in message
>> news:address@hidden
>>> Urs Liska <address@hidden> writes:
>>>
>>>> I don't know if it's a regression because I don't know if it
>>>> _deliberately_ worked in an earlier version.
>>>>
>>>> But I just checked all versions I still have around and can say that
>>>> 2.15.40
>>>> 2.16.0 and
>>>> 2.17.3
>>>> compiled the example correctly
>>>>
>>>> while 2.17.16 crops the bracket as shown in the attachment to my initial
>>>> email.
>>>
>>> My current /usr/local/lilypond/binary (I only do make install once in a
>>> while) still works and claims to be 2.17.12.  Since that version number
>>> is output for everything after 2.17.11, this means that the problem has
>>> been introduced in version 2.17.12 or later.
>>>
>>> Someone up for bisection?
>>
>>
>> Watch this space.
>
> git bisect bad
> 7b2cb93fc69c7d7c45f0ae6495f688752efeb107 is the first bad commit
> commit 7b2cb93fc69c7d7c45f0ae6495f688752efeb107
> Author: Mike Solomon <address@hidden>
> Date:   Sat Mar 23 19:09:28 2013 +0100
>
>    Fixes manual beaming over rests and vertical spacing problem (issue
> 3242)

Wow.

commit 7b2cb93fc69c7d7c45f0ae6495f688752efeb107
Author: Mike Solomon <address@hidden>
Date:   Sat Mar 23 19:09:28 2013 +0100

    Fixes manual beaming over rests and vertical spacing problem (issue 3242)

diff --git a/lily/side-position-interface.cc b/lily/side-position-interface.cc
index 095b8b4..4d8efe1 100644
--- a/lily/side-position-interface.cc
+++ b/lily/side-position-interface.cc
@@ -324,13 +324,6 @@ Side_position_interface::aligned_side (Grob *me, Axis a, bo
       dim.set_minimum_height (0.0);
     }
 
-  // Ditto - seems kludgy, but this time logic of SystemStartBrackets
-  if (my_dim.is_empty ())
-    {
-      my_dim = Skyline (my_dim.direction ());
-      my_dim.set_minimum_height (isinf (max_raise) ? 0.0 : max_raise);
-    }
-
   // Many cross-staff grobs do not have good height estimations.
   // We give the grob the best chance of not colliding by shifting
   // it to the maximum height in the case of cross-staff alignment.
diff --git a/lily/stencil-integral.cc b/lily/stencil-integral.cc
index 58b2ff4..0f1f14f 100644
--- a/lily/stencil-integral.cc
+++ b/lily/stencil-integral.cc
@@ -981,16 +981,6 @@ Grob::maybe_pure_internal_simple_skylines_from_extents (Gro
                  ? Interval (-infinity_f, infinity_f)
                  : me->maybe_pure_extent (me, Y_AXIS, pure, beg, end);
 
-  // In horizontal spacing, there are grobs like SystemStartBracket
-  // that take up no vertical spcae.  So, if the y extent is empty,
-  // we use the entire Y extent ot make the X a sort of horizontal wall.
-  // Ditto for vertical spacing and grobs like BassFigureAlginmentPositioning.
-  if (a == Y_AXIS && yex.is_empty ())
-    yex.set_full ();
-
-  if (a == X_AXIS && xex.is_empty ())
-    xex.set_full ();
-
   if (xex.is_empty () || yex.is_empty ())
[...]

The patch set removes code that does not appear to have any relation to
the purported purpose of the patch and skimming over the code review, I
can see no explanation for that removal.  The removed code carries
comments regarding what it is supposed to be for, and apparently it was
not checked thoroughly that the removal does not affect that
functionality.

-- 
David Kastrup