sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] 03/03: ConcatenatedTransform should check sooner if two transforma are inverse, since matrix multiplication will not tell us if matrices contain NaN values.
Date Sun, 16 Dec 2018 16:47:12 GMT
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

commit 04c4a7af2544a9ed9116e855b7ab0f2883ae35b0
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Sun Dec 16 17:06:33 2018 +0100

    ConcatenatedTransform should check sooner if two transforma are inverse, since matrix
multiplication will not tell us if matrices contain NaN values.
---
 .../org/apache/sis/coverage/grid/GridChange.java   | 11 ++--
 .../operation/transform/AbstractMathTransform.java |  5 +-
 .../operation/transform/ConcatenatedTransform.java | 63 +++++++++++-----------
 3 files changed, 45 insertions(+), 34 deletions(-)

diff --git a/core/sis-raster/src/main/java/org/apache/sis/coverage/grid/GridChange.java b/core/sis-raster/src/main/java/org/apache/sis/coverage/grid/GridChange.java
index ce3cdeb..d61aab8 100644
--- a/core/sis-raster/src/main/java/org/apache/sis/coverage/grid/GridChange.java
+++ b/core/sis-raster/src/main/java/org/apache/sis/coverage/grid/GridChange.java
@@ -164,11 +164,16 @@ public class GridChange implements Serializable {
     private static MathTransform path(final GridGeometry source, final CoordinateOperation
crsChange,
             final GridGeometry target, final PixelInCell anchor) throws NoninvertibleTransformException
     {
-        MathTransform tr = source.getGridToCRS(anchor);
+        MathTransform step1 = source.getGridToCRS(anchor);
+        MathTransform step2 = target.getGridToCRS(anchor);
         if (crsChange != null) {
-            tr = MathTransforms.concatenate(tr, crsChange.getMathTransform());
+            step1 = MathTransforms.concatenate(step1, crsChange.getMathTransform());
+        }
+        if (step1.equals(step2)) {                                          // Optimization
for a common case.
+            return MathTransforms.identity(step1.getSourceDimensions());
+        } else {
+            return MathTransforms.concatenate(step1, step2.inverse());
         }
-        return MathTransforms.concatenate(tr, target.getGridToCRS(anchor).inverse());
     }
 
     /**
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/AbstractMathTransform.java
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/AbstractMathTransform.java
index 73e5642..7681f2b 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/AbstractMathTransform.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/AbstractMathTransform.java
@@ -863,7 +863,10 @@ public abstract class AbstractMathTransform extends FormattableObject
      * implementation.
      *
      * <p>The default implementation always returns {@code null}. This method is ought
to be overridden
-     * by subclasses capable of concatenating some combination of transforms in a special
way.</p>
+     * by subclasses capable of concatenating some combination of transforms in a special
way.
+     * {@link LinearTransform} implementations do not need to override this method since
matrix multiplications
+     * will be handled automatically, and this method does not need to handle the {@link
#isIdentity()} and
+     * {@link #inverse()} cases.</p>
      *
      * @param  applyOtherFirst  {@code true} if the transformation order is {@code other}
followed by {@code this}, or
      *                          {@code false} if the transformation order is {@code this}
followed by {@code other}.
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 4135bb8..03c6867 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
@@ -195,36 +195,11 @@ class ConcatenatedTransform extends AbstractMathTransform implements
Serializabl
         if (tr1.isIdentity()) return tr2;
         if (tr2.isIdentity()) return tr1;
         /*
-         * If both transforms use matrix, then we can create
-         * a single transform using the concatenated matrix.
-         */
-        final Matrix matrix1 = MathTransforms.getMatrix(tr1);
-        if (matrix1 != null) {
-            final Matrix matrix2 = MathTransforms.getMatrix(tr2);
-            if (matrix2 != null) {
-                final Matrix matrix = Matrices.multiply(matrix2, matrix1);
-                if (Matrices.isIdentity(matrix, IDENTITY_TOLERANCE)) {
-                    return MathTransforms.identity(matrix.getNumRow() - 1);         // Returns
a cached instance.
-                }
-                /*
-                 * 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,
-                 * 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.
-                 */
-                if (factory != null) {
-                    return factory.createAffineTransform(matrix);
-                } else {
-                    return MathTransforms.linear(matrix);
-                }
-            }
-        }
-        /*
-         * Give a chance to AbstractMathTransform to returns an optimized object.
-         * Examples: Logarithmic versus Exponential transforms, PassThrouthTransform.
-         * We try both ways (concatenation and pre-concatenation) and see which way
-         * produce the shortest concatenation chain.
+         * Give a chance to AbstractMathTransform to return an optimized object. For example
LogarithmicTransform
+         * concatenated with ExponentialTransform can produce a new formula, PassThrouthTransform
may concatenate
+         * its sub-transform, etc. We try both ways (concatenation and pre-concatenation)
and see which way gives
+         * the shortest concatenation chain. It is not that much expensive given that must
implementations return
+         * null directly.
          */
         int stepCount = 0;
         MathTransform shortest = null;
@@ -249,12 +224,40 @@ class ConcatenatedTransform extends AbstractMathTransform implements
Serializabl
         }
         /*
          * If one transform is the inverse of the other, return the identity transform.
+         * We need to test this case before the linear transform case below, because the
+         * matrices may contain NaN values.
          */
         if (areInverse(tr1, tr2) || areInverse(tr2, tr1)) {
             assert tr1.getSourceDimensions() == tr2.getTargetDimensions();
             assert tr1.getTargetDimensions() == tr2.getSourceDimensions();
             return MathTransforms.identity(tr1.getSourceDimensions());          // Returns
a cached instance.
         }
+        /*
+         * If both transforms use matrix, then we can create
+         * a single transform using the concatenated matrix.
+         */
+        final Matrix matrix1 = MathTransforms.getMatrix(tr1);
+        if (matrix1 != null) {
+            final Matrix matrix2 = MathTransforms.getMatrix(tr2);
+            if (matrix2 != null) {
+                final Matrix matrix = Matrices.multiply(matrix2, matrix1);
+                if (Matrices.isIdentity(matrix, IDENTITY_TOLERANCE)) {
+                    return MathTransforms.identity(matrix.getNumRow() - 1);         // Returns
a cached instance.
+                }
+                /*
+                 * 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,
+                 * 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.
+                 */
+                if (factory != null) {
+                    return factory.createAffineTransform(matrix);
+                } else {
+                    return MathTransforms.linear(matrix);
+                }
+            }
+        }
         // No optimized case found.
         return null;
     }


Mime
View raw message