Re: Issue 3061 (issue 7033045)

[Zefram]

address@hidden wrote:
>All this seems far too complex.  Why not use ly:duration-length and just
>divide the respective values of ly:moment-main?  No need to fiddle with
>counting dots and shift factors manually.

That's a valid approach, but doesn't win quite as much as you'd think.
In the most obvious form, it would change the tgt-nrep binding to
something like this (untested):

         (tgt-nrep (/ (ly:moment-main (ly:duration-length totaldur))
                      (ly:moment-main (ly:make-duration tremtype-log 0
                                        (ly:duration-scale totaldur)))))

It does involve fewer function applications (6 instead of 11).  But on
the downside it'll break if the note has been scaled by a factor of zero.
Fixing that increases the complexity:

         (tgt-nrep (/ (ly:moment-main
                        (ly:duration-length
                          (ly:make-duration (ly:duration-log totaldur)
                                            (ly:duration-dot-count totaldur)
                                            1)))
                      (ly:moment-main (ly:make-duration tremtype-log 0 1))))

Eight function applications, still fewer than my original, but it has
more long function names (and so takes more text lines given a reasonable
length limit), and doesn't avoid the need for the code to know about dot
counts.  It doesn't have to *understand* dot counts, though, so that's
a bit of a win.  Also, the de-scaled version of totaldur can be brought
out as a separate binding and then reused for the "non-integer tremolo"
warning message.  I'd be happy for such a variant of my patch to be used
in place of the original; I see no compelling grounds to go either way.

Generally, the complexity of the code I submitted comes from being
robust in the face of possibly uncooperative data.  I'm thinking about
situations such as where the overall duration is not an exact multiple
of the nominal tremolo duration, or the music has been subjected to time
scaling, possibly by awkward factors.

I think what would *really* make this code more readable is a bunch of
library functions covering common subexpressions that are likely to come
up in music processing.  For example:

    (define (duration-dot-factor dotcount)
        (/ (1- (ash 2 dotcount)) (ash 1 dotcount)))

    (define (duration-descale dur)
        (ly:make-duration (ly:duration-log dur) (ly:duration-dot-count dur) 1))

I've written some much larger chunks of LP Scheme code that I ought to
trawl for opportunities like this.

Also, of some relevance to this code, the system of "moment" objects
is a bit cumbersome.  I can see why you need the two-part structure for
aligning music columns in the output, but it looks like most things using
moment structures only use the main part, making it just an unnecessary
wrapper around rationals.  The arithmetic on them hasn't been thought out
properly: ly:moment-div returning a moment is just ridiculous.  It'd be
nice if moments were restricted to their proper role in typesetting,
and everything else just used rationals.  (With grace note articulation
through \articulate working, I think it'd be viable for basic MIDI output
to totally ignore grace notes, just as it ignores most articulations,
so even it wouldn't need moment structures.  But I digress.)

So, getting back to the subject, let me know if you'd like me to produce
a complete tremolo patch based on the variant that I described above.
Or a patch factoring out some of the code into a library of wider utility.

-zefram