servo / euclid

Geometry primitives (basic linear algebra) for Rust
Other
462 stars 103 forks source link

Documentation of Transform2D does not reflect the actual calculation #523

Closed paxbun closed 2 months ago

paxbun commented 3 months ago

In the doc-comment of Transform2D, there is a matrix formula that describes the transformation performed by Transform2D:

https://github.com/servo/euclid/blob/6b2aece8d8bdd3e872603a3fe07d6bc438ae3d24/src/transform2d.rs#L51-L55

and this does not reflect the actual calculation, because Transform2D does not transform Point3D or Vector3D, so the w component in the result is always 1.

Furthermore, all matrices are transposed in the Transform2D or Transform3D documentation. Although it is mentioned that Transform2D or Transform3D use column-major order, I think putting transposed matrices without the transpose symbol is counter-intuitive for readers.

At least the following code must be represented as below.

https://github.com/servo/euclid/blob/6b2aece8d8bdd3e872603a3fe07d6bc438ae3d24/src/transform2d.rs#L546-L554

 | m11 m12 0 |T   |x|   |x'|
 | m21 m22 0 |  x |y| = |y'|
 | m31 m32 1 |    |1|   |1 |
nical commented 3 months ago

Good points. I think that the doc should read:

| m11 m21 m31 |   |x|   |x'|
| m12 m22 m32 | x |y| = |y'|
|   0   0   1 |   |1|   |1 |