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 18b10ac7caf86b0401616b99fb38a7f0fc3c5a5c
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Wed Sep 16 19:53:59 2020 +0200
Enable optimization in `TranslationTransform` which were missed.
---
.../transform/AbstractLinearTransform.java | 40 +++++---
.../operation/transform/CopyTransform.java | 112 +++++++++------------
.../operation/transform/ProjectiveTransform.java | 9 +-
.../operation/transform/TranslationTransform.java | 32 +++++-
4 files changed, 112 insertions(+), 81 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 84335d7..1ca6a39 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
@@ -44,7 +44,7 @@ import org.opengis.util.FactoryException;
* </ul>
*
* @author Martin Desruisseaux (Geomatys)
- * @version 1.0
+ * @version 1.1
* @since 0.6
* @module
*/
@@ -58,6 +58,8 @@ abstract class AbstractLinearTransform extends AbstractMathTransform implements
* The inverse transform, or {@code null} if not yet created.
* This field is part of the serialization form in order to avoid rounding errors if
a user
* asks for the inverse of the inverse (i.e. the original transform) after deserialization.
+ *
+ * @see #inverse()
*/
volatile LinearTransform inverse;
@@ -126,7 +128,8 @@ abstract class AbstractLinearTransform extends AbstractMathTransform implements
}
/**
- * Creates the inverse transform of this object.
+ * Returns the inverse transform of this object.
+ * This method invokes {@link #createInverse()} when first needed, then caches the result.
*/
@Override
@SuppressWarnings("DoubleCheckedLocking") // Okay since 'inverse'
is volatile.
@@ -136,19 +139,7 @@ abstract class AbstractLinearTransform extends AbstractMathTransform
implements
synchronized (this) {
inv = inverse;
if (inv == null) {
- /*
- * Should never be the identity transform at this point (except during
tests) because
- * MathTransforms.linear(…) should never instantiate this class in
the identity case.
- * But we check anyway as a paranoiac safety.
- */
- if (isIdentity()) {
- inv = this;
- } else {
- inv = MathTransforms.linear(Matrices.inverse(this));
- if (inv instanceof AbstractLinearTransform) {
- ((AbstractLinearTransform) inv).inverse = this;
- }
- }
+ inv = createInverse();
inverse = inv;
}
}
@@ -157,6 +148,25 @@ abstract class AbstractLinearTransform extends AbstractMathTransform
implements
}
/**
+ * Invoked by {@link #inverse()} the first time that the inverse transform needs to be
computed.
+ */
+ LinearTransform createInverse() throws NoninvertibleTransformException {
+ /*
+ * Should never be the identity transform at this point (except during tests) because
+ * MathTransforms.linear(…) should never instantiate this class in the identity
case.
+ * But we check anyway as a paranoiac safety.
+ */
+ if (isIdentity()) {
+ return this;
+ }
+ final LinearTransform inv = MathTransforms.linear(Matrices.inverse(this));
+ if (inv instanceof AbstractLinearTransform) {
+ ((AbstractLinearTransform) inv).inverse = this;
+ }
+ return inv;
+ }
+
+ /**
* Returns the parameter descriptors for this math transform.
*
* @return {@inheritDoc}
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/CopyTransform.java
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/CopyTransform.java
index edf2ee7..c5c25e6 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/CopyTransform.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/CopyTransform.java
@@ -19,7 +19,6 @@ package org.apache.sis.referencing.operation.transform;
import java.util.Arrays;
import org.opengis.geometry.DirectPosition;
import org.opengis.referencing.operation.Matrix;
-import org.opengis.referencing.operation.NoninvertibleTransformException;
import org.apache.sis.referencing.operation.matrix.MatrixSIS;
import org.apache.sis.referencing.operation.matrix.Matrices;
@@ -36,7 +35,7 @@ import org.apache.sis.referencing.operation.matrix.Matrices;
* an {@link java.awt.geom.AffineTransform} for every 2D affine conversions.</p>
*
* @author Martin Desruisseaux (Geomatys)
- * @version 0.7
+ * @version 1.1
* @since 0.5
* @module
*/
@@ -326,72 +325,61 @@ final class CopyTransform extends AbstractLinearTransform {
}
/**
- * Creates the inverse transform of this object.
+ * Invoked by {@link #inverse()} the first time that the inverse transform needs to be
computed.
*/
@Override
- @SuppressWarnings("DoubleCheckedLocking") // Okay since
'inverse' is volatile.
- public LinearTransform inverse() throws NoninvertibleTransformException {
- LinearTransform inv = inverse;
- if (inv == null) {
- synchronized (this) {
- inv = inverse;
- if (inv == null) {
- /*
- * Note: no need to perform the following check at this point because
MathTransforms.linear(…)
- * should never instantiate this class in the identity case and
because we perform an
- * equivalent check later anyway.
- *
- * if (isIdentity()) {
- * inverse = this;
- * } else { ... }
- */
- final int srcDim = this.srcDim;
- final int dstDim = indices.length;
- final int[] reverse = new int[srcDim];
- Arrays.fill(reverse, -1);
- for (int i=dstDim; --i>=0;) {
- reverse[indices[i]] = i;
- }
- /*
- * Check if there is any unassigned dimension. In such case,
- * delegates to the generic ProjectiveTransform with a matrix
- * which set the missing values to NaN.
- */
- for (int j=srcDim; --j>=0;) {
- if (reverse[j] < 0) {
- final MatrixSIS matrix = Matrices.createZero(srcDim + 1, dstDim
+ 1);
- for (j=0; j<srcDim; j++) { // Okay to reuse 'j' since
the outer loop will not continue.
- final int i = reverse[j];
- if (i >= 0) {
- matrix.setElement(j, i, 1);
- } else {
- matrix.setElement(j, dstDim, Double.NaN);
- }
- }
- matrix.setElement(srcDim, dstDim, 1);
- inv = MathTransforms.linear(matrix);
- if (inv instanceof AbstractLinearTransform) {
- ((AbstractLinearTransform) inv).inverse = this;
- }
- inverse = inv;
- return inv;
- }
- }
- /*
- * At this point, we know that we can create the inverse transform.
- * If this transform is the identity transform or an anti-diagonal matrix
except last row
- * (e.g. matrix used for swapping axis order), then the old and new arrays
would be equal.
- */
- CopyTransform copyInverse = this;
- if (!Arrays.equals(reverse, indices)) {
- copyInverse = new CopyTransform(indices.length, reverse);
- copyInverse.inverse = this;
+ final LinearTransform createInverse() {
+ /*
+ * Note: no need to perform the following check at this point because MathTransforms.linear(…)
+ * should never instantiate this class in the identity case and because we
perform an
+ * equivalent check later anyway.
+ *
+ * if (isIdentity()) {
+ * inverse = this;
+ * } else { ... }
+ */
+ final int srcDim = this.srcDim;
+ final int dstDim = indices.length;
+ final int[] reverse = new int[srcDim];
+ Arrays.fill(reverse, -1);
+ for (int i=dstDim; --i>=0;) {
+ reverse[indices[i]] = i;
+ }
+ /*
+ * Check if there is any unassigned dimension. In such case,
+ * delegates to the generic ProjectiveTransform with a matrix
+ * which set the missing values to NaN.
+ */
+ for (int j=srcDim; --j>=0;) {
+ if (reverse[j] < 0) {
+ final MatrixSIS matrix = Matrices.createZero(srcDim + 1, dstDim + 1);
+ for (j=0; j<srcDim; j++) { // Okay to reuse `j` since the outer loop
will not continue.
+ final int i = reverse[j];
+ if (i >= 0) {
+ matrix.setElement(j, i, 1);
+ } else {
+ matrix.setElement(j, dstDim, Double.NaN);
}
- inverse = inv = copyInverse;
}
+ matrix.setElement(srcDim, dstDim, 1);
+ final LinearTransform inv = MathTransforms.linear(matrix);
+ if (inv instanceof AbstractLinearTransform) {
+ ((AbstractLinearTransform) inv).inverse = this;
+ }
+ return inv;
}
}
- return inv;
+ /*
+ * At this point, we know that we can create the inverse transform.
+ * If this transform is the identity transform or an anti-diagonal matrix except
last row
+ * (e.g. matrix used for swapping axis order), then the old and new arrays would
be equal.
+ */
+ CopyTransform copyInverse = this;
+ if (!Arrays.equals(reverse, indices)) {
+ copyInverse = new CopyTransform(indices.length, reverse);
+ copyInverse.inverse = this;
+ }
+ return copyInverse;
}
/**
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/ProjectiveTransform.java
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/ProjectiveTransform.java
index 7516f1f..5dc40ff 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/ProjectiveTransform.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/ProjectiveTransform.java
@@ -113,11 +113,14 @@ class ProjectiveTransform extends AbstractLinearTransform implements
ExtendedPre
*/
boolean isScale = true; // ScaleTransform accepts non-square
matrix.
boolean isTranslation = (numRow == numCol); // TranslationTransform is restricted
to square matrix.
+ final int lastColumn = numCol - 1;
for (int i=0; i<n; i++) {
- if (elt[i] != 0) {
+ final double v = elt[i];
+ if (v != 0) {
+ final int row = (i / numCol);
final int col = (i % numCol);
- isScale &= (i / numCol) == col;
- isTranslation &= (col == numCol - 1);
+ isScale &= row == col;
+ isTranslation &= (col == lastColumn) || (isScale && v == 1);
if (!(isScale | isTranslation)) {
return this;
}
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/TranslationTransform.java
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/TranslationTransform.java
index 2071d61..b149a0d 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/TranslationTransform.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/TranslationTransform.java
@@ -32,7 +32,7 @@ import org.apache.sis.util.ArraysExt;
* {@link org.apache.sis.internal.referencing.j2d.AffineTransform2D} should be used in such
case.</div>
*
* @author Martin Desruisseaux (Geomatys)
- * @version 1.0
+ * @version 1.1
*
* @see <a href="http://issues.apache.org/jira/browse/SIS-176">SIS-176</a>
*
@@ -75,6 +75,28 @@ final class TranslationTransform extends AbstractLinearTransform implements
Exte
}
/**
+ * Creates a transform as the inverse of the given transform.
+ */
+ private TranslationTransform(final TranslationTransform other) {
+ offsets = negate(other.offsets);
+ errors = negate(other.errors);
+ inverse = other;
+ }
+
+ /**
+ * Returns a new array with negative values of given array (can be {@code null}).
+ */
+ private static double[] negate(double[] array) {
+ if (array != null) {
+ array = array.clone();
+ for (int i=0; i<array.length; i++) {
+ array[i] = -array[i];
+ }
+ }
+ return array;
+ }
+
+ /**
* Constructs a translation transform from a matrix having the given elements.
* This constructors assumes that the matrix is square, affine and contains only
* translation terms (this is not verified).
@@ -296,6 +318,14 @@ final class TranslationTransform extends AbstractLinearTransform implements
Exte
}
/**
+ * Invoked by {@link #inverse()} the first time that the inverse transform needs to be
computed.
+ */
+ @Override
+ final LinearTransform createInverse() {
+ return new TranslationTransform(this);
+ }
+
+ /**
* {@inheritDoc}
*
* @return {@inheritDoc}
|