Affine transforms - matrix algebra

[Pavel Safrata]

Hi Martin,
thank you for your input.

On 10.7.2012 18:36, Martin Desruisseaux wrote:
> Hello Pavel
>
> Many thanks for taking care of this task!
>
>
> Le 10/07/12 17:00, Pavel Safrata a écrit :
>> On Transform class:
>>     public Transform getConcatenation(Transform transform) // 
>> multiplication
>>     public Transform getInverse() throws 
>> NoninvertibleTransformException // negation
>>     public Transform copy()
> Sound fine to me, while I'm not sure why a 'copy' method instead than 
> overriding the 'clone()' method?

Good point, we can use clone().

>
>
>> Constructors:
>>     public Affine(Transform transform)
>>     public Affine(double mxx, double mxy, double mxz, double tx,
>>             double myx, double myy, double myz, double ty,
>>             double mzx, double mzy, double mzz, double tz)
> Look fine.
>
>
>> Setters of the entire matrix:
>>     (...snip...)
> I don't know for JavaFX, but in my experience with Java2D, I wasn't 
> using the setter methods often, except 'setToIdentity' and 
> 'setToTransform'. For example rather than invoking 'setToTranslation', 
> I strongly push our developers to use 'translate' (or 
> 'concatWithTranslation' in this proposal) instead. If a developer 
> really wants the functionality of 'setToTranslation', he can get it by 
> invoking 'setToIdentity()' followed by 'concatWithTranslation'. Or yet 
> better, 'setToTransform(...)' instead than 'setToIdentity' with the 
> coefficients of some previous state that the user saved.

I don't insist on having the setters, anybody wants them?

>
> The rational is that in many cases, the affine transform is already 
> initialized to some important value. For example in Java2D, 
> AffineTransform is initialized to the transform from 'dot' to whatever 
> units the underlying device uses. When rendering on screen, this is 
> the identity transform. But when printing, this is something different 
> that depends on the printer resolution. In GIS applications, it 
> depends on the zoom level. Other applications may use magnifier glass 
> over some areas. Because the initial transform is often (but not 
> always) the identity one, developers with limited experience with 
> affine transforms often use 'setTranslate' or 'setScale' in situations 
> where they should really use 'translate' or 'scale', and do not 
> realize their bug before late in the development process. For this 
> reason, I would be inclined to discourage every setter methods except 
> 'setToIdentity()' and 'setToTransform'. Keeping in mind that it is 
> often easier to fill a hole later than to fix something broken, I 
> think it would be safer to leave out all other setter methods for now, 
> and revisit later if experience show that they are really needed.
>

While I don't think our Affine class will ever have varying initial 
values I'm ok with keeping only setToIdentity() and setToTransform() if 
there is no demand for the other setters right now.

>
>> Operations on the matrix (modifying it in place):
>>     (...snip...)
> Sound good, minus the unfortunate 'concatWith*' naming :-(.
>
>
>> Instead of "concatWithTranslation" it would be more natural to use 
>> just "translate" (and similarly for the others), but unfortunately 
>> these method names are already taken by the static factory methods on 
>> Transform class. This is unpleasant but we need to be backward 
>> compatible so we have to introduce different names. We'll be happy to 
>> hear better naming suggestions than the concatWith* (which is pretty 
>> descriptive I think but not really nice).
> Hard to say... A consistency with "preTranslate" would be nice, but 
> "postTranslate" doesn't look very nice... What about the following?
>
> * Rename "preTranslate" as "appendTranslation"
> * Rename "concatWithTranslation" as "prependTranslation"
>
> The "preConcatenate(Tx)" name in Java2D was actually misleading to 
> some developers, because it works as if points were first transformed 
> by the original transform, then transformed by 'Tx'. Maybe 
> "appendTranslation(Tx)" would make clear that the translation is 
> applied after the original transform. This would also make operation 
> order clearer. The following code using Java2D API:
>
> tr.translate(...)
> tr.scale(...)
> tr.rotate(...)
>
> must be read from bottom to up: it is as if points were rotated first, 
> then scaled, then translated. So maybe the above proposition would 
> make that more obvious:
>
> tr.prependTranslation(....)
> tr.prependScale(...)
> tr.prependRotate(...)
> tr.appendTranslation(...) // Just for fun.
>

These names indeed sound way better.

If we are going this way, shouldn't we do also this?
* Rename 'concatenate' as 'prepend'
* Rename 'preConcatenate' as 'append'
* Rename 'getConcatenation' as .. well .. 'getPrependage' :-)

As a black-box transformation composing this makes sense. As a matrix 
algebra, it may be also confusing: tA.append(tB) means matrix 
multiplication 'B x A', does it sound good? Maybe yes, I'm not sure. 
Anyway, so far the best proposal I think.

>
>> Would you want static factory methods on Affine (creating Affine 
>> instances with the simple transforms)?
> I don't think it is necessary. I found the static factory methods of 
> Java2D AffineTransform rarely used.
>
>> Would it be important to you whether or not matrix changes are 
>> atomic? If you call one of the methods that modify the entire matrix, 
>> can be listeners for each member called immediately as the members 
>> are set, or do they need to be called after all the members are updated?
> I don't have experience in this area. But naively, it seems to we that 
> it would be better to be notified only after the full matrix has been 
> updated...

Yes, I would also pick this option. It's just that it would mean writing 
much more complicated (and a bit less effective) code so the question is 
whether or not you think it is important.

Thanks,
Pavel

>
>     Regards,
>
>         Martin
>