Re: Adds glissando stems to Lilypond. (issue4661061)

[hanwenn]

hi there,

as you see I am really strict here; I hope you understand it is my duty
as maintainer to push back on anything invasive, so we can keep lilypond
as simple as possible; nothing personal.



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 13:42:06, MikeSol wrote:
> 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.

I don't think this is cleaner.  Acknowledge_glissando_stem is a much
more direct way of saying what you want.

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 13:42:06, MikeSol wrote:
> 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

Don't create syntax; it's hard and not worth the payoff.  Use a music
function instead that substitutes NoteEvents with GlissandoStemEvents

  \asGlissandoStems {  c4 c16 c8. }

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 13:42:06, MikeSol wrote:
> 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

can you imagine or do you really need it now? If it's a future
extension, drop it for now and worry about it when someone asks for it.

Since the cross "notehead" has a continuous Y position (depending on the
glissando slope and whatnot), it is not a normal notehead. Much easier
to have a GlissandoHead, and have that adjust its position by reading
stem-begin-position of the glissando stem.


> 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#newcode1168
lily/stem.cc:1168: me->set_property ("flag", Stem::calc_flag_proc);
On 2011/07/19 13:42:06, MikeSol wrote:
> 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.

huh?

spacing spanner should not even see these stems.  You should remove
stem-interface from GlissandoStem. Alternatively, you could add
stem-spacing-interface to distinguish between a stem that affects
spacing and one that doesnt.

However, since the glissandostem is such an odd beast, I think it will
be easier to keep it completely separate.

http://codereview.appspot.com/4661061/diff/29001/lily/stem.cc#newcode1184
lily/stem.cc:1184: }
On 2011/07/19 13:42:06, MikeSol wrote:
> 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.

like above, figure a way to not have the problem in the first place,
rather than fixing it up afterwards.

http://codereview.appspot.com/4661061/diff/29001/lily/stem.cc#newcode1248
lily/stem.cc:1248: "Like stems, but for glissandi",
start out with an empty interface, and add properties until your regtest
compiles cleanly.

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