On Jul 13, 2011, at 10:48 PM, Han-Wen Nienhuys wrote: > On Wed, Jul 13, 2011 at 5:12 PM, m...@apollinemike.com > <m...@apollinemike.com> wrote: >> On Jul 13, 2011, at 4:42 PM, Han-Wen Nienhuys wrote: >> >>> On Wed, Jul 13, 2011 at 11:27 AM, m...@apollinemike.com >>> <m...@apollinemike.com> 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 _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel