On Wed, 29 Jul 2020 19:02:16 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:

>> Nir Lisker has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Corrected javadoc generation errors
>
> modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 5523:
> 
>> 5522:      * Defines the {@code ObservableList} of {@link Transform} objects 
>> to be applied to this {@code Node}. The
>> transforms in this list 5523:      * are applied in the <i>reverse</i> order 
>> of which they are specified as per matrix
>> multiplication rules. This list of transforms 5524:      * is applied before 
>> scaling ({@link #scaleXProperty scaleX},
>> {@link #scaleYProperty scaleY} and {@link #scaleZProperty scaleZ}),
> 
> `The transforms in this list are applied in the <i>reverse</i> order of which 
> they are specified as per matrix
> multiplication rules`
> To apply transformations in a specific order their respective matrices should 
> be multiplied in reverse order. So just
> the order of multiplication is reverse but the transformation are applied in 
> same order as they are added into the
> `getTransforms()`. I think phrasing it as 'the transforms are applied in 
> reverse order...' would not be accurate and
> confusing to reader.

In reading the existing doc carefully, it is both wrong and confusing. It has 
the wrong ordering of transform, scale
and rotate. It is also confusing given that no distinction is made between 
matrix multiplication order and the observed
effect on the geometry of the node. You have fixed the part about it being 
wrong, but the confusion remains, especially
since you made a point of saying that the transforms list is applied in the 
reverse order.

The matrices are multiplied in the following order:

1. layoutX/Y + translateX/Y/Z
2. rotate
3. scale
4. transforms[0]
5. transforms[1]
...

We need to both list the actual matrix multiplication order, and then describe 
that the object is transformed from
object coordinates to local coordinates of the node, to parent coordinated, 
etc., by first applying the transforms in
the list in reverse order, then scale, rotate, translate+layout. I don't think 
that the `transforms` list is the place
to do that (it belongs in the Transformations section).

This doesn't seem like something we can sort out for JavaFX 15, so I would 
split this out and defer it.

-------------

PR: https://git.openjdk.java.net/jfx/pull/276

Reply via email to