View Ticket
Not logged in
Ticket Hash: 36481a3e0805a941bcb90d6478dac92836381155
Title: Make tkp::matrix::rotate accept angles in degrees too
Status: Closed Type: Feature_Request
Severity: Minor Priority: Immediate
Subsystem: Resolution: Fixed
Last Modified: 2019-07-10 15:56:19
Version Found In: current
User Comments:
anonymous added on 2019-07-04 23:19:18:

A patch to tkpath.tcl allowing [tkp::matrix::rotate] and [tkp::matrix::rotateflip] to accept
angles in degrees, by appending a d to the angle value, e.g. tkp::matrix::rotate 123d.

36a37,39
>     if {[string match "*d" $angle]} {
>     set angle [expr {[string range $angle 0 end-1] / 45.0 * atan(1)}]
>     }
78a82,84
>     if {[string match "*d" $angle]} {
>     set angle [expr {[string range $angle 0 end-1] / 45.0 * atan(1)}]
>     }

dzach


chw added on 2019-07-07 04:43:17:
Added with check-in [09d123a672], please review and test.

anonymous added on 2019-07-07 21:21:58:

Thank you for expanding the patch to cover all angles in tkpath.tcl.

Tests:
1. Using raw numeric values [expr {$angle * 0.017453292519943295}] has a 40% advantage in execution time over [expr {$angle / 45.0 * atan(1)}] which I used in my patch. I should have tested performance before submitting the patch.
2. [tkp::matrix::rotateflip] does not seem to give the expected results, but the problem is unrelated to this ticket, as it is also present to [tkp::matrix::flip]. Try the following:

package require tkpath
pack [tkp::canvas .c -xscrollincrement 1 -yscrollincrement 1]
.c xview scroll -50 u
.c yview scroll -50 u
# draw axis lines and rotation/flip center
.c create path {M -200 0 H 200 M 0.5 -200 V 200} -stroke red -tags axes
.c create path {M 5 10 L 15 10 M 10 5 L 10 15} -tags cross
# draw some shape
.c create ptext 10 10 -text Test -fill blue -tags t -textanchor nw -matrix [tkp::matrix::flip 10 10 1 1]
# result looks ok
.c itemconfig t -matrix [tkp::matrix::flip 10 10 -1 1]; # reflect on the y axis
# result looks ok
.c itemconfig t -matrix [tkp::matrix::flip 10 10 1 -1]; # reflect on the x axis
# the result doesn't look ok. The reflection transformation is correct
# for a cartesian coordinate system, but not for a canvas, where
# the y coordinates are inverted. Changing the transformation matrix to reflect
# this fact, i.e. negating last value in the matrix, solves the problem.
proc ::tkp::matrix::flip {{cx 0} {cy 0} {fx 1} {fy 1}} {
    return [list [list $fx 0] [list 0 $fy]  [list [expr {$cx*(1-$fx)}] [expr {-1 * $cy*($fy-1)}]]]
}
# now
.c itemconfig t -matrix [tkp::matrix::flip 10 10 1 -1]
# looks right, as does:
.c itemconfig t -matrix [tkp::matrix::flip 10 10 -1 -1]
The problem is more pronounced with [tkp::matrix:::rotateflip], since there are more calculations involved and the error is amplified.
dzach


anonymous added on 2019-07-08 00:09:44:

Looking closer to the matrix transformation for [::tkp::matrix::flip], it seems the tcl transformation matrix takes into account the canvas coordinate system, i.e. the inversion of the cy coordinate. However, if the y coordinates are already being inversed elsewhere, then the transformation matrix should not do another inversion. In this case the code in tkpath.tcl should be symmetric with respect to both cx and cy coordinates:


proc ::tkp::matrix::flip {{cx 0} {cy 0} {fx 1} {fy 1}} {
    return [list [list $fx 0] [list 0 $fy]  [list [expr {$cx*(1-$fx)}] [expr {$cy*(1-$fy)}]]]
}
which is essentially the same as above, only written better. I hope the above explanation makes some sense.


