[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Bug in Lookup::bezier_sandwich
|
From: |
Mike Solomon |
|
Subject: |
Re: Bug in Lookup::bezier_sandwich |
|
Date: |
Mon, 1 Sep 2014 11:27:36 +0300 |
On Sep 1, 2014, at 11:16 AM, James <address@hidden> wrote:
> 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
>
Good idea.
Jurgen’s solution works- I’ll post a patch tomorrow unless someone else beats
me to it!
Cheers,
MS