sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] branch geoapi-4.0 updated: Remove the package-private AbstractMathTransform.inverseOf(…) method. It has been retrofitted in tryConcatenate(…) instead.
Date Tue, 25 Dec 2018 22:08:28 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


The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
     new 9d2431d  Remove the package-private AbstractMathTransform.inverseOf(…) method.
It has been retrofitted in tryConcatenate(…) instead.
9d2431d is described below

commit 9d2431d891fceb3834d7003e167349659ae3a929
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Tue Dec 25 23:07:35 2018 +0100

    Remove the package-private AbstractMathTransform.inverseOf(…) method. It has been retrofitted
in tryConcatenate(…) instead.
---
 .../transform/AbstractLinearTransform.java         | 16 ++++---
 .../operation/transform/AbstractMathTransform.java | 52 +++++++---------------
 .../operation/transform/ConcatenatedTransform.java | 31 ++++---------
 .../operation/transform/PassThroughTransform.java  | 25 +++--------
 4 files changed, 42 insertions(+), 82 deletions(-)

diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/AbstractLinearTransform.java
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/AbstractLinearTransform.java
index ec4a01c..571ee02 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/AbstractLinearTransform.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/AbstractLinearTransform.java
@@ -22,6 +22,7 @@ import org.opengis.parameter.ParameterValueGroup;
 import org.opengis.parameter.ParameterDescriptorGroup;
 import org.opengis.referencing.operation.Matrix;
 import org.opengis.referencing.operation.MathTransform;
+import org.opengis.referencing.operation.MathTransformFactory;
 import org.opengis.referencing.operation.NoninvertibleTransformException;
 import org.apache.sis.referencing.operation.matrix.Matrices;
 import org.apache.sis.internal.referencing.provider.Affine;
@@ -29,6 +30,7 @@ import org.apache.sis.internal.referencing.Resources;
 import org.apache.sis.internal.util.Numerics;
 import org.apache.sis.util.ComparisonMode;
 import org.apache.sis.util.resources.Errors;
+import org.opengis.util.FactoryException;
 
 
 /**
@@ -110,13 +112,17 @@ abstract class AbstractLinearTransform extends AbstractMathTransform
implements
     }
 
     /**
-     * Returns {@code true} if this transform is the inverse of the given transform.
-     * If this method is unsure, it conservatively returns {@code false}.
+     * Returns an identity transform if this transform is the inverse of the given transform.
+     * If this method is unsure, it conservatively returns {@code null}.
      */
     @Override
