Re: Caches the interior skylines of vertical axis groups and systems. (issue 7185044)

[address@hidden]

On 24 févr. 2013, at 17:18, David Kastrup <address@hidden> wrote:

> "address@hidden" <address@hidden> writes:
> 
>> On 24 févr. 2013, at 16:37, address@hidden wrote:
>> 
>>> On 2013/02/24 13:27:39, mike7 wrote:
>>>> On 24 févr. 2013, at 12:40, mailto:address@hidden wrote:
>>> 
>>>>> Stupid question: could you not just check whether
>>>>> outside-staff-priority has been set, and if it has, pass the buck
>>>>> to the right kind of callback automatically?  That way, the user
>>>>> does not need to meddle with callbacks himself.
>>>>> 
>>>>> https://codereview.appspot.com/7185044/
>>> 
>>>> Yup, but it'd require creating an Outside_staff_callback engraver
>>>> with a call to acknowledge_grob.  Calls to acknowledge_grob are
>>>> expensive and should be used only if there's no other viable
>>>> solution.
>>> 
>>> Why can't the current Y-offset callback check outside-staff-priority
>>> on the grob?  Why would there be a need for an acknowledger?
>>> 
>> 
>> That's exactly how the new Y-offset callbacks are implemented in this
>> patch set.  The problem is that not all grobs have a Y-offset
>> callback.
> 
> Well, something heeded outside-staff-priority previously, apparently
> calculating an offset based on it.  What happened to it?
> 

Before, there was a call to translate_axis.  It wasn't done as a callback for a 
Y-offset property but rather a vertical-skylines property for VerticalAxisGroup 
that called translate_axis many times (follow, in current master, 
Axis_group_interface::calc_vertical_skylines to avoid_outside_staff_collisions 
and you'll see how this is done).  This makes it impossible to wipe caches and 
recompute properties with more information.  The goal of this patch is to 
eliminate that call to translate_axis so that the callback(s) for Y-offset 
calculate the entire offset, which means we can cache and recache pure 
equivalents.

I think the current patch is fine.  Any behavior that this patch changes (for 
example, setting an outside-staff-priority on MultiMeasureRest) is undocumented 
and was likely never intended when outside-staff-priority was created in the 
first place.  The original language in the documentation only states that 
outside-staff-priority is relevant for grobs that are always (Measure Marks) or 
sometimes (Slurs) placed outside the staff.  All of these cases are covered by 
the patch.

Cheers,
MS