Re: Improve implementation of dashed slurs

[Joe Neeman]

On Sat, 2009-04-18 at 21:08 -0600, Carl D. Sorensen wrote:
> 
> 
> On 4/18/09 7:56 PM, "Joe Neeman" <address@hidden> wrote:
> 
> > On Fri, 2009-04-17 at 22:17 -0600, Carl D. Sorensen wrote:
> > 
> >>> http://codereview.appspot.com/41099/diff/1021/62#newcode347
> >>> Line 347: SCM dash_details)
> >>> Since dash_details seems to just be a list of Reals, perhaps its better
> >>> to have a vector<Real> const& (with an empty vector to signify a solid
> >>> slur).
> >> 
> >> No, dash_details is a list of lists of floating point numbers.  There is 
> >> one
> >> list for each segment of the slur; there can be as many segments as the 
> >> user
> >> desires.
> > 
> > Ah, OK. I won't insist, but I think it might be nicer to make this more
> > statically typed. For example:
> > typedef struct {
> >   Real t_min;
> >   Real t_max;
> >   Real dash_fraction;
> >   Real dash_period;
> > } Dash_definition;
> > and then make the argument a vector<Dash_definition>. Since C++ is a
> > statically typed language, I think it's nicer to make use of that static
> > typing and do the type checking/conversion for SCM variables as early as
> > possible instead of propagating them through the C++ code.
> 
> I actually like the idea of checking it early, because then I could throw a
> meaningful error if there's a problem.  However, if I do that, I'll have to
> parse the SCM list in three different places:  slur::print, tie::print, and
> arpeggio::print.  If I let it be a SCM, then I only parse it one place:
> Lookup::slur.  I thought that the lack of duplicated code might balance out
> with the lack of static typing.
> 
> If I do change to static typing, then I should have one routine that is
> called from all three places, something like scm2dash_definition.  If I were
> to write such a function, where would I put it?  It seems like
> lily/lily-guile.cc is supposed to contain more general things than this
> would be.
> 
> I could make it part of Lookup, and call it there, but it's not really
> consistent with a Lookup:: function, since they're supposed to be stencils.
> I could put it in lily/misc.cc, but it doesn't seem much like those
> functions.
> 
> How about if I added it to Slur::, so we had a Slur::parse_dash_definition,
> and then I could call it from Slur::print, Tie::print, and Arpeggio::print?
> That seems to make sense to me.

Of those options, I prefer lily-guile.cc because Dash_definition is
probably useful more generally than just for slurs (maybe it will be
used for line spanners in the future?).

Another option would be to make Dash_definition a smob (like Pitch, for
example) and provide a ly:make-dash-definition function. This has the
disadvantage, though, that it makes the ly code more verbose:

\override Slur #'dash-definition =
`(,(ly:make-dash-definition 0 0.25 1 1)
,(ly:make-dash-definition 0.3 0.7 0.4 0.75) etc..)

Joe