Move transform_rect, transform_point, tranform_vector to the corresponding objects

[pbor]

Transfrorm2D has transform_rect, transform_point, tranform_vector methods.

This is kind of surprising at least to me, I would have expected to have a transform method on Rect, Point and Vector.

On the other hand, this means that we need separate methods for transform2D and 3D.

For context: I am raising these issues as I look at porting librsvg to euclid and right now I was coverting away from to Rect extension trait which has a tranform method. If you do not think this API bikeshedding is useful, feel free to say so.

[nical]

I'm open to bikeshedding the API, of course. In this particular case it feels more natural to me to have the methods on the transform rather than the rect/point/etc for a usage point of view (but I don't have any particular argument to back that, other than that French and English tend to go in the subject-verb-complement direction rather than the opposite order that you suggest, but that's a matter of where one is from), and (more importantly) I want that the matrix/rotation/scale/etc code to transform the various objects stays near the rest of the matrix/rotation/scale/etc code.

If we wanted have methods on rects points and friends that take the transforms we would have them just be functions that call into Transform2D::transform_point etc.

On the other hand, this means that we need separate methods for transform2D and 3D.

We don't only have Transform2D and Transform3D, we also have 2D and 3D types to represent translations, rotations, scales, and rigid transforms. So that mean either add a function for each of them (that's quite a few combinations and names to add), or we'd need a trait for 2D and 3D transforms, something like:

// Implemented by Transform2D, Transform3D, RigidTransform3D, Rotation2D, Rotation3D, etc.
pub trait Transform<Src, Dst> {
    // Transforming other types of objects can be built on top of these:
    fn transform_point2d(&self, p: Point2D<Src>) -> Point2D<Dst>;
    fn transform_point3d(&self, p: Point3D<Src>) -> Point3D<Dst>;
    fn transform_vector2d(&self, p: Vector2D<Src>) -> Vector2D<Dst>;
    fn transform_vector3d(&self, p: Vector3D<Src>) -> Vector3D<Dst>;
    fn transform_homogeneous_vector(&self, p: HomogeneousVector<Src>) -> HomogeneousVector<Dst>;
}

// In Point2D:
pub fn transform<Dst, Tx: Transform<U, Dst>>(&self, transform: &Tx) -> Point2D<T, Dst> {
     transform.transform_point2d(self)
}

That would add quite a bit of boilerplate for something that we already have. Perhaps a bit much just to have a "rect.transform(&projection)" syntax instead of "projection.transform_rect(&rect)". The reason I'm not actually fully opposed to that (despite what I wrote in the other issue about adding traits), is that I would like to be able to apply simple transforms like rotations to paths and curves in lyon without having to convert to a full matrix beforehand (which is what happens right now, short of having an abstraction over transforms), and without having to implement the same code for all types of transformations.

Now if such a Transform trait were to see the light of day, where would the line be? Should it have methods like is_2d(), invert(), to_transform2D(), etc. ? Maybe at this point.

[pbor]

Thanks for the reply.

My expectation is due to the fact that usually methods operate on the object, but yeah, this is mostly due to the linguistic background one comes from.

I can see the benefit of keeping all the code in transform2d.rs, on the other hand the benefit of moving to rect.rs etc is that you do not need for transform2d.rs to depend on the other objects. So for instance if one day we need to add a Triangle object, you would be able to implement Triangle::transform() with no changes to transform2d.rs. Also if one day we need to factor out a trait like

trait Transformable {
   fn transform2D(t: &Transform);
   fn transform3D(t: &Transform);
}

then we could implement that for all the shapes.

[pbor]

Also, fwiw I already refactored the librsvg code in preparation:

https://gitlab.gnome.org/GNOME/librsvg/commit/ef685ba2634914d27e1368fa0529d2430dba6f80

Feel free to close this

[nical]

trait Transformable {
   fn transform2D(t: &Transform);
   fn transform3D(t: &Transform);
}

then we could implement that for all the shapes.

This trait would maybe serve a different purpose, however:

• Vectors, points and friends are tagged with vector spaces in euclid, so that would have to appear in the signature somehow, to enforce that transforms only take points in a given space and output points in a (potentially different) given space.
• If shapes implement this they can only be efficiently transformed by one kind of transform (presumably a matrix). Only the transform type (is it a matrix, a quaternion, a simple translation vector?) can know the minimal amount of work needed to transform the points, unless you mean it in addition to the Transform<Src, Dst> trait I mentioned earlier.
• While I have a use case for abstracting over the kind of transform and run the most fastest code to transform shapes depending on it, it's unclear to me what the value of being able to abstract over the kinds of transformed object is.

[pbor]

Note that the trait I wrote there was just for illustrative purposes, if we ever did that maybe the signatures would be different, eg apply_matrx, apply_vector, which would allow to have the right implementation.

The reasons why I thought the trait (or even simply the method on the object) is useful are:

• ease refactoring: for instance if one replaces Rect with Box, the code which calls transform would not change
• ease composition: if I make a composed shape, for instance a bounding box with inner and outer rects, it could have a transform method without having to extend Transform2D
• if it is plausible to have collections of shapes to be transformed

Anyway, I think there are pros and cons for both approaches. I am going to close this for now, thanks!