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

[Han-Wen Nienhuys]

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.


> Lemme know if you see in my explanation anything that seems frivolous/too 
> much, and thanks for all of your feedback!
>
> Cheers,
> MS



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