Re: Strange loop executed only once!

[Łukasz Czerwiński]

On 22 April 2012 13:28, Han-Wen Nienhuys <address@hidden> wrote:

On Sun, Apr 22, 2012 at 7:20 AM, Łukasz Czerwiński <address@hidden> wrote:
> Hi,
>
> I have spotted a strange loop in lily/ledger-line-spanner.cc (lines 46-69):
>
> Direction d = UP;
>   do
>     {
>       .............
>     }
>   while (flip (&d) != DOWN);
>
> It will be executed only once - with d set to UP. To be executed for d = UP
> and then d = DOWN, it should be:
> while (flip (&d) != UP);
>
> Could someone take a look at this?

Looks like a bug.  Maybe you could work on a test/repro case? I think
it will trigger if you have noteheads with ledgers very closely spaced
under the staff (the code is to enforce a minimum distance so the
ledger lines remain visible).

this also looks a little suspect:

         Real total_head_length = previous_extents[d].length ()
                                  + current_extents[d].length ();

I would expect that it should add

 current[d][RIGHT] - previous[d][LEFT]

perhaps this will set rods too large if you have lots of ledgered
chords with seconds in them, in a tight spacing configuration.

--
Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen

Keith answered at: http://codereview.appspot.com/6109046/diff2/1:6001/lily/ledger-line-spanner.cc :

There is exactly the problem that HanWen predicted in the
email chain. Chords in seconds get a bit too much space, if they have ledgers.
{ \time 4/1
\override Score.SpacingSpanner #'packed-spacing = ##t
c'1 a <b'' c'''> <g''' a'''> }
The cause seems to be the decision to scale the ledger from the notecolumn
width, so it seems to fix it would require a bit of a redesign.

My answer:

Was that bug already filed or should I do it? 

Anyway, I don't understand:

"The cause seems to be the decision to scale the ledger from the notecolumn

width, so it seems to fix it would require a bit of a redesign.". 

Does it refer to using total_head_length in calculating rod.distance_?

And where to find what's min_length_fraction for? Googling that didn't help... 

Thanks for any tips,

Łukasz