-    final boolean isInverseOf(final MathTransform other) {
-        // Skip the check if the other transform is not linear.
-        return (other instanceof LinearTransform) && super.isInverseOf(other);
+    protected MathTransform tryConcatenate(boolean applyOtherFirst, MathTransform other,
MathTransformFactory factory)
+            throws FactoryException
+    {
+        if (other instanceof LinearTransform) {
+            return super.tryConcatenate(applyOtherFirst, other, factory);
+        }
+        return null;        // No need to compute the inverse if the other transform is not
linear.
     }
 
     /**
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 7681f2b..3ac113d 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
@@ -791,40 +791,15 @@ public abstract class AbstractMathTransform extends FormattableObject
     }
 
     /**
-     * Returns {@code true} if this transform is the inverse of the given transform.
+     * Returns {@code true} if {@code tr1} is the inverse of {@code tr2}.
      * If this method is unsure, it conservatively returns {@code false}.
+     * The transform that may be inverted is {@code tr1}.
      *
-     * <p>The default implementation invokes {@link #inverse()} and compares the transforms.
-     * Subclasses should override with a more efficient implementation if they can avoid
the
-     * call to {@link #inverse()} (unless that call is cheap).</p>
-     *
-     * @param  other  the transform that may be the inverse of this transform.
+     * @param  tr1  the transform to inverse.
+     * @param  tr2  the transform that may be the inverse of {@code tr1}.
      * @return whether this transform is the inverse of the given transform. If unsure, {@code
false}.
      */
-    boolean isInverseOf(final MathTransform other) {
-        return inverseEquals(this, other);
-    }
-
-    /**
-     * Returns {@code true} if {@code tr1} is the inverse of {@code tr2}.
-     * If this method is unsure, it conservatively returns {@code false}.
-     * This implementation delegates to {@link #isInverseOf(MathTransform)}
-     * if possible, or to the default implementation otherwise. The transform
-     * that may be inverted is {@code tr1}.
-     */
-    static boolean areInverse(final MathTransform tr1, final MathTransform tr2) {
-        if (tr1 instanceof AbstractMathTransform) {
-            return ((AbstractMathTransform) tr1).isInverseOf(tr2);
-        } else {
-            return inverseEquals(tr1, tr2);
-        }
-    }
-
-    /**
-     * Implementation of {@link #isInverseOf(MathTransform)} for arbitrary math transform.
-     * This method invokes {@link #inverse()} on {@code tr1}.
-     */
-    private static boolean inverseEquals(MathTransform tr1, final MathTransform tr2) {
+    static boolean isInverseEquals(MathTransform tr1, final MathTransform tr2) {
         if (tr1.getSourceDimensions() != tr2.getTargetDimensions() ||
             tr1.getTargetDimensions() != tr2.getSourceDimensions())
         {
@@ -846,7 +821,7 @@ public abstract class AbstractMathTransform extends FormattableObject
 
     /**
      * Concatenates or pre-concatenates in an optimized way this math transform with the
given one, if possible.
-     * A new math transform is created to perform the combined transformation.
+     * If an optimization is possible, a new math transform is created to perform the combined
transformation.
      * The {@code applyOtherFirst} value determines the transformation order as bellow:
      *
      * <ul>
@@ -858,15 +833,15 @@ public abstract class AbstractMathTransform extends FormattableObject
      *       <var>p</var> by {@code this} and then transforming the result by
{@code other}.</li>
      * </ul>
      *
-     * If no special optimization is available for the combined transform, then this method
returns {@code null}.
-     * In the later case, the concatenation will be prepared by {@link DefaultMathTransformFactory}
using a generic
-     * implementation.
+     * If no optimization is available for the combined transform, then this method returns
{@code null}.
+     * In the later case, the concatenation will be prepared by {@link DefaultMathTransformFactory}
using
+     * a generic implementation.
      *
-     * <p>The default implementation always returns {@code null}. This method is ought
to be overridden
+     * <p>The default implementation returns the identity transform if the other transform
is the inverse
+     * of this transform, or returns {@code null} otherwise. This method is ought to be overridden
      * 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>
+     * will be handled automatically, and this method does not need to handle the {@link
#isIdentity()} case.</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}.
@@ -882,6 +857,9 @@ public abstract class AbstractMathTransform extends FormattableObject
     protected MathTransform tryConcatenate(boolean applyOtherFirst, MathTransform other,
MathTransformFactory factory)
             throws FactoryException
     {
+        if (isInverseEquals(this, other)) {
+            return MathTransforms.identity(applyOtherFirst ? getTargetDimensions() : getSourceDimensions());
+        }
         return null;
     }
 
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 03c6867..d68f159 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
@@ -203,8 +203,10 @@ class ConcatenatedTransform extends AbstractMathTransform implements
Serializabl
          */
         int stepCount = 0;
         MathTransform shortest = null;
+        boolean inverseCaseTested = false;
         if (tr1 instanceof AbstractMathTransform) {
             final MathTransform optimized = ((AbstractMathTransform) tr1).tryConcatenate(false,
tr2, factory);
+            inverseCaseTested = true;
             if (optimized != null) {
                 stepCount = getStepCount(optimized);
                 shortest  = optimized;
@@ -212,6 +214,7 @@ class ConcatenatedTransform extends AbstractMathTransform implements Serializabl
         }
         if (tr2 instanceof AbstractMathTransform) {
             final MathTransform optimized = ((AbstractMathTransform) tr2).tryConcatenate(true,
tr1, factory);
+            inverseCaseTested = true;
             if (optimized != null) {
                 if (shortest == null || getStepCount(optimized) < stepCount) {
                     return optimized;
@@ -227,7 +230,7 @@ class ConcatenatedTransform extends AbstractMathTransform implements Serializabl
          * 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)) {
+        if (!inverseCaseTested && (isInverseEquals(tr1, tr2) || isInverseEquals(tr2,
tr1))) {
             assert tr1.getSourceDimensions() == tr2.getTargetDimensions();
             assert tr1.getTargetDimensions() == tr2.getSourceDimensions();
             return MathTransforms.identity(tr1.getSourceDimensions());          // Returns
a cached instance.
@@ -816,26 +819,6 @@ class ConcatenatedTransform extends AbstractMathTransform implements
Serializabl
     }
 
     /**
-     * Returns {@code true} if this transform is the inverse of the given transform.
-     * If this method is unsure, it conservatively returns {@code false}.
-     */
-    @Override
-    final boolean isInverseOf(final MathTransform other) {
-        final List<MathTransform> s1 = getSteps();
-        final List<MathTransform> s2 = MathTransforms.getSteps(other);
-        final int size = s1.size();
-        if (s2.size() != size) {
-            return false;
-        }
-        for (int i=0; i<size; i++) {
-            if (!areInverse(s1.get(size-1 - i), s2.get(i))) {
-                return false;
-            }
-        }
-        return true;
-    }
-
-    /**
      * 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:
@@ -866,7 +849,11 @@ class ConcatenatedTransform extends AbstractMathTransform implements
Serializabl
                 return create(transform1, candidate, factory);
             }
         }
-        return super.tryConcatenate(applyOtherFirst, other, factory);
+        /*
+         * Do not invoke super.tryConcatenate(applyOtherFirst, other, factory); the test
of whether 'this'
+         * is the inverse of 'other' has been done indirectly by the calls to 'createOptimized'.
+         */
+        return null;
     }
 
     /**
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/PassThroughTransform.java
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/PassThroughTransform.java
index 225585a..6f98355 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/PassThroughTransform.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/PassThroughTransform.java
@@ -638,23 +638,6 @@ public class PassThroughTransform extends AbstractMathTransform implements
Seria
     }
 
     /**
-     * Returns {@code true} if this transform is the inverse of the given transform.
-     * If this method is unsure, it conservatively returns {@code false}.
-     */
-    @Override
-    final boolean isInverseOf(final MathTransform other) {
-        if (other instanceof PassThroughTransform) {
-            final PassThroughTransform ps = (PassThroughTransform) other;
-            if (firstAffectedOrdinate == ps.firstAffectedOrdinate &&
-                numTrailingOrdinates  == ps.numTrailingOrdinates)
-            {
-                return areInverse(subTransform, ps.subTransform);
-            }
-        }
-        return false;
-    }
-
-    /**
      * Concatenates or pre-concatenates in an optimized way this transform with the given
transform, if possible.
      * This method applies the following special cases:
      *
@@ -799,7 +782,13 @@ public class PassThroughTransform extends AbstractMathTransform implements
Seria
                 }
             }
         }
-        return super.tryConcatenate(applyOtherFirst, other, factory);
+        /*
+         * Do not invoke super.tryConcatenate(applyOtherFirst, other, factory); we do not
want to test if this transform
+         * is the inverse of the other transform as it is costly and unnecessary.  If it
was the case, the concatenation
+         * of 'this.subTransform' with 'other.subTransform' done at the beginning of this
method would have produced the
+         * identity transform already.
+         */
+        return null;
     }
 
     /**


Mime
View raw message