Discussion:
[jira] [Created] (GEOMETRY-14) AffineTransform?D Classes
Matt Juntunen (JIRA)
2018-09-07 02:00:00 UTC
Permalink
Matt Juntunen created GEOMETRY-14:
-------------------------------------

Summary: AffineTransform?D Classes
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen


We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Matt Juntunen (JIRA)
2018-09-23 19:09:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16625232#comment-16625232 ]

Matt Juntunen commented on GEOMETRY-14:
---------------------------------------

Working on this here: https://github.com/darkma773r/commons-geometry/tree/transforms
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
ASF GitHub Bot (JIRA)
2018-10-03 03:38:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16636381#comment-16636381 ]

ASF GitHub Bot commented on GEOMETRY-14:
----------------------------------------

darkma773r opened a new pull request #14: GEOMETRY-14: Initial AffineTransform3D class
URL: https://github.com/apache/commons-geometry/pull/14


This is an initial merge request for the work on GEOMETRY-14, primarily to gain feedback on the new transform architecture in 3D space before implementing for other dimensions. Rotations are not yet included since that will involve restructuring of the Rotation class as well.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
ASF GitHub Bot (JIRA)
2018-10-03 03:38:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

ASF GitHub Bot updated GEOMETRY-14:
-----------------------------------
Labels: pull-request-available (was: )
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Matt Juntunen (JIRA)
2018-10-03 03:39:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Matt Juntunen updated GEOMETRY-14:
----------------------------------
Description:
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.

 

Pull Request #1: https://github.com/apache/commons-geometry/pull/14

was:We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Matt Juntunen (JIRA)
2018-10-03 03:40:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16636383#comment-16636383 ]

Matt Juntunen commented on GEOMETRY-14:
---------------------------------------

I just submitted a partial pull request for this. I'm primarily interested in getting feedback, so we don't necessarily need to merge this one in if it's not ready.
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Gilles (JIRA)
2018-10-05 14:52:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16639906#comment-16639906 ]

Gilles commented on GEOMETRY-14:
--------------------------------

I didn't look in detail; here are a few remarks:
* The name {{multiply}} seems strange; wouldn't {{compose}} be more appropriate?
* {{tranform.apply(v)}} looks more natural than {{v.applyt(transform)}}.
* Incremental building from the unit transform seems fine
* Doesn't {{createTranslation}} duplicate {{translate}}?
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Matt Juntunen (JIRA)
2018-10-06 02:19:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16640527#comment-16640527 ]

Matt Juntunen commented on GEOMETRY-14:
---------------------------------------

{quote}The name {{multiply}} seems strange; wouldn't {{compose}} be more appropriate?
{quote}
Yes, {{multiply}} does seem to expose the underlying matrix implementation. We could do {{compose}} but then I think the API would be a bit more awkward, assuming that by {{A.compose(B)}} we mean "perform B then perform A". For example, if we want to perform a series of transforms in order {{A}}, {{B}}, {{C}}, and then {{D}}, we would need to do {{D.compose(C.compose(B.compose(A)))}}. If we rename it to {{apply}} and define {{A.apply(B)}} as "take A and perform B on it", then we can write the same series of transforms a {{A.apply(B).apply(C).apply(D)}}. I feel like that's cleaner and better matches the behavior of the other methods, like {{translate}}.
{quote}{{tranform.apply(v)}} looks more natural than {{v.applyt(transform)}}.
{quote}
I used {{transform.applyTo(vec)}} since that's typically how one talks about transforms operating, ie, they are applied _to_ points and vectors. The vector and point classes have corresponding {{vec.apply(transform)}}.
{quote}Doesn't {{createTranslation}} duplicate {{translate}}?
{quote}
They are similar but slightly different. {{translate}} is an instance method and applies a translation to the current transform. This involves a matrix multiplication. {{createTranslation}} is a static factory method for creating a brand new transform that performs a translation. The matrix for this can be created directly; no matrix multiplication is involved. I would have liked to have given them the same name but then the methods would conflict.
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Gilles (JIRA)
2018-10-06 11:19:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16640689#comment-16640689 ]

Gilles commented on GEOMETRY-14:
--------------------------------

