Re: Cleaned up style (issue 4951062)
Simply have a look how other note heads are implemented, and watch how the shape changes for different design sizes. http://codereview.appspot.com/4951062/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Cleaned up style (issue 4951062)
http://codereview.appspot.com/4951062/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Cleaned up style (issue 4951062)
Can we please have a more meaningful issue title than Cleaned up style, which does not tell me at all what this review is about when I get all those notifications mails. http://codereview.appspot.com/4951062/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Cleaned up style (issue 4951062)
I created https://code.google.com/p/lilypond/issues/detail?id=1873 and called it 'Added glyphs for Kievan Notation' so when this issue is changed can we use the same title? http://codereview.appspot.com/4951062/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Cleaned up style (issue 4951062)
http://codereview.appspot.com/4951062/diff/1/mf/feta-kievan.mf
File mf/feta-kievan.mf (right):
http://codereview.appspot.com/4951062/diff/1/mf/feta-kievan.mf#newcode78
mf/feta-kievan.mf:78: fill z1{dir -6.9} .. z2 .. z3 z3 .. z4 .. z5
z5 -- z6 z6 .. z7 .. z8 z8{left} .. z9 z9 .. z10 ... {dir
-76.9}cycle;
fill z1{dir -6.9}
.. z2
.. z3
z3
.. z4
.. z5
z5
-- z6
z6
.. z7
.. z8 z8{left}
.. z9 z9
.. z10
... {dir -76.9}cycle;
Well, I generally prefer vertical alignment since it makes the code much
easier to read. However, it shouldn't be a religious thing. For
example, given that the fill command is quite long and clearly separated
into subpaths, the following looks good also:
fill z1{dir -6.9} .. z2 .. z3
z3 .. z4 .. z5
z5 -- z6
z6 .. z7 .. z8
z8{left} .. z9
z9 .. z10... {dir -76.9}cycle;
http://codereview.appspot.com/4951062/diff/6001/mf/feta-kievan.mf
File mf/feta-kievan.mf (right):
http://codereview.appspot.com/4951062/diff/6001/mf/feta-kievan.mf#newcode32
mf/feta-kievan.mf:32: x1 = 0.09 * staff_space;
I'm not really happy with the MF code: It looks like a direct
translation of PostScript points into MF points. This definitely works
but is quite clumsy. If you look at the design of other LilyPond
glyphs, you can see that all coordinates rely on meta-parameters which
control the appearance. For your glyphs, we have to directly shift
coordinates in case of a change, with a great chance to inadvertently
break important relationships within the glyph shape.
I would really like to see a `LilyPond approach' for Kievan notes also,
treating the current approach as a temporary solution.
http://codereview.appspot.com/4951062/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Cleaned up style (issue 4951062)
Changed code style and glyph names based on the comments of Carl and lemzwerg. Carl, is it OK that the group name is kievan, so now we have glyphs like kievan.s1kievan? Is this how it should be? Re: you can see that all coordinates rely on meta-parameters which control the appearance I'm new to MetaFont, so I'm not sure I know what a meta-parameter is. lemzwerg, could you or someone else more knowledgeable about MF than me code up one of these glyphs the LilyPond way as an example? I think if I see it done once correctly, I'll be able to figure it out. http://codereview.appspot.com/4951062/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Cleaned up style (issue 4951062)
Added to Tracker: https://code.google.com/p/lilypond/issues/detail?id=1873 http://codereview.appspot.com/4951062/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Cleaned up style (issue 4951062)
Looks very promising.
A couple of style comments.
I think the naming should be revised to include Kievan as part of the
name, at least for the notes like s1.
Thanks,
Carl
http://codereview.appspot.com/4951062/diff/1/mf/feta-kievan.mf
File mf/feta-kievan.mf (right):
http://codereview.appspot.com/4951062/diff/1/mf/feta-kievan.mf#newcode28
mf/feta-kievan.mf:28: % this draws the quarter note with the stem down
Is this indented with tabs? We use tabs in metafont files.
http://codereview.appspot.com/4951062/diff/1/mf/feta-kievan.mf#newcode78
mf/feta-kievan.mf:78: fill z1{dir -6.9} .. z2 .. z3 z3 .. z4 .. z5
z5 -- z6 z6 .. z7 .. z8 z8{left} .. z9 z9 .. z10 ... {dir
-76.9}cycle;
I know that Werner left a review somewhere on a font change that showed
the proper way to break lines in a case like this.
Looking, for example, at feta-noteheads.mf line 636, we see that the
proper format for this command would be
fill z1{dir -6.9}
.. z2
.. z3
z3
.. z4
.. z5
z5
-- z6
z6
.. z7
.. z8 z8{left}
.. z9 z9
.. z10
... {dir -76.9}cycle;
I guess we should add something about this to the CG. I'll do it
tonight.
http://codereview.appspot.com/4951062/diff/6001/mf/feta-kievan.mf
File mf/feta-kievan.mf (right):
http://codereview.appspot.com/4951062/diff/6001/mf/feta-kievan.mf#newcode215
mf/feta-kievan.mf:215: fet_beginchar (kievan whole note, s1);
The name should probably not be s1, since that's the name of the
standard whole note glyph.
Look at the parmesan note naming for the kind of names you should
probably use for the kievan glyphs. Something like s1Keivan.
http://codereview.appspot.com/4951062/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Cleaned up style (issue 4951062)
Cleaned up code style based on comments from janek. http://codereview.appspot.com/4951062/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
