Add Transform3d for homogeneous layer transforms - #590
Conversation
|
This PR was done with assistance from Codex (5.5, xhigh). |
DJMcNab
left a comment
DJMcNab
left a comment
There was a problem hiding this comment.
As an implementation of this feature, this is unobjectionable. I do have a few nits, but none are especially blocking. I don't feel like I'm especially qualified to decide whether we want this (this is the kind of thing I miss having a strong office hours for). I definitely don't want to make that decision at the end of a workday.
I do agree that as a project, Linebender does need a way to talk about 3d transforms in 2d rendering. I don't want this to sound like I'm trying to blackhole this feature, but part of me wonders if it would actually belong in Peniko (as it's more of a 2d graphics than 2d shapes thing). For the sake of keeping moving forward, I'm definitely not against landing this here.
@tomcur do you have any thoughts on this conceptually?
|
|
||
| /// Construct a transform from its four columns. | ||
| /// | ||
| /// Each column is `[x, y, z, w]`. |
There was a problem hiding this comment.
Could you expand on this comment? The column is the contribution of their respective element to the final result, right? So when you say x in the doc comment, it's the contribution of the element corresponding to the column number to the resulting x?
There was a problem hiding this comment.
Adjusted in next push.
| self.0[i] | ||
| } | ||
|
|
||
| /// Construct the 3D transform with the same effect as a 2D [`Affine`]. |
There was a problem hiding this comment.
I'd probably add the "in the xy plane" here, but I think we could get away without it.
| /// Panics if `i >= 4`. | ||
| #[inline] | ||
| #[must_use] | ||
| pub const fn col(self, i: usize) -> [f64; 4] { |
There was a problem hiding this comment.
Is there a use for a row function?
There was a problem hiding this comment.
There is, for matrix inspection but also to get the bottom / perspective row. Added in next push.
| let c = self.0; | ||
| c[0][0] == 1.0 | ||
| && c[0][1] == 0.0 | ||
| && c[0][2] == 0.0 | ||
| && c[0][3] == 0.0 | ||
| && c[1][0] == 0.0 | ||
| && c[1][1] == 1.0 | ||
| && c[1][2] == 0.0 | ||
| && c[1][3] == 0.0 | ||
| && c[2][0] == 0.0 | ||
| && c[2][1] == 0.0 | ||
| && c[2][2] == 1.0 | ||
| && c[2][3] == 0.0 | ||
| && c[3][0] == 0.0 | ||
| && c[3][1] == 0.0 | ||
| && c[3][2] == 0.0 | ||
| && c[3][3] == 1.0 |
There was a problem hiding this comment.
Could we use self == Self::IDENTITY?
There was a problem hiding this comment.
const fn in the workspace MSRV, I think.
There was a problem hiding this comment.
It might be possible to do self.0 == IDENTITY.0, maybe?
Add `Transform3d` as a column-major 4 by 4 homogeneous transform in `kurbo`, with CSS/`DOMMatrix` coefficient order and multiplication semantics matching `Affine`. The initial API covers construction from coefficients and columns, `Affine` and `TranslateScale` conversion, exact 2D and 3D-affine classification, CSS-style perspective, homogeneous point transformation, and projection with `w > 0` handling. This intentionally leaves full projective `inverse` out of the first slice, avoiding an affine-only inverse on a type that can represent perspective.
343a8fd to
f0f28be
Compare
2 participants
Add
Transform3das a column-major 4 by 4 homogeneous transform inkurbo, with CSS/DOMMatrixcoefficient order and multiplication semantics matchingAffine.The initial API covers construction from coefficients and columns,
AffineandTranslateScaleconversion, exact 2D and 3D-affine classification, CSS-style perspective, homogeneous point transformation, and projection withw > 0handling.This intentionally leaves full projective
inverseout of the first slice, avoiding an affine-only inverse on a type that can represent perspective.