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 <[email protected]> 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); }