{quote}the API would be a bit more awkward, assuming that by A.compose(B) we mean "perform B then perform A"
{quote}
It's not what I'd assume; so I see the possible confusion. ;)
Couldn't we leverage the [JDK API|https://docs.oracle.com/javase/8/docs/api/java/util/function/UnaryOperator.html]? I.e. have something like
{code:java}
public class AffineTransform3D implements UnaryOperator<P extends Point3D>
{code}
{quote}{{transform.applyTo(vec)}} [because] they are applied _to_ points and vectors.
{quote}
Yes, but {{apply(vec)}} is as clear and more concise, and now favored by the JDK.
{quote}The vector and point classes have corresponding {{vec.apply(transform)}}
{quote}
Usual question: Why adding to the API?
Furthermore, this one is fairly counter-intuitive: to be consistent it should have been {{vec.isAppliedToBy(transform)}}.
{quote}translate is an instance method and applies a translation to the current transform.
{quote}
Does that mean that the class is mutable? :(
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Matt Juntunen (JIRA)
2018-10-06 19:15:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16640846#comment-16640846 ]

Matt Juntunen commented on GEOMETRY-14:
---------------------------------------

{quote}Yes, but {{apply(vec)}} is as clear and more concise, and now favored by the JDK.
{quote}
That makes sense. I think the naming of these methods is going to be critical to the ease of use of the API. How about this: the transform classes follow the JDK convention and have {{apply}} methods, eg {{transform.apply(vec)}} and {{transform.apply(pt)}}. We define _apply_ here to mean "take the calling transform and apply it to the given argument." For combining/chaining transforms, we use a _transform_ method, which we define as meaning "take the calling object and transform it with the argument." We would then have {{A.transform(B)}} mean "transform A with B", which would give us the correct order of operations, eg {{A.transform(B).transform(C)}} would logically perform A, then B, then C. We could also use this same definition of _transform_ in the vector/point classes. For example, {{vec.transform(t)}} means "transform vec with t."

 
{quote}Usual question: Why adding to the API?
{quote}
I want transforms to be able to be applied inline with other vector/point methods. For example, {{p1.vectorTo(p2).transform(t).normalize()}} instead of {{t.transform(p1.vectorTo(p2)).normalize()}}, which is more difficult to read.
{quote}Does that mean that the class is mutable?
{quote}
It is immutable from outside of the class. All of the methods like {{translate}} return new instances. I do not have the private fields set as final so that I can do things like copying the results of internal computations back into an existing instance.

 
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Matt Juntunen (JIRA)
2018-10-06 19:16:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16640846#comment-16640846 ]

Matt Juntunen edited comment on GEOMETRY-14 at 10/6/18 7:15 PM:
----------------------------------------------------------------

{quote}Yes, but {{apply(vec)}} is as clear and more concise, and now favored by the JDK.
{quote}
That makes sense. I think the naming of these methods is going to be critical to the ease of use of the API. How about this: the transform classes follow the JDK convention and have {{apply}} methods, eg {{transform.apply(vec)}} and {{transform.apply(pt)}}. We define _apply_ here to mean "take the calling transform and apply it to the given argument." For combining/chaining transforms, we use a _transform_ method, which we define as meaning "take the calling object and transform it with the argument." We would then have {{A.transform(B)}} mean "transform A with B", which would give us the correct order of operations, eg {{A.transform(B).transform(C)}} would logically perform A, then B, then C. We could also use this same definition of _transform_ in the vector/point classes. For example, {{vec.transform(t)}} means "transform vec with t."

 
{quote}Usual question: Why adding to the API?
{quote}
I want transforms to be able to be applied inline with other vector/point methods. For example, {{p1.vectorTo(p2).transform(t).normalize()}} instead of {{t.apply(p1.vectorTo(p2)).normalize()}}, which is more difficult to read.
{quote}Does that mean that the class is mutable?
{quote}
It is immutable from outside of the class. All of the methods like {{translate}} return new instances. I do not have the private fields set as final so that I can do things like copying the results of internal computations back into an existing instance.

 


was (Author: mattjuntunen):
{quote}Yes, but {{apply(vec)}} is as clear and more concise, and now favored by the JDK.
{quote}
That makes sense. I think the naming of these methods is going to be critical to the ease of use of the API. How about this: the transform classes follow the JDK convention and have {{apply}} methods, eg {{transform.apply(vec)}} and {{transform.apply(pt)}}. We define _apply_ here to mean "take the calling transform and apply it to the given argument." For combining/chaining transforms, we use a _transform_ method, which we define as meaning "take the calling object and transform it with the argument." We would then have {{A.transform(B)}} mean "transform A with B", which would give us the correct order of operations, eg {{A.transform(B).transform(C)}} would logically perform A, then B, then C. We could also use this same definition of _transform_ in the vector/point classes. For example, {{vec.transform(t)}} means "transform vec with t."

 
{quote}Usual question: Why adding to the API?
{quote}
I want transforms to be able to be applied inline with other vector/point methods. For example, {{p1.vectorTo(p2).transform(t).normalize()}} instead of {{t.transform(p1.vectorTo(p2)).normalize()}}, which is more difficult to read.
{quote}Does that mean that the class is mutable?
{quote}
It is immutable from outside of the class. All of the methods like {{translate}} return new instances. I do not have the private fields set as final so that I can do things like copying the results of internal computations back into an existing instance.

 
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Gilles (JIRA)
2018-10-06 23:17:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16640904#comment-16640904 ]

Gilles commented on GEOMETRY-14:
--------------------------------

{quote}A.transform(B).transform(C) would logically perform A, then B, then C.
{quote}
With [UnaryOperator|https://docs.oracle.com/javase/8/docs/api/java/util/function/UnaryOperator.html], this would be expressed either as
{code:java}
A.andThen(B).andThen(C)
{code}
or as
{code:java}
C.compose(B).compose(A)
{code}
Using it will bring all the advantages of a standard API.
{quote}{{t.apply(p1.vectorTo(p2)).normalize()}} is more difficult to read.
{quote}
Maybe, but caution still applies for anything that goes into the public API. The fluent API is nice but we should explore the alternatives.
{quote}I do not have the private fields set as final so that I can do things like copying
{quote}
This is a can of worms, as it will prevent any simple implementation of multi-threaded processing.
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Matt Juntunen (JIRA)
2018-10-07 05:03:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16640970#comment-16640970 ]

Matt Juntunen commented on GEOMETRY-14:
---------------------------------------

{quote}Using it will bring all the advantages of a standard API.
{quote}
I agree with using the standard API here. However, there are a couple of slight issues.
# The transform classes are used to transform both points and vectors, but we can only choose one to be the type for UnaryOperator. It should probably be points, but it seems weird to leave vectors out like that.
# The {{andThen}} and {{compose}} methods from {{java.util.Function}} don't really provide the behavior we want when chaining transforms. The JDK versions return new functions that invoke the original ones in the correct order. What we want in our method is to multiply the transform matrices together to create a new matrix that mathematically performs the same operations as if the original matrices were applied in order. So, the end result is the same but the internals (and performance) are quite different. I think we need a separate method name to convey this concept. I previously proposed {{transform}} but some other options might be {{build}}, {{combine}}, ...

{quote}Maybe, but caution still applies for anything that goes into the public API. The fluent API is nice but we should explore the alternatives.
{quote}
Yes. I do quite like it, though, and I think it will get some good use. Did you have any other alternatives in mind?
{quote}This is a can of worms, as it will prevent any simple implementation of multi-threaded processing.
{quote}
I've refactored this in my working branch so that the classes are truly immutable, ie all fields are final.
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Gilles (JIRA)
2018-10-07 11:13:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16641031#comment-16641031 ]

Gilles commented on GEOMETRY-14:
--------------------------------

{quote}The transform classes are used to transform both points and vectors
{quote}
What are the different use-cases?
Having had a quick look at {{Point3D}} and {{Vector3}}, they seem fairly similar in their API and fairly convoluted in their inheritance hierarchy.
We may want to have a user guide explaining the design and how it is put to use with concrete examples of when to use this vs that class/interface. It will surely spare us puzzled faces later on.

When we wonder whether an {{AffineTransform3D}} _is-a_ {{UnaryOperator<Point3D>}} or {{UnaryOperator<Vector3D>}}, there is some alarm starting to ring...
{quote}Did you have any other alternatives in mind?
{quote}
In principle, there are two use-cases that arise from the above discussion:
# Expressing a sequence of {{AffineTransform3D}} operations
# Combining a sequence of {{AffineTransform3D}} operations

If the former is ever useful, then the {{UnaryOperator}} approach seems to allow more flexibility in using the JDK API (stream, filter, ...).

If the second is necessary for performance reason, for example (IIUC) when the same transform is to be applied to many points (or was it vectors? ;)), a simpler (IMHO) API may be:
{code:java}
public static AffineTransform3D combine(AffineTransform3D ... transforms) {
AffineTransform3D c = identity();
for (int i = 0; i < transforms.length; i++) {
c = c.times(transforms[i]);
}
return c;
}
{code}
So perhaps, it is the right to setup "real-life" use-cases as you mentioned in your roadmap post to the ML, in order to keep us in the right track (i.e. trying to avoid API and performance mistakes).

Also, have you already looked at how e.g.
# existing classes such as {{Rotation}} fit with {{AffineTransform3D}}
# usage of {{Quaternion}} (from ["Commons Numbers"|https://git1-us-west.apache.org/repos/asf?p=commons-numbers.git;a=blob;f=commons-numbers-quaternion/src/main/java/org/apache/commons/numbers/quaternion/Quaternion.java;h=86d7fdfa53d2b61bd07cebd05c02008f2809ebd9;hb=HEAD])

?
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Matt Juntunen (JIRA)
2018-10-07 20:05:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16641196#comment-16641196 ]

Matt Juntunen commented on GEOMETRY-14:
---------------------------------------

{quote}What are the different use-cases? Having had a quick look at {{Point3D}} and {{Vector3}}, they seem fairly similar in their API and fairly convoluted in their inheritance hierarchy.
{quote}
It surprises me that this seems new since we've discussed the point/vector API quite a bit. Are you asking what's the difference between the usage of {{Point?D}} and {{Vector?D}}? If so, the difference is mathematical: points represent locations in space and vectors represent translations and directions. Not many operations are allowed on points but vectors come with the full range of operations allowed on vector spaces in linear algebra. Transformations can be applied to both types, so {{AffineTransform?D}} needs to support both.
{quote}When we wonder whether an {{AffineTransform3D}} _is-a_ {{UnaryOperator<Point3D>}} or {{UnaryOperator<Vector3D>}}, there is some alarm starting to ring...
{quote}
It's logically both. However, the language doesn't directly support this so we need to figure something else out. I don't see anything wrong with that.

As a side note, I'm planning on making the {{AffineTransform?D}} classes implement {{o.a.c.g.core.partitioning.Transform}} as well, which means that we will also have the method {{apply(Hyperplane<>)}}.
{quote}re: transform sequences
{quote}
I think that users should nearly always want to combine transform sequences into a single transform instead of applying multiple transforms in order. For example, with the current implementation of {{AffineTransform3D}}, the number of multiplications involved in transforming a point with multiple transforms applied one after another is {{9nt}} where {{n}} is the number of points and {{t}} is the number of transforms. When combining transforms together, the {{t}} value drops out and this becomes {{9n + 36}}, where {{n}} is again the number of points and the {{36}} value is the one-time cost of multiplying the transform matrices together. So, even with the simplest case of applying two transforms in order, the combination method becomes the better choice when {{n > 4}}. The difference becomes even more pronounced as more transforms are involved and {{n}} increases.

Also, the combination approach has the benefit that an inverse can be calculated that undoes _all_ of the combined transformations. This cannot be done easily with a simple sequence of transforms.

I say all of this to make the point that we may not be losing a lot if {{AffineTransform?D}} does not implement {{UnaryOperator}}. The function composition and chaining provided by the latter does not quite fit our use case.

 
{quote}Also, have you already looked at how e.g.
# existing classes such as {{Rotation}} fit with {{AffineTransform3D}}
# usage of {{Quaternion}} (from ["Commons Numbers"|https://git1-us-west.apache.org/repos/asf?p=commons-numbers.git;a=blob;f=commons-numbers-quaternion/src/main/java/org/apache/commons/numbers/quaternion/Quaternion.java;h=86d7fdfa53d2b61bd07cebd05c02008f2809ebd9;hb=HEAD]){quote}
Yes, I have. I'm planning on getting the API sorted with translations and scale operations before jumping into that.
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Gilles (JIRA)
2018-10-08 08:42:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16641524#comment-16641524 ]

Gilles commented on GEOMETRY-14:
--------------------------------

About {{Point}} and {{Vector}}, the question is simply to have (programming) use-cases for both.
{quote}the difference is mathematical
{quote}
I like when the design can be close to the mathematical concepts. But you argued that having
{code:java}
public class Vector3D extends Cartesian3D implements MultiDimensionalEuclideanVector<Point3D, Vector3D>
{code}
and
{code:java}
public final class Point3D extends Cartesian3D implements EuclideanPoint<Point3D, Vector3D>
{code}
was going to be more effective from a usage POV even though the class hierarchy does not agree with the mathematics (a point exists independently of the coordinate system).
So my (design) question is of the same order: If programming is allowed to be a step away from mathematics, maybe (?) that a "point" class is not strictly necessary (here).
As a basic user of a very small fraction of that code, I don't know (yet). That's why I'm suggesting that, if possible, some real-use cases should be presented for the concepts (a.o. _both_ "point" and "vector") that are represented in the API.
If now is not a good time, then I can wait; but this is "for the record" of trying to avoid issues as they seem to appear (at least to me). ;)

About affine transforms: I'm convinced that {{java.util.function}} is not going to help then.

However, do you see a real use-case where the composition of transforms would need more than this method:
{code:java}
public AffineTransform3D combine(AffineTransform3D transform) { /* ... */ }
public static AffineTransform3D combine(AffineTransform3D ... transforms) { /* ... */ }
{code}
together with the varargs static method shown above (as syntactic sugar)?
Note: I couldn't figure out the 3-arguments {{multiply}} method (in the PR).

