lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Adds glissando stems to Lilypond. (issue4661061)


From: hanwenn
Subject: Re: Adds glissando stems to Lilypond. (issue4661061)
Date: Tue, 19 Jul 2011 12:33:44 +0000


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))
acknowledge_glissando_stem ?

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
file glissando-stem.{cc,h}

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 ());
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.

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)
can you try to use property callbacks instead?

what does this do?

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);
what are the extremal heads of a glissando stem, and why do they need to
be translated?

http://codereview.appspot.com/4661061/diff/29001/lily/stem.cc#newcode1168
lily/stem.cc:1168: me->set_property ("flag", Stem::calc_flag_proc);
init in define-grobs.scm

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

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);
why can't the scripts figure this out for themselves?

http://codereview.appspot.com/4661061/diff/29001/lily/stem.cc#newcode1184
lily/stem.cc:1184: }
why doesnt the engraver already arrange this ?

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.")
this will use the stem-begin property you introduced in the other
patch, right?

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...
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.

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]