mensural notation improvements

mensural notation improvements

- support coloratio (even in ligatures)
- more flexible ligatura shapes

[benko.pal, 2011-01-05 20:24:24]

here is the patch set I hinted at several times:
http://lists.gnu.org/archive/html/lilypond-devel/2010-11/msg00212.html
http://lists.gnu.org/archive/html/lilypond-devel/2011-01/msg00097.html

some changes are just very slightly related:
mf/parmesan-custodes.mf just makes custodes longer (inspired by Petrucci's
Odhecaton, see
 http://imslp.org/wiki/Harmonice_Musices_Odhecaton_(Petrucci,_Ottaviano)

scm/parser-clef.scm adds Petrucci-like subbass clef

lily/ligature-bracket-engraver.cc is hair splitting over what to include and
what not.

the important ones:
mf/parmesan-noteheads.mf adds the noteheads

scm/output-lib.scm makes them usable as note head styles

the remaining ones add ligature support; to achieve it, I had to reorganise the
code, e.g. to create separate stencils for the two notes of a flexa shape
(oblique ligature); while at it, I did some todos as well (removed
join-right-amount) and added support for some customization:
now the user can request flexa shapes even where they are not compulsory by the
ligature-flexa property and can control their size within a ligature.  these
properties must be set for the second note of the flexa; I know it is ugly, but
I don't have any better idea, since these are properties of neither a single
note, nor of the whole ligature.

test files: is it ok to modify existing regression tests?
I mean not only adding more, but changing in mensural-ligatures.ly the Ockeghem
examples to look more like the ones in codex Chigi, see scans in attachment to
http://lists.gnu.org/archive/html/lilypond-devel/2010-12/msg00398.html

thanks,
p

[benko.pal, 2011-01-06 08:44:11]

One thing I forgot to mention: I've also rewritten dot handling within
ligatures.  The old code
- didn't avoid staff lines
- didn't conform actual usage: dotting notes above is not a practice, only if
they are first within a flexa, see examples in
http://anaigeon.free.fr/mes_facs/fsbarb.jpg
http://upload.wikimedia.org/wikipedia/commons/d/d7/Eton_Choirbook.jpg

(actually I know one example when the second flexa note is dotted above, but
general usage is like the two above, when such notes are dotted behind.)

[benko.pal, 2011-01-22 23:23:38]

corresponding to
http://code.google.com/p/lilypond/issues/detail?id=1424

[Carl, 2011-01-24 02:36:53]

Let me start by saying I know *nothing* about mensural notation.

The code looks good to me.

I found only one real issue:

LilyPond coding standards for C++ say that if there is only one statement in an
if clause, we omit {} around that clause.

I also had a question (and it probably doesn't matter much).  When I've written
font glyphs that are sometimes solid and sometimes hollow, I always calculate
both the inner and outer paths.  And then when I create the glyph I only use the
paths that are actually needed.  In your code, you didn't create the path unless
it was needed.  It probably makes no difference at all, but I'd like to hear
from the font gurus if there is a preference.

My take was we only make the fonts at install, so the code doesn't need to be
optimized for speed, so I optimized for readability, which in my mind, meant not
putting the inner path inside a conditional.

Great work!

Thanks,

Carl

http://codereview.appspot.com/3797046/diff/1/lily/mensural-ligature-engraver.cc
File lily/mensural-ligature-engraver.cc (right):

http://codereview.appspot.com/3797046/diff/1/lily/mensural-ligature-engraver....
lily/mensural-ligature-engraver.cc:277: {
Remove unneeded {}

http://codereview.appspot.com/3797046/diff/1/lily/mensural-ligature-engraver....
lily/mensural-ligature-engraver.cc:315: prev_primitive->set_property
("add-join", ly_bool2scm (true));
Remove unneeded {} from previous line and next line

http://codereview.appspot.com/3797046/diff/1/lily/mensural-ligature-engraver....
lily/mensural-ligature-engraver.cc:380: {
Why put the {} in this case?

http://codereview.appspot.com/3797046/diff/1/lily/mensural-ligature-engraver....
lily/mensural-ligature-engraver.cc:448: {
Remove {}

http://codereview.appspot.com/3797046/diff/1/lily/mensural-ligature-engraver....
lily/mensural-ligature-engraver.cc:453: {
Remove {}

http://codereview.appspot.com/3797046/diff/1/lily/mensural-ligature-engraver....
lily/mensural-ligature-engraver.cc:455: {
Remove {}

http://codereview.appspot.com/3797046/diff/1/lily/mensural-ligature-engraver....
lily/mensural-ligature-engraver.cc:460: {
Remove {}

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

http://codereview.appspot.com/3797046/diff/1/lily/mensural-ligature.cc#newcod...
lily/mensural-ligature.cc:101: {
remove {}

http://codereview.appspot.com/3797046/diff/1/lily/mensural-ligature.cc#newcod...
lily/mensural-ligature.cc:144: flexa_width = robust_scm2double (me->get_property
("flexa-width"), 2.0)
Remove {}

http://codereview.appspot.com/3797046/diff/1/lily/mensural-ligature.cc#newcod...
lily/mensural-ligature.cc:221: {
Remove {}

[benko.pal, 2011-01-24 11:39:01]

hi Carl, thanks for reviewing!

> Let me start by saying I know *nothing* about mensural notation.
>
> The code looks good to me.
>
> I found only one real issue:
>
> LilyPond coding standards for C++ say that if there is only one
> statement in an if clause, we omit {} around that clause.

that's a pity - I hate adding/removing braces when the number of
statement changes.  nevertheless I'll change these instances.

> I also had a question (and it probably doesn't matter much).  When I've
> written font glyphs that are sometimes solid and sometimes hollow, I
> always calculate both the inner and outer paths.  And then when I create
> the glyph I only use the paths that are actually needed.  In your code,
> you didn't create the path unless it was needed.  It probably makes no
> difference at all, but I'd like to hear from the font gurus if there is
> a preference.
>
> My take was we only make the fonts at install, so the code doesn't need
> to be optimized for speed, so I optimized for readability, which in my
> mind, meant not putting the inner path inside a conditional.

in this case the shape of the inner path is conditional as well
(and it's truly nasty, I admit).

I'll upload a modified patchset tonight.

p

[benko.pal, 2011-01-24 21:05:40]

now I see I forgot to answer a question:

>
http://codereview.appspot.com/3797046/diff/1/lily/mensural-ligature-engraver....
> lily/mensural-ligature-engraver.cc:380: {
> Why put the {} in this case?

because variables defined there should not be seen in the default case.

p

[benko.pal, 2011-01-24 21:27:35]

new patchset up.

please advise me about regression tests: can I modify ligatures within
mensural-ligatures.ly?  if not, can I add new ones to the same file?

[Carl, 2011-01-24 22:29:29]

On 2011/01/24 21:27:35, benko.pal wrote:
> new patchset up.
> 
> please advise me about regression tests: can I modify ligatures within
> mensural-ligatures.ly?  if not, can I add new ones to the same file?

Ancient music has been abandoned by developers for a number of years.  IMO, you
may do whatever you think makes the most sense as you try to bring ancient music
back into the mainstream of LilyPond development.

Thanks,

Carl

[benko.pal, 2011-01-24 22:32:05]

>> please advise me about regression tests: can I modify ligatures within
>> mensural-ligatures.ly?  if not, can I add new ones to the same file?
>
> Ancient music has been abandoned by developers for a number of years.
> IMO, you may do whatever you think makes the most sense as you try to
> bring ancient music back into the mainstream of LilyPond development.

I thought there might be a general policy about editing files in the
regression suite.

p

[c_sorensen, 2011-01-24 22:36:57]

On 1/24/11 3:31 PM, "Benkő Pál" <benko.pal@gmail.com> wrote:

>>> please advise me about regression tests: can I modify ligatures within
>>> mensural-ligatures.ly?  if not, can I add new ones to the same file?
>> 
>> Ancient music has been abandoned by developers for a number of years.
>> IMO, you may do whatever you think makes the most sense as you try to
>> bring ancient music back into the mainstream of LilyPond development.
> 
> I thought there might be a general policy about editing files in the
> regression suite.

As far as I know, the only policies we have about writing regression tests
are in the Contributors' Guide, section 10.8.2 Writing regression tests.

"
Regression tests should be as brief as possible to demonstrate the
functionality of the code.

Regression tests should generally cover one issue per test. Several short,
single-issue regression tests are preferred to a single, long,
multiple-issue regression test.

Use existing regression tests as templates to demonstrate the type of header
information that should be included in a regression test.
"

Sometimes a regression test is intended to show all possible functionality.
In such a case, an existing test should be extended to show the added
functionality.

But where possible, we try to keep tests to a single issue.

Personally, I will trust your judgment in this case.  Do what seems best.

Thanks,

Carl

[benko.pal, 2011-01-24 22:42:42]

>> please advise me about regression tests: can I modify ligatures within
>> mensural-ligatures.ly?  if not, can I add new ones to the same file?
>
> Ancient music has been abandoned by developers for a number of years.
> IMO, you may do whatever you think makes the most sense as you try to
> bring ancient music back into the mainstream of LilyPond development.

I thought there might be a general policy about editing files in the
regression suite.

p