This is an automated email from the ASF dual-hosted git repository.
desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git
The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
new 7498ef3 More robust concatenation of transforms when some steps may
be dropping or adding a dimension.
7498ef3 is described below
commit 7498ef3fc02859c7cb2bc23c114554dac9829416
Author: Martin Desruisseaux <[email protected]>
AuthorDate: Thu May 7 17:19:50 2020 +0200
More robust concatenation of transforms when some steps may be dropping or
adding a dimension.
---
.../operation/transform/ConcatenatedTransform.java | 66 +++++++++++++++++++---
.../operation/transform/IdentityTransform.java | 10 +++-
.../transform/ConcatenatedTransformTest.java | 43 +++++++++++++-
3 files changed, 110 insertions(+), 9 deletions(-)
diff --git
a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/ConcatenatedTransform.java
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/ConcatenatedTransform.java
index 987429e..801919d 100644
---
a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/ConcatenatedTransform.java
+++
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/ConcatenatedTransform.java
@@ -37,6 +37,7 @@ import
org.apache.sis.internal.referencing.provider.GeocentricAffine;
import org.apache.sis.internal.referencing.WKTKeywords;
import org.apache.sis.internal.referencing.Resources;
import org.apache.sis.internal.system.Semaphores;
+import org.apache.sis.internal.system.Loggers;
import org.apache.sis.internal.util.Strings;
import org.apache.sis.util.Classes;
import org.apache.sis.util.ComparisonMode;
@@ -44,6 +45,7 @@ import org.apache.sis.util.Utilities;
import org.apache.sis.io.wkt.Convention;
import org.apache.sis.io.wkt.Formatter;
import org.apache.sis.io.wkt.FormattableObject;
+import org.apache.sis.util.logging.Logging;
import org.apache.sis.util.resources.Errors;
@@ -56,7 +58,7 @@ import org.apache.sis.util.resources.Errors;
* <p>Concatenated transforms are serializable if all their step transforms
are serializable.</p>
*
* @author Martin Desruisseaux (IRD, Geomatys)
- * @version 1.0
+ * @version 1.1
*
* @see
org.opengis.referencing.operation.MathTransformFactory#createConcatenatedTransform(MathTransform,
MathTransform)
*
@@ -182,7 +184,7 @@ class ConcatenatedTransform extends AbstractMathTransform
implements Serializabl
/**
* Tries to returns an optimized concatenation, for example by merging two
affine transforms
- * into a single one. If no optimized cases has been found, returns {@code
null}. In the later
+ * into a single one. If no optimized case has been found, returns {@code
null}. In the later
* case, the caller will need to create a more heavy {@link
ConcatenatedTransform} instance.
*
* @param factory the factory which is (indirectly) invoking this
method, or {@code null} if none.
@@ -240,6 +242,53 @@ class ConcatenatedTransform extends AbstractMathTransform
implements Serializabl
* If both transforms use matrix, then we can create
* a single transform using the concatenated matrix.
*/
+ final MathTransform concatenated = multiply(tr1, tr2, factory);
+ if (concatenated instanceof AbstractLinearTransform) {
+ /*
+ * Following code computes the inverse of `concatenated`
transform. In principle this is
+ * not necessary because `MathTransform.inverse()` would do that
computation itself when
+ * first needed. However if the matrices are not square (e.g. if a
transform is dropping
+ * a dimension) some information may be lost. By computing inverse
transform immediately
+ * as the concatenation of the inverse of individual transforms,
we use information that
+ * would otherwise be lost (e.g. the inverse of the transform
dropping a dimension may be
+ * a transform setting that dimension to a constant value, often
zero). Consequently the
+ * inverse transform here may have real values for coefficients
that `MathTransform.inverse()`
+ * would have set to NaN, or may succeed while
`MathTransform.inverse()` would have throw
+ * an exception. Even with square matrices, computing the inverse
transform now may avoid
+ * some rounding errors.
+ */
+ final AbstractLinearTransform impl = (AbstractLinearTransform)
concatenated;
+ if (impl.inverse == null) try {
+ final MathTransform inverse = multiply(tr2.inverse(),
tr1.inverse(), factory);
+ if (inverse != null) {
+ if (inverse.isIdentity()) {
+ return inverse;
+ }
+ if (inverse instanceof LinearTransform) {
+ impl.inverse = (LinearTransform) inverse;
+ }
+ }
+ } catch (NoninvertibleTransformException e) {
+ /*
+ * Ignore. The `concatenated.inverse()` method will try to
compute the inverse itself,
+ * possible at the cost of more NaN values than what above
code would have produced.
+ */
+
Logging.recoverableException(Logging.getLogger(Loggers.COORDINATE_OPERATION),
+ ConcatenatedTransform.class, "create", e);
+ }
+ }
+ return concatenated;
+ }
+
+ /**
+ * Returns a transform resulting from the multiplication of the matrices
of given transforms.
+ * If the given transforms does not provide matrix, then this method
returns {@code null}.
+ *
+ * @param factory the factory which is (indirectly) invoking this
method, or {@code null} if none.
+ */
+ private static MathTransform multiply(final MathTransform tr1, final
MathTransform tr2,
+ final MathTransformFactory factory) throws FactoryException
+ {
final Matrix matrix1 = MathTransforms.getMatrix(tr1);
if (matrix1 != null) {
final Matrix matrix2 = MathTransforms.getMatrix(tr2);
@@ -251,9 +300,9 @@ class ConcatenatedTransform extends AbstractMathTransform
implements Serializabl
/*
* NOTE: It is quite tempting to "fix rounding errors" in the
matrix before to create the transform.
* But this is often wrong for datum shift transformations
(Molodensky and the like) since the datum
- * shifts are very small. The shift may be the order of
magnitude of the tolerance threshold. Intead,
+ * shifts are very small. The shift may be the order of
magnitude of the tolerance threshold. Instead,
* Apache SIS performs matrix operations using double-double
arithmetic in the hope to get exact
- * results at the 'double' accuracy, which avoid the need for
a tolerance threshold.
+ * results at the `double` accuracy, which avoid the need for
a tolerance threshold.
*/
if (factory != null) {
return factory.createAffineTransform(matrix);
@@ -355,10 +404,13 @@ class ConcatenatedTransform extends AbstractMathTransform
implements Serializabl
}
/**
- * Returns all concatenated transforms, modified with the pre- and
post-processing required for WKT formating.
+ * Returns all concatenated transforms, modified with the pre- and
post-processing required for WKT formatting.
* More specifically, if there is any Apache SIS implementation of Map
Projection in the chain, then the
* (<var>normalize</var>, <var>normalized projection</var>,
<var>denormalize</var>) tuples are replaced by single
* (<var>projection</var>) elements, which does not need to be instances
of {@link MathTransform}.
+ *
+ * <p>This method is used only for producing human-readable parameter
values.
+ * It is not used for coordinate operations or construction of operation
chains.</p>
*/
private List<Object> getPseudoSteps() {
final List<Object> transforms = new ArrayList<>();
@@ -821,8 +873,8 @@ class ConcatenatedTransform extends AbstractMathTransform
implements Serializabl
/**
* Concatenates or pre-concatenates in an optimized way this transform
with the given transform, if possible.
- * This method try to delegate the concatenation to {@link #transform1} or
{@link #transform2}. Assuming that
- * transforms are associative, this is equivalent to trying the following
arrangements:
+ * This method tries to delegate the concatenation to {@link #transform1}
or {@link #transform2}.
+ * Assuming that transforms are associative, this is equivalent to trying
the following arrangements:
*
* {@preformat text
* Instead of : other → tr1 → tr2
diff --git
a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/IdentityTransform.java
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/IdentityTransform.java
index 6cce32a..0663f4c 100644
---
a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/IdentityTransform.java
+++
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/IdentityTransform.java
@@ -63,6 +63,13 @@ final class IdentityTransform extends
AbstractLinearTransform {
*/
private IdentityTransform(final int dimension) {
this.dimension = dimension;
+ /*
+ * This value is not used by `inverse()` method, but we set it anyway
because
+ * `ConcatenatedTransform.createOptimized(…)` checks if the value is
non-null.
+ * A null value would cause unwanted computation of inverse
transform(because
+ * `ConcatenatedTransform` computes it itself.
+ */
+ inverse = this;
}
/**
@@ -224,10 +231,11 @@ final class IdentityTransform extends
AbstractLinearTransform {
}
/**
- * Returns the inverse transform of this object, which is this transform
itself
+ * Returns the inverse transform of this object, which is this transform
itself.
*/
@Override
public LinearTransform inverse() {
+ // Avoid unnecessary access to `inverse` volatile field.
return this;
}
diff --git
a/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/ConcatenatedTransformTest.java
b/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/ConcatenatedTransformTest.java
index bf29c78..aebf03f 100644
---
a/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/ConcatenatedTransformTest.java
+++
b/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/ConcatenatedTransformTest.java
@@ -19,11 +19,13 @@ package org.apache.sis.referencing.operation.transform;
import org.opengis.util.FactoryException;
import org.opengis.referencing.operation.MathTransform;
import org.opengis.referencing.operation.TransformException;
+import org.opengis.referencing.operation.NoninvertibleTransformException;
import org.apache.sis.internal.referencing.j2d.AffineTransform2D;
import org.apache.sis.referencing.operation.matrix.Matrices;
import org.apache.sis.referencing.operation.matrix.Matrix4;
import org.apache.sis.test.DependsOn;
import org.junit.Test;
+import org.opengis.test.Assert;
import static org.opengis.test.Assert.*;
@@ -32,7 +34,7 @@ import static org.opengis.test.Assert.*;
* Tests the {@link ConcatenatedTransform} class.
*
* @author Martin Desruisseaux (Geomatys)
- * @version 1.0
+ * @version 1.1
* @since 0.5
* @module
*/
@@ -147,4 +149,43 @@ public final strictfp class ConcatenatedTransformTest
extends MathTransformTestC
assertEquals("Source dimensions", 3, transform.getSourceDimensions());
assertEquals("Target dimensions", 4, transform.getTargetDimensions());
}
+
+ /**
+ * Tests concatenation of transforms built from non-square matrices. The
transforms are invertible
+ * when taken separately, but the transform resulting from concatenation
can not be inverted unless
+ * {@link ConcatenatedTransform#createOptimized(MathTransform,
MathTransform, MathTransformFactory)}
+ * prepares in advance the inverse transform using the inverse of original
transforms.
+ *
+ * @throws NoninvertibleTransformException if a transform can not be
inverted.
+ */
+ @Test
+ public void testNonSquares() throws NoninvertibleTransformException {
+ final LinearTransform tr1 = MathTransforms.scale(8, 6, 0.5);
+ final LinearTransform tr2 = MathTransforms.linear(Matrices.create(4,
3, new double[] {
+ 2, 0, 0, // Scale first dimension.
+ 0, 3, 0, // Scale second dimension.
+ 0, 0, 5, // Set third dimension to a constant.
+ 0, 0, 1})); // Usual row in affine transforms.
+ /*
+ * Request for a transform going from 3D points to 2D points.
+ * Dropping a dimension is not a problem.
+ */
+ final MathTransform c = MathTransforms.concatenate(tr1, tr2.inverse());
+ Assert.assertMatrixEquals("Forward", Matrices.create(3, 4, new
double[] {
+ 4, 0, 0, 0, // scale = 8/2
+ 0, 2, 0, 0, // scale = 6/3
+ 0, 0, 0, 1}), MathTransforms.getMatrix(c), 0);
+ /*
+ * Following test is the interesting part. By inverting the transform,
we ask for a conversion
+ * from 2D points to 3D points. Without contextual information we
would not know which value to
+ * put in the third dimension (that value would fallback on NaN). But
with the knowledge that
+ * this concatenation was built from a transform which was putting
value 5 in third dimension,
+ * we can complete the matrix as below with value 10 in third
dimension.
+ */
+ Assert.assertMatrixEquals("Inverse", Matrices.create(4, 3, new
double[] {
+ 0.25, 0, 0,
+ 0, 0.5, 0,
+ 0, 0, 10, // Having value 10 instead of NaN is the main
purpose of this test.
+ 0, 0, 1}), MathTransforms.getMatrix(c.inverse()), 0);
+ }
}