anonymous added on 2019-07-08 00:35:19:

... in which case, the following code should solve the problem for the [::tkp::matrix::rotateflip] proc too:


proc ::tkp::matrix::rotateflip {{angle 0} {cx 0} {cy 0} {fx 1} {fy 1}} {
    if {[string match "*d" $angle]} {
	set angle [expr {[string range $angle 0 end-1] / 45.0 * atan(1)}]
    }
    set myCos [expr {cos($angle)}]
    set mySin [expr {sin($angle)}]
    if {$cx == 0 && $cy == 0} {
	return [list [list [expr {$fx*$myCos}] [expr {$fx*$mySin}]] \
	    [list [expr {-1.*$mySin*$fy}] [expr {$myCos*$fy}]] {0 0}]
    }
    return [list [list [expr {$fx*$myCos}] [expr {$fx*$mySin}]] \
	[list [expr {-1.*$mySin*$fy}] [expr {$myCos*$fy}]] \
        [list \
       	[expr {$myCos*$cx*(1.-$fx) - $mySin*$cy*(1.-$fy) + $cx - $myCos*$cx + $mySin*$cy}] \
        [expr {$mySin*$cx*(1.-$fx) + $myCos*$cy*(1.-$fy) + $cy - $mySin*$cx - $myCos*$cy}] \
	]]

}
dzach


anonymous added on 2019-07-09 14:49:09:

I've replicated this problem with TIP510's [path::matrix::flip] and [path::matrix::rotateflip] by using a TIP510 executable, and substituting [path] for [tkp::canvas].
The problem seems to be in the transformation matrix tcl code, where an unnecessary sign inversion seems to have been inserted. Doing the matrix math by hand, for flip:

                       translate                 translate
                       to origin      flip       back

                      | 1 0 cx |   | fx 0 0 |   | 1 0 -cx |
[x' y' 1] = [x y 1] . | 0 1 cy | . | 0 fy 0 | . | 0 1 -cy | =
                      | 0 0  1 |   | 0  0 1 |   | 0 0   1 |

                      | fx 0 cx |   | 1 0 -cx |
          = [x y 1] . | 0 fy cy | . | 0 1 -cy | =
                      | 0  0  1 |   | 0 0  1  |

                      | fx 0 cx-cx.fx |             | fx 0 cx.(1-fx) |
          = [x y 1] . | 0 fy cy-cy.fy | = [x y 1] . | 0 fy cy.(1-fy) |
                      | 0  0      1   |             | 0  0   1       |

and in tkp's matrix code:

          list [list $fx 0] [list 0 $fy] [list [expr {$cx*(1-$fx)}] [expr {$cy*(1-$fy)}]]
instead of:
          list [list $fx 0] [list 0 $fy] [list [expr {$cx*(1-$fx)}] [expr {$cy*($fy-1)}]]
                                                           sign inversion -------^
What do you think?


chw added on 2019-07-09 20:32:29:
Sorry, I have currently no spare brain cycles for this topic and
thus would prefer to receive and integrate ready and tested patches.

anonymous added on 2019-07-09 23:10:03:

The following patch fixes the issues documented above, i.e.:

  1. replaces all occurances of / 45.0 * atan(1) with * 0.017453292519943295, in tkpath.tcl, to increase performance
  2. fixes a wrong sign bug in transformation matrics for [tkp::matrix::flip] and [tkp::matrix::rotateflip], as documented above.
Tests cases, using the script from 2nd post in this ticket:
# flip
.c itemconfig t -matrix [tkp::matrix::flip 10.5 10.5  1  1]
.c itemconfig t -matrix [tkp::matrix::flip 10.5 10.5 -1  1]
.c itemconfig t -matrix [tkp::matrix::flip 10.5 10.5  1 -1]
.c itemconfig t -matrix [tkp::matrix::flip 10.5 10.5 -1 -1]

