[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Woodwind diagrams (issue1425041)
From: |
Mike Solomon |
Subject: |
Re: Woodwind diagrams (issue1425041) |
Date: |
Sun, 20 Jun 2010 02:53:43 +0200 |
User-agent: |
Microsoft-Entourage/11.4.0.080122 |
Thank you Patrick!
All of your comments have been incorporated into a new patch, which draws
the svgs a-ok (see attached).
~Mike
On 6/19/10 12:42 AM, "address@hidden" <address@hidden> wrote:
> Hi Mike,
>
> This is very impressive. Thanks for your work on this. I just have a
> few comments for you about the SVG-related code.
>
> Thanks,
> Patrick
>
>
> http://codereview.appspot.com/1425041/diff/12001/13007
> File scm/lily-library.scm (right):
>
> http://codereview.appspot.com/1425041/diff/12001/13007#newcode583
> scm/lily-library.scm:583:
> If you want to use these procedures in output-svg.scm, they all need to
> be `define-public'-ified. Like so:
>
> (define-public PI (* 4 (atan 1)))
>
> etc.
>
> http://codereview.appspot.com/1425041/diff/12001/13010
> File scm/output-svg.scm (right):
>
> http://codereview.appspot.com/1425041/diff/12001/13010#newcode352
> scm/output-svg.scm:352: (define (partial-ellipse x-radius y-radius
> start-angle end-angle thick connect fill)
> Move the definition of `new-start-angle' to here so that this code will
> compile (`new-start-angle' is used in `make-ellipse-radius')
>
> http://codereview.appspot.com/1425041/diff/12001/13010#newcode362
> scm/output-svg.scm:362: (new-end-angle (* PI-OVER-180 (angle-0-360
> end-angle)))
> PI-OVER-180 and other procedures will work once you make the changes in
> lily-library.scm.
>
> http://codereview.appspot.com/1425041/diff/12001/13010#newcode399
> scm/output-svg.scm:399: (if
> Please condense this block like so:
>
> (if connect
> (ly:format "L~4f,~4f"
> (* start-radius (cos new-start-angle))
> (- (* start-radius (sin new-start-angle))))
> "")))))))
>
> http://codereview.appspot.com/1425041/diff/12001/13010#newcode418
> scm/output-svg.scm:418: ;; Hopefully will be mitigated by ~4f below.
> Uh, how is this a security risk? PS code is not interpreted by SVG
> agents, and here we are just dealing with simple numbers and strings
> that turn into SVG markup.
>
> http://codereview.appspot.com/1425041/diff/12001/13010#newcode421
> scm/output-svg.scm:421: (string-concatenate
> I would condense this block too, to make it more readable:
>
> (string-concatenate
> (map (lambda (x)
> (apply
> (if (eq? (length x) 6)
> (lambda (x1 x2 x3 x4 x5 x6)
> (ly:format "C~4f ~4f ~4f ~4f ~4f ~4f"
> (* x1 x-scale)
> (- (* x2 y-scale))
> (* x3 x-scale)
> (- (* x4 y-scale))
> (* x5 x-scale)
> (- (* x6 y-scale))))
> (lambda (x1 x2)
> (ly:format "L~4f ~4f"
> (* x-scale x1)
> (- (* y-scale x2)))))
> x))
> pointlist))
>
> http://codereview.appspot.com/1425041/show
>
svgtest.svg
Description: Binary data