Affine transforms - matrix algebra

[Richard Bair]

Hi Pavel,

Here are my thoughts:

> New NoninvertibleTransformException

I believe it should be NonInvertibleTransformException with a capital "I".

> New TransformChangedEvent with single event type TRANSFORM_CHANGED

This event goes on the Transform object itself, and would replace our usage of impl_add, impl_remove and such. Is that right?

> On Point2D:
>    // similar to Point3D mapped to 2D (except of crossProduct still returning Point3D)

Do Point2D and Point3D have a common base class?

>    public Transform clone()
>        // Of course Transform implementing Cloneable

Why does it have to implement clone?

>    public Point3D deltaTransform(Point3D point)
>    public Point2D deltaTransform(Point2D point)
>        // transformation without translation (transformation of a vector)
>    public Point3D inverseDeltaTransform(Point3D point)
>    public Point2D inverseDeltaTransform(Point2D point)
>        // inverse transformation without translation

I haven't followed the entire thread, but this was the first part of the API who's meaning wasn't immediately obvious. What is a delta transform, and what is it used for?

>    public double determinant()

Why isn't this a getter?

>    public boolean is2D()
>    public boolean is3D()
>    public boolean isIdentity()

All four of these, determinant, is2D, is3D, and isIdentity can all (in theory) change their value as the Transform is changed, is that right? In that case, they should be read only properties.

>    public double getElement(MatrixType type, int row, int column)
> 
> On Affine class:
> 
>    public Affine(double[] array, MatrixType type, int beginIndex)

In some methods in Transform, MatrixType is first, but in Affine, it is second with the array being the first parameter. I think I would put the array as the last parameter and have MatrixType first. The reason is that this code:

new Affine(new double[] {
    1, 1, .5, 1,
    3, 2, 1, 0,
    1, 1, .5, 1,
    3, 2, 1, 0}, MatrixType.MT_3D_4x4, 0);

is uglier than:

new Affine(MatrixType.MT_3D_4x4, 0, new double[] {
    1, 1, .5, 1,
    3, 2, 1, 0,
    1, 1, .5, 1,
    3, 2, 1, 0});

It is a minor thing but it brings peace to the heart :-). Of course with the array as the last parameter, it also means it could be a double... if you wanted to, but I'm not sure it matters.

>    public void appendTranslation(double tx, double ty)
>    public void appendTranslation(double tx, double ty, double tz)
>    public void appendScale(double sx, double sy)
>    public void appendScale(double sx, double sy, double pivotX, double pivotY)
>    public void appendScale(double sx, double sy, Point2D pivot)
>    public void appendScale(double sx, double sy, double sz)
>    public void appendScale(double sx, double sy, double sz, Point3D pivot)
>    public void appendRotation(double angle)
>    public void appendRotation(double angle, double pivotX, double pivotY)
>    public void appendRotation(double angle, Point2D pivot)
>    public void appendRotation(double angle,
>                               double pivotX, double pivotY, double pivotZ,
>                               double axisX, double axisY, double axisZ)
>    public void appendRotation(double angle,
>                               double pivotX, double pivotY, double pivotZ,
>                               Point3D axis)
>    public void appendRotation(double angle, Point3D pivot, Point3D axis)
>    public void appendShear(double shx, double shy)
>    public void appendShear(double shx, double shy, double pivotX, double pivotY)
>    public void appendShear(double shx, double shy, Point2D pivot)

>    public void prepend* // 20 methods

I don't know that it is worth it to add these 32 methods. Somebody trying to eek performance and skilled in the art can just use the long form append method, and somebody who is just hacking away where performance isn't a concern can do:

t.append(Transform.translate(5, 10));

It create a short-lived object, yes, but if that bothered me I could instead use the append(one billion parameters) method call instead. Do you think having these convenience methods is compelling enough to justify their addition?

Thanks
Richard