Re: Issue 1268 in lilypond: [PATCH] span-bar problem

[Benkő Pál]

hi Joe,

>> > need to be careful about what happens when bar-size is set.
>> > Currently, your patch will break (for example) input/regression/drums.ly
>> > because it ignores bar-size.
>>
>> well, I think extent shouldn't be computed from size
>> as it loses information, but the other way round.
>
> That's true, but we currently allow users to override bar-size (for example
> in percussion parts). With your current patch, if the user overrides
> bar-size, then Bar_line::calc_bar_size and Bar_line::calc_bar_extent will
> give conflicting results (I was wrong earlier when I said that this would
> affect the printed bar line, but I still think it's bad for the functions to
> conflict).

certainly.  btw, I think the span bar code looks like this
to allow putting percussion staves into StaffGroup's.

>> AFAICS the intent of the span bar code is to draw lines
>> not between staves but from line to line, and my patch
>> reverts this.
>> of course there's no difference in general, but there is
>> if bar lines are to have different extent than that of
>> their staff; and there is a further difference whether
>> the bar is shorter or longer than the staff: if longer,
>> there may be no point in using span bars at all; if
>> shorter, then perhaps span bars are better placed just
>> between staves (see spantest5.ly).
>
> I don't think the current intention is documented anywhere. If you want to
> fix a policy now, that's ok with me.
>
>> I still don't know exactly how should spantest3 behave
>> (spantest5 makes me feel it works acceptably), neither
>> what should happen if different bar types are connected
>> (see spantest4.ly - I hope this is farthest from a real
>> world case of all my tests).
>>
>> >>> I removed the center setting code and that
>> >>> (with my patch still active) made my example perfect;
>> >>> however, the attached complementary test (with bar
>> >>> lines only within staff, not between them) failed,
>> >>> but it's perfect with current center setting
>> >>> (independently whether my original patch is active or not).
>>
>> >> Since Bar_line::compound_barline is used in both BarLine and SpanBar,
>> >> you
>> >> will need to find some solution that works for both cases. It won't be
>> >> as
>> >> simple as just enabling or disabling the centering code.
>>
>> I moved the centering code from compound_barline to print,
>> and this way all regtests are OK and all my spantests are
>> good or at least acceptable.
>
> Thanks, this patch looks good. Just a couple of things I'd like to see:
>
> - a comment explaining the positioning of span-bars (ie. between bar-lines
> or between staves?)

I'm thinking about a bit more: take the union of bar-extent and staff extent,
and draw the span bar outside those.  Then mensurstrich layout can be achieved
by setting bar-size to 0 (or rather, setting bar-extent to '( 0 . 0 )), which
is perhaps less hackish than the current method of hiding the bar.

> - agreement between bar-size and bar-extent. There are two solutions I can
> see, but feel free to invent your own:
>    - deprecate the bar-size property and use bar-extent from now on. This
> will require a convert-ly rule.

I think this is a must, to avoid setting incompatible size and extent.

>    - unset bar-size by default (in scm/define-grob-properties.scm). In
> calc_bar_extent, check to see if bar-size is set. If it is, use it.
> Otherwise, use the staff extent
>
> Here are a couple other suggestions that I think would improve the code. I'd
> be happy to commit your patch without them because they reflect problems
> with the existing code rather than problems with your patch. But if you have
> time, it would be nice :)
> - Shouldn't Bar_line::print use bar-extent rather than bar-size? That seems
> more natural.

me too, and getting rid of bar-extent makes this a must, too.

> - The centering issue regarding compound_barline still seems strange.
> Wouldn't it be nicer if compound_barline took Real bottom and Real top (or
> maybe Interval) parameters (instead of Real h) and drew a bar line whose
> Y-extent stretched from bottom to top? That seems like a more intuitive API
> to me (and unlike the current one, it's self-documenting).

Good idea!  I have to figure out the centering issues and how to convert
SCM to Interval, but this is a nice way.

I hope I can find some time to do it; thanks for the hints,
p