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

[dak]

On 2013/02/24 16:20:53, mike7 wrote:
> On 24 févr. 2013, at 17:18, David Kastrup <mailto:address@hidden> wrote:

> > "address@hidden" <mailto:address@hidden> writes:
> >
> >> On 24 févr. 2013, at 16:37, mailto: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.
> >>>
> >>>> 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.

This is really like pulling teeth.  Something _obviously_ consults
Y-offset, or overriding it with a callback would not have an effect.
Now said something can also check Y-offset for being unset (presumably
the default) and outside-staff-priority for being set (the
non-default) and call up the most likely of the totally dissimilarly
named callbacks you mention in changes.tely without bothering to say
which one to choose under what circumstances.

Can you _merge_ those callbacks?  Then there is no need to pick one.

Setting something like outside-staff-priority is still stuff that
users can comprehend (and that's actually documented in the manuals).
Having to pick one of several totally differently named callbacks
without useful documentation is _not_ a user interface.

That's internals.

> I think the current patch is fine.

It does not have a usable user interface.  It is not an option for a
user to choose between several undocumented callbacks.


https://codereview.appspot.com/7185044/