Unifies mensural ligatures with blot-diameter.

Unifies mensural ligatures with blot-diameter.

Reverts some changes of commit 0dcc93c0a5a97d048db2f7631966f41ae0059ab5

Use blot-diameter either in C and MetaFont.

[Bertrand Bordage, 2011-09-17 20:15:00]

Hi,

The new mensural style introduced with commit
0dcc93c0a5a97d048db2f7631966f41ae0059ab5 created some ugliness's in mensural
ligatures.

This patch fix these. I also added serifs to flexas.

Regards,
Bertrand

[pkx166h, 2011-09-17 21:38:09]

Passes make and reg tests - I get differences but they look ok

see:

http://code.google.com/p/lilypond/issues/detail?id=1898#c1

[benko.pal, 2011-09-18 18:36:19]

hi Bertrand,

> The new mensural style introduced with commit
> 0dcc93c0a5a97d048db2f7631966f41ae0059ab5 created some ugliness's in
> mensural ligatures.

which commit is the patch supposed to apply to?
I couldn't apply it to my HEAD
f8a4491c571dc57c87aec33dc8e34c436e222537;
having applied it to 0dcc93 it produced empty longae in ligatures,
after removing the build directory even make failed.

p

[Bertrand Bordage, 2011-09-18 19:22:30]

Hi Pal!

I updated the patch.
This wasn't working since Werner's e10a13.
It couldn't be applied to 0dcc93: I forgot some files that I added in 0e31cd.

Bertrand

[Bertrand Bordage, 2011-09-18 19:55:47]

Another update that fixes some variable errors.
It now passes make.

[benko.pal, 2011-09-18 20:39:57]

hi Bertrand,

> Another update that fixes some variable errors.
> It now passes make.

this is ok, passes apply, make and all my test; thanks!
p

[lemzwerg, 2011-09-18 21:04:54]

MF code LGTM.

http://codereview.appspot.com/5030053/diff/9001/mf/parmesan-noteheads.mf
File mf/parmesan-noteheads.mf (right):

http://codereview.appspot.com/5030053/diff/9001/mf/parmesan-noteheads.mf#newc...
mf/parmesan-noteheads.mf:497: draw_mensural_longa (m_longa_width, m_holeheight,
true, true, -1, true);
Indentation (here and similar places below).

[Bertrand Bordage, 2011-09-18 21:26:07]

Oh, yes. Thanks Werner, this is fixed.

[janek, 2011-09-18 21:47:04]

I think LGTM, but it would be great if you'd add a regtest to demonstrate what
this patch is fixing.  (i was going to write "before/after pdfs attached to
tracker issue would be priceless!" but i've just saw that you added them -
perfect!)

thanks,
Janek

http://codereview.appspot.com/5030053/diff/9001/lily/mensural-ligature.cc
File lily/mensural-ligature.cc (right):

http://codereview.appspot.com/5030053/diff/9001/lily/mensural-ligature.cc#new...
lily/mensural-ligature.cc:79: stencil = Lookup::beam (corrected_slope, width *
0.5, staff_space, blotdiameter);
What does this do? (sorry for a stupid question)

http://codereview.appspot.com/5030053/diff/9001/lily/mensural-ligature.cc#new...
lily/mensural-ligature.cc:201: ("noteheads.sM2ligmensural").extent
(Y_AXIS).length () * 0.5
I don't get it - why is this commented?

[Bertrand Bordage, 2011-09-18 22:23:26]

On 2011/09/18 21:47:04, janek wrote:
> I think LGTM, but it would be great if you'd add a regtest to demonstrate what
> this patch is fixing.

I don't think so. mensural-ligatures.ly contains every case fixed by this patch.
If I make a regtest to show such tiny graphical differences, then we would need
to do the same thing for almost every graphical object...


>
http://codereview.appspot.com/5030053/diff/9001/lily/mensural-ligature.cc#new...
> lily/mensural-ligature.cc:79: stencil = Lookup::beam (corrected_slope, width *
> 0.5, staff_space, blotdiameter);
> What does this do? (sorry for a stupid question)

This is making a beam with round corners (blotdiameter is the radius of the
round corner). These beams are used to represent the flexa (the big 'slide' at
the end of the before/after PNGs).


>
http://codereview.appspot.com/5030053/diff/9001/lily/mensural-ligature.cc#new...
> lily/mensural-ligature.cc:201: ("noteheads.sM2ligmensural").extent
> (Y_AXIS).length () * 0.5
> I don't get it - why is this commented?

Because the stems of these noteheads are not inside their Y-extent. This comment
shows how this should ideally work.

[janek, 2011-09-20 20:17:58]

2011/9/19  <bordage.bertrand@gmail.com>:
> On 2011/09/18 21:47:04, janek wrote:
>>
>> I think LGTM, but it would be great if you'd add a regtest to
>
> demonstrate what
>>
>> this patch is fixing.
>
> I don't think so. mensural-ligatures.ly contains every case fixed by
> this patch.

True.  However, your changes are very subtle and extremely easy to
overlook.  I'm sure that if someone looked at the regtest comparison
without knowing what to look for, he would miss it entirely and think
that nothing changed.

> If I make a regtest to show such tiny graphical differences, then we
> would need to do the same thing for almost every graphical object...

I seriously consider adding dedicated regtests for all such tiny
graphical differencies.  Please see this thread for a more detailed
discussion about it:
http://lists.gnu.org/archive/html/lilypond-devel/2011-09/msg00308.html

>
http://codereview.appspot.com/5030053/diff/9001/lily/mensural-ligature.cc#new...
>>
>> lily/mensural-ligature.cc:79: stencil = Lookup::beam (corrected_slope,
>
> width *
>>
>> 0.5, staff_space, blotdiameter);
>> What does this do? (sorry for a stupid question)
>
> This is making a beam with round corners (blotdiameter is the radius of
> the round corner). These beams are used to represent the flexa (the big
> 'slide' at the end of the before/after PNGs).

Ah, so Lookup::beam () prints a beam sybol!  Nice.

>
http://codereview.appspot.com/5030053/diff/9001/lily/mensural-ligature.cc#new...
>>
>> lily/mensural-ligature.cc:201: ("noteheads.sM2ligmensural").extent
>> (Y_AXIS).length () * 0.5
>> I don't get it - why is this commented?
>
> Because the stems of these noteheads are not inside their Y-extent. This
> comment shows how this should ideally work.

ok, i understand now.

thanks,
Janek

[benko.pal, 2011-09-21 19:48:20]

On 2011/09/18 19:55:47, Bertrand Bordage wrote:
> Another update that fixes some variable errors.
> It now passes make.

thanks Bertrand, this is great work;
I can now print flexae just the default way!
(so long I had to use non-default viewers and lpr commands.)

p

[Neil Puttock, 2011-09-21 21:11:50]

http://codereview.appspot.com/5030053/diff/2004/lily/mensural-ligature.cc
File lily/mensural-ligature.cc (right):

http://codereview.appspot.com/5030053/diff/2004/lily/mensural-ligature.cc#new...
lily/mensural-ligature.cc:74: = (me->layout ()->get_dimension (ly_symbol2scm
("blot-diameter")));
remove extra parens

[Bertrand Bordage, 2011-09-21 21:22:05]

On 2011/09/21 21:11:50, Neil Puttock wrote:
> remove extra parens

Done.

Janek and Pal: thanks!

[Bertrand Bordage, 2011-09-22 08:23:14]

Pushed as 829f0ded77ee44ea6f0566fb5e5318802a8857ad.