Add inverse method to AffineTransform

[urschrei]

• I agree to follow the project's code of conduct.
• I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

---

This is technically breaking due to the addition of an additional trait bound (Neg). In practice, it's unlikely to affect library consumers.

[michaelkirk]

This is clearly useful. I had one take-it-or-leave it thing, but LGTM.

[michaelkirk]

These T::from are kind of unfortunate.

To get rid of them, we could constrain these newly added Neg's to instead be Neg<Output=T>?

Or do we actually want to support Neg w/ other Outputs? I don't think it really even occurred to me before that you could specify the output of Neg. I'm guessing it's for when you want to do something like:

// unsigned
impl Neg for MyUnsignedNumber {
    // output of negation can be negative, so output is signed
   type Output= MySignedNumber;
    ...
}

maybe that's why? I have no idea.

Anyway, if you'd rather keep it, it's NBD. I wouldn't be super surprised if it's all optimized away anyway. 😅

[lnicola]

Do we actually need to add these Neg bounds?

[urschrei]

In some form, because they're needed by the arithmetic that produces the determinant (according to the compiler).
But I haven't tried @michaelkirk's suggestion (see his previous comment) yet.

[lnicola]

But it's only needed for inverse, right? It still seems to build if I split out inverse into its own impl block and drop the new Neg bounds from everywhere else:

impl<T: CoordNum + Neg> AffineTransform<T> {
    /// Return the inverse of a given transform. Composing a transform with its inverse yields
    /// the [identity matrix](Self::identity)
    pub fn inverse(&self) -> Option<Self>
    where
        <T as Neg>::Output: Mul<T>,
        <<T as Neg>::Output as Mul<T>>::Output: ToPrimitive,
    {
        let a = self.0[0][0];
        let b = self.0[0][1];
        let xoff = self.0[0][2];
        let d = self.0[1][0];
        let e = self.0[1][1];
        let yoff = self.0[1][2];

        let determinant = a * e - b * d;

        if determinant == T::zero() {
            return None; // The matrix is not invertible
        }

        let inv_det = T::one() / determinant;
        Some(Self::new(
            e * inv_det,
            T::from(-b * inv_det).unwrap(),
            (b * yoff - e * xoff) * inv_det,
            T::from(-d * inv_det).unwrap(),
            a * inv_det,
            (d * xoff - a * yoff) * inv_det,
        ))
    }
}

PS: should probably be #[must_use] too.

[michaelkirk]

Both of these are good points!

[michaelkirk]

Though I'd recommend doing:

impl<T: CoordNum + Neg<Output=T>> AffineTransform<T> {

[urschrei]

But it's only needed for inverse, right? It still seems to build if I split out inverse into its own impl block and drop the new Neg bounds from everywhere else:

impl<T: CoordNum + Neg> AffineTransform<T> {

    /// Return the inverse of a given transform. Composing a transform with its inverse yields

    /// the [identity matrix](Self::identity)

    pub fn inverse(&self) -> Option<Self>

    where

        <T as Neg>::Output: Mul<T>,

        <<T as Neg>::Output as Mul<T>>::Output: ToPrimitive,

    {

        let a = self.0[0][0];

        let b = self.0[0][1];

        let xoff = self.0[0][2];

        let d = self.0[1][0];

        let e = self.0[1][1];

        let yoff = self.0[1][2];



        let determinant = a * e - b * d;



        if determinant == T::zero() {

            return None; // The matrix is not invertible

        }



        let inv_det = T::one() / determinant;

        Some(Self::new(

            e * inv_det,

            T::from(-b * inv_det).unwrap(),

            (b * yoff - e * xoff) * inv_det,

            T::from(-d * inv_det).unwrap(),

            a * inv_det,

            (d * xoff - a * yoff) * inv_det,

        ))

    }

}

PS: should probably be #[must_use] too.

Ahh, it didn't occur to me to split it into its own impl. This is much better I think.

[lnicola]

We could also return None if the cast fails..

[michaelkirk]

Oh and breaking out this impl means it's no longer a breaking change, nice!