# rotateflip
.c itemconfig t -matrix [tkp::matrix::rotateflip 90d 10.5 10.5  1  1]
.c itemconfig t -matrix [tkp::matrix::rotateflip 90d 10.5 10.5 -1  1]
.c itemconfig t -matrix [tkp::matrix::rotateflip 90d 10.5 10.5  1 -1]
.c itemconfig t -matrix [tkp::matrix::rotateflip 90d 10.5 10.5 -1 -1]
The patch:
38c38
< 	set angle [expr {[string range $angle 0 end-1] / 45.0 * atan(1)}]
---
> 	set angle [expr {[string range $angle 0 end-1] * 0.017453292519943295}]
66a67
> 
69c70
< 	[list [expr {$cx*(1-$fx)}] [expr {$cy*($fy-1)}]]]
---
> 		[list [expr {$cx*(1-$fx)}] [expr {$cy*(1-$fy)}]]]
71a73
> 
94,95c96,97
<        	[expr {$myCos*$cx*(1.-$fx) - $mySin*$cy*($fy-1.) + $cx - $myCos*$cx + $mySin*$cy}] \
<         [expr {$mySin*$cx*(1.-$fx) + $myCos*$cy*($fy-1.) + $cy - $mySin*$cx - $myCos*$cy}] \
---
>        	[expr {$myCos*$cx*(1.-$fx) - $mySin*$cy*(1.-$fy) + $cx - $myCos*$cx + $mySin*$cy}] \
>         [expr {$mySin*$cx*(1.-$fx) + $myCos*$cy*(1.-$fy) + $cy - $mySin*$cx - $myCos*$cy}] \
107c109
< 	set angle [expr {[string range $angle 0 end-1] / 45.0 * atan(1)}]
---
> 	set angle [expr {[string range $angle 0 end-1] * 0.017453292519943295}]
119c121
< 	set angle [expr {[string range $angle 0 end-1] / 45.0 * atan(1)}]
---
> 	set angle [expr {[string range $angle 0 end-1] * 0.017453292519943295}]
183c185
< 	set a [expr {[string range $a 0 end-1] / 45.0 * atan(1)}]
---
> 	set a [expr {[string range $a 0 end-1] * 0.017453292519943295}]
200c202
< 	set a [expr {[string range $a 0 end-1] / 45.0 * atan(1)}]
---
> 	set a [expr {[string range $a 0 end-1] * 0.017453292519943295}]
dzach


chw added on 2019-07-10 04:35:07:
Patch applied in [d5b30d3ec3], thank you.

anonymous added on 2019-07-10 11:22:09:

It seems the patch was not applied correctly. Occurances of / 45.0 * atan(1) should have been replaced with * 0.017453292519943295, not / 0.017453292519943295

New patch:

38c38
< 	set angle [expr {[string range $angle 0 end-1] / 0.017453292519943295}]
---
> 	set angle [expr {[string range $angle 0 end-1] * 0.017453292519943295}]
83c83
< 	set angle [expr {[string range $angle 0 end-1] / 0.017453292519943295}]
---
> 	set angle [expr {[string range $angle 0 end-1] * 0.017453292519943295}]
107c107
< 	set angle [expr {[string range $angle 0 end-1] / 0.017453292519943295}]
---
> 	set angle [expr {[string range $angle 0 end-1] * 0.017453292519943295}]
119c119
< 	set angle [expr {[string range $angle 0 end-1] / 0.017453292519943295}]
---
> 	set angle [expr {[string range $angle 0 end-1] * 0.017453292519943295}]
183c183
< 	set a [expr {[string range $a 0 end-1] / 0.017453292519943295}]
---
> 	set a [expr {[string range $a 0 end-1] * 0.017453292519943295}]
200c200
< 	set a [expr {[string range $a 0 end-1] / 0.017453292519943295}]
---
> 	set a [expr {[string range $a 0 end-1] * 0.017453292519943295}]
dzach


chw added on 2019-07-10 13:06:33:
Hopefully fixed in [e95304b5ba], finally. Thanks.

anonymous added on 2019-07-10 15:06:07:
That did it. Thank you.