On 2013/01/01 22:05:58, zefram_fysh.org wrote:
mailto:d...@gnu.org 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.
Excellent point. Of course, it begs the question how to articulate _anything_ in the presence of scaled durations.
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.
You are counting characters here. That's not a useful measure of complexity.
It doesn't have to *understand* dot counts, though, so that's a bit of a win.
It is not just "a bit" of a win. The code for expanding and displaying tremolo dot counts have been at odds with the execution for the longest amount of time (check out issue 2602 <URL:http://code.google.com/p/lilypond/issues/detail?id=2602>). Not having to interpret the contents, even if it takes just 15 characters in APL, is a big 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.
The question is whether this is the most useful approach. Other possibilities would be to give ly:duration-length an optional argument for overriding the scale factor to an arbitrary value, or make an extra function ly:visual-duration-length that ignores the scale factor.
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-dot-factor dotcount) (- 2 (/ (ash 1 dotcount)))) Not sure whether it is clearer, but at least it makes glaringly obvious that the factor does not go above 1, a frequent problem with bad dot-count expansion expressions. It also makes reasonably clear that a dotcount of 0 corresponds to a factor of 1 without special-casing.
(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.
duration-rescale with an optional argument defaulting to 1 maybe? Or would that never be useful?
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.
It is a _necessary_ wrapper around Rational, an internal C++ data type predating the availability of rational numbers in Guile.
The arithmetic on them hasn't been thought out properly: ly:moment-div returning a moment is just ridiculous.
It is just a consequence of ly:moment-div being a substitute for (/ <Rational> <Rational>) -> <Rational>.
It'd be nice if moments were restricted to their proper role in typesetting, and everything else just used rationals.
Moments with grace timing don't have useful context-independent arithmetic properties, so it would make sense to obliterate them altogether and just work with rationals everywhere. Also there is no point in having a separate C++ type "Rational", it is just cause for trouble. Replacing it with SCM is not entirely trivial since rational numbers are not immediate entities and thus need to be marked during garbage-collection, also affecting every C++ type containing rationals.
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.
"a" library of wider utility is not really required: we already have lily-library.scm and music-functions.scm (with no really clear criteria about when something should be put into either). The existing library, including the available provisions of ly:... exported functions, is badly documented, inconsistent, not described in any part of Documentation, partly duplicates existing functionality in the Guile libraries, and has partly never been used or updated. If you plan to have a go at it, you have my blessings. You'll likely get a lot of feedback of mixed quality and conflicting opinions, but there is certainly a lot of progress in that area to be made. Oh, with regard to obbliterating the C++ type Rational: I have some branch flying around called "unrational" where I try doing just that. It is just that the work in this branch, done alphabetically, has stalled somewhere in the middle of the letter "A" or "B". https://codereview.appspot.com/7033045/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel