Re: Bug in Lookup::bezier_sandwich

[James]

On 31/08/14 08:22, Mike Solomon wrote:
> On Aug 31, 2014, at 1:42 AM, Jürgen Reuter <address@hidden> wrote:
> 
>> Mike,
>>  
>> appearently, the following patch works fine for my purposes (i.e. for flexa 
>> / porrectus shapes):
>>  
>> diff --git a/lily/lookup.cc b/lily/lookup.cc
>> index 344d42c..306d04e 100644
>> --- a/lily/lookup.cc
>> +++ b/lily/lookup.cc
>> @@ -466,6 +466,9 @@ Lookup::bezier_sandwich (Bezier top_curve, Bezier 
>> bottom_curve, Real thickness)
>>                               scm_from_double 
>> (top_curve.control_[2][Y_AXIS]),
>>                               scm_from_double 
>> (top_curve.control_[3][X_AXIS]),
>>                               scm_from_double 
>> (top_curve.control_[3][Y_AXIS]),
>> +                             ly_symbol2scm ("lineto"),
>> +                             scm_from_double 
>> (bottom_curve.control_[3][X_AXIS]),
>> +                             scm_from_double 
>> (bottom_curve.control_[3][Y_AXIS]),
>>                               ly_symbol2scm ("curveto"),
>>                               scm_from_double 
>> (bottom_curve.control_[2][X_AXIS]),
>>                               scm_from_double 
>> (bottom_curve.control_[2][Y_AXIS]),
>>  
>> Could you eventually verify that this patch is also fine for the slur code 
>> (afaics your commit aimed at the slur code)?  That would be great!
>>  
>> Thanks,
>> Jürgen
>>  
>>  
>> On Sat, Aug 30, 2014 at 6:24 PM, address@hidden <address@hidden> wrote:
>>
>>
>> Sent from my iPhone
>>
>>> On 30 août 2014, at 18:54, "Jürgen Reuter" <address@hidden> wrote:
>>>
>>>   Hi all,
>>>   there is a bug in Lookup::bezier_sandwich that severely affects ancient
>>>   notation.  This method was originally added to lookup.cc for
>>>   flexa/porrectus support.
>>>
>>>   In version 2.14, the bezier_sandwich curve still looks correctly, see
>>>   here:
>>>   http://lilypond.org/doc/v2.14/Documentation/a9/lily-551aed0c.png
>>>   or (with more context) here:
>>>   http://lilypond.org/doc/v2.14/Documentation/notation/ancient-notation
>>>
>>>   In version 2.15 and later, the bezier_sandwich curve has zero height at
>>>   its right end, which is bad; see here:
>>>   http://lilypond.org/doc/v2.15/Documentation/bf/lily-ac979051.png
>>>   or (with more context) here:
>>>   http://lilypond.org/doc/v2.15/Documentation/notation/ancient-notation
>>>
>>>   I tried to track down the problem and found the following suspicious
>>>   commit:
>>>
>>>   commit 35725a573e47be7c02c51964641ea534fb88be6b
>>>   Author: Mike Solomon <address@hidden>
>>>   Date:   Mon Feb 6 15:03:20 2012 +0100
>>>       Gets rid of bezier-sandwich stencil
>>>   diff --git a/lily/lookup.cc b/lily/lookup.cc
>>>   index 3f393e0..7b63b83 100644
>>>   --- a/lily/lookup.cc
>>>   +++ b/lily/lookup.cc
>>>   @@ -449,22 +449,32 @@ Lookup::slur (Bezier curve, Real curvethick, Real
>>>   linethick,
>>>    Stencil
>>>    Lookup::bezier_sandwich (Bezier top_curve, Bezier bottom_curve, Real
>>>   thickness)
>>>    {
>>>   -  /*
>>>   -    Need the weird order b.o. the way PS want its arguments
>>>   -  */
>>>   -  SCM list = SCM_EOL;
>>>   -  list = scm_cons (ly_offset2scm (bottom_curve.control_[3]), list);
>>>   -  list = scm_cons (ly_offset2scm (bottom_curve.control_[0]), list);
>>>   -  list = scm_cons (ly_offset2scm (bottom_curve.control_[1]), list);
>>>   -  list = scm_cons (ly_offset2scm (bottom_curve.control_[2]), list);
>>>   -  list = scm_cons (ly_offset2scm (top_curve.control_[0]), list);
>>>   -  list = scm_cons (ly_offset2scm (top_curve.control_[3]), list);
>>>   -  list = scm_cons (ly_offset2scm (top_curve.control_[2]), list);
>>>   -  list = scm_cons (ly_offset2scm (top_curve.control_[1]), list);
>>>   -
>>>   -  SCM horizontal_bend = scm_list_n (ly_symbol2scm ("bezier-sandwich"),
>>>   -                                    ly_quote_scm (list),
>>>   +  SCM commands  = scm_list_n (ly_symbol2scm ("moveto"),
>>>   +                              scm_from_double
>>>   (top_curve.control_[0][X_AXIS]),
>>>   +                              scm_from_double
>>>   (top_curve.control_[0][Y_AXIS]),
>>>   +                              ly_symbol2scm ("curveto"),
>>>   +                              scm_from_double
>>>   (top_curve.control_[1][X_AXIS]),
>>>   +                              scm_from_double
>>>   (top_curve.control_[1][Y_AXIS]),
>>>   +                              scm_from_double
>>>   (top_curve.control_[2][X_AXIS]),
>>>   +                              scm_from_double
>>>   (top_curve.control_[2][Y_AXIS]),
>>>   +                              scm_from_double
>>>   (top_curve.control_[3][X_AXIS]),
>>>   +                              scm_from_double
>>>   (top_curve.control_[3][Y_AXIS]),
>>>   +                              ly_symbol2scm ("curveto"),
>>>   +                              scm_from_double
>>>   (bottom_curve.control_[2][X_AXIS]),
>>>   +                              scm_from_double
>>>   (bottom_curve.control_[2][Y_AXIS]),
>>>   +                              scm_from_double
>>>   (bottom_curve.control_[1][X_AXIS]),
>>>   +                              scm_from_double
>>>   (bottom_curve.control_[1][Y_AXIS]),
>>>   +                              scm_from_double
>>>   (bottom_curve.control_[0][X_AXIS]),
>>>   +                              scm_from_double
>>>   (bottom_curve.control_[0][Y_AXIS]),
>>>   +                              ly_symbol2scm ("closepath"),
>>>   +                              SCM_UNDEFINED);
>>>   +
>>>   +  SCM horizontal_bend = scm_list_n (ly_symbol2scm ("path"),
>>>   scm_from_double (thickness),
>>>   +                                    ly_quote_scm (commands),
>>>   +                                    ly_quote_scm (ly_symbol2scm
>>>   ("round")),
>>>   +                                    ly_quote_scm (ly_symbol2scm
>>>   ("round")),
>>>   +                                    SCM_BOOL_T, SCM_UNDEFINED);
>>>
>>>      Interval x_extent = top_curve.extent (X_AXIS);
>>>
>>>   I do not fully understand the rationale / implications of this change,
>>>   so I do really know what to do here without affecting other places in
>>>   the code.  By the way, ancient notation does not make use of the
>>>   "thickness" argument; probably it has been introduced for some other
>>>   use elsewhere.
>>>
>>>   Could someone of the active developers look into this?  That would be
>>>   great!
>>>
>>>   Thanks a lot,
>>>   Juergen
>>> _______________________________________________
>>> lilypond-devel mailing list
>>> address@hidden
>>> https://lists.gnu.org/mailman/listinfo/lilypond-devel
>>
>> I can have a look at it this week - thanks for pointing it out!
>>
>> ~Mike
>>  
> 
> 
> Good catch - I hadn’t realized/seen that the sandwiches were used in cases 
> where they’re open on either end.
> I’ll test later today and I’ll let you know!
> 
> Cheers,
> MS

I'm guessing we at least need a tracker for this?

James