Implement some missing convenience traits. #200
Conversation
|
r? @kvark |
src/transform3d.rs
Outdated
| } | ||
| } | ||
|
|
||
| impl<T: Copy, Src, Dst> Into<[T; 16]> for TypedTransform3D<T, Src, Dst> { |
There was a problem hiding this comment.
weird. Why is 2D transform just 6 floats but 3D is full 16?
There was a problem hiding this comment.
2D transforms don't need to implement perspective or other weird operations, so you only need the 2x2 matrix which can encode the scale and rotation, plus the two components that have the transform this gives you a 3x3 matrix where all of the missing components are zero.
3D transforms have a richer set of operation that require all of the components of the 4x4 matrix.
There was a problem hiding this comment.
don't you see it somewhat inconsistent? it's almost asking for AffineTransform3D being different from just Transform3D.
There was a problem hiding this comment.
In practice, most 2d rendering libraries (cairo, skia, moz2d, etc.) store 2d transforms the way we do, so using arrays of 6 elements is what's most consistent in a useful way.
If we added the implicit components we could serialize but not deserialize since we would not know what to do with the extra values if they are not zero.
|
Another concern is the choice of |
That comes down to personal taste, I have a tendency to favor flat arrays because it subjectively feels simpler, but I an see why some may want to structure that into row arrays. I don't see a reason to not do both, so I added |
nical
changed the title
| } | ||
|
|
||
| impl<T: Hash, U> Hash for TypedRect<T, U> | ||
| { |
There was a problem hiding this comment.
style nit: opening bracket on the new line?
To be honest, I was thinking that since it's not obvious, we should rather not provide any conversions. |
|
@bors-servo r+ |
|
📌 Commit 0e9bf89 has been approved by |
Implement some missing convenience traits. Implement `From<[T; N]>` and `Into<[T; N]>` for points and vectors, Implement Hash for rects, and add some helper methods to convert transforms from and to arrays. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/euclid/200) <!-- Reviewable:end -->
|
💔 Test failed - status-travis |
|
@bors-servo retry |
1 similar comment
|
@bors-servo retry |
Implement some missing convenience traits. Implement `From<[T; N]>` and `Into<[T; N]>` for points and vectors, Implement Hash for rects, and add some helper methods to convert transforms from and to arrays. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/euclid/200) <!-- Reviewable:end -->
|
☀️ Test successful - status-travis |
4 participants
Implement
From<[T; N]>andInto<[T; N]>for points and vectors, Implement Hash for rects, and add some helper methods to convert transforms from and to arrays.This change is