Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1132)

Issue 263690043: Simplify coord-rotate (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 5 months ago by dak
Modified:
8 years, 4 months ago
Reviewers:
thomasmorley651
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Simplify coord-rotate This does not fix the problem that (sin PI) is not really zero because of the MPU working with higher precision than PI is specified, but at least it makes the code nicer to read and modify.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -14 lines) Patch
M scm/lily-library.scm View 1 chunk +7 lines, -14 lines 0 comments Download

Messages

Total messages: 3
thomasmorley651
Great simplification! Though, I'd like to add two features 1) Making it possible to rotate ...
8 years, 5 months ago (2015-10-10 20:08:28 UTC) #1
thomasmorley651
Please disregard my code from comment #1, it's fishy. More appropiate would be (dropping the ...
8 years, 5 months ago (2015-10-15 12:34:17 UTC) #2
dak
8 years, 5 months ago (2015-10-15 12:46:52 UTC) #3
On 2015/10/15 12:34:17, thomasmorley651 wrote:
> Please disregard my code from comment #1, it's fishy.
> 
> More appropiate would be (dropping the idea to rotate around other ponts than
> zero):
> 
> #(define (get-PI/4-rotated-quadrants radians)
>   (cond ((>= radians TWO-PI)
>          (get-PI/4-rotated-quadrants (- radians TWO-PI)))
>         ((< radians 0)
>          (get-PI/4-rotated-quadrants (+ radians TWO-PI )))
>         (else 
>           (let* (;; get the octants
>                  (oct (truncate (/ radians (/ PI 4)))))
>             ;; get the rotated quadrants
>             (cond ((= oct 7) 0)
>                   ((even? oct)
>                    (/ oct 2))
>                   (else (/ (1+ oct) 2)))))))
> 
> (define-public (coord-rotate-p-II coordinate angle-in-radians)
>   ;; getting around (sin PI) not being exactly zero by switching to cos at
>   ;; appropiate angles and/or taking the negative value (vice versa for cos)
>   (let* ((angle (angle-0-2pi angle-in-radians))
>          (quadrant (get-PI/4-rotated-quadrants angle))
>          (moved-angle (- angle (/ (* quadrant PI) 2)))
>          (cm (cos moved-angle))
>          (sm (sin moved-angle))
>          (c (cond ((= 1 quadrant) (- sm))
>                   ((= 2 quadrant) (- cm))
>                   ((= 3 quadrant) sm)
>                   (else cm)))
>          (s (cond ((= 1 quadrant) cm)
>                   ((= 2 quadrant) (- sm))
>                   ((= 3 quadrant) (- cm))
>                   (else sm)))
>          (x (coord-x coordinate))
>          (y (coord-y coordinate)))
>     (cons (- (* c x) (* s y))
>           (+ (* s x) (* c y)))))
> 
> 
> Maybe in a follow up?
> The patch itself LGTM

I prefer a followup since this is a pretty hefty change with a different focus
and warranting a separate Rietveld review.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b