Re: Adds glissando stems to Lilypond. (issue4661061)

[mtsolo]

Most of my comments below are Han-Wen specific, but I do have one
request (also echoed below) for anybody.

I need to figure out an input syntax for this stuff, which means I'm
gonna have to write parser/lexer related code.  Ideally, I'd like the
syntax to be

a4 mikeSolomonIsANinja8 mikeSolomonIsANinja16 a16

Where mikeSolomonIsANinja (or a suitable variant) would be the input
syntax for a glissando stem.  This would have to insert into the music
stream a glissando-note-event (probably within an EventChord), and from
that I could create a new engraver and dispense with all the cruft in
music-functions-init.ly (meaning no more startGlissandoStem,
startGlissandoStem).

I just need a brief tutorial on how to do this and I'll be able to do
it.  I tried to learn about modifying input syntax from the
chord-repetition symbol commits (q), but there is a lot of extra stuff
about repeating chords in there and I'm having trouble figuring out what
parts I'd need to follow.

Cheers,
MS


http://codereview.appspot.com/4661061/diff/29001/lily/glissando-engraver.cc
File lily/glissando-engraver.cc (right):

http://codereview.appspot.com/4661061/diff/29001/lily/glissando-engraver.cc#newcode83
lily/glissando-engraver.cc:83: if (Glissando_stem::has_interface (stem))
On 2011/07/19 12:33:45, hanwenn wrote:
> acknowledge_glissando_stem ?

So long as we can get the stems from the note columns, it seems that
it'd be cleaner to get the glissando stems this way.

http://codereview.appspot.com/4661061/diff/29001/lily/include/stem.hh
File lily/include/stem.hh (right):

http://codereview.appspot.com/4661061/diff/29001/lily/include/stem.hh#newcode75
lily/include/stem.hh:75: class Glissando_stem
On 2011/07/19 12:33:45, hanwenn wrote:
> file glissando-stem.{cc,h}

Done.

http://codereview.appspot.com/4661061/diff/29001/lily/stem-engraver.cc
File lily/stem-engraver.cc (right):

http://codereview.appspot.com/4661061/diff/29001/lily/stem-engraver.cc#newcode67
lily/stem-engraver.cc:67: stem_ = make_item (to_boolean (get_property
("glissandoStem")) ? "GlissandoStem" : "Stem", gi.grob ()->self_scm ());
On 2011/07/19 12:33:45, hanwenn wrote:
> this is borderline acceptable. Put a TODO here to make a separate
glissando Note
> Column engraver if the stem engraver needs to become more elaborate in
the
> future.

> Probably, you could have a engraver create a GlissandoStem directly
from a
> glissando-note-event.

Doable - in general, I'm gonna need to figure out a good input syntax
for this sorta thing.

Question: how would one modify the parser/lexer/whatever to create a new
symbol for glissando stems?  It'd work just like notes (a, b, c...),
rests (r), or the repeat chord thingee (q).  Once I figured out how to
do this and how to get a glissando-note-event into the music stream, I
could make this happen and make the input syntax a lot cleaner.

http://codereview.appspot.com/4661061/diff/29001/lily/stem.cc
File lily/stem.cc (right):

http://codereview.appspot.com/4661061/diff/29001/lily/stem.cc#newcode1071
lily/stem.cc:1071: Glissando_stem::after_line_breaking (SCM smob)
On 2011/07/19 12:33:45, hanwenn wrote:
> can you try to use property callbacks instead?

> what does this do?

It repositions the note-heads to their appropriate place so that the
glissando stem links up with them.

But why reposition noteheads, you ask, when they have no positive
extent?  Why not dispense with this notehead business and just roll this
into stem-begin-position?

Because I can imagine a case where people want noteheads printed (i.e.
crosses to signify stopping a string momentarily) and want the glissando
stem to pass through the notehead

The bulk of this can be farmed out to a property callback for something
like "position-noteheads" - I'd be calling that callback, though, from
after_line_breaking.

http://codereview.appspot.com/4661061/diff/29001/lily/stem.cc#newcode1164
lily/stem.cc:1164: extremals[d]->translate_axis (ypos[d] - (0.5 *
Staff_symbol_referencer::get_position (extremals[d])), Y_AXIS);
On 2011/07/19 12:33:45, hanwenn wrote:
> what are the extremal heads of a glissando stem, and why do they need
to be
> translated?

See the above comment.

Ideally, there will be as many noteheads on a glissando stem as there
are glissandi, and each one will be positioned right over the glissando.
 In fact, I should work on this next...

http://codereview.appspot.com/4661061/diff/29001/lily/stem.cc#newcode1168
lily/stem.cc:1168: me->set_property ("flag", Stem::calc_flag_proc);
On 2011/07/19 12:33:45, hanwenn wrote:
> init in define-grobs.scm

The problem is that they take dummy-inits in define-grobs.scm that allow
for the spacing-spanner to do its thing.  This resets these properties
so that the correct values can be calculated after the note-heads are
moved above.

http://codereview.appspot.com/4661061/diff/29001/lily/stem.cc#newcode1171
lily/stem.cc:1171: beam->set_property ("direction",
Beam::calc_direction_proc);
On 2011/07/19 12:33:45, hanwenn wrote:
> why can't the beam figure this out itself?

It can, but like the properties above, it needs to be reset from the
incorrect cached value used before line breaking.

http://codereview.appspot.com/4661061/diff/29001/lily/stem.cc#newcode1177
lily/stem.cc:1177: elts[i]->set_property ("direction",
Script_interface::calc_opposite_direction_proc);
On 2011/07/19 12:33:45, hanwenn wrote:
> why can't the scripts figure this out for themselves?

It can, but like the properties above, it needs to be reset.

http://codereview.appspot.com/4661061/diff/29001/lily/stem.cc#newcode1184
lily/stem.cc:1184: }
On 2011/07/19 12:33:45, hanwenn wrote:
> why doesnt the engraver already arrange this ?

It does, but like the properties above, these properties need to be
reset.

http://codereview.appspot.com/4661061/diff/29001/scm/define-grob-properties.scm
File scm/define-grob-properties.scm (right):

http://codereview.appspot.com/4661061/diff/29001/scm/define-grob-properties.scm#newcode848
scm/define-grob-properties.scm:848: at the correct places.")
On 2011/07/19 12:33:45, hanwenn wrote:
> this will use the stem-begin property you introduced in the other
patch, right?

Yes'r.

http://codereview.appspot.com/4661061/diff/29001/scm/define-grobs.scm
File scm/define-grobs.scm (right):

http://codereview.appspot.com/4661061/diff/29001/scm/define-grobs.scm#newcode953
scm/define-grobs.scm:953: (GlissandoStem ; ugh...massive code dup...
On 2011/07/19 12:33:45, hanwenn wrote:
> this is not really code, but data. You can also drop a lot of the
variables
> (stemlets?)  Perhaps it's worth the trouble to put the details into a
separate
> scheme variable.

Agreed.

After I get this to a stable point, I am going to make a series of
comprehensive regtests and attempt to pick these variables off
one-by-one to see which ones are actually necessary.

http://codereview.appspot.com/4661061/