Can the API of {{AffineTransform3D}} be discussed without including rotations?  Shall we just have
{code:java}
public Rotation extends AffineTransform3D { /* ... */ }
{code}
?
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Matt Juntunen (JIRA)
2018-10-10 02:28:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16644363#comment-16644363 ]

Matt Juntunen commented on GEOMETRY-14:
---------------------------------------

As far as the point/vector thing goes, I'm hesitant to change anything now since a lot of work has gone into the current structure. You are right in that there isn't much of a _need_ to have separate classes, although I still like it the way it is. I think it makes algorithms very clear and the meaning of variables very precise. If we go back to a single class that implements both point and vector, then I feel like we'd be back where we were at the end of MATH-1284 with the Cartesian?D classes being used everywhere. I'm not a fan of that structure since I feel like it obfuscates the meaning of the code to have a class name that's not either Point or Vector. So, do we want to be mathematically pure or programmatically concise? Now that we've moved to a VALJO system for these types and no one is calling {{new}} on anything, we may actually have more options available in the design on this point. I'm not sure yet. Currently, I think I just want to keep it the way it is and maybe ponder if there are any better options.
{quote}However, do you see a real use-case where the composition of transforms would need more than this method:
{quote}
Nope. The {{combine}} methods should do it.
{quote}Can the API of {{AffineTransform3D}} be discussed without including rotations?
{quote}
No, I don't want to merge anything without that. I left it out for now so we could discuss the less complicated parts of the API first. For the 3D case, I'm planning on having {{Rotation}} be a separate class that implements {{Transform}} from core but is not part of the {{AffineTransform}} hierarchy. There will be methods that convert back and forth between them. I'm picturing methods like this:
{code:java}
// AffineTransform3D

// apply a rotation to the current transform and return a new instance
public AffineTransform3D rotate(Rotation rot) {...}

// extract the rotation portion of the transform
public Rotation getRotation() {... }

// convert the given rotation to a matrix-based transform
public AffineTransform3D createRotation(Rotation rot) { ... }
{code}
 
I think I have a good idea of where to go on this issue now, so I'm going to close my pull request and work on it some more. It might be a bit before I can get another pull request in since I'm anticipating a fair amount of work and I'm not going to have any time available for at least a week.
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Gilles (JIRA)
2018-10-10 09:24:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16644710#comment-16644710 ]

Gilles commented on GEOMETRY-14:
--------------------------------

bq. Currently, I think I just want to keep it the way it is and maybe ponder if there are any better options.

Since I cannot offer a concrete alternative, it's quite fine as an attempt to improve on the previous implementation, but since part of the design is not "mathematically pure" (because of the Cartesian system of coordinates being so high in the class hierarchy), I deem it necessary that you write a summary of the design decisions (with known advantages and shortcomings) so that we don't flip-flop every time someone thinks there is some "obvious" flaw in the code structure.

bq. Rotation be a separate class that implements Transform from core but is not part of the AffineTransform hierarchy

Why?
It would also seem to be going away from the goal of "mathematically pure".
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Gilles (JIRA)
2018-10-10 09:25:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16644713#comment-16644713 ]

Gilles commented on GEOMETRY-14:
--------------------------------

bq. I'm not going to have any time available for at least a week.

Thanks for letting me know. :)
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
ASF GitHub Bot (JIRA)
2018-10-17 01:45:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16652819#comment-16652819 ]

ASF GitHub Bot commented on GEOMETRY-14:
----------------------------------------

darkma773r closed pull request #14: GEOMETRY-14: Initial AffineTransform3D class
URL: https://github.com/apache/commons-geometry/pull/14




