You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sis.apache.org by de...@apache.org on 2026/01/09 12:29:03 UTC
(sis) 02/02: Add a tolerance factor in `TranslatedTransform`. Fix a missing computation of inverse transform.
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 30bf8f0c405ee4f027a3a6dd4019060029a289b9
Author: Martin Desruisseaux <ma...@geomatys.com>
AuthorDate: Fri Jan 9 13:28:04 2026 +0100
Add a tolerance factor in `TranslatedTransform`.
Fix a missing computation of inverse transform.
---
.../sis/coverage/grid/TranslatedTransform.java | 70 ++++++++++++++--------
.../transform/AbstractLinearTransform.java | 62 ++++++++++---------
.../operation/transform/CopyTransform.java | 16 ++---
.../operation/transform/IdentityTransform.java | 1 -
.../operation/transform/ScaleTransform.java | 3 +-
.../operation/transform/TransformJoiner.java | 4 +-
.../operation/transform/TranslationTransform.java | 3 +-
7 files changed, 86 insertions(+), 73 deletions(-)
diff --git a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/grid/TranslatedTransform.java b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/grid/TranslatedTransform.java
index e7bbeb6ea0..9c4603b970 100644
--- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/grid/TranslatedTransform.java
+++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/grid/TranslatedTransform.java
@@ -27,6 +27,8 @@ import org.apache.sis.referencing.operation.matrix.MatrixSIS;
import org.apache.sis.referencing.operation.transform.MathTransforms;
import org.apache.sis.referencing.operation.transform.AbstractMathTransform;
import org.apache.sis.referencing.internal.shared.DirectPositionView;
+import org.apache.sis.util.Utilities;
+import org.apache.sis.util.internal.shared.Numerics;
import org.apache.sis.util.resources.Errors;
@@ -53,6 +55,11 @@ final class TranslatedTransform extends AbstractMathTransform {
*/
private final double offset;
+ /**
+ * Tolerance factor for deciding that a value was equal to {@link #offset}.
+ */
+ private final double tolerance;
+
/**
* The transform to apply after the translation.
*/
@@ -68,6 +75,7 @@ final class TranslatedTransform extends AbstractMathTransform {
private TranslatedTransform(final int dimension, final double offset, final MathTransform transform) {
this.dimension = dimension;
this.offset = offset;
+ this.tolerance = Math.abs(offset) * Numerics.COMPARISON_THRESHOLD;
this.transform = transform;
}
@@ -127,7 +135,9 @@ final class TranslatedTransform extends AbstractMathTransform {
final MathTransform fromCenter = MathTransforms.getFirstStep(gridGeometry.gridToCRS);
final MathTransform fromCorner = MathTransforms.getFirstStep(gridGeometry.cornerToCRS);
analyzing = analyzing.inverse();
- if (analyzing.equals(fromCenter) || analyzing.equals(fromCorner)) {
+ if (Utilities.equalsApproximately(analyzing, fromCenter) ||
+ Utilities.equalsApproximately(analyzing, fromCorner))
+ {
fromGrid = MathTransforms.getMatrix(fromCenter);
fallback = MathTransforms.getMatrix(fromCorner);
if (fromGrid == null) {
@@ -150,9 +160,11 @@ final class TranslatedTransform extends AbstractMathTransform {
final int j = Long.numberOfTrailingZeros(dimensionBitMask);
dimensionBitMask &= ~(1L << j);
double offset = fromGrid.getElement(j, translationColumn);
- if (Double.isNaN(offset) && fallback != null) {
- offset = fallback.getElement(j, translationColumn);
- if (Double.isNaN(offset)) continue;
+ if (Double.isNaN(offset)) {
+ if (fallback == null || Double.isNaN(offset = fallback.getElement(j, translationColumn))) {
+ zeroingGridIndex &= ~(1L << j);
+ continue;
+ }
}
offsets[j] = offset;
}
@@ -163,27 +175,29 @@ final class TranslatedTransform extends AbstractMathTransform {
* all this stuff. But the coordinates modified by `offsets` may have an impact on some terms
* in other rows.
*/
- final MatrixSIS translated = Matrices.copy(toGrid);
- translated.translate(offsets);
- do {
- final int j = Long.numberOfTrailingZeros(zeroingGridIndex);
- translated.setElement(j, translated.getNumCol() - 1, 0);
- zeroingGridIndex &= ~(1L << j);
- } while (zeroingGridIndex != 0);
- /*
- * Rebuild a chain of transforms from last step to first step, but with translations
- * before the affine transform in order to get NaN × 0 = 0 operations when possible.
- */
- final List<MathTransform> steps = MathTransforms.getSteps(crsToGrid);
- crsToGrid = MathTransforms.linear(translated);
- for (int dimension = offsets.length - 2; dimension >= 0; dimension--) {
- final double offset = offsets[dimension];
- if (offset != 0) {
- crsToGrid = new TranslatedTransform(dimension, offset, crsToGrid);
+ if (zeroingGridIndex != 0) {
+ final MatrixSIS translated = Matrices.copy(toGrid);
+ translated.translate(offsets);
+ do {
+ final int j = Long.numberOfTrailingZeros(zeroingGridIndex);
+ translated.setElement(j, translated.getNumCol() - 1, 0);
+ zeroingGridIndex &= ~(1L << j);
+ } while (zeroingGridIndex != 0);
+ /*
+ * Rebuild a chain of transforms from last step to first step, but with translations
+ * before the affine transform in order to get NaN × 0 = 0 operations when possible.
+ */
+ final List<MathTransform> steps = MathTransforms.getSteps(crsToGrid);
+ crsToGrid = MathTransforms.linear(translated);
+ for (int dimension = offsets.length - 2; dimension >= 0; dimension--) {
+ final double offset = offsets[dimension];
+ if (offset != 0) {
+ crsToGrid = new TranslatedTransform(dimension, offset, crsToGrid);
+ }
+ }
+ for (int i = steps.size() - 2; i >= 0; i--) { // Omit last step because it has been replaced.
+ crsToGrid = MathTransforms.concatenate(steps.get(i), crsToGrid);
}
- }
- for (int i = steps.size() - 2; i >= 0; i--) { // Omit last step because it has been replaced.
- crsToGrid = MathTransforms.concatenate(steps.get(i), crsToGrid);
}
}
}
@@ -226,7 +240,9 @@ final class TranslatedTransform extends AbstractMathTransform {
Matrix derivative = null;
if (derivate) {
final double[] coordinates = Arrays.copyOfRange(srcPts, srcOff, srcOff + getSourceDimensions());
- coordinates[dimension] -= offset;
+ if (Math.abs(coordinates[dimension] -= offset) < tolerance) {
+ coordinates[dimension] = 0;
+ }
derivative = transform.derivative(new DirectPositionView.Double(coordinates));
}
transform(srcPts, srcOff, dstPts, dstOff, 1);
@@ -256,7 +272,9 @@ final class TranslatedTransform extends AbstractMathTransform {
final int srcDim = getSourceDimensions();
final int stop = srcOff + numPts * srcDim;
for (int i = srcOff + dimension; i < stop; i+= srcDim) {
- srcPts[i] -= offset;
+ if (Math.abs(srcPts[i] -= offset) < tolerance) {
+ srcPts[i] = 0;
+ }
}
transform.transform(srcPts, srcOff, dstPts, dstOff, numPts);
}
diff --git a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/AbstractLinearTransform.java b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/AbstractLinearTransform.java
index 03f4c38c44..6921b185e6 100644
--- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/AbstractLinearTransform.java
+++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/AbstractLinearTransform.java
@@ -56,7 +56,7 @@ abstract class AbstractLinearTransform extends AbstractMathTransform implements
* @see #inverse()
*/
@SuppressWarnings("serial") // Most SIS implementations are serializable.
- volatile LinearTransform inverse;
+ private volatile LinearTransform inverse;
/**
* Constructs a transform.
@@ -130,38 +130,42 @@ abstract class AbstractLinearTransform extends AbstractMathTransform implements
* This method invokes {@link #createInverse()} when first needed, then caches the result.
*/
@Override
- @SuppressWarnings("DoubleCheckedLocking") // Okay since 'inverse' is volatile.
+ @SuppressWarnings("DoubleCheckedLocking") // Okay since `inverse` is volatile.
public LinearTransform inverse() throws NoninvertibleTransformException {
- LinearTransform inv = inverse;
- if (inv == null) {
+ @SuppressWarnings("LocalVariableHidesMemberVariable")
+ LinearTransform inverse = this.inverse;
+ if (inverse == null) {
synchronized (this) {
- inv = inverse;
- if (inv == null) {
- inv = createInverse();
- inverse = inv;
+ inverse = this.inverse;
+ if (inverse == null) {
+ setInverse(inverse = createInverse());
}
}
}
- return inv;
+ return inverse;
}
/**
* Invoked by {@link #inverse()} the first time that the inverse transform needs to be computed.
+ *
+ * @return the inverse transform computed on-the-fly.
+ * @throws NoninvertibleTransformException if the inverse transform cannot 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;
+ protected LinearTransform createInverse() throws NoninvertibleTransformException {
+ return MathTransforms.linear(Matrices.inverse(this));
+ }
+
+ /**
+ * Sets the inverse transform to the given value. This method should usually not be invoked directly,
+ * unless the caller needs to compute the inverse transform in a way different than the default.
+ *
+ * @param inverse the inverse transform to assign to this transform.
+ */
+ final void setInverse(final LinearTransform inverse) {
+ if (inverse instanceof AbstractLinearTransform) {
+ ((AbstractLinearTransform) inverse).inverse = this;
}
- return inv;
+ this.inverse = inverse;
}
/**
@@ -275,7 +279,7 @@ abstract class AbstractLinearTransform extends AbstractMathTransform implements
@Override
public final boolean equals(final Object object, final ComparisonMode mode) {
if (object == this) {
- return true; // Slight optimization
+ return true; // Slight optimization
}
if (object == null) {
return false;
@@ -315,12 +319,12 @@ abstract class AbstractLinearTransform extends AbstractMathTransform implements
*/
if (object instanceof AbstractLinearTransform) {
/*
- * If the 'inverse' matrix was not computed in any of the transforms being compared
- * (i.e. if 'this.inverse' and 'object.inverse' are both null), then assume that the
+ * If the `inverse` matrix was not computed in any of the transforms being compared
+ * (i.e. if `this.inverse` and `object.inverse` are both null), then assume that the
* two transforms will compute their inverse in the same way. The intent is to avoid
* to trig the inverse transform computation.
*
- * Note that this code requires the 'inverse' fields to be volatile
+ * Note that this code requires the `inverse` fields to be volatile
* (otherwise we would need to synchronize).
*/
if (inverse == ((AbstractLinearTransform) object).inverse) {
@@ -328,7 +332,7 @@ abstract class AbstractLinearTransform extends AbstractMathTransform implements
}
}
/*
- * Get the matrices of inverse transforms. In the following code 'null' is really the intended
+ * Get the matrices of inverse transforms. In the following code `null` is really the intended
* value for non-invertible matrices because the Matrices.equals(…) methods accept null values,
* so we are okay to ignore NoninvertibleTransformException in this particular case.
*/
@@ -336,7 +340,7 @@ abstract class AbstractLinearTransform extends AbstractMathTransform implements
try {
mt = inverse().getMatrix();
} catch (NoninvertibleTransformException e) {
- // Leave 'mt' to null.
+ // Leave `mt` to null.
}
try {
if (object instanceof LinearTransform) {
@@ -345,7 +349,7 @@ abstract class AbstractLinearTransform extends AbstractMathTransform implements
mo = Matrices.inverse((Matrix) object);
}
} catch (NoninvertibleTransformException e) {
- // Leave 'mo' to null.
+ // Leave `mo` to null.
}
return Matrices.equals(mt, mo, isApproximate ? Numerics.COMPARISON_THRESHOLD : 0, isApproximate);
}
diff --git a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/CopyTransform.java b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/CopyTransform.java
index edf8f01ffe..4202647372 100644
--- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/CopyTransform.java
+++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/CopyTransform.java
@@ -320,7 +320,7 @@ final class CopyTransform extends AbstractLinearTransform {
* Invoked by {@link #inverse()} the first time that the inverse transform needs to be computed.
*/
@Override
- final LinearTransform createInverse() {
+ protected 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
@@ -353,11 +353,7 @@ final class CopyTransform extends AbstractLinearTransform {
}
}
matrix.setElement(srcDim, dstDim, 1);
- final LinearTransform inv = MathTransforms.linear(matrix);
- if (inv instanceof AbstractLinearTransform) {
- ((AbstractLinearTransform) inv).inverse = this;
- }
- return inv;
+ return MathTransforms.linear(matrix);
}
}
/*
@@ -365,12 +361,10 @@ final class CopyTransform extends AbstractLinearTransform {
* 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(dstDim, reverse);
- copyInverse.inverse = this;
+ if (Arrays.equals(reverse, indices)) {
+ return this;
}
- return copyInverse;
+ return new CopyTransform(dstDim, reverse);
}
/**
diff --git a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/IdentityTransform.java b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/IdentityTransform.java
index 6cc5c137c2..b790ba1acd 100644
--- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/IdentityTransform.java
+++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/IdentityTransform.java
@@ -60,7 +60,6 @@ final class IdentityTransform extends AbstractLinearTransform {
*/
private IdentityTransform(final int dimension) {
this.dimension = dimension;
- inverse = this;
}
/**
diff --git a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/ScaleTransform.java b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/ScaleTransform.java
index c07899bdb7..9246f16465 100644
--- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/ScaleTransform.java
+++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/ScaleTransform.java
@@ -85,7 +85,6 @@ final class ScaleTransform extends AbstractLinearTransform implements ExtendedPr
factors[i] = 1 / other.factors[i];
numbers[i] = Arithmetic.inverse(other.numbers[i]);
}
- inverse = other;
numDroppedDimensions = 0;
}
@@ -313,7 +312,7 @@ final class ScaleTransform extends AbstractLinearTransform implements ExtendedPr
* Invoked by {@link #inverse()} the first time that the inverse transform needs to be computed.
*/
@Override
- final LinearTransform createInverse() throws NoninvertibleTransformException {
+ protected final LinearTransform createInverse() throws NoninvertibleTransformException {
if (numDroppedDimensions == 0) {
return new ScaleTransform(this);
}
diff --git a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/TransformJoiner.java b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/TransformJoiner.java
index b1ed65343c..5652422f6d 100644
--- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/TransformJoiner.java
+++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/TransformJoiner.java
@@ -828,10 +828,10 @@ valid: if (i >= 0 && i < steps.size()) {
* coefficients that `AbstractLinearTransform.inverse()` would have set to NaN, or may succeed
* where `AbstractLinearTransform.inverse()` would have throw an exception.
*/
- if (impl.inverse == null) try {
+ try {
final MathTransform inverse = multiply(tr2.inverse(), tr1.inverse());
if (inverse instanceof LinearTransform) {
- impl.inverse = (LinearTransform) inverse;
+ impl.setInverse((LinearTransform) inverse);
}
} catch (NoninvertibleTransformException e) {
/*
diff --git a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/TranslationTransform.java b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/TranslationTransform.java
index 4dbfac003b..8ad2304e47 100644
--- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/TranslationTransform.java
+++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/TranslationTransform.java
@@ -85,7 +85,6 @@ final class TranslationTransform extends AbstractLinearTransform implements Exte
offsets[i] = -other.offsets[i];
numbers[i] = Arithmetic.negate(other.numbers[i]);
}
- inverse = other;
}
/**
@@ -288,7 +287,7 @@ 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() {
+ protected final LinearTransform createInverse() {
return new TranslationTransform(this);
}