Re: crash moving DotColumn

[Thomas Morley]

2016-05-13 12:27 GMT+02:00 David Kastrup <address@hidden>:
> Thomas Morley <address@hidden> writes:
>
>> 2016-05-12 15:51 GMT+02:00 David Kastrup <address@hidden>:
>>> Thomas Morley <address@hidden> writes:
>>>
>>>> To get more space between NoteColumn and DotColumn the following used
>>>> to work with 2.18.2
>>>>
>>>> foo =
>>>> \override Score.DotColumn.before-line-breaking =
>>>>   #(lambda (dot-column)
>>>>     (ly:grob-set-property! dot-column 'positioning-done
>>>>       (lambda (grob)
>>>>         (ly:dot-column::calc-positioning-done grob)
>>>>         (ly:grob-translate-axis! grob 15 X))))
>>>>
>>>>
>>>> {
>>>>   \foo
>>>>   <gis' a' c''>4.
>>>> }
>>
>> Thanks a lot for your analysis and findings.
>>
>> \version "2.19.42"
>>
>> foo =
>> \override Score.DotColumn.before-line-breaking =
>>   #(lambda (dot-column)
>>     (ly:grob-set-property! dot-column 'positioning-done
>>       (lambda (grob)
>>         (ly:dot-column::calc-positioning-done grob)
>>         (ly:grob-translate-axis! grob 15 X)
>>         #t)))
>>
>> {
>>   \foo
>>   <gis' a' c''>4.
>> }
>>
>> Works indeed.
>> I could have sworn I tested this already...
>>
>> Can't think about further implications right now, too tired.
>
> This is all sort-of shaky.
>
> ly:dot-column::calc-positioning-done does
>
>
>   /*
>     Trigger note collision resolution first, since that may kill off
>     dots when merging.
>   */
>   if (Grob *collision = unsmob<Grob> (me->get_object ("note-collision")))
>     (void) collision->get_property ("positioning-done");
>
>   me->set_property ("positioning-done", SCM_BOOL_T);
>   [...]
>
> so it rather explicitly sets positioning-done to #t early on instead of
> waiting to return #t regularly (which it also does eventually),
> presumably to avoid mutual recursion.
>
> Now _your_ routine returns *unspecified* even _after_ positioning-done
> has been explicitly set to #t in the callback.
>
> This is quite explicitly the case that the assertion is supposed to
> complain about.  Since obviously this is triggerable by user error (in
> this case yours), maybe the assertion should be demoted to a programming
> error.  I don't see that this can trigger except by a callback directly
> (or indirectly) setting a value and then returning *unspecified*.  In
> which case there does not seem the danger of recursion unless the
> callback directly or indirectly reinstated itself.

In earlier versions (<= 2.18)

{
  \override NoteHead.stencil =
    #(lambda (grob) (ly:grob-set-property! grob 'stencil point-stencil))
  c''2
}

used to work without any warnings or errors.
Which is exactly the sort of problem we're talking about, iiuc.

> So there does not seem to be much of a point in an assertion failure.
> It does seem to constitute a programming error in general, however: a
> callback should likely always return a specific value here in order to
> let the caching mechanisms work properly.
>
> So maybe always complain about value == SCM_UNSPECIFIED here but only on
> programming_error level?

Well, one could think to make it work as in earlier versions, but I'm
absolutely against this route, although I'm pretty sure there's a lot
of user-code around doing so and likely in the LSR as well.

I'd vote for the programming error.

Sorry for late reply,
  Harm