This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/exception/NonInvertibleTransformException.java b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/exception/NonInvertibleTransformException.java
new file mode 100644
index 0000000..88748dd
--- /dev/null
+++ b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/exception/NonInvertibleTransformException.java
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.geometry.euclidean.exception;
+
+import org.apache.commons.geometry.core.exception.GeometryException;
+
+/** Exception thrown when a transform matrix is not
+ * able to be inverted.
+ */
+public class NonInvertibleTransformException extends GeometryException {
+
+ /** Serializable version identifier */
+ private static final long serialVersionUID = 20180927L;
+
+ /** Simple constructor accepting an error message.
+ * @param msg error message
+ */
+ public NonInvertibleTransformException(String msg) {
+ super(msg);
+ }
+}
diff --git a/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/exception/package-info.java b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/exception/package-info.java
new file mode 100644
index 0000000..d441d1d
--- /dev/null
+++ b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/exception/package-info.java
@@ -0,0 +1,23 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+/**
+ *
+ * <p>
+ * This package provides exception types for Euclidean space.
+ * </p>
+ */
+package org.apache.commons.geometry.euclidean.exception;
diff --git a/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/AffineTransform3D.java b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/AffineTransform3D.java
new file mode 100644
index 0000000..4f40c45
--- /dev/null
+++ b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/AffineTransform3D.java
@@ -0,0 +1,569 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.geometry.euclidean.threed;
+
+import java.io.Serializable;
+import java.util.Arrays;
+
+import org.apache.commons.geometry.core.internal.DoubleFunction3N;
+import org.apache.commons.geometry.euclidean.exception.NonInvertibleTransformException;
+import org.apache.commons.geometry.euclidean.internal.Vectors;
+import org.apache.commons.numbers.arrays.LinearCombination;
+
+/** Class representing an affine transformation in 3 dimensional Euclidean space.
+ *
+ * <p>Instances of this class use a 4x4 matrix internally for all transform operations.
+ * The last row of this matrix is always set to the values <code>[0 0 0 1]</code> and so
+ * is not stored. Hence, the methods in this class that accept or return arrays always
+ * use arrays containing 12 elements, instead of 16.
+ * </p>
+ */
+public final class AffineTransform3D implements Serializable {
+
+ /** Serializable version identifier */
+ private static final long serialVersionUID = 20180923L;
+
+ /** The number of internal matrix elements */
+ private static final int NUM_ELEMENTS = 12;
+
+ /** String used to start the transform matrix string representation */
+ private static final String MATRIX_START = "[ ";
+
+ /** String used to end the transform matrix string representation */
+ private static final String MATRIX_END = " ]";
+
+ /** String used to separate elements in the matrix string representation */
+ private static final String ELEMENT_SEPARATOR = ", ";
+
+ /** String used to separate rows in the matrix string representation */
+ private static final String ROW_SEPARATOR = "; ";
+
+ /** Shared transform set to the identity matrix. */
+ private static final AffineTransform3D IDENTITY = new AffineTransform3D();
+
+ /** Transform matrix entry <code>m<sub>0,0</sub></code> */
+ private double m00 = 1.0;
+ /** Transform matrix entry <code>m<sub>0,1</sub></code> */
+ private double m01 = 0.0;
+ /** Transform matrix entry <code>m<sub>0,2</sub></code> */
+ private double m02 = 0.0;
+ /** Transform matrix entry <code>m<sub>0,3</sub></code> */
+ private double m03 = 0.0;
+
+ /** Transform matrix entry <code>m<sub>1,0</sub></code> */
+ private double m10 = 0.0;
+ /** Transform matrix entry <code>m<sub>1,1</sub></code> */
+ private double m11 = 1.0;
+ /** Transform matrix entry <code>m<sub>1,2</sub></code> */
+ private double m12 = 0.0;
+ /** Transform matrix entry <code>m<sub>1,3</sub></code> */
+ private double m13 = 0.0;
+
+ /** Transform matrix entry <code>m<sub>2,0</sub></code> */
+ private double m20 = 0.0;
+ /** Transform matrix entry <code>m<sub>2,1</sub></code> */
+ private double m21 = 0.0;
+ /** Transform matrix entry <code>m<sub>2,2</sub></code> */
+ private double m22 = 1.0;
+ /** Transform matrix entry <code>m<sub>2,3</sub></code> */
+ private double m23 = 0.0;
+
+ /** Simple constructor. The internal matrix elements are initialized
+ * to the identity matrix.
+ */
+ private AffineTransform3D() {
+ }
+
+ /** Return a 12 element array containing the variable elements from the
+ * internal transformation matrix. The elements are in row-major order.
+ * The array indices map to the internal matrix as follows:
+ * <pre>
+ * [
+ * arr[0], arr[1], arr[2], arr[3]
+ * arr[4], arr[5], arr[6], arr[7],
+ * arr[8], arr[9], arr[10], arr[11],
+ * 0 0 0 1
+ * ]
+ * </pre>
+ * @return 12 element array containing the variable elements from the
+ * internal transformation matrix
+ */
+ public double[] toArray() {
+ return new double[] {
+ m00, m01, m02, m03,
+ m10, m11, m12, m13,
+ m20, m21, m22, m23
+ };
+ }
+
+ /** Apply this transform to the given point. A new point is returned.
+ * @param pt the point to transform
+ * @return the new, transformed point
+ */
+ public Point3D applyTo(final Point3D pt) {
+ return applyTo(pt, Point3D::of);
+ }
+
+ /** Apply this transform to the given vector. A new vector is returned.
+ * @param vec the vector to transform
+ * @return the new, transformed vector
+ */
+ public Vector3D applyTo(final Vector3D vec) {
+ return applyTo(vec, Vector3D::of);
+ }
+
+ /** Get a new transform containing the result of applying a translation logically after
+ * the transformation represented by the current instance. This is achieved by
+ * creating a new translation transform and pre-multiplying it with the current
+ * instance. In other words, the returned transform contains the matrix
+ * <code>B * A</code>, where <code>A</code> is the current matrix and <code>B</code>
+ * is the matrix representing the given translation.
+ * @param translation vector containing the translation values for each axis
+ * @return a new transform containing the result of applying a translation to
+ * the current instance
+ */
+ public AffineTransform3D translate(final Vector3D translation) {
+ return translate(translation.getX(), translation.getY(), translation.getZ());
+ }
+
+ /** Get a new transform containing the result of applying a translation logically after
+ * the transformation represented by the current instance. This is achieved by
+ * creating a new translation transform and pre-multiplying it with the current
+ * instance. In other words, the returned transform contains the matrix
+ * <code>B * A</code>, where <code>A</code> is the current matrix and <code>B</code>
+ * is the matrix representing the given translation.
+ * @param x translation in the x direction
+ * @param y translation in the y direction
+ * @param z translation in the z direction
+ * @return a new transform containing the result of applying a translation to
+ * the current instance
+ */
+ public AffineTransform3D translate(final double x, final double y, final double z) {
+ final AffineTransform3D result = createTranslation(x, y, z);
+
+ return multiply(result, this, result);
+ }
+
+ /** Get a new transform containing the result of applying a scale operation of the
+ * given value in all axes logically after the transformation represented by the current instance.
+ * This is achieved by creating a new scale transform and pre-multiplying it with the current
+ * instance. In other words, the returned transform contains the matrix
+ * <code>B * A</code>, where <code>A</code> is the current matrix and <code>B</code>
+ * is the matrix representing the given scale operation.
+ * @param factor the scale factor to apply to all axes
+ * @return a new transform containing the result of applying a scale operation to
+ * the current instance
+ */
+ public AffineTransform3D scale(final double factor) {
+ return scale(factor, factor, factor);
+ }
+
+ /** Get a new transform containing the result of applying a scale operation
+ * logically after the transformation represented by the current instance.
+ * This is achieved by creating a new scale transform and pre-multiplying it with the current
+ * instance. In other words, the returned transform contains the matrix
+ * <code>B * A</code>, where <code>A</code> is the current matrix and <code>B</code>
+ * is the matrix representing the given scale operation.
+ * @param scaleFactors vector containing scale factors for each axis
+ * @return a new transform containing the result of applying a scale operation to
+ * the current instance
+ */
+ public AffineTransform3D scale(final Vector3D scaleFactors) {
+ return scale(scaleFactors.getX(), scaleFactors.getY(), scaleFactors.getZ());
+ }
+
+ /** Get a new transform containing the result of applying a scale operation
+ * logically after the transformation represented by the current instance.
+ * This is achieved by creating a new scale transform and pre-multiplying it with the current
+ * instance. In other words, the returned transform contains the matrix
+ * <code>B * A</code>, where <code>A</code> is the current matrix and <code>B</code>
+ * is the matrix representing the given scale operation.
+ * @param x scale factor for the x axis
+ * @param y scale factor for the y axis
+ * @param z scale factor for the z axis
+ * @return a new transform containing the result of applying a scale operation to
+ * the current instance
+ */
+ public AffineTransform3D scale(final double x, final double y, final double z) {
+ final AffineTransform3D result = createScale(x, y, z);
+
+ return multiply(result, this, result);
+ }
+
+ /** Get a new transform created by multiplying the given transform with the current
+ * instance. The computed value is <code>A * B</code> where <code>A</code> is the matrix
+ * of the current instance and <code>B</code> is the matrix of the given instance.
+ * @param b the other transform to multiply with
+ * @return the result of multiplying this transform with {@code b}
+ */
+ public AffineTransform3D multiply(final AffineTransform3D b) {
+ return multiply(this, b, new AffineTransform3D());
+ }
+
+ /** Get a new transform representing the inverse of the current instance.
+ * @return inverse transform
+ * @throws NonInvertibleTransformException if the transform matrix cannot be inverted
+ */
+ public AffineTransform3D getInverse() {
+
+ // compute the determinant of the matrix
+ final double det = determinant(
+ m00, m01, m02,
+ m10, m11, m12,
+ m20, m21, m22
+ );
+
+ if (!Vectors.isRealNonZero(det)) {
+ throw new NonInvertibleTransformException("Transform is not invertible; matrix determinant is " + det);
+ }
+
+ // validate the remaining matrix elements that were not part of the determinant
+ validateElementForInverse(m03);
+ validateElementForInverse(m13);
+ validateElementForInverse(m23);
+
+ // compute the necessary elements of the cofactor matrix
+ // (we need all but the last column)
+ final double c00 = determinant(m11, m12, m21, m22);
+ final double c01 = - determinant(m10, m12, m20, m22);
+ final double c02 = determinant(m10, m11, m20, m21);
+
+ final double c10 = - determinant(m01, m02, m21, m22);
+ final double c11 = determinant(m00, m02, m20, m22);
+ final double c12 = - determinant(m00, m01, m20, m21);
+
+ final double c20 = determinant(m01, m02, m11, m12);
+ final double c21 = - determinant(m00, m02, m10, m12);
+ final double c22 = determinant(m00, m01, m10, m11);
+
+ final double c30 = - determinant(
+ m01, m02, m03,
+ m11, m12, m13,
+ m21, m22, m23
+ );
+ final double c31 = determinant(
+ m00, m02, m03,
+ m10, m12, m13,
+ m20, m22, m23
+ );
+ final double c32 = - determinant(
+ m00, m01, m03,
+ m10, m11, m13,
+ m20, m21, m23
+ );
+
+ // the final answer is the adjugate matrix (the transpose of the cofactor matrix)
+ // multiplied by the inverse of the determinant
+ final double invDet = 1.0 / det;
+
+ AffineTransform3D inverse = new AffineTransform3D();
+ inverse.m00 = invDet * c00;
+ inverse.m01 = invDet * c10;
+ inverse.m02 = invDet * c20;
+ inverse.m03 = invDet * c30;
+
+ inverse.m10 = invDet * c01;
+ inverse.m11 = invDet * c11;
+ inverse.m12 = invDet * c21;
+ inverse.m13 = invDet * c31;
+
+ inverse.m20 = invDet * c02;
+ inverse.m21 = invDet * c12;
+ inverse.m22 = invDet * c22;
+ inverse.m23 = invDet * c32;
+
+ return inverse;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public int hashCode() {
+ final int prime = 31;
+ int result = 1;
+
+ result = (result * prime) + (Double.hashCode(m00) + Double.hashCode(m01) + Double.hashCode(m02) + Double.hashCode(m03));
+ result = (result * prime) + (Double.hashCode(m10) + Double.hashCode(m11) + Double.hashCode(m12) + Double.hashCode(m13));
+ result = (result * prime) + (Double.hashCode(m20) + Double.hashCode(m21) + Double.hashCode(m22) + Double.hashCode(m23));
+
+ return result;
+ }
+
+ /**
+ * Return true if the given object is an instance of {@link AffineTransform3D}
+ * and all matrix element values are exactly equal.
+ * @param obj object to test for equality with the current instance
+ * @return true if all transform matrix elements are exactly equal; otherwise false
+ */
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof AffineTransform3D)) {
+ return false;
+ }
+
+ AffineTransform3D other = (AffineTransform3D) obj;
+
+ return Arrays.equals(toArray(), other.toArray());
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public String toString() {
+ final StringBuilder sb = new StringBuilder();
+
+ sb.append(MATRIX_START)
+
+ .append(m00)
+ .append(ELEMENT_SEPARATOR)
+ .append(m01)
+ .append(ELEMENT_SEPARATOR)
+ .append(m02)
+ .append(ELEMENT_SEPARATOR)
+ .append(m03)
+ .append(ROW_SEPARATOR)
+
+ .append(m10)
+ .append(ELEMENT_SEPARATOR)
+ .append(m11)
+ .append(ELEMENT_SEPARATOR)
+ .append(m12)
+ .append(ELEMENT_SEPARATOR)
+ .append(m13)
+ .append(ROW_SEPARATOR)
+
+ .append(m20)
+ .append(ELEMENT_SEPARATOR)
+ .append(m21)
+ .append(ELEMENT_SEPARATOR)
+ .append(m22)
+ .append(ELEMENT_SEPARATOR)
+ .append(m23)
+
+ .append(MATRIX_END);
+
+ return sb.toString();
+ }
+
+ /** Multiply two transform matrices together, storing the result in a third instance. The result is computed
+ * completely and then stored into the output transform, meaning that the output transform can be the same
+ * as one of the inputs.
+ * @param a first transform
+ * @param b second transform
+ * @param c output transform; may be one of {@code a} or {@code b}
+ * @return the output matrix given in {@code c}, which contains the result of multiplying {@code a} and {@code b}
+ */
+ private AffineTransform3D multiply(final AffineTransform3D a, final AffineTransform3D b, final AffineTransform3D c) {
+
+ // calculate the matrix elements
+ final double c00 = LinearCombination.value(a.m00, b.m00, a.m01, b.m10, a.m02, b.m20);
+ final double c01 = LinearCombination.value(a.m00, b.m01, a.m01, b.m11, a.m02, b.m21);
+ final double c02 = LinearCombination.value(a.m00, b.m02, a.m01, b.m12, a.m02, b.m22);
+ final double c03 = LinearCombination.value(a.m00, b.m03, a.m01, b.m13, a.m02, b.m23) + a.m03;
+
+ final double c10 = LinearCombination.value(a.m10, b.m00, a.m11, b.m10, a.m12, b.m20);
+ final double c11 = LinearCombination.value(a.m10, b.m01, a.m11, b.m11, a.m12, b.m21);
+ final double c12 = LinearCombination.value(a.m10, b.m02, a.m11, b.m12, a.m12, b.m22);
+ final double c13 = LinearCombination.value(a.m10, b.m03, a.m11, b.m13, a.m12, b.m23) + a.m13;
+
+ final double c20 = LinearCombination.value(a.m20, b.m00, a.m21, b.m10, a.m22, b.m20);
+ final double c21 = LinearCombination.value(a.m20, b.m01, a.m21, b.m11, a.m22, b.m21);
+ final double c22 = LinearCombination.value(a.m20, b.m02, a.m21 , b.m12, a.m22, b.m22);
+ final double c23 = LinearCombination.value(a.m20, b.m03, a.m21 , b.m13, a.m22, b.m23) + a.m23;
+
+ // assign to the output
+ c.m00 = c00;
+ c.m01 = c01;
+ c.m02 = c02;
+ c.m03 = c03;
+
+ c.m10 = c10;
+ c.m11 = c11;
+ c.m12 = c12;
+ c.m13 = c13;
+
+ c.m20 = c20;
+ c.m21 = c21;
+ c.m22 = c22;
+ c.m23 = c23;
+
+ return c;
+ }
+
+ /** Apply the transform to the given set of Cartesian coordinates. The transformed
+ * coordinates are passed to the given factory function and its return value is
+ * returned.
+ * @param <T> Type returned by {@code factory}
+ * @param coords coordinates to transform
+ * @param factory function accepting transformed coordinates and returning a value
+ * @return the return value from {@code factory}
+ */
+ private <T> T applyTo(final Cartesian3D coords, DoubleFunction3N<T> factory) {
+ final double x = coords.getX();
+ final double y = coords.getY();
+ final double z = coords.getZ();
+
+ final double resultX = LinearCombination.value(m00, x, m01, y, m02, z) + m03;
+ final double resultY = LinearCombination.value(m10, x, m11, y, m12, z) + m13;
+ final double resultZ = LinearCombination.value(m20, x, m21, y, m22, z) + m23;
+
+ return factory.apply(resultX, resultY, resultZ);
+ }
+
+ /** Get a new transform with the given matrix elements. The array must contain 12 elements.
+ * @param arr 12-element array containing values for the variable entries in the
+ * transform matrix
+ * @return a new transform initialized with the given matrix values
+ * @throws IllegalArgumentException if the array does not have 12 elements
+ */
+ public static AffineTransform3D of(final double ... arr) {
+ if (arr.length != NUM_ELEMENTS) {
+ throw new IllegalArgumentException("Dimension mismatch: " + arr.length + " != " + NUM_ELEMENTS);
+ }
+
+ AffineTransform3D result = new AffineTransform3D();
+
+ result.m00 = arr[0];
+ result.m01 = arr[1];
+ result.m02 = arr[2];
+ result.m03 = arr[3];
+
+ result.m10 = arr[4];
+ result.m11 = arr[5];
+ result.m12 = arr[6];
+ result.m13 = arr[7];
+
+ result.m20 = arr[8];
+ result.m21 = arr[9];
+ result.m22 = arr[10];
+ result.m23 = arr[11];
+
+ return result;
+ }
+
+ /** Get the transform representing the identity matrix. This transform does not
+ * modify point or vector values when applied.
+ * @return transform representing the identity matrix
+ */
+ public static AffineTransform3D identity() {
+ return IDENTITY;
+ }
+
+ /** Get a transform representing the given translation.
+ * @param translation vector containing translation values for each axis
+ * @return a new transform representing the given translation
+ */
+ public static AffineTransform3D createTranslation(final Vector3D translation) {
+ return createTranslation(translation.getX(), translation.getY(), translation.getZ());
+ }
+
+ /** Get a transform representing the given translation.
+ * @param x translation in the x direction
+ * @param y translation in the y direction
+ * @param z translation in the z direction
+ * @return a new transform representing the given translation
+ */
+ public static AffineTransform3D createTranslation(final double x, final double y, final double z) {
+ final AffineTransform3D transform = new AffineTransform3D();
+
+ transform.m03 = x;
+ transform.m13 = y;
+ transform.m23 = z;
+
+ return transform;
+ }
+
+ /** Get a transform representing a scale operation with the given scale factor applied to all axes.
+ * @param factor scale factor to apply to all axes
+ * @return a new transform representing a uniform scaling in all axes
+ */
+ public static AffineTransform3D createScale(final double factor) {
+ return createScale(factor, factor, factor);
+ }
+
+ /** Get a transform representing a scale operation.
+ * @param factors vector containing scale factors for each axis
+ * @return a new transform representing a scale operation
+ */
+ public static AffineTransform3D createScale(final Vector3D factors) {
+ return createScale(factors.getX(), factors.getY(), factors.getZ());
+ }
+
+ /** Get a transform representing a scale operation.
+ * @param x scale factor for the x axis
+ * @param y scale factor for the y axis
+ * @param z scale factor for the z axis
+ * @return a new transform representing a scale operation
+ */
+ public static AffineTransform3D createScale(final double x, final double y, final double z) {
+ final AffineTransform3D transform = new AffineTransform3D();
+
+ transform.m00 = x;
+ transform.m11 = y;
+ transform.m22 = z;
+
+ return transform;
+ }
+
+ /** Compute the determinant of the 2x2 matrix represented by the given values.
+ * @param a00 matrix entry <code>a<sub>0,0</sub></code>
+ * @param a01 matrix entry <code>a<sub>0,1</sub></code>
+ * @param a10 matrix entry <code>a<sub>1,0</sub></code>
+ * @param a11 matrix entry <code>a<sub>1,1</sub></code>
+ * @return computed 2x2 matrix determinant
+ */
+ private static double determinant(
+ final double a00, final double a01,
+ final double a10, final double a11) {
+
+ return (a00 * a11) - (a01 * a10);
+ }
+
+ /** Compute the determinant of the 3x3 matrix represented by the given values.
+ * @param a00 matrix entry <code>a<sub>0,0</sub></code>
+ * @param a01 matrix entry <code>a<sub>0,1</sub></code>
+ * @param a02 matrix entry <code>a<sub>0,2</sub></code>
+ * @param a10 matrix entry <code>a<sub>1,0</sub></code>
+ * @param a11 matrix entry <code>a<sub>1,1</sub></code>
+ * @param a12 matrix entry <code>a<sub>1,2</sub></code>
+ * @param a20 matrix entry <code>a<sub>2,0</sub></code>
+ * @param a21 matrix entry <code>a<sub>2,1</sub></code>
+ * @param a22 matrix entry <code>a<sub>2,2</sub></code>
+ * @return computed 3x3 matrix determinant
+ */
+ private static double determinant(
+ final double a00, final double a01, final double a02,
+ final double a10, final double a11, final double a12,
+ final double a20, final double a21, final double a22) {
+
+ return ((a00 * a11 * a22) + (a01 * a12 * a20) + (a02 * a10 * a21)) -
+ ((a00 * a12 * a21) + (a01 * a10 * a22) + (a02 * a11 * a20));
+ }
+
+ /** Checks that the given matrix element is valid for use in calculation of
+ * a matrix inverse. Throws a {@link NonInvertibleTransformException} if not.
+ * @param element matrix entry to check
+ * @throws NonInvertibleTransformException if the element is not valid for use
+ * in calculating a matrix inverse, ie if it is NaN or infinite.
+ */
+ private static void validateElementForInverse(final double element) {
+ if (!Double.isFinite(element)) {
+ throw new NonInvertibleTransformException("Transform is not invertible; invalid matrix element: " + element);
+ }
+ }
+}
diff --git a/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/Point3D.java b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/Point3D.java
index 24626ec..ddb7e2d 100644
--- a/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/Point3D.java
+++ b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/Point3D.java
@@ -112,6 +112,16 @@ public Point3D add(Vector3D v) {
);
}

+ /** Apply the given transform to this point, returning the result as a
+ * new point instance.
+ * @param transform the transform to apply
+ * @return a new, transformed point
+ * @see AffineTransform3D#applyTo(Point3D)
+ */
+ public Point3D apply(AffineTransform3D transform) {
+ return transform.applyTo(this);
+ }
+
/**
* Get a hashCode for the point.
* <p>All NaN values have the same hash code.</p>
diff --git a/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/Vector3D.java b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/Vector3D.java
index 252848d..3016a54 100644
--- a/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/Vector3D.java
+++ b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/Vector3D.java
@@ -298,6 +298,16 @@ public Vector3D crossProduct(final Vector3D v) {
LinearCombination.value(getX(), v.getY(), -getY(), v.getX()));
}

+ /** Apply the given transform to this vector, returning the result as a
+ * new vector instance.
+ * @param transform the transform to apply
+ * @return a new, transformed vector
+ * @see AffineTransform3D#applyTo(Vector3D)
+ */
+ public Vector3D apply(AffineTransform3D transform) {
+ return transform.applyTo(this);
+ }
+
/**
* Get a hashCode for the vector.
* <p>All NaN values have the same hash code.</p>
diff --git a/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/AffineTransform3DTest.java b/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/AffineTransform3DTest.java
new file mode 100644
index 0000000..89bafb6
--- /dev/null
+++ b/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/AffineTransform3DTest.java
@@ -0,0 +1,636 @@
+package org.apache.commons.geometry.euclidean.threed;
+
+import org.apache.commons.geometry.core.Geometry;
+import org.apache.commons.geometry.core.GeometryTestUtils;
+import org.apache.commons.geometry.euclidean.EuclideanTestUtils;
+import org.apache.commons.geometry.euclidean.exception.NonInvertibleTransformException;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class AffineTransform3DTest {
+
+ private static final double EPS = 1e-12;
+
+ @Test
+ public void testOf() {
+ // arrange
+ double[] arr = {
+ 1, 2, 3, 4,
+ 5, 6, 7, 8,
+ 9, 10, 11, 12
+ };
+
+ // act
+ AffineTransform3D transform = AffineTransform3D.of(arr);
+
+ // assert
+ double[] result = transform.toArray();
+ Assert.assertNotSame(arr, result);
+ Assert.assertArrayEquals(arr, result, 0.0);
+ }
+
+ @Test
+ public void testOf_invalidDimensions() {
+ // act/assert
+ GeometryTestUtils.assertThrows(() -> AffineTransform3D.of(1, 2),
+ IllegalArgumentException.class, "Dimension mismatch: 2 != 12");
+ }
+
+ @Test
+ public void testIdentity() {
+ // act
+ AffineTransform3D transform = AffineTransform3D.identity();
+
+ // assert
+ double[] expected = {
+ 1, 0, 0, 0,
+ 0, 1, 0, 0,
+ 0, 0, 1, 0
+ };
+ Assert.assertArrayEquals(expected, transform.toArray(), 0.0);
+ }
+
+ @Test
+ public void testCreateTranslation_xyz() {
+ // act
+ AffineTransform3D transform = AffineTransform3D.createTranslation(2, 3, 4);
+
+ // assert
+ double[] expected = {
+ 1, 0, 0, 2,
+ 0, 1, 0, 3,
+ 0, 0, 1, 4
+ };
+ Assert.assertArrayEquals(expected, transform.toArray(), 0.0);
+ }
+
+ @Test
+ public void testCreateTranslation_vector() {
+ // act
+ AffineTransform3D transform = AffineTransform3D.createTranslation(Vector3D.of(5, 6, 7));
+
+ // assert
+ double[] expected = {
+ 1, 0, 0, 5,
+ 0, 1, 0, 6,
+ 0, 0, 1, 7
+ };
+ Assert.assertArrayEquals(expected, transform.toArray(), 0.0);
+ }
+
+ @Test
+ public void testCreateScale_xyz() {
+ // act
+ AffineTransform3D transform = AffineTransform3D.createScale(2, 3, 4);
+
+ // assert
+ double[] expected = {
+ 2, 0, 0, 0,
+ 0, 3, 0, 0,
+ 0, 0, 4, 0
+ };
+ Assert.assertArrayEquals(expected, transform.toArray(), 0.0);
+ }
+
+ @Test
+ public void testTranslate_xyz() {
+ // arrange
+ AffineTransform3D a = AffineTransform3D.of(
+ 2, 0, 0, 10,
+ 0, 3, 0, 11,
+ 0, 0, 4, 12
+ );
+
+ // act
+ AffineTransform3D result = a.translate(4, 5, 6);
+
+ // assert
+ double[] expected = {
+ 2, 0, 0, 14,
+ 0, 3, 0, 16,
+ 0, 0, 4, 18
+ };
+ Assert.assertArrayEquals(expected, result.toArray(), 0.0);
+ }
+
+ @Test
+ public void testTranslate_vector() {
+ // arrange
+ AffineTransform3D a = AffineTransform3D.of(
+ 2, 0, 0, 10,
+ 0, 3, 0, 11,
+ 0, 0, 4, 12
+ );
+
+ // act
+ AffineTransform3D result = a.translate(Vector3D.of(7, 8, 9));
+
+ // assert
+ double[] expected = {
+ 2, 0, 0, 17,
+ 0, 3, 0, 19,
+ 0, 0, 4, 21
+ };
+ Assert.assertArrayEquals(expected, result.toArray(), 0.0);
+ }
+
+ @Test
+ public void testCreateScale_vector() {
+ // act
+ AffineTransform3D transform = AffineTransform3D.createScale(Vector3D.of(4, 5, 6));
+
+ // assert
+ double[] expected = {
+ 4, 0, 0, 0,
+ 0, 5, 0, 0,
+ 0, 0, 6, 0
+ };
+ Assert.assertArrayEquals(expected, transform.toArray(), 0.0);
+ }
+
+ @Test
+ public void testCreateScale_singleValue() {
+ // act
+ AffineTransform3D transform = AffineTransform3D.createScale(7);
+
+ // assert
+ double[] expected = {
+ 7, 0, 0, 0,
+ 0, 7, 0, 0,
+ 0, 0, 7, 0
+ };
+ Assert.assertArrayEquals(expected, transform.toArray(), 0.0);
+ }
+
+ @Test
+ public void testScale_xyz() {
+ // arrange
+ AffineTransform3D a = AffineTransform3D.of(
+ 2, 0, 0, 10,
+ 0, 3, 0, 11,
+ 0, 0, 4, 12
+ );
+
+ // act
+ AffineTransform3D result = a.scale(4, 5, 6);
+
+ // assert
+ double[] expected = {
+ 8, 0, 0, 40,
+ 0, 15, 0, 55,
+ 0, 0, 24, 72
+ };
+ Assert.assertArrayEquals(expected, result.toArray(), 0.0);
+ }
+
+ @Test
+ public void testScale_vector() {
+ // arrange
+ AffineTransform3D a = AffineTransform3D.of(
+ 2, 0, 0, 10,
+ 0, 3, 0, 11,
+ 0, 0, 4, 12
+ );
+
+ // act
+ AffineTransform3D result = a.scale(Vector3D.of(7, 8, 9));
+
+ // assert
+ double[] expected = {
+ 14, 0, 0, 70,
+ 0, 24, 0, 88,
+ 0, 0, 36, 108
+ };
+ Assert.assertArrayEquals(expected, result.toArray(), 0.0);
+ }
+
+ @Test
+ public void testScale_singleValue() {
+ // arrange
+ AffineTransform3D a = AffineTransform3D.of(
+ 2, 0, 0, 10,
+ 0, 3, 0, 11,
+ 0, 0, 4, 12
+ );
+
+ // act
+ AffineTransform3D result = a.scale(10);
+
+ // assert
+ double[] expected = {
+ 20, 0, 0, 100,
+ 0, 30, 0, 110,
+ 0, 0, 40, 120
+ };
+ Assert.assertArrayEquals(expected, result.toArray(), 0.0);
+ }
+
+ @Test
+ public void testApplyTo_identity() {
+ // arrange
+ AffineTransform3D transform = AffineTransform3D.identity();
+
+ // act/assert
+ runWithCoordinates((x, y, z) -> {
+ Vector3D v = Vector3D.of(x, y, z);
+ Point3D p = Point3D.of(x, y, z);
+
+ EuclideanTestUtils.assertCoordinatesEqual(v, transform.applyTo(v), EPS);
+ EuclideanTestUtils.assertCoordinatesEqual(p, transform.applyTo(p), EPS);
+ });
+ }
+
+ @Test
+ public void testApplyTo_translate() {
+ // arrange
+ Vector3D translation = Vector3D.of(1.1, -Geometry.PI, 5.5);
+
+ AffineTransform3D transform = AffineTransform3D.identity()
+ .translate(translation);
+
+ // act/assert
+ runWithCoordinates((x, y, z) -> {
+ Vector3D vec = Vector3D.of(x, y, z);
+ Point3D pt = vec.asPoint();
+
+ Vector3D expectedVec = vec.add(translation);
+ Point3D expectedPt = pt.add(translation);
+
+ EuclideanTestUtils.assertCoordinatesEqual(expectedVec, transform.applyTo(vec), EPS);
+ EuclideanTestUtils.assertCoordinatesEqual(expectedPt, transform.applyTo(pt), EPS);
+ });
+ }
+
+ @Test
+ public void testApplyTo_scale() {
+ // arrange
+ Vector3D factors = Vector3D.of(2.0, -3.0, 4.0);
+
+ AffineTransform3D transform = AffineTransform3D.identity()
+ .scale(factors);
+
+ // act/assert
+ runWithCoordinates((x, y, z) -> {
+ Vector3D vec = Vector3D.of(x, y, z);
+ Point3D pt = vec.asPoint();
+
+ Vector3D expectedVec = Vector3D.of(factors.getX() * x, factors.getY() * y, factors.getZ() * z);
+ Point3D expectedPt = expectedVec.asPoint();
+
+ EuclideanTestUtils.assertCoordinatesEqual(expectedVec, transform.applyTo(vec), EPS);
+ EuclideanTestUtils.assertCoordinatesEqual(expectedPt, transform.applyTo(pt), EPS);
+ });
+ }
+
+ @Test
+ public void testApplyTo_translateThenScale() {
+ // arrange
+ Vector3D translation = Vector3D.of(-2.0, -3.0, -4.0);
+ Vector3D scale = Vector3D.of(5.0, 6.0, 7.0);
+
+ AffineTransform3D transform = AffineTransform3D.identity()
+ .translate(translation)
+ .scale(scale);
+
+ // act/assert
+ EuclideanTestUtils.assertCoordinatesEqual(Point3D.of(-5, -12, -21), transform.applyTo(Point3D.of(1, 1, 1)), EPS);
+
+ runWithCoordinates((x, y, z) -> {
+ Vector3D vec = Vector3D.of(x, y, z);
+ Point3D pt = vec.asPoint();
+
+ Vector3D expectedVec = Vector3D.of(
+ (x + translation.getX()) * scale.getX(),
+ (y + translation.getY()) * scale.getY(),
+ (z + translation.getZ()) * scale.getZ()
+ );
+ Point3D expectedPt = expectedVec.asPoint();
+
+ EuclideanTestUtils.assertCoordinatesEqual(expectedVec, transform.applyTo(vec), EPS);
+ EuclideanTestUtils.assertCoordinatesEqual(expectedPt, transform.applyTo(pt), EPS);
+ });
+ }
+
+ @Test
+ public void testApplyTo_scaleThenTranslate() {
+ // arrange
+ Vector3D scale = Vector3D.of(5.0, 6.0, 7.0);
+ Vector3D translation = Vector3D.of(-2.0, -3.0, -4.0);
+
+ AffineTransform3D transform = AffineTransform3D.identity()
+ .scale(scale)
+ .translate(translation);
+
+ // act/assert
+ runWithCoordinates((x, y, z) -> {
+ Vector3D vec = Vector3D.of(x, y, z);
+ Point3D pt = vec.asPoint();
+
+ Vector3D expectedVec = Vector3D.of(
+ (x * scale.getX()) + translation.getX(),
+ (y * scale.getY()) + translation.getY(),
+ (z * scale.getZ()) + translation.getZ()
+ );
+ Point3D expectedPt = expectedVec.asPoint();
+
+ EuclideanTestUtils.assertCoordinatesEqual(expectedVec, transform.applyTo(vec), EPS);
+ EuclideanTestUtils.assertCoordinatesEqual(expectedPt, transform.applyTo(pt), EPS);
+ });
+ }
+
+ @Test
+ public void testMultiply() {
+ // arrange
+ AffineTransform3D a = AffineTransform3D.of(
+ 1, 2, 3, 4,
+ 5, 6, 7, 8,
+ 9, 10, 11, 12
+ );
+ AffineTransform3D b = AffineTransform3D.of(
+ 13, 14, 15, 16,
+ 17, 18, 19, 20,
+ 21, 22, 23, 24
+ );
+
+ // act
+ AffineTransform3D result = a.multiply(b);
+
+ // assert
+ double[] arr = result.toArray();
+ Assert.assertArrayEquals(new double[] {
+ 110, 116, 122, 132,
+ 314, 332, 350, 376,
+ 518, 548, 578, 620
+ }, arr, EPS);
+ }
+
+ @Test
+ public void testMultiply_composeTransformOperations() {
+ // arrange
+ Vector3D translation1 = Vector3D.of(1, 2, 3);
+ double scale = 2.0;
+ Vector3D translation2 = Vector3D.of(4, 5, 6);
+
+ AffineTransform3D a = AffineTransform3D.createTranslation(translation1);
+ AffineTransform3D b = AffineTransform3D.createScale(scale);
+ AffineTransform3D c = AffineTransform3D.identity();
+ AffineTransform3D d = AffineTransform3D.createTranslation(translation2);
+
+ // act
+ AffineTransform3D transform = d.multiply(c).multiply(b).multiply(a);
+
+ // assert
+ runWithCoordinates((x, y, z) -> {
+ Vector3D vec = Vector3D.of(x, y, z);
+
+ Vector3D expectedVec = vec
+ .add(translation1)
+ .scalarMultiply(scale)
+ .add(translation2);
+
+ EuclideanTestUtils.assertCoordinatesEqual(expectedVec, transform.applyTo(vec), EPS);
+ });
+ }
+
+ @Test
+ public void testGetInverse_identity() {
+ // act
+ AffineTransform3D inverse = AffineTransform3D.identity().getInverse();
+
+ // assert
+ double[] expected = {
+ 1, 0, 0, 0,
+ 0, 1, 0, 0,
+ 0, 0, 1, 0
+ };
+ Assert.assertArrayEquals(expected, inverse.toArray(), 0.0);
+ }
+
+ @Test
+ public void testGetInverse_multiplyByInverse_producesIdentity() {
+ // arrange
+ AffineTransform3D a = AffineTransform3D.of(
+ 1, 3, 7, 8,
+ 2, 4, 9, 12,
+ 5, 6, 10, 11
+ );
+
+ AffineTransform3D inv = a.getInverse();
+
+ // act
+ AffineTransform3D result = inv.multiply(a);
+
+ // assert
+ double[] expected = {
+ 1, 0, 0, 0,
+ 0, 1, 0, 0,
+ 0, 0, 1, 0
+ };
+ Assert.assertArrayEquals(expected, result.toArray(), EPS);
+ }
+
+ @Test
+ public void testGetInverse_translate() {
+ // arrange
+ AffineTransform3D transform = AffineTransform3D.createTranslation(1, -2, 4);
+
+ // act
+ AffineTransform3D inverse = transform.getInverse();
+
+ // assert
+ double[] expected = {
+ 1, 0, 0, -1,
+ 0, 1, 0, 2,
+ 0, 0, 1, -4
+ };
+ Assert.assertArrayEquals(expected, inverse.toArray(), 0.0);
+ }
+
+ @Test
+ public void testGetInverse_scale() {
+ // arrange
+ AffineTransform3D transform = AffineTransform3D.createScale(10, -2, 4);
+
+ // act
+ AffineTransform3D inverse = transform.getInverse();
+
+ // assert
+ double[] expected = {
+ 0.1, 0, 0, 0,
+ 0, -0.5, 0, 0,
+ 0, 0, 0.25, 0
+ };
+ Assert.assertArrayEquals(expected, inverse.toArray(), 0.0);
+ }
+
+ @Test
+ public void testGetInverse_undoesOriginalTransform_translationAndScale() {
+ // arrange
+ Vector3D v1 = Vector3D.ZERO;
+ Vector3D v2 = Vector3D.PLUS_X;
+ Vector3D v3 = Vector3D.of(1, 1, 1);
+ Vector3D v4 = Vector3D.of(-2, 3, 4);
+
+ // act/assert
+ runWithCoordinates((x, y, z) -> {
+ AffineTransform3D transform = AffineTransform3D
+ .createTranslation(x, y, z)
+ .scale(2, 3, 4)
+ .translate(x / 3, y / 3, z / 3);
+
+ AffineTransform3D inverse = transform.getInverse();
+
+ EuclideanTestUtils.assertCoordinatesEqual(v1, inverse.applyTo(transform.applyTo(v1)), EPS);
+ EuclideanTestUtils.assertCoordinatesEqual(v2, inverse.applyTo(transform.applyTo(v2)), EPS);
+ EuclideanTestUtils.assertCoordinatesEqual(v3, inverse.applyTo(transform.applyTo(v3)), EPS);
+ EuclideanTestUtils.assertCoordinatesEqual(v4, inverse.applyTo(transform.applyTo(v4)), EPS);
+ });
+ }
+
+ @Test
+ public void testGetInverse_nonInvertible() {
+ // act/assert
+ GeometryTestUtils.assertThrows(() -> {
+ AffineTransform3D.of(
+ 0, 0, 0, 0,
+ 0, 0, 0, 0,
+ 0, 0, 0, 0).getInverse();
+ }, NonInvertibleTransformException.class, "Transform is not invertible; matrix determinant is 0.0");
+
+ GeometryTestUtils.assertThrows(() -> {
+ AffineTransform3D.of(
+ 1, 0, 0, 0,
+ 0, 1, 0, 0,
+ 0, 0, Double.NaN, 0).getInverse();
+ }, NonInvertibleTransformException.class, "Transform is not invertible; matrix determinant is NaN");
+
+ GeometryTestUtils.assertThrows(() -> {
+ AffineTransform3D.of(
+ 1, 0, 0, 0,
+ 0, Double.NEGATIVE_INFINITY, 0, 0,
+ 0, 0, 1, 0).getInverse();
+ }, NonInvertibleTransformException.class, "Transform is not invertible; matrix determinant is NaN");
+
+ GeometryTestUtils.assertThrows(() -> {
+ AffineTransform3D.of(
+ Double.POSITIVE_INFINITY, 0, 0, 0,
+ 0, 1, 0, 0,
+ 0, 0, 1, 0).getInverse();
+ }, NonInvertibleTransformException.class, "Transform is not invertible; matrix determinant is NaN");
+
+ GeometryTestUtils.assertThrows(() -> {
+ AffineTransform3D.of(
+ 1, 0, 0, Double.NaN,
+ 0, 1, 0, 0,
+ 0, 0, 1, 0).getInverse();
+ }, NonInvertibleTransformException.class, "Transform is not invertible; invalid matrix element: NaN");
+
+ GeometryTestUtils.assertThrows(() -> {
+ AffineTransform3D.of(
+ 1, 0, 0, 0,
+ 0, 1, 0, Double.POSITIVE_INFINITY,
+ 0, 0, 1, 0).getInverse();
+ }, NonInvertibleTransformException.class, "Transform is not invertible; invalid matrix element: Infinity");
+
+ GeometryTestUtils.assertThrows(() -> {
+ AffineTransform3D.of(
+ 1, 0, 0, 0,
+ 0, 1, 0, 0,
+ 0, 0, 1, Double.NEGATIVE_INFINITY).getInverse();
+ }, NonInvertibleTransformException.class, "Transform is not invertible; invalid matrix element: -Infinity");
+ }
+
+ @Test
+ public void testHashCode() {
+ // arrange
+ double[] values = new double[] {
+ 1, 2, 3, 4,
+ 5, 6, 7, 8,
+ 9, 10, 11, 12
+ };
+
+ // act/assert
+ int orig = AffineTransform3D.of(values).hashCode();
+ int same = AffineTransform3D.of(values).hashCode();
+
+ Assert.assertEquals(orig, same);
+
+ double[] temp;
+ for (int i=0; i<values.length; ++i) {
+ temp = values.clone();
+ temp[i] = 0;
+
+ int modified = AffineTransform3D.of(temp).hashCode();
+
+ Assert.assertNotEquals(orig, modified);
+ }
+ }
+
+ @Test
+ public void testEquals() {
+ // arrange
+ double[] values = new double[] {
+ 1, 2, 3, 4,
+ 5, 6, 7, 8,
+ 9, 10, 11, 12
+ };
+
+ AffineTransform3D a = AffineTransform3D.of(values);
+
+ // act/assert
+ Assert.assertTrue(a.equals(a));
+
+ Assert.assertFalse(a.equals(null));
+ Assert.assertFalse(a.equals(new Object()));
+
+ double[] temp;
+ for (int i=0; i<values.length; ++i) {
+ temp = values.clone();
+ temp[i] = 0;
+
+ AffineTransform3D modified = AffineTransform3D.of(temp);
+
+ Assert.assertFalse(a.equals(modified));
+ }
+ }
+
+ @Test
+ public void testToString() {
+ // arrange
+ AffineTransform3D a = AffineTransform3D.of(
+ 1, 2, 3, 4,
+ 5, 6, 7, 8,
+ 9, 10, 11, 12
+ );
+
+ // act
+ String result = a.toString();
+
+ // assert
+ Assert.assertEquals("[ 1.0, 2.0, 3.0, 4.0; "
+ + "5.0, 6.0, 7.0, 8.0; "
+ + "9.0, 10.0, 11.0, 12.0 ]", result);
+ }
+
+ @FunctionalInterface
+ private static interface Coordinate3DTest {
+
+ void run(double x, double y, double z);
+ }
+
+ private static void runWithCoordinates(Coordinate3DTest test) {
+ runWithCoordinates(test, -1e-2, 1e-2, 5e-3);
+ runWithCoordinates(test, -1e2, 1e2, 5);
+ }
+
+ private static void runWithCoordinates(Coordinate3DTest test, double min, double max, double step)
+ {
+ for (double x = min; x <= max; x += step) {
+ for (double y = min; y <= max; y += step) {
+ for (double z = min; z <= max; z += step) {
+ test.run(x, y, z);
+ }
+ }
+ }
+ }
+}
diff --git a/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/Point3DTest.java b/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/Point3DTest.java
index e1cd149..c5c41c0 100644
--- a/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/Point3DTest.java
+++ b/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/Point3DTest.java
@@ -182,6 +182,21 @@ public void testAdd() {
checkPoint(p2.add(Vector3D.of(0, 0, 1)), -4, -5, -5);
}

+ @Test
+ public void testApply() {
+ // arrange
+ AffineTransform3D transform = AffineTransform3D.identity()
+ .scale(2)
+ .translate(1, 2, 3);
+
+ Point3D p1 = Point3D.of(1, 2, 3);
+ Point3D p2 = Point3D.of(-4, -5, -6);
+
+ // act/assert
+ checkPoint(p1.apply(transform), 3, 6, 9);
+ checkPoint(p2.apply(transform), -7, -8, -9);
+ }
+
@Test
public void testHashCode() {
// arrange
diff --git a/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/Vector3DTest.java b/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/Vector3DTest.java
index b555e2b..619ed88 100644
--- a/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/Vector3DTest.java
+++ b/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/Vector3DTest.java
@@ -807,6 +807,21 @@ public void testLerp() {
checkVector(v1.lerp(v3, 1), 10, -4, 0);
}

+ @Test
+ public void testApply() {
+ // arrange
+ AffineTransform3D transform = AffineTransform3D.identity()
+ .scale(2)
+ .translate(1, 2, 3);
+
+ Vector3D v1 = Vector3D.of(1, 2, 3);
+ Vector3D v2 = Vector3D.of(-4, -5, -6);
+
+ // act/assert
+ checkVector(v1.apply(transform), 3, 6, 9);
+ checkVector(v2.apply(transform), -7, -8, -9);
+ }
+
@Test
public void testHashCode() {
// arrange




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Gilles (JIRA)
2018-11-09 12:30:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gilles updated GEOMETRY-14:
---------------------------
Fix Version/s: 1.0
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
Fix For: 1.0
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Gilles (JIRA)
2018-12-10 14:53:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16714824#comment-16714824 ]

Gilles commented on GEOMETRY-14:
--------------------------------

bq. get NUMBERS-80 sorted out

I'll take care of the fields rename, probably later today.

bq. when should a class not be a VALJO?

Whenever there is a reason for not complying with at least one of the ValJO requirements. ;-)
Otherwise, it is always possible to start with a ValJO and relax the requirements later on (at the cost of BC breaking perhaps) when a compelling use-case is described.
Using {{of}} factory methods is just one requirement, that is good to follow (because it allows overloading) whenever possible, even for non-ValJO classes.
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
Fix For: 1.0
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Matt Juntunen (JIRA)
2018-12-10 13:24:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16714718#comment-16714718 ]

Matt Juntunen commented on GEOMETRY-14:
---------------------------------------

This is [finally] almost done. I have all of the functionality and tests in place and I'm just polishing up the docs and making sure that the reports are clean. The code is not currently using the commons-numbers {{Quaternion}} class but I'm thinking I'll at least create a merge request so we can start reviewing this code while we get NUMBERS-80 sorted out. There's quite a bit here.

I do have one question, though: when should a class _not_ be a VALJO? I currently have most of the classes for this issue using the "of" VALJO factory methods but I'm not entirely sure if they should be VALJOs. For example, {{AxisAngleSequence}} contains enum references and primitive doubles. Should this be a VALJO or not? I want to decide this now so the rest of the library will be consistent.
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
Fix For: 1.0
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
ASF GitHub Bot (JIRA)
2018-12-11 02:30:00 UTC
Permalink
[ https://issues.apache.org/jira/browse/GEOMETRY-14?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16716013#comment-16716013 ]

ASF GitHub Bot commented on GEOMETRY-14:
----------------------------------------

darkma773r opened a new pull request #16: GEOMETRY-14: Adding Transform Classes
URL: https://github.com/apache/commons-geometry/pull/16


Adding AffineTransformMatrix?D, QuaternionRotation, and AxisAngleSequence classes. Removing previous Rotation class.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
Post by Matt Juntunen (JIRA)
AffineTransform?D Classes
-------------------------
Key: GEOMETRY-14
URL: https://issues.apache.org/jira/browse/GEOMETRY-14
Project: Apache Commons Geometry
Issue Type: New Feature
Reporter: Matt Juntunen
Priority: Major
Labels: pull-request-available
Fix For: 1.0
We should create AffineTransform?D classes that implement matrix-based affine transforms. They should have simple methods for creating translations, rotations, and scaling and calculating the inverse.
 
Pull Request #1: https://github.com/apache/commons-geometry/pull/14
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Loading...