(in-package :clim-user)
(defun t1 () ;; works same on mcclim and lw (let* ((a (make-translation-transformation 10 10)) (b (make-scaling-transformation 2 2))) (multiple-value-bind (x y) ;; Spec says; ;; b (scaling) first, then a (translation) (transform-position (compose-transformations a b) 1 1)) (format t "T1 => ~a ~a~%" x y) (assert (= 12 x y))))
(defun t2 () ;; LW CLIM gives (values 12 12), McCLIM gives (values 22 22) ?? (let ((a (compose-transformation-with-translation ;; Spec says; ;; transformation (scaling) first, then translation (make-scaling-transformation 2 2) 10 10))) (multiple-value-bind (x y)(transform-position a 1 1) (format t "T2 => ~a ~a~%" x y) (assert (= 12 x y)))))
#+bugged-McCLIM definition ;;; oops, order is reversed! (defun compose-transformation-with-translation (transformation dx dy) (compose-transformations transformation (make-translation-transformation dx dy)))
;;;;; Proposed fixes
#+McCLIM (progn (defun compose-translation-with-transformation (transformation dx dy) (compose-transformations transformation (make-translation-transformation dx dy)))
(defun compose-scaling-with-transformation (transformation sx sy &optional origin) (compose-transformations transformation (make-scaling-transformation sx sy origin)))
(defun compose-rotation-with-transformation (transformation angle &optional origin) (compose-transformations transformation (make-rotation-transformation angle origin)))
(defun compose-transformation-with-translation (transformation dx dy) (compose-transformations (make-translation-transformation dx dy) transformation))
(defun compose-transformation-with-scaling (transformation sx sy &optional origin) (compose-transformations (make-scaling-transformation sx sy origin) transformation))
(defun compose-transformation-with-rotation (transformation angle &optional origin) (compose-transformations (make-rotation-transformation angle origin) transformation)) ) ;;;;
Attached is a patch file to fix the problem I reported last week.
I hope someone with privs will commit these changes to transforms.lisp.
Paul
I'll be happy to commit the patch. I have just applied it and it seems fine. Can you give me (remind me?) of an example that will show that this works? Preferably one that shows it failing in the past.
R
Sorry. Never mind. I found what I needed in the list archives. I'll try your test and let you know. If all is well, I'll do the commit in the next half an hour or so.
OK, I tested that with your tests and checked it against the spec. Looks fine according to my (admittedly lame) understanding of CLIM, and works on my (admittedly lame) single test on Allegro CL 7.0. Committed it.
I also added a docstring to MAKE-SCALING-TRANSFORMATION. I'm inclined to favor having these added, although not enough to say that people ought to slave to put them in everywhere. But I tend to add them to the code when I'm modding it for some other reasons. How do others feel about this? Is it a Bad Thing (do you prefer to have skinnier binaries at the expense of not having docstrings)?
R
| OK, I tested that with your tests and checked it against the spec. | Looks fine according to my (admittedly lame) understanding of CLIM, | and works on my (admittedly lame) single test on Allegro CL 7.0. | Committed it. | | I also added a docstring to MAKE-SCALING-TRANSFORMATION. I'm inclined | to favor having these added, although not enough to say that people | ought to slave to put them in everywhere. But I tend to add them to | the code when I'm modding it for some other reasons. How do others | feel about this? Is it a Bad Thing (do you prefer to have skinnier | binaries at the expense of not having docstrings)?
Thanks!
I tend to favor having doc strings - some editors will display them via ctl-alt-a on the name.
Paul