Double G clef, tenor G clef, varpercussion clef and varC clef

Double G clef, tenor G clef, varpercussion clef and varC clef

This patch covers the clefs requested in:

http://lists.gnu.org/archive/html/lilypond-user/2010-02/msg00029.html
(double G clef and varpercussion clef)

http://lists.gnu.org/archive/html/lilypond-user/2013-11/msg00661.html
(tenor G clef)

http://code.google.com/p/lilypond/issues/detail?id=693
(varC clef)

[dak, 2014-01-04 16:59:15]

https://codereview.appspot.com/47840043/diff/1/input/regression/clefs.ly
File input/regression/clefs.ly (right):

https://codereview.appspot.com/47840043/diff/1/input/regression/clefs.ly#newc...
input/regression/clefs.ly:1: \version "2.19.1"
This version will not make Patchy happy.  We are at 2.19.0, at least today.

[marc, 2014-01-04 18:23:43]

Version corrected

[marc, 2014-01-04 18:27:33]

Am 04.01.2014 17:59, schrieb dak@gnu.org:
>
> https://codereview.appspot.com/47840043/diff/1/input/regression/clefs.ly
> File input/regression/clefs.ly (right):
>
>
https://codereview.appspot.com/47840043/diff/1/input/regression/clefs.ly#newc...
>
> input/regression/clefs.ly:1: \version "2.19.1"
> This version will not make Patchy happy.  We are at 2.19.0, at least
> today.

Sorry, done.

BTW, the name of the patchset is chosen from the last local commit
message – strange, but changed now ;-)

>
> https://codereview.appspot.com/47840043/
>

[lemzwerg, 2014-01-05 04:51:22]

https://codereview.appspot.com/47840043/diff/20001/mf/feta-clefs.mf
File mf/feta-clefs.mf (right):

https://codereview.appspot.com/47840043/diff/20001/mf/feta-clefs.mf#newcode186
mf/feta-clefs.mf:186: reduced_ss# = staff_space# * reduction;
Uh, oh, the diff here on Rietveld is so big and unreadable because you've
converted tabs to spaces, probably inadvertently.  Please post another revision
of your patch that fixes this, and ensure that your editor doesn't do this
conversion automatically again.

[marc, 2014-01-05 10:17:00]

https://codereview.appspot.com/47840043/diff/20001/mf/feta-clefs.mf
File mf/feta-clefs.mf (right):

https://codereview.appspot.com/47840043/diff/20001/mf/feta-clefs.mf#newcode186
mf/feta-clefs.mf:186: reduced_ss# = staff_space# * reduction;
On 2014/01/05 04:51:22, lemzwerg wrote:
> Uh, oh, the diff here on Rietveld is so big and unreadable because you've
> converted tabs to spaces, probably inadvertently.  Please post another
revision
> of your patch that fixes this, and ensure that your editor doesn't do this
> conversion automatically again.

I moved the definition of new_bulb to the beginning of the file. Diff seems
to try to check differences paragraph-wise (i.e. splitted on empty lines),
and this is where this ugly diff result comes from, I assume.

The last time I had tab/space conversion problems, the diff looked completely
different, so I think this is not the case here.

[wl_gnu.org, 2014-01-05 10:40:35]

>> Uh, oh, the diff here on Rietveld is so big and unreadable because
>> converted tabs to spaces, probably inadvertently.  [...]
> 
> I moved the definition of new_bulb to the beginning of the
> file. Diff seems to try to check differences paragraph-wise
> (i.e. splitted on empty lines), and this is where this ugly diff
> result comes from, I assume.

Not really.

> The last time I had tab/space conversion problems, the diff looked
> completely different, so I think this is not the case here.

Well, you *should* use tabs, so this deserves a new patch set anyway.
It's easy to see in the web browser:

  -       after := z2'{dir (turning_dir * 90)}
  -              .. end_point{dir (end_angle)};
  -       (u, v) = before intersectiontimes after;
  +        thin := hround (0.15 reduced_ss);
  +        thin := hround (3/2 hair);

The `-' and `+' lines are not aligned vertically.


    Werner

[marc, 2014-01-06 09:44:37]

tabs vs. spaces issue

[marc, 2014-01-06 09:46:58]

Am 05.01.2014 11:40, schrieb Werner LEMBERG:
[...]
> Well, you *should* use tabs, so this deserves a new patch set anyway.
[...]

Done.

Marc

[lemzwerg, 2014-01-06 11:17:47]

I've now had some time to check the glyphs.  Here my comments.

1. Please make all corners round in `clefs.varC' and `clefs.tenorG'.

2. In `clefs.tenorG', I'm not sure that the left `slanted' end of the top
horizontal bar should really look like that.  IMHO, it's simply a standard
horizontal bar with rounded corners, and the slantedness in the image of
http://lists.gnu.org/archive/html/lilypond-user/2013-11/msg00661.html is a
printing artifact.  Are there better scans for this glyph?

Besides that, everything looks good to me, thanks.

[marc, 2014-01-13 10:04:05]

Corrections: bigger bulbs, rounded corners, more clef types

[marc, 2014-01-14 08:35:25]

tenorG: left slanted edge rounded

[lemzwerg, 2014-01-18 06:12:15]

First of all: Looking at clefs.tenorG, patch set 5 compared to patch set 4 has a
side effect: It reduces the height of the glyph's top.  Is this intentional?  I
guess it's not, so please fix this.

Compiling with

  mf2pt1 --rounding=0.0001 feta20.mf

The script returns the following errors (from a self-compiled fontforge binary
based on the git repository, identifying itself as 9-Dec-2013-ML-TtfDb-D):

  Internal Error (overlap) in clefs.tenorG:
    monotonic is both needed and unneeded
    (378,988,152,446)->(232,924,31,5377).
    x=371,416 (prev=287,662)
  Internal Error (overlap) in clefs.tenorG:
    monotonic is both needed and unneeded
    (378,988,152,446)->(232,924,31,5377).
    x=359,647 (prev=287,662)
  Internal Error (overlap) in clefs.tenorG:
    Humph. This monotonic leads nowhere
    (401,125,-82,4102)->(357,086,151,324).
  Internal Error (overlap) in clefs.tenorG:
    Closing contour with unneeded path
  Internal Error (overlap) in clefs.tenorG:
    couldn't find a needed exit from an intersection

This is a bug in FontForge; I guess I've reported about 10 similar issues to the
original author.  However, such bugs are hard to find and to fix, I was told,
and I doubt that the current development team has enough expertise to correct
that at all, or within a short time frame (the original author has retired,
unfortunately).  My solution to previous problems with FontForge's outline
overlap remover was to simplify the topology of the problematic Emmentaler
glyph.

Looking at the glyph without overlaps removed (created with a call to
`FONTFORGE=foo ./mf2pt1 ...'), I see that the horizontal line starting in point
1''' almost exactly crosses two other outlines in a single point, and this
near-intersection point of three outlines is only given implicitly so that
mf2pt1 doesn't convert it to a real PostScript point.  Due to rounding issues –
even if FontForge is compiled with double precision – this might lead to the
current FontForge problem.

I see two solutions:

  1. Make this intersection point of three outlines a real point, which should
be then part of all three METAFONT paths.

  2. Slightly shift up the horizontal lines starting at points 1''' and 8''' to
avoid this triple intersection point completely.

One of the two solutions should work, I expect.

https://codereview.appspot.com/47840043/diff/80001/mf/feta-clefs.mf
File mf/feta-clefs.mf (right):

https://codereview.appspot.com/47840043/diff/80001/mf/feta-clefs.mf#newcode531
mf/feta-clefs.mf:531: -- z108
Whitespace...

https://codereview.appspot.com/47840043/diff/80001/mf/feta-clefs.mf#newcode563
mf/feta-clefs.mf:563: forsuffixes e := l,,r:
Wrong indentation.

https://codereview.appspot.com/47840043/diff/80001/mf/feta-clefs.mf#newcode575
mf/feta-clefs.mf:575: .. z210e{down_}
Wrong indentation (this is, missing vertical alignment to `z221e').

https://codereview.appspot.com/47840043/diff/80001/mf/feta-clefs.mf#newcode613
mf/feta-clefs.mf:613: pair ne_beam_dir,nw_dist;
Wrong indentation.

[marc, 2014-01-18 09:34:19]

indentation fixed; preserve tenorG clef height; intersection problem fixed(?)

[marc, 2014-01-18 09:47:18]

Am 18.01.2014 07:12, schrieb lemzwerg@googlemail.com:
> First of all: Looking at clefs.tenorG, patch set 5 compared to patch
> set 4 has a side effect: It reduces the height of the glyph's top.  Is
> this intentional?  I guess it's not, so please fix this.

Done.

I don't fully understand why (yet), but the order in which the two parts
(g clef and tenor extension) are drawn seems to matter somehow despite
the fact that all variables are saved and made internal.
>
> Compiling with
>
>    mf2pt1 --rounding=0.0001 feta20.mf
>
> The script returns the following errors (from a self-compiled fontforge
> binary based on the git repository, identifying itself as
> 9-Dec-2013-ML-TtfDb-D):
>
>    [...]
>
> This is a bug in FontForge;
 > [...]

Well, that's bad, but kind of reassuring that this is not a fault of 
mine ...
[...]
>
> I see two solutions:
>
>    1. Make this intersection point of three outlines a real point, which
> should be then part of all three METAFONT paths.
>
>    2. Slightly shift up the horizontal lines starting at points 1''' and
> 8''' to avoid this triple intersection point completely.
>
> One of the two solutions should work, I expect.

I decided to go for sulution 2 but I can't test it on my machine
(I don't have a self-compiled double-precision fontforge around)
>
>
>
> https://codereview.appspot.com/47840043/diff/80001/mf/feta-clefs.mf
> File mf/feta-clefs.mf (right):
>
>
https://codereview.appspot.com/47840043/diff/80001/mf/feta-clefs.mf#newcode531
>
> mf/feta-clefs.mf:531: -- z108
> Whitespace...

Done.

>
https://codereview.appspot.com/47840043/diff/80001/mf/feta-clefs.mf#newcode563
>
> mf/feta-clefs.mf:563: forsuffixes e := l,,r:
> Wrong indentation.
Done.

>
https://codereview.appspot.com/47840043/diff/80001/mf/feta-clefs.mf#newcode575
>
> mf/feta-clefs.mf:575: .. z210e{down_}
> Wrong indentation (this is, missing vertical alignment to `z221e').
Done.
>
>
https://codereview.appspot.com/47840043/diff/80001/mf/feta-clefs.mf#newcode613
>
> mf/feta-clefs.mf:613: pair ne_beam_dir,nw_dist;
> Wrong indentation.
Done.
> https://codereview.appspot.com/47840043/

[lemzwerg, 2014-01-18 19:15:38]

It now works for me, thanks.