Re: Allows for rider grobs in outside-staff-priority. (issue4639075)

[address@hidden]

On Jul 13, 2011, at 10:48 PM, Han-Wen Nienhuys wrote:

> On Wed, Jul 13, 2011 at 5:12 PM, address@hidden
> <address@hidden> wrote:
>> On Jul 13, 2011, at 4:42 PM, Han-Wen Nienhuys wrote:
>> 
>>> On Wed, Jul 13, 2011 at 11:27 AM, address@hidden
>>> <address@hidden> wrote:
>>> 
>>>>> Wouldnt it be much easier for the tuplet number's extents  to be
>>>>> ignored for skyline purposes if there already is an associated tuplet
>>>>> bracket?
>>>>> 
>>>> 
>>>> That's what the new line 250 of axis-group-interface.cc in the patch does: 
>>>> it makes sure that the rider grob is not counted in the skyline (I think).
>>>> The tuplet number sticks out of the tuplet bracket, so it makes sense for 
>>>> its extent to be combined with that of its carrier (lines 244 and 255 of 
>>>> axis-group-interface.cc in the patch).
>>>> 
>>>>> Practically speaking, the tupletnumber adapts its position to the
>>>>> bracket, so it is best for the implementation to also do this.
>>>>> 
>>>> 
>>>> The idea of the rider grob in this implementation is a generic concept 
>>>> that would allow for certain grobs (i.e. TupletNumbers) to ride their 
>>>> carriers (i.e. TupletBrackets) outside of the staff and then subsequently 
>>>> be counted as part of their carrier's extent instead of as part of the 
>>>> staff extent (or as another outside-staff grob).
>>> 
>>> Can you think of a way to make the impact of this to axis-group code
>>> minimal?  I think the only thing needed is that the axis-group doesnt
>>> know about the tuplet number.  You could either not add it to the
>>> axis-group in the first place, or alternatively, you could remove the
>>> number from the 'elements list in axis group once you know it is
>>> irrelevant.
>>> 
>>> The number should be parented by the bracket so you get the
>>> translations for free.
>>> 
>> 
>> Most of the current patch does the minimal amount of work to get the result. 
>>  Note that this does not mean that this is the ideal way to do it - it is 
>> just the only way I know how given my understanding of the code.
> 
> It's not about the minimal amount of work, it's about having minimal
> dependencies between parts; it were much better if this feature did
> not have to modify the axis-group code at all.  Can you try to do it
> without having axis-group know about a 'rider/carrier' concept?
> 
>> Added line 199: identifies if a grob should be ignored (is it an 
>> outside-staff-rider?)
> 
> if you don't add the grob to the list to begin with, you don't have need this.
> 
> 
>> Added line 240-250 and 255-264: Admittedly code-dupious, so these really do 
>> the same thing.  I am trying to follow the 
>> patches-as-tiny-and-self-contained-as-possible policy as strictly as 
>> possible, especially as I am a cardinal offender in other patches under 
>> review.  These lines do two things: (1) Add the extent of the tuplet number 
>> to the tuplet bracket (which I do here instead of in the Y-extent override 
>> because it seems that it should be temporary); and (2) ignores all rider 
>> grobs that have already been added into their carriers' extent.
> 
> I would leave handling the vertical extent of the number for a next change.
> 
>> Added lines 653-655, 674-675: Move the rider relative to its carrier.  This 
>> is the spot where I may be able to get some of the code outside of 
>> axis-group-interface.cc
> 
> +               for (vsize j = 0; j < riders.size (); j++)
> +                 riders[j]->translate_axis (dir*dist, Y_AXIS);
> 
> if the rider has the carrier as it's Y parent, moving the carrier
> already accomplishes this.
> 

I think the newest patch set should do the trick - you're right about not 
adding the grob to the list to begin with, and I went down this route instead.

Cheers